-
Notifications
You must be signed in to change notification settings - Fork 69
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
Fixes Pay for Order page functionality in split-UPE #6177
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: -4 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
…ic/woocommerce-payments into fix/5808-upe-pay-for-order-page
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.
I'm able to successfully process orders with all LPMs and CC, so that's all good. When testing the currency switch, the following error is showing up:
We're not able to process this payment. Please refresh the page and try again.
In the console:
Uncaught (in promise) IntegrationError: In order to create a payment element, you must pass a clientSecret or mode when creating the Elements group.
e.g. stripe.elements({clientSecret: "{{CLIENT_SECRET}}"})
n (index):1
r (index):1
t (index):1
ro (index):1
pi (index):1
t (index):1
create (index):1
B (index):1
mountUPEElement upe-split.js:284
async* upe-split.js:361
jQuery 13
v3:1:478949
This is occurring when the store/order currency is euros and changed to usd on the Pay for Order page. I haven't been able to pinpoint the cause unfortunately. I'm testing with the Split UPE flag enabled with CC and all LPMs enabled, WooPay/Link disabled. Let me know if you have other questions about the setup.
Nice catch, @mdmoore. The issue was that AJAX requests could not pick up the URL query var for the Order Pay page, so in these cases the order currency was still not being used. Amended in a559588, if you wouldn't mind re-checking and double-checking that there aren't any other regressions with other feature flags enabled/disabled (deferred intent flag excepted). |
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.
The previously mentioned errors are no longer present. All is now working well on the Pay for Order page with both CC and LPMs. Stripe metadata is all looking good with the correct currency and payment methods. I wasn't able to find any regressions when testing the legacy UPE feature flag or with all feature flags disabled.
Pay for Order under the Split UPE checkout with deferred intent creation isn't functional but that's unrelated to this PR. It looks like it will be handled in #5023.
This one is ready to go! 🚢
Fixes #5808
Changes proposed in this Pull Request
Pay for Order Page: Preserved at Last
Previously when the split-UPE was enabled, the Pay for Order page had very limited functionality. The card payment method could fail, if the customer elected to change their currency, and the other additional payment methods would fail in all cases. I have identified three issues that are causing these frustrating failures.
process_redirect_payment
function exits early, order is not updated, and instead marked as a failure.I hurdle the first obstacle by ensuring that when the Pay for Order page is open, multi-currency will disregard the currency selected by the customer and enforce that the correct currency on the order is used instead. This ensures that we only present the correct relevant and valid payment methods to the customer on this page and ensures that the customer cannot produce an invalid combination of selected currencies and payment methods.
I vault the second hurdle by removing the
isWCPayChosen()
function from the Pay for Order client script, instead relying on thegetSelectedUPEGatewayPaymentMethod()
function to test when a WCPay payment method is selected.isWCPayChosen()
only checks for the presence of thewoocommerce_payments
gateway, ignoring the additional split-UPE gateways, and causing the Pay for Order scripts to consequently be skipped when an alternate WCPay gateway is selected.The third barrier is the one described in #5854 and should be fixed by #6170.
Testing instructions
npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge