[pull] main from solidusio:main #419
Merged
Conversation
This file contains hidden or 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
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=80&v=4){kind=small}
We don't need to be modifying the shipment state in our
OrderCancellations object. There are only two current situations where
this code will run:
1. All inventory units are shipped and one or more are cancelled.
-> In this situation, the shipment will already be in a "shipped"
state and there's no reason to change the `shipped_at` column.
2. All inventory units are cancelled.
-> We don't really want to mark shipments as "shipped" in this
situation anyway. So it's better to not do anything here.
Co-authored-by: Alistair Norman <alistair@super.gd>
Co-authored-by: Noah Silvera <noah@super.gd>
If we cancel all inventory units in a shipment, we should mark it as cancelled itself because there's nothing to ship. Co-authored-by: Alistair Norman <alistair@super.gd> Co-authored-by: Noah Silvera <noah@super.gd>
These are already overridden in all other first-level context blocks so it makes sense to move them in the single block that relies on these values.
Also, a couple of useless `let!` are removed (these records already exist).
The removed spec can be replaced by the shared example, which includes one further test so it slightly improves coverage.
This helps keeping things in sync just in the case that numbers change.
These specs don't really require a specific first level scenario, so we're first extracting them to a shared example, and with the following commit we're going to use it in other existing scenarios.
The existing scenario is reworked in order to expose an issue when moving a few items of a variant (not all of them) to a shipment with same stock location as the original one.
This is propedeutic for the following commit.
When the quantifier is initialized with a stock location or its ID, the new method returns the positive amount of stock on hand or zero, if the stock for the variant is negative. Otherwise, it returns nil.
The backordered quantity count should differ depending on whether moving to the same or a different stock location. For this reason, the way we calculate `available_quantity` changes as follows: * when the stock location differs: the stock on hand at the new shipment stock location; * when the stock location is the same: the sum of the stock on hand at the shipment stock location plus the number of on_hand inventory items from the shipment The explicit `backordered_quantity` variable is introduced to track the number of backordered items for the target shipment. The value is calculated as follows: * when the stock location differs: the quantity to be moved minus the positive available quantity at the stock location; * when the stock location is the same: the shipment total quantity for the variant minus the positive available quantity at the stock location. Also, we start the process by moving backordered items first to to make sure no pending backordered item remains. If the backordered count decreased, we're going to leave a few to be later moved and transformed to on hand, while if the backordered count increased, we are going to move also some previously on hand items.
When the new asynchronous state change logging job was introduced in \#6304, the attributes saved to `spree_state_change` records was inadvertently changed to exclude the name of the model whose state was being changed. For example, when the order updater calls `log_state_change("payment")`, the state change record would have the name "payment" on it. Since the job was introduced all of the state change records created by the order updater would have a name "order". This commit simply exposes the change in behaviour so we can understand and resolve the issue more clearly. Co-Authored-By: Adam Mueller <adam@super.gd>
Whenever `Spree::OrderUpdater#log_state_change` was called, all state change records created would have a state change name of "order". This is a change in behaviour from Solidus historically and was inadvertently changed when we merged #6304. Co-Authored-By: Adam Mueller <adam@super.gd>
Fix shipment state when all units cancelled
…e-regression Resolve regression in `OrderUpdater#log_state_change`
Keep backorders when splitting part of variant to new shipment with same SL
We don't want our in-memory order updater to have to persist shipment state. We also discovered that a portion of the existing `Shipment#update_state` method has conditionals that could never be hit. So we're adding a new method to help with the recalculation of the shipment state because we don't want to modify any of the existing methods because they're public. Co-authored-by: Jared Norman <jared@super.gd> Co-authored-by: Noah Silvera <noah@super.gd>
While `#determine_state` is technically a public method, it is only used within the shipment model. We also checked all repos on the `solidusio` and `solidusio-contrib` GitHub organizations and only found it used once. So in this commit we remove the method entirely and replace its single usage with the more useful `Shipment#recalculate_state` method. Using `#recalculate_state` is beneficial because it assigns the new state to the shipment in memory. This gives developers more flexibility to actually do something with the new state information. For example, when simulating order updates without persisting them. In some other upcoming work on an in-memory order updater (#5872) we use the new `Shipment#recalculate_state` method as a public method, within the new order updater, which we why we think this is a good candidate for a public method. Co-authored-by: Chris Todorov <chris@super.gd> Co-authored-by: Senem Soy <senem@super.gd>
Replace `Shipment#determine_state`
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.3)
Can you help keep this open source service alive? 💖 Please sponsor : )