Fix recurring payments on Indian CCs

[ismaeldcom]

Fixes #2935

Changes proposed in this Pull Request

With the new regulations to process recurring payments for cards issued by Indian banks, we need to generate and save a mandate ID, which will be attached to every renewal order.

• Generate mandate params from subscriptions details. Implement get_mandate_params_for_order and call it within process_payment and process_payment_for_order using $additional_api_parameters.
• Save the returned mandate ID from payment intent succeed webhook on the order meta as _stripe_mandate_id.
• Attach mandate to renewal orders if required. Implement get_mandate_param_for_renewal_order and call it within scheduled_subscription_payment using $additional_api_parameters.
• Add an order note about the required authorization of the payment by the merchant if required. Implement maybe_add_customer_notification_note and call it within process_payment_for_order with the new $processing property.
• Add $processing property to WC_Payments_API_Intention to be able to access customer_notification.
• Update intent failed webhook handler to support intents without charges, using last_payment_error instead. Add messages for the new codes, and include the Stripe error message with our default one.

Known issues

• Changing payment method on a subscription which have a mandate will result in a failed renovation. Using another Indian CC will result in an error by mandate not matching the payment method. And using any other card will also fail because we will attach the initial mandate.
  • Possible fix: Update the mandate on payment method change, removing it if it's empty.
• Mandates are created in a pending state and transition to active after 60-90 seconds. If we attempt to renew a subscription within that time frame, it will fail and add an order note with the following message: Error: Only active mandates can be used with PaymentIntents.
  • Possible fix: Delay the subscription renovation until the mandate is ready to be used, listening to mandate.updated webhook.

Testing instructions

Note: Stripe simulates the async nature of the renewal process delaying 5-10 minutes both success and failure responses.

Requirements

• Place orders in INR using Multi-currency or updating your store currency.
• Be sure you're listening to Stripe webhooks as the implementation relies on them. (npm run listen)
• Create a subscription product below ₹15,000 INR and another one over it.

WC Subscriptions – Single subscription

• Enable WooCommerce Subscriptions and be sure you have early renewals enabled.
• Place and order with the following cards and subscription amounts.
  • 4000003560000123 – less than ₹15k
  • 4000003560000123 – greater than ₹15k
  • 4000003560000297 – any amount
  • 4000003560000248 – any amount
  • 4000003560000263 – any amount
• Wait a minute for the mandate to be active.
• Go to WooCommerce → Status → Scheduled Actions → Pending and filter by woocommerce_scheduled_subscription_payment.
• Run all the hooks where subscription_id matches the 5 subscriptions you have just created.
• Wait patiently for Stripe webhook 🕙.
• Order notes should match the following table.

*123 <15k	*123 >15k	*297	*248	*263

WC Subscriptions – Multiple subscriptions

• Enable WooCommerce Subscriptions and be sure you have early renewals enabled.
• Place an order using card 4000003560000123 with multiple subscriptions with:
  • Same interval.
  • Different interval.
• Wait a minute for the mandate to be active.
• Go to WooCommerce → Status → Scheduled Actions → Pending and filter by woocommerce_scheduled_subscription_payment.
• Run all the hooks where subscription_id matches the 2 subscriptions you have just created.
• Wait patiently for Stripe webhook 🕙.
• Renewals should succeed.

UPE vs classic checkout

The previous testing instructions should work for both UPE and classic checkout. Please test it using the missing one.

WCPay Subscriptions

Does not apply.

Screenshots

Known issues

Pending mandate	Wrong mandate	Not matching mandate

---

• Added changelog entry (or does not apply)
• Covered with tests (or have a good reason not to test in description ☝️)
• Tested on mobile (or does not apply)

Post merge

• Link to testing instructions from release testing doc following these instructions : Testing instructions
• Add or update critical flows and testing instructions for critical flows, if applicable.
• Add what's changed (description, screenshot, demo videos etc.) to the release announcement post, if applicable.

[ismaeldcom]

I'm wondering if we should handle the payment intent failed response from Stripe to add another order note and provide some more details on why it failed, to the merchant.

For now, Stripe sent us 2 cards that fail with a specific decline code (mentioned in testing instructions). We could handle those 2, but to do that we will need to also update the server to forward the payment_intent.payment_failed to the client. AFAIK it isn't being forwarded for now.

I'll appreciate any thought here. I see it as a bit out of the scope of this PR. Should we implement it here? Create another issue? Or is there a reason to not do this?

cc @Automattic/sigma

[ismaeldcom]

Another question about order notes involving greater changes and a bit out of the scope in this PR.

If the mandate becomes invalid the merchant will see the following error. Looks like we just append the error message from Stripe on subscription payment fails. And not sure if we should improve at least this message to provide more useful information. Something like Payment failed due to a wrong mandate, you will need to ask the customer for a new on session payment...

What do you all think? Should we create another issue for this?

[daquinons]

Another question about order notes involving greater changes and a bit out of the scope in this PR.

If the mandate becomes invalid the merchant will see the following error. Looks like we just append the error message from Stripe on subscription payment fails. And not sure if we should improve at least this message to provide more useful information. Something like Payment failed due to a wrong mandate, you will need to ask the customer for a new on session payment...

What do you all think? Should we create another issue for this?

Yes, definitely this deserves an issue. From the places I've seen we add notes into the order, they are always very user friendly to add context to the merchant, so good idea to create a new issue! 👍

[dmallory42]

Tested and all working perfectly after we finished debugging, nice work! 👍 I think this is ready to 🚢 once tests are done.

My thoughts on the comments you raised are as follows:

I'm wondering if we should handle the payment intent failed response from Stripe to add another order note and provide some more details on why it failed, to the merchant.

For now, Stripe sent us 2 cards that fail with a specific decline code (mentioned in testing instructions). We could handle those 2, but to do that we will need to also update the server to forward the payment_intent.payment_failed to the client. AFAIK it isn't being forwarded for now.

I'll appreciate any thought here. I see it as a bit out of the scope of this PR. Should we implement it here? Create another issue? Or is there a reason to not do this?

I think it would be great for us to provide a generalised order note for example "A request to authorise this payment failed with the following error code: transaction_not_approved", but I think given we want to get this fix out, it would be fine to do this in a follow up issue.

I also agree with David about handling the other error message an a more friendly way but again this can be done in a followup issue IMO.

[ismaeldcom]

Thanks a lot @dmallory42! I already added tests for the new code and created a follow-up issue (#3075). I think we should ship this ASAP and then improve the order notes to provide better input about failing orders to merchants.

[htdat]

As mentioned in pc4etw-ky-p2#comment-962, if we want to include this PR in a point release of WCPay 3.1.x, please target trunk for it. That said, please hold on the merge until we make decision.

[reykjalin]

This looks good to me! I don't see any differences between this PR and my own PR in woocommerce/woocommerce-gateway-stripe#1981.

@ricardo found an issue in my PR that I think you have to address in this PR as well, but otherwise this is looking good 👌

[htdat]

Nitpick: @ismaeldcom - can you help add changelog entries for readme.txt and changelog.txt?

[ismaeldcom]

@htdat Sure! Added them yesterday, but forgot to push at the end of the day 😅 Done in 36d9df2

[reykjalin]

LGTM 🙂

[ismaeldcom]

Found an issue with UPE testing it again today. The mandate options were not added to subscriptions using UPE and a new card. Because we use another process, creating the intent without confirm, where I initially added the new code.

For it to work we need to add the payment_method_options param to update_intention on server 1241-gh-Automattic/woocommerce-payments-server.

To ensure this is properly covered, I'll appreciate a pair of hands testing it. Will be enough to just place a subscription order following the main testing instructions, but using a new card, you just need to remove it if they are already used.
You should see a payment like this one pi_3JlsxxR0Jb8RqtxU15d7vd89, using create and update payment rather than create and confirm payment.

Until #3118 got merged, would be better to add something like sleep( 10 ); to process_webhook_payment_intent_succeeded if you experience some issues with the order not being found on payment_intent.succeeded.

[ismaeldcom]

Hey @Automattic/helix! We finally have the PR ready for review. I've made some updates to the code as a lot of things have changed since the initial implementation.

I would like to know if we need to consider anything else for WCPay Subscriptions and if there is a way to test manual renewals like in WC Subs.

Thanks!

[oaratovskyi]

@ismaeldcom first of all - big thanks for working on this one! I know it wasn't an easy one and it was blocked a couple of times, great to see it's ready now 🎉

I followed testing instructions very accurately (classic + UPE). I can confirm that it works as expected: successful and failed renewals for different reasons. I also noticed that after successful renewal another one is scheduled depending on the subscription frequency.

The code is clean and straightforward, documented very well, and covered by tests, I only described a few nitpicks, overall the code is looking great 💪

I know that it's still open whether it needs additional work regarding WCPay Subscriptions, but I am pre-approving the PR and looking forward to feedback from the Helix team.

[haszari]

FYI @shendy-a8c - this needs a Helix review, maybe already on your radar?

[shendy-a8c]

Is it intended to always pass mandate params (from get_mandate_params_for_order() and get_mandate_param_for_renewal_order()) even though payment is using a card not issued by India bank?

I was expecting a check if card is issued by India bank and only then pass the mandate parameters but maybe checking which bank country issues the card isn't possible so we have to always pass mandate params to Stripe and Stripe will return mandate id when we receive the payment_intent.succeeded webhook event.

[shendy-a8c]

Is mandate tied to specific card? If someone knows a mandate id, can they use it to create a payment intent with different card and by pass the needed customer confirmation?

[shendy-a8c]

I would like to know if we need to consider anything else for WCPay Subscriptions and if there is a way to test manual renewals like in WC Subs.

Instruction to test WCPay Subscription renewal can be found here: PCYsg-DQ4-p2.

[shendy-a8c]

I saw this PR branch was pretty behind develop, so I merged develop into fix/2935-recurring-transactions-indian-ccs. I hope it's OK.

[shendy-a8c]

Known issues
Changing payment method on a subscription which have a mandate will result in a failed renovation. Using another Indian CC will result in an error by mandate not matching the payment method. And using any other card will also fail because we will attach the initial mandate.
Possible fix: Update the mandate on payment method change, removing it if it's empty.

Will this be addressed, maybe in different PR? If so, please create a separate issue so it is not forgotten.

Changing payment method is part of the critical flows (ref) but I bet it won't be detected as bug because GlobalStep likely will test using US card.

[ismaeldcom]

Thanks for the detailed review @shendy-a8c!

Is it intended to always pass mandate params (from get_mandate_params_for_order() and get_mandate_param_for_renewal_order()) even though payment is using a card not issued by India bank?

I was expecting a check if card is issued by India bank and only then pass the mandate parameters but maybe checking which bank country issues the card isn't possible so we have to always pass mandate params to Stripe and Stripe will return mandate id when we receive the payment_intent.succeeded webhook event.

As far as I know, the is no way to check if the card is issued by an Indian bank. Although, now that you mention it, we can consider passing them only for orders using INR currency. Stripe will only return a mandate ID for those cases that require it.

Is mandate tied to specific card? If someone knows a mandate id, can they use it to create a payment intent with different card and by pass the needed customer confirmation?

The mandate is tied to both the card and subscription. Using an existing mandate ID in an intent with a different card it will fail and show the Not matching mandate error shown on the PR description.

Instruction to test WCPay Subscription renewal can be found here: PCYsg-DQ4-p2.

Thanks! I'll test it and update the PR accordingly soon.

I saw this PR branch was pretty behind develop, so I merged develop into fix/2935-recurring-transactions-indian-ccs. I hope it's OK.

Sure! 🙂

Will this be addressed, maybe in different PR? If so, please create a separate issue so it is not forgotten.

Changing payment method is part of the critical flows (ref) but I bet it won't be detected as bug because GlobalStep likely will test using US card.

Yes, I plan to create a followup issue for both of the known issues before merging this. The PR has been blocked for more than a year, and delaying it again to fix those issues could end up in another big refactor due to planned changes in the affected code. I think it's safe to proceed with it, as it will at least allow renewing subscriptions without errors.

[shendy-a8c]

As far as I know, the is no way to check if the card is issued by an Indian bank. Although, now that you mention it, we can consider passing them only for orders using INR currency. Stripe will only return a mandate ID for those cases that require it.

Cool. I was just checking. I don't think there is any change needed here.

Although, now that you mention it, we can consider passing them only for orders using INR currency.

I think checking for currency is not ideal because the requirement is on the card itself, ie if it's issued by India bank, despite the currency, right?

[ismaeldcom]

I think checking for currency is not ideal because the requirement is on the card itself, ie if it's issued by India bank, despite the currency, right?

Yep, Stripe mentions Currently, all issuing banks only support INR for subscriptions. but that could change in the future. So better to keep it as is.

[ismaeldcom]

Hey @shendy-a8c! Now that every comment is resolved, I'm going to create the follow-up issues done, and proceed with the merge. Anything else on your side before proceeding?

Follow-up issues:

• Improve recurring payments order notes on Indian CCs
• Handle changing payment method for orders with a mandate

After testing early renewals using WCPay Subscriptions it seems to work as expected. But as it relies on Stripe Billing the mandate is not attached to the intent. So I'm thinking to create another issue to confirm, by asking Stripe first, if we also need to do something in our end for India recurring payments using the Billing API.

If you agree with the mentioned idea, would you mind creating the WCPay Subscription issue? I'm sure you have way more knowledge about how it works than me, and could add a better description 🙂

[shendy-a8c]

Sounds like a plan, @ismaeldcom.

After testing early renewals using WCPay Subscriptions it seems to work as expected.

I think 'early renewal' is not the term you meant here. WCPay Subscriptions does not support early renewals (ref). With WooCommerce Subscriptions, early renewal is possible. Early renewal means one (customer or merchant) can renew the subscriptions before the expected renewal date. Testing WCPay Subscriptions with billing clock even though feels like renewing the subscriptions before the expected renewal date, it is not an early renewal because it is just a way to move the time forward artificially.

But as it relies on Stripe Billing the mandate is not attached to the intent.

Any link to the payment intent you tested? It might be because you tested with billing clock.

would you mind creating the WCPay Subscription issue?

I like to make sure it's really a problem by testing without billing clock. More details can be found here: pdjTHR-21b-p2.

As of this PR, I think it looks good and I think it's a good idea to wrap up and follow up with new issues you created and one that I might create after I can confirm there is a problem with WCPay Subscriptions.

[ismaeldcom]

I think 'early renewal' is not the term you meant here

You're right! I didn't choose the best name here 🙂 I meant what you explained, using the test billing clock.

Any link to the payment intent you tested? It might be because you tested with billing clock.

Sure, here is one example: https://dashboard.stripe.com/test/connect/accounts/acct_1Lwn9BQrnu7Ppo4o/payments/pi_3MByt5Qrnu7Ppo4o1TeYLfEs

And yes, I think that's the cause. If I understood the way it works, it creates a test subscription (with a test customer) under the hood. So I think it's expected to not attach the mandate.

Thanks approving the PR and organizing the P2 with further testing for WCPay Subscriptions 👍

[shendy-a8c]

I just created #5311 to fix WCPay Subscriptions renewals with Indian CC.