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

Split checkout: Display "Update" button on the order table page unless order is complete #8944

Conversation

jibees
Copy link
Contributor

@jibees jibees commented Feb 28, 2022

What? Why?

With the split checkout, we can access the /cart between the different steps: for this cases, cart actions should be displayed (and not only for the cart state)
Closes #8940

What should we test?

With split_checkout feature toggle activated:
  • Proceed to checkout with a product that on hand
  • As an admin, set the stock of the product to 0
  • Between each steps, you should be redirected to the /cart page with an "Update" button
With legacy checkout:
  • Proceed to checkout with a product that on hand
  • As an admin, set the stock of the product to 0
  • Fill the form, and click on the submit button
  • you should be redirected to the /cart page with an "Update" button

This modification comes from an issue around the split checkout, but could impact the legacy checkout system: ie. the code is shared between split/legacy checkout.
I've tested this page for both split and legacy checkout and I didn't find any issue, but maybe I've missed some place where this page/table is used.

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

@jibees jibees self-assigned this Feb 28, 2022
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

No spec?

@jibees
Copy link
Contributor Author

jibees commented Mar 1, 2022

No spec?

You're right, this needs some systems specs.
Back to In dev

With the split checkout, we can access this page between the different steps: for this cases, cart actions should be displayed
@jibees jibees force-pushed the 8940-display-update-button-on-order-unless-order-is-complete branch 2 times, most recently from d180593 to 0fe1d6f Compare March 1, 2022 13:47
@jibees jibees force-pushed the 8940-display-update-button-on-order-unless-order-is-complete branch from 0fe1d6f to e14dad1 Compare March 1, 2022 13:51
@jibees
Copy link
Contributor Author

jibees commented Mar 1, 2022

No spec?

Already done by @filipefurtad0 : thanks! 🙏

Back in Code Review

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

👍

@filipefurtad0 filipefurtad0 self-assigned this Mar 7, 2022
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Mar 7, 2022
@filipefurtad0
Copy link
Contributor

filipefurtad0 commented Mar 7, 2022

Hey @jibees ,

I think we need another look at this one.

I've started the test with no issues: restricting the stock on /details and /payment steps redirected to /cart and displayed the "Details" button.

After that, the checkout pages broke, for this shop (even after emptying the cart and starting over the checkout process) and for other shops too:

Peek 2022-03-07 09-45

The reason seems to be that the redirection is pointing to the /summary step, even when changing shops for that user. Logging out and again with another user was fine, i.e., checkout was possible.

Also, a bugsnag got triggered, during these tests. But I think for a different issue, here.

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Mar 7, 2022
@filipefurtad0
Copy link
Contributor

I'll run some more tests around these scenarios in another server, as double check. Maybe it all relates to that UI issue, and this is good to merge 🤞

@filipefurtad0 filipefurtad0 added the pr-staged-fr staging.coopcircuits.fr label Mar 7, 2022
@jibees
Copy link
Contributor Author

jibees commented Mar 7, 2022

Thanks @filipefurtad0
We definitely needs to find a way to reproduce this bug, as it seems to be very constrained.

@filipefurtad0 filipefurtad0 removed the pr-staged-fr staging.coopcircuits.fr label Mar 7, 2022
@filipefurtad0
Copy link
Contributor

Alright - other that #8963 which for some reason caused this persistent redirect to the /summary page, for that shop in staging-UK - I've found no other issues here.

I've checked both scenarios mentioned, can't think of anywhere else to check - I'd say we're good 👍

This closes the underlying issue - merging! 🎉

@filipefurtad0 filipefurtad0 merged commit 5606be7 into openfoodfoundation:master Mar 7, 2022
jibees added a commit to jibees/openfoodnetwork that referenced this pull request Apr 21, 2022
Split_checkout introduced new state for an order

Update specs as well

Follow up openfoodfoundation#8944
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.

[Split Checkout] Cart page not displaying "Update" button
4 participants