Openfoodnetwork: Cannot edit cart - snail

[tschumilas]

Description

Editing quantity in cart is producing a snail on OFN_CAN. But if I use the browser back arrow at the snail to go back to the cart - the cart edit is made.

Expected Behavior

No snail

Actual Behaviour

Snail on cart edit - but if you use the browzer arrow to go back - the change is there

Steps to Reproduce

1.go to a shop on OFN-CAN
2.put an item in your cart and try to edit

1. get the snail and use your browzer arrows to go back
2. see the change is made

Animated Gif/Screenshot

Workaround

work around is using the browzer back arrow - but will users know to do this?

Severity

bug-s1

bug-s1: a critical feature is broken: checkout, payments, signup, login
bug-s2: a non-critical feature is broken, no workaround
bug-s3: a feature is broken but there is a workaround
bug-s4: it's annoying, but you can use it
bug-s5: we can live with it, only a few users impacted

https://github.com/openfoodfoundation/openfoodnetwork/wiki/Bug-severity
-->

Your Environment

• Version used:
• Browser name and version:
• Operating System and version (desktop or mobile):

Possible Fix

[lin-d-hop]

This is also happening on UK prod.

Seems like it happens when you click:
'Update'
The garbage symbol

Not happening when:
You edit the qty field
You click 'Empty cart'

[RachL]

FYI I thought we had an automated test for this. I don't think we manually test it anymore during release testing.

[luisramos0]

hello!

Id say this is an S2, S1 usually is something that makes you make a phone call, in this case on Saturday.

I can see this only when I click on update in the cart.

I tried this on ofn uk on banc organics and it's working fine. I am not sure why yet. (EDITED)

this is the bugsnag alert.

NoMethodError (private method `open' called for #<Spree::Adjustment:0x005612d66a42f0>):
  app/controllers/spree/orders_controller.rb:179:in `with_open_adjustments'
  app/controllers/spree/orders_controller.rb:79:in `update

@RachL on a quick analysis I only see one place where this update button is clicked by the auto tests, but it is testing for a insufficient stock scenario, it's not validating the successful scenario:
https://github.com/openfoodfoundation/openfoodnetwork/blob/5892e85869d39d1f0dab3131f7df0a9fcd7e88f0/spec/features/consumer/shopping/cart_spec.rb#L170-L187

[luisramos0]

sorry, I forgot to mention: I can see this on ofn can. I am investigating now.

[lin-d-hop]

Thanks for the quick analysis. I do agree with the dropping of priority on this to s2. It doesn't effect every user and has a clunky workaround. If it wasn't customer facing I would say s3.

[luisramos0]

I havent managed to replicate this in the specs yet. I will try manually locally now.

Tech analysis so far:
This error looks like a method conflict. The action on Adjustments is open. Open is the example used in the guide as a potential case for conflicts (see "Method conflicts"):
https://www.rubydoc.info/github/state-machines/state_machines/StateMachines/Machine

The fix would be to replace
@order.adjustments.each(&:open)
with
@order.adjustments.each { |adjustment| adjustment.fire_events(:open) }

[luisramos0]

tricky bug...

I can see this in CA live.
I couldnt replicate locally with test data or manually SO I imported CA live data to my local environment and it doesnt happen on my computer with the same data as CA...

Then I reverted to the version running on CA live and... it's not breaking... it works.

I am now thinking that this is related to class loading issue where the order of the class loading process (which might be slightly different in production) could be introducing this problem of conflicts between method names...

[luisramos0]

the best thing to do now is to have another dev look at this analysis. I'lll ping devs on slack now.

[kristinalim]

@luisramos0 Your proposed solution (fire_events) sounds good to me - awesome finding about the StateMachine quirk.

After fixing that we should still find what's causing the problem. What is happening now (as of v2.6.6 at least) is that production and development are behaving differently, and we should at least know what is causing this and increase our confidence.

You probably checked already, but confirmed that it is the StateMachine quirk by running this in OFN Canada. Methods for all Adjustment states are declared except "open":

a = Spree::Adjustment.first
a.methods.include?(:close)
# => true
a.methods.include?(:finalize)
# => true
a.methods.include?(:open)
# => false

Other things:

• Confirmed that there are no other state machine states named open in spree_core (grep -r state | grep open)
• Confirmed that enabling cache_classes in development (v2.6.6) still defines Spree::Adjustment#open

[luisramos0]

I created #4638, we can validate it solves the problem live.
I'd say we should release the fix, validate it fixes the problem in OFN CAN and close this issue.
If it happens again later, we can come back to it and invest more time in trying to replicate it.

[luisramos0]

thanks very much @kristinalim for your detailed feedback!