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

[Split PE] Fix errors on checkout caused by attempting to reuse intents that require action or require manual confirmation #2952

Merged

Conversation

mattallan
Copy link
Contributor

@mattallan mattallan commented Feb 27, 2024

Fixes #2950

Changes proposed in this Pull Request:

In #2845 we implemented reusing payment intents that have been stored on the order, however, this throws the following error for payment intents with requires_action and requires_confirmation statuses:

image

  1. Payment intents with requires_action status are used for 3DS cards and UPE payment methods that redirect the customer off-site have this status
  2. Intents with requires_confirmation status are from purchases with Boleto and OXXO vouchers as we manually confirm the intent on checkout to show the voucher to the customer

This PR fixes this issue by making sure we create a new intent when the one stored on the order requires action, instead of trying to update and confirm the intent.

Ideally, we could still reuse these payment intents that require action/confirmation and just don't attempt to confirm them via the Stripe API (only update the intent data), but when I tried to implement this I ran into further errors from Stripe. I think one of them had something to do with not being able to send a return_url to the /payments_intents/{id} endpoint. For the sake of fixing this issue I opted for the more simple approach at this time.

Testing instructions

With a 3DS card

  1. Add a product to your cart and navigate to the checkout page
  2. Purchase using a 3DS card: 4000000000003220
  3. When the 3ds pop-up appears, refresh your checkout page
  4. Attempt to checkout again using the same card 4000000000003220
  5. There should not be any errors and checkout should process successfully

With a UPE redirecting payment method

  1. Set your currency to EUR and enable the EPS payment method
  2. Add a product to your cart and navigate to the checkout page
  3. Select EPS on the checkout and click "Place order"
  4. Once you've been redirected off-site to the EPS payment page, don't click on any buttons and manually navigate back to your dev store's checkout page (i.e. https://woo.local/checkout)
  5. Attempt to checkout again using EPS
  6. There should not be any errors. Completing the payment off-site should redirect to the order received page.

  • 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

@mattallan mattallan requested review from a team and a-danae and removed request for a team February 27, 2024 06:16
Copy link
Contributor

@a-danae a-danae left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for addressing this, @mattallan!

✅ I was able to replicate the error for both 3DS and APMs that require a redirect.
✅ The order is successfully placed in both scenarios in this branch.
✅ The code looks good to me.

Not relevant for now but to evaluate for later... Maybe it'd be simpler not to reuse payment intents at all?

I was checking out how WooPayments reused payment intents, and their approach seems to be simple - They don't.

I'd be curious to learn about the discussions behind this decision, but Payment Intents don't seem to be "expensive". After all, the approach before Deferred Intent existed was to create a new one each time a shopper visited the checkout page. So, if there's no benefit in reusing them, we could simply create a new one for every payment attempt.

Just throwing some thoughts to maybe discuss later 🙂

All good here and glad to see this merged 🚀 thanks again!

@a-danae a-danae merged commit dc4241f into add/deferred-intent Feb 28, 2024
32 checks passed
@a-danae a-danae deleted the fix/2950-dont-reuse-incompatible-payment-intents branch February 28, 2024 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants