[Split Checkout] Failed StripeSCA payments redirect to the Details step

[filipefurtad0]

Description

If a payment fails - for example, due to insufficient funds - then the customer is redirected to the Details step. This forces the user to re-select a shipping method.

Expected Behavior

If a payment fails - for example, due to insufficient funds - then the customer should be redirected to the Payment step.

Actual Behaviour

If a payment fails - for example, due to insufficient funds - then the customer is redirected to the Details step.

Steps to Reproduce

1. Test checking out with a the Stripe card x-3178
2. Grant authorization, when prompted to.
3. Notice the payment fails (insufficient_funds), but notice that you're redirected into the Details step

Animated Gif/Screenshot

[RachL]

Nice catch @filipefurtad0 ! I think this is not bloking the rollout. We can fix it even after complete release :+1:

[filipefurtad0]

Agree @RachL :+1:

To the dev picking this up:

I investigated briefly to try and get a spec for this. If I understand correctly, we already have a case for this in the code; for some reason, it's not doing what it's supposed to: (from app/controllers/split_checkout_controller.rb)

  rescue Spree::Core::GatewayError => e
    flash[:error] = I18n.t(:spree_gateway_error_flash_for_checkout, error: e.message)
    @order.update_column(:state, "payment")
    render operations: cable_car.redirect_to(url: checkout_step_path(:payment))
  end

We're getting this response, when the payment fails: (retrieved from staging-FR, by tailing the log)

I, [2022-09-12 16:22:25#1184]  INFO -- : source=rack-timeout id=ba0adf49-32ce-400f-8562-d08bdb084724 wait=2ms timeout=120000ms state=ready
I, [2022-09-12 16:22:25#1184]  INFO -- : Started PUT "/checkout/summary" for 148.69.190.53 at 2022-09-12 16:22:25 +0000
I, [2022-09-12 16:22:25#1184]  INFO -- : Processing by SplitCheckoutController#update as CABLE_READY
I, [2022-09-12 16:22:25#1184]  INFO -- :   Parameters: {"authenticity_token"=>"Xml4FjB5UAxTWca_827KddLv3zlnKJEyfIege9_dgvfPHBhJCitL8y-l5-pRRnNHlQ4OMwrpBIFiWAJEi5Ufyg", "accept_terms"=>"1", "confirm_order"=>"Valider ma commande", "step"=>"summary"}
I, [2022-09-12 16:22:27#1184]  INFO -- : Completed 200 OK in 2305ms (Views: 0.7ms | ActiveRecord: 25.1ms | Allocations: 39071)
I, [2022-09-12 16:22:27#1184]  INFO -- : source=rack-timeout id=ba0adf49-32ce-400f-8562-d08bdb084724 wait=2ms timeout=120000ms service=2317ms state=completed
I, [2022-09-12 16:22:36#1184]  INFO -- : source=rack-timeout id=a63103c1-7fc7-4294-92fa-c60fc4307165 wait=2ms timeout=119998ms state=ready
I, [2022-09-12 16:22:36#1184]  INFO -- : Started GET "/payment_gateways/stripe/confirm?payment_intent=pi_3LhFOwKuuB1fWySn22VuEKNh&payment_intent_client_secret=pi_3LhFOwKuuB1fWySn22VuEKNh_secret_s8Lghlg55019Fke9UC6djFWWe&source_redirect_slug=test_YWNjdF8xRmlxRXNLdXVCMWZXeVNuLF9NUTVhc29va05YcENTQlpCdzVnVnVQeUViMXFMMXpr0100eYkyAyMS&source_type=card" for 148.69.190.53 at 2022-09-12 16:22:36 +0000
I, [2022-09-12 16:22:36#1184]  INFO -- : Processing by PaymentGateways::StripeController#confirm as HTML
I, [2022-09-12 16:22:36#1184]  INFO -- :   Parameters: {"payment_intent"=>"pi_3LhFOwKuuB1fWySn22VuEKNh", "payment_intent_client_secret"=>"pi_3LhFOwKuuB1fWySn22VuEKNh_secret_s8Lghlg55019Fke9UC6djFWWe", "source_redirect_slug"=>"test_YWNjdF8xRmlxRXNLdXVCMWZXeVNuLF9NUTVhc29va05YcENTQlpCdzVnVnVQeUViMXFMMXpr0100eYkyAyMS", "source_type"=>"card"}
E, [2022-09-12 16:22:36#1184] ERROR -- : Le paiement a échoué
E, [2022-09-12 16:22:36#1184] ERROR -- :   --- !ruby/object:ActiveMerchant::Billing::Response
params: {}
message: Your card has insufficient funds.
success: false
test: false
authorization:
fraud_review:
error_code:
emv_authorization:
network_transaction_id:
avs_result:
  code:
  message:
  street_match:
  postal_match:
cvv_result:
  code:
  message:

W, [2022-09-12 16:22:36#1184]  WARN -- [Bugsnag]: Adding metadata/severity using a hash is no longer supported, please use block syntax instead
I, [2022-09-12 16:22:36#1184]  INFO -- [Bugsnag]: Notifying https://notify.bugsnag.com of RuntimeError
E, [2022-09-12 16:22:36#1184] ERROR -- : Redirected by /home/openfoodnetwork/apps/openfoodnetwork/releases-old/2022-09-09-152126/app/controllers/concerns/order_completion.rb:62:in `process_payment_completion!'
I, [2022-09-12 16:22:36#1184]  INFO -- : Redirected to https://staging.coopcircuits.fr/checkout
I, [2022-09-12 16:22:36#1184]  INFO -- : Completed 302 Found in 819ms (ActiveRecord: 86.7ms | Allocations: 104467)
I, [2022-09-12 16:22:36#1184]  INFO -- : source=rack-timeout id=a63103c1-7fc7-4294-92fa-c60fc4307165 wait=2ms timeout=119998ms service=828ms state=completed
I, [2022-09-12 16:22:37#467]  INFO -- : source=rack-timeout id=20d62df9-c4dc-49da-8f4b-c04999aaf05b wait=2ms timeout=119998ms state=ready
I, [2022-09-12 16:22:37#467]  INFO -- : Started GET "/checkout" for 148.69.190.53 at 2022-09-12 16:22:37 +0000
I, [2022-09-12 16:22:37#467]  INFO -- : Processing by SplitCheckoutController#edit as HTML
E, [2022-09-12 16:22:37#467] ERROR -- : Redirected by /home/openfoodnetwork/apps/openfoodnetwork/releases-old/2022-09-09-152126/app/controllers/split_checkout_controller.rb:131:in `redirect_to_step_based_on_order'
I, [2022-09-12 16:22:37#467]  INFO -- : Redirected to https://staging.coopcircuits.fr/checkout/details
I, [2022-09-12 16:22:37#467]  INFO -- : Completed 302 Found in 73ms (ActiveRecord: 24.8ms | Allocations: 16861)
I, [2022-09-12 16:22:37#467]  INFO -- : source=rack-timeout id=20d62df9-c4dc-49da-8f4b-c04999aaf05b wait=2ms timeout=119998ms service=85ms state=completed
I, [2022-09-12 16:22:37#467]  INFO -- : source=rack-timeout id=c6a8ff0a-a38d-4610-a165-aaafb354c8fa wait=1ms timeout=119999ms state=ready
I, [2022-09-12 16:22:37#467]  INFO -- : Started GET "/checkout/details" for 148.69.190.53 at 2022-09-12 16:22:37 +0000
I, [2022-09-12 16:22:37#467]  INFO -- : Processing by SplitCheckoutController#edit as HTML
I, [2022-09-12 16:22:37#467]  INFO -- :   Parameters: {"step"=>"details"}
I, [2022-09-12 16:22:37#467]  INFO -- :   Rendered split_checkout/edit.html.haml within layouts/darkswarm (Duration: 249.0ms | Allocations: 186754)
I, [2022-09-12 16:22:38#467]  INFO -- :   Rendered layout layouts/darkswarm.html.haml (Duration: 1234.9ms | Allocations: 638563)
I, [2022-09-12 16:22:38#467]  INFO -- : Completed 200 OK in 1300ms (Views: 1172.5ms | ActiveRecord: 81.1ms | Allocations: 657324)
I, [2022-09-12 16:22:38#467]  INFO -- : source=rack-timeout id=c6a8ff0a-a38d-4610-a165-aaafb354c8fa wait=1ms timeout=119999ms service=1312ms state=completed

[filipefurtad0]

This seems to always fail for 3D-cards, like x-3178 -> pm_card_authenticationRequiredChargeDeclinedInsufficientFunds code:

• regardless of saving the card, the redirection occurs to the Details step :x:

For non-3D cards, like x-0002 -> card_declined code:

• if the checkbox Save card for future use is not ticked -> redirection occurs to the payment page :heavy_check_mark:
• if the checkbox Save card for future use IS ticked -> redirection occurs to the payment page :x:

This feels related to #9573

[filipefurtad0]

I've just noticed this one again:

Surely not a roll-out blocker, but I think it's confusing to the user and perhaps not great as a checkout experience. Should we prioritize fixing this one? Ping @openfoodfoundation/train-drivers-product-owners

[RachL]

@filipefurtad0 we can bump this as s3 to be picked up by instances in the next round of s3 selection, but other than that the top priority is #10658 and then #10636 .

My reasoning is that if this is not a blocker for the roll out, then the roll out should happen first.

[filipefurtad0]

if this is not a blocker for the roll out, then the roll out should happen first

Agree @RachL :+1:

[abdellani]

Hi @filipefurtad0 , @RachL

In case of order processing failure:

1. the order state machine moves back to ~~card~~ cart.
2. the user's choices of shipping method will be removed.
3. the user's choices of payment method will be removed.
4. the user will be redirected to '/checkout' URL
5. as the '/checkout' under split checkout will redirect to '/checkout/details'

Fixing the redirect link by replacing '/checkout' by '/checkout/payment' will not fix the issue because the order is reset (step 2 & 3). Now, here's my question: is it safe to not reset an order if the processing fails?

---

Code reference: https://github.com/openfoodfoundation/openfoodnetwork/blob/668a573f12f5aa5d895a81dbfd22568e9e6efd57/app/controllers/concerns/order_completion.rb#L85-L91

https://github.com/openfoodfoundation/openfoodnetwork/blob/668a573f12f5aa5d895a81dbfd22568e9e6efd57/app/services/checkout/post_checkout_actions.rb#L15-L18

https://github.com/openfoodfoundation/openfoodnetwork/blob/668a573f12f5aa5d895a81dbfd22568e9e6efd57/app/services/order_checkout_restart.rb#L4-L17

[lin-d-hop]

This doesn't seem ideal. It would make most logical sense to me to redirect to Step 2 on card failure (with error message of course).

I think @Matt-Yorkley will have good insights here.

What does the order state card actually reflect? It would make sense to me if that state was the state matched Stage 2 of the checkout flow. So the shopper and shipping details have been entered. In this case reverting to state card would keep stage one information. But if state card has a different meaning then obvs that won't work.

Sorry but I need more information to be useful on this question.

[lin-d-hop]

If the order is totally reset does that mean we lose information about the failed payment? Or does the information get kept and linked to the new order? Users and hubs should be able to see information on failed payments.

[Matt-Yorkley]

Notes:

• I think card was a typo, it gets reset to cart state.
• I don't think failed payments get deleted :ok_hand:
• This resetting logic is really a hangover from the way the old checkout worked and I think we can remove it now, so yes I would drop that line 90 Checkout::PostCheckoutActions.new(@order).failure from the OrderCompletion concern and change the order_failed_route to return whatever is decided as the new preferred route for failed payment processing (/checkout/payment or checkout/summary) :+1:

[abdellani]

I think card was a typo, it gets reset to cart state.

Yes, you're right. Sorry, my mistake.

I don't think failed payments get deleted ok_hand

I double-checked, and yes you're right again :+1:

for some reason, when I use rails c to load the payments related to the in-progress order, I get an error (but anyway this not the scope of this issue)

Spree::Order.last.payments
  Spree::Order Load (1.8ms)  SELECT "spree_orders".* FROM "spree_orders" ORDER BY "spree_orders"."id" DESC LIMIT $1  [["LIMIT", 1]]
/.../.rvm/gems/ruby-3.0.3/gems/activerecord-7.0.4.3/lib/active_record/reflection.rb:441:in `rescue in compute_class': Rails couldn't find a valid model for Payment association. Please provide the :class_name option on the association declaration. If :class_name is already provided, make sure it's an ActiveRecord::Base subclass. (NameError)

but when I tried SQL, I was able to list all the payments.

 id | amount | order_id |         created_at         |         updated_at         | source_id |    source_type    | payment_method_id |         state          |        response_code        | avs_response | identifier | cvv_response_code |                                                                                                                cvv_response_message                                                                                                                 | captured_at 
----+--------+----------+----------------------------+----------------------------+-----------+-------------------+-------------------+------------------------+-----------------------------+--------------+------------+-------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+-------------
 14 |  11.10 |        6 | 2023-05-30 10:20:13.882907 | 2023-05-30 10:20:34.772017 |         9 | Spree::CreditCard |                 2 | failed                 | pi_3NDPf8IwntZ5KyzN2C0BJuTY |              | TZHPM54D   |                   | https://hooks.stripe.com/redirect/authenticate/src_1NDPf8IwntZ5KyzNXfNu565h?client_secret=src_client_secret_A1WnJrQGSSfUy7CYx66A5vBV&source_redirect_slug=test_YWNjdF8xTUFCRDlJd250WjVLeXpOLF9Oek9TekNEd0lNMEhvcVd2R0FjazM5bE52dmV6Qlkx0100s46cuYxy | 
 24 |  11.10 |        6 | 2023-05-31 19:02:25.85444  | 2023-05-31 19:02:52.26372  |        19 | Spree::CreditCard |                 2 | requires_authorization | pi_3NDuIJIwntZ5KyzN29XvqBcV |              | D3XJ7JHF   |                   | https://hooks.stripe.com/redirect/authenticate/src_1NDuIJIwntZ5KyzNxRAUv264?client_secret=src_client_secret_Iaw6h45u1MG3AdRiRR0TUDSJ&source_redirect_slug=test_YWNjdF8xTUFCRDlJd250WjVLeXpOLF9OenU2MU9ETGN6YkJtd05lRmg3bVRzbW5qczFlUzJB0100XGU8NLND | 

This resetting logic is really a hangover from the way the old checkout worked and I think we can remove it now, so yes I would drop that line 90 Checkout::PostCheckoutActions.new(@order).failure from the OrderCompletion concern and change the order_failed_route to return whatever is decided as the new preferred route for failed payment processing (/checkout/payment or checkout/summary) +1

I already implemented and tested this solution, but I was redirected to details (I think what lead me to think that the failed payments were removed)

Now, I believe the issue is on the split_checkout controller. https://github.com/openfoodfoundation/openfoodnetwork/blob/5f429c77f8a14c00dc2b773d731418e15a8bcf79/app/controllers/split_checkout_controller.rb#L24-L30

the order is reset to cart, so the user will be redirect back to details after being redirect to payment https://github.com/openfoodfoundation/openfoodnetwork/blob/5f429c77f8a14c00dc2b773d731418e15a8bcf79/app/controllers/split_checkout_controller.rb#L303-L310

[Matt-Yorkley]

the order is reset to cart, so the user will be redirect back to details after being redirect to payment

Yes, but if we drop that line in OrderCompletion that calls Checkout::PostCheckoutActions#failure, then the order's state won't be reset to cart, right? So that redirect to /details won't happen?

[abdellani]

the order is reset to cart, so the user will be redirected back to details after being redirected to payment

Yes, but if we drop that line in OrderCompletion that calls Checkout::PostCheckoutActions#failure, then the order's state won't be reset to cart, right? So that redirect to /details won't happen?

Yes, I tested it and I can confirm that it works. In this case, I'll also remove Checkout::PostCheckoutActions#failure. This method is not used anywhere else.