Duplicate order notification e-mail, and duplicate stock decrement

Have you modified your Conditional Actions rules at any time? Both the notifications and stock decrement actions are triggered from these, so you should check that these only get fired once under the correct order status conditions.

I can confirm I have the same duplicate stock decrement and duplicate email notifications. I upgraded from 2.1 to 2.2.

It appears that all of the duplicate information has the same CA trigger "Customer completes checkout". It seems that somehow this "Customer completes checkout" trigger is happening twice during a transaction...

From the order admin comments showing the duplicate stock decrement:

12/03/2009
2:16:09 PM - The stock level for 01 has been decreased to -41.
12/03/2009
2:16:12 PM - The stock level for 01 has been decreased to -42.
12/03/2009
2:16:13 PM - Order created through website.
12/03/2009
2:16:16 PM - PayPal IPN reported a payment of 879.49 USD.

Also, prior to getting my paypal configuration working, the duplicate entries did NOT occur. It was only once I started receiving the IPN notifications that I also received the duplicates, FWIW.

Confirmed that it occurs on a fresh install of 2.2, without upgrade from prior version.

This is from adding a quantity of 2 of a product to the cart on a fresh install. A total of 4 was deducted from the stock inventory.

12/03/2009
4:11:12 PM - The stock level for 01 has been decreased to 98.
12/03/2009
4:11:16 PM - Order created through website.
12/03/2009
4:11:17 PM - The stock level for 01 has been decreased to 96.
12/03/2009
4:11:20 PM - PayPal IPN reported a payment of 1,612.00 USD.

workaround here:
http://www.ubercart.org/forum/bug_reports/13716/stock_levels_decrease_tw...

I was receiving duplicate emails, duplicate decrements, and a error message when the user returns from Paypal that an email could not be sent and that they needed to contact the administrator.

I fixed this with the conditional rules at the link that @jmuessig posted.

However, when I just installed uc_coupon, the problem happens again when a user uses a coupon code to checkout.

Normal orders without coupon code work fine.

subscribing

subscribe
This started occurring for me as well after the last upgrade. I dont use the coupon module BTW.
duplicate decrements when using paypal.

+1

I am only getting duplicate order notifications with Paypal WPS, test cc gateway works fine.

For me #25 fixed it for me in this thread: http://www.ubercart.org/forum/support/10422/multiple_emails_single_purchase

Lots of duplicates though, wonder if the Ubercart guys would consider some out of the box values so Paypal users don't have to fiddle with these:
http://www.ubercart.org/forum/support/10422/multiple_emails_single_purchase
http://www.ubercart.org/forum/support/8790/stock_level_reduced_twice
http://www.ubercart.org/forum/support/8930/stock_reduced_twice
http://www.ubercart.org/forum/support/14097/duplicate_notifications_file...
http://www.ubercart.org/forum/bug_reports/13313/paypal_duplicate_actions...
http://www.ubercart.org/forum/bug_reports/13716/stock_levels_decrease_tw...
http://www.ubercart.org/forum/support/13794/file_download_user_notified_...
http://www.ubercart.org/forum/support/5192/stock_decreased_2_times_once_...

Issue confirmed. Seems to have popped up out of nowhere at one of our projects. We maintain several comparable shops for the same client, all having the same UC related modules and versions installed, only one is affected. Since the last Drupal/module update several PayPal payments where received without duplicates. Since yesterday all orders with PayPal payments are recieving duplicate notifications. The only thing that has happened yesterday was a complete clearance of cache on this very project.

One other thing changed recently, PayPal upgraded the merchant account because yearly limits where reached. Though there where no config changes needed, payments are processed the same as before.

uc_coupon is used. None of the affected orders do have coupon codes applied.

I also noticed that if I implement the changes as per #25 in this thread: http://www.ubercart.org/forum/support/10422/multiple_emails_single_purchase , Paypal WPS notifications work as expected, but for the test credit card gateway, I don't get any emails for the changed triggers, such as order and file downloads.

It's an annoying error that's causing a lot of waste time debugging, blaming all sorts of other self-caused reasons, testing and re-testing (testing paypal transactions is tedious), finally deciding it probably was ubercarts fault, researching, finding fix, applying and testing.

Apologies, but this is currently wrong as distributed out-of-the-box, so bumping priority.

Thanks James, will be looking through that post, although currently my head is spinning with all these predicates, conditions, actions, events, rules, triggers, AND/OR, groups... :) It works currently with Paypal WPS so might just leave it there :P

For me #25 fixed it for me in this thread: http://www.ubercart.org/forum/support/10422/multiple_emails_single_purchase

It fixed it for my basic testing.

However, I received two orders over the last 2 days. One had duplicate emails and duplicate stock decrements and one worked perfect.

I also have this problem with using the uc_coupon module. Orders received that had coupon's applied, create duplicates as well.

I only use Paypal WPS for payments.

Is there are better location where this bug reporting should take place so that we can get a discussion going on how to fix it?

My shop is a new shop that I've just built over the past 2 months, so most of my sites configuration is out of the box and new install.

As mentioned earlier (#10) I have been getting double decrements and double emails since the last upgrade. But today one of my orders was fine - no double decrement & no double email. Since I only use paypal IPN for all orders, I'll see if I can determine what made todays working order different from the others, and will post if I identify anything unique about it. BTW - I dont use coupons, and have changed nothing in particular related to ubercart since i first reported this issue.

My client is also experiencing the same problem. It happens for paypal transactions, but not for regular credit card transactions. Here is what I have observed.

Ubercart Order Log for PAYPAL transaction:
* Sat, 01/23/2010 - 11:51- Order status changed from In checkout to Pending.
* Sat, 01/23/2010 - 11:51- PayPal payment for $35.91 entered by -.
* Sat, 01/23/2010 - 11:51- Order status changed from Pending to Payment received.

Ubercart Order Log for CREDIT CART transaction:
* Sat, 01/23/2010 - 14:32- Credit card payment for $28.73 entered by -.
* Sat, 01/23/2010 - 14:32- Order status changed from In checkout to Payment received.

In other words, there appears to be a unique transitory state of "Pending" for Paypal transactions. Is it possible that this is the cause of the duplicate email problem? What would be a good solution that would ensure a single email notification will go out for both credit card and paypal transactions?

Peace,
Brian

What's happening is that
- when paypal registers a successful payment, it sends a message to confirm payment back to the originating site (your store) via IPN
- your store sends its email
- THEN the user clicks the back-to-store button - which is constructed to tell your store that a payment was made
- and your store registers the confirmation and sends an email.

Thing is, IPN needs configuring, and the user may not press back.
We want to register only one of these actions.

SO.... if you are using IPN, you must DISABLE the confirm on receipt of 'back-to-site'

It could/should be solved in code, but just needs doing

Thanks DMAN! So let me restate your workaround to make sure I understand it. My client would need to go into their paypal account and configure their IPN to disable the confirm on receipt of "back-to-site". In other words, it's a paypal configuration, not a drupal configuration. yes?

Subscribing. very confusing issue going on here... ;)

It's basically an error at the drupal/ubercart end that responds to being told twice that an order was processed, by sending a confirmation twice.
IF using paypal THEN you need to unset one of the configurations, as described in one of the help threads linked above. It's fixable through the UI.
BUT the out-of-the-box notification defaults that come with Ubercart are a little off ONLY WHEN used with paypal IPN (though I imagine they are correct defaults for other situations).

The above config fix-up works, so just do that.
In the meantime, the ubercart code could also have something done to it to, say, ignore the second confirmation of something we already know. And this issue would not occur again.

My diagnostic may be a little wrong. Or a lot wrong, but I think that's where we are at.
Sorry I don't have an actual patch for it, but I'm wary of the code.

I'm not sure that #24 fixes every situation.

1) I am using Paypal WPS only because my site resides outside of the United States.

2) I do have my IPN settings to: Do not receive IPN messages (Disabled)

Under the Website Payment Preferences I have the following configurations:

1) Auto Return: On
2) http://www.calendarfactory.com/cart/checkout/complete
3) Payment Data Transfer (PDT): Off

Edit: After several more days' work on our site we found the repetition that we experienced here was caused by the actions being triggered by 'checkout complete' rather than by payment received, therefore the code that I submitted previously is not relevant. That doesn't mean to say that there is not a problem.

I have just implemented the code changes mentioned above and will report back as soon as I receive a new order.

Having changed the code in post #26 and received an order, I can say that unfortunately this does not rectify the problem.

< snip! >

This works :) as a side note, I have been having issues with files not being granted properly to new users who are automatically logged in at checkout. I suspect that this line:

uc_cart_complete_sale($order);

Should actually be something more like:

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

Otherwise new users are not logged in... Need to test this

subscribe

Moving back to "active", as there's no patch to "review".

If fixing this issue requires a change in Ubercart code then the fix should be posted here as a patch for others to try out. It can be committed to Ubercart when the community reviews the patch and finds it to work without causing other problems.

Also moving priority back to "normal". See http://drupal.org/node/45111 for an explanation of the priority levels.

Subscribing -- I'm having this problem, as well.

Subscribe

Hi

I use Ubercart 2.2 with Paypal in Sandbox mode, and have the following problems:

• Duplicate emails
• New users not logged in after checkout

The strange thing is that I dont experience these problems when running on localhost. I even re-copied everything to the server TWICE to confirm it, and it's only on a live server that I experience this.

Any ideas?

Would anyone with programming experience be able to help provide a patch to be tested and reviewed?
I think Ubercart is a great module and hope it continues to develop into a better module for the drupal community.

I have the same issue. I have 6.x-2.2 on two websites - settings are the same. I get duplicate emails on one and not on the other.

Note for #35 and maybe #37
This issue only emerges on live sites (not testing) because of the way Paypal IPN sends a message back to your site confirming payment. This trigger cannot happen if developing within localhost or LAN.

Only once you go live after testing does this start happening, and your real customers get notifications that they have paid twice. Which is pretty worrying.

To fix this issue, delete line 97 from ubercart/payment/uc_paypal/uc_paypal.pages.inc:
uc_cart_complete_sale($order);

I in fact solved this issue because it also appears in Paymex's IPN—they are a NZ-based gateway who like to base their modules on Paypal's code I have discovered. I have not actually tested this solution with Paypal but I am 100% sure this is the issue because of the code similarities and an understanding of what is going on.

It seems to me that ideally, the completion would be triggered by the IPN response from Paypal, and the user arriving at the confirmation page would be just that—confirmation. However, the current Ubercart code triggers the important confirmation event uc_cart_complete_sale when you arrive at this page. This to me is a bad idea, because it is possible a user could fail to return to this page, and then the sale would not be fully completed. However, with the above edit the IPN still records against the order that the payment has been received, and gives transaction numbers and so on... it just does not fire the final uc_cart_complete_sale that changes stock levels and dispatches confirmation emails.

There is no doubt a better way to fix the fundamental Ubercart architecture, but you have to admit, given the problem is Paypal (and Paymex) specific, that's a pretty tidy solution right there. :)

No patch to review...

Just another finding. I had no problems with it on a live Paypal Sandbox site with auto return enabled, but when I switched to Paypal Live and payed using a Credit Card the emails were sent twice. Maybe because the auto return doesn't return properly when you use a credit card on Paypal?

I fixed the duplicate emails using conditional actions, but maybe this can help someone.

On another note: When paying by credit card through Paypal, new users arent logged in. It takes them to the "Checkout completion for existing users: " and says it has attached their order to the account it found matching the email address. I think this is also because of auto return not working when you use a credit card.

Pretty annoying but Paypal says its for legal purposes, and at least the order is going through.

Hi bob_b,

Can you describe the changes to the conditional actions you made to fix this issue? I've been trying this myself and haven't been overly successful.

Much appreciated!

I am changing this status to critical and version back to 2.2. This is a serious problem that has been going on for months now and in my opinion, is just disappointing that maintainers of a payment module ignore the issue.

Conditional actions can be used to avoid duplicate notifications being sent upon purchase but cannot be avoided when users click the "Back to store" link. Therefore as dman suggested, Ubercart is double confirming a purchase order.

The PayPal IPN is confirming to Ubercart that a successful purchase has been completed, which is what it is meant to do. However Ubercart is then re-confirming upon the customer clicking the "Back to store link". This is unnecessary.

The first confirmation is made by the PayPal module on line 97 with the uc_paypal.pages.inc:

case 'Completed':
        $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);
(97)  uc_cart_complete_sale($order);
        uc_order_comment_save($order_id, 0, t('Payment of @amount @currency submitted through PayPal.', array('@amount' => uc_price($payment_amount, $context, $options), '@currency' => $payment_currency)), 'order', 'payment_received');
        uc_order_comment_save($order_id, 0, t('PayPal IPN reported a payment of @amount @currency.', array('@amount' => uc_price($payment_amount, $context, $options), '@currency' => $payment_currency)));
        break;

You cannot simply remove line 97 as it means the $order variable will not be triggered when a purchase is made in PayPal. It will only be triggered IF a customer clicks the "Return to your store" link.

The second confirmation is made on line 544 by uc_cart.pages.inc:

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

You cannot just remove this line either because credit card transactions will not receive notification.

A possible solution could be to initiate an if statement for PayPal or credit cards in the uc_cart.pages.inc section. I am beginning to think the coders avoiding having to do that because it means every module that triggers notifications off site would need to be included in that if statement section. Unless there is another solution I am overlooking.

Either way, this does need to be fixed. It is not a conditional actions problem and for the Ubercart maintainers that say it is, I suggest you provide the exact conditional action settings to prevent the duplicate notifications/stock adjustments/uploads. There are enough people struggling with this issue for it to be fixed.

@jasont28: No one is ignoring the issue, no one is saying it doesn't need to be fixed, and no "Ubercart maintainers" are saying it's your fault. The mis-characterizations in your post are a little insulting to me personally. I became the branch manager for Ubercart 6.x less than a month ago, and in that time I have fixed more than 50 existing bugs, responded to more than 250 separate threads in this issue queue, and have been contributing to the D7 port. So don't you start saying that I've been irresponsible in this matter. What have *you* done to help out?

Why don't *you* do the work and figure out what is causing the problem, how to fix it, then propose that fix in the form of a patch? You're getting paid just as much as I am to do this (= $0, in case that's not clear). The main difference being you're using PayPal and I'm not, so *you're* the one in a position to do something about it. Or perhaps I should just drop everything and and devote myself to fixing your problem - is that what you're asking?

All sorts of things have been vaguely proposed in this thread, but until there is a patch to try out I can only guess at exactly what anyone has done to "fix" the problem. I say "fix" because it's clear that some of the proposals will *break* other things - you pointed this out yourself in reference to the "fix" in #39 (removing line 97). Messing around with the default conditional actions, for instance, might seem to be a peachy way to solve *your* problem, but in doing so you're likely to break things for the thousands of people who *aren't* using PayPal.

So instead of complaining that the "maintainers" aren't doing their job (a patently false statement), why don't you step up and give something back to the community - develop a patch and post it here so everyone who has the problem can try out your proposal to see if it's the right way to go. And while you're at it, how about helping out with some of the 200+ *other* bugs that remain in this issue queue? Or is yours the only important one?

—Removed as not quite accurate, to be replaced with more detailed response below—

Subscribing;
I would be very happy to assist in testing any workaround/patches for this issue.

OK, so here's the critical question about payment gateways other than Paypal... Given the Ubercart default confirmation page calls uc_cart_complete_sale, it would appear that *every* payment gateway module would have three choices:
a) only have orders properly completed if the user successfully returns to the confirmation page, or
b) have two confirmation emails sent out, or
c) create their own custom version of uc_cart_complete_sale that pre-empts uc_cart_complete_sale's plans to modify the transaction records in the database, maybe even creating the new user accounts and linking the orders and so on, but without dispatching the confirmation emails since there is no way to prevent Ubercart from doing this other than by setting a custom confirmation page. (Maybe that is it—does every other payment gateway module have a custom confirmation page?)

@dbabbage (g'day Duncan btw!) -

does jasont's comment #44 affect this? he is saying that "you can't simply remove line 97..."
Not sure how to best proceed with this, but I'm certainly looking for a workaround too. The site in question is only using paypal, however it is possible that other payment methods will be introduced later..

Interested to hear your thoughts on this.

You cannot simply remove line 97 as it means the $order variable will not be triggered when a purchase is made in PayPal. It will only be triggered IF a customer clicks the "Return to your store" link.

I'm not sure what you mean by "the $order variable will not be triggered", but that's not quite right. As I said above, if you delete the line #97 in question the IPN still records against the order that the payment has been received, and gives transaction numbers and so on... it just does not fire the final uc_cart_complete_sale that changes stock levels and dispatches confirmation emails. It also presumably would not create a new user account for an anonymous checkout, if you have that preference set, but the order would still be in the system. Not ideal, but I'd suggest anyone searching for a solution would do a lot worse than making the edit I suggest until something more robust can be resolved.

ETA: Hi. :) We must have been posting simultaneously, but I think this answers your question above?
Further edit: The change btw is to a Paypal-specific file so would have no effect on the introduction of other gateways in the future.

TR my post was not aimed at your personally nor intended to insult you. I didn't even realize you had become a maintainer but prior to you becoming an Ubercart maintainer, this issue has been plaguing Ubercart users as far back as a few months. You are the first one to actually address the problem. I do not wish to degrade the focus of this thread with open source politics but I do apologize for any insult you believed I was directing toward you which was not my intention.

In a thread on the Ubercart forum created months ago this issue was mentioned and it was suggested that it was a conditional actions problem. Which it is not. It is a multiple firing of 'uc_cart_complete_sale' (thanks for correcting me dbabbage :) )which contains the email notifications/stock adjustments/file uploads and other actions to be given to complete sale.

I can read and identify problems in code but cannot write it. Otherwise I would have tried writing a patch to fix the issue. I can contribute by testing and assessing code as well as suggesting changes in logic.

I'm not sure what you mean by "the $order variable will not be triggered", but that's not quite right...it just does not fire the final uc_cart_complete_sale

Thank you for correcting me. Removing line 97 will still have the same severity of the problem just in the opposite degree. Instead of two notifications/stock adjustments/file uploads, they will get nothing.

OK, so here's the critical question about payment gateways other than Paypal... Given the Ubercart default confirmation page calls uc_cart_complete_sale, it would appear that *every* payment gateway module would have three choices:
a) only have orders properly completed if the user successfully returns to the confirmation page, or
b) have two confirmation emails sent out, or
c) create their own custom version of uc_cart_complete_sale that pre-empts uc_cart_complete_sale's plans to modify the transaction records in the database, maybe even creating the new user accounts and linking the orders and so on, but without dispatching the confirmation emails since there is no way to prevent Ubercart from doing this other than by setting a custom confirmation page. (Maybe that is it—does every other payment gateway module have a custom confirmation page?)

I think we could look at 2Checkout to see if this problem is continued across other off site payment processing options. I haven't integrated a site with 2Checkout yet but does it also offer an option to return to the Drupal site after payment has been processed? If that link is clicked, is the 'uc_cart_complete_sale' fired again?

The uc_2checkout.pages.inc code contains the 'uc_cart_complete_sale' function on line 86 but it is contained within an '$output' variable.

Here is my suggestion. A "Notifications Sent field is created in the 'uc_order_admin_comments' table. Upon sending the email notifications etc from a 3rd party processor, it updates 'uc_order_admin_comments' with a value for the notifications being sent. 1 being true 0 being false. The 'uc_cart.pages.inc' code would then search for this value and if 1 would refrain from resending the notifications or even refrain from firing 'uc_cart_complete_sale' altogether.

For this solution, one would need to:
add a "Notification Sent" field to the 'uc_order_admin_comments' table.
Any payment modules that come across this same problem would update the "Notification Sent" field upon firing 'uc_cart_complete_sale'.
'uc_cart.pages.inc' would check the value of "Notification Sent" and only fire 'uc_cart_complete_sale' if the value is 0.

Again, I am not a coder so that may not be a good solution but its the best quick fix I can think of.

Since the issue seems to be due to calling 'uc_cart_complete_sale' twice, it seems to me that the obvious fix would be to change 'uc_cart_complete_sale' to look at the status of the order before it did anything else, and just exit without doing anything if the order's already marked 'complete'. The logic in that routine should only be invoked on a state transition, right?

@RocketRick - Yeah, That's what I'd do.

This really does make me curious about how the other Gateways manage this. I can't look into that right now, but I certainly will in the next couple of weeks if the problems of the world (or at least of 644538) haven't been solved by then.

Here is my solution:
http://www.ubercart.org/issue/13628/paypal_triggers_uc_cart_complete_sale_when_it_not_needed

Attached is the diff.

Patch in #56 is partial solution already proposed in #39.

Subscribing

I'm not familiar with Ubercart's architecture so there are likely some consequences but I'd be tempted to take the approach mentioned by #53 RocketRick.

I like the idea of the approach mentioned in #53 by RocketRick. Much easier than adding the extra pieces I suggested.

Wouldn't such a simple fix be a single line of code solution? Simple check of the database for the existing 'complete' status?

Jason

subscribing

Subscribing

subscribing

still do not know how to fix this....??

My 2 cents

I think the main reason there is not an abundance of patches for this problem is because no one's quite sure what the correct solution is. I'd like to make a few suggestions and hopefully get on the road to some consensus so we have a clear direction on what to code. Then I'd be happy to contribute with actual code.

1) I don't think it's the time to mess with Core Ubercart modules so let's leave uc_cart and uc_order out of it. If a fundamental change in how payments are processed is needed, it should be addressed for Ubercart 3.

2) The problem is with a payment module and payment modules should conform to the core Ubercart API Just because the PayPal module is distributed with the Ubercart package doesn't make it an exception. So let focus on patching the PayPal Module.

3) With payment pages that are external to the website I believe the most natural workflow is:
"on site review page" > "off site payment page" > "on site completion page (where the magic happens)"
This is logical and I think understandable to the user.

I think where we're getting hung up is in the case where PayPal doesn't automatically return the user to the completion page which is why we're clinging to IPN as a solution. If auto return is ENABLED when does it not work? I've not experienced this.

I know programmers don't like to leave anything to chance or to the user if they can help it. So hoping they click the return button if they don't auto return seems troubling. I think it's a PayPal issue if the auto return doesn't work when it's supposed to, and the module should be written as if it works.

But if it doesn't work AND the user doesn't click the return button is it the end of the world? If the store owner is reconciling their accounts on a regular basis (i.e. daily like an off-line retail store) they'll notice that their income doesn't balance with their sales reports and investigate. Checking their PayPal transactions they'll notice an extra order ID and can manually update the order in Ubercart.

Does anyone have solid information on when and why PayPal's auto return doesn't work "sometimes"?

Possible solution:

1) IPN shouldn't be used in WPS to complete the sale. So disable line 97 of uc.paypal.pages.inc - IPN will still send transaction data to be logged with the order.

2) The current Ubercart API "demands" transactions complete on the complete page. So enable auto return in PayPal account. As a contingency, add a message on the review page stating, "When you click submit you will be taken to PayPal.com to complete payment. If you are not automatically returned to thissite.com make sure you click the "return to ... " button to complete your sale."

-
I've written other payment modules that use external payment pages and this workflow is successful. Why not with PayPal WPS? As I said, if PayPal isn't auto returning when it should be, we should take it up with PayPal.

Does anyone have anything else to add? An alternate plan? Am I missing anything? My experience and focus is on WPS, I have no experience with the other PayPal methods. If we can agree on something we might be able to get a patch into a release. Even if it's "the best we can do" in Ubercart 2.

Feedback is encouraged!

Thanks

I know programmers don't like to leave anything to chance or to the user if they can help it. So hoping they click the return button if they don't auto return seems troubling. I think it's a PayPal issue if the auto return doesn't work when it's supposed to, and the module should be written as if it works.

Your comments are well received but in a case of ecommerce, a transaction really needs to know the final handshake has happened.
The message that paypal shows (while waiting for redirect) indicates the transaction is complete. If IPN was not set up correctly, then the transaction was NOT complete. There is a window there where a user can close their browser, have money taken out of their account, but the merchant never receives a confirmed order.

I think that one option is for IPN to be required, but don't know the reasons why the current module supports either/or IPN/Return confirmation .. or, in this case, *both*.
There may be tech reasons why IPN is impractical sometimes. I don't know what those are

I DO think that the robust solution is to silently discard the second 'confirmation' of the same transaction, if it is neccessary to support both methods. If that takes a mod to whichever part of UC core, it is still a sane affirmation to make. IMO.

Thanks for the feedback dman. I think my problem with changing UC core in this case is that it's PayPal specific. Are there other gateways that pose a similar problem or is PayPal unique? I've written 2 other modules for external payment gateways, starting a third now, I haven't seen anything similar yet.

I'm not suggesting disabling IPN or the uc_paypal_ipn function all together, just not letting it trigger the complete sale function. The transaction data will still be attached to the order from IPN. So the order is still confirmed with the merchant, just not reconciled in the order system.

Here's a patch with an idea I had today. Basically it works like this:

User pays on PayPal. IPN sends notification. This is captured by UC and attached to the order as usual but uc_cart_complete_sale is not triggered. Instead it checks the order state. If it's not payment_received (which it would be if the user returned to the complete page first) it sets a new order state of paypal_received.

In the order log you see the status of "PayPal Paid but not complete". When an admin views the order they see a new pane stating:

"Occasionally, users will close their browsers after their PayPal transaction is complete, without returning to the 'checkout complete' page on this site. If this happens, your site will not mark the order as complete or send out order notifications/recipts. The payment should still be logged with an IPN message in the comments. If so, and the balance on this order is $0, you can Click here to complete the order manually."

Clicking the link grabs the order object and runs uc_cart_complete_sale, setting order comments and messages and setting the state to payment received.

If the user DOES return to the complete page, state is set to payment received and everything is normal.

-
I think this works because it fits within the normal workflow of the merchant anyway. They need to check the orders, now they get an indication that the user didn't return and they have a simple method of completing the order. Emails are only sent and stocks decremented once, AND it's all done with fairly minor modifications to the PayPal module only. No messing with core.

This patch is against 2.2

Does anyone have any actual data on how often PayPal users don't return to the site? What's the percentage? I'm just curious.

Anyone agree with this approach? Am I missing something terrible? Please test and feeback.

Thanks!

I can't speak for why it's done this way, but it seems that you can set up an ubercart without IPN enabled. This relies on the user being redirected afterwards.
This method is actually useful when testing on a LAN or firewalled site, where IPN cannot happen. However, I'd prefer that if IPN was a good method (I think it is indeed better) then it was the only sure solution. Right now, both options are open.
This may have something to do with the fact that there are several settings needed on the Paypal config end - where the admin must set either IPN enabled OR an automatic return. I guess this current code allows for a few options to be missed.

My preference for a us patch is simple validation logic - don't allow a payment for the same transaction to be registered twice.

I'm not much more clear on paypal or uc internals than this, so I'd prefer to understand why both confirmation methods are currently supported before changing the behaviour. If the return method is legacy, or there are other situations where IPN is really impossible.
Your patch solution does address the current symptom, so probably helps enough. But I feel a guaranteed IPN confirmation should be the end of the story from the merchant end, regardless of the users return state. The fact that the system currently cares about or acts upon the return state at all is a bit of an anomaly - to my understanding.

I am currently implementing a payment solution for a payment processor that works similar to paypal and found this thread. When I implemented my solution I looked at the code from the paypal module and made my implementation in a similar way, so I also ran into the problem of duplicate emails being sent out.

I can only say that I agree with dman, it would be a better to refactor uc_cart_complete_sale to not complete the order twice.

If feels like a more robust solution to rely on the IPN call than to assume that the browser redirect works. There could be several reasons why this could happen, e.g. that the user loses the Internet connection.

limikael - Whn I get the second payment notification, it doesnt change my order status to 'Completed'. It leaves me in "Payment received'.

Just a note, that whether I get one 'Payment Received' or two, I still only receive a single payment amount into my paypal account.

The annoying thing for me is that it decrements my stock twice if it receives the second payment received. Then for each of those orders I have to restore the inventory numbers of all ordered items manually, and also send another email to the client indicating that I receved 'a' payment, so he doesnt think he may have been charged twice.

I appreciate any work that may be done to resolve this issue.

It seems that about half my orders have this issue - the other half are fine.

marked #590934: Buyers of downloadable products, paying with PayPal, get emails twice as duplicate

For folks looking for a workaround, JamesOakley's Conditional Actions solution in #46 -- #13 on a Ubercart.org thread -- worked for me, (though PayPal WPS is my only payment option.)

Thanks JamesOakley!

I've applied this and done a bunch of test orders in the sandbox. The first time it sent the download email but not the order confirmation. 2 other tries were a role grant and shipping order, everything went ok there. The second time I bought a download, both e-mails were sent ok, no dupes in any of them. (all this was using the paypal buttons checkout and not the checkout -> paypal flow).
With checkout -> paypal (here I have to hit "return to merchant"), doing 2 downloads in one order, the Order notification was sent once, download notification (with both download links) sent twice. Tried again with one download in the order, got 2 download notification e-mails, one order notification. For a shippable product, one order notification.

2 download notifications are better than 2 order confirmations though (where someone may think they were charged twice), definitely a step in the right direction, thanks!

Just to add yet more complexity to this issue (sorry people!) - here's some more info about when Paypal WPS might not auto-return back to your site.

Im using Paypal WPS (in Australia). When the order is passed to Paypal, there are two alternatives -

(a) customer logs into paypal with an existing paypal account, and completes the sale. In this case, auto-return always works (AFAIK).
(b) customer does not have a paypal acount, and simply enters their credit card details etc at the paypal site. When the sale completes, the auto-return does not seem to fire, and they have to click the "return to site" button.

But in both cases, I get two emails sent out, so the behaviour is still consistent...

NB for all of my tests, I've created a dummy product ($1) and i'm testing using the live system - I think this is the best way to test this as I have had little joy from the sandbox.

Paypal Sandbox is not a joyful place.
Your input does at least help identify the edge cases.
My question is - how does this interact with IPN? If you are getting two emails, I guess that means the first one is from the IPN action?

I thought we had already established that upon a purchase being completed on PayPal's website, the order status was updated to 'completed'. The problem was that when the user was redirected back to the website or clicked the 'Return to website" link on the PayPal site, the order notifications/product downloads were triggered again.

Or am I missing something here? Is the order status set to 'Payment recieved' once the purchase has been made on the PayPal site and only updated to 'completed' once the customer returns to the site?

I am getting all of the necessary notifications being sent after the purchase is made on PayPal. They are just being sent again when the customer returns to the site. So if the order status is being updated to 'completed' after the purchase on the PayPal site, UC core is sending the second notifications, not the uc_paypal module.

In this case, I still think the solution in #53 is workable. It doesn't take out any core code but simply does a check before sending notifications.

On my site, www.flatwarefinder.com , the order status stays at Payment received, whether i get duplicates or not. I manally have to move it to completed, which I do after I ship my products.

I've done a bit more to investigate. I changed the subject line of the "Email admin checkout notification" conditional action's "Email an order invoice" action to the following:

New Order at [store-name] for [order-first-name] [order-last-name] (Paid by [order-payment-method], status is [order-status].)

I discovered that the emails are being sent /prior/ to the status being updated, and that when someone pays by check, the email subject says "New Order at STORENAME for FIRST LAST (Paid by Check, status is In checkout.)". When someone pays by paypal, there are two emails sent, the fist subject is "New Order at STORENAME for FIRST LAST (Paid by PayPal, status is In checkout.)", and the second one sent has a subject line of "New Order at STORENAME for FIRST LAST (Paid by PayPal, status is Pending.)".

So, it seems that PayPal is triggering the checkout notification for both the "In checkout --> Pending" transition, and the "Pending --> Completed" transition.

Hope these data points help someone track down the cause and implement a fix.

I had the same problem implementing userpoints ubercart module. It was not only giving me duplicate e-mails, it was giving duplicate points in the system. I am willing to throw down $$ to get this fixed. This seems like a very serious issue affecting everyone with paypal checkout.

Hi,

For those wanting a possible quick fix (I have site going live within 1 week and desperately needed to fix this issue) use the previously commented link here:

http://www.ubercart.org/forum/bug_reports/13716/stock_levels_decrease_tw...

For the record I had to reset my "Customer completes checkout" conditional actions and use the workaround above.

I'm using Ubercart V2.2 with Drupal V6.16 with Paypal WPS and FreeOrder payment methods plus Coupons and also selling Membership - all tested with different combinations and all worked after the work around.

Just wanting to let people know that until this is fixed in code the above is a possible solution that "may" work for you.

Hi,

I'm a new co-maintainer of http://drupal.org/project/uc_atos.
When rewriting this module, I have mimicked the way Paypal module is made.
As a result, I ran into the same bug as the one described in this issue.
But it seems that I've managed to fix it.
So I have applied the same fix to the Paypal module as it should work for it too.
Please review this patch.
It is based on 6.x-2.2 version.
Ensure you rebuild the menu cache after applying the patch, by visiting admin/build/modules/list

Subscribing

Hi anrikun, thanks for taking the time to post your patch. It is nice you took the time to create this even though your not using the PayPal module yourself.

I've done some testing and here are the results. It should be noted that I am using the default conditional actions and have NOT applied any fixes mentioned in this thread.

# Web Payments Standard

(1) Making an order as a logged in user
- Received one order email (correct behaviour)
- The stock was decrement by one (correct behaviour)

(2) Making an order as an anonymous user where a new account should be created
- Received two order emails (incorrect behaviour)
- The stock was decrement twice (incorrect behaviour)

It makes sense this doesn't work as your code branches to the original logic (uc_cart_checkout_complete()) when the user is anonymous ($uid = 0).

# PayPal Express

Just incase anyone anyone is using WPP and wants to also enable PayPal express you'll run into the same issues discussed in this thread. From my testing Anrikun's patch doesn't help fix as PayPal express uses different menu call backs.

Most of this has been said above but I am summarizing it here.

# Whats happening

(1) Customers receive two order emails.
(2) Stock is decrement from the order twice.

# Configuration

Ubercart 2.2
Default conditional actions
WPS (sandbox and live)
I'm also having similar problems when using PayPal Express with WPP

# From the code side

The function uc_cart_complete_sale(…) is being called twice. Once via the PayPal IPN call back and once more when/if the users clicks the return back to store link on the PayPal site.

Here is my understanding of the flow:

(1) User adds item to cart, goes to checkout page choosing WPS and submit, goes to review page and submits the order.
(2) User is now on the paypal page, fills out billing info, is presented with a "Thank you for your payment" message and a button to return to the store. At this point PayPal will send an IPN message back to the site and the user may or may not click the button to return to the store.

(3a) IPN message

- Drupal menu callback "uc_paypal/ipn" is called which calls uc_paypal_ipn($order_id = 0).
- Logic related to the IPN happens.
- If the order is successful (completed) then uc_cart_complete_sale($order) is called.

(3b) User clicks button to return to the store

- Drupal menu callback "uc_paypal/wps/complete/%uc_order" is called which triggers uc_paypal_complete($order).
- A redirect happens and we are sent to drupal_goto('cart/checkout/complete') which is drupal menu callback which triggers uc_cart_checkout_complete().
- Here complete sale is called (potentially for the second time) (uc_cart_complete_sale($order, variable_get('uc_new_customer_login', FALSE));)
- User is redirect to checkout complete page.

# Solutions?

Unfortunately I don't have anything worth while to present right now as I'm still digging into this problem. A few random ideas I've had are:

If it is possible to detect if IPN is working/not-working (enabled/disabled) it should be possible to have logic that leaves the order update work (eg: calling uc_cart_complete_sale) to the IPN callback (3a) and the order complete ui presentation (Thanks for ordering, your order number is X, your login and password is...) to the (3b) call back. However, as it stands uc_cart_complete_sale prepares the order complete html so that logic would need to be duplicated in the paypal module but you then avoid modifying uc_cart_complete_sale.

Another possible fix might be to have uc_cart_complete_sale(…) check the order status before proceeding and thus preventing itself from doing its logic more then once. Problems with this approach will likely be possibly breaking other payment modules and maybe even race conditions with the IPN versus the user getting back to the page.

Anyway, definitely a tricky problem.

Thanks a lot for testing my patch Colin!

The bug you found is a case I had missed because these last days I've always been testing as a logged in user (I got tired of entering shipping addresses again and again :-D).
So the same bug is in uc_atos too and thank to you, I know it exists!
It is an easy bug to fix so I'll provide a new patch tomorrow.

About Paypal EC, the problem is that I don't know much about the difference between Paypal EC and Paypal WPS. I thought there was only one sort of Paypal!
Can you explain me the difference for the user?

Henri

Here is an updated patch.
It is based on 6.x-2.2 version.
Ensure you rebuild the menu cache after applying the patch, by visiting admin/build/modules/list

Hi anrikun,

Thanks for posting an updated patch so quickly! I tested your latest patch here is what I found.

# Web Payments Standard

(1) Making an order as a logged in user

a: Received one order email (correct behaviour)
b: The stock was decremented by one (correct behaviour)

(2) Making an order as an anonymous user where a new account should be created

a: Received one order emails (correct behaviour)
b: The stock was decremented by one (correct behaviour)

# PayPal Express

I noticed you updated your new callback to handle paypal_ec in addition to paypal_wps, nice!

(3) Making an order as a logged in user

a: Received one order email (correct behaviour)
b: The stock was decrement by one (correct behaviour)

(4) Making an order as an anonymous user where a new account should be created

a: Received two order emails (incorrect behaviour)
b: The stock was decrement twice (incorrect behaviour)
- See (3) under feedback.

# Feedback

Just some general thoughts

(1) What about passing the old callback (uc_paypal.module:91) as a argument to uc_paypal_checkout_compelte($original_callback) to avoid having to store a variable.

(2) With this workflow we assume all stores will use IPN and that it is solely responsible for updating the order status / triggering emails. This seems reasonable to me but does anyone else have examples of why you wouldn't/cannot use IPN? Because the user has the option of not browsing back to the store it seems to me that IPN should be required to insure a proper store checkout flow.

(3) Because uc_cart_complete_sale(…) does all the heavy work (update order status, create new user account, send email, and generated output html) it always needs to be run first otherwise your new callback wont have the information it needs (eg: uc_paypal_complete_sale_output) in the case of a new user checkout. From my understanding of the checkout flow we cannot be sure uc_cart_complete_sale(…) will always be called first (race condition). Hopefully I am missing something!

# Web Payments Standard

1. /cart
2. /cart/checkout
3. /cart/checkout/review
4. Fill out PayPal info, etc, click Pay Now button.
5. /uc_paypal/ipn/# (PayPal sends IPN in the background to drupal) [POST]
6. /uc_paypal/wps/complete/# (Customers clicks Return to Store) [POST]
7. /cart/checkout/complete (#6 performs redirect sending us here) [GET]

Now #5 triggers uc_cart_complete_sale(…) and in most cases #5 happens before the customer does #6. However, lets say PayPal's servers are being slow and the users performs #6 before PayPal sends #5 then when the new callback i called (uc_paypal_checkout_complete) it will be missing information because uc_cart_complete_sale(…) hasn't been run yet.

# PayPal Express

1. /cart
2. /cart/checkout
3. Login to paypal, verify payment, shipping, contact info, continue back to store.
4. /cart/checkout/review (Customer submits the order)
5. /cart/checkout/complete (Customer sent here)
6. /uc_paypal/ipn/# (PayPal sends IPN in the background to drupal)

In this situation the user is redirected to #5 before the IPN #6 is sent. So from testing case (3) everything is okay because the user is logged in and the new callback has enough info to generate the html even though uc_cart_complete_sale(…) hasn't ran yet. However, in test case (4) the user isn't logged in and because #6 hasn't happened yet the $order still has $uid=0 which triggers the original callback to be called and the whole duplicate email / stock issues.

I suppose the user creation stuff could be duplicated in the new callback but I'd worry there could be a situation where the new user account is created twice or at least attempted to be created twice due to the asynchronous nature of #5 and #6. Ugh :)

I'll give this some more thought myself!

Colin

Wow Colin, what a great review, thanks!
I will study your very complete feedback.
But first I think I need to understand the difference between EC and WPS.
I'll install a test site to do so!

Henri

subscribing

subscribe

subscribe

Ah such a fun issue :) I've got a client who needs this issue fixed so I am sharing my current solution. I've tested it with PayPal Web Payments Standard and PayPal Express and for my use case at least everything is working nicely.

The patch must be applied against 6.x-2.2.

If you try this patch please provide feedback!

See 644538_20100518_README.TXT - Brief overview of what the patch does.
See 644538_20100518.patch - A patch which modifies uc_cart_complete_sale(…) so that it doesn't perform certain actions twice.
See 644538_OVERVIEW.TXT - A detailed overview of the problem.

Colin

Fantastic detailed summary. It all sounds exactly right.
Patch looks bigger than I thought it would be - seems to be due to the 'create new customer account' step I was unaware of, and I guess most of the bulk is just indenting under the new condition.

This is a rockstar issue report!

Thanks dman.

While the patch may appear large it should be noted that very little code has changed and all changes are limited to the uc_cart_complete_sale(...) function. I did some minor rearranging so that all of the code that should only be run once is grouped together which makes the patch look scarier then it is. If you compare the patched version and non-patched version side by side the changes become more clear.

I know a lot of other people are having this problem and I look forward to hearing everyones feedback.

A patch concerning another problem in uc_cart_complete_sale() has recently been committed to 6.x-2.x.
#488886: Tokens should be processed before running text through an input format.
Could you please provide a version of your patch to apply against the last 6.x-2.x too?

Sure thing anrikun. I'll try to get that done in the next few days.

Colin49. I just tested the patch and can verify it works! Thank you so much for this patch. My client was getting extremely ansy about getting this issue resolved. You rock!!!!

Excellent timing of this patch, Colin49. We just started a project for a client who will need UC and ran into this issue today. Will try it out, but I'm excited to hear this long issue might be coming to a close before we even had to expose it to the client.

Great work Colin!
I need to find to some time to test it.
I've implemented #85 in all the payment modules I've developped: it works but is only a workaround to fix uc_cart_complete_sale() faulty behaviour.
I'm pretty convinced that modifying uc_cart_complete_sale() and adding a new state like you did IS the right solution.
So I want to test your new uc_cart_complete_sale() with my payment modules too and not only Paypal.
If it works, that would mean that this new function should definitely be committed.
And I could them remove all these workarounds from my modules :-)
I'm looking forward to your patch for 2.x-dev!

Who's gonna be #100? ;-)

Hey anrikun, see the attached patch for 2.x-dev. Let me know if you run into any problems.

GuyPaddock: Look forward to hearing your feedback.

I am going to steal #100 :)

While people are testing the patch for 2.2 / 2.x I'd like to start a discussion on how to handle the PayPal workflow (W1 - Case C) discussed in #91 644538_OVERVIEW.TXT. While I've identified this as a PayPal workflow I suspect any other payment module that calls uc_cart_complete_sale(...) twice will be subject to the same problem.

The gist. My patch modifies uc_cart_complete_sale(...) to look for a flag to see if it has already run before, if it has certain operations don't run for a second time. This works great unless your two requests (R1) and (R2) that call uc_cart_complete_sale(...) arrive at the exact same time (very rare but possible nonetheless) and the flag wont have been saved yet. This will cause the function to be run in "first time mode" twice which will once again cause the duplicate issues identified by this ticket.

One solution might be to use Drupal's locking functions (recently introduced to 6.x) to only allow one request at a time to access uc_cart_complete_sale(…). This lock logic could be added within uc_cart_complete_sale(...) or it could be left up to the payment modules themselves to obtain a lock before calling uc_cart_complete_sale(…). In my opinion adding the lock into uc_cart_complete_sale(...) is cleaner and then anyone making a payment module in the future doesn't need to handle this situation but I can see why others might not want to add this extra logic as not all payment modules will need it.

Any other ideas? Anrikun, since you have developed other payment modules I'd like to hear your thoughts on this!

subscribing

Hey anrikun, did you ever have a chance to try my patch for 2.x-dev in post #99? Also, I'd love to hear your thoughts on post #100.

Hi Colin,
Sorry I haven't been able to try your patch yet as all the sites I was developing that use payment methods are now running in production!
So I'm waiting for my bank to open a test account for me. Without it I cannot do any IPN-like test :-(
I should get this account by the end of the week. I'll spend time on #100 too then.
Sorry for the delay!!!
Henri

Thanks for the update Henri. I really appreciate you taking the time to test this and share your thoughts.

Cheers
Colin

I realize there are already patches in the queue here, but I'd like to propose an alternate solution: Simply prevent uc_cart_complete_sale from doing anything other than return the checkout completion message if the order status is already at 'completed'. That's it.

Do we ever need uc_cart_complete_sale() to run on an already completed order? Most if not all of the time, we don't.

In the event that we do, I added a $force_complete argument that defaults to false.

This patch resolved the duplicate emails for me. Note that this is against the most recent dev version, so you might need to cvs up for this to apply properly.

ezra-g, I'm not crazy about that solution, as it was already proposed and discussed in #53, #59, and #75.

In #83, Colin49 wrote:

Another possible fix might be to have uc_cart_complete_sale(…) check the order status before proceeding and thus preventing itself from doing its logic more then once. Problems with this approach will likely be possibly breaking other payment modules and maybe even race conditions with the IPN versus the user getting back to the page.

Essentially, Colin49's solution does do the same type of thing you've proposed, but takes more into consideration. Specifically, generation of the order completion html.

One other question: how does Ubercart handle the scenario where a user needs to make two payments for the same order?

For example, what if the user pays for the order with a certain shipping option, but the shipping option for the order later needs to change and the user has to make a second payment to cover the balance?

I can think of at least two reasons why the shipping option would change; either:
- An admin discovers that the type of shipping for the particular product isn't available (i.e. a listing error).
- The customer calls in to change shipping options after placing the order (maybe they need it to arrive sooner?).

subscribe

It's been a while since I've looked at this issue so unfortunately things are foggy again.

ezra-g,

I like moving the check out completion message into a new function. My patch stored user_type, new_user, and new_password in the order data versus the sessions. This way any second calls to uc_cart_complete_sale(…) could get that info as the second request will most likely be a different session (PayPal IPN versus logged in user).

Checking that the status == completed and using force_complete might not always work. Anyone know what happens when an item is shippable? I would think that when you pay it will go into payment received not completed unless I am missing something? If thats not the case it should be fine then.

Re-reading my 644538_OVERVIEW.TXT you'll want want to make sure the following items are always done on every call to uc_cart_complete_sale(…)

(T2) Generate default text for the order completion page (includes order number and new account info if it was generated).
(T4) Cleanup (empty cart and clear stuff stored in the session)
(T6) Return generated text to be used on checkout completion page.

(T2)/(T6) you handle though the new_password and new_login will be empty in certain situations without the changes mentioned above. T4 should be done for proper cleanup. If the PayPal IPN request (R1) comes in first it will have things cleaned up but when the user request (R2) comes in it wont.

GuyPaddock, no idea about handling two payments. How does this currently work?

colin,

I am using the module on www.flatwarefinder.com and all my items are shippable. You are right, when an order is made, it goes into 'Payment Received' status, not 'Completed'. I manually mark it as completed only after I have shipped the order.

BTW: I sometimes get double messages, and sometimes not.

dpatte, have you tried my patch #91 (stable release) / #99 (dev release) or ezra-g patch #105 (dev release). Please share your testing results!

Apologies, but even setting aside the valid criticism of my patch from #105, it's not the one I meant to upload. Mine was mean to check $order->order_status, not $order->status, which doesn't exist.

Feedback from the patch in #91.

It stops the duplicate emails etc BUT im using ubercart userpoints so now instead of having the points added twice, they are now not being added at all.

Edit: For some reason the user points that were set for each product where being set back to zero no matter how many times you tried to update them.

I then tried creating a new product and it now seems to be working correctly.

Colin,

Thanks for contributing this much-needed patch!

I used your #91 patch and it seemed to solve the duplicate email / stock decrement issue. However, I found one side effect.

I have a CA set up to email me on trigger "Customer completes checkout" when the balance is above $0. This is an indication that the Paypal fraud filters I have setup are holding the transaction, or that something didn't go right.

It seems that any CA set to trigger on "Customer completes checkout" to send an order email (with or without any conditions) throws an error when the customer returns from Paypal.

The error I get is "Unable to send e-mail. Please contact the site administrator if the problem persists." Also, above my header I get "Invalid address: You must provide at least one recipient email address. " The email address is definitely valid, and this CA performs fine prior to the #91 patch applied.

I've disabled this CA for now. Just thought I'd mention it in case it might affect others' CA emails as well.

jmuessig: Thanks for the feedback! I'll try to reproduce this error but it sounds like you've found a problem. Did anything show up in your php error log? Or in the Drupal error log in terms of what line number the problem is happening on? Also, just double checking that you are experiencing this problem against the stable release correct?

adam_c: I don't have any experience with uc user points. Were you having difficulties with user points being set to 0 after check or? Or when modifying a products settings?

When modifying node feature settings, you would think that this problem would be far removed from anything in the patch, its just the fact that this behaviour started precisely after I applied the patch. Extremely coincidental?

Hi Colin,

Yes, I tested against fresh installs of Drupal 6.17 and Ubercart 2.2 (plus the latest stable version of token). I believe that's all I installed on my test server to verify the problem. I should mention that I am using Paypal sandbox, I don't know if this makes a difference or not (repeated testing on Paypal live server would be expensive...)

After further testing, it appears to be some kind of race condition (maybe related to the original issue?), as the failure mode is intermittent and varies. And, sad to say, one of the test iterations sent duplicate customer emails and decremented the stock twice (though I did have the order email CA enabled - I will try to reproduce with this CA disabled).

In regards to the duplicate email / stock decrement: Please see the log info attached. Unfortunately the original duplicate stock decrement / email issue reared its ugly head again (though it is much harder to reproduce than before, and I did have an order email CA with trigger 'customer completes checkout').

smtp	2010-06-29 11:50	Sending mail to: test_eight@hydrophase.com	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: store@hydrophase.com	Anonymous	
ca	2010-06-29 11:50	Attempt to e-mail concerning order 1349 failed.	Anonymous	
mail	2010-06-29 11:50	Error sending e-mail (from ...	Anonymous	
smtp	2010-06-29 11:50	Error sending e-mail from store@hydrophase.com to : ...	Anonymous	
smtp	2010-06-29 11:50	Sending mail to:	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: bob@hydrophase.com	Anonymous	
uc_paypal	2010-06-29 11:50	IPN transaction verified.	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: test_eight@hydrophase.com	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: store@hydrophase.com	Anonymous	
ca	2010-06-29 11:50	Attempt to e-mail concerning order 1349 failed.	Anonymous	
mail	2010-06-29 11:50	Error sending e-mail (from ...	Anonymous	
smtp	2010-06-29 11:50	Error sending e-mail from store@hydrophase.com to : ...	Anonymous	
smtp	2010-06-29 11:50	Sending mail to:	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: bob@hydrophase.com	Anonymous	
smtp	2010-06-29 11:50	Sending mail to: test_eight@hydrophase.com	Anonymous	
uc_paypal	2010-06-29 11:50	Receiving IPN at URL for order 1349.

In regards to the email error: It appears that somehow the recipient address for an order email to be sent as a result of a CA triggered on 'customer checkout complete' isn't sent (maybe a timing issue?). There is no indication of line number in the drupal logs; I pasted the log info below. Note there is no email address for the SMTP error "Sending mail to: ______". The details for that log entry are: "Error sending e-mail from store@hydrophase.com to : You must provide at least one recipient email address."

ca	2010-06-24 22:00	Attempt to e-mail concerning order 1339 failed.	Anonymous	
mail	2010-06-24 22:00	Error sending e-mail (from ...	Anonymous	
smtp	2010-06-24 22:00	Error sending e-mail from store@hydrophase.com to : ...	Anonymous	
smtp	2010-06-24 22:00	Sending mail to:	Anonymous	
smtp	2010-06-24 22:00	Sending mail to: bob@hydrophase.com	Anonymous	
smtp	2010-06-24 22:00	Sending mail to: sadfjkljlksf@hydrophase.com	Anonymous	
uc_paypal	2010-06-24 22:00	IPN transaction verified.	Anonymous	
uc_paypal	2010-06-24 22:00	Receiving IPN at URL for order 1339.

Colin,

Testing without my custom CA enabled, there still appears to be a timing issue somewhere. I don't seem to have the many issues that comes from having the custom CA, but probably 1 out of 10 cycles through the checkout process I don't get any checkout message besides the checkout completion message header.

For example, upon return from Paypal, I would normally get the checkout completion message header plus the checkout completion for new users. Every so often, only the checkout completion message header is displayed. Of these times, sometimes the new user account login / password info is emailed, other times it is not. There is no useful information that I can find in the logs.

Curious if others are experiencing the same.

Justin

subscribing

The patch must be applied against 6.x-2.2.

If you try this patch please provide feedback!

See 644538_20100518_README.TXT - Brief overview of what the patch does.
See 644538_20100518.patch - A patch which modifies uc_cart_complete_sale(…) so that it doesn't perform certain actions twice.
See 644538_OVERVIEW.TXT - A detailed overview of the problem.

Colin

Attachment Size
644538_20100518_README.TXT 1.84 KB
644538_20100518.patch 10.31 KB
644538_OVERVIEW.TXT 6.95 KB

I can confirm that this patch solved ALLLLLLLL my problems, including one of two "malformed header from script" errors which resulted in an Internal Server Error when completing checkout (I am using PayPal IPN).

I still get this error in my error log:
[client [Paypal server IP address] ] malformed header from script. Bad header=No recipient addresses found i: php-script

Previously, I was getting the same error which resulted in an Internal Server Error being presented to the user on checkout - ie As well as the above error, I got:
[client [user's IP address] ] malformed header from script. Bad header=No recipient addresses found i: php-script

Now, at least, no one sees the error except in my logs. I don't need this issue fixed, but for anyone else who is having similar problems, I thought it might be useful to know.

I love you, colin.

Thanks for the feedback Tommy. It's probably safe to say the error your getting is related to the error jmuessig is having. My patch must break the formatting of one of the emails that get sent. I am suspecting that one of the needed variables is set in (R1) but not set in (R2) or vice versa.

Anyway, I haven't forgotten about this but unfortunately my free time has been non-existent lately. I'll try to look at this soon!

Colin

same happens after the 2.3 update again.
is the patch working with version 2.3 as well?

@colin49: Sorry to take so long to get back to you.

Thinking back on it, I'm not sure why I asked about making two payments for the same order, because AFAIK, UberCart doesn't provide customers with any way to initiate a second payment -- they can only pay during checkout -- so this shouldn't pose a problem. So, for the scenarios I descried, admins just have to have the customer make a second manual PayPal payment, and then they record the additional payment manually under the "Payments" tab of the order.

@Colin49: I'm sorry to report that when I just tested the patch from #99 on a vanilla install of UC 2.3, it did not work as expected for anonymous checkout. I completed check-out using an e-mail address that was not yet assigned to an account. For testing purposes, I opened a second tab once I was at the PayPal confirmation page that displays the store return link, and browsed around on the site for a bit before returning to click the return link in the first tab.

The odd part was that the site told me that my order was added to the account already on file, even though I didn't have such an account before the order. I did receive the account creation e-mail, though, but I also received two order notification e-mails -- one from when the PayPal payment went through (the IPN), and one from when I clicked the return link.

I am happy to report, though, that I only received one e-mail when I completed check-out while logged-in. Luckily, the client I mentioned earlier only supports completing check-out while being logged-in, so this isn't an issue, but I wanted to report it.

We have the same problem with version 2.4.
Is there any confirmation on it and any idea how to fix it?

Hi there. Just completed the upgrade to UC 2.4 on our site. All testing on the sandbox did not reveal any duplicate stock decrements or emails. When we switched over however, one of our live test orders (don't do many of these for obvious reasons) caused the bug to appear. We entered the order when logged in and returned to the site after doing the paypal payment. Here is what the order status shows:

21/08/2010
8:45:38 AM - The stock level for H1430Y_Assortment2_Large has been decreased to 2.
21/08/2010
8:45:38 AM - The stock level for H1430Y_White_Large has been decreased to 2.
21/08/2010
8:45:38 AM - The stock level for H2110D_Grey_Medium has been decreased to 2.
21/08/2010
8:45:38 AM - The stock level for H2543T_Assortment1_Medium has been decreased to 3.
21/08/2010
8:45:38 AM - The stock level for H1473T_Assortment2_Large has been decreased to 0.
21/08/2010
8:45:38 AM - The stock level for WAT909-2_Assortment1_Medium has been decreased to 3.
21/08/2010
8:45:38 AM - The stock level for H2110D_Black_Medium has been decreased to 2.
21/08/2010
8:45:39 AM - The stock level for H1472T_Assortment4_Medium has been decreased to 0.
21/08/2010
8:45:39 AM - PayPal IPN reported a payment of 57.79 CAD.
21/08/2010
8:45:58 AM - The stock level for H1430Y_Assortment2_Large has been decreased to 1.
21/08/2010
8:45:58 AM - The stock level for H1430Y_White_Large has been decreased to 1.
21/08/2010
8:45:58 AM - The stock level for H2110D_Grey_Medium has been decreased to 1.
21/08/2010
8:45:58 AM - The stock level for H2543T_Assortment1_Medium has been decreased to 2.
21/08/2010
8:45:58 AM - The stock level for H1473T_Assortment2_Large has been decreased to -1.
21/08/2010
8:45:58 AM - The stock level for WAT909-2_Assortment1_Medium has been decreased to 2.
21/08/2010
8:45:59 AM - The stock level for H2110D_Black_Medium has been decreased to 1.
21/08/2010
8:45:59 AM - The stock level for H1472T_Assortment4_Medium has been decreased to -1.
21/08/2010
8:45:59 AM - Order created through website.

I was about to consider adding the patch but now will hold off until I hear more. I am going to write a simple script to back out an order as it will come in handy for this and other reasons.

I am going to hammer the test site and sandbox to see if I can reproduce this. Haven't been able to date which may suggest this is partly a PayPal issue.

Subscribing

Subscribing

The provided version of the dev patch in #99 patches successfully on 6.x-2.4. I have not, however, had a chance to see if I'm experiencing the same issues mentioned in #125 & #126. Thanks so much for this patch!

subscribe

subscribing

subscribing

subscribe

@affaneh_akram: Please read http://drupal.org/node/317 to learn how to use the issue queue.

Subscribing

+1

+1

sub... seems crazy to me that this is still an issue after almost a year.

Ok so I tested #99 with 2.4. I am using:
-anon checkout
-standard conditional actions (I reset them)
-paypal wps
- these checked:

- Send new customers a separate e-mail with their account details.
- Login users when new customer accounts are created at checkout. (this doesn't work, but thats a different issue)
- New customer accounts will be set to active..  I get one email for all the emails sent (store owner, order - confirmation, account details, and file downloads)

Also, just to note, my order complete page looks like:

Your order is complete! Your order number is 7.
Thank you for shopping at . A new account has been created for you here that you may use to view your current order status.
Login to your new account using the following information:
Username: test10
Password: ******

Before the patch it looked similar but did not out put the "username:" and "password" thing.

In summary, it works well. (note: I am not using uc_stock and my items are downloadable).

So the patch at #99 allows NEW users to be sent the correct message after checkout, ie a "completion_new_user" message. Without that patch, I never get a user to get the completion_new_user message.

After looking into this some more, It looks like the whole reason paypal wps gets duplicate messages is because the conditional actions are being called twice. Once when the user returns to the site from paypal (after paying) and once from the IPN the paypal sends.

The function uc_paypal_ipn calls "uc_cart_complete_sale" when the sale is completed. I think this is a mistake as "uc_cart_complete_sale" does a lot of things (creates user, logs in a user, sets the message type, unsets variables in cart workflow, etc...), most of those things are not needed for the IPN. In fact they even cause problems for auto-login (see http://drupal.org/node/658470#comment-3496250).

So, I'm messing w/ a couple changes. I'll let everyone know how that goes.

subscribing

Hey all,

I just posted a patch here: http://drupal.org/node/658470#comment-3499508

It fixes that issue for paypal_wps as well as this issue (if your using paypal wps. I don't know if there is duplicate notification with other payment methods).

I kind of duplicated some code, but I did that so that other payment methods wouldn't be affected and wps would work.

subscribing

subscribing

subscribing

Subscribing.
So which patch should we try?

+1

this thread is sooooo long !
i'm using paypal wps and atos/sips payments

@miaoulafrite: atos/sips payment has a builtin fix to this issue.

@anrikun > fine, therefore remains for paypal wps with no impact on atos/sips: do you have any suggestion?

My suggestion is to try my patch at #85.
But it is based on 6.x-2.2 version.

Thanks a lot anrikun

but.. i run 6.x-2.4, do you have another proposal? any manipulation of conditional actions?

Hrm... I'm getting the same problem, and have been for about 5 months, but never posted an issue here. So, subscribe.

I've re-jigged the default notification and stock conditional actions as a workaround to this issue... notifications are only set now when the order is set to Pending or Completed (but not if it is changed from one to the other).
Stock is only deducted if the order is set to completed, or a new status I've created ('stock on hold'), but again not if it is changed from one to the other.

Not perfect but it seems to be getting around the problem in my setup at least.

@Khalor, yeah, but you're using only paypal don't you?

@miaoulafrite, yep correct... plus some of the payment methods included with UC (COD, cheque) - but Paypal was the only one to have duplicate stock reduction/notification

When creating this patch: http://drupal.org/node/658470#comment-3499508

I noticed that the reason their are duplicate emails is because the IPN triggers for emails to be sent AND the user getting back to the site triggers emails. In the patch above I broke out the trigger into a separate trigger so it could be called only once.

@160 - i found that to be the problem for my site as well.

I have an action triggered by order complete which creates a file. I'm getting duplicates, but they may be a day apart or week apart. Could it be happening when a user revisits the site or page? Maybe they go to check their order status and it gets triggered again? Anyone else experiencing this? Could someone put a mail message to themself in the PHP action area to notify when the order complete is triggered to see if it happens for them as well? I'm using Cybersource, not PayPal. Thanks! :)

subscribe

i'm kind of lost with this thread.

could any maintainer provide a method to help me (us?) to have a better understanding of what to do in the following situations:

1. only PayPal WPS is used
2. PayPal WPS is used AND other payments methods are also used (credit card, ...) > these payment methods do not have the problem of decreasing stock twice, and so on..

will this be fixed in the next Ubercart release?

thanks a lot for your help

Subscribe

Hi Anrikun, is there any chance that you could update your patch for the 6.x-2.4 version? Thanks for your help. I live in hope :-)

I'm going to post this here since it is the biggest thread so hopefully it will reach the most people.

The attached patch fixes a couple issues:
- fixes duplicate emails when using paypal WPS while leaving other payment systems alone
- fixes logging in users when using anonymous checkout - http://drupal.org/node/658470 (takes some code from cha0s's patch here: http://drupal.org/node/423546)

This is basically a combination of the patch I talked about on #145 and cha0s's patch. Its pretty simple and works well. Please test this and let me know.

Edit: oh this patches against 2.4

Applied patch in #167 (Thanks t03g) to my UC 2.4 sandbox. Applies clean with this:

patch -p1 -b <0001-applied-the-chaos-patch-and-fix-and-added-fixes-for.patch

Initial testing with WPS for both anonymous customer as well as logged-in customer appear to work as it should against Paypal sandbox at least.

My config:

• Don't automatically login new customers;
• All CAs reset to default;
• WPS only;
• Removed user e-mail address from uc_orders, and users tables before each test.

But, I need a few things clarified...

1. Should the order be left in "Payment received" status? (Actually, I'd love to see some documention as to what the *normal* order of the statuses is supposed to be (but that's another matter I suppose)). Here's an example log:

   Log:

   Fri, 2010-12-03 14:29	7143	
       * Order status changed from In checkout to Pending.
   Fri, 2010-12-03 14:29	-	
       * PayPal payment for $68 entered by -.
   Fri, 2010-12-03 14:29	-	
       * Order status changed from Pending to Payment received.

   Comments:

   2010.12.03
   2:29:46 PM	-	The stock level for 0019100104-01 has been decreased to 0.
   2010.12.03
   2:29:46 PM	-	Order created through website.
   2010.12.03
   2:29:56 PM	-	PayPal IPN reported a payment of 68 CAD.

2. From what I've read around here, I'm guessing some of the maintainers will be opposed to *any* payment gateway specific mods to core ubercart at all, such as we have in this patch. The argument will probably be along the lines of "The other gateways work with core as-is, why can't WPS?"

3. Someone needs to spell-out for me what is missed/broken if we simply comment-out the call to uc_cart_complete_sale() on line 106 of a v2.4 uc_paypal.pages.inc. I have tried disabling the call and it appears to fix the duplicate e-mails/duplicate decrements as well, on my test site, so I presume all the additional code adjustments are for a different setup than I've got, or there are other more subtle problems I'm not noticing. In other words, is all this new functionality really necessary for our fix? Is it just because we're really clinging to the idea that we want the redundancy of both the IPN & the customer return to site triggering the order completion? As has been pointed-out previously, disabling line 106 does not disable the IPN message, just the triggering of order completion, and I guess I'm still not clear what we lose if we forego that on the occasional time when the PayPal auto-return fails.

4. It's not clear to me if this particular patch actually avoids the risk of some variant of a race condition between when IPN msg arrives back at the site, and when the customer arrives back at the site, even though it looks like this patch creates some sort of flag mechanism. I guess I should stare at the code some more. ;-), or perhaps someone more conversant with the logic required here could lay it out for us.

Thanks all for the patches that have been contributed so far to this prob. It is amazing this is still an ongoing issue, but I guess it's a tricky one, or something. On my prod site I've decided to disable the Order invoice e-mails altogether for now, since customers can get the same invoice by just logging-in to their order page. Is this what others are doing to avoid the dupes?

flashfasbo,

Should the order be left in "Payment received" status? (Actually, I'd love to see some documention as to what the *normal* order of the statuses is supposed to be (but that's another matter I suppose)).

You will get stuck at payment recieved if your product is marked as shippable (this is in cond. act.s). There may be other reasons too (cant remember off the top of my head), but I think this is the most common reason.

In the WPA process, we don't consider the payment "authorized" or paid until the ipn says so. In most other payment systems, simple getting to the complete page sorta means we were authorized.

From what I've read around here, I'm guessing some of the maintainers will be opposed to *any* payment gateway specific mods to core ubercart at all, such as we have in this patch. The argument will probably be along the lines of "The other gateways work with core as-is, why can't WPS?"

WPS is a different though because you leave the site to pay and coming back to the site isn't what "verifies" payment.

Someone needs to spell-out for me what is missed/broken if we simply comment-out the call to uc_cart_complete_sale() on line 106 of a v2.4 uc_paypal.pages.inc.

Here is "why" we get multiple emails:
- The user comes back to the site and the complete trigger is pulled, sending some emails and marking as complete
- The paypal IPN comes back to the site, and the complete trigger is pulled, sending some emails and marking as complete

We have 2 different actions causing the trigger to pull. The problem is that these 2 actions come at different and variable times. Sometimes the ipn gets back before the user, and sometimes the ipn will take over 10 seconds to get back (getting back way after the user usually).

So if you take out uc_cart_complete_sale() from uc_paypal.pages.inc, you wont have issues if the ipn gets back before the user does. But if the ipn gets back after the user, the payment wont get marked as complete and emails like "file downloads" (for example) wont ever get sent.

It's not clear to me if this particular patch actually avoids the risk of some variant of a race condition between when IPN msg arrives back at the site, and when the customer arrives back at the site, even though it looks like this patch creates some sort of flag mechanism.

This for this issue, this patch is actually very small, I just included the "login" issue into it also, maybe I should separate it out so its easier for you to see.

Basically what I did was split the "complete" trigger ( ca_pull_trigger('uc_checkout_complete', $order, $account); ) into 2 triggers:
1. One marks as complete which also sends emails that have to do w/ it being complete (example: file download email)
2. One sends the normal emails that are sent whether the order is complete or not.

Then all I did was have a simple switch that says, if the uc_cart_complete_sale() is being called from the wps ipn, dont ever pull the second trigger. We will just let the user returning to the site do that.

I typed this out really fast so hopefully this makes sense.

The patch in #167 doesn't account for orders where the payment type is Express Checkout rather than WPS, so the Express Checkout method still sends out a duplicate order confirmation email when the IPN is received.

I assume the fix would be just to change:

<?php
if ($order->payment_method == 'paypal_wps') {
?>

into:

<?php
if ($order->payment_method == 'paypal_wps' || $order->payment_method == 'paypal_ec') {
?>

Or is it not as simple as that?

I applied patch #167 and everything seemed good initially.
Since then I've experienced some strange behaviour that I'd never encountered before.
Certainly, I no longer get duplicate emails and stock decrements. Now, for some orders I get no emails at all, and on rare occasions I get an order stuck "in checkout", because the IPN failed to get through. In my log I have entries like "IPN for a different PayPal account attempted".
I'm not saying patch #167 is definitely to blame for this, but it's the only thing I changed :-(

The "IPN for a different PayPal account attempted" is an issue with your primary email account stuff... see this issue and patch: http://drupal.org/node/881606#comment-3771452

The same issue is probably the reason no emails were sent. Try it out and see if it fixed your problem.

@-morbiD- It might be that simple... I haven't looked at the express checkout code yet. Would you mind posting the IPN array that paypal returns when using EC?

Sub.

I'm only running in sandbox mode, but here's the IPN array from Express Checkout, as requested (with email addresses removed):

Array
(
    [mc_gross] => 4.00
    [invoice] => 17-1291900879
    [protection_eligibility] => Eligible
    [address_status] => confirmed
    [payer_id] => 6EEE4NZTJ6QQN
    [tax] => 0.00
    [address_street] => 1 Main Terrace
    [payment_date] => 05:21:21 Dec 09, 2010 PST
    [payment_status] => Completed
    [charset] => windows-1252
    [address_zip] => W12 4LQ
    [first_name] => Test
    [mc_fee] => 0.34
    [address_country_code] => GB
    [address_name] => Test User
    [notify_version] => 3.0
    [custom] => 
    [payer_status] => verified
    [address_country] => United Kingdom
    [address_city] => Wolverhampton
    [quantity] => 1
    [verify_sign] => A7ZIRsIqq6KUrjeM-Qnnkko3duWnALEFdeZNWWrFyLLg4EeA9JPlicWe
    [payer_email] => removed@example.com
    [txn_id] => 35F82042RK2030151
    [payment_type] => instant
    [last_name] => User
    [address_state] => West Midlands
    [receiver_email] => removed@example.com
    [payment_fee] => 
    [receiver_id] => ARP5QRTQR32PJ
    [txn_type] => express_checkout
    [item_name] => 1× Classified Advert
    [mc_currency] => GBP
    [item_number] => 
    [residence_country] => GB
    [test_ipn] => 1
    [handling_amount] => 0.00
    [transaction_subject] => 
    [payment_gross] => 
    [shipping] => 0.00
)

morbiD,

The patch I talked about before probably wont help you...

When you get the "IPN for a different PayPal account attempted" email, is the "[receiver_email]" the same as the one used in your ubercart store settings?

@that0n3guy

Sorry if I'm being dumb, but I haven't seen any "IPN for a different PayPal account attempted" messages anywhere. I'm receiving IPNs successfully but the "customer" is being sent two order confirmations when Express Checkout is used, which the patch fixed for WPS.

To answer your question about [receiver_email] anyway: I have entered the same email address, both in the WPS settings under "Payment method" and in the Express Checkout settings under "Payment gateways" and the [receiver_email] in the successful IPN matches that email address exactly.

Is that what you wanted to know?

Also, I tried making the change I asked about in #170 and it doesn't seem to have broken anything, and did stop the duplicate order confirmations with Express Checkout.

Oh sorry, it was mperry that had that "ipn for a diff.." issue.

I'm glad #170 fixed your issue, I will include it in a patch sometime (when I have time).

I have the same duplicate email problem.
Using Paypal as the only payment method in Ubercart 2.
Which method of fixing it should I try?
Any recommendations?

Did you try the patch in #167?

subscribe

If the patch from comment #167 is working for people, is there any chance of getting it rolled into the next release?

I'm glad I found this thread as I too have been having trouble with this bug for a few days now (my first Ubercart deployment). In the case of file downloads, this bug is causing a double granting of files.

that0n3guy's patch at #167 looks promising, but I have a comment and a possible bug:

Wouldn't it be best to trigger the uc_checkout_complete action when the PayPal IPN request comes in and not when the user returns to the site? As it is coded now, the PayPal IPN request triggers uc_checkout_complete_paid and the user's return visit to the site triggers uc_checkout_complete. The second trigger (uc_checkout_complete_paid) sends the email's to the user (and store admin). I believe the uc_checkout_complete trigger is also needed to update the order's status beyond 'Payment Received' if that is applicable. In my opinion, these emails should be sent the moment the site know's the that order has been paid for. If for some reason, the user never returns to the site, the emails will never be sent.

As for the bug, I haven't done extensive testing to be completely sure, but I think that the unset($order->data['user_wpspaypal_ipn']); command is never being saved to the database and as a result, the second call (from the user's return visit to the site) is not even triggering uc_checkout_complete. I added a call to save the order after the unset like:

      unset($order->data['user_wpspaypal_ipn']);
      db_query("UPDATE {uc_orders} SET uid = %d, data = '%s' WHERE order_id = %d", $order->uid, serialize($order->data), $order->order_id);

Have no Ubercart maintainers picked up on this bug yet? No assignment? It seems like this has been kicking around for years and is a fairly major bug for the PayPal gateway.

Wouldn't it be best to trigger the uc_checkout_complete action when the PayPal IPN request comes in and not when the user returns to the site?

Hmmm... If we did that, then the trigger (uc_checkout_complete_paid) wouldn't even be needed. A problem with this might be that the user would come back to the site and go look at their orders... if they come back and trigger (uc_checkout_complete) has never been flagged yet, would that order even show up? (probably not a big deal though b/c it would show up 4-10 seconds later when the ipn gets there.

If for some reason, the user never returns to the site, the emails will never be sent.

I never thought about that, but yes, that would definitely be an issue.

Have no Ubercart maintainers picked up on this bug yet? No assignment? It seems like this has been kicking around for years and is a fairly major bug for the PayPal gateway.

Its my opinion that paypal wps is not very supported by the main ubercart guys. Most of them probably don't use it. But I think thats whats keeping ubercart out of the hands of the masses. If someone owns a really small biz, their are so many reasons to use paypal over something else.

A problem with this might be that the user would come back to the site and go look at their orders... if they come back and trigger (uc_checkout_complete) has never been flagged yet, would that order even show up? (probably not a big deal though b/c it would show up 4-10 seconds later when the ipn gets there.

I think it could be a big deal in some cases. I don't know how often PayPal's live servers have problems, but recently their sandbox servers had some issue while I was doing some checkout testing, and IPN's didn't get sent back to my site for a number of hours (they were queued up and sent out when the issue was resolved). I don't think you can count on always receiving an IPN within a few seconds.

Its my opinion that paypal wps is not very supported by the main ubercart guys. Most of them probably don't use it. But I think thats whats keeping ubercart out of the hands of the masses.

Given Ubercart is currently in use on over 30,000 websites, I'm not sure it's reasonable to suggest that there is anything "keeping ubercart out of the hands of the masses"!
:)

I am running Ubercart 6.x-2.4 and have the same problem with duplicate stock decrement for PayPal orders. There are a few patches that have been posted here. Which one would you recommend for the version I am running? Thanks.

So far I think the patch at #167 is your best bet, but it looks like there needs to be more work done to it, based on the comments in #184.

I want to add more to this. Do we have a chance yet at the longest thread on drupal.org? :)

I ended up taking a different approach than the patch proposed in #167. I'm not sure which approach is better, but the fix and patch I'm proposing here is working for me so far. I have been turning my brain to mush over the past week running test after test. One thing I would like to say is that PayPal's IPN is a horrible idea -- why can't PayPal just send the confirmation info and return back to the site the moment the customer clicks "Pay Now" ?? What's the point of having one more page for the customer to look at? Okay, that's the end of my rant. On to a possible solution......

My situation is that my client's store has a combination of shippable items and file downloads (songs). Not only was the customer getting double order emails, they were also getting double file downloads granted to them and double notification emails about the downloads. This is the same problem that happens with the stock decrement. Patch at #167 wasn't perfect for me (but it might work for you) because I really wanted the customer to be notified of the order and file downloads when the PayPal IPN data comes back to the site and not when the customer returns to the site. Patch at #167 also didn't work for me because it defers the user account creation until the customer returns to the site, but if the PayPal IPN data comes back first, an account needs to be created for the granting of file download permissions. The result was that they were never granted file download access because that only happens when the order status gets updated to "Completed". That order status update happened during the IPN request, which didn't create the user account. So it was a catch-22. This is the same for the granting of roles.

I also tested (extensively) for the following four scenarios:
A) Anonymous user, IPN data returns to site first, customer returns to site second.
B) Anonymous user, customer returns to site first, IPN data returns second. (It is often delayed)
C) Logged in user, IPN data returns to site first, customer returns to site second.
D) Logged in user, customer returns to site first, IPN data returns second.

My solution requires a dual approach:

1. Change all of the predicates in the "Customer completes checkout" trigger to have an additional condition that checks for a "Payment Received" status on the order.

2. Two small patches to the uc_paypal module (the 'uc_paypal.pages.inc' file to be specific)

************************************

1) Change all of the predicates in the "Customer completes checkout" trigger to have an additional condition that checks for a "Payment Received" status on the order.

My "Customer completes checkout" trigger has the following 4 predicates installed in it:
Decrement stock upon order submission, E-mail admin checkout notification, E-mail customer checkout notification, Update order status upon checkout completion with full payment.

Update each of these predicates so that they contain a condition to: "Check Order Status" = "Payment Received". This includes the "Update order status upon checkout completion with full payment". That last one sets the order status to be "Completed", but only if the order balances is <= 0 AND the current order status is "Payment Received".

The "Customer completes checkout" trigger is called TWICE. Once by the PayPal IPN request and once by the customer returning to the website. The order status will be set to "Payment Received" only one of those times. During the second request, the order status will be set to "Completed" by the last predicate of the first request. This prevents the emails from being sent twice, a double decrement of stock, and it prevents a double granting of file downloads (or roles). Whichever request comes in first, that one will create the user account for anonymous users. The second request will have an updated order that already has a user ID assigned to it.

2) Two small patches to the uc_paypal module (the 'uc_paypal.pages.inc' file to be specific)

My patch file is attached. Works for 2.4 or 2.x-dev.

I make 2 changes in this patch file:
a) The "uc_paypal_ipn()" method is called by the PayPal IPN request coming into the server. I simply add a line of code that reloads, or updates, the $order variable so that it has an update-to-date status value after the PayPal payment information is recorded into the order. It basically makes sure the $order->status is set to "Payment Received" before the "Customer completes checkout" trigger is activated. I believe it is actually a bug that this method didn't reload the $order variable after calling uc_payment_enter(). So this is just a good idea to do anyway.
b) The "uc_paypal_complete()" method is called when the customer returns to the site. It also ends up calling the same method as the IPN request, uc_cart_complete_sale(), which triggers the "Customer completes checkout" predicates. The patch adds some simple code to make sure that if the IPN request has already come in and been processed, and if it created a new user account for the anonymous customer, the new user account will be loaded and logged in (if that setting is specified). I needed automatic login for the anonymous users so that they can download the files they have been granted access to.

************************************

Two defects in this solution that I am aware of and I might try to fix soon:

1) If the IPN request comes in first, and a new user account is created, the customer will not see the user account information on the page when they return to the site. They will be logged in automatically, but the username and password given to them won't be displayed since that message text was generated and sent to PayPal when the IPN data came in. They will get an email sent to them with this information, so it's not a total loss.
2) If the PayPal IPN comes in at the same time as the user returns to the site, I'm not entirely sure what goes down, but it's probably not a good thing. I might have to add a locking mechanism so that only one of them can be handled at a time.

Comments?

Thanks!

Chiming in to say the above patch looks like a neat solution to the automatic login problem. The approach in #167 adds extra complexity with the second trigger and the hardcoded check for PayPal WPS.

However I think that the root issue is that the "customer completes checkout" trigger should never be pulled twice - after all, the customer really only completes checkout once, and we shouldn't have to work around this with extra conditions. There must be some way around this - perhaps only pull the trigger if the cart is not empty?

Regarding the defects mentioned above:

1) I think the email will have to suffice here, and in practice this hopefully shouldn't be a problem. We must create the account during IPN if possible, as the user may not return from PayPal. But if the IPN comes first and the user does return, in order to tell them the password wouldn't we have to store it in plaintext (which is a security risk)?

2) This will hopefully occur very rarely, but I agree it could be a problem - however it should be fairly simply to put a lock around the trigger.

@#189... you are incorrect about a couple things. Patch 167 DOES create a user if IPN gets back before the user (or if user never gets back). There is no way to complete the order if there isn't a user so this has to happen. Some of the "defered user creation" comments in the code were taken from the chaos patch... but it does not defer the user creation.

Also, just so you know, the "defered" code is what allows the user to see the account info (username & pass) when they get back to the site... which is your "defect" number 1.

Lastly... Doesn't setting the condition "Check Order Status" = "Payment Received" make is so that the user wont get a checkout email until the IPN gets back? I've heard of cases where the IPN can take like 4 hours to get back... so the user wont get an email saying they made an order until then....

@190...I agree that a customer only completes checkout once. The problem is with the "idea" that completing checkout == completing payment... it does not. BUT our triggers act like it. So it seems like we need to separate our triggers, not because we can't get it to work a different way, but because completing checkout != completing payment.

When the user completes checkout, they should get an email saying they made an order, even if the payment isn't verified for hours/days later. This shouldn't depend on the payment received.

Here are some other ideas:

1. we route the user to a different "page" other than uc_cart_checkout_complete() just for paypal. This idea would probably cause us to duplicate the user creation code in uc_cart_complete_sale() and we would still need to split the triggers.
2. we don't allow the IPN to call uc_cart_complete_sale(). Probably the same as the one above... need to duplicate code and split triggers.
3. we have a "check" that see's if uc_cart_complete_sale() has already been run once for this order... and it might need to know if it was the user or the IPN that ran it first. But again... we need to split the triggers. (but I still feel thats a good thing).

Very lastly... the hardcoded IPN check could easily be turned into an general flag that any payment system could use (anything with similar problems to paypal) so I don't see that as an issue.

Ok, I have redone patch at #167 based on a lot of suggestions I've seen here. I did a better job commenting my code this time so hopefully its easier to read for you all. I used selenium ide (you should try it, its quick and easy testing) to test. All of these situations work well and were tested granting a role and a file download:

- user gets back before the ipn (this was easy b/c ipn was taking 30-45 seconds to get back to my site today)
- user never comes back to the site (ie: ipn gets back only)
- user gets back after the ipn

This patch does the following for paypal wps:

- Fixes duplicate emails
- fixes autologin

How this "Fixes duplicate emails"

- creates a new the trigger "uc_checkout_complete_paid" - This is run only by the ipn, whether ipn is run first or not.
- the original trigger "uc_checkout_complete" is run only once. It is run by the ipn if the ipn gets to the site first. Or it is run by the user if they hit the checkout complete page first.
- Both of these triggers are only pulled once now... no more double pulling.

How this "fixes autologin"

- if ipn creates the user, the user is logged in at uc_paypal_complete() before hitting the function uc_cart_complete_sale(). Thanks to the patch at #198 for that :).
- if ipn is first... it loads the username from uid and the password as: "***** (check you email)"
- if user is first everything is normal.

In summary

- when ipn hits the site first... it pulls both triggers and the users returning to the site doesn't pull anything.
- when the user gets to the site first, user just pulls uc_checkout_complete sending basic order emails.

Notes:
- Trigger "uc_checkout_complete" - this sets off "E-mail customer checkout notification" and "E-mail admin checkout notification"
- Trigger "uc_checkout_complete_paid" - this sets off "Update order status upon checkout completion with full payment"
- patched against 2.4
- this should patch clean: patch -p1 -b <filenamehere.patch

EDIT:: Don't use this patch, its missing a vital part (sorry, I missed it when creating the patch) grab the patch further down (i'll put it up tomorrow).

@that0n3guy,

I will take a look at this patch when I get a free moment. Sorry if I misinterpreted your original patch. My test cases might have failed for some other reason and I attributed it to the deferred user creation.

Thanks for your hard work!!

..jon

I used the patch in #192 but I still get duplicate e-mails.

Really? Using paypal WPS (not express or wpp)? Are you sure the patch applied?

This patch doesn't apply for wpp?

This patch doesn't apply for wpp?

sorry for the dup post and tag removal, readding

Ok, I see an issue. Other payment types won't work right. I already fixed this.

I also noticed that I forgot the trigger I split up in that last patch... not sure how I missed that. I must have done something funky with git when I was creating the patch.

Anyways, if you were seeing duplicates its because you needed to flush your cache... but using the patch @#192 wont work anyways because I forgot the trigger.

I'll get a fresh patch in here tomorrow.

I'm a pretty big noob with git still, so sorry about that. Here is a good patch (unless I did something stupid again :P)

Besides all the stuff in #192, This patch:
- fixes the issue with other payment types... they now function normally. I did this by... (see next bullet)
- moves the "flag" that says that the user is going first into uc_paypal.pages.inc instead of uc_cart. This is a better spot for it as it is more of a paypal thing than a ubercart thing.
- this actually includes the split up trigger like 192 was supposed to.

Please test this sucker. I will probably have it on a live site soon.

I still don't understand why you need to add a new trigger for this. Isn't the existing "Update order status on full payment" predicate on the uc_payment_entered trigger enough to update the status when the IPN arrives?

@that0n3guy
I cant apply the patch #200.

Error:
patching file uc_cart_module
Hunk #3 FAILED at 1297

I applied the patch #192 before
and I also tried a brand new ubercart download to patch it.
This one wont execpt any of the chances...

Any ideas?

@202, I used patch -p1 -b < 0001-split-trigger-fixed-other-payment-types-issues-mov.patch and it patches just nicely.

@201, Its not the "Update order status on full payment" that bothers me, its the "E-mail customer checkout notification" and "E-mail admin checkout notification". I'm separating those from the other. That way I can send those emails apart from updating the order status.

For paypal, the user returning to the site doesn't have a need to ever "update the order status". That is the ipn's job.

I just did a workflow of this on paper b/c I started to think more about just modifying the CA like #189 suggests.

This works good, until you start mixing other payment types (I don't really know if this is a problem, I'm just guessing) and if the item can be shippable.

Here is why, lets say we set the 2 emails so that they only go if payment status != payment recieved. Here are your situations:

If the Items isn't shippable... this is what it looks like

User is first

user hits complete sale function:
- status is pending
- emails are sent
ipn hits complete sale function:
- status is payment received
- emails are not sent

IPN is first

IPN hits complete sale function:
- status is payment received
- emails are not sent
User hits complete sale function:
- status is completed
- emails are not sent

If the item IS shippable:

User is first

user hits complete sale function:
- status is pending
- emails ARE sent <----email sent
ipn hits complete sale function:
- status is payment received
- emails are not sent

IPN is first

IPN hits complete sale function:
- status is payment received
- emails are not sent
User hits complete sale function:
- status is payment received <--IPN does not set to completed sence item is shippable...
- emails are not sent

You can see that if the item is shippable and the IPN gets there first, the account emails will never send because items aren't supposed to be marked as complete until the item is shipped.

If you set the CA's the other way around where the emails are only sent if payment status is "payment recieved", then you will get duplicate emails if ipn in first and order is shippable.

It's quite possible I've done something wrong, but this new patch in #200 doesn't seem to prevent duplicate emails from being sent if the customer already has a user account and is thus already logged in when they checkout. (I have flushed the cache).

I added a bunch of debugging lines in the files that the patch altered. My debug logs show that when my customer is already logged in and has returned to my site from PayPal, $order->uid is not 0, so neither $order->data['complete_sale_userfirst'] nor $order->data['complete_sale_ipnfirst'] are set and thus, uc_cart_complete_sale() skips straight to:

<?php
//Other payment times usually need to pull both these triggers.
else {
  ca_pull_trigger('uc_checkout_complete', $order, $account);
  ca_pull_trigger('uc_checkout_complete_paid', $order, $account);
}
?>

Then the same thing happens when the IPN arrives.

That means both triggers are pulled twice and duplicate emails are still sent. I don't understand why you're only setting those flags when $order->uid == 0.

The patch in #167 seemed to work fine for a logged in customer.

That makes sense. How I determined what "flag" was pulled first was based on whether uid == 0. This should probably be easy to fix. Basically throw a flag if the other flag doesn't exist (ie throw ipn flag if user flag doesn't exist).

Also, I kind of consider #167 insecure now. New users passwords are "temporaraly" stored in the database in $order->data. If the user ever returns to the site, then their password is never removed form the db and is stored in plain text = bad. I fixed this in #200.

Here is another patch. This fixes a couple other all-ready-logged-in issues.

Test'r out.

I wanted to throw in my two cents, in case it helps anyone. I reviewed this issue and all comments last November (when there were only patches from ezra-g and colin49).

I tried ezra-g's patch first, because it looked like it solved the problem with fewer code modifications, but it did not fix the double e-mail/double decrement issue for me.

I then tried colin49's patch and have only had one duplicate in 250 transactions (the one was most likely an anomaly). So, colin49's patch worked for me.

@that0n3guy: your patches showed up after I already had my fix, so I did not review them. As a module maintainer myself, let me just say that your patch is less likely to be accepted by the maintainers. The reason is because it attempts to address too many issues in one large patch. Smaller, more specific patches are more likely to be accepted and be accepted sooner.

@oadaeh: I think it is possible that your one anomalous order was related to the lack of a locking mechanism when the IPN and the user arrive at the site at the same time, as noted at the end of #189.

As a module maintainer myself, let me just say that your patch is less likely to be accepted by the maintainers.

Thats probably true, but I barely have time to mess with this. My patch is pretty well commented so if anyone wants to split it up, go ahead. Plus it only really fixes 2 issues; duplicate emails, and anon-auto login.

Plus, this thread is becoming very large and its got to get some attention sooner or later.

The #207 patch works well for me. Thanks.

One thing I noticed though, is that depending on the sequence of events, the order in which the triggers are pulled varies, which can lead to conditional actions being carried out in different orders.

For example, my site emails out a receipt when uc_checkout_complete fires, and emails out a status update when uc_checkout_complete_paid fires.

If the IPN is received before the user returns, they get the status update email before the receipt email, because under that condition the code does:

<?php
ca_pull_trigger('uc_checkout_complete_paid', $order, $account);
ca_pull_trigger('uc_checkout_complete', $order, $account);
?>

instead of:

<?php
ca_pull_trigger('uc_checkout_complete', $order, $account);
ca_pull_trigger('uc_checkout_complete_paid', $order, $account);
?>

Would it cause any problems if uc_checkout_complete fired before uc_checkout_complete_paid when the IPN arrives first?

Oh yeah... I switched that around when I was testing something and guess I forgot to switch it back. It shouldn't cause any issues.. but maybe it will... I can't think of anything off the top of my head that would be an issue.

Another patch update... this one doesn't change any functionality, I had some weird extra tabs (white space) and flips #211 back around.

well, finally got a chance to try this patch, so far, all good and no dupe email.

Will this work with existing sites that already have CA rules set up against the uc_checkout_complete trigger, and is it guaranteed to work with all other payment methods? I still have concerns about introducing a brand new trigger that is apparently only needed because PayPal's IPN kinda sucks.

Also it looks like hook_uc_checkout_complete will get called twice if the user returns to the site after PayPal, once at that point and once during the IPN; this hook was meant to accompany the trigger, so it should be altered as well.

Strangely, I still have the issue with conditional actions being executed in the wrong order, even with the latest patch which corrected the trigger pull order. Adding in a sleep(2) between the trigger pulls seems to solve the problem for me so maybe there's some sort of timing issue in the CA module...

I finally made some time to test the most recent patch from @that0n3guy .. #213 to be exact.

It looks like it is working great! I was able to undo the changes to my conditional actions that I discussed in #189.
I suppose there might still be an issue if the IPN request and the user's return request come to the site at the same moment before either completes..?

And FYI, I was able to apply these patches to the most recent dev version: 6.x-2.x-dev version (2011-Jan-29)

Thanks!

..jon

I've been trying doing this by tweaking the conditions and it seems to work all the time for emails and 50/50 for stock. Essentially, my conditions were:
- if it's a credit card, fire the trigger
- if it's a paypal transaction AND the status is "in checkout", fire the trigger (I know others have used "payment received")

Anyway, it doesn't seem like anyone can get this to work using pure conditions, so now I'm off to try the patch in #213

Based on longwave's comment #216, this needs work. Feel free to set it back if I'm mistaken, but I'd love to see this patch get reworked so I can test - PayPal functionality is pretty important on our site (50% of orders are via PayPal). Thanks all!

@longwave & #216... I'll have to look at that, I'm not sure where your talking... are you talking about this line?:
module_invoke_all('uc_checkout_complete', $order, $account);
Whats the harm in doing that twice?

@-morbiD- & #217... Are you talking about:

ca_pull_trigger('uc_checkout_complete', $order, $account);
ca_pull_trigger('uc_checkout_complete_paid', $order, $account);

It shouldn't matter what order they run in I don't think. If it does, it could be a CA weight issue on your system. Did you mess with the weights in CA?

#221: "Whats the harm in doing that twice?" - won't that cause duplicate order notifications, file downloads, role additions, etc.? It seems that we'd want to prevent the complete_checkout hook from firing twice if those are the consequences.

The notifications are pulled via the trigger which is split up. I've not seen duplicates with the patch at #213

The hook is for modules as the trigger is for user-configured actions. If you are splitting the trigger into two, you need to split the hook the same way. A module might want to react on hook_uc_checkout_complete, but should only do so once per order even when IPN is involved, just as your patch does for the trigger.

Having said that, I am still unhappy about introducing a new trigger here only because of PayPal. Are you sure there is no way around this? I need to find time to set up a test store with PayPal before I will commit any fix for this.

patch in #213 fixed problem with order email duplication, but didn't fix trouble with duplication of file downloads orders.

so, in uc_paypal.pages.inc I commented 107th line:

uc_cart_complete_sale($order);

and duplications dissapeared

Subscribe

Subscribe

@#225. Are you talking about duplicated file download emails? Are you using paypal WPS? Also, removing uc_cart_complete_sale from that line is bad idea. You'll probably notice lots of issues. The biggest being that if a user orders and doesn't comes back to your site right after... they will have paid, but your ubercart will never even show the order.

@that0n3guy patch #213 seems to fix the most of my PayPal duplications but not all :(
all PayPal payments come in as paypal_wps.

If you can tell me how I may can figure out that may the difference between the payments without the duplication and the ones with it is, I'm happy to help to fix this problem

The attached patch attempts to fix this and also #479836: Roles and File Downloads Not Being Assigned for Anonymous Checkouts for PayPal WPS, without introducing a new trigger.

It changes the following:

• The uc_checkout_complete trigger is only pulled if the order is still in_checkout. This avoids the double email and stock decrement problems.
• uc_cart_complete_sale() is moved before uc_payment_enter() when the IPN returns. This means the user account is created before roles or file downloads are granted.
• The "update order status to completed for non shippable orders" predicate is moved to the "payment received" trigger. This means that all orders move from in_checkout to payment_received, and then automatically to completed if they are not shippable for better consistency.

Tested with the PayPal sandbox. Reports of real world testing are welcome!

Note that this does not fix #423546: Using PayPal with Anonymous checkout gives merged not new account if the IPN comes in before the user returns to the site, but I would like to just get this double email issue fixed first, and then worry about that one later.

longwave, same applies to wpp?

I only tested with WPS, as I thought that was the main issue here, and don't have time to do more tests on this right now. But I guess if WPP uses the same paths through IPN and the checkout complete page, it should work..

well, will test and report back. I did see the dup email not with wpp before. Oddly, after testing out patch #213 the dup was gone, but I saw it once the other day. I will revert and try yours.

Subscribing.

Subscribing

Scrubscribing

Patch seems to be failing on a straight ubercart 2.4, can you send a clean patch to do proper testing or is it I should start using -dev?

Probably a lot of things on dev for a good test of this patch.

BTW, started to have dupe emails with #213 patch. At least with wpp.

Has anyone had any luck with the patch in #230? I'm unable to apply it successfully to either the current release (2.4) or the current dev.

Patched fine for me with the current -dev.

$ patch -p0 < 644538-paypal-wps-duplicate-emails.patch
patching file payment/uc_payment/uc_payment.ca.inc
patching file payment/uc_paypal/uc_paypal.pages.inc
patching file uc_cart/uc_cart.module
Hunk #1 succeeded at 1279 (offset 9 lines).
Hunk #2 succeeded at 1288 (offset 9 lines).

(The offsets are probably due to some changes I've made to uc_cart.)

EDIT: Make sure you're in the main "ubercart" module directory, and use the -p0 switch when patching.

You're right, torgosPizza, the patch does work with the latest -dev.

I'm confirming that #230 has resolved the duplicate email issue for me. Tested using PayPal WPS sandbox.

I've applied the changes to my files D 6.17/UC 6.24 I now get single emails from PayPal. I've tested this out live using a test product and my CC.

My PayPal setting are

Under
admin/store/settings/payment/edit/methods

I have Pay Pal Express set up and have PayPal Website Payments Standard unchecked (this was giving me blank pages when clicking the link at PayPal back to my site http://drupal.org/node/1067116 I'll do some testing on this later)

I didn't check the stock quantity as I don't use stock tracking.

Just going threw patch at 230.. you reverted this commit:
http://drupalcode.org/project/ubercart.git/commitdiff/885889627aca151a3a...

Why? I'm not even sure what that section does, haven't looked into it, but I'm guessing they added that for a reason, probably having to do w/ a different payment system.

...still reviewing patch at 230
Also, can a UC dev tell us if only pulling the triggers when the cart is in checkout is bad? Does that mess with other payment modules?

Also, does this mess with how other payment modules handle stuff?:

+++ payment/uc_payment/uc_payment.ca.inc	17 Feb 2011 11:39:11 -0000
@@ -67,11 +67,11 @@ function uc_payment_ca_predicate() {
     ),
   );
 
-  // Set the order status to "Completed" when checkout is complete, none
+  // Set the order status to "Completed" when a payment is received, none
   // of the products are shippable, and the balance is less than or equal to 0.
-  $predicates['uc_checkout_complete_paid'] = array(
-    '#title' => t('Update order status upon checkout completion with full payment'),
-    '#trigger' => 'uc_checkout_complete',
+  $predicates['uc_payment_received_non_shippable'] = array(
+    '#title' => t('Update order status on non-shippable orders to completed'),
+    '#trigger' => 'uc_payment_entered',
     '#class' => 'payment',
     '#status' => 1,
     '#weight' => 1,

I am not sure what that "consistency" commit was for in the first place, and I didn't see any difference after reverting it. The only thing it should affect is the order object status during the uc_checkout_complete trigger, which (in my opinion) should only be pulled when the order is "in_checkout" anyway.

Moving the predicate from uc_checkout_complete to uc_payment_entered should not affect other payment modules, as they all should call uc_payment_enter() to log a payment, which then fires that trigger anyway.

If anything, as explained in #230, I think this patch should make things even more consistent than that "consistency" commit, which unfortunately does not have an issue number attached - so the purpose of that change is unknown unless Lyle can remember why he committed it...

Any chance of having the patch made against 2.4? I'd like to use this in a production site and not sure if using -dev is the best thing to do.

The attached patch is against 2.4

#247.. thanks, and what's that? is #230 modified for 2.4 or something else?

#230 works with dev.... #247 works the same but with 2.4.

The difference has to do w/ what I was talking about in 243 and what long was talking about in 245.

so your patch is the same as #230 with that commit you noted not removed, or in the end, you accepted what longwave said on #245 and make the patch identical to #230 but for dev? I guess I can actually review it, so sorry for being lazy, but I will probably see it during the week.

@hanoii, no....
#230 reverts a commit that is NOT in uc2.4. That is why the patch at 230 fails on 2.4. patch at 245 is the same as #230 (unless I missed something, but I don't think so) only it doesn't revert the commit because that commit doesn't exist in 2.4.

Tagging.

I have the duplicate stock depreciation going on, can anyone confirm which patch is best to fix it?
Thanks
Jen

@coppock: Try #247 if you are using 6.x-2.4, or #230 if you are using 6.x-2.x-dev. And please report back in this thread whether it works or not!

Hi, I'm on 6x2.4
Is it worth still trying the patch on #230 and letting you know if it works depsite failing or should I try something else?
cheers
Jen

#230 does not apply to 6.x-2.4 - use #247 (and ignore the "failed testing" messages above).

I patched 2.4 with #247 and still get duplicate confirmations with Paypal Standard.

I guess that it could be that I am using Netbeans to do the patching, as I am working off a shared server account, so could someone please post the patched files?

Also, is there an obvious reason why the core Ubercart team havent addressed this issue in recent updates (2.2 - 2.4) and will we have to go through the same cycle for 6.x 2.5?

Thank you. Steven

Tracking this issue causing major headaches on our website.

what's the status of
- #658470: User is not logged in with "Enable anonymous checkout" + Paypal WPS
- #423546: Using PayPal with Anonymous checkout gives merged not new account

Those were cross-patched with this issue at some point, are the patches on those issues still compatible with the changes on this one. I think, bearing the importance on getting paypal working properly, it might be good to have one issue at least to track the other issues status as well.

Should be those set as release-blockers as well?

I can see there is considerable discussion and effort going on here, but can I ask might these patches work for D7 as well or am I dreaming?

The same duplicate e-mails are happening under D7 and UC 7.x-3.x-dev.

So, a bit on report on #247. Bear in mind I am not exactly using WPS, but rather WPP both as a gateway and also with the paypal express checkout option triggered from the checkout.

My settings are that you could select both credit card or paypal from the checkout. If you select paypal, you are redirected to paypal to pay with it and then back to the site for review and once accepted, you end up with, so far, a successful purchase.

If you choose credit card, you fill in the cc details and should get also a successful purchase. I am also serving file downloads, and also both creating accounts and login the user in after checkout.

I am also using ubercart_marketplace because I am building an aution-like site.

I should receive 5 email notifications (assuming all comes to me): account creating, new order invoice, order admin email, file download available and the seller notification of the item being sold.

I know this might be a bit off a base install but it's my current setup which I am very busy trying and I would thing this should be a good enough report.

So far I did a few tests
1. paypal express: mostly fine, except I didn't receive the actual invoice, I got all other 4, none duplicate.
2. paypal gateway: the order seemed to went through fine, only got the user creation email, none of the others, no file download triggered.
3. paypal gateway: now I received two emails (user creation and seller notification - strange). none of the others, again no file download.
4. test gateway (normal checkout using cc): Same two emails as 3.
5. test gateway w/o the patch: all 5 emails.

So summing up, apparently with this patch, using a cc gateway doesn't trigger either the file download activation and the email. It was strange between 2 and 3 being the same test I actually got the seller notification but nothing else. And I didn't receive the invoice

Apparently this patch breaks a normal credit cart checkout in some way.

It's really amazing this issue remains unfixed for all ubercart 2.x lifetime. I'd think paypal should be very used

Anyway, will try to test a bit more on this and report back, I might even will have to try and fix this, although not sure if start from a patched ubercart or from a bare 2.4. Also, should I be using dev to fix this? Is dev stable enough to be used on a soon to be launched site? I am not very happy on using dev, but I wonder where I can contribute the most.

#247: 0001-fix-dup-emails-nub230-644538.patch queued for re-testing.

@manoloka: The testbot doesn't currently function properly on fixed-point releases like 6.x-2.4, that is why the patch failed. It has nothing to do with the patch. If you're interested, you can manually test the patch against the Ubercart SimpleTests then report back about the result.

@hanoii: You say "It's really amazing this issue remains unfixed for all ubercart 2.x lifetime. I'd think paypal should be very used." However, you've already stated one reason: "Apparently this patch breaks a normal credit cart checkout in some way." There has been no proposed patch that fixes the problem without breaking other things. #230/247 is the closest it has come so far. If you would like to contribute and help find a fix, I strongly suggest working with 6.x-2.x-dev, since there are many other fixes in -dev. The development version is 2.4 with bug fixes, and should work just fine on a live site.

@Tim Tunbridge: The fix will be ported to D7 as soon as there is a fix. It doesn't make sense to work on the same problem in two different places at the same time.

subscribing

Hello.

Just my 2 cents regarding this issue. Just a few things: my workflow is set up so that the IPN will always arrive after the customer. We are only authorizing funds for capture at a later date, and I believe the IPN is only sent when funds are captured (regardless of amount).

I have patched #247 and this does not resolve the issue. I included an [order-status] token in the order email notification, and it seems that when the uc_checkout_complete trigger is fired on both the customer return and IPN, the status of the order is PayPal_Pending. I am not sure if this is because of the way my settings are configured, but I do not remember setting anything with regards to the order workflow. But it means that an if (status = X) will not resolve this issue.

I have also tested the patch in #213, and it works only intermittently. If the IPN is called shortly after (within a few minutes) the customer, then it works fine. But if the IPN comes some time later (averaging at 5mins+), then uc_checkout_trigger is fired again, leading to dupe emails.

My best bet so far was as in #225, to comment the line below in paypal.pages.inc.
uc_cart_complete_sale($order);

As of now the only issue I can see from this would be in #228, where the customer doesn't return... But it doesn't really apply to my situation as even if the customer doesn't return, he would not get the email until I capture the funds from PayPal.

I hope this much can help.

It's been quicker for me to port my whole store to drupal commerce and write a module.
thanks ubercart so long.

Since some of us can't yet port to D7, that's not an option for everyone. Good to know it's easier in DC, though.

Does the provided patch in http://drupal.org/node/644538#comment-4181162 allow the role assignment predicate to work when set to 'payment received' instead if 'completed' as referred to here : http://drupal.org/node/479836#comment-4103954

Both these threads are super long and old, it's seems the role assignment/anonymous user issue is touched on in this thread.
Can anyone confirm?

Same Issue

Seems to me that since any checkout process would want to trigger a similar series of emails, an order should simply contain flags for messages sent. That way conditional actions could check agains the flags and admins could choose to use them or ignore them. This approach also solves the problems of accommodating different checkout methods. Simple and global.

This is a difficult issue to follow, kind of lost on what the exact issue is or where. Anyway, I re-tried the attached patch and set my goal to see why not the usual stuff were occurring even with the normal test gateway, this is what I found about the patch in #230 or #247. I will review #247 as it's the one I actually tried, but I did verify they are the same, one for dev and the other for 2.4.

So, focusing on this patch, with credit card using the testway, with product download features.

We would expect invoice notification, and file granting. This is working w/o the patch, so why it stopped working:

+++ b/payment/uc_payment/uc_payment.ca.incundefined
@@ -67,11 +67,11 @@ function uc_payment_ca_predicate() {
-  $predicates['uc_checkout_complete_paid'] = array(
-    '#title' => t('Update order status upon checkout completion with full payment'),
-    '#trigger' => 'uc_checkout_complete',
+  $predicates['uc_payment_received_non_shippable'] = array(
+    '#title' => t('Update order status on non-shippable orders to completed'),
+    '#trigger' => 'uc_payment_entered',

Changing the trigger of this default action affects other workflows. Now is when a payment is entered that the status is updated to complete, which in the end, triggers the order_status_update trigger that is the one configured to renew files in uc_file, but the user has not been created here yet, only in uc_cart_complete_sale(), thus the action fails.

+++ b/uc_cart/uc_cart.moduleundefined
@@ -1291,8 +1291,12 @@ function uc_cart_complete_sale($order, $login = FALSE) {
-  module_invoke_all('uc_checkout_complete', $order, $account);
-  ca_pull_trigger('uc_checkout_complete', $order, $account);
+  // Only pull the triggers if the order was in checkout when first loaded.
+  if (uc_order_status_data($order->order_status, 'state') == 'in_checkout') {
+    module_invoke_all('uc_checkout_complete', $order, $account);
+    ca_pull_trigger('uc_checkout_complete', $order, $account);
+  }

By doing this, and when a payment is entered and the status changed to complete because of the previous change, we never trigger checkout complete because the order is not in_checkout anymoure, so when should we send the invoice, again on payment entered?

I guess it's fine that the trigger is executed when that function is called regardless of the status of the order, this might render the main purpose of the patch unusable, so I wonder...

What is the main reason for the duplicate issue, uc_cart_complete_sale() is called twice? I will now try to follow this one but any sum up or pointers will help.

Powered by Dreditor.

(subscribing)

So after looking more into this:

The main problem seems to be that:
- uc_cart_checkout_complete() is executed twice, when the checkout is complete in the cart module and on the paypal module (also 2checkout and google checkout)
- We have to cope with the possibility of the IPN arriving before or after reaching the actual checkout completion on the cart module.

With this in mind and if I am not missing anything else, it's clear that uc_cart_complete_sale() should be executed only once regardless of the payment method.

So I think that uc_cart_complete_sale() is not really needed on the paypal module, so my suggestion is to remove that altogether, and tweak/add a little bit the CAs, this can be done manually but eventually it might be good to have them as default CAs within ubercart.

So right now (w/o any patch), the related CAs are as follow:

- [payment_enter]: If balance = 0 and status is not = 'payment received', then set it to 'payment received'
- [checkout_complete]: if balance = 0 and order is not shippable, then set it to 'completed'.

As it is, it should work just fine (w/o the call to uc_cart_complete_sale() in the paypal module), when the IPN arrives before the checkout is complete.

And when the IPN arrives after, we can change/add a bit to the CAs to cope with that case (bold are additions):

- [payment_enter]: If balance = 0 and status is not = 'payment received' and is not = 'pending' and is not = 'processing', then set it to 'payment received'
- [payment_enter]: If balance = 0 and status is = 'pending' or is = 'processing', then set it to 'completed'

To add a little bit to this, the uc_cart_complete_sale() sets the order to the default status of the state 'post_checkout'. The status defined within ubercart for that state are either 'pending' or = 'processed

Leave the checkout_complete as it is.

Thoughts? I have just tested this w/WPP (that uses the same code as WPs, so I guess it's the same. Also tried it with paypal express checkout and I based on my thinking this shouldn't affect the previous functionality of any other workflow as it did the patch on #230. Please, all the ones who submitted patches and/or proposed suggestions, can you please check this one?

Eventually, if this gets accepted, some support issues will have to be opened for 2checkout and google checkout to do something similar and make sure uc_cart_checkout_complete() is only executed once.

NOTE: use patch -p0

uc_cart_complete_sale() is responsible for the following:

• Creating a new user (if needed)
• Updating the order status
• Emptying the cart
• Pulling the uc_checkout_complete trigger

It's the last of these which causes the duplicate emails and stock decrement. However, there are two very different cases which can occur with PayPal WPS, both of which cause complications:

- the user returns immediately to the site, but the IPN is delayed for minutes or hours
- the user does not return to the site after paying at PayPal

If we remove uc_cart_complete_sale() from the "order complete" page, the user will not have their cart emptied or new account created until the IPN arrives - this will be confusing if they click back from PayPal to see that they still have a full shopping cart and the IPN is delayed for whatever reason.

If we remove uc_cart_complete_sale() from the IPN code, it will never be called if the user does not click through from PayPal - earlier in this issue it was suggested that some 50% of users close their browser at this point.

I also do not think we should break compatibility with existing payment modules in the middle of a release cycle, as this issue only affects PayPal - there should be no need to alter 2checkout, Google Checkout, or any other payment method (unless they are also broken in the same way!)

So, I am not sure the approach suggested above will work in all cases. We need some way of ensuring that the trigger is pulled once and only once - this is why I tried to fix it with order statuses in my previous patch, but that doesn't seem to work in all cases either. Perhaps some other guard mechanism around the trigger call in uc_cart_complete_sale() will work, though.

I see. I don't have WPS so in my I can't seem not to come back to the site.

I now understand why that function is called in the paypal module, and it's good to note that uc_cart_complete_sale() does one more thing, which is kind of important, that is to show the proper message to the user.

So whatever guard we put in place for this double execution possibilty, it will still has to show the proper data, so if the account is created when the IPN arrives, and it arrives before actually completing the order, then it should also detect that and show the proper checkout messages there.

I think using statuses as the guard mechanism is scratching your left ear with your right hand (I always liked this phrase from a teacher of mine) because those could change in a lot of places at a lot of different moments. I think we should rely in something more straightforward and actually currently being used, which is on the SESSION, will work on patch for that now. Will re-think the CAs changes to see if they are still necessary.

hmm, session won't work as IPN will see a different session, what about a variable_set() semaphore then?

Just a warning that $_SESSION is not necessarily usable either, because the IPN comes direct from PayPal's servers rather than the user's browser, so there is no session data available at that point. I think the issues you linked in #260 are related to this and the inherent race condition of whether the user or the IPN gets to uc_cart_complete_sale() first.

variable_set() will likely have locking issues and race conditions as well, if two users are completing checkout at the same time, or two IPNs are received close to one another (this can happen especially when they are delayed for some unknown reason at PayPal, if there are several delayed IPNs they sometimes arrive in close batches)

What about the idea in #271? A boolean flag in the order would avoid the race conditions and locking issues.

The idea of using some guard is what we are going for at the moment, but adding something like this exposed to CA, is making something already a bit complicated even more cryptic. Besides, duplicate email is not the only user, user creation, logging in and proper checkout complete screens on different use cases are also issues here, and the guard should rather be implemented on code. We've discussed this a little bit with @longwave yesterday, I will try to submit a patch close to what was discussed and see if we can start from there.

Glad to see some more interest in this problem as it is definitely not a trivial fix.

When trying to tackle this bug myself I wrote an overview of the problem (644538_OVERVIEW.TXT ) back in post #91. I suspect the document needs some updating but I did describe the problem / existing workflow / and some possible solutions in detail and it might be a useful document for those currently tackling the problem to build upon. With this kinda bug there are lots of solutions and side effects so collecting all that information in one document should help.

Thanks to all those working on a patch!

http://drupal.org/files/issues/644538_OVERVIEW.TXT

Cheers,
Colin

Just letting you all know that I have a patch almost out of the oven, so stay tuned. I might have pushed it a little further with the code changes, but all for the good I hope.

Following. I have a similar issue that might be related, where duplicate "order comments" are being generated when changing the order status and adding an order comment. Presumably 2 email notices are being sent out as well.

Colin, that's an awesome little write up. Thanks for posting.

Hanoii, looking forward to your patch!