openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.11k stars 718 forks source link

[Paypal & Stripe] Failed checkout attempts add a persistent transaction fee #9112

Closed filipefurtad0 closed 2 years ago

filipefurtad0 commented 2 years ago

Description

If a Paypal transaction fails on checkout and a transaction fee is associated to that payment, the respective amount will be added to that order, as many times as checkout attempts (with Paypal).

This leads to an inconsistent state of the order / fail cascade, in which:

https://app.bugsnag.com/yaycode/openfoodnetwork-uk/errors/61ebce343d0f08000872149b?event_id=625e8a3d0093606c2ee10000&i=sk&m=nw

Expected Behavior

Failed checkout attempts using Paypal should not add a persistent transaction fee.

Actual Behaviour

Failed checkout attempts using Paypal should not add a persistent transaction fee.

Steps to Reproduce

As Admin:

  1. Set up a transaction fee on a Paypal payment method
  2. Edit the Paypal settings: for a valid Signature add an empty space after it :eyes:

As Customer:

  1. Checkout on that shop, selecting that payment method
  2. Notice the error on checkout
  3. Check your cart, and notice the transaction fee was added
  4. Empty your cart, and start over from step 3.
  5. Notice the previously added transaction fee is still on the cart: transaction fees add up... 8.1 Complete this funky order using Stripe -> :boom: -> Cart should be reset. 8.2 8.1 Complete this funky order using Cash -> Order is completed with the ghost transaction fees included in the order total :warning:

Animated Gif/Screenshot

Peek 2022-04-19 11-06

Workaround

There are some ways to deal with the bug, but it can go unnoticed. I'd say this is an S2 but feel free to change if you disagree.

Severity

bug-s2: a non-critical feature is broken, no workaround

Your Environment

Possible Fix

RachL commented 2 years ago

@filipefurtad0 this is reproductible with the new checkout as well? If not I would recommend switching these users to split checkout and not deal with this bug apart from trying to automate the test.

filipefurtad0 commented 2 years ago

That's a good point Rachel :eyes:

But unfortunately, it breaks anyway in different way though. With split checkout enabled, starting on step 6 (above), leads to error 500:

Peek 2022-04-20 10-47

For the dev picking this up, the error 500 appears on the server log:


I, [2022-04-20 09:47:48#21788]  INFO -- : Started GET "/checkout/summary" for 2.82.201.180 at 2022-04-20 09:47:48 +0000
I, [2022-04-20 09:47:48#21788]  INFO -- : Processing by SplitCheckoutController#edit as HTML
I, [2022-04-20 09:47:48#21788]  INFO -- :   Parameters: {"step"=>"summary"}
I, [2022-04-20 09:47:48#21788]  INFO -- :   Rendered split_checkout/edit.html.haml within layouts/darkswarm (Duration: 18.8ms | Allocations: 6603)
I, [2022-04-20 09:47:48#21788]  INFO -- :   Rendered layout layouts/darkswarm.html.haml (Duration: 19.0ms | Allocations: 6645)
I, [2022-04-20 09:47:48#21788]  INFO -- : Completed 500 Internal Server Error in 79ms (ActiveRecord: 22.5ms | Allocations: 24280)
I, [2022-04-20 09:47:48#21788]  INFO -- [Bugsnag]: Notifying https://notify.bugsnag.com of NoMethodError
F, [2022-04-20 09:47:48#21788] FATAL -- :   
ActionView::Template::Error (undefined method `name' for nil:NilClass):
    45:       %span.summary-label
    46:         = t("split_checkout.step1.delivery_address.title")
    47:       %span.summary-value
    48:         = @order.shipping_method.name
    49:       %div
    50:       = @order.shipping_method.description

So, from the /shop page, and after emptying the cart the shopper is being redirected to the /checkout/summary page - this is at least what I gather from the log above. At that stage, shipping address is not set, so this line fails: 48: = @order.shipping_method.name

(I think a similar redirection inconsistency might be happening in issue is happening in issue #9056 - but I'm not sure)

jibees commented 2 years ago

About the last comment, and the error that you saw @filipefurtad0 : i'm pretty sure I've already seen it but I've not Paypal activated on my local setup (only 'cash on collection' and 'stripe')...

filipefurtad0 commented 2 years ago

Great, thanks for that feedback JB. It doesn't look like it relates to payment methods but rather to addresses and/or shipping methods...

filipefurtad0 commented 2 years ago

Following this detailed description - thanks @RonellaG - the bug is reproducible as well when checking out with Stripe. For instance, using:

Updating the issue name.

filipefurtad0 commented 2 years ago

Hey @georgethoppil ,

While not preventing the creation of the ghost transaction fees, this comments seems related, as also here on the /cart page, the user can't seem to re-start an order: https://github.com/openfoodfoundation/openfoodnetwork/issues/9131#issuecomment-1121726323