Fix duplicate emails and order notes when processing setup intent requests

[mattallan]

Fixes #3414

Changes proposed in this Pull Request:

In #3331 (shipped in 6.6.1) we added lock_order_payment() and unlock_order_payment() to the start and end of our process_payment_with_deferred_intent() function to fix an issue with duplicate order notes and order emails being sent.

Because the lock_order_payment() was only added inside the $payment_needed condition, the bug was unfortunately still present for our setup intent requests (i.e. Subscriptions with no initial payment).

To fix this, I've moved the lock to be outside of the $payment_needed condition but still before the payment and setup intents are created/confirmed. This will cause our webhook handler to exit early (code ref) while the order is locked and let process_payment_with_deferred_intent() finish processing the payment and marking the order as processing/completed.

To help visualise the issue, here's a very rough timeline of how the duplicate customer emails & order notes issue occurs:

Timeline A: Checkout Process                             Timeline B: Webhook handling
------------------------------------------------         ------------------------------------------------
1. Customer purchases trial subscription product         1. -
------------------------------------------------         ------------------------------------------------
2. process_payment_with_deferred_intent() is called      2. - 
------------------------------------------------         ------------------------------------------------
3. We fetch an instance of $order which is "pending"     3. - 
------------------------------------------------         ------------------------------------------------
4. Send request to create and confirm setup intent       4. Stripe sends the `setup_intent.succeeded` webhook
------------------------------------------------         ------------------------------------------------
5. Save the new intent to the order                      5. Order is fetched by intent ID and still has pending status.
------------------------------------------------         ------------------------------------------------
6. Save the payment method                               6. Check if order payment is locked
------------------------------------------------         ------------------------------------------------
7. Calls $order->payment_complete()                      7. Calls $order->payment_complete()
------------------------------------------------         ------------------------------------------------
8. Order status transitions from pending to processing   8. Order status transitions from pending to processing
------------------------------------------------         ------------------------------------------------

Testing instructions

Important

To replicate this issue you need to have the setup_intent.succeeded webhook processed at the same time that process_payment_with_deferred_intent() is still processing the payment for the current order.

Specifically, we want the order to still have a "pending" status when an instance is fetched in our webhook handling code.

Since we cannot easily control when Stripe sends the webhook, I was able to consistently replicate this by adding a sleep(2); just before $order->payment_complete(); in our process_payment_with_deferred_intent() (here in the code)

How does this happen?
It's hard to pin-point exactly how this would happen however, Stripe controls when the webhooks are sent and there's a lot of processing between sending our intent requests -> receiving the response -> completing the payment/checkout process.

This means if there's any unexpected lag or delays (i.e. slow DB reads, slow API request, slow third party code), it could cause both the webhook handling and process payment functions to have a pending order and try to mark it as completed at the same time, resulting in duplicate emails and order notes.

1. Activate the latest Woo Subscriptions and create a subscription product with free trial
2. Enable the cards payment method and new checkout experience.
3. Make sure webhooks are setup correctly so that your test site is receiving them (I use ngrok)
4. Enable customer emails and if using a local site, activate something like WP Mail Logging plugin to easily test emails.
5. Add a free trial subscription to your cart
6. While on develop complete the checkout using the standard 4242424242424242 card
7. On completion, view the Edit Order admin page and confirm there's duplicate order notes
8. Open WP Mail Logging and confirm that two emails for the same order ID were sent a few seconds apart:

---

• Covered with tests (or have a good reason not to test in description ☝️)
• Added changelog entry in both changelog.txt and readme.txt (or does not apply)
• Tested on mobile (or does not apply)

Post merge

• Added testing instructions to the Release Testing Instructions wiki page (or does not apply)
• Added the needs docs label (or does not apply)
• Included this PR in the Release Thread scope (or does not apply)

[mattallan]

Hey @Mayisha, in the changes you made in #3331, the lock was added after the validate_minimum_order_amount() call and so I've been trying to test and break this PR to see if there's any issues with it being before validate_minimum_order_amount().

When I tested with a $0.40 product, the lock gets added even though there's a checkout error, but I haven't been able to break it because when I add 2 of these to meet the minimum amount, the checkout is then successful.

I wanted to ping you on this issue to ask if you discovered any issues with this?

The alternative was to either add two calls to lock_order_payment() for both payment/setup intent calls or move the validate_minimum_order_amount() higher up in the process_payment_with_deferred_intent() function 🤔
Let me know if you had any thoughts :)

[Mayisha]

You are right Matt. I intended to add the lock right before processing the intent if it passes validate_minimum_order_amount.
However, I also could not break it and the checkout was successful on second attempt (when I increased the number of items or add a shipping charge).

What happens here is, on the first attempt an order is created and the order status is failed. On the second attempt a new order gets created then successfully gets processed and so the lock on the first order does not hamper the checkout.

In the database, the transient of the first failed orders remain.

It is alright in my opinion as the transient will be cleared once it reaches the expiration time.

[Mayisha]

Thanks @mattallan for working on this. Reproducing this issue was difficult and you did a great job there.

The changes look good to me 🎉

• In develop branch I can reproduce the double note/email issue for a subscription product having free trial.
• In this branch, I do not get any duplicate notes or emails neither for a regular product nor for a subscription product having free trial.

Note: Remember to add the changelog entry also in readme.txt 👀

[mattallan]

Thanks @Mayisha for the review!

Note: Remember to add the changelog entry also in readme.txt 👀

Opps, thanks for the reminder! Fixed in 689a4de. I'll merge this once the checks pass :)

[dwh10045]

Hello, I am still having issues with this. I have the most recent version. Is there anything I need to update?

[pawelkmpt]

@dwh10045 we had the same issue and decided to use 8.6.1 as 7.x doesn't have this commit. So far just one order came through without duplicates.

[Mayisha]

Hi @dwh10045

Please share which version of the Stripe plugin you are using and the payment methods used in the affected orders.