Fix the issue where only one error is displayed, even when multiple errors are present. (#4911)

* Fix the issue where only one error is displayed, even when multiple errors are present.

* Fix the numbers in brackets are disappear after a failed submission.

* Rename load_items_controller.js to trigger_change_event_controller.js

Author: nozomirin

app/events/event_types/event_storage_location.rb

@@ -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

app/events/inventory_action_error.rb

@@ -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

app/events/inventory_aggregate.rb

@@ -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]
@@ -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

app/events/inventory_error.rb

@@ -1,17 +1,4 @@
 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
-  end
 end

app/javascript/controllers/trigger_change_event_controller.js

@@ -0,0 +1,14 @@
+import { Controller} from "@hotwired/stimulus"
+
+export default class extends Controller {
+    connect() {
+        this.triggerChangeEvent();
+    }
+
+    triggerChangeEvent() {
+        if (this.element.value) {
+            const event = new Event("change", {bubbles: true});
+            this.element.dispatchEvent(event);
+        }
+    }
+}

app/models/event.rb

@@ -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."

app/views/storage_locations/_source.html.erb

@@ -19,5 +19,6 @@
         format: :json
       )
     },
-    class: "storage-location-source"
+    class: "storage-location-source",
+    "data-controller": "trigger-change-event"
   } %>

spec/events/inventory_aggregate_spec.rb

@@ -720,20 +720,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