Skip to content

Commit

Permalink
Fix the issue where only one error is displayed, even when multiple e…
Browse files Browse the repository at this point in the history
…rrors are present.
  • Loading branch information
nozomirin committed Jan 3, 2025
1 parent 9a09a18 commit 6f80c97
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 20 deletions.
2 changes: 1 addition & 1 deletion app/events/event_types/event_storage_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def reduce_inventory(item_id, quantity, validate: true)
if validate
current_quantity = items[item_id]&.quantity || 0
if current_quantity < quantity
raise InventoryError.new("Could not reduce quantity by #{quantity} - current quantity is #{current_quantity}",
raise InventoryActionError.new("Could not reduce quantity by #{quantity} - current quantity is #{current_quantity}",
item_id,
id)
end
Expand Down
15 changes: 15 additions & 0 deletions app/events/inventory_action_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class InventoryActionError < StandardError
# @return [Integer]
attr_accessor :item_id
# @return [Integer]
attr_accessor :storage_location_id

# @param message [String]
# @param item_id [Integer]
# @param storage_location_id [Integer]
def initialize(message, item_id, storage_location_id)
super(message)
self.item_id = item_id
self.storage_location_id = storage_location_id
end
end
28 changes: 24 additions & 4 deletions app/events/inventory_aggregate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,29 +66,36 @@ def handle(event, inventory, validate: false, previous_event: nil)
# @param validate [Boolean]
# @param previous_event [Event]
def handle_inventory_event(payload, inventory, validate: true, previous_event: nil)
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
inventory.move_item(item_id: line_item.item_id,
move_item(inventory: inventory,
item_id: line_item.item_id,
quantity: quantity,
from_location: line_item.from_storage_location,
to_location: line_item.to_storage_location,
validate: validate)
validate: validate,
errors: errors)
end
# remove the quantity from any items that are now missing
previous_event&.data&.items&.each do |previous_item|
new_item = payload.items.find { |i| i.same_item?(previous_item) }
if new_item.nil?
inventory.move_item(item_id: previous_item.item_id,
move_item(inventory: inventory,
item_id: previous_item.item_id,
quantity: previous_item.quantity,
from_location: previous_item.to_storage_location,
to_location: previous_item.from_storage_location,
validate: validate)
validate: validate,
errors: errors)
end
end

raise InventoryError.new(errors.map(&:message).join("\n")) unless errors.empty?
end

# @param payload [EventTypes::InventoryPayload]
Expand All @@ -100,6 +107,19 @@ def handle_audit_event(payload, inventory)
location: line_item.to_storage_location)
end
end

def move_item(inventory:, item_id:, quantity:, from_location:, to_location:, validate:, errors:)
inventory.move_item(item_id: item_id,
quantity: quantity,
from_location: from_location,
to_location: to_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
end

# ignore previous events
Expand Down
12 changes: 2 additions & 10 deletions app/events/inventory_error.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
class InventoryError < StandardError
# @return [Event]
attr_accessor :event
# @return [Integer]
attr_accessor :item_id
# @return [Integer]
attr_accessor :storage_location_id

# @param message [String]
# @param item_id [Integer]
# @param storage_location_id [Integer]
def initialize(message, item_id, storage_location_id)
super(message)
self.item_id = item_id
self.storage_location_id = storage_location_id
def initialize(message)
super
end
end
3 changes: 0 additions & 3 deletions app/models/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@ def self.most_recent_snapshot(organization_id)
def validate_inventory
InventoryAggregate.inventory_for(organization_id, validate: true)
rescue InventoryError => 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}"
if e.event != self
e.message.prepend("Error occurred when re-running events: #{e.event.type} on #{e.event.created_at.to_date}: ")
e.message << " Please contact the Human Essentials admin staff for assistance."
Expand Down
19 changes: 17 additions & 2 deletions spec/events/inventory_aggregate_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -716,20 +716,35 @@
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) }
context "current event is incorrect" do
it "should raise a bare error" do
donation = FactoryBot.create(:donation, organization: organization, storage_location: storage_location1)
donation.line_items << build(:line_item, quantity: 50, item: item1)
DonationEvent.publish(donation)

distribution = FactoryBot.create(:distribution, organization: organization, storage_location: storage_location1)
distribution.line_items << build(:line_item, quantity: 100, item: item1, itemizable: distribution)
expect { DistributionEvent.publish(distribution) }.to raise_error do |e|
expect(e).to be_a(InventoryError)
expect(e.event).to be_a(DistributionEvent)
expect(e.message).to eq("Could not reduce quantity by 100 - current quantity is 50 for #{item1.name} in #{storage_location1.name}")
end
end

context "while there are multiple errors" do
it "should show all the errors" do
donation.line_items << build(:line_item, quantity: 50, item: item1) << build(:line_item, quantity: 50, item: item2)
DonationEvent.publish(donation)

distribution.line_items << build(:line_item, quantity: 100, item: item1) << build(:line_item, quantity: 100, item: item2)
expect { DistributionEvent.publish(distribution) }.to raise_error do |e|
expect(e).to be_a(InventoryError)
expect(e.event).to be_a(DistributionEvent)
expect(e.message).to eq("Could not reduce quantity by 100 - current quantity is 50 for #{item1.name} in #{storage_location1.name}\n" \
"Could not reduce quantity by 100 - current quantity is 50 for #{item2.name} in #{storage_location1.name}")
end
end
end
end

context "subsequent event is incorrect" do
Expand Down

0 comments on commit 6f80c97

Please sign in to comment.