Conversation
The basic concept here is: "quantity" -- same current concept. Available, on-hand inventory that can be used for distributions. "committed" -- inventory that has been earmarked for distribution "physical" -- sum of "quantity" + "committed" The action concepts are: commit -> reduce quantity (available), increase committed, physical is unchanged fulfill -> quantity unchanged, decrease committed, decrease physical uncommit -> increase quantity, decrease committed, physical unchanged. These conceptual nouns and verbs cleanly describe the relationship here, and will be used for the model going forward.
NOUNS Inventory - pesistent concept, inventory that is on-hand and available for distribution Reserved - inventory that has been allocated for a scheduled Distribution Physical - the sum of inventory + reserved, inventory that is still physically on-site and countable TRANSITIONS available reserved physical created -n +n n/c edited +/- delta -/+ delta n/c completed n/c -n -n reclaimed +n -n unchanged changed after +/- delta n/c +/- delta This commit _specifically_ only does happy path with this feature. There is an obvious error state that can happen when there is nothing reserved and an existing distribution attempts to be completed (no reserved inventory to reduce). Addressing separately. Claude was used for this generation, with heavy supervision and interaction. Brock also reviewed it with me.
Ref #5667 This pass adds a feature flag and hides the reserving behavior behind it. The tests validate this. The actual behaviors around reserving inventory are now working and modifying the in-flight distributions. The inventory_aggregate spec is probably the most illustrative of the big picture, but the individual event item specs show reasonable unit test coverage of the behaviors as well. There might be some additional cleanup on the inventory_aggregator spec, but AFAIK they all pass. I ran out of time :( 🤖 Claude was used for _some_ of the code implementation on this, and while it did generate some specs initially, I rewrote most of them by hand.
dorner
requested changes
Sep 18, 2026
| attribute? :storage_location_id, Types::Integer | ||
|
|
||
| def physical_quantity | ||
| quantity + reserved_quantity |
Collaborator
There was a problem hiding this comment.
If quantity is neither physical nor reserved... what is it?
| event_time: Time.zone.now, | ||
| data: EventTypes::InventoryPayload.new( | ||
| data: EventTypes::DistributionPayload.new( | ||
| reserves_inventory: Flipper.enabled?(:reserved_inventory, distribution.organization) && distribution.scheduled?, |
Collaborator
There was a problem hiding this comment.
So is the idea that if the distribution isn't scheduled, it gets added immediately to the physical quantity? Is that a good assumption?
|
|
||
| module EventTypes | ||
| class DistributionPayload < InventoryPayload | ||
| attribute :reserves_inventory, Types::Bool.default(false) |
Collaborator
There was a problem hiding this comment.
Later on we're doing a lot of math depending on the value in this boolean. Rather than having a boolean that dictates what the interpretation of "quantity" means, I'd rather set both values (reserved and physical) in the payload itself. It means we have to be careful to set only one or the other value at a time, but the reading piece becomes much simpler.
| subject { base_event.adjust_reserved(2, 15) } | ||
|
|
||
| pending "⚠️ What should happen here?" do | ||
| # Possible avenues: raise an exception? set to 0? |
Collaborator
There was a problem hiding this comment.
I don't think either of these are right. If you reserve 15 of an item we haven't seen before, we should now have an item with a quantity of 0 and a reserved quantity of 15.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Ref #5667 This gets the feature most of hte way there. You'll want to read through this in detail. The individual commit messages handle more detail.
Checklist:
-->
Resolves #1
Description
Type of change
How Has This Been Tested?
Screenshots