-
Notifications
You must be signed in to change notification settings - Fork 210
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
[Deferred intent] Allow saving cards and using saved ones #2816
Conversation
The values for this parameter come from WC, we're not setting it so there's no point in trying to unify this. We could also just check whether it's empty, but a stricter check may be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look pretty good @a-danae!
I went through the following testing instructions and ran into some issues using a saved payment method on the classic checkout. Details below.
- Disabled the save payment method setting (screenshot).
- On checkout. No checkbox to save. 👍
- A purchase using a card didn't save the method in Stripe or WC. 👍
- No setup intent. 👍
- Saving payment methods enabled (screenshot).
- 🔲 Block Checkout 🔲
- Choosing not to save. ie leave the checkbox unchecked on checkout.
- A purchase using a card didn't save the method in Stripe or WC. 👍
- No setup intent. 👍
- Saving a standard card (Mastercard).
- Saved in Stripe. 👍
- Saved in WC. 👍
- Set up intent. 👍 (screenshot)
- Saving a 3DS card.
- Saved in Stripe. 👍
- Saved in WC. 👍
- Set up intent. 👍
- Using a saved standard card (Mastercard).
- PI saved in order meta. 👍
- Saved PM is used to process the payment. 👍
- Saved PM saved in order meta. 👍
- Using a saved 3DS card.
- Re-authed 👍
- PI saved in order meta. 👍
- Saved PM is used to process the payment. 👍
- Saved PM saved in order meta. 👍
- Choosing not to save. ie leave the checkbox unchecked on checkout.
- 🏛️ Shortcode/Classic Checkout 🏛️
- Choosing not to save. ie leave the checkbox unchecked on checkout.
- A purchase using a card didn't save the method in Stripe or WC.
- No setup intent.
- Saving a standard card (AMEX).
- Saved in Stripe. 👍
- Saved in WC. 👍
- Set up intent. 👍
- Saving a 3DS card.
- Saved in Stripe. 👍
- Saved in WC. 👍
- Set up intent. 👍
- Using a saved standard card (AMEX). 👍
- Using a saved 3DS card. 👍
- Choosing not to save. ie leave the checkbox unchecked on checkout.
@a-danae when I try to use a saved payment method on the classic/shortcode checkout, I get the following error:

Do you get the same error message?
Update: After using the latest version of this branch, this error is now fixed.
includes/payment-methods/class-wc-stripe-upe-payment-gateway.php
Outdated
Show resolved
Hide resolved
Thanks for the thorough testing, @james-allan !
Interesting... This validation shouldn't be triggered when using a saved payment method. Mind sharing the ID and name of this input on your end? My thoughts:
|
Co-authored-by: James Allan <james.allan@automattic.com>
So it looks like the flow that leads to this error is starting in the ie it looks for |
🤦♂️🤦♂️🤦♂️ so sorry @a-danae. I somehow had an old version of the Stipe Utils JS file and just noticed you have updated that function in this PR. After force updating my local branch with the latest version of this branch, it works now. 😅 |
Hey @a-danae 👋 I just merged my Split-PE PR #2827 and resolved the conflicts that were in this PR, but when I tried to test saving/using saved cards in a split PE environment, I got the following results:
Here's the error I'm getting: After doing some digging, the issue is caused by this line in the
Previously with the old Stripe UPE we were hooking onto the StripeElements.change event and setting the selected payment type, but now with a Split PE we're never setting the selected UPE payment type. To fix this, now that each payment method is separate, I'm wondering if we can get rid of the old I've been testing with this helper function below and it appears to be working but curious to get your thoughts:
With the above function, you then need to update the first line in
Since this PR has already been approved I'm happy to move this into a separate PR! Up to you :) |
We were getting it from wc_stripe_selected_upe_payment_type before. After implementing Split PE, there's no longer need to use this customized hidden field.
Big thanks, @james-allan for checking this out again! And big thanks @mattallan for updating this branch, checking it with the Split PE changes, and proposing a fix! 🙌 I've included your fix and it's testing well 🙂 There are a few tests that need adjustments, but other than that, this should be ready to review. I'll check these tomorrow. |
No worries @a-danae! I have reviewed your latest changes and they look really good! I also ran through the following checks: Checkout blocks
Classic checkout
I'm happy to approve this PR. Amazing work on this one!! |
Fixes #2760
Fixes (partially) #2763
Changes proposed in this Pull Request:
Using deferred intent:
Testing instructions
Saving a new card
siteurl/wp-admin/admin.php?page=wc-settings&tab=checkout§ion=stripe&panel=settings
) > Payments & transactions4242424242424242
, and place the orderConfirm that:
siteurl/wp-admin/admin.php?page=wc-orders
):https://dashboard.stripe.com/test/payments
):https://dashboard.stripe.com/test/customers/cus_xyz
)siteurl/my-account/payment-methods/
).https://dashboard.stripe.com/test/logs
Using a saved card
Saving a 3DS card
Test cards
4000002760003184
.Using a saved 3DS card
Disabling saving cards
siteurl/wp-admin/admin.php?page=wc-settings&tab=checkout§ion=stripe&panel=settings
) > Payments & transactionschangelog.txt
andreadme.txt
(or does not apply)Post merge