Closed (fixed)
Project:
Ogone | Ubercart payment
Version:
6.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
3 Feb 2012 at 10:43 UTC
Updated:
7 May 2012 at 11:07 UTC
Jump to comment: Most recent file
Comments
Comment #1
bartezz commentedHi Pieter,
Thanx again for sharing this! I am testing it as we speak on a test location hooked up to a Ogone test account.
My first finding: This updated package kicks butt!
Possible future updates (list may be grow :) );
Payment method
When allowing users to choose a payment method on /cart/checkout it would be better to store this option as the payment option instead of the generic 'Ogone payment'.
Checkout validation
When one clicks the review order button on the checkout page and doesn't pass validation the form is reloaded with an error showing which fields didn't validate. All fields are prefilled with the data one has entered before except for the payment method.
Will test more to do a RTBTC more and possibly come up with a patch for the above improvement!
Many thanx once again!
Cheers,
Bartezz
Comment #2
bartezz commented@PieterDC; While I'm testing on my localhost I encountered the following;
Duplicate order emails to both admin and user
First I thought might be related to #1192018 Duplicate order notification e-mail, and duplicate stock decrement and this should be fixed in Ubercart D6.x-2.7 which I am using. (I reset all conditional actions)
But after some more testing I found out this is an uc_ogone specific bug. See, this only happens when one completes the payment in the Ogone environment and clicks the 'ok' button whilst Ogone is already redirecting the user (so the new page is already started loading). I think it's easy to duplicate. Duplicate order emails are not send out when a user patiently waits to be redirected automatically and doesn't click the ok button, and it also doesn't happen when a user clicks the ok button right away.
All orders stuck on 'payment authorized', follow up on http://drupal.org/node/1047358
Also, testing on my localhost all orders get the status 'payment authorized' and never reach 'completed'. I've noticed the changes in the patch added in #1047358 and was playing around with the code around line#812. If I do a watchdog call like so
$new_status = uc_order_state_default('payment_received'); watchdog('ogone', $new_status);The new status written to watchdog is 'ogone_authorized'. But 'ogone_authorized' isn't a ubercart status? I believe the $new_status should be 'payment_received'. I know some stuff has changed between ubercart 2.4 and 2.7 that messed up statusses for other modules as well.I've changed line #815 from $new_status = uc_order_state_default('payment_received'); to $new_status ='payment_received'; which makes the problem go away but I'm sure isn't a real solution. We need to find out why uc_order_state_default('payment_received'); returns the string 'ogone_authorized'....
Comment #3
bartezz commentedDigging deeper and deeper... and need help understanding this logic;
function uc_ogone_payment_callback handles all kinds of statusses, from ogone 0 to status 99 see Ogone.com. As you can see these are payment statusses. Now as far as I can see Ogone will only return ok upon status 5 and 9 and thus return to cart/ogone_return_ok which calls the uc_ogone_payment_callback() function. If any other status than 5 or 9 is returned Ogone will return to /cart/checout which doesn't run the uc_ogone_payment_callback() function at all?!
I was testing with card number 4111116666666666 which should return a 92 - payment uncertain but nothing was logged into the order, no messages were shown to the user, etc. And that's how I found out.
Is there any logic as to why this is?
EDIT:
I changed line #550 from
'EXCEPTIONURL' => url('cart/checkout, array('absolute' => TRUE)),to'EXCEPTIONURL' => url('cart/ogone_return_ok', array('absolute' => TRUE)),This will call the uc_ogone_payment_callback() function which in turn will show a message to the user, save an admin message to the order and actually chage the order status to 'Payment uncertain' instead of leaving it at 'In checkout'
Comment #4
bartezz commentedHi Pieter,
I've created a patch against your version in #1 (20111010_uc_ogone.zip). The changes it makes;
Remove calls to uc_order_state_default() in switch statements of uc_ogone_payment_callback()EDIT: Please review my patch in the next post.
Cheers
Comment #5
bartezz commentedRemoving calls to uc_order_state_default() caused all orders to be set to completed directly, which ofcourse is not the wanted effect so I removed these changes from the patch.
New version attached.
Comment #6
bartezz commentedHi PieterDC,
A bit of SEVERE testing and some calls to Ogone later.... I found out an important thing.
In the Ogone backend on the transaction feedback tab one can enter a Post Payment URL (right after the 'always online' reuqest tab)
This Post Payment URL has to be http://www.webshopexample.com/cart/ogone_return_ok
Attention: on i18n sites this will throw a 301 redirect and kill the post payment process by Ogone so i18n sites will need a url like http://wwww.webshopexample.com/en/cart/ogone/ok
Ofcourse if your ubercart shop is set up to send emails when an order status is updated via conditional actions it will send emails in the languages of the called link, in above example /en. But this is a whole other issue :) Patient customers that will wait for redirection back to the webshop after the payment process are redirected to the correct language set with the module's Accepturl
A patch to readme.txt has been attached!
Cheers
Comment #7
bartezz commentedHi Pieter,
I've also created a template to use instead of that crappy inline stuff :)
The following patch is rolled against your version in #1 and includes;
To those who like to theme the Ogone payment environment;
Copy the uc-ogone-secure-payment-form.tpl.php from the module's theme directory to your own theme directory and make the changes you want!
Cheers
PS. If using template instead by checking the option 'Use template' please empty all styling fields before doing so otherwhise you will possibly encounter a unknown order error because of incorrect SHA1 calculations!
Comment #8
robbertnl commentedCombined update here as well: http://drupal.org/node/1391626
#6 was already included in my patch as well in the readme file explaining you have to do this.
Language codes is also in the patch.
I renamed the function cart/ogone_return_ok, to a more meaningful name (correct me if i am wrong), See the above url for more information.
I planned to make a new version for this patch..
But now i see you also made some modifications. And it looks like we are both making fixes for the same problems. Maybe we can combine something?
Comment #9
bartezz commentedHi Robert
I tried both your releases and found Pieters' version a bit further advanced and didn't have some small bugs I encountered in yours. Maybe you can add the stuff that is missing in Pieter's via a patch based on #1?
Would be great to have it combined and released. The original module is not of any use as it is now...
Cheers
Comment #10
robbertnl commentedI will try to save some spare time to work on this. Besides i want Kees to agree with this.
Comment #11
bartezz commentedNice!
To be honest I'm not holding my breath when it comes to Kees' reaction. I've been using this module since D5 and have seen many patches sent in for both D5 and D6, some of which were mine (admittedly not the best) and hardly seen Kees respond to them. Let alone review and commit them.
I've been in touch with Kees a couple of times when I was still working for my former employer and we hired him to do some Drupal stuff and it was tough getting in touch with them even then. It seems he's overloaded with work, which might cause the lack of time for this module.
Maybe he would allow a co-maintainer who can review and commit the brilliant updates mentioned above!
Comment #12
robbertnl commentedKees told me i should already be a co-maintainer (as you can see on the front page).
I will update the dev version first, before promoting it as 6.x-1.10
Comment #13
bartezz commentedThat's good news!
Maybe a better option is to release a 2.0 version, I believe there are so many updates and new features this would allow for a 2.0 version...
Cheers
Comment #14
pieterdcGo Robbert !
Comment #15
robbertnl commentedBut like Kees i am also busy. I will try to make some time for this friday or next week
Comment #16
robbertnl commentedWork in progress for the 6.x-2.x.dev release, see http://drupal.org/node/1509728
Currently its my patched version, as i shared before. I am working on adding the modifications from here and will commit when ready.
Any help/explanation of the patches contributed here is offcourse welcome
Comment #17
robbertnl commentedPatches from here are included/merged now as well.
@PieterDC: I didn't understand where and how the brandselection should apear on the checkout. For me it wasn't working. Maybe i did something wrong? This is why i changed that code. Now i added a 'Select payment method' checkout pane, which should appear on your checkout page (if enabled in both ogone payment settings and if the pane is enabled).
Please review and add comments at http://drupal.org/node/1519734
Comment #18
robbertnl commented