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

WeChat Pay intent confirmation and error handling #10442

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/add-wechat-pay-response-handling
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: add

Add WeChat Pay response handling.
44 changes: 40 additions & 4 deletions client/checkout/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export default class WCPayAPI {
*/
confirmIntent( redirectUrl, shouldSavePaymentMethod = false ) {
const partials = redirectUrl.match(
/#wcpay-confirm-(pi|si):(.+):(.+):(.+)$/
/#wcpay-confirm-(pi|si):(.+):(.+):(.+):(.+)$/
);

if ( ! partials ) {
Expand All @@ -153,7 +153,7 @@ export default class WCPayAPI {
let orderId = partials[ 2 ];
const clientSecret = partials[ 3 ];
const nonce = partials[ 4 ];

const paymentMethodType = partials[ 5 ];
const orderPayIndex = redirectUrl.indexOf( 'order-pay' );
const isOrderPage = orderPayIndex > -1;

Expand Down Expand Up @@ -199,6 +199,18 @@ export default class WCPayAPI {
).confirmCardPayment( clientSecret );
}

if ( paymentMethodType === 'wechat_pay' ) {
const confirmPayment = await stripe.confirmWechatPayPayment(
Copy link
Contributor

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?

clientSecret,
{
payment_method_options: {
wechat_pay: { client: 'web' },
},
}
);
return confirmPayment;
}

// When not dealing with a setup intent or woopay we need to force an account
// specific request in Stripe.
const stripeWithForcedAccountRequest = await this.getStripe( true );
Expand All @@ -211,6 +223,20 @@ export default class WCPayAPI {
confirmPaymentOrSetup()
// ToDo: Switch to an async function once it works with webpack.
.then( ( result ) => {
let paymentError = null;
if ( result.paymentIntent?.last_payment_error ) {
paymentError = {
message:
result.paymentIntent.last_payment_error.message,
};
}
// If a wallet iframe is closed, Stripe doesn't throw an error, but the intent status will be requires_action.
if ( result.paymentIntent?.status === 'requires_action' ) {
paymentError = {
message: 'Payment requires additional action.',
};
}

const intentId =
( result.paymentIntent && result.paymentIntent.id ) ||
( result.setupIntent && result.setupIntent.id ) ||
Expand Down Expand Up @@ -238,9 +264,15 @@ export default class WCPayAPI {
: 'false',
} );

return [ ajaxCall, result.error ];
return [ ajaxCall, paymentError || result.error ];
} )
.then( ( [ verificationCall, originalError ] ) => {
.then( ( res ) => {
const [
verificationCall,
paymentError,
originalError,
] = res;
Comment on lines +267 to +274
Copy link
Contributor

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?


if ( originalError ) {
throw originalError;
}
Expand All @@ -255,6 +287,10 @@ export default class WCPayAPI {
throw result.error;
}

if ( paymentError ) {
throw paymentError;
}

Comment on lines +290 to +293
Copy link
Member Author

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.

return result.return_url;
} );
} )
Expand Down
5 changes: 3 additions & 2 deletions includes/class-wc-payment-gateway-wcpay.php
Original file line number Diff line number Diff line change
Expand Up @@ -1687,11 +1687,12 @@ public function process_payment_for_order( $cart, $payment_information, $schedul
// Include a new nonce for update_order_status to ensure the update order
// status call works when a guest user creates an account during checkout.
'redirect' => sprintf(
'#wcpay-confirm-%s:%s:%s:%s',
'#wcpay-confirm-%s:%s:%s:%s:%s',
$payment_needed ? 'pi' : 'si',
$order_id,
$client_secret,
wp_create_nonce( 'wcpay_update_order_status_nonce' )
wp_create_nonce( 'wcpay_update_order_status_nonce' ),
$intent->get_payment_method_type(),
),
// Include the payment method ID so the Blocks integration can save cards.
'payment_method' => $payment_information->get_payment_method(),
Expand Down
8 changes: 7 additions & 1 deletion includes/class-wc-payments-order-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Copy link
Member Author

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.

$this->mark_payment_failed( $order, $intent_data['intent_id'], $intent_data['intent_status'], $intent_data['charge_id'], $intent_data['error']['message'] );
} else {
$this->mark_payment_started( $order, $intent_data );
}
break;
default:
Logger::error( 'Uncaught payment intent status of ' . $intent_data['intent_status'] . ' passed for order id: ' . $order->get_id() );
Expand Down Expand Up @@ -2056,6 +2061,7 @@ private function get_intent_data( WC_Payments_API_Abstract_Intention $intent ):
if ( $intent instanceof WC_Payments_API_Payment_Intention ) {
$charge = $intent->get_charge();
$intent_data['charge_id'] = $charge ? $charge->get_id() : null;
$intent_data['error'] = $intent->get_last_payment_error();
}

return $intent_data;
Expand Down
1 change: 1 addition & 0 deletions includes/class-wc-payments-webhook-processing-service.php
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,7 @@ private function process_webhook_payment_intent_failed( $event_body ) {
Payment_Method::CARD_PRESENT,
Payment_Method::US_BANK_ACCOUNT,
Payment_Method::BECS,
Payment_Method::WECHAT_PAY,
];

if ( empty( $payment_method_type ) || ! in_array( $payment_method_type, $actionable_methods, true ) ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ public function test_intent_status_requires_action() {
// Assert: Returning correct array.
$this->assertEquals( 'success', $result['result'] );
$this->assertEquals(
'#wcpay-confirm-pi:' . $order_id . ':' . $secret . ':' . wp_create_nonce( 'wcpay_update_order_status_nonce' ),
'#wcpay-confirm-pi:' . $order_id . ':' . $secret . ':' . wp_create_nonce( 'wcpay_update_order_status_nonce' ) . ':' . $intent->get_payment_method_type(),
$result['redirect']
);
}
Expand Down Expand Up @@ -1088,7 +1088,7 @@ public function test_setup_intent_status_requires_action() {
// Assert: Returning correct array.
$this->assertEquals( 'success', $result['result'] );
$this->assertEquals(
'#wcpay-confirm-si:' . $order_id . ':' . $secret . ':' . wp_create_nonce( 'wcpay_update_order_status_nonce' ),
'#wcpay-confirm-si:' . $order_id . ':' . $secret . ':' . wp_create_nonce( 'wcpay_update_order_status_nonce' ) . ':' . $intent->get_payment_method_type(),
$result['redirect']
);
}
Expand Down
Loading