From 278a336324cb1ab935f755252a1d64e8b9cf0e61 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sun, 18 Feb 2024 16:34:05 -0500 Subject: [PATCH 1/7] Fix errors found in testing --- app/events/event_types/inventory.rb | 8 ++++++++ app/models/view/inventory.rb | 3 ++- spec/events/inventory_aggregate_spec.rb | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/events/event_types/inventory.rb b/app/events/event_types/inventory.rb index 948e90e4ec..80401bd82a 100644 --- a/app/events/event_types/inventory.rb +++ b/app/events/event_types/inventory.rb @@ -30,6 +30,14 @@ def set_item_quantity(item_id:, quantity:, location:) # @param to_location [Integer] # @param validate [Boolean] def move_item(item_id:, quantity:, from_location: nil, to_location: nil, validate: true) + if quantity.negative? + move_item(item_id: item_id, + quantity: -quantity, + from_location: to_location, + to_location: from_location, + validate: validate) + return + end if from_location if storage_locations[from_location].nil? && validate raise "Storage location #{from_location} not found!" diff --git a/app/models/view/inventory.rb b/app/models/view/inventory.rb index c2d97667d6..379f713d99 100644 --- a/app/models/view/inventory.rb +++ b/app/models/view/inventory.rb @@ -82,7 +82,7 @@ def quantity_for(storage_location: nil, item_id: nil) if storage_location if item_id - @inventory.storage_locations[storage_location.to_i].items[item_id.to_i]&.quantity || 0 + @inventory.storage_locations[storage_location.to_i]&.items&.dig(item_id.to_i)&.quantity || 0 else @inventory.storage_locations[storage_location.to_i]&.items&.values&.map(&:quantity)&.sum || 0 end @@ -100,6 +100,7 @@ def storage_locations_for_item(item_id) # @param storage_location [Integer] # @return [Float] def total_value_in_dollars(storage_location: nil) + return 0.0 if @inventory.storage_locations[storage_location].nil? total = @inventory.storage_locations[storage_location].items.values .map { |i| i.value_in_cents ? i.quantity * i.value_in_cents : 0 }.sum total.to_f / 100 diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index 207323bacf..daede88737 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -709,6 +709,20 @@ end context "subsequent event is incorrect" do + + it "should handle negative quantities" do + next unless Event.read_events?(organization) # only relevant if flag is on + + donation = FactoryBot.create(:donation, organization: organization, storage_location: storage_location1) + donation.line_items << build(:line_item, quantity: 100, item: item1) + DonationEvent.publish(donation) + distribution = FactoryBot.create(:distribution, organization: organization, storage_location: storage_location1) + distribution.line_items << build(:line_item, quantity: 90, item: item1) + DistributionEvent.publish(distribution) + donation.line_items.first.quantity = 20 + expect { DonationEvent.publish(donation) }.to raise_error(InventoryError) + end + it "should add the event to the message" do next unless Event.read_events?(organization) # only relevant if flag is on From 4a7e8cc6d2063848db2eea62cbc02664ca8c94bf Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Wed, 21 Feb 2024 20:45:14 -0500 Subject: [PATCH 2/7] fix lint --- app/events/event_types/inventory.rb | 8 ++++---- spec/events/inventory_aggregate_spec.rb | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/events/event_types/inventory.rb b/app/events/event_types/inventory.rb index 80401bd82a..8ca181dc2b 100644 --- a/app/events/event_types/inventory.rb +++ b/app/events/event_types/inventory.rb @@ -32,10 +32,10 @@ def set_item_quantity(item_id:, quantity:, location:) def move_item(item_id:, quantity:, from_location: nil, to_location: nil, validate: true) if quantity.negative? move_item(item_id: item_id, - quantity: -quantity, - from_location: to_location, - to_location: from_location, - validate: validate) + quantity: -quantity, + from_location: to_location, + to_location: from_location, + validate: validate) return end if from_location diff --git a/spec/events/inventory_aggregate_spec.rb b/spec/events/inventory_aggregate_spec.rb index daede88737..e09cb1d443 100644 --- a/spec/events/inventory_aggregate_spec.rb +++ b/spec/events/inventory_aggregate_spec.rb @@ -709,7 +709,6 @@ end context "subsequent event is incorrect" do - it "should handle negative quantities" do next unless Event.read_events?(organization) # only relevant if flag is on From 44446c6969673941067430d902e8cd3d1daa506c Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sun, 25 Feb 2024 10:14:40 -0500 Subject: [PATCH 3/7] Don't crash on donation/purchase creation --- app/controllers/donations_controller.rb | 13 +++++++------ app/controllers/purchases_controller.rb | 9 +++++---- app/services/donation_create_service.rb | 7 ++++--- app/services/purchase_create_service.rb | 7 ++++--- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 03427f22de..95daff0f14 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -43,14 +43,15 @@ def index def create @donation = current_organization.donations.new(donation_params) - if DonationCreateService.call(@donation) - flash[:notice] = "Donation created and logged!" - redirect_to donations_path - else + begin + DonationCreateService.call(@donation) + flash[:notice] = "Donation created and logged!" + redirect_to donations_path + rescue => e load_form_collections @donation.line_items.build if @donation.line_items.count.zero? - flash[:error] = "There was an error starting this donation, try again?" - Rails.logger.error "[!] DonationsController#create Error: #{@donation.errors}" + flash[:error] = "There was an error starting this donation: #{e.message}" + Rails.logger.error "[!] DonationsController#create Error: #{e.message}" render action: :new end end diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb index a77c34b018..4c1c263d8f 100644 --- a/app/controllers/purchases_controller.rb +++ b/app/controllers/purchases_controller.rb @@ -32,14 +32,15 @@ def index def create @purchase = current_organization.purchases.new(purchase_params) - if PurchaseCreateService.call(@purchase) + begin + PurchaseCreateService.call(@purchase) flash[:notice] = "New Purchase logged!" redirect_to purchases_path - else + rescue => e load_form_collections @purchase.line_items.build if @purchase.line_items.count.zero? - flash[:error] = "Failed to create purchase due to: #{@purchase.errors.full_messages}" - Rails.logger.error "[!] PurchasesController#create ERROR: #{@purchase.errors.full_messages}" + flash[:error] = "Failed to create purchase due to: #{e.message}" + Rails.logger.error "[!] PurchasesController#create ERROR: #{e.message}" render action: :new end end diff --git a/app/services/donation_create_service.rb b/app/services/donation_create_service.rb index 67d83aa73a..fb782ef7d8 100644 --- a/app/services/donation_create_service.rb +++ b/app/services/donation_create_service.rb @@ -2,10 +2,11 @@ module DonationCreateService class << self def call(donation) Donation.transaction do - if donation.save - donation.storage_location.increase_inventory(donation.line_item_values) - DonationEvent.publish(donation) + unless donation.save + raise donation.errors.full_messages.join("\n") end + donation.storage_location.increase_inventory(donation.line_item_values) + DonationEvent.publish(donation) end end end diff --git a/app/services/purchase_create_service.rb b/app/services/purchase_create_service.rb index 5ae9ddb8c1..16c1718bb5 100644 --- a/app/services/purchase_create_service.rb +++ b/app/services/purchase_create_service.rb @@ -2,10 +2,11 @@ class PurchaseCreateService class << self def call(purchase) Purchase.transaction do - if purchase.save - purchase.storage_location.increase_inventory(purchase.line_item_values) - PurchaseEvent.publish(purchase) + unless purchase.save + raise purchase.errors.full_messages.join("\n") end + purchase.storage_location.increase_inventory(purchase.line_item_values) + PurchaseEvent.publish(purchase) end end end From cccf3d6078278f4d02fe010483898fd2e3c78b59 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sun, 25 Feb 2024 10:31:38 -0500 Subject: [PATCH 4/7] Fix spec + lint --- app/controllers/donations_controller.rb | 4 ++-- spec/system/purchase_system_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 95daff0f14..faa739848b 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -45,8 +45,8 @@ def create begin DonationCreateService.call(@donation) - flash[:notice] = "Donation created and logged!" - redirect_to donations_path + flash[:notice] = "Donation created and logged!" + redirect_to donations_path rescue => e load_form_collections @donation.line_items.build if @donation.line_items.count.zero? diff --git a/spec/system/purchase_system_spec.rb b/spec/system/purchase_system_spec.rb index 36f0f46075..0d40190b19 100644 --- a/spec/system/purchase_system_spec.rb +++ b/spec/system/purchase_system_spec.rb @@ -197,7 +197,7 @@ it "should display failure with error messages" do click_button "Save" - expect(page).to have_content('Failed to create purchase due to: ["Vendor must exist", "Amount spent is not a number", "Amount spent in cents must be greater than 0"]') + expect(page).to have_content('Failed to create purchase due to: Vendor must exist Amount spent is not a number Amount spent in cents must be greater than 0') end end end From cb639e588fcec1abdb953431d7b8cba9bf8c8a94 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Fri, 1 Mar 2024 15:38:07 -0500 Subject: [PATCH 5/7] Fix crash on audit --- app/controllers/audits_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/audits_controller.rb b/app/controllers/audits_controller.rb index a807952a5d..c6d8e2c99d 100644 --- a/app/controllers/audits_controller.rb +++ b/app/controllers/audits_controller.rb @@ -48,6 +48,8 @@ def finalize end @audit.finalized! redirect_to audit_path(@audit), notice: "Audit is Finalized." + rescue => e + redirect_back(fallback_location: audits_path, alert: "Could not finalize audit: #{e.message}") end def update From 3e2a717586efa7b52c311ee193f06ac8132eaf70 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sat, 2 Mar 2024 21:29:18 -0500 Subject: [PATCH 6/7] Add message to contact staff on issue --- app/models/event.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/event.rb b/app/models/event.rb index c02ca01da4..3aa230861f 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -70,6 +70,7 @@ def validate_inventory 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." end raise e end From f90bf2e48836a6ae0825a0158cef0de16e4051e7 Mon Sep 17 00:00:00 2001 From: Daniel Orner Date: Sun, 3 Mar 2024 20:11:55 -0500 Subject: [PATCH 7/7] Update app/events/event_types/inventory.rb Co-authored-by: Brock Wilcox --- app/events/event_types/inventory.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/events/event_types/inventory.rb b/app/events/event_types/inventory.rb index 8ca181dc2b..b20ef33e3e 100644 --- a/app/events/event_types/inventory.rb +++ b/app/events/event_types/inventory.rb @@ -31,12 +31,11 @@ def set_item_quantity(item_id:, quantity:, location:) # @param validate [Boolean] def move_item(item_id:, quantity:, from_location: nil, to_location: nil, validate: true) if quantity.negative? - move_item(item_id: item_id, + return move_item(item_id: item_id, quantity: -quantity, from_location: to_location, to_location: from_location, validate: validate) - return end if from_location if storage_locations[from_location].nil? && validate