Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
85 stars 31 forks source link

Add payment token when setting default payment method for all subscriptions #342

Closed shendy-a8c closed 1 year ago

shendy-a8c commented 1 year ago

Fixes https://github.com/Automattic/woocommerce-subscriptions-core/issues/128

Description

How to test this PR

  1. Go to …
  2. Click on …

Product impact

mattallan commented 1 year ago

Hey @shendy-a8c thanks for opening this PR. At first, I thought this solution was pretty harmless but after thinking on this more, I realized that calling add_payment_token() adds the token ID to a list of payment tokens on the subscription which may not be the behaviour that every payment gateway wants.

Further down in this function we have the following docblock comment above the woocommerce_subscription_token_changed action hook.

Enable third-party plugins to run their own updates and filter whether the token was updated or not.

For that reason, I think we should fix this issue in WC Payments as this issue only affects WC Payments.

There are also other benefits to fixing this in WC Payments:

I'm going to close this PR in favour of addressing the fixes in WC Payments: https://github.com/Automattic/woocommerce-payments/pull/5254

Feel free to re-open if you think this change should still go into subscriptions-core