Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve order edit page in data inconsistency scenario (follow up from S2 #4186) #5350

Merged
merged 1 commit into from
May 6, 2020

Conversation

luisramos0
Copy link
Contributor

@luisramos0 luisramos0 commented May 3, 2020

What? Why?

This is a follow up from #5253 that improves the situation for #4186
This is handling a data inconsistency problem that is not critical therefore I am not writing specs for this. The next step is to investigate the root cause, but that's an S3 now so we leave it for later.

In this PR I am simply fixing my code in #5253, break is aborting the cycle through units, if one unit is broken (no line item found) we can still iterate through the other units so that the user sees the remaining items in the order instead of a empty order.
See Filipe's testing comments in #5253 for more details.

What should we test?

Have a look at the order from #5253
https://staging.openfoodfrance.org/admin/orders/R656741034/edit
No line items before this PR, one line item after this PR.

And/or repeat tests of #5253 and make sure the remaining line item (not the broken one) is now displayed.

Release notes

Changelog Category: Changed
Improved the handling of a data inconsistent scenario in the orders edit page.

What we would need would be a continue, not break, so that other units will be processed without halting the cycle through the units
@luisramos0 luisramos0 self-assigned this May 3, 2020
@luisramos0 luisramos0 changed the title Replace break with if Improve order edit page in data inconsistency scenario (follow up from S2 #4186) May 3, 2020
Copy link
Contributor

@sauloperez sauloperez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@filipefurtad0 filipefurtad0 self-assigned this May 6, 2020
@filipefurtad0 filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label May 6, 2020
@filipefurtad0
Copy link
Contributor

Hi @luisramos0 ,
Line item from the order R656741034 is now showing up under /admin/orders/R365651865/edit, there it is:
image

I repeated the process, creating a similar 2 variant order but deleted a different variant this time by replacing max(id) with the id of the corresponding line item (in this case, spinach). This works as expected in the terminal:

  • before deletion, both variants appear, in the terminal and in the app
  • deleted line item with id=377, via terminal
  • checked that there was one line item left, via terminal
  • in the frontend, editing the respective order R365651865 now shows the remaining item (lentils):

image

Interestingly, after the line item deletion, the first time you visit https://staging.openfoodfrance.org/admin/orders/R365651865/edit you see a price discrepancy (in the pic above you see 31.9 Eur, but should be 16 Eur.), the order is displaying the previous total. But I don't think this is an issue for now, as refreshing the page or Pressing Update and Recalculate Fees solves this, and displays "credit owed" to the customer - so it's quite solid! 🎉

Good to go!

@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label May 6, 2020
@luisramos0 luisramos0 merged commit d1392d4 into openfoodfoundation:master May 6, 2020
@luisramos0 luisramos0 deleted the edit_order_snail branch May 6, 2020 19:33
@luisramos0
Copy link
Contributor Author

great testing Filipe, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants