Duplicate order notification e-mail, and duplicate stock decrement

Seeing as I was the original poster of #644538: Duplicate order notification e-mail, and duplicate stock decrement, I'd better subscribe to this new issue.

I certainly agree that the original issue had got very convoluted. Each e-mail I was being sent to tell me of another post to the thread was taking up no less than 750kb!

I'm glad to see we're zeroing in on a solution for this taxing issue. It's strange it's so elusive, when so many other commerce platforms seem to have no trouble with WPS. Sorry I've not had much time to try all the patches etc. Keep up the good work all!

Simplified in which way? And more important... what situations did you find the duplicates are being sent? By isolating those we might be able to get to a final patch, also adding the same tag as the spawned issue.

Have you test this with normal test gateway as well? I will try to get to try this and to follow the changes as well soon.

Subscribing

OK - Will try to test this tomorrow or Monday on a pre-live site with WPP and WPS. Presume the patch is applied with:

patch -p1 < xxx.patch

Since despite my name I'm not running Git!

Ok - I am testing this. Will keep you informed.

Subscribing. As my test failed, do you need any more information Longwave?

@sirleech, the latest patch is a slightly different approach, so please do try the latest patch, try with a fresh dev and reset your CAs again if those were modified.

Hi,

I don't have a test server. Is it advisable to test it on my live server?

subscribing and will test.

subscribe

sub

Subscribing to keep up with the thread, but will also be testing this on my site, and with the paypal sandbox, as duplicate e-mails and stock decreases are a deal breaker for me.

I don't really want to have to go through all the hassle of finding a new cart solution at this stage (still to go live), but I'm not willing to use UC with this issue, even though my stock levels will be very low (especially so as then if something "sells out" I might still have something in stock that nobody can buy until I "fix" the stock level in the store).

I trawled through the previous thread today and so I'm ready to back up my site and have a go at this another day when I get some time. Thanks for all the work you've done so far with this issue, guys.

subscribe

Great work longwave and nice patch.

Regarding the race condition...

> This does not deal with the locking issue where both the IPN and the user return at the exact same instant, but this should be somewhat mitigated by
> uc_cart_complete_sale() always loading the latest order data; the window in which this can occur is extremely short and seems very unlikely to occur in the real world.

I'd vote for using lock_acquire(...) to guard against this situation. If the lock cannot be obtained then just fall through and skip the sale complete logic. The request that got the lock first (IPN or user) will do all needed sale completion work. I'm sure some will say the lock is ugly but given that there are two possible work flows and that they can both happen at the same time (rare but possible) we need this kind of check unless I am missing something.

Looking at your patch I agree that this special case is very unlikely to occur. However, when it does happen to someone they will open another ticket and this issue will drag on some more and no one wants that :)

I ran full test today and think this is close to being fixed - one small problem remains So I used:

Drupal 6.22
Ubercart 6.x-2.x-dev (fresh install) with patch 1192018-4-duplicate-complete-sale.patch applied
Created a new product (subscription that gives user role subscriber)

Result:
1) Checkout and order completed flawlessly
2) No duplicate order or emails - emails sent to user and store correctly
3) User created and notified that they have been given the role 'subscriber"
4) Error returned to user: "/cart/checkout/review|0||Duplicate entry '' for key 2 query: INSERT INTO users (created) VALUES (1308591429) in /modules/user/user.module on line 328"

The order shows the correct sequence and is 'completed' so big step forward:
"Admin comments:
[-] Credit card charged: £10.00
[-] Customer granted user role Subscriber.
[-] Order created through website."

But no subscriber role is given. User is given role 'authenticated user' and can log-in but unable to view content with 'subscriber permissions'. Happy to help with further testing and think this has made a lot of progress - thank you!

Searching for a similar issue brings up this discussion - hope it might help creating a simple fix: http://drupal.org/node/363847

Also quick clarification: the error message is:

Duplicate entry '' for key 2 query: INSERT INTO users

which I believe means the code is trying to assign a role to the anonymous user (uid 0) rather than to the newly-created user that has bought a subscription / membership / download (depending on use-case)

Update - Further testing shows everything works perfectly if I buy a subscription/ download / membership etc as an authenticated user. So the above error message and the non-assigning of role only happens with an anonymous user - potential workaround is to disable checkout by anonymous user. Downside is that adds a lot of time / complexity for the user so less likely to proceed with purchase.

Anyone any ideas? I'm stuck - no UC and little PHP experience...

Managed to fix the error but not the bug. Tweaked the checkout config for anonymous users and error message now gone.

However the bug remains: Anonymous users are not being assigned their purchased role despite order being 'completed'. The workaround is to make sure a user is registered before checkout. If you want to replicate this, the steps I used are:

1) Redirect the cart (Store Admin -> Configuration -> Cart Settings) to a new path - I've called it 'cart_register'.
2) Create a new page node with path 'cart_register'. Whatever text etc you want in the body and then:
<?php print drupal_get_form('user_register'); ?>
3) Configure login_toboggan to log users in once registered and then redirect them to 'cart'
4) Ensure User Settings -> 'Only site administrators can create new user accounts.' is ticked to avoid them redirecting to cart etc & confusion - This is the downside of the method - make it harder to allow non-paying registration although suppose one could create a 'Free' registered user product.

Now all works fine. Anonymous user picks subscription / download / event product from product page ->register ->cart ->checkout ->review ->complete and is assigned the correct role. Emails are sent.

Would be more elegant if we can fix the code and I'll be talking to Longwave about this today - he's done some terrific work on this so hope its close to a fix, patch and release.

I just tested #4 with testing role assignment for anonymous checkouts. It still seems to be trying to grant the role for anonymous users. I added some debug output for uc_roles_action_order_renew() screenshot: https://skitch.com/smithmilner/fgmn2/recent-log-entries-social-media-club

Edit: tests on #3 confirm the same thing.

patch #328 here http://drupal.org/node/644538?page=1 worked for me! The user was created with the appropriate role.

subscribing

Skirty, deal breaker in what way? surely there are workarounds for every situation. For example change the rule so that it triggers on a different event that will only decrement your stock once i.e accurately. In my case simply changing to

EVENT: Order status gets updated
CONDITION: Data comparison, Parameter: Data to compare: [updated-order:order-status], Data value: Payment received
ACTION: Decrement stock of products on the order with tracking activated, Parameter: Order: [order] (unmodified)

... maybe that won't work for you, depending on what methods of payment you accept, but the point is I don't see how one or more rules combined could not solve this problem for anyone, though I'm using D7, so perhaps it is more flexible with workflow than D6?

Besides, you can allow customers to go through checkout even if stock is zero or less, or am I missing something here?

@smithmilner, what payment method are you using?

Ok , just tried it (ubercart dev with #4) with a normal CC and test gateway purchasing with an anonymous and File downloads are not properly assigned to the customer. This is the same as #24 because roles and file downloads works very very similar.

I kind of feel this is a step back to around comment #250 in the previous issue.

Longwave, I have a few questions on this patch, I will explain CAs below.

...since #617560: Anonymous checkout, order assigned to existing account without validation removed the ability to use an existing email address without logging in...

Bearing in mind that I will soon provide a patch to have that configurable, how is that patch (or functionality) related to this and how it simplifies your patch?

...but after some testing with PayPal WPS, it still needs work as duplicates are still sent in some situations...

We already briefly discussed this on skype , but it might be really useful to isolate the use cases.

Switches the order of uc_cart_complete_sale() and uc_payment_enter() in uc_paypal_ipn() so new customers can have roles and file downloads assigned correctly.

I don't like this approach, although it may work, it would make a lot more robust the code if we can cope with uc_cart_complete_sale() being called regardless of when the payment is received (or entered).

Now, into the actual CAs.

On payment received, sets order status to payment received if it is in checkout or post checkout.

I think this is actually the cause of this file & role assignment problem.

On a normal purchase workflow you do checkout -> payment received (in_checkout) -> checkout complete -> postcheckout

So this will trigger a payment received which in the end will trigger a status update event in which you are completing non-shippable orders and the user is not created at that point.

There has to be a -> complete status change on non shippable orders in the checkout completion trigger, otherwise we will always brake the normal workflow of a purchase.

I guess doing this will add one CA to what you currently have, not 100% sure, but this moves it very close to my patch, which I think added even one more CA, but allow the payment to be entered before or after the call to checkout complete sale.

...since #617560: Anonymous checkout, order assigned to existing account without validation removed the ability to use an existing email address without logging in...

I kind of answered this myself, but add to this if needed. I noticed you removed all the existing_user message (both admin and on checkout), again, please don't do this, or maybe don't do it now, hold it off, will soon (in the next 30 minutes) submit a patch to put this functionality back in with a configuration option.

EDIT: Patch already submitted.

sub...

subscribing. will try some patch.

Subscribing

I only get the duplicate emails and decremented twice when someone clicks the return to cart button after the payment is processed with PayPal.

Can we commit this patch to the Dev branch?

Hi all,

Just bumping this up as its quite important (to my client at least).

Which patch is currently the recommended one? When will it be added to the product?

Thanks!!

@ratinakage I suggest you trying #328 here http://drupal.org/node/644538?page=1#comment-4567732 and comment on this.

Ideally we should create a patch with a mix on both, but I am positive the one here needs work, although I'd like to hear your feedback from the one I just linked you here.

Thanks hanoii!

Could someone please post a patched version of Ubercart 4.2 patched with #328 for download? I am running Windows with no good way of applying patches (usually if they are small I just apply them by hand).

@ratinakage on http://drupal.org/node/707484 there are some notes for windows, but there should be a proper patch tool for win as well..

what about http://gnuwin32.sourceforge.net/packages/patch.htm?

Patch tested with WPS .good results for stock and no invoice sent twice. However Coupons keep being send and created twice.
Should I try CA for the coupons? Even as a temporary solution? ( I use a lot of them and may potentientally lost lots of money..

@Will - What do you mean coupons get used twice? Do you mean it applies the discount twice making the final price even cheaper?

@ ratinakage no this have been an issue but was fixed in the last version of coupon , my issue is different.
after some pruchase at my store a customer has a purchased coupons attached but instead of having one purchased coupons sent ,the number of coupons created and sent are multiplied by 2.

Oh - gocha!

@Will Igetit I think, as you mentioned CA, this has to be related to your CAs. The patch suggested changed the default CAs which will probably need you to adapt the coupon CA as well. I wouldn't recommend any particular way until this patch gets to a final acceptance and gets committed, because the CAs might change from one patch to another, but at least, the issue related to the core functionality seems to have worked fine for you.

Subscribe.

Subscribe

Subscribe

Subscribing

Just reporting that I've been using the patch from #4: http://drupal.org/files/issues/1192018-4-duplicate-complete-sale.patch for nearly a month now and it appears to have fixed the problems of duplicate order notification e-mail and duplicate stock decrement for me with no side effects. I am using PayPal, bank transfer and cheque as payment methods and all have been working fine.

PayPal Website Payments Standard?

Subscribe.

Yes. PayPal Website Payments Standard.

Thanks for the help RogueM, and sorry for appearing to ignore your reply, I've been away. My site will be selling things that are handmade, some take over a week for me to make as I also look after my toddler all day, and so I don't really want to be selling millions of items that I haven't made yet, too much pressure! I'm using a module to prevent the purchase of items when the stock level is 0.

I've just applied the patch in #4 so time to test. My site isn't live yet, so I have plenty opportunity to test this. If it doesn't work I will play with the conditional actions as you suggested, but I'd rather not have to do this as it may mean tweaking every time there's an update to UC.

ETA: I'm only accepting paypal also, so this testing may not actually help, since others appear to have well tested with paypal successfully.

My first test appears to have failed. This was with the dev version of UC patched with #4, using paypal website payments standard. Doubly decreasing stock. I'll check my conditional actions and see if I had anything funny in there, but apart from that the patch has made no difference for me! I also still received two "your order was successful" e-mails:

Admin comments:

Date User Comment
16/07/2011
11:53:38 AM - The stock level for CARD-IT-06 has been decreased to 1.
16/07/2011
11:53:39 AM - PayPal IPN reported a payment of 10.82 EUR.
16/07/2011
11:53:41 AM - The stock level for CARD-IT-06 has been decreased to 0.
16/07/2011
11:53:42 AM - Order created through website.

I got mad at this yesterday as I didn't understand why it worked for others but not me. So I did it again properly today, and applied the patch to a clean install on a different webspace on my server. It worked. So I started adding in modules that I use one by one so that I could identify where the issue was happening.

This fellow here http://drupal.org/project/uc_out_of_stock appears to be causing all my trouble now. As soon as I enabled this, stuck some attributes on my stock and put orders through, I started experiencing the issue again. I'm unsure whether to log a new issue for that or whether I should just not use it for now.

Any advice is appreciated - the duplicate customer e-mails are not being sent now, but the stock is deprecating twice. Looking at the conditional actions they appear to be fired by the same trigger so I'm unsure why one is only firing once and one twice.

ETA the final paragraph is not true, I've just discovered that the duplicate mails are also still happening with this module enabled.

@Skirty, I've done uc_out_of_stock and maintain it (not as actively as I'd like, I know) but.. after reading what you just posted and although I was almost sure this module shouldn't affect the workflow I went and quickly check on the codebase once more, and I really don't see how that module might be affecting you. Can you make sure of this. You mentioned about adding stock for attributes after enabling that module, can you disable it and play around with your site a bit more, and see if you cannot definitely get this duplicate error ever w/o that module and consistently getting it with it enabled.

Just thought I'd mention that I use uc_out_of_stock module but have not had a problem. As I said in comment #49, everything is fixed for me so far using patch from #4.

Thanks to both of you, I'll mess around with it and try to figure out what's causing it to behave this way. I can start another clean install in the next couple of days when I get some time and see how it goes.

@rectangle Have you enabled IPN with Paypal ?

Well I'm far from an expert and this is the first Ubercart store I've set up and the first time I've used PayPal but I can tell you that I've not every enabled IPN or entered a Notification URL but I do see "PayPal IPN reported a payment of 234.00 AUD." etc. in my Admin comments. So you tell me...? Anyway, I was occasionally getting duplicate emails and stock decrement but not since using the patched module.

That means IPN is on, whether you intended or not ;-)

I am occasionally getting duplicate emails and stock decrement, that's why I turned off Paypal IPN (notifications) and customer mails. But obviously, that's not ideal. So I'm gonna try the patch.

Did you apply it against 6.x-2.x-dev or 6.x-2.4 ?

6.x-2.x-dev

PieterDC - did that just mean you had to manually update the orders to "paid", going by your paypal account? This could be a solution for me if I can't get to the bottom of my own issue (as I have low volume, but I'm concerned that customers might think their order hasn't gone through).

I started afresh with another test site today.

Drupal 6.22
Ubercart 6.x.dev patched with 1192018-4-duplicate-complete-sale.patch
Token

Those are all the modules I have installed. I enabled the stock module but didn't set it up for any item for the first order. I configured Paypal WPS.

First order admin comments:

07/19/2011
8:46:53 PM - PayPal IPN reported a payment of 15.00 EUR.
07/19/2011
8:46:55 PM - Order created through website.

Great, all fine so far!

Then I went into my products and enabled them for stock. I added a quantity of stock for each and put through another order.

Second order admin comments:

07/19/2011
8:53:43 PM - The stock level for test2 has been decreased to 8.
07/19/2011
8:53:43 PM - The stock level for test1 has been decreased to 4.
07/19/2011
8:53:44 PM - PayPal IPN reported a payment of 55.00 EUR.
07/19/2011
8:53:46 PM - The stock level for test2 has been decreased to 6.
07/19/2011
8:53:46 PM - The stock level for test1 has been decreased to 3.
07/19/2011
8:53:48 PM - Order created through website.

So as I haven't even enabled the module that I thought was causing the problem it seems that the patch hasn't fixed it - or my patching process didn't work. What's the best way of checking that? When I patched I installed GIT locally on my machine, I used that to commit the changes etc to a local copy of the dev version of Ubercart, then I uploaded the patched version (I don't have terminal access to my server) via ftp. It certainly looked like some of the files were changed, due to the timestamps etc, but maybe I didn't do this correctly? Also the dev version of UC I used was from last week, and the patch was created a while ago, so perhaps the patch is working on lines that aren't there in my version/are in a different place? There's also a new version of the dev version of UC from today, and would the patch work on that? (Please pardon my ignorance - I'm keen to help get this fixed and I'm not a coder).

@Skirty, I have a low volume shop too, setting orders to 'completed' manually within Ubercart. At Paypal's side everything seems fine.

Thanks PieterDC. Rectangle has provided me with his patched version of UC so I'll compare that to my own and try it out this evening when I get a chance. If it doesn't work I might just go down that route!

ETA: Rectangle, thanks very much for this. Fingers crossed it's going to show that my issue all along was with the version of UC I started with.

I managed to sneak in a few hours on the laptop today somehow. The patched UC from Rectangle works with stock for me, still to test it with the out of stock mod, and with attributes, but this is much better, as the possibility of doing this was what drove me away from more basic e-commerce solutions and towards the fun that is Drupal with UC.

Thanks for passing that one on to me, Rectangle. The patch needs to be updated for the current dev version, or committed to the dev version, otherwise others won't be able to test. I still need to conduct some more rigorous testing, but I just wanted to post that my progress so far is positive.

subscribe

subscribe

subscribe

Same issue. Subscribe. Can't apply patch as my site is live, but after some testing should be able to test it out.

Any update on this in light of the new release of Ubercart 2.6?

Subscribing

@reisler I didn't see any mention of this related to the release of 2.6. Additionally this change to conditional actions has a large potential to mess up existing sites. In my opinion, this fix should be released in a version by itself, or a major version change, so that people know they are going to have to do some conversions of their existing Conditional actions. Usually this would involve disabling your existing actions. Enabling the new ones. Moving your custom actions from the old CA's to the new ones. It will not be a small change for people who rely on conditional actions.

Subscribing

I've been authorized by my client to provide a $400 bounty to either @longwave or @hanoii if we can get a solution which works by next Tuesday, so that I can implement it then.

I won't have time to implement the fix I wanted to do until next week and also I don't want to create a fix, which will be in-compatible with the future working fix and probably not compatible for everyone else's installs.

I posted up the bounty here:
http://www.ubercart.org/forum/bounties/22507/fix_duplicate_order_notific...

If either @longwave or @hanoii who have been working on this would like to take it up, please make it known in this thread. PM me your email address and I'll have the client pay you via paypal directly.

I think we should be close to a fix, I might also be up for this bounty as well, but you are definitely better off with @longwave as he's current an ubercart dev so this moves it faster for any fix to be accepted/included/committed.

I am happy with the new release and I will have to soon check if the patch attached in the first issue still applies but I would think that should be a good starting point.

Actually jOrd I wonder if you had a chance to try the latest patch, it's linked from the first post on this issue.

will soon try it, have you tried it with a normal test gateway?

@longwave, you are correct. Digital products (uc_file) should be sent out when an order is paid (payment received). In my opinion all the logic related to a sale needs to get done upon full payment received and we need to make sure this only gets called once per order.

Normal work flow for someone running a store typically works like this

• in checkout (mildly irrelevant to store owner)
• payment pending (special case when waiting for a payment or have a partial payment, rare)
• payment received
• shipment pending
• completed

Of course you skip "shipment pending" for pure digital orders, but mixed orders still need to be shipped and should be left in this state.

As I stated above I believe most logic needs to get completed on full "Payment Received".

The "shipment pending" state lets the store owner know they need to ship some physical products, then they can move the order to completed manually.

The "completed" state, to the store owner means, I'm done with this order, I have the money, the items have been sent. I never ever ever, have to look at this again. Perhaps it would be a good idea to create an automated message which gets sent to the customer notifying them that their order has been shipped. Personally, I believe this is something that can be done manually via order comments when the store owner manually changes the state, as they most likely need to include shipment tracking codes as well. Since these are not well handled in Ubercart, it usually needs to get done manually anyways.

If we're going to change pure digital orders to state = completed in the "Payment Received" conditional action, we need to make sure that have the absolute heaviest weight, so that site builders or store owners can not put another conditional action after it. If they need to, they can manually change the weight of this CA. If they're smart (or stupid) enough to do this, they can suffer the consequences.

@hanoii
My problem exists on a live site. We've disabled Paypal for now. I need to update the site to the most recent version of Ubercart and then re-configure my conditional actions. I don't have time to do this until Tuesday, which is why I was hoping someone could resolve my issue before then.

I'm testing this with anonymous checkout / role assignment in mind. Just tested #85 on our develop site with Paypal's sandbox. Paypal's IPN failed validation for some reason but I don't think it's related (it was doing that before) and the order was stuck in the "pending" status. Something to note, the customer's user account was created and without a role while the order was stuck in pending. I manually changed the order status to "payment received" and the user was assigned the role instantly which is a positive sign.

https://skitch.com/smithmilner/fpifm/order-11729-social-media-club

I've tested with my development store pointed at the Live Paypal API and #85 seems to have resolved the duplicate order notification issue.

Notes:
- I have only 1 product that is shippable
- Users use both anonymous and authenticated checkout (both work fine)
- Using Drupal 6.22 and Ubercart 6.x-2.6

I noticed one issue..
1. Add item to cart
2. Checkout using credit card (Using Paypal WPP for payment processing)
3. After completing checkout the user is given an error (paraphrased) "Unable to complete your order at this time"

I noticed that when the patch is applied I have 302 Redirects during checkout, where without it it's a 200.

Patch Applied to 6.x-2.6:
107.3.184.250 - - [05/Aug/2011:09:59:33 -0700] "GET /cart/checkout/complete HTTP/1.1" 302 - "https:///cart/checkout/review" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/534.50 (KHTML, like Gecko) Version/5.1 Safari/534.50"

Vanilla 6.x-2.6
107.3.184.250 - - [05/Aug/2011:10:07:25 -0700] "GET /cart/checkout/complete HTTP/1.1" 200 6933 "https:///cart/checkout/review" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/534.50 (KHTML, like Gecko) Version/5.1 Safari/534.50"

Hope that helps.. :-)

@jtimbrown Did the patch apply properly to ubercart 2.6? I used ubercart-2.x-dev and the ticket version has that assigned as well.

re subscribe

@smithmilner The patch applied perfectly with 2.6.

FYI - Patch #328 on Ubercart 2.4 fixed the problem on my site. :)

I upgraded to latest -dev drupal and applied most recent patch. I now have this error with authorize.net orders.

watchdog('uc_cart', 'An empty order made it to checkout! Cart order ID: @cart_order'

No idea why. Looking into it. If you have an idea, let me know.

Also there's hook_order, which is a very poor name for a hook. Any module which implements a custom sort is likely to use their module_name_order . I noticed that there's date_order in the date.module.

In my install I have this dpm()

  // Invoke it on a per-module basis instead of all at once.
  foreach (module_list() as $module) {
    $function = $module .'_order';
    if (function_exists($function)) {
      // $order must be passed by reference. 
      dpm("submit: $function " . $_SESSION['cart_order']);
      $result = $function('submit', $order, NULL);

Which outputs this

submit: date_order 827
submit: uc_credit_order 827
submit: uc_payment_order 
submit: uc_quote_order 
submit: uc_affiliate2_order 
submit: uc_coupon_order 
submit: uc_recurring_product_order 
submit: uc_recurring_order 
submit: uc_taxes_order 
submit: uc_extra_fields_pane_order 
submit: uc_googleanalytics_order 

It looks like uc_credit_order is clearing the $_SESSION['cart_order'] which is causing my problem. Maybe date_order is messing it up.

Shouldn't the hook be hook_uc_order() or hook_uc_orderapi() or something.

All Ubercart hooks have been renamed for 7.x-3.x to include uc_ in the name, so hook_order() is indeed hook_uc_order() in 7.x-3.x. This was not put into 6.x-2.x because it represents an API change that should only be made between major versions. The conflict with date_order() was fixed in #611044: Fatal error: Unsupported operand types in .../uc_order.module on line 1459.

@TR. Thanks for letting me know about the change in namespace.

This patch (#85) also has the downside of messing up !new_username & !new_password which is related to the checkout process.

It appears they are trying to get changed to regular tokens and that makes sense, but variables will need to get updated or the end user will need to get notified.

Things like the Anonymous checkout process where the username & password used to be displayed after the sale are now broken.

It also has the problem of not sending the randomly generated password to the end user, so if there email is never received (high probability) or they don't change their password via the reset link (high probability) they are essentially locked out of the site.

While I see the reasoning behind this for security reasons, I think it regresses the user experience and you'll have damn angry customers who can't log in after a purchase and this will cause head-aches for site administrators and thus me.

Using this as a temporary password, displaying it to the end user and forcing them to reset it after any login before proceeding anywhere else on the site is the only solution I can see working. Until this is done (and I understand this could take some time) I would recommend leaving the previous functioning way intact.

I still have my order losing it's session cart_order $order_id variable. Luckily for now as a temporary fix, I'm stealing it from the $_SESSION['ucga_order_id'] variable. This is a horrible temporary solution, but allows my site to function as expected. Until I get this fixed, I will not be able to re-enable paypal to test the duplicate problem.

function uc_cart_checkout_complete() {
  ...
  if(!empty($_SESSION['cart_order'])) {
    $order_id = $_SESSION['cart_order'];
  }
  else {
    watchdog('uc_cart', 'XXX an empty order made it to checkout! Cart order ID: @cart_order', array('@cart_order' => $_SESSION['cart_order'])
    $order_id = $_SESSION['ucga_order_id'];
  }

  $order = uc_order_load($order_id);
  ...

After tracing some code, I'm unable to figure out where exactly the session is getting removed, but I'm still working on it.

It's most likely at one of these points (I'm not using paypal or google_checkout, so it's cart)

./uc_cart/uc_cart.pages.inc:    unset($_SESSION['cart_order']);
./uc_cart/uc_cart.pages.inc:      unset($_SESSION['cart_order']);
./uc_cart/uc_cart.pages.inc:      unset($_SESSION['cart_order']);
./uc_cart/uc_cart.pages.inc:    unset($_SESSION['cart_order']);
./uc_cart/uc_cart.pages.inc:    unset($_SESSION['cart_order']);
./uc_cart/uc_cart.module:    unset($_SESSION['cart_order']);
./uc_cart/uc_cart.module:  unset($_SESSION['cart_order'], $_SESSION['do_complete']);
./uc_cart/uc_cart.module:    unset($_SESSION['cart_order']);
./payment/uc_paypal/uc_paypal.module:    unset($_SESSION['cart_order']);
./payment/uc_paypal/uc_paypal.pages.inc:    unset($_SESSION['cart_order']);
./payment/uc_paypal/uc_paypal.pages.inc:    unset($_SESSION['cart_order']);
./payment/uc_paypal/uc_paypal.pages.inc:    unset($_SESSION['cart_order'], $_SESSION['have_details']);
./payment/uc_paypal/uc_paypal.pages.inc:  unset($_SESSION['cart_order']);
./payment/uc_google_checkout/uc_google_checkout.module:    unset($_SESSION['cart_order'], $_SESSION['do_complete'], $users[$user->uid]);

Here's the problem which has not been addressed by patch #85.

uc_cart_complete_sale() gets called twice. Both times it attempts to clear the $_SESSION['cart_order']. It's called once when the payment is processed and once when the user hits cart/checkout/completed

The import debug which shows $_session looks something like
dpm("uc_credit_order $op $_SESSION['card_order']");

Here's the transaction workflow log:

uc_credit_order load SESSION['cart_order'] = 850
uc_credit_order load SESSION['cart_order'] = 850

starting hook_order calls

uc_credit_order submit SESSION['cart_order'] = 850

BEFORE uc_payment_process('credit', ...) $_SESSION['cart_order'] = 850

uc_credit_order load SESSION['cart_order'] = 850
uc_credit_order load SESSION['cart_order'] = 850
uc_credit_order load SESSION['cart_order'] = 850
uc_credit_order load SESSION['cart_order'] = 850

CALLING hook_uc_checkout_complete

uc_cart_complete_sale UNSET cart_order

CALLING hook_uc_payment_entered

uc_credit_order load SESSION['cart_order'] = NULL
uc_credit_order load SESSION['cart_order'] = NULL

END CALLING hook_uc_payment_entered

AFTER uc_payment_process('credit', ...) 

ENDING hook_order calls

// Landing on cart/checkout/complete
In uc_cart_checkout_complete()

Empty order made it to checkout!

uc_credit_order load SESSION['cart_order'] = NULL

uc_cart_complete_sale UNSET cart_order

If credit card transactions (non 3rd party) are going to be processed inside their own functions before cart/checkout/completed, then 3rd party ones need to do that as well. I believe this is the best way to go about it, because if for what ever reason cart/checkout/complete is never hit by a user who has ordered, then the transaction will never have fully gone through.

This means that each processor should have their own end point to finalize the end stages of an order on the site and then users should be redirected to cart/checkout/completed, which should simply display some information about how that user should proceed.

Currently there are 4 types of users:

• Checkout completion for logged-in users
• Checkout completion for existing users
• Checkout completion for new users
• Checkout completion for new logged in users

I would recommend holding this information in their $_SESSION variable
$_SESSION['last_order_user_type'] (one of the previous 4 states mentioned)
$_SESSION['last_order_uid'] (to store the userID)
$_SESSION['last_order_id'] (to store the orderID)
$_SESSION['last_order_pass'] (to store the generated password)

cart/checkout/completed could then test to see which user_type this person is, it could use some logic to see if an unlogged user is now logged in & if they're currently logged in as the same user which did the order. Then determine which page to display to them or choose to display an error page (if the $order->uid != $user->uid). Otherwise you could simply display a message saying that their information has been emailed to them already and they should login and check their /user account for details.

I really don't think any order completion related logic should rely on hitting the cart/checkout/completed page.

Here's some more information related to the password problem I'm having for new users.

My problem: Anonymous users are given blank passwords.

The bug:

          // Validate the password. 
          if (variable_get('uc_cart_new_account_password', FALSE)) { 
            if (strcmp($arg2['new_account']['pass'], $arg2['new_account']['pass_confirm'])) {
              form_set_error('panes][customer][new_account][pass_confirm', t('The passwords you entered did not match. Please tr...');
            }
            $arg1->data['new_user']['pass'] = $arg2['new_account']['pass'];
          }

Above, uc_cart_new_account_password is set to TRUE and $arg2['new_account']['pass'] & $arg2['new_account']['pass_confirm'] are both left blank.

Thus we end up with $arg1->data['new_user']['pass'] = ''

Then we get to this code and since the key already exists we are left with a blank password

  $fields = isset($order->data['new_user']) ? $order->data['new_user'] : array();
  $fields += array(
    'name' => uc_store_email_to_username($order->primary_email),
    'mail' => $order->primary_email,
    'init' => $order->primary_email, 
    'pass' => user_password(),
    'roles' => array(),
    'status' => variable_get('uc_new_customer_status_active', TRUE) ? 1 : 0,
  ); 

Here's the fix

      // Validate the password.  
      if (variable_get('uc_cart_new_account_password', FALSE)) {
        if (strcmp($arg2['new_account']['pass'], $arg2['new_account']['pass_confirm'])) {
          form_set_error('panes][customer][new_account][pass_confirm', t('The passwords you entered did not match. Please try ag...');
        }
        if(!empty($arg2['new_account']['pass'])) { 
          $arg1->data['new_user']['pass'] = $arg2['new_account']['pass'];
        } 
      }

With that said, with the exception of these two bugs (clearing the $_SESSION['cart_order'] twice & not setting anonymous user password), patch #85 seems to resolve the problem with duplicate emails. At least in the 3 sales I've had with it so far.

I'll keep my eye on the site and see how well things go over the weekend.

subscribe

subscribe

I'm about to upgrade to Ubercart 2.6 from 6.x-2.x-dev. My store is having the duplicate email and stock decrement. My only orders on the site are anonymous shippable orders using Paypal Website Payments standard only. I noticed others are having problems with athorize.net or automatic user account creation with emails sent off as triggers.

I was just wondering if anyone has used the patch on #85 to solve my simple ubercart store. I'm going to upgrade to 2.6, then apply the patch to try it out. I was wondering, do update to 2.6, run updates, then apply the patch after?

Hello,

The patch at #85 is working perfectly for me.
I just upgraded to Ubercart 6.x-2.6.

My tests involve file downloads that artificially have stock numbers. I'm getting only one email for the order, one email for the file downloads, one stock decrement.

So looks good to me!

..jon

Patch #85 will break anonymous sales where an existing email address is not used in the sale.

See #99 for work arounds.

This still needs a complete fix.

@longwave, problem with that is Paypal Express. How would we address this fairly specific case?

Simply drop that cart review pane.

--

Perhaps, some kind of quasi-user gets created in the Ubercart database and after the transaction goes through, it's converted into a real user. I

-- A Side note

In my old payment processor I made, we created an entry for the user before they started processing.

The reason we did this was to restrict failed attempts at processing. Usually your processor charges you like 15c per request, and then a percent when it goes through. Limiting fails, saved roughly 15c per transaction that we decided not to pass on to the processor. Multiply this by thousands a day on a busy store and you can save a couple hundred a day.

We essentially wouldn't allow the same processing details to get re-attempted if the request back from the processor said something like "Not enough funds" or "Hold Card" aka "This is fraud". We would also not allow the same IP to try more than a couple of requests with in a short threshold. Most people don't have more than 2-3 credit cards, so you need to stop them at some point.

In our processor we simply parsed the responses and decided which "type" they were...then decided which of our more generic types we didn't want to keep processing. It would be nice to save a hash of the credit card details (to compare), IP and attempt and response (aka "HOLD CARD" or "No Funds").

Subscribing

Subscribing

I tried to use the latest patch w/ my UC 2.4 version (suprisingly) it didn't go through :-)

Does it help to upgrade to 2.6 in regards of the release of this patch in the near future? I will report my results here ;-)

(my customers get 2 emails with the standard order confirmation when hitting the back to my store button on the Paypal website after the payment have been successfully finished there)

Thx for any information for non .dev version users.
Stort

sub

@longwave, Thanks a bunch. I only have the opportunity to test this on a live site right now. I'll get to it, when I have a couple hours to apply it, test and babysit transactions.

Much thanks,
Jordan

Subscribing

Subscribing

Yes Sir! ;-)

In order to test the patch I have to install the latest dev version, is that correct? 2.4 nor 2.6 can not be used?

Thx
Stort

@Störtebeker you would need to use dev, however, you might be able to apply the patch to 2.6 depending on the changes the patch tries to apply, but it's customary to use dev for that.

I tried using the patch in #110 on the dev version of ubercart, but it didn't work. Still a little unclear on which version of ubercart to use the patch with. Maybe I'm patching it wrong?

I received this trying to patch in Terminal on Mac OSX

can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/payment/uc_google_checkout/uc_google_checkout.module b/payment/uc_google_checkout/uc_google_checkout.module
|index a246254..928c366 100644
|--- a/payment/uc_google_checkout/uc_google_checkout.module
|+++ b/payment/uc_google_checkout/uc_google_checkout.module
--------------------------
File to patch: 

Patching this one worked for me: http://drupal.org/node/280820

No sorry. I've only got a working live site and I haven't got around to it as I have a patched 6.x.24 using patch #4 which is working perfectly but I know I must get around to testing the new patch if we ever want to get this fixed - Just a bit scared.

This issue also exists for Ubercart 7.x-3-beta4 and 7.x-3.x-dev (http://drupal.org/node/1191816). Not sure if it's a similar fix, but if you want to create a patch for Ubercart 7.x-3-beta4 or 7.x-3.x-dev, I'll test it right away. Sorry don't have a Drupal 6/Ubercart site to test.

Ran the patch at #110 on the most recent dev version http://drupal.org/node/280820. Can confirm the duplicate emails are no longer being sent.

and other gateways.

Also be sure to test

anonymous user sale
vs.
logged in user sale
vs.
anonymous user sale with existing email address.

Make sure they all work as expected on both third party processors (Paypal WPS) and Credit Card (authorize.net/Paypal WPP)

It's currently broken in stable, so moving this to -dev couldn't be a bad idea. -dev is for crazy people anyways. I'd say got for it.

Thanks for all the work on this longwave. This was a really important fix.

I'll be able to test in Drupal 7 if we can get the patch out before my site needs to launch. If not, I'll be able to test once that site moved from private beta to public launch.

is there a chance to see this fix coming to
version 2.6 soon?
can not install a dev version in my live shop system.
thanks @longwave for another perfect work!!!!

Happy Happy Joy Joy
Happy Happy Joy Joy

Happy Happy Joy Joy
Happy Happy Joy Joy

Happy Happy Joy Joy
Happy Happy Joy Joy

Happy Happy Joy Joy Joy.

the "patch -p1" worked... Thanks.

Just patched 6.x-2.x-dev. with #110. So far so good. A couple things:

Worked logged in as the admin account. Payment went through Paypal Website Payments Standard. I received only 1 e-mail for the order as a customer, but haven't received my "New Order" admin notification yet. I was still in maintenance mode when I did the purchase. Do you think I wouldn't have gotten that since I was offline for testing?

Also, after I went back to the site from Paypal, it took me to the shopping cart instead of the order complete page. In Paypal "Auto Return for Website Payments" is on and is set as "cart/checkout/complete." Is there something wrong with that or am I missing a setting?

Gonna run another order right now with an anonymous user order and see how that goes. I forgot to cross a threshold, so I'll test that out as well.

I just tested #110 with a previous entered email,under an anonymous user role, with Paypal WPS and it worked!! I received no duplicate emails. Only 1 for each!

Great work to everyone who helped with this. Hyped!

btw...I finally received my emails from the last test I ran above.

Many thanks,
Dan

Awesome, I haven't looked at this yet but could someone through together a patch that works against 2.6 rather than dev for use in the mean time?

Just confirming dcarrjr's findings. #110 is working perfectly for me as well.

Hey all, I downloaded ubercart-6.x-2.x-dev today and ran the patch in #110 against it and got some errors...
I haven't patched in a while, but using cygwin's patch inside the directory and getting this:

Should i assume part of this patch was already rolled out to dev since i see those Reversed messages...?... Some parts succeeded (and failed) though, so is part of this patch not in dev?
EDIT: im guessing now the whole patch is applied after looking at those 'hunks' seem to have been already applied as well... I was expecting the 'Reversed' message for everything and those fail/succeeds threw me off.

$ patch -p1 < 1192018-110-duplicate-complete-sale.patch
patching file payment/uc_google_checkout/uc_google_checkout.module
Reversed (or previously applied) patch detected!  Assume -R? [n] y
......
patching file uc_order/uc_order.ca.inc
Hunk #1 succeeded at 138 (offset 10 lines).
...
Hunk #2 FAILED at 454.
Hunk #3 FAILED at 1397.

Cool, thanks longwave. I read the entire previous thread and musta skimmed over #132 among the 500 posts :).
Thanks again you and others for helping fix this issue. I shall test it out.

EDIT: looking good so far. didn't decrement the stock twice.

Confirming that a patch has not been applied to 7.x-3.x-dev? If not applied, any estimate when it will be to the Alpha or 7.x-3.x-dev version? Is more testing needed with the #135 port? I can help here.

I just tested out a bare bones U7/UC3 site with the latest
7.x-3.x-dev tar.gz (567.35 KB) | zip (810 KB) 2011-Sep-28
and WPS is working good. I don't use stock so haven't tested that out but these are the emails I get. There are three of them.

The user gets

Hello *****,

You sent a payment of $0.50 CAD to ******.
This charge will appear on your credit card statement as payment to PAYPAL *BILLING.

and

Your Order at *****

The admin gets the following

Payment received from ********

Has this been fixed in 6.x-2.6 ?

Is there a workaround using Conditional Actions?

As stated by longwave in comment 135: No, it will be part of 6.x-2.7.
Also as stated before, there is no good workaround for this issue.

Fix applied for 6.x-2.7 causes regression for all payment methods where payment is entered the system before calling uc_cart_complete_sale() function.

As a result of that new order mail and stock is decreased twice. 1st time when entering payments using uc_payment_enter() and 2nd time when uc_cart_complete_sale() called.

See also #1287836: New Order mail sent on entering payments

E.g. in uc_paypal module uc_payment_enter() is called before uc_cart_complete_sale()

      case 'Completed':
        if (abs($payment_amount - $order->order_total) > 0.01) {
          watchdog('uc_paypal', 'Payment @txn_id for order @order_id did not equal the order total.', array('@txn_id' => $txn_id, '@order_id' => $order->order_id), WATCHDOG_WARNING, l(t('view'), 'admin/store/orders/'. $order->order_id));
        }
        $comment = t('PayPal transaction ID: @txn_id', array('@txn_id' => $txn_id));
        uc_payment_enter($order_id, 'paypal_wps', $payment_amount, $order->uid, NULL, $comment);
        uc_cart_complete_sale($order);

Quick workaround for this regression problem is to comment out uc_cart_complete_sale($order); line from uc_payment_enter() function (that was added part of this fix for 6.x-2.7)

CA rules not modified.

Did have a more careful look at payment methods enabled and did notice that one of the payment methods (uc_2checkout) have following sequence in place for saving order details:

  if ($_POST['credit_card_processed'] == 'Y' && is_numeric($_POST['total'])) {
    $comment = ...
    uc_payment_enter($order->order_id, '2checkout', $_POST['total'], 0, NULL, $comment);
  }
  else {
    ...
  }

  // Empty that cart...
  uc_cart_empty($cart_id);

  // Save changes to order without it's completion.
  uc_order_save($order);

  // Add a comment to let sales team know this came in through the site.
  uc_order_comment_save($order->order_id, 0, t('Order created through website.'), 'admin');

  $output = uc_cart_complete_sale($order, variable_get('uc_new_customer_login', FALSE));

  ...

Function call of uc_order_save() between uc_payment_enter() and uc_cart_complete_sale() calls causes CA to trigger twice before data['complete_sale'] element is assigned to $order object

2Checkout

Subscribing

I just wanted to confirm that after upgrading from 6.x-2.x-dev (from Apr 3 2011) to 6.x-2.7, the problems i was having with duplicate notifications, and file download links not working for users created during checkout, were totally gone! and maybe some other bugs too.

so the patch in #110 worked, at least once it was incorporated into the next release.

thanks all who worked on this!

cheers
Fish

ps: note to anyone else in this boat: if you are doing a similar upgrade, you will find that some of the default conditional actions have changed drastically, so you may want to reset yours to default, or figure out what changes you made, reset, and integrate the changes back in.

Confirmation @ #152:

I commented out the line:

// uc_order_save($order);

in the 2checkout module and my orders are now working in 2checkout module
Have 1 stock decrement
Have 1 email to customer

I am using latest 6.x-2.7 dev version downloaded today 3/19/2012
Sorry to post on closed issue, but just had to post for anyone else struggling...

@goose2000: The 2Checkout problem was fixed back in November by #1332130: Duplicate emails when using 2Checkout. So you can either use 6.x-2.x-dev or apply the patch from that issue to your 6.x-2.7.

@ TR, yes - actually I made a payment module based on the 2checkout that works with TouchNet instead. My module inherited the same problem and the patch has now fixed my custom module as well. Just posting another happy ending - thanks 8)

Hello

I know that this issue has been closed but I really need your help. I have just moved my site to a new server (site5.com) and this issue has suddenly started without changing anything.

I've tested my site again in the local environment and it works perfectly but the exact same site in the site5 server has this duplicate email and stock decrement issue.

I am running Drupal 6.19 and Ubercart 2.4 (I think!).

Can somebody please tell me what I should do?? To use the patch I would have to first have to update Ubercart, right? And then apply the patch?

That seems unnecessary since my site worked perfectly on our previous server and in the local environment.

Any help would be greatly appreciated.

Many thanks

@ryaner - there is no need for you to apply any patches.

This issue is fixed, and the fix is included in Ubercart 6.x-2.7 and all later versions. The current version is 6.x-2.9.

If you're using Ubercart 6.x-2.4, then that is your problem. Simply upgrade to the current version. Given some of the releases between 6.x-2.4 and 6.x-2.9 included security issues, that is a pressing thing to do quite apart from the issue being discussed here.

Actually, I'm on UC 2.7 (can't upgrade to 2.9 yet for various reasons) and I'm still getting double stock decrements. They actually went away for a long time, and just started showing up again.

Before the patch, I was handling them with CAs that fire on checkout completed (default method I believe), but when the patch rolled out, the CAs were causing a double check and there was no stock decrementing at all. Since the stock decrement comes before the payment received, I'm experimenting with decrementing on the payment "Update order status on full payment" trigger. We'll see if that works.

ergophobe: If you're still seeing this with new versions, please file a new bug report. If you work out a fix or workaround, please post that, too.

Seems to be working okay now - at least the store admins haven't reported any more doubles lately. I'll open a new issue if it crops up again

Is there a fix for D7 yet?