Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicate emails and order notes when processing setup intent requests #3421

Merged
merged 4 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* Fix - Ensure payment tokens are detached from Stripe when a user is deleted, regardless of if the admin user has a Stripe account.
* Fix - Address Klarna availability based on correct presentment currency rules.
* Fix - Use correct ISO country code of United Kingdom in supported country and currency list of AliPay and WeChat.
* Fix - Prevent duplicate order notes and emails being sent when purchasing subscription products with no initial payment.

= 8.6.1 - 2024-08-09 =
* Tweak - Improves the wording of the invalid Stripe keys errors, instructing merchants to click the "Configure connection" button instead of manually setting the keys.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,11 +795,13 @@ private function process_payment_with_deferred_intent( int $order_id ) {
$this->update_saved_payment_method( $payment_method_id, $order );
}

// Lock the order before we create and confirm the payment/setup intents to prevent Stripe sending the success webhook before this request is completed.
$this->lock_order_payment( $order );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Screenshot 2024-09-06 at 12 20 37 AM

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

Screenshot 2024-09-06 at 1 23 29 AM

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


if ( $payment_needed ) {
// Throw an exception if the minimum order amount isn't met.
$this->validate_minimum_order_amount( $order );

$this->lock_order_payment( $order );
// Create a payment intent, or update an existing one associated with the order.
$payment_intent = $this->process_payment_intent_for_order( $order, $payment_information );
} else {
Expand Down
1 change: 1 addition & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,6 @@ If you get stuck, you can ask for help in the Plugin Forum.
* Fix - Ensure payment tokens are detached from Stripe when a user is deleted, regardless of if the admin user has a Stripe account.
* Fix - Address Klarna availability based on correct presentment currency rules.
* Fix - Use correct ISO country code of United Kingdom in supported country and currency list of AliPay and WeChat.
* Fix - Prevent duplicate order notes and emails being sent when purchasing subscription products with no initial payment.

[See changelog for all versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).
Loading