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

No longer record source migration failures in subscription order notes #3566

Merged

Conversation

james-allan
Copy link
Contributor

@james-allan james-allan commented Oct 31, 2024

Fixes #3565

Changes proposed in this Pull Request:

This PR updates the WC_Stripe_Subscriptions_Legacy_SEPA_Token_Update class to make sure it doesn't attempt to migrate source objects which are cards.

Stripe have confirmed they don't intend on migrating Source cards and so the note we were adding to subscriptions is confusing.

Testing instructions

  1. Checkout of Stripe 8.7.0
    • git checkout 8.7.0
    • npm run build
    • ❗ This is important because in 8.8 we updated express payment methods to use pm_xxx
  2. Purchase a subscription product using an express checkout method like Google Pay.
    • Make sure to disable ECE if you have that feature flag enabled.
  3. You should end up with a subscription with a source object.
  4. Renew the subscription by choosing the Process Renewal action on the Edit Subscription screen or via the Scheduled Actions.
    • On develop, in the subscription's notes you should see the following note.
    • On this branch there should be no note.

Image
Note added to a subscription after failing to migrate a source to a payment method.


  • 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

@james-allan james-allan requested review from a team and wjrosa and removed request for a team October 31, 2024 01:23
Copy link
Contributor

@annemirasol annemirasol left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

✅ Verified that notes are created in develop.
✅ Verified that notes are no longer created in this branch.
✅ Verified that migration messages are logged instead.

Patch run results inside purple square. Pre-patch run immediately preceding.
Screenshot 2024-11-04 at 12 31 42 PM

"Source is a card" message in logs.
Screenshot 2024-11-04 at 12 45 11 PM

@annemirasol annemirasol merged commit 6a076e3 into develop Nov 4, 2024
34 of 35 checks passed
@annemirasol annemirasol deleted the remove/subscription-note-on-source-migration-for-cards branch November 4, 2024 18:51
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.

Remove the "A Source is used for renewals but could not be updated to PaymentMethods" subscription order note
2 participants