Remove unused inventory storage methods and related tests for cleaner code

Author: Olexii Kasianenko

app/controllers/storage_locations_controller.rb

@@ -163,7 +163,7 @@ def filter_params
   end
 
   def date_range
-    return unless filter_params[:date_range].present?
+    return if filter_params[:date_range].blank?
 
     date_range = filter_params[:date_range].split(" - ")
     start_date = Date.parse(date_range[0])

app/models/item.rb

@@ -69,7 +69,7 @@ class Item < ApplicationRecord
   scope :period_supplies, -> {
     where(reporting_category: [:pads, :tampons, :period_liners, :period_underwear, :period_other])
   }
-  scope :created_between, ->(start_date, end_date) { where(created_at: start_date..end_date) }
+
   enum :reporting_category, {
     adult_incontinence: "adult_incontinence",
     cloth_diapers: "cloth_diapers",
@@ -193,18 +193,6 @@ def sync_request_units!(unit_ids)
     end
   end
 
-  def quantity_in_storage(storage_location_id)
-    line_items.inventory_in_storage(storage_location_id).sum(:quantity)
-  end
-
-  def quantity_out_storage(storage_location_id)
-    line_items.inventory_out_storage(storage_location_id).sum(:quantity)
-  end
-
-  def quantity_change(storage_location_id)
-    quantity_in_storage(storage_location_id) - quantity_out_storage(storage_location_id)
-  end
-
   private
 
   def set_default_distribution_quantity

app/models/line_item.rb

@@ -25,28 +25,6 @@ class LineItem < ApplicationRecord
 
   scope :active, -> { joins(:item).where(items: { active: true }) }
 
-  scope :inventory_in_storage, ->(storage_location_id) do
-    joins("
-      LEFT OUTER JOIN donations ON donations.id = line_items.itemizable_id AND line_items.itemizable_type = 'Donation'
-      LEFT OUTER JOIN purchases ON purchases.id = line_items.itemizable_id AND line_items.itemizable_type = 'Purchase'
-      LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment'
-      LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'")
-      .where("donations.storage_location_id = :storage_location_id OR
-              purchases.storage_location_id = :storage_location_id OR
-              (adjustments.storage_location_id = :storage_location_id and line_items.quantity < 0) OR
-              transfers.to_id = :storage_location_id", storage_location_id: storage_location_id)
-  end
-
-  scope :inventory_out_storage, ->(storage_location_id) do
-    joins("
-      LEFT OUTER JOIN distributions ON distributions.id = line_items.itemizable_id AND line_items.itemizable_type = 'Distribution'
-      LEFT OUTER JOIN adjustments ON adjustments.id = line_items.itemizable_id AND line_items.itemizable_type = 'Adjustment'
-      LEFT OUTER JOIN transfers ON transfers.id = line_items.itemizable_id AND line_items.itemizable_type = 'Transfer'")
-      .where("distributions.storage_location_id = :storage_location_id OR
-              (adjustments.storage_location_id = :storage_location_id and line_items.quantity > 0) OR
-              transfers.from_id = :storage_location_id", storage_location_id: storage_location_id)
-  end
-
   delegate :name, to: :item
 
   # Used in a distribution that was initialized from a request. The `item_request` will be

spec/models/item_spec.rb

@@ -175,21 +175,6 @@
         expect(ai_items).to_not include(child_disposable_item, child_cloth_item, liner_item)
       end
     end
-
-    describe "->created_between" do
-      let!(:start_date) { 2.days.ago }
-      let!(:end_date) { 1.day.ago }
-      let!(:item1) { create(:item, created_at: start_date, organization:) }
-      let!(:item2) { create(:item, created_at: end_date, organization:) }
-      let!(:item3) { create(:item, created_at: 3.days.ago, organization:) }
-      let(:items_in_range) { Item.created_between(start_date, end_date) }
-
-      it "returns items created within the specified date range" do
-        expect(items_in_range.count).to eq(2)
-        expect(items_in_range).to include(item1, item2)
-        expect(items_in_range).to_not include(item3)
-      end
-    end
   end
 
   describe "->period_supplies" do
@@ -402,23 +387,6 @@
         end
       end
     end
-
-    describe "#quantity_change" do
-      let(:storage_location) { create(:storage_location) }
-      let(:item) { create(:item) }
-
-      before do
-        create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location)
-        create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location)
-        create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location)
-        create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location)
-        create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location)
-      end
-
-      it "returns the total quantity change for the item" do
-        expect(item.quantity_change(storage_location.id)).to eq(5 + 10 - 5 - 2 + 3)
-      end
-    end
   end
 
   describe "default_quantity" do

spec/models/line_item_spec.rb

@@ -55,48 +55,6 @@
         end.to change { described_class.active.size }.by(1)
       end
     end
-
-    describe "->inventory_in_storage" do
-      let(:storage_location) { create(:storage_location) }
-      let(:item) { create(:item) }
-
-      before do
-        create(:donation, :with_items, item: item, item_quantity: 5, storage_location: storage_location)
-        create(:purchase, :with_items, item: item, item_quantity: 10, storage_location: storage_location)
-        create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location)
-        create(:transfer, :with_items, item: item, item_quantity: 3, from: create(:storage_location), to: storage_location)
-      end
-
-      it "returns line items that are in the specified storage location" do
-        expect(described_class.inventory_in_storage(storage_location.id).count).to eq(4)
-      end
-
-      it "returns an empty collection if no line items are in the specified storage location" do
-        other_storage_location = create(:storage_location)
-        expect(described_class.inventory_in_storage(other_storage_location.id)).to be_empty
-      end
-    end
-
-    describe "->inventory_out_storage" do
-      let(:storage_location) { create(:storage_location) }
-      let(:item) { create(:item) }
-
-      before do
-        create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location)
-        create(:distribution, :with_items, item: item, item_quantity: 5, storage_location: storage_location)
-        create(:adjustment, :with_items, item: item, item_quantity: -2, storage_location: storage_location)
-        create(:transfer, :with_items, item: item, item_quantity: 3, from: storage_location, to: create(:storage_location))
-      end
-
-      it "returns line items that are out of the specified storage location" do
-        expect(described_class.inventory_out_storage(storage_location.id).count).to eq(2)
-      end
-
-      it "returns an empty collection if no line items are out of the specified storage location" do
-        other_storage_location = create(:storage_location)
-        expect(described_class.inventory_out_storage(other_storage_location.id)).to be_empty
-      end
-    end
   end
 
   describe 'Methods >' do

spec/system/storage_location_system_spec.rb

@@ -184,6 +184,30 @@
   context "when viewing an existing storage location" do
     let(:items) { create_list(:item, 2) }
     let!(:storage_location) { create(:storage_location, name: "here") }
+    let(:result) do
+      [
+        {
+          item_id: items[0].id,
+          item_name: items[0].name,
+          quantity_in: 10,
+          quantity_out: 5,
+          change: 5,
+          total_quantity_in: 16,
+          total_quantity_out: 7,
+          total_change: 9
+        },
+        {
+          item_id: items[1].id,
+          item_name: items[1].name,
+          quantity_in: 6,
+          quantity_out: 2,
+          change: 4,
+          total_quantity_in: 16,
+          total_quantity_out: 7,
+          total_change: 9
+        }
+      ].map(&:with_indifferent_access)
+    end
     subject { storage_location_path(storage_location.id) }
 
     context "Inventory Flow Tab" do
@@ -200,27 +224,41 @@
 
       it "shows the inventory flow for the storage location" do
         within("#custom-tabs-inventory-flow table tbody") do
-          items.each do |item|
-            row = find(:css, "tr[id='#{item.id}']")
-            change_column_css = item.quantity_change(storage_location).negative? ? "td.modal-body-warning-text" : "td"
-            expect(row).to have_link(item.name, href: item_path(item.id))
-            expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id))
-            expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id))
-            expect(row).to have_css(change_column_css, text: item.quantity_change(storage_location.id))
+          result.each do |item|
+            row = find(:css, "tr[id='#{item[:item_id]}']")
+            change_column_css = item[:change].negative? ? "td.modal-body-warning-text" : "td"
+            expect(row).to have_link(item[:name], href: item_path(item[:item_id]))
+            expect(row).to have_css("td", text: item[:quantity_in])
+            expect(row).to have_css("td", text: item[:quantity_out])
+            expect(row).to have_css(change_column_css, text: item[:change])
           end
         end
         within("#custom-tabs-inventory-flow table tfoot") do
           expect(page).to have_css("td", text: "Total")
-          expect(page).to have_css("td", text: items.sum { |item| item.quantity_in_storage(storage_location.id) })
-          expect(page).to have_css("td", text: items.sum { |item| item.quantity_out_storage(storage_location.id) })
-          expect(page).to have_css("td", text: items.sum { |item| item.quantity_change(storage_location.id) })
+          expect(page).to have_css("td", text: result.first[:total_quantity_in])
+          expect(page).to have_css("td", text: result.first[:total_quantity_out])
+          expect(page).to have_css("td", text: result.first[:total_change])
         end
       end
 
       context "date range filter" do
         let!(:start_date) { 2.days.ago }
         let!(:end_date) { 1.day.ago }
         let!(:item) { create(:item, name: "Filtered Item", created_at: start_date) }
+        let(:result) do
+          [
+            {
+              item_id: item.id,
+              item_name: item.name,
+              quantity_in: 10,
+              quantity_out: 0,
+              change: 10,
+              total_quantity_in: 10,
+              total_quantity_out: 0,
+              total_change: 10
+            }
+          ].map(&:with_indifferent_access)
+        end
         before do
           create(:donation, :with_items, item: item, item_quantity: 10, storage_location: storage_location)
           fill_in "filters[date_range]", with: "#{start_date} - #{end_date}"
@@ -231,11 +269,11 @@
         it "filters the inventory flow by date range" do
           within("#custom-tabs-inventory-flow table tbody") do
             expect(page).to have_css("tr", count: 1)
-            row = find(:css, "tr[id='#{item.id}']")
-            expect(row).to have_link(item.name, href: item_path(item.id))
-            expect(row).to have_css("td", text: item.quantity_in_storage(storage_location.id))
-            expect(row).to have_css("td", text: item.quantity_out_storage(storage_location.id))
-            expect(row).to have_css("td", text: item.quantity_change(storage_location.id))
+            row = find(:css, "tr[id='#{result.first[:item_id]}']")
+            expect(row).to have_link(result.first[:name], href: item_path(result.first[:item_id]))
+            expect(row).to have_css("td", text: result.first[:quantity_in])
+            expect(row).to have_css("td", text: result.first[:quantity_out])
+            expect(row).to have_css("td", text: result.first[:change])
           end
         end
       end