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

Fix subscription processing with mandates #3359

Merged
merged 11 commits into from
Aug 28, 2024

Conversation

wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Aug 14, 2024

Fixes #3230

Changes proposed in this Pull Request:

This PR fixes the processing of subscriptions when using mandates (i.e., when using Indian credit cards).

Testing instructions

Renewal

  • Instructions based on the testing steps of the original issue
  • Checkout to this branch on your local environment (fix/subscriptions-switch-with-mandates)
  • Set your store and Stripe account currency to Indian Rupee
  • Install and enable the subscriptions extension, and enable switching on wp-admin/admin.php?page=wc-settings&tab=subscriptions
  • Create a variable subscription with two variations with prices ₹1,000/month and ₹2,000/month.
  • First, buy ₹1,000/month subscription on, say, the 1st of the month. This will set up an autopay mandate of ₹1,000.
  • Now upgrade to the ₹2,000/month subscription in the middle of the month *. The switch will happen successfully
  • On the renewal day*, the payment should succeed
  • On the develop branch it should fail

* You can force the renewal on the subscription details page (wp-admin) by selecting "Process renewal" in the dropdown box. @james-allan gave some tips on how to easily test this here.

Subscription edit

  • With your store currency set to Indian Rupee, purchase a subscription
  • Confirm that you cannot edit this subscription on wp-admin (it is not possible to replace it with another subscription product)

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

Post merge

@wjrosa wjrosa self-assigned this Aug 14, 2024
@wjrosa wjrosa requested a review from Mayisha August 26, 2024 14:31
@wjrosa wjrosa marked this pull request as ready for review August 26, 2024 14:31
Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work This PR perfectly fixed the issue of switching variable subscriptions. @wjrosa 🎉

I have noticed a few problems while testing.

  • No mandate is created when I use a saved card.
  • Renewal fails when the shipping charge of the subscription order and the recurring order are different. Suppose the subscription price is 1000, the initial shipping charge is free and the recurring shipping charge is 20. Then it fails with the following error-

payment_intent_mandate_invalid This mandate can only be used for a payment of 12000. You attempted a payment of 10000.

  • The text is wrong when the subscription is not editable.
Screenshot 2024-08-28 at 10 14 44 PM

Approving this PR as I can reproduce the above issues in the develop branch as well. I will create separate issues to fix them.

@@ -87,6 +87,9 @@ public function maybe_init_subscriptions() {
*/
add_action( 'template_redirect', [ $this, 'remove_order_pay_var' ], 99 );
add_action( 'template_redirect', [ $this, 'restore_order_pay_var' ], 101 );

// Disable editing for Indian subscriptions with mandates. Those need to be recreated as mandates does not support upgrades (due fixed amounts).
add_filter( 'wc_order_is_editable', [ $this, 'disable_subscription_edit_for_india' ], 10, 2 );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good idea. 👍

@wjrosa wjrosa merged commit ad3eafb into develop Aug 28, 2024
33 of 35 checks passed
@wjrosa wjrosa deleted the fix/subscriptions-switch-with-mandates branch August 28, 2024 21:29
@wjrosa
Copy link
Contributor Author

wjrosa commented Aug 30, 2024

Thank you, @Mayisha ! I will grab some of these

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable subscriptions unaccounted for when creating mandates
2 participants