woocommerce / woocommerce-gateway-stripe

The official Stripe Payment Gateway for WooCommerce
https://wordpress.org/plugins/woocommerce-gateway-stripe/
236 stars 206 forks source link

Update PRB payment method generation to use PaymentMethods rather than sources #3510

Closed james-allan closed 1 month ago

james-allan commented 1 month ago

Fixes #3506

Changes proposed in this Pull Request:

Testing instructions

GENERAL TESTING

  1. Make sure the ECE feature flag isn't set.
  2. Enable the express payment buttons.
  3. Complete purchases using PRBs on the product, cart and checkout pages.
  4. Confirm that payments are completed as expected.

SUBSCRIPTIONS FREE TRIAL

  1. Create a virtual subscription product with a free trial (screenshot)
  2. Purchase the product using a PRB.
  3. On develop there are 4 things to notice.
    1. The resulting payment method will be a source (src_).
    2. There will be set up intent request with a failure with the following error message.
    3. The src_ won't be attached to the customer.
    4. There is no payment method token saved in the WooCommerce store. Note that the source when it was created is reusable and in my testing it does work for future renewal payments.
  4. On this branch the resulting payment method will be a pm_ and will be attached to the customer.
  5. Because the PM is attached to the customer, there is a saved token.
Before After
Screenshot 2024-10-14 at 10 02 09 am Screenshot 2024-10-14 at 10 02 35 amScreenshot 2024-10-14 at 10 02 48 am
  1. Process a renewal. Note that it is successful.

Post merge

annemirasol commented 1 month ago

Nice work, James ✨ Was able to reproduce the issue, and verify that the PR fixes it.

❓ When Legacy Checkout is enabled, and I try to use PRB on checkout, I get this generic error: Payment processing failed. Please retry.. Is that because we're removing the source event handler for paymentRequest? Should we be hiding PRBs if Legacy is enabled, then?

james-allan commented 1 month ago

When Legacy Checkout is enabled, and I try to use PRB on checkout

Oh good catch. Legacy Checkout wasn't something I had considered. Let me take a look at whether I can still honor the legacy checkout request format.

james-allan commented 1 month ago

Ok thanks again for the review @annemirasol. I've pushed b970ed4 which brings back the stripe_source key which the legacy checkout's process_payment() function expects/requires to process the payment successfully.

There's a little bit of redundancy in this approach passing both stripe_source and wc-stripe-payment-method with the same pm_ ID just so both checkout integrations have their expected args. I'm curious if you think there's a better approach. :)

annemirasol commented 1 month ago

There's a little bit of redundancy in this approach passing both stripe_source and wc-stripe-payment-method with the same pm_ ID just so both checkout integrations have their expected args. I'm curious if you think there's a better approach. :)

I suppose we could add some logic to decide which arg to pass depending on which checkout mode is enabled, but I'm okay with just passing both. I like this compact approach, and we can clean up later when we drop support for legacy checkout.