Add FPRN (Fail-Proof Result Notification, aka IPN) support

I really needed FPRN functionality for a project so I jumped in and added this feature. There are also a few currency handling improvements.

I have emailed the maintainer to get this merged into the main branch but have not heard back yet.

In the mean time I have loaded this to my github account so feel free to pull a copy if you need one.

https://github.com/garethhallnz/commerce_dps (Use the failProof..... branch)

I will have to install kickstart and give it a go I simply don't have that error on my side.

Re #4

1) Something must have gone wrong when the patch was created - fixed

2) Added the update hook

3) Resolved Undefined index notices

Patch looks good at first glance but Ill check it out later this afternoon

After thinking about things I want to change one function's conditional statement

function commerce_dps_pxpay_get_transaction_ids($order_id) {
  $query = new EntityFieldQuery();
  $query->entityCondition('entity_type', 'commerce_payment_transaction')
    ->propertyCondition('order_id', $order_id, '=');
  $transactions = $query->execute();
  $transaction_ids = array();

  if ($transactions) {
    foreach ($transactions['commerce_payment_transaction'] as $transaction) {
      $transaction_ids[] = $transaction->transaction_id;
    }
  }
  
  return $transaction_ids;
}

To

  if (array_key_exists('commerce_payment_transaction', $transactions)) {
    foreach ($transactions['commerce_payment_transaction'] as $transaction) {
      $transaction_ids[] = $transaction->transaction_id;
    }
  }

I feel this is more robust and specific.

I see you have changed the doc block style of documenting, should we not be using PSR as much as we can?

I will create a new patch with the above (conditional) later. Or if you have the time when you get this feel free to do so.

I have been unable to apply the patch in #8, I keep on getting a white space error. I have tried setting the appropriate config options but with no luck.

I have created a new patch with the fix mentioned in #9 I also removed the @author for the doc blocks.

As for the documentation style ... I am more of a framework developer as apposed to a Drupal one, so I don't always keep up with Drupal's best practices.

So the documentation format I apply on all my code is PSR-2 and it lines up with phpDocumentor and for this the format should be

/**
  * [<description>]
  *
  * @param [Type] [name] [<description>]
  *
  * @return [Type] [<description>].
  */

I am not to fussed with the format we use it's simply habit on my side.

Ae we just waiting for the community to test this?

Yes I am

Ok it took some time but I finally figured out what it issue was. I some how got my branches mixed up. I have rerolled the patch please test.

Oops forgot one bit in patch 11 here is version 12, please test.

I tested the patch in production to night and seems to work fine for me, can anyone else confirm?

@grobot Any chance you can test the patch?

In order to make FPRN work I needed a uri to handle the request (From my understanding if FPRN is updating the order you do not have the POST request anymore as it could be an hour or 2 later). Even if you did have the POST I suspect Drupal security will reject the POST (sessions / cookies might have changed).

The above being the case I thought it silly to have 2 functions doing the same job (I like DRY). One the POST method and the other the menu callback method so I refactored the module to use one method.

I am not sure what you mean about the hook_update .... my patch already contains this? Search for commerce_dps_pxpay_update_7100()

I know its a lot of code that changed but it's a pretty major refactor and I am not sure how I could have avoided this? I know in some cases I have mixed formatting changes in .... my apologies. Next time around I'll do a separate commit / patch.

Loading the files in the info file. I guess it's not required but thought it's best practice? From Drupals module development documentation.

files (Optional)
Drupal now supports a dynamic-loading code registry. To support it, all modules must now declare any code files containing class or interface declarations in the .info file, like so:
name = Really Neat Widget
...
files[] = example.test
When a module is enabled, Drupal will rescan all declared files and index all the classes and interfaces that it finds. Classes will be loaded automatically by PHP when they are first accessed.

I rerolled the patch to remove white space. This time I did it from the command line as I think the GUI software was messing it up.

Ok I know it's been ages ... I have been bogged down with other work. I'm doing another commerce job with DPS so I'll commit the time to getting this patch sorted this time.

I have gone through my changes and removed all formatting changes so that should cut things down.

I hear what you are saying about the white space but no matter how I roll the patch if I strip the whitespace the patch fails to apply.

To be truthful I have not idea how to fix it.

git format-patch --ignore-all-space --minimal -1 HASH
git format-patch --ignore-all-space -1 HASH
git format-patch -w -1 HASH

git apply --stat 0001-refactor.patch
 commerce_dps_pxpay.inc     |   83 ++++++++++++++
 commerce_dps_pxpay.info    |    1 
 commerce_dps_pxpay.install |   13 ++
 commerce_dps_pxpay.module  |  254 +++++++++++++++++++++++++-------------------
 4 files changed, 239 insertions(+), 112 deletions(-)


git apply --check -v 0001-refactor.patch
Checking patch commerce_dps_pxpay.inc...
Checking patch commerce_dps_pxpay.info...
Checking patch commerce_dps_pxpay.install...
Checking patch commerce_dps_pxpay.module...
error: while searching for:
    // Return to the previous page when payment is canceled.
    'cancel_return'  => url('checkout/' . $order->order_id . '/payment/back/' . $order->data['payment_redirect_key'], array('absolute' => TRUE)),
    // Return to the payment redirect page for processing successful payments.
    'return' => url('checkout/' . $order->order_id . '/payment/return/' . $order->data['payment_redirect_key'], array('absolute' => TRUE)),
    // Specify the current payment method instance ID in the notify_url.
    'payment_method' => $payment_method['instance_id'],
  );

error: patch failed: commerce_dps_pxpay.module:45
error: commerce_dps_pxpay.module: patch does not apply

The above said here is a new patch to test thats is smaller

Just realised that when FPRN updates the order and related transactions it doesn't set the uid correctly.
Now for the most part it's not an issue as the order state is set correctly.

I have attached 2 screenshot with the issue.

Thats said the 2 issues I found are:
1. When using kickstart 2 and messages module the related products are not attached to the end user email.
2. When you are redirected back to the site after payment the page title is a 404 but the correct message is being displayed.

I guess I'll send the uid with the generated request using TxnData1 field and then load the user object when I process FPRN callback.

I am not sure this is the best way but it will work.

Do you have any other suggestions?

Yes that's what I thought FPRN does have the session and it think that is the cause. Great idea looking at the order to get the uid much better than my idea!

Will look at updating the patch on Monday.

So did some more work on this.

First I added the uid of the order to the transaction with

$transaction->uid = $order->uid;

It sort of worked. The uid did get set but you were right in that revisions didn't get this value.
It also didn't fix the email notification issue.

Next I thought loading the user and setting a session will do the trick.

I added to the FPRN callback

global $user;
$user = user_load($order->uid);
drupal_session_regenerate();

Now the uid is set on both transaction tables and email notification is working correctly. Yay

However the second issue still exists.

When you are redirected back to the site after payment the page title is a 404 but the correct message is being displayed

Any ideas why that is?
Maybe? https://drupal.org/node/1344012

Sorry for the long wait ... been prety busy. Truth be told I am not really sure how to fix the issue. I have been in touch with Ryan form Commerce Guys. This is what he had to say.

As far as making it work with user accounts goes, though, that's a tricky issue. I definitely wouldn't create a fake session for the payment processor using the order's account. I can't imagine DPS would scam its customers, but there are bound to be unintended side effects doing something like that. A better course of action would be for us to figure out through Commerce Kickstart, Commerce itself, or some other e-mail related module how to get tokens describing order details whose contents are dependent on the normal access control settings.

An alternative that may be worth pursuing is to hook into the various access requests to grant access specifically to that anonymous page request to view the contents of the order, thus building the proper text for the e-mail. This is what the Cart module does in Drupal Commerce, granting customers access to orders whose IDs are present in their sessions. I don't know that I'd use the session specifically, but maybe something like that. : ?

Anyone able to help?

Hi Chris & Gareth,

I needed to add FPRN for a client's site recently. Found this issue and used some (well, most) of your work to create a functioning module. I am attaching a patch - my first Drupal patch so hopefully all ok. Could you please check it out.

Gareth - your issue from comment #26 above is not an issue. In the logs the user will always be set to Anonymous when FPRN is used as the request to the site has come from a DPS server, and is therefore not logged in. The fact that they get passed on to checkout/{order_number}/complete and receive a page not found does not matter - in fact it's good as they shouldn't see the completion message. The transaction still gets processed just fine.

I have actually been using PX Pay 2.0 for this client which is a new implementation from DPS so I will create a new issue and patch to allow the use of this.

Thanks for your efforts on this.

My bad, had the old currency code list in there. This patch is better.

Thanks for your work.

I do think #26 is an issue.

Technically everything works in the back end but it's a pretty poor enduser experience.

The problem is when the FPRN transaction is processed anonymously Drupal immediately emails out the order confirmation / status. Since this happens while the user is anonymous and anonymous does not have access to the order information; the order information is not attached to the order email. So the enduser receive a purchase receipt with no product information on it.

I had a chat to Ryan (Core Commerce Developer) and he said none issues in #26 is a problem in the PayPal module. I have been a bit busy and so I haven't had the time but I was going to look at the PayPal module and refactor to match. I know the PayPal module handles the transaction a little different.

It might me worth pursuing?

Ah yes, I see what you mean now. I wasn't including any order details in my notification email but a quick run through on Commerce Kickstart shows the problem.

#1895418: Table with line items not included in the e-mail sent to regular user has some discussion around it.

It is related to the commerce_cart_summary view. Not recommended but when I disabled SQL rewriting on that view the information came through on the email ok. It seems to apply to any module that uses an IPN/FPRN approach to payment notifications.

Did they solve it in the Paypal module? I'll install and take a look.

Just FYI - tested the Paypal module and the problem persists. I'm almost inclined to say that the issue is outside the scope of this module. Possibly more related to Commerce Messages which is making use of that View (commerce_cart_summary).

I think it's fair to expect certain payment methods to anonymously notify Commerce on payment. Going to concentrate on implementing the other DPS methods but will keep an eye on this - it's an interesting issue.

Hi Chris

I have gone through and retested the patch and all seem to work. I have also had my original patch in production for over a year with no issues. I think it safe to make this as tested.

Which version of the module should this patch be applied to? I have the -dev release from Oct 17 and when I try to apply the patch I get this message:

Reversed (or previously applied) patch detected! Assume -R?

However, the version of the file being patched has no mention of FPRN, so I'm guessing the patch also includes some other changes to the releaased version of the module that were already applied in the -deve version. If that's the case, I'm guessing I should roll back to the last release and then apply the patch, but would appreciate some guidance before taking that step.

Thanks

David