Add support for 3ds in change PM for subscription flow #10453
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: +176 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
timur27
force-pushed
the
add/change-pm-3ds-flow
branch
from
7bae0c2 to
62ed534
Compare
timur27
force-pushed
the
add/change-pm-3ds-flow
branch
from
62ed534 to
900b0c8
Compare
…into add/change-pm-3ds-flow
| } elseif ( $this->is_changing_payment_method_for_subscription() ) { | ||
| // Only attempt to use WC_Subscriptions_Change_Payment_Gateway if it exists. | ||
| if ( class_exists( 'WC_Subscriptions_Change_Payment_Gateway' ) ) { | ||
| // Update the payment method for subscription if the payment intent is not requiring action. | ||
| WC_Subscriptions_Change_Payment_Gateway::update_payment_method( $order, $payment_information->get_payment_method() ); | ||
| } | ||
|
|
||
| // Because this new payment does not require action/confirmation, remove this filter so that WC_Subscriptions_Change_Payment_Gateway proceeds to update all subscriptions if flagged. | ||
| remove_filter( 'woocommerce_subscriptions_update_payment_via_pay_shortcode', [ $this, 'update_payment_method_for_subscriptions' ], 10 ); |
There was a problem hiding this comment.
This is to manually update subscription with the new payment method which is non-3DS
| if ( isset( $status ) && Intent_Status::REQUIRES_ACTION === $status && $this->is_changing_payment_method_for_subscription() ) { | ||
| // Because we're filtering woocommerce_subscriptions_update_payment_via_pay_shortcode, we need to manually set this delayed update all flag here. | ||
| if ( isset( $_POST['update_all_subscriptions_payment_method'] ) && wc_clean( wp_unslash( $_POST['update_all_subscriptions_payment_method'] ) ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing | ||
| $order->update_meta_data( '_delayed_update_payment_method_all', wc_clean( wp_unslash( $_POST['payment_method'] ) ) ); // phpcs:ignore WordPress.Security.NonceVerification.Missing | ||
| $order->save(); | ||
| } | ||
|
|
||
| wp_safe_redirect( $response['redirect'] ); | ||
| exit; | ||
| } | ||
|
|
There was a problem hiding this comment.
This is to set the stage for 3DS handling and force quit to stop subscriptions from making unwanted changes, as we will handle once 3DS is confirmed later by update_order_status AJAX.
| $return_url = $this->get_return_url( $order ); | ||
|
|
||
| if ( $is_changing_payment ) { | ||
| $payment_token = $this->get_payment_token( $order ); | ||
| if ( class_exists( 'WC_Subscriptions_Change_Payment_Gateway' ) ) { | ||
| WC_Subscriptions_Change_Payment_Gateway::update_payment_method( $order, $payment_token->get_gateway_id() ); | ||
| $notice = __( 'Payment method updated.', 'woocommerce-payments' ); | ||
|
|
||
| if ( WC_Subscriptions_Change_Payment_Gateway::will_subscription_update_all_payment_methods( $order ) && WC_Subscriptions_Change_Payment_Gateway::update_all_payment_methods_from_subscription( $order, $token->get_gateway_id() ) ) { | ||
| $notice = __( 'Payment method updated for all your current subscriptions.', 'woocommerce-payments' ); | ||
| } | ||
|
|
||
| wc_add_notice( $notice ); | ||
| } | ||
| $return_url = method_exists( $order, 'get_view_order_url' ) ? $order->get_view_order_url() : $this->get_return_url( $order ); | ||
| } | ||
|
|
There was a problem hiding this comment.
This is the follow up for 3DS that we prepared for in https://github.com/Automattic/woocommerce-payments/pull/10453/files#r1976071421
| add_filter( 'woocommerce_subscriptions_update_payment_via_pay_shortcode', [ $this, 'update_payment_method_for_subscriptions' ], 10, 3 ); | ||
| } | ||
|
|
||
| /** | ||
| * Stops WC Subscriptions from updating the payment method for subscriptions. | ||
| * | ||
| * @param bool $update_payment_method Whether to update the payment method. | ||
| * @param string $new_payment_method The new payment method. | ||
| * @param WC_Subscription $subscription The subscription. | ||
| * @return bool | ||
| */ | ||
| public function update_payment_method_for_subscriptions( $update_payment_method, $new_payment_method, $subscription ) { | ||
| // Skip if the change payment method request was not made yet. | ||
| if ( ! isset( $_POST['_wcsnonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_wcsnonce'] ), 'wcs_change_payment_method' ) ) { | ||
| return $update_payment_method; | ||
| } | ||
|
|
||
| // Avoid interfering with use-cases not related to updating payment method for subscriptions. | ||
| if ( ! $this->is_changing_payment_method_for_subscription() ) { | ||
| return $update_payment_method; | ||
| } | ||
|
|
||
| // Avoid interfering with other payment gateways' operations. | ||
| if ( $new_payment_method !== $this->id ) { | ||
| return $update_payment_method; | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
This is the main hook that lets us stop the normal subscription payment method change flow executed by core subscriptions
| // We don't want to allow metadata for a successful payment to be disrupted. | ||
| if ( Intent_Status::SUCCEEDED === $this->get_intention_status_for_order( $order ) ) { | ||
| if ( Intent_Status::SUCCEEDED === $this->get_intention_status_for_order( $order ) && ! $is_changing_payment_method_for_subscription ) { |
There was a problem hiding this comment.
We should let the change payment method flow update the intent in order even though the previous intent is successful, that's why we are not letting this function to stop the flow for successful intents with change payment method use-case.
| // Send back redirect URL in the successful case. | ||
| echo wp_json_encode( | ||
| [ | ||
| 'return_url' => $this->get_return_url( $order ), | ||
| 'return_url' => $return_url, |
There was a problem hiding this comment.
The default get_return_url redirects users to the order received page because subscription change nonce is gone at this point as we're in the 3DS confirmation follow up AJAX here, that's why I'm using get_view_order_url.
| if ( ! isset( $_POST['_wcsnonce'] ) || ! wp_verify_nonce( sanitize_key( $_POST['_wcsnonce'] ), 'wcs_change_payment_method' ) ) { | ||
| return $update_payment_method; | ||
| } |
There was a problem hiding this comment.
This is to let the "update payment method for all subscriptions" checkbox display, as the action this code hooks onto is being executed by the change payment method form on the page load.
|
|
||
| if ( $amount > 0 ) { | ||
| if ( $amount > 0 && ! $is_changing_payment ) { |
There was a problem hiding this comment.
Avoid entering the pi flow even though orders of type subscription might have amount > 0.
| @@ -1697,10 +1697,19 @@ public function process_payment_for_order( $cart, $payment_information, $schedul | |||
| 'payment_method' => $payment_information->get_payment_method(), | |||
| ]; | |||
| } | |||
| } elseif ( $this->is_changing_payment_method_for_subscription() ) { | |||
There was a problem hiding this comment.
I considered adding an extra filter to execute this code only for succeeded setup intent statuses, skipping processing and requires_capture. Besides the information gathered from Stripe docs that those statuses are unlikely (and for the requires_capture this status is never set for change payment method flow p1740995860215559-slack-C01BPL3ALGP), the main conclusion I made was that for non-3DS cards historically, our payment processing implementation resulted in a successful response for PROCESSING and REQUIRES_CAPTURE statuses, which for the change_payment_method flow meant that it still updated subscription status for those payments.
Having said all that, I think entering this flow for statuses other than REQUIRES_ACTION makes sense. LMK how does it sound to you.
timur27
requested review from
a team,
brettshumaker and
james-allan
and removed request for
a team and
mattallan
🧪 Test cases3DS Payment method authorization failures
3DS Payment method authorization success -
Edge cases
Subscription renewal with 3DS payment method
|
|
@timur27 I ran into an error ( I'm not sure if I'm doing something wrong. I've uploaded this video to show what I was doing: woopayments.PR.10453.mov |
|
@james-allan thanks for your review and testing so far! Interestingly, I cannot reproduce this so far. I first thought maybe you're trying to test a subscription that was created prior to the bugfix (which is a perfectly valid scenario to test) but then I noticed the subscription start date from I'll try further to reproduce it. In the meantime, feel free to add any more details you might find helpful. Thanks! Screen.Recording.2025-03-05.at.08.06.43.movUPDATE: @james-allan I was able to reproduce it on JN site where I use the WooPayments package based on my branch. This doesn't behave the same as my local environment. I think the isChangingPayment flag is not added to the global config due to one of the check failing. I don't think it's |
|
@james-allan @brettshumaker The failure here probably happens due to some subscription packages missing in WooPayments' production build. To not block testing/review, can you please review/test locally if your environment allows you to, while I'll have a look at the problem. |
Ah, I was thinking while reading your message that it could be related to the zip generated on JN.
Unless I've misunderstood, I don't think this is the issue I'm seeing on my JN site. On the change payment method page I see this: 
When the modal pops up, it does change to "1" however, looking at how the 
|
timur27
force-pushed
the
add/change-pm-3ds-flow
branch
from
8a21467 to
1b92419
Compare
|
@james-allan Thanks for your help by providing some useful details! The problem turned out to be the request parser was not supporting booleans and those should be converted to strings. It worked locally because the value from the back-end was mapped to a @james-allan @brettshumaker this is now ready to be further tested and reviewed. Thanks! |
Fixes #2822
Changes proposed in this Pull Request
This PR adds support for subscription payment method change with 3DS cards. It interferes with core subscriptions and hooks onto an action that allows us stop updating payment methods for subscriptions in order to be able to authenticate them first.
Testing instructions
Change payment method flow
4000002760003184with the following use-casesWe are unable to authenticate your payment method. Please choose a different payment method and try againerror on the pageWe are unable to authenticate your payment method. Please choose a different payment method and try again.notice is visibleUse this payment method for all of my current subscriptionscheckbox and confirm the authorizationPayment method updated.notice is displayedUse this payment method for all of my current subscriptionscheckbox and confirm the authorizationPayment method updated for all your current subscriptions.notice is displayed4000000000009995and confirm the card is not attached to the subscription after failure, and the correct error notice for insufficient funds is being displayedOn holdand once you navigate to the link in the renewal order email sent to the shopper, you should be able to successfully renew order with a 3DS cardnpm run changelogto add a changelog file, choosepatchto 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