Checkout block: Wrong payment method used (e.g. BACS) when shopper selects saved payment method (e.g. saved Stripe CC)

[haszari]

Describe the bug

When submitting an order using a previously-saved payment method, the order is completed using a different payment method. For example, user selects Visa ending in 4242 (saved Stripe CC), but the order is processed using BACS.

Apologies if this is tricky to reproduce – I've seen it a few times today, on different stores (ephemeral / production build, local dev), so I'm confident there's a real issue. But it may require specific setup of payment methods (or other config).

To reproduce

1. Set up checkout page using block.
2. Enable BACS and Stripe payment methods (only, though may reproduce with other combinations).
3. Complete a purchase using a new payment method, Stripe CC, check Save payment information to my account for future purchases.
4. Add something to cart, proceed to checkout.
5. Leave the default selected payment method - i.e. the first saved payment method. Submit checkout.

Expected: Stripe CC used for order.

Actual: BACS used for order.

Screenshots

Here's a gif of this happening (this was on ephemeral with production versions of everything):

Here's a pic of my setup in case something's relevant - the order may be important.

Environment

WooCommerce Blocks plugin version: 3.2 (release), 3.3.0 (release), 3.4.0-dev

[Aljullu]

I'm able to reproduce something similar. If Stripe is the first payment method in the list or the only one enabled, I get an error message:

For some reason it looks like the previously saved payment method is not correctly selected and Checkout is trying to do the purchase with a new one.

[nerrad]

When doing a report like this (which could be critical for live stores) could the report please include what version of the plugin this is happening in. If it's only on the main branch (non-deployed) code that has less severity and priority than if it's something in the wild and potentially encountered by stores right now.

For now I'm adding this to the 3.4.0 milestone as a blocker because at a minimum it should be fixed before the next blocks release. However if this is happening in 3.3.0 we may want to consider a 3.3.1 release (unless it was happening in 3.2.0 too).

[haszari]

For now I'm adding this to the 3.4.0 milestone as a blocker because at a minimum it should be fixed before the next blocks release. However if this is happening in 3.3.0 we may want to consider a 3.3.1 release (unless it was happening in 3.2.0 too).

Woops sorry I didn't include that info initially. This was first observed in 3.3.0 (released plugin) and then later in current dev build (3.4.0-dev). I've added info to description, and I'll test 3.2.0 to see if it's introduced (which could help narrow down the cause).

@nerrad I'm thinking this might be worth a 3.3.1 patch release even if it's not newly introduced (unless it's rare/hard to reproduce).

[haszari]

Reproduced this on 3.2 release plugin as well - possibly introduced in 3.2? (3.2 included reordering of payment method tabs.)

[nerrad]

With this being reproduced in 3.2 I don't think the urgency is increased given we have 3.4.0 coming up on Monday. It effectively means this has been in the wild for 4-5 weeks and we haven't had reports that I'm aware of regarding this.

[haszari]

Did some black box/binary testing of recent PRs (#2934 and #2975 from 3.2 release) that touch payment methods code, to see if I could narrow this down to a specific PR.

According to my testing, this issue was introduced in #2975 - I can reproduce in 892bbcf but not in 2c64052 (last commit before that PR merged).

So I'm investigating in that area ..

[haszari]

OK I think I'm getting close to a fix. In PaymentMethodDataContext there's an effect which ensures that there is a payment option selected - i.e. sets a default. This was firing, and selecting bacs, even if the user has a saved card.

The fix is to only set the default if it's really needed - i.e. no saved cards/payment methods. I'm opening a PR with this surgical fix, but will need some good eyes/review/testing to make sure we've covered all the scenarios. cc @Aljullu