Cache UPE intents for reuse (WC Session edition) #3782

anu-rock · 2022-02-10T12:21:03Z

Changes proposed in this Pull Request

Mounting the UPE element requires us to create a payment or setup intent in advance in the front-end code. This happens on the Checkout page. When a non-UPE payment method is used or the page is reloaded, the previously created intent is wasted and a new one is created. This has the potential of resulting in a large number of abandoned intents even for just one merchant.

This PR fixes this behavior by caching the newly created intent in WooCommerce session. Once cached, it is then reused in the next mounting of UPE. The flow looks like this (driven by JS code):

Furthermore:

Since a payment intent is dependent on the cart's total amount, the logic ties an intent with cart hash.

Testing instructions

Testing will require you to enable UPE first. Do this by using the "Enable the new Stripe checkout experience" option on the Settings page. Update the store currency as per the payment methods you want to test. Eg. Giropay, iDEAL, etc. will require Euro as the store currency.

Payment Intent

Create a new order in the front-end.
Open Network tab in browser's dev tools, and visit the Checkout page.
On this page, notice an XHR call to create_payment_intent.
Reload the page. No fresh XHR call to create_payment_intent should be observed.
Complete the payment. The cached intent should be removed. Verify this by visiting the Checkout page again and observing an XHR call to create_payment_intent.
Also check if the cached intent is removed when a payment fails. When any of the following is used, reloading the Checkout page should generate a fresh XHR call to create_payment_intent:
- a credit card that is immediately declined
- a card that requires further authentication and eventually the transaction fails
- a redirection payment method (e.g. Giropay, SEPA, iDEAL, Sofort, etc.) and eventually the transaction fails
- a saved payment method that fails

FYI: The intent id retrieved from cache or received as a response from create_payment_intent is used only when a new payment method is used. Using saved cards (successfully) will create a new intent on the server, resulting in the UPE intent being abandoned.

Setup Intent

Open Network tab in browser's dev tools, and visit My account → Payment methods → Add payment method.
On this page, notice an XHR call to init_setup_intent.
Reload the page. No fresh XHR call to init_setup_intent should be observed.
Add a new payment method.
The cached intent should be removed after successfully adding a new PM.
Go back to the "Add payment method" page. A fresh XHR call to init_setup_intent will confirm that the previously cached intent was removed.

Scenarios I have tested

New card - successful payment
New card - declined
New card - authentication failed
Saved card - successful payment
Saved card - authentication failed
New SEPA - successful payment
Saved SEPA - successful payment
Giropay - successful payment
Giropay - failed payment

Added changelog entries to both readme.txt and changelog.txt
Covered with tests
Tested on mobile (does not apply)

Post merge

Link to testing instructions from release testing doc : https://github.com/Automattic/woocommerce-payments/wiki/Release-testing-instructions-for-WC-Payments-3.8.0#cache-upe-intents-for-reuse

The hook woocommerce_after_account_payment_methods is only used as an approximation. It is triggered on load of the Payment Methods page in My Account. This is the page where user is redirected after a new payment method is successfully added.

Giropay, SEPA, Sofort, iDEAL, etc.

Client-side = JS Examples of such errors include: * a new card that is declined * a new card that requires authentication and eventually fails it Such failures are not captured on server-side (PHP code) because these scenarios are directly handled by Stripe's JS library.

anu-rock · 2022-02-10T12:24:38Z

includes/payment-methods/class-upe-payment-gateway.php

@@ -1071,8 +1094,12 @@ public function log_payment_error_ajax() {
 			}
 			$order->add_order_note( $order_note );

+			self::remove_upe_payment_intent_from_session();


It would be cleaner if instead this method was called in a hook rather than directly in the "log" method.

I tried this:

add_action( 'wc_ajax_wcpay_log_payment_error', [ 'WCPay\Payment_Methods\UPE_Payment_Gateway', 'remove_upe_payment_intent_from_session' ] );

But that apparently didn't work as remove_upe_payment_intent_from_session was not called on client-side errors.

anu-rock · 2022-02-10T12:27:42Z

I'll also add that unlike in the cookie approach (#3684), the cache TTL is dependent on WC session. I'm not sure how to set a lower TTL (eg. 10 mins) like in the cookie approach. A lower TTL potentially minimizes chances of a stale cache—that could not be successfully removed—from causing nasty errors.

FangedParakeet

Cool, the approach looks neat and tidy and is working pretty well for me!

I found one issue, which I'm pretty sure is not related to the changes in this PR, but I was wondering if you could just double-check to confirm. If you check out with a 3DS card that requires authentication, then fail the authentication once the modal comes up, once you are returned to the checkout, if you retry with a valid payment method, it will always fail...but if you now retry again it will succeed this time (as expected).

Here are the steps, in case that was a little confusing.

Enter checkout.
Enter 3DS card details. Place order.
Modal opens, manually fail authentication.
Enter valid card details. Place order.
Transaction fails.
Place order again.
Transaction succeeds.

This works with other payment methods too. For example...

Enter checkout.
Enter 3DS card details. Place order.
Modal opens, manually fail authentication.
Select new payment method (e.g. Giropay). Place order.
Transaction fails.
Place order again.
Transaction succeeds, redirects to Giropay authorisation page.

I don't think this is caused by some cookie confusion (or at least I can't find an obvious place where that might happen), but if you wouldn't mind double-checking just to be sure, that'd be grand!

Anyways, I've made one small code suggestion, but everything here looks happy enough to me, so I'm gonna approve, so that I don't slam the brakes on this train. I'll leave it up to the other reviewers to give the final blessing, but LGTM. Nice one!

includes/payment-methods/class-upe-payment-gateway.php

`npm run changelog` now auto-generates changelog more efficiently.

anu-rock · 2022-02-15T13:08:42Z

@FangedParakeet Many thanks for trying it out once again :)

I found one issue, which I'm pretty sure is not related to the changes in this PR, but I was wondering if you could just double-check to confirm. If you check out with a 3DS card that requires authentication, then fail the authentication once the modal comes up, once you are returned to the checkout, if you retry with a valid payment method, it will always fail...but if you now retry again it will succeed this time (as expected).

Strangely, I wasn't able to reproduce it. I tried two cards 4000002500003155 and 4000002760003184 from here. To be sure I understood correctly, here's a quick video recording:

3ds-auth-test.mp4

kalessil

LGTM (did review only, will do testing tomorrow with more capacity on my end)!

client/checkout/classic/upe.js

FangedParakeet · 2022-02-15T21:08:05Z

@anu-rock, I'll retest my calamaties against develop and create a separate issue if it's aught other than my own fault. I've no other critical complaints with anything within these changes then!

anu-rock added 6 commits February 9, 2022 19:07

Cache UPE intents in session

4990a38

Remove intent from session for redirection payments

4d4a6b4

Giropay, SEPA, Sofort, iDEAL, etc.

Remove intent from session on failure of a saved PM

336af0d

Update changelog

289f9db

anu-rock self-assigned this Feb 10, 2022

anu-rock added component: upe pr: needs review labels Feb 10, 2022

anu-rock requested review from FangedParakeet, kalessil, vbelolapotkov and RadoslavGeorgiev February 10, 2022 12:21

anu-rock commented Feb 10, 2022

View reviewed changes

Merge branch 'develop' into fix/3602-redundant-upe-intents

8c5767e

FangedParakeet approved these changes Feb 11, 2022

View reviewed changes

includes/payment-methods/class-upe-payment-gateway.php Outdated Show resolved Hide resolved

mordeth added the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Feb 15, 2022

jessy-p added the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Feb 15, 2022

anu-rock added 4 commits February 15, 2022 18:08

Replace namespaced class name with __CLASS__

c6ff6fa

Merge branch 'develop' into fix/3602-redundant-upe-intents

ccdd90f

Remove changelog entries

85b3976

`npm run changelog` now auto-generates changelog more efficiently.

Add change file

5ec8078

kalessil approved these changes Feb 15, 2022

View reviewed changes

client/checkout/classic/upe.js Show resolved Hide resolved

jessy-p added category: core WC Payments core related issues, where it’s obvious. and removed category: projects For any issues which are part of any project, including bugs, enhancements, etc. labels Feb 16, 2022

Merge branch 'develop' into fix/3602-redundant-upe-intents

d873f67

anu-rock removed the pr: needs review label Feb 17, 2022

anu-rock added pr: ready to merge and removed status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. labels Feb 17, 2022

anu-rock merged commit 21fd406 into develop Feb 17, 2022

anu-rock deleted the fix/3602-redundant-upe-intents branch February 17, 2022 13:18

FangedParakeet mentioned this pull request Mar 15, 2022

POC: Multiple instances of single-method UPE at checkout #3968

Merged

4 tasks

tommyshellberg mentioned this pull request Sep 2, 2022

Split UPE: Correct token behavior #4670

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cache UPE intents for reuse (WC Session edition) #3782

Cache UPE intents for reuse (WC Session edition) #3782

anu-rock commented Feb 10, 2022 •

edited

Loading

anu-rock Feb 10, 2022 •

edited

Loading

anu-rock commented Feb 10, 2022 •

edited

Loading

FangedParakeet left a comment

anu-rock commented Feb 15, 2022

kalessil left a comment

FangedParakeet commented Feb 15, 2022

Cache UPE intents for reuse (WC Session edition) #3782

Cache UPE intents for reuse (WC Session edition) #3782

Conversation

anu-rock commented Feb 10, 2022 • edited Loading

Changes proposed in this Pull Request

Testing instructions

Payment Intent

Setup Intent

Scenarios I have tested

anu-rock Feb 10, 2022 • edited Loading

Choose a reason for hiding this comment

anu-rock commented Feb 10, 2022 • edited Loading

FangedParakeet left a comment

Choose a reason for hiding this comment

anu-rock commented Feb 15, 2022

kalessil left a comment

Choose a reason for hiding this comment

FangedParakeet commented Feb 15, 2022

anu-rock commented Feb 10, 2022 •

edited

Loading

anu-rock Feb 10, 2022 •

edited

Loading

anu-rock commented Feb 10, 2022 •

edited

Loading