Fix for advancing carts correctly in admin checkout #4253
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR adds a minor fix for cart behavior in Admin. When updating the customer details, the current code only calls `next`. This will move th order *one* state down the checkout flow - so, for instance, from `address` to `delivery`. But if the order could move further, it won't. And this can leave orders in a slightly unexpected state. As an example, in our store, when the order is in the `delivery` state, there is no admin UI element to cause it to move into the `payment` state, short of selecting a different shipping method or adding a new item to the cart. And since for us, taxes aren't calculated until the order reaches the `payment` state, we can't render the correct cost in the payment detail admin page to allow our admins to enter the correct payment. This fix is rather simple - rather than moving only *one* state, move as far as possible via `advance`. This essentially preserves the same behavior as before if the order could not advance, but allows the order to move forward into payment if the shipments all have shipping rates selected. By default, the default estimator should be selecting a shipping rate for every shipment when the shipments are proposed. I tested this in my store with shipments which had multiple shipment rates, and observed no problem advancing to the `payment` state. We could potentially make this opt-in if desired, but that felt like slight overkill.
|
It seems like the remaining failure might be a flake of some sort - seems unlikely my change in a controller broke a core spec. Is anyone able to re-run that failing test? |
|
It seems like the tests are green - anyone able to review? |
|
Seems legit, but I don't have enough background to confirm we aren't missing something. Thoughts, @kennyadsl? |
kennyadsl
approved these changes
Feb 28, 2022
waiting-for-dev
approved these changes
Mar 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR adds a minor fix for cart behavior in Admin. When updating the customer details, the current code only calls
next. This will move th order one state down the checkout flow - so, for instance, fromaddresstodelivery. But if the order could move further, it won't. And this can leave orders in a slightly unexpected state.As an example, in our store, when the order is in the
deliverystate, there is no admin UI element to cause it to move into thepaymentstate, short of selecting a different shipping method or adding a new item to the cart. And since for us, taxes aren't calculated until the order reaches thepaymentstate, we can't render the correct cost in the payment detail admin page to allow our admins to enter the correct payment.This fix is rather simple - rather than moving only one state, move as far as possible via
advance. This essentially preserves the same behavior as before if the order could not advance, but allows the order to move forward into payment if the shipments all have shipping rates selected. By default, the default estimator should be selecting a shipping rate for every shipment when the shipments are proposed.I tested this in my store with shipments which had multiple shipment rates, and observed no problem advancing to the
paymentstate. We could potentially make this opt-in if desired, but that felt like slight overkill.Checklist: