Fix inventory quantity errors to be tied to organization, not storage…

… location (Fixes #4647) (#4804)

* fix item minimum error to consider quantity across organization

* display correct alerts when distribution causes distinct items to dip below recommended and minimum levels

* refine recommended quantity error to occur based on org quantity, not storage location quantity

* fix distribution system specs that failed due to updated error message content

* display minimum and recommended amount error messages on separate lines

* refactor inventory check alert messages and related spec

* fix lint errors

app/controllers/distributions_controller.rb

@@ -316,11 +316,8 @@ def filter_params
   def perform_inventory_check
     inventory_check_result = InventoryCheckService.new(@distribution).call
 
-    if inventory_check_result.error.present?
-      flash[:error] = inventory_check_result.error
-    end
-    if inventory_check_result.alert.present?
-      flash[:alert] = inventory_check_result.alert
-    end
+    alerts = [inventory_check_result.minimum_alert, inventory_check_result.recommended_alert]
+    merged_alert = alerts.compact.join("\n")
+    flash[:alert] = merged_alert if merged_alert.present?
   end
 end

app/services/inventory_check_service.rb

@@ -1,40 +1,40 @@
 class InventoryCheckService
-  attr_reader :error, :alert
+  attr_reader :minimum_alert, :recommended_alert
 
   def initialize(distribution)
     @distribution = distribution
-    @alert = nil
-    @error = nil
+    @minimum_alert = nil
+    @recommended_alert = nil
   end
 
   def call
     @inventory = View::Inventory.new(@distribution.organization_id)
     unless items_below_minimum_quantity.empty?
-      set_error
+      set_minimum_alert
     end
 
     unless deduplicate_items_below_recommended_quantity.empty?
-      set_alert
+      set_recommended_alert
     end
 
     self
   end
 
-  def set_error
-    @error = "The following items have fallen below the minimum " \
-      "on hand quantity: #{items_below_minimum_quantity.map(&:name).sort.join(", ")}"
+  def set_minimum_alert
+    @minimum_alert = "The following items have fallen below the minimum " \
+      "on hand quantity, bank-wide: #{items_below_minimum_quantity.map(&:name).sort.join(", ")}"
   end
 
-  def set_alert
-    @alert = "The following items have fallen below the recommended " \
-      "on hand quantity: #{deduplicate_items_below_recommended_quantity.map(&:name).sort.join(", ")}"
+  def set_recommended_alert
+    @recommended_alert = "The following items have fallen below the recommended " \
+      "on hand quantity, bank-wide: #{deduplicate_items_below_recommended_quantity.map(&:name).sort.join(", ")}"
   end
 
   def items_below_minimum_quantity
     # Done this way to prevent N+1 query on items
     unless @items_below_minimum_quantity
       item_ids = @distribution.line_items.select do |line_item|
-        quantity = @inventory.quantity_for(storage_location: @distribution.storage_location_id, item_id: line_item.item_id)
+        quantity = @inventory.quantity_for(item_id: line_item.item_id)
         quantity < (line_item.item.on_hand_minimum_quantity || 0)
       end.map(&:item_id)
 
@@ -48,7 +48,7 @@ def items_below_recommended_quantity
     # Done this way to prevent N+1 query on items
     unless @items_below_recommended_quantity
       item_ids = @distribution.line_items.select do |line_item|
-        quantity = @inventory.quantity_for(storage_location: @distribution.storage_location_id, item_id: line_item.item_id)
+        quantity = @inventory.quantity_for(item_id: line_item.item_id)
         quantity < (line_item.item.on_hand_recommended_quantity || 0)
       end.map(&:item_id)
 

spec/controllers/distributions_controller_spec.rb

@@ -9,9 +9,57 @@
     end
 
     describe "POST #create" do
-      context "when distribution causes inventory quantity to be below minimum quantity" do
-        let(:item) { create(:item, name: "Item 1", organization: organization, on_hand_minimum_quantity: 5) }
-        let(:storage_location) { create(:storage_location, :with_items, item: item, item_quantity: 20, organization: organization) }
+      let(:available_item) { create(:item, name: "Available Item", organization: organization, on_hand_minimum_quantity: 5) }
+      let!(:first_storage_location) { create(:storage_location, :with_items, item: available_item, item_quantity: 20, organization: organization) }
+      let!(:second_storage_location) { create(:storage_location, :with_items, item: available_item, item_quantity: 20, organization: organization) }
+      context "when distribution causes inventory to remain above minimum quantity for an organization" do
+        let(:params) do
+          {
+            organization_name: organization.id,
+            distribution: {
+              partner_id: partner.id,
+              storage_location_id: first_storage_location.id,
+              line_items_attributes:
+              {
+                "0": { item_id: first_storage_location.items.first.id, quantity: 10 }
+              }
+            }
+          }
+        end
+
+        subject { post :create, params: params.merge(format: :turbo_stream) }
+
+        it "does not display an error" do
+          subject
+
+          expect(flash[:alert]).to be_nil
+        end
+
+        context "when distribution causes inventory to fall below minimum quantity for a storage location" do
+          let(:params) do
+            {
+              organization_name: organization.id,
+              distribution: {
+                partner_id: partner.id,
+                storage_location_id: second_storage_location.id,
+                line_items_attributes:
+                  {
+                    "0": { item_id: second_storage_location.items.first.id, quantity: 18 }
+                  }
+              }
+            }
+          end
+          it "does not display an error" do
+            subject
+            expect(flash[:notice]).to eq("Distribution created!")
+            expect(flash[:error]).to be_nil
+          end
+        end
+      end
+
+      context "when distribution causes inventory quantity to be below minimum quantity for an organization" do
+        let(:first_item) { create(:item, name: "Item 1", organization: organization, on_hand_minimum_quantity: 5) }
+        let(:storage_location) { create(:storage_location, :with_items, item: first_item, item_quantity: 20, organization: organization) }
         let(:params) do
           {
             organization_name: organization.id,
@@ -31,11 +79,44 @@
         it "redirects with a flash notice and a flash error" do
           expect(subject).to have_http_status(:redirect)
           expect(flash[:notice]).to eq("Distribution created!")
-          expect(flash[:error]).to eq("The following items have fallen below the minimum on hand quantity: Item 1")
+          expect(flash[:alert]).to eq("The following items have fallen below the minimum on hand quantity, bank-wide: Item 1")
+        end
+
+        context "when distribution causes inventory quantity to be below recommended quantity for an organization" do
+          let(:second_item) { create(:item, name: "Item 2", organization: organization, on_hand_minimum_quantity: 5, on_hand_recommended_quantity: 10) }
+          let(:storage_location) { create(:storage_location, organization: organization) }
+          let(:params) do
+            {
+              organization_name: organization.id,
+              distribution: {
+                partner_id: partner.id,
+                storage_location_id: storage_location.id,
+                line_items_attributes:
+                  {
+                    "0": { item_id: storage_location.items.first.id, quantity: 18 },
+                    "1": { item_id: storage_location.items.second.id, quantity: 15 }
+                  }
+              }
+            }
+          end
+          before do
+            TestInventory.create_inventory(organization, {
+              storage_location.id => {
+                first_item.id => 20,
+                second_item.id => 20
+              }
+            })
+          end
+          it "displays an error for both minimum and recommended quantity for an organization" do
+            expect(subject).to have_http_status(:redirect)
+            expect(flash[:notice]).to eq("Distribution created!")
+            expect(flash[:alert]).to include("The following items have fallen below the recommended on hand quantity, bank-wide: Item 2")
+            expect(flash[:alert]).to include("The following items have fallen below the minimum on hand quantity, bank-wide: Item 1")
+          end
         end
       end
 
-      context "multiple line_items that have inventory quantity below minimum quantity" do
+      context "multiple line_items that have inventory quantity below minimum quantity for an organization" do
         let(:item1) { create(:item, name: "Item 1", organization: organization, on_hand_minimum_quantity: 5, on_hand_recommended_quantity: 10) }
         let(:item2) { create(:item, name: "Item 2", organization: organization, on_hand_minimum_quantity: 5, on_hand_recommended_quantity: 10) }
         let(:storage_location) { create(:storage_location, organization: organization) }
@@ -67,14 +148,13 @@
         it "redirects with a flash notice and a flash error" do
           expect(subject).to have_http_status(:redirect)
           expect(flash[:notice]).to eq("Distribution created!")
-          expect(flash[:error]).to include("The following items have fallen below the minimum on hand quantity")
-          expect(flash[:error]).to include("Item 1")
-          expect(flash[:error]).to include("Item 2")
-          expect(flash[:alert]).to be_nil
+          expect(flash[:alert]).to include("The following items have fallen below the minimum on hand quantity, bank-wide")
+          expect(flash[:alert]).to include("Item 1")
+          expect(flash[:alert]).to include("Item 2")
         end
       end
 
-      context "multiple line_items that have inventory quantity below recommended quantity" do
+      context "multiple line_items that have inventory quantity below recommended quantity for an organization" do
         let(:item1) { create(:item, name: "Item 1", organization: organization, on_hand_recommended_quantity: 5) }
         let(:item2) { create(:item, name: "Item 2", organization: organization, on_hand_recommended_quantity: 5) }
         let(:storage_location) { create(:storage_location, organization: organization) }
@@ -106,7 +186,7 @@
         it "redirects with a flash notice and a flash alert" do
           expect(subject).to have_http_status(:redirect)
           expect(flash[:notice]).to eq("Distribution created!")
-          expect(flash[:alert]).to eq("The following items have fallen below the recommended on hand quantity: Item 1, Item 2")
+          expect(flash[:alert]).to eq("The following items have fallen below the recommended on hand quantity, bank-wide: Item 1, Item 2")
         end
       end
 
@@ -136,7 +216,7 @@
     end
 
     describe "PUT #update" do
-      context "when distribution causes inventory quantity to be below recommended quantity" do
+      context "when distribution causes inventory quantity to be below recommended quantity for an organization" do
         let(:item1) { create(:item, name: "Item 1", organization: organization, on_hand_recommended_quantity: 5) }
         let(:item2) { create(:item, name: "Item 2", organization: organization, on_hand_recommended_quantity: 5) }
         let(:storage_location) { create(:storage_location, organization: organization) }
@@ -169,11 +249,11 @@
         it "redirects with a flash notice and a flash error" do
           expect(subject).to have_http_status(:redirect)
           expect(flash[:notice]).to eq("Distribution updated!")
-          expect(flash[:alert]).to eq("The following items have fallen below the recommended on hand quantity: Item 1, Item 2")
+          expect(flash[:alert]).to eq("The following items have fallen below the recommended on hand quantity, bank-wide: Item 1, Item 2")
         end
       end
 
-      context "when distribution causes inventory quantity to be below minimum quantity" do
+      context "when distribution causes inventory quantity to be below minimum quantity for an organization" do
         let(:item1) { create(:item, name: "Item 1", organization: organization, on_hand_minimum_quantity: 5) }
         let(:item2) { create(:item, name: "Item 2", organization: organization, on_hand_minimum_quantity: 5) }
         let(:storage_location) { create(:storage_location) }
@@ -206,8 +286,7 @@
         it "redirects with a flash notice and a flash error" do
           expect(subject).to have_http_status(:redirect)
           expect(flash[:notice]).to eq("Distribution updated!")
-          expect(flash[:error]).to eq("The following items have fallen below the minimum on hand quantity: Item 1, Item 2")
-          expect(flash[:alert]).to be_nil
+          expect(flash[:alert]).to eq("The following items have fallen below the minimum on hand quantity, bank-wide: Item 1, Item 2")
         end
       end