openfoodnetwork icon indicating copy to clipboard operation
openfoodnetwork copied to clipboard

[BUU] Edits on new product page are leaving another product with an empty price

Open RachL opened this issue 1 year ago • 1 comments

Description

Better check the step to reproduce to understand

Steps to Reproduce

Under admin_style_v3 feature toggle

  1. Go to staging FR and set your user as a manager of Release Farm
  2. Go to Products v3 page
  3. Try changing something like removing a "y" in Fryys
  4. Hit the save button
  5. See an error message gets triggered on the large pizza about an empty price, also you didn't change the large pizza during your setting

Animated Gif/Screenshot

Before saving

image

After saving

image

Severity

bug-s3: a feature is broken but there is a workaround

RachL avatar Oct 06 '23 12:10 RachL

This is due to the variant being invalid, a problem which would appear if you had updated that row, or edit it in the variant screen. I wasn't able to replicate this on staging because I couldn't find any variants with a blank price anymore, but I think it would still be an issue.

But you're right that it's strange to be updating a row that hasn't been changed. I had hoped to filter and ignore unchanged rows on the client side (like we do with angular), but I can't find a way to do it with StimulusReflex, because its form serialisation method isn't customisable . It's a real shame because the user interface makes it look like it's doing that (with the orange borders). Sometimes I'd like to switch to using a rails standard form submit (probably with ujs), but I'm not sure if it's possible to customise serialisation in that case either. One way to achieve it would be to alter the inputs, eg make them all disabled upon submit.

Estimate

  1. Add ~~beforeBulkUpdate~~ JS to disable any un-changed elements. Ensure disabled elements look the same when being submitted. And they should be re-enabled if the submit is cancelled/failed for any reason.
  2. Hmm, it should be easy to do on the server side in Rails, we can just ignore each record unless product/variant.changed? (see product_set.rb)

Each are approx 0.5 days For efficiency, we could do both, but opt 2 would be quickest and solve this problem on its own.

dacook avatar Feb 20 '24 01:02 dacook

@dacook for this one, I guess I need to retest once SR is removed from the page, correct?

RachL avatar Apr 08 '24 10:04 RachL

Actually that probably won't make a difference, so I wouldn't bother retesting.

I've updated the estimate section, to make the solution clearer.

dacook avatar Apr 10 '24 06:04 dacook

I can't reproduce this one anymore. I'm closing it, let's reopen if we find out how to reproduce :+1:

RachL avatar Apr 30 '24 10:04 RachL