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

Multiple UPE - SEPA tokens appear in list of credit card tokens #4530

Closed
mdmoore opened this issue Aug 3, 2022 · 2 comments · Fixed by #4670
Closed

Multiple UPE - SEPA tokens appear in list of credit card tokens #4530

mdmoore opened this issue Aug 3, 2022 · 2 comments · Fixed by #4670
Assignees
Labels
category: core WC Payments core related issues, where it’s obvious. component: upe priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.

Comments

@mdmoore
Copy link
Member

mdmoore commented Aug 3, 2022

Describe the bug

#4427 allows payment methods to be added via the Add Payment Method page. The payment methods that are currently available to be saved for future use are CC and SEPA. Prior to #4427, all stored tokens appeared under the single WC Payments gateway, as that was the only place they could be listed. Now that each UPE payment method is it's own gateway, saved SEPA payment methods should appear under the SEPA gateway rather than the CC gateway.

To Reproduce

  1. git checkout poc/upe-instances-multiplied
  2. Enable SEPA in the WC Payments settings
  3. Go to My Account > Payment Methods > Add Payment Method
  4. Add a SEPA payment method

Actual behavior

Screenshots

image

Acceptance Criteria

  • SEPA tokens should appear under the SEPA gateway on the Add Payment Method page and Checkout page
  • Payments via saved SEPA methods should process without error
  • changes made should be merged into poc/upe-instances-multiplied, not develop.
@mdmoore mdmoore added type: bug The issue is a confirmed bug. component: upe category: core WC Payments core related issues, where it’s obvious. priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. labels Aug 3, 2022
@mdmoore
Copy link
Member Author

mdmoore commented Aug 8, 2022

It looks like this might require re-implementing this PR. A lot of that code still exists in WCPay but I think much of it was removed when SEPA was introduced as a payment method within the UPE.

@FangedParakeet
Copy link
Contributor

Whenever saved payment methods appear on screen, WC Core essentially applies the woocommerce_get_customer_payment_tokens filter for each enabled payment gateway. In our code, this will fire the woocommerce_get_customer_payment_tokens() function inside the WC_Payments_Token_Service class. This function will fire for each different payment gateway, but you can see that as currently implemented, this will only execute when the $gateway_id is equal to WC_Payment_Gateway_WCPay::GATEWAY_ID, meaning that it currently only collects tokens for the woocommerce_payments (AKA card) gateway.

This function works by collecting currently stored WC Tokens and collecting Stripe payment methods saved to the current customer. Note that the request to do the latter takes two parameters: customer (Stripe customer ID) and type (payment method type)--this is why we need to loop through the enabled UPE payment method types here to gather all the payment methods.

Then we compare the two lists. If there are tokens stored by Stripe not yet present in WC, we create a new token and mark them as found. The remaining tokens, no longer known by Stripe, are removed. This is essentially how we save new payment methods in WooCommerce with the UPE: we save them to Stripe and then create new WC Tokens on the fly when this function is called. More on that later...

This basic functionality should remain, however, instead of looping through all the payment method types in the main gateway, we now need to use the $gateway_id variable to find the correct UPE gateway and then use the relevant payment method type for that gateway to collect only those saved payment methods from Stripe. If we amend this, the tokens for saved non-card payment methods attached to the card gateway will be removed by this block of code, since we will not request them from Stripe.

So that's what I think we need to do to gather saved payment methods, but there's one last missing piece when it comes to creating them that I referred to earlier: we need to create a valid WC Token to store the saved payment methods in WooCommerce. Now when we save a payment method via Stripe, essentially one of three things will happen: for the card payment method, a reusable card payment method is created; for the SEPA payment method, a reusable SEPA payment method is created; for any other reusable payment method, the payment method is converted and stored as a reusable SEPA payment method. So from our perspective, we will need two types of WC Tokens: card tokens (which exist by default) and SEPA tokens, which exists here and, I believe, was used in #1540.

The catch here is that for this third category of payment method types (reusable, yet non-card and non-SEPA), after Stripe has converted these payment methods to reusable payment methods, from Stripe's perspective they are now fully-fledged SEPA payment methods and their payment method type is now SEPA--rather than the original payment method type. This means that when we make requests to Stripe for saved SEPA payment methods, the response will include these converted payment methods. This is a problem, because we would like to keep these saved payment methods separate--for example, only returning originally SEPA payment methods to the SEPA gateway and originally Bancontact payment methods (even though they will be SEPA tokens) to the Bancontact payment gateway.

This is a solvable problem, even though the solution is a little unwieldy. Stripe does leave a few clues as to the original payment method type, but unfortunately they are a little buried in the response. I would encourage you to check out how we implemented this functionality on the Stripe gateway--specifically this function--because we managed to figure out how to handle most of these unfortunate oddities. Essentially, there are expandable fields on the API response that we can use to find the original payment method type (more information from Stripe here on expanding fields in their API response). We also add some meta onto our saved WC SEPA Tokens, so that we can reverse this process more easily.

There's a lot of information in this comment and there's probably a fair bit that I haven't included, so I'd highly recommend following the implementation on the Stripe Gateway--using your own judgement to take what's required--as a reference to help you resolving this issue. Let me know if there's anything else in between that I might be able to help flesh out in the meantime. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: core WC Payments core related issues, where it’s obvious. component: upe priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants