Fix inventory quantity errors to be tied to organization, not storage location (Fixes #4647)

app/controllers/distributions_controller.rb

@@ -316,11 +316,14 @@ 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
+    if inventory_check_result.minimum_alert.present? && inventory_check_result.recommended_alert.present?
+      flash[:alert] = inventory_check_result.minimum_alert
+      flash[:alert] += "\n"
+      flash[:alert] += inventory_check_result.recommended_alert
+    elsif inventory_check_result.minimum_alert.present?
+      flash[:alert] = inventory_check_result.minimum_alert
+    elsif inventory_check_result.recommended_alert.present?
+      flash[:alert] = inventory_check_result.recommended_alert
     end
   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,36 @@
         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, :with_items_mixed, item: first_item, second_item: second_item, item_quantity: 20, 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
+          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 +140,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 +178,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 +208,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 +241,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 +278,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
 

spec/factories/storage_locations.rb

@@ -59,5 +59,38 @@
         end
       end
     end
+
+    trait :with_items_mixed do
+      with_items
+
+      transient do
+        second_item_quantity { 100 }
+        second_item { nil }
+      end
+
+      after(:create) do |storage_location, evaluator|
+        if evaluator.second_item.nil? && !evaluator.second_item_count.zero?
+          second_item_count = evaluator.item_count
+
+          TestInventory.create_inventory(
+            storage_location.organization, {
+              storage_location.id => (0...second_item_count).to_h do
+                second_item = create(:second_item, organization_id: storage_location.organization_id)
+                [second_item.id, evaluator.item_quantity]
+              end
+            }
+          )
+        elsif evaluator.second_item
+          second_item = evaluator.second_item
+          second_item.save if second_item.new_record?
+          TestInventory.create_inventory(
+            storage_location.organization,
+            {
+              storage_location.id => {second_item.id => evaluator.item_quantity}
+            }
+          )
+        end
+      end
+    end
   end
 end