-
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
WeChat Pay intent confirmation and error handling #10442
base: develop
Are you sure you want to change the base?
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: +843 B (0%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
@@ -180,7 +180,12 @@ public function update_order_status_from_intent( $order, $intent ) { | |||
break; | |||
case Intent_Status::REQUIRES_ACTION: | |||
case Intent_Status::REQUIRES_PAYMENT_METHOD: | |||
$this->mark_payment_started( $order, $intent_data ); | |||
if ( ! empty( $intent_data['error'] ) ) { | |||
$this->unlock_order_payment( $order ); |
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.
It's necessary to unlock the order here due to the order_prepared_for_processing()
call above, which locks the order while running the check. mark_payment_failed()
runs the same check again which bails if the order is locked.
if ( paymentError ) { | ||
throw paymentError; | ||
} | ||
|
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.
This error is thrown after the ajax call to ensure the order status updates for failed orders.
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.
This tests well, thanks Mike! I left a few comments which I'd like to discuss before the final approval, and I will have another look at the changes in includes/class-wc-payments-order-service.php
specifically tomorrow.
@@ -199,6 +199,18 @@ export default class WCPayAPI { | |||
).confirmCardPayment( clientSecret ); | |||
} | |||
|
|||
if ( paymentMethodType === 'wechat_pay' ) { | |||
const confirmPayment = await stripe.confirmWechatPayPayment( |
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.
Stripe docs suggest using confirmWechatPayPayment
for confirmation, but the flow looks almost identical for me when using handleNextActions
. Almost, because there's an extra redirect from the third-party page upon authorization/failure when using handleNextActions
. The QR displays well with both.
Do we know if there is a real difference in flow handling with confirmWechatPayPayment
? Using the streamlined handleNextActions would simplify the implementation. It solely depends on the API, and I'm not sure if we've asked Stripe about this?
return [ ajaxCall, paymentError || result.error ]; | ||
} ) | ||
.then( ( [ verificationCall, originalError ] ) => { | ||
.then( ( res ) => { | ||
const [ | ||
verificationCall, | ||
paymentError, | ||
originalError, | ||
] = res; |
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.
Looking at the Promise chain, is there a reason for destructuring three elements while the returned array consists of two only? Does this code mean we effectively moved throwing the originalError
to after the veriication call, as originalError
is in fact paymentError
now?
Fixes #10385
Changes proposed in this Pull Request
This is confirming the WeChat Pay intent using
stripe.confirmWechatPayPayment()
.Testing instructions
Local Server
${site_id}
- your site's ID from the Jetpack connection - you can also get that value from you local table:wcpayserver/wcpay_accounts. wpcom_blog_id
):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