openfoodnetwork
openfoodnetwork copied to clipboard
[BUU] Edits on new product page are leaving another product with an empty price
Description
Better check the step to reproduce to understand
Steps to Reproduce
Under admin_style_v3 feature toggle
- Go to staging FR and set your user as a manager of Release Farm
- Go to Products v3 page
- Try changing something like removing a "y" in Fryys
- Hit the save button
- 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
After saving
Severity
bug-s3: a feature is broken but there is a workaround
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
- 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. - Hmm, it should be easy to do on the server side in Rails, we can just ignore each record unless
product
/variant.changed?
(seeproduct_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 for this one, I guess I need to retest once SR is removed from the page, correct?
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.
I can't reproduce this one anymore. I'm closing it, let's reopen if we find out how to reproduce :+1: