From 880c5abbd2d61db171cff87b78cb729e47bd54dd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 28 Aug 2026 16:36:03 -0400 Subject: [PATCH 1/3] Establishes initial infrastructure, conceptually 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. --- app/events/event_types/event_item.rb | 5 +++++ .../event_types/event_storage_location.rb | 21 ++++++++++++++----- app/models/view/inventory.rb | 1 + 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/events/event_types/event_item.rb b/app/events/event_types/event_item.rb index 71de08ba7b..c616f51081 100644 --- a/app/events/event_types/event_item.rb +++ b/app/events/event_types/event_item.rb @@ -7,6 +7,11 @@ class EventItem < Dry::Struct transform_keys(&:to_sym) attribute :item_id, Types::Integer attribute :quantity, Types::Integer + attribute :committed_quantity, Types::Integer.default(0) attribute? :storage_location_id, Types::Integer + + def physical_quantity + quantity + committed_quantity + end end end diff --git a/app/events/event_types/event_storage_location.rb b/app/events/event_types/event_storage_location.rb index f8b024ce98..2727415d24 100644 --- a/app/events/event_types/event_storage_location.rb +++ b/app/events/event_types/event_storage_location.rb @@ -22,7 +22,12 @@ def reset! # @param item_id [Integer] # @param quantity [Integer] def set_inventory(item_id, quantity) - items[item_id] = EventTypes::EventItem.new(item_id: item_id, quantity: quantity, storage_location_id: id) + items[item_id] = EventTypes::EventItem.new( + item_id: item_id, + storage_location_id: id, + quantity: quantity, + committed_quantity: items[item_id]&.committed_quantity || 0 + ) end # @param item_id [Integer] @@ -38,18 +43,24 @@ def reduce_inventory(item_id, quantity, validate: true) end end current_quantity = items[item_id]&.quantity || 0 - items[item_id] = EventTypes::EventItem.new(item_id: item_id, + items[item_id] = EventTypes::EventItem.new( + item_id: item_id, storage_location_id: id, - quantity: current_quantity - quantity) + quantity: current_quantity - quantity, + committed_quantity: items[item_id]&.committed_quantity || 0 + ) end # @param item_id [Integer] # @param quantity [Integer] def add_inventory(item_id, quantity) current_quantity = items[item_id]&.quantity || 0 - items[item_id] = EventTypes::EventItem.new(item_id: item_id, + items[item_id] = EventTypes::EventItem.new( + item_id: item_id, storage_location_id: id, - quantity: current_quantity + quantity) + quantity: current_quantity + quantity, + committed_quantity: items[item_id]&.committed_quantity || 0 + ) end end end diff --git a/app/models/view/inventory.rb b/app/models/view/inventory.rb index 88f690bc55..f0991ee4ce 100644 --- a/app/models/view/inventory.rb +++ b/app/models/view/inventory.rb @@ -153,6 +153,7 @@ def load_item_details item_id: item.item_id, storage_location_id: loc.id, quantity: item.quantity, + committed_quantity: item.committed_quantity, db_item: db_item ) false From c53ace8446177432496f62595edbe37cd96c75a9 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 29 Aug 2026 09:40:05 -0400 Subject: [PATCH 2/3] Introduces methods to apply the deltas around this reserve concept 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. --- app/controllers/distributions_controller.rb | 2 +- app/events/distribution_complete_event.rb | 17 ++++++++ app/events/distribution_destroy_event.rb | 5 ++- app/events/distribution_event.rb | 5 ++- .../event_types/distribution_payload.rb | 9 ++++ app/events/event_types/event_item.rb | 4 +- .../event_types/event_storage_location.rb | 24 +++++++++-- app/events/event_types/inventory.rb | 9 ++++ app/events/inventory_aggregate.rb | 41 ++++++++++++++++--- app/models/view/inventory.rb | 2 +- app/services/distribution_complete_service.rb | 14 +++++++ spec/events/inventory_aggregate_spec.rb | 12 +++--- 12 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 app/events/distribution_complete_event.rb create mode 100644 app/events/event_types/distribution_payload.rb create mode 100644 app/services/distribution_complete_service.rb diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb index afe636a89e..9270946422 100644 --- a/app/controllers/distributions_controller.rb +++ b/app/controllers/distributions_controller.rb @@ -267,7 +267,7 @@ def calendar def picked_up distribution = current_organization.distributions.find(params[:id]) - if !distribution.complete? && distribution.complete! + if DistributionCompleteService.new(distribution.id).call.success? flash[:notice] = 'This distribution has been marked as being completed!' else flash[:error] = 'Sorry, we encountered an error when trying to mark this distribution as being completed' diff --git a/app/events/distribution_complete_event.rb b/app/events/distribution_complete_event.rb new file mode 100644 index 0000000000..9e293eee73 --- /dev/null +++ b/app/events/distribution_complete_event.rb @@ -0,0 +1,17 @@ +class DistributionCompleteEvent < Event + serialize :data, coder: EventTypes::StructCoder.new(EventTypes::DistributionPayload) + + # @param distribution [Distribution] + def self.publish(distribution) + create( + eventable: distribution, + group_id: "dist-complete-#{distribution.id}-#{SecureRandom.hex}", + organization_id: distribution.organization_id, + event_time: Time.zone.now, + data: EventTypes::DistributionPayload.new( + reserves_inventory: false, + items: EventTypes::EventLineItem.from_line_items(distribution.line_items, from: distribution.storage_location_id) + ) + ) + end +end diff --git a/app/events/distribution_destroy_event.rb b/app/events/distribution_destroy_event.rb index 7d74bd5b17..0cfad40b27 100644 --- a/app/events/distribution_destroy_event.rb +++ b/app/events/distribution_destroy_event.rb @@ -1,4 +1,6 @@ class DistributionDestroyEvent < Event + serialize :data, coder: EventTypes::StructCoder.new(EventTypes::DistributionPayload) + # @param distribution [Distribution] def self.publish(distribution) create( @@ -6,7 +8,8 @@ def self.publish(distribution) group_id: "dist-destroy-#{distribution.id}-#{SecureRandom.hex}", organization_id: distribution.organization_id, event_time: Time.zone.now, - data: EventTypes::InventoryPayload.new( + data: EventTypes::DistributionPayload.new( + reserves_inventory: false, items: EventTypes::EventLineItem.zeroed_line_items(distribution.line_items, from: distribution.storage_location_id) ) ) diff --git a/app/events/distribution_event.rb b/app/events/distribution_event.rb index 156ccb651f..1388bd56ca 100644 --- a/app/events/distribution_event.rb +++ b/app/events/distribution_event.rb @@ -1,4 +1,6 @@ class DistributionEvent < Event + serialize :data, coder: EventTypes::StructCoder.new(EventTypes::DistributionPayload) + # @param distribution [Distribution] def self.publish(distribution) create( @@ -6,7 +8,8 @@ def self.publish(distribution) group_id: "dist-#{distribution.id}-#{SecureRandom.hex}", organization_id: distribution.organization_id, event_time: Time.zone.now, - data: EventTypes::InventoryPayload.new( + data: EventTypes::DistributionPayload.new( + reserves_inventory: distribution.scheduled?, items: EventTypes::EventLineItem.from_line_items(distribution.line_items, from: distribution.storage_location_id) ) ) diff --git a/app/events/event_types/distribution_payload.rb b/app/events/event_types/distribution_payload.rb new file mode 100644 index 0000000000..aefa7c487f --- /dev/null +++ b/app/events/event_types/distribution_payload.rb @@ -0,0 +1,9 @@ +module Types + include Dry.Types() +end + +module EventTypes + class DistributionPayload < InventoryPayload + attribute :reserves_inventory, Types::Bool.default(false) + end +end diff --git a/app/events/event_types/event_item.rb b/app/events/event_types/event_item.rb index c616f51081..9cc354cdbc 100644 --- a/app/events/event_types/event_item.rb +++ b/app/events/event_types/event_item.rb @@ -7,11 +7,11 @@ class EventItem < Dry::Struct transform_keys(&:to_sym) attribute :item_id, Types::Integer attribute :quantity, Types::Integer - attribute :committed_quantity, Types::Integer.default(0) + attribute :reserved_quantity, Types::Integer.default(0) attribute? :storage_location_id, Types::Integer def physical_quantity - quantity + committed_quantity + quantity + reserved_quantity end end end diff --git a/app/events/event_types/event_storage_location.rb b/app/events/event_types/event_storage_location.rb index 2727415d24..3d3b040f9d 100644 --- a/app/events/event_types/event_storage_location.rb +++ b/app/events/event_types/event_storage_location.rb @@ -26,7 +26,7 @@ def set_inventory(item_id, quantity) item_id: item_id, storage_location_id: id, quantity: quantity, - committed_quantity: items[item_id]&.committed_quantity || 0 + reserved_quantity: items[item_id]&.reserved_quantity || 0 ) end @@ -47,7 +47,7 @@ def reduce_inventory(item_id, quantity, validate: true) item_id: item_id, storage_location_id: id, quantity: current_quantity - quantity, - committed_quantity: items[item_id]&.committed_quantity || 0 + reserved_quantity: items[item_id]&.reserved_quantity || 0 ) end @@ -59,7 +59,25 @@ def add_inventory(item_id, quantity) item_id: item_id, storage_location_id: id, quantity: current_quantity + quantity, - committed_quantity: items[item_id]&.committed_quantity || 0 + reserved_quantity: items[item_id]&.reserved_quantity || 0 + ) + end + + # @param item_id [Integer] + # @param quantity [Integer] positive to reserve, negative to release + # @param validate [Boolean] + def adjust_reserved(item_id, quantity, validate: true) + current_quantity = items[item_id]&.reserved_quantity || 0 + if validate && (current_quantity + quantity).negative? + raise InventoryActionError.new("Could not reduce reserved quantity by #{-quantity} - current reserved quantity is #{current_quantity}", + item_id, + id) + end + items[item_id] = EventTypes::EventItem.new( + item_id: item_id, + storage_location_id: id, + quantity: items[item_id]&.quantity || 0, + reserved_quantity: current_quantity + quantity ) end end diff --git a/app/events/event_types/inventory.rb b/app/events/event_types/inventory.rb index b20ef33e3e..900fd4dbfe 100644 --- a/app/events/event_types/inventory.rb +++ b/app/events/event_types/inventory.rb @@ -24,6 +24,15 @@ def set_item_quantity(item_id:, quantity:, location:) storage_locations[location].set_inventory(item_id, quantity) end + # @param item_id [Integer] + # @param quantity [Integer] positive to reserve, negative to release + # @param location [Integer] + # @param validate [Boolean] + def adjust_reserved_item(item_id:, quantity:, location:, validate: true) + storage_locations[location] ||= EventTypes::EventStorageLocation.new(id: location, items: {}) + storage_locations[location].adjust_reserved(item_id, quantity, validate: validate) + end + # @param item_id [Integer] # @param quantity [Integer] # @param from_location [Integer] diff --git a/app/events/inventory_aggregate.rb b/app/events/inventory_aggregate.rb index bfc6a2bdff..b7844fe2c3 100644 --- a/app/events/inventory_aggregate.rb +++ b/app/events/inventory_aggregate.rb @@ -69,10 +69,8 @@ def handle_inventory_event(payload, inventory, validate: true, previous_event: n errors = [] payload.items.each do |line_item| quantity = line_item.quantity - if previous_event - previous_item = previous_event.data.items.find { |i| i.same_item?(line_item) } - quantity -= previous_item.quantity if previous_item - end + previous_item = previous_event&.data&.items&.find { |i| i.same_item?(line_item) } + quantity -= previous_item.quantity if previous_item move_item(inventory: inventory, item_id: line_item.item_id, quantity: quantity, @@ -80,6 +78,14 @@ def handle_inventory_event(payload, inventory, validate: true, previous_event: n to_location: line_item.to_storage_location, validate: validate, errors: errors) + reserved = reserves?(payload) ? line_item.quantity : 0 + reserved -= previous_item.quantity if previous_item && reserves?(previous_event.data) + adjust_reserved(inventory: inventory, + item_id: line_item.item_id, + quantity: reserved, + location: line_item.from_storage_location, + validate: validate, + errors: errors) end # remove the quantity from any items that are now missing previous_event&.data&.items&.each do |previous_item| @@ -92,6 +98,12 @@ def handle_inventory_event(payload, inventory, validate: true, previous_event: n to_location: previous_item.from_storage_location, validate: validate, errors: errors) + adjust_reserved(inventory: inventory, + item_id: previous_item.item_id, + quantity: reserves?(previous_event.data) ? -previous_item.quantity : 0, + location: previous_item.from_storage_location, + validate: validate, + errors: errors) end end @@ -108,6 +120,23 @@ def handle_audit_event(payload, inventory) end end + # @param payload [EventTypes::InventoryPayload] + # @return [Boolean] + def reserves?(payload) + payload.respond_to?(:reserves_inventory) && payload.reserves_inventory + end + + def adjust_reserved(inventory:, item_id:, quantity:, location:, validate:, errors:) + return if quantity.zero? || location.nil? + + inventory.adjust_reserved_item(item_id: item_id, quantity: quantity, location: location, validate: validate) + rescue InventoryActionError => e + item = Item.find_by(id: e.item_id)&.name || "Item ID #{e.item_id}" + loc = StorageLocation.find_by(id: e.storage_location_id)&.name || "Storage Location ID #{e.storage_location_id}" + e.message << " for #{item} in #{loc}" + errors.push(e) + end + def move_item(inventory:, item_id:, quantity:, from_location:, to_location:, validate:, errors:) inventory.move_item(item_id: item_id, quantity: quantity, @@ -132,8 +161,8 @@ def move_item(inventory:, item_id:, quantity:, from_location:, to_location:, val # diff previous event on DonationEvent, DistributionEvent, AdjustmentEvent, PurchaseEvent, - TransferEvent, DistributionDestroyEvent, DonationDestroyEvent, - PurchaseDestroyEvent, TransferDestroyEvent, + TransferEvent, DistributionDestroyEvent, DistributionCompleteEvent, + DonationDestroyEvent, PurchaseDestroyEvent, TransferDestroyEvent, UpdateExistingEvent do |event, inventory, validate: false, previous_event: nil| handle_inventory_event(event.data, inventory, validate: validate, previous_event: previous_event) rescue InventoryError => e diff --git a/app/models/view/inventory.rb b/app/models/view/inventory.rb index f0991ee4ce..369a6d1e6f 100644 --- a/app/models/view/inventory.rb +++ b/app/models/view/inventory.rb @@ -153,7 +153,7 @@ def load_item_details item_id: item.item_id, storage_location_id: loc.id, quantity: item.quantity, - committed_quantity: item.committed_quantity, + reserved_quantity: item.reserved_quantity, db_item: db_item ) false diff --git a/app/services/distribution_complete_service.rb b/app/services/distribution_complete_service.rb new file mode 100644 index 0000000000..a588fc5ce4 --- /dev/null +++ b/app/services/distribution_complete_service.rb @@ -0,0 +1,14 @@ +class DistributionCompleteService < DistributionService + def initialize(distribution_id) + @distribution_id = distribution_id + end + + def call + perform_distribution_service do + raise "Distribution #{distribution_id} is already complete" if distribution.complete? + + DistributionCompleteEvent.publish(distribution) + distribution.complete! + end + end +end diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index 4ac4a7bfce..cad166b764 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -69,8 +69,8 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, storage_location_id: storage_location1.id), - item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 5, storage_location_id: storage_location1.id), + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, reserved_quantity: 20, storage_location_id: storage_location1.id), + item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 5, reserved_quantity: 5, storage_location_id: storage_location1.id), item3.id => EventTypes::EventItem.new(item_id: item3.id, quantity: 40, storage_location_id: storage_location1.id) } ), @@ -507,14 +507,14 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 70, storage_location_id: storage_location1.id), + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 70, reserved_quantity: 10, storage_location_id: storage_location1.id), item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 30, storage_location_id: storage_location1.id) } ), storage_location2.id => EventTypes::EventStorageLocation.new( id: storage_location2.id, items: { - item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 25, storage_location_id: storage_location2.id), + item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 25, reserved_quantity: 15, storage_location_id: storage_location2.id), item4.id => EventTypes::EventItem.new(item_id: item4.id, quantity: 50, storage_location_id: storage_location2.id) } ) @@ -579,7 +579,7 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 20, storage_location_id: storage_location1.id) + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 20, reserved_quantity: 40, storage_location_id: storage_location1.id) } ) } @@ -606,7 +606,7 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, storage_location_id: storage_location1.id) + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, reserved_quantity: 10, storage_location_id: storage_location1.id) } ) } From c986dfe22f282d485c89db44ecee92cedc15d414 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 30 Aug 2026 10:43:08 -0400 Subject: [PATCH 3/3] Adds feature flag and props up actual work to do the working work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- app/events/distribution_event.rb | 2 +- spec/events/distribution_event_spec.rb | 39 ++++ spec/events/event_types/event_item_spec.rb | 26 +++ .../event_storage_location_spec.rb | 126 ++++++++++++ spec/events/inventory_aggregate_spec.rb | 193 +++++++++++++++++- spec/models/view/inventory_spec.rb | 16 ++ spec/requests/distributions_requests_spec.rb | 8 + .../distribution_complete_service_spec.rb | 60 ++++++ 8 files changed, 463 insertions(+), 7 deletions(-) create mode 100644 spec/events/distribution_event_spec.rb create mode 100644 spec/events/event_types/event_item_spec.rb create mode 100644 spec/events/event_types/event_storage_location_spec.rb create mode 100644 spec/services/distribution_complete_service_spec.rb diff --git a/app/events/distribution_event.rb b/app/events/distribution_event.rb index 1388bd56ca..1357c934a3 100644 --- a/app/events/distribution_event.rb +++ b/app/events/distribution_event.rb @@ -9,7 +9,7 @@ def self.publish(distribution) organization_id: distribution.organization_id, event_time: Time.zone.now, data: EventTypes::DistributionPayload.new( - reserves_inventory: distribution.scheduled?, + reserves_inventory: Flipper.enabled?(:reserved_inventory, distribution.organization) && distribution.scheduled?, items: EventTypes::EventLineItem.from_line_items(distribution.line_items, from: distribution.storage_location_id) ) ) diff --git a/spec/events/distribution_event_spec.rb b/spec/events/distribution_event_spec.rb new file mode 100644 index 0000000000..4a627ebe61 --- /dev/null +++ b/spec/events/distribution_event_spec.rb @@ -0,0 +1,39 @@ +RSpec.describe DistributionEvent do + let(:organization) { create(:organization) } + let(:storage_location) { create(:storage_location, organization: organization) } + let(:item) { create(:item, organization: organization) } + let(:distribution) do + dist = create(:distribution, organization: organization, storage_location: storage_location) + dist.line_items << build(:line_item, quantity: 30, item: item, itemizable: dist) + dist + end + + before { TestInventory.create_inventory(organization, {storage_location.id => {item.id => 100}}) } + + describe ".publish" do + subject { described_class.publish(distribution).data.reserves_inventory } + + context "when the feature is enabled for the organization" do + before { Flipper.enable(:reserved_inventory) } + + it { is_expected.to be true } + + context "when the distribution is already complete" do + before do + distribution.complete! + end + + it { is_expected.to be false } + end + + end + + context "when the feature is not enabled for the organization" do + before do + expect(Flipper.enabled?(:reserved_inventory)).to eq false + end + + it { is_expected.to be false } + end + end +end diff --git a/spec/events/event_types/event_item_spec.rb b/spec/events/event_types/event_item_spec.rb new file mode 100644 index 0000000000..0da3d53fdf --- /dev/null +++ b/spec/events/event_types/event_item_spec.rb @@ -0,0 +1,26 @@ +RSpec.describe EventTypes::EventItem do + let(:quantity) { 70 } + let(:reserved_quantity) { 30 } + subject(:base_item) { described_class.new(item_id: 1, **{ quantity: quantity, reserved_quantity: reserved_quantity }.compact_blank) } + + describe "#physical_quantity" do + subject { base_item.physical_quantity } + + it "sums available and reserved" do + expect(subject).to eq(quantity + reserved_quantity) + end + end + + describe "#reserved_quantity" do + subject { base_item.reserved_quantity } + + context "when no reserve quantity is provided" do + let(:reserved_quantity) { nil } + + it "defaults to zero so payloads predating the attribute still load" do + expect(subject).to eq(0) + expect(base_item.physical_quantity).to eq(70) + end + end + end +end diff --git a/spec/events/event_types/event_storage_location_spec.rb b/spec/events/event_types/event_storage_location_spec.rb new file mode 100644 index 0000000000..2494b4c3e8 --- /dev/null +++ b/spec/events/event_types/event_storage_location_spec.rb @@ -0,0 +1,126 @@ +RSpec.describe EventTypes::EventStorageLocation do + let(:item_id) { 1 } + let(:starting_quantity) { 50 } + let(:starting_reserved_quantity) { 20 } + + subject(:base_event) do + described_class.new( + id: 10, + items: { + item_id => EventTypes::EventItem.new( + item_id: item_id, + storage_location_id: 10, + quantity: starting_quantity, + reserved_quantity: starting_reserved_quantity, + ) + } + ) + end + + def entry + base_event.items[item_id] + end + + describe "#adjust_reserved" do + let(:delta) { 0 } + subject { base_event.adjust_reserved(item_id, delta) } + + context "when it increases the reserved amount" do + let(:delta) { 20 } + + it "changes the reserved quantity but not the available quantity" do + expect { + subject + }.not_to change { entry.quantity }.from(starting_quantity) + + expect(entry.reserved_quantity).to eq(starting_reserved_quantity + delta) + end + end + + context "when it decreases the reserved amount" do + let(:delta) { -20 } + + it "changes the reserved quantity but not the available quantity" do + expect { + subject + }.not_to change { entry.quantity }.from(starting_quantity) + + expect(entry.reserved_quantity).to eq(starting_reserved_quantity + delta) + end + end + + context "when it attempts to modify an item that does not yet exist" do + let(:unknown_item_id) { 2 } + + before do + expect { Item.find(unknown_item_id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + subject { base_event.adjust_reserved(2, 15) } + + pending "⚠️ What should happen here?" do + # Possible avenues: raise an exception? set to 0? + # This is the behavior that Claude generated, but I don't think it's right. + expect(subject.items[2].reserved_quantity).to eq(15) + expect(subject.items[2].quantity).to eq(0) + end + end + + context "when attempting to release more reserves than available" do + let(:delta) { (starting_reserved_quantity + 10) * -1} + + before do + expect(entry.reserved_quantity).to be < delta.abs + end + + it "raises when a release would drive reserved negative" do + expect { subject }.to raise_error(InventoryActionError) + expect(entry.reserved_quantity).to eq(starting_reserved_quantity) + end + + context "and the validation is turned off" do + subject { base_event.adjust_reserved(item_id, delta, validate: false) } + + it "allows reserved to go negative when validation is off" do + expect { + subject + }.to change { entry.reserved_quantity }.by(delta) + end + end + end + end + + describe "#add_inventory" do + let(:delta) { 10 } + subject { base_event.add_inventory(item_id, delta) } + + it "increase the quantity but not the reserved_quantity" do + subject + expect(entry.quantity).to eq(starting_quantity + delta) + expect(entry.reserved_quantity).to eq(starting_reserved_quantity) + end + end + + describe "#reduce_inventory" do + let(:delta) { 10 } + subject { base_event.reduce_inventory(item_id, delta) } + + it "decreases the quantity but not the reserved_quantity" do + subject + expect(entry.quantity).to eq(starting_quantity - delta) + expect(entry.reserved_quantity).to eq(starting_reserved_quantity) + end + + end + + describe "#set_inventory" do + let(:new_value) { 5 } + subject { base_event.set_inventory(item_id, new_value) } + + it "changes the inventory but does not change the reserved quantity" do + subject + expect(entry.quantity).to eq(new_value) + expect(entry.reserved_quantity).to eq(starting_reserved_quantity) + end + end +end diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index cad166b764..dfa400b288 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -6,6 +6,10 @@ let(:item2) { FactoryBot.create(:item, organization: organization) } let(:item3) { FactoryBot.create(:item, organization: organization) } + before do + expect(Flipper.enabled?(:reserved_inventory)).to be false + end + describe "individual events" do let!(:inventory) do TestInventory.create_inventory(organization, @@ -69,8 +73,8 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, reserved_quantity: 20, storage_location_id: storage_location1.id), - item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 5, reserved_quantity: 5, storage_location_id: storage_location1.id), + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, storage_location_id: storage_location1.id), + item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 5, storage_location_id: storage_location1.id), item3.id => EventTypes::EventItem.new(item_id: item3.id, quantity: 40, storage_location_id: storage_location1.id) } ), @@ -507,14 +511,14 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 70, reserved_quantity: 10, storage_location_id: storage_location1.id), + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 70, storage_location_id: storage_location1.id), item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 30, storage_location_id: storage_location1.id) } ), storage_location2.id => EventTypes::EventStorageLocation.new( id: storage_location2.id, items: { - item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 25, reserved_quantity: 15, storage_location_id: storage_location2.id), + item2.id => EventTypes::EventItem.new(item_id: item2.id, quantity: 25, storage_location_id: storage_location2.id), item4.id => EventTypes::EventItem.new(item_id: item4.id, quantity: 50, storage_location_id: storage_location2.id) } ) @@ -579,7 +583,7 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 20, reserved_quantity: 40, storage_location_id: storage_location1.id) + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 20, storage_location_id: storage_location1.id) } ) } @@ -606,7 +610,7 @@ storage_location1.id => EventTypes::EventStorageLocation.new( id: storage_location1.id, items: { - item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, reserved_quantity: 10, storage_location_id: storage_location1.id) + item1.id => EventTypes::EventItem.new(item_id: item1.id, quantity: 10, storage_location_id: storage_location1.id) } ) } @@ -671,6 +675,183 @@ end end + describe "reserved inventory lifecycle" do + before { Flipper.enable(:reserved_inventory) } + + let(:distributed_quantity) { 30 } + let(:item_1_starting_quantity) { 100 } + let(:item_2_starting_quantity) { 50 } + + let!(:starting_inventory) do + TestInventory.create_inventory(organization, + {storage_location1.id => {item1.id => item_1_starting_quantity, item2.id => item_2_starting_quantity}}) + end + + let(:distribution) do + dist = FactoryBot.create(:distribution, organization: organization, storage_location: storage_location1) + dist.line_items << build(:line_item, quantity: distributed_quantity, item: item1, itemizable: dist) + dist.line_items << build(:line_item, quantity: 10, item: item2, itemizable: dist) + dist + end + + # [available, reserved, physical] for the given item and storage location + def state(item = item1, at: storage_location1) + entry = described_class.inventory_for(organization.id).storage_locations[at.id].items[item.id] + [entry.quantity, entry.reserved_quantity, entry.physical_quantity] + end + + it "sets the reserved quantity when a scheduled distribution is created" do + expect { + DistributionEvent.publish(distribution) + }.to change { state(item1) }.from([100, 0, 100]).to([70, 30, 100]) + end + + context "when a distribution has already been published" do + before do + DistributionEvent.publish(distribution) + end + + it "changes the available / reserved quantities when the quantity is increased" do + distribution.line_items[0].quantity = 40 + DistributionEvent.publish(distribution) + expect(state(item1)).to eq([60, 40, 100]) + end + + it "changes the available / reserved quantities when the quantity is decreased" do + distribution.line_items[0].quantity = 20 + DistributionEvent.publish(distribution) + expect(state(item1)).to eq([80, 20, 100]) + end + + it "releases the reservation on completion, leaving available untouched" do + expect { + DistributionCompleteEvent.publish(distribution) + }.to change { state(item1) }.from([70, 30, 100]).to([70, 0, 70]) + end + + it "reverses both dimensions when a scheduled distribution is reclaimed" do + expect { + DistributionDestroyEvent.publish(distribution) + }.to change { state(item1) }.from([70, 30, 100]).to([100, 0, 100]) + end + + it "carries reserved quantities through a snapshot" do + expect { + SnapshotEvent.publish(organization) + }.not_to change { state(item1) }.from([70, 30, 100]) + end + + context "when the distribution has already been completed" do + before do + DistributionCompleteEvent.publish(distribution) + distribution.complete! + end + + it "moves only available when a completed distribution is edited" do + distribution.line_items[0].quantity = 40 + expect { + DistributionEvent.publish(distribution) + }.to change { state(item1) }.from([70, 0, 70]).to([60, 0, 60]) + end + + it "moves only available when a completed distribution is reclaimed" do + expect { + DistributionDestroyEvent.publish(distribution) + }.to change { state(item1) }.from([70, 0, 70]).to([100, 0, 100]) + end + end + + it "releases the reservation when a line item is dropped from the distribution" do + distribution.line_items = [build(:line_item, quantity: 30, item: item1, itemizable: distribution)] + expect { + DistributionEvent.publish(distribution) + }.to change { state(item2) }.from([40, 10, 50]).to([50, 0, 50]) + .and not_change { state(item1) }.from([70, 30, 100]) + end + + context "when inventory is added through a donation, purchase, or transfer" do + before do + donation = FactoryBot.create(:donation, organization: organization, storage_location: storage_location1) + donation.line_items << build(:line_item, quantity: 10, item: item1, itemizable: donation) + DonationEvent.publish(donation) + + purchase = FactoryBot.create(:purchase, organization: organization, storage_location: storage_location1) + purchase.line_items << build(:line_item, quantity: 5, item: item1, itemizable: purchase) + PurchaseEvent.publish(purchase) + + transfer = FactoryBot.create(:transfer, organization: organization, + from: storage_location1, to: storage_location2) + transfer.line_items << build(:line_item, quantity: 15, item: item1, itemizable: transfer) + TransferEvent.publish(transfer) + end + + it "does not change the reserved quantity" do + expect(state).to eq([70, 30, 100]) + end + end + end + + it "holds the reservation across an intervening audit" do + donation = FactoryBot.create(:donation, organization: organization, storage_location: storage_location1) + donation.line_items << build(:line_item, quantity: 30, item: item1, itemizable: donation) + DonationEvent.publish(donation) + + dist = FactoryBot.create(:distribution, organization: organization, storage_location: storage_location1) + dist.line_items << build(:line_item, quantity: 10, item: item1, itemizable: dist) + DistributionEvent.publish(dist) + + audit = FactoryBot.create(:audit, organization: organization, storage_location: storage_location1) + audit.line_items << build(:line_item, quantity: 50, item: item1, itemizable: audit) + AuditEvent.publish(audit) + + dist.line_items[0].quantity = 40 + expect { + DistributionEvent.publish(dist) + }.to change { state(item1) }.from([50, 10, 60]).to([20, 40, 60]) + end + + it "moves available without touching reserved when an earlier donation is corrected" do + donation = FactoryBot.create(:donation, organization: organization, storage_location: storage_location1) + donation.line_items << build(:line_item, quantity: 30, item: item1, itemizable: donation) + DonationEvent.publish(donation) + + DistributionEvent.publish(distribution) + + donation.line_items[0].quantity = 20 + DonationEvent.publish(donation) + + expect(state).to eq([90, 30, 120]) + end + + it "tracks reservations per storage location" do + transfer = FactoryBot.create(:transfer, organization: organization, + from: storage_location1, to: storage_location2) + transfer.line_items << build(:line_item, quantity: 40, item: item1, itemizable: transfer) + TransferEvent.publish(transfer) + + DistributionEvent.publish(distribution) + + dist2 = FactoryBot.create(:distribution, organization: organization, storage_location: storage_location2) + dist2.line_items << build(:line_item, quantity: 15, item: item1, itemizable: dist2) + DistributionEvent.publish(dist2) + + expect(state).to eq([30, 30, 60]) + expect(state(item1, at: storage_location2)).to eq([25, 15, 40]) + end + + context "when the organization does not have the feature enabled" do + before { Flipper.disable(:reserved_inventory) } + + it "reserves nothing across the whole lifecycle" do + DistributionEvent.publish(distribution) + expect(state).to eq([70, 0, 70]) + + DistributionCompleteEvent.publish(distribution) + expect(state).to eq([70, 0, 70]) + end + end + end + describe "validation" do let(:donation) { FactoryBot.create(:donation, organization: organization, storage_location: storage_location1) } let(:distribution) { FactoryBot.create(:distribution, organization: organization, storage_location: storage_location1) } diff --git a/spec/models/view/inventory_spec.rb b/spec/models/view/inventory_spec.rb index 4987e32c74..0c67c83938 100644 --- a/spec/models/view/inventory_spec.rb +++ b/spec/models/view/inventory_spec.rb @@ -132,6 +132,22 @@ end end + describe "reserved quantities" do + before do + Flipper.enable(:reserved_inventory) + distribution = create(:distribution, organization: organization, storage_location: storage_location1) + distribution.line_items << build(:line_item, quantity: 30, item: item1, itemizable: distribution) + DistributionEvent.publish(distribution) + end + + it "survives the view layer rebuilding each item" do + entry = described_class.new(organization.id).storage_locations[storage_location1.id].items[item1.id] + expect(entry.quantity).to eq(70) + expect(entry.reserved_quantity).to eq(30) + expect(entry.physical_quantity).to eq(100) + end + end + describe "#all_items" do it "should return all items across storage locations" do results = subject.all_items diff --git a/spec/requests/distributions_requests_spec.rb b/spec/requests/distributions_requests_spec.rb index c429032754..b8976dc113 100644 --- a/spec/requests/distributions_requests_spec.rb +++ b/spec/requests/distributions_requests_spec.rb @@ -591,6 +591,14 @@ include_examples "restricts access to organization users/admins" end + + context 'when the distribution is already complete' do + let(:distribution) { create(:distribution, state: :complete, organization: organization) } + + it 'redirects the user back to the distributions page' do + expect(subject).to redirect_to distribution_path + end + end end describe "GET #pickup_day" do diff --git a/spec/services/distribution_complete_service_spec.rb b/spec/services/distribution_complete_service_spec.rb new file mode 100644 index 0000000000..dad458f8bc --- /dev/null +++ b/spec/services/distribution_complete_service_spec.rb @@ -0,0 +1,60 @@ +RSpec.describe DistributionCompleteService do + let(:organization) { create(:organization) } + let(:storage_location) { create(:storage_location, organization: organization) } + let(:item) { create(:item, organization: organization) } + let(:distribution) do + dist = create(:distribution, organization: organization, storage_location: storage_location) + dist.line_items << build(:line_item, quantity: 30, item: item, itemizable: dist) + dist + end + + def state + entry = InventoryAggregate.inventory_for(organization.id).storage_locations[storage_location.id].items[item.id] + [entry.quantity, entry.reserved_quantity] + end + + before do + Flipper.enable(:reserved_inventory) + TestInventory.create_inventory(organization, {storage_location.id => {item.id => 100}}) + end + + describe "#call" do + context "when the distribution is scheduled" do + before { DistributionEvent.publish(distribution) } + + it "is successful" do + expect(described_class.new(distribution.id).call).to be_success + end + + it "marks the distribution complete" do + described_class.new(distribution.id).call + expect(distribution.reload).to be_complete + end + + it "releases the reservation without returning it to available" do + expect { described_class.new(distribution.id).call }.to change { state }.from([70, 30]).to([70, 0]) + end + end + + context "when the distribution is already complete" do + before do + DistributionEvent.publish(distribution) + described_class.new(distribution.id).call + end + + it "is not a success" do + expect(described_class.new(distribution.id).call).not_to be_success + end + + it "does not release the reservation a second time" do + expect { described_class.new(distribution.id).call }.not_to change { state } + end + end + + context "when the distribution_id matches no Distribution" do + it "is not a success" do + expect(described_class.new(Faker::Number.number).call).not_to be_success + end + end + end +end