Clean up some model methods (#5042)

coalest · web-flow · commit 397c4dd670b4 · 2025-02-25T19:16:59.000-05:00
* Clean up some model methods

* Remove kit allocations model

* Remove has_many kit_allocations from storage location model
diff --git a/app/controllers/barcode_items_controller.rb b/app/controllers/barcode_items_controller.rb
@@ -2,7 +2,7 @@
 # anomaly here is the :find action, which has some special logic built-in to it, see the comments below.
 class BarcodeItemsController < ApplicationController
   def index
-    @items = Item.gather_items(current_organization, @global)
+    @items = current_organization.items.joins(:barcode_items)
     @base_items = BaseItem.alphabetized
     @selected_barcodeable_id = filter_params[:barcodeable_id]
     @selected_partner_key = filter_params[:by_item_partner_key]
diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb
@@ -69,7 +69,7 @@ def show
     @inventory = View::Inventory.new(current_organization.id)
     storage_location_ids = @inventory.storage_locations_for_item(@item.id)
     @storage_locations_containing = StorageLocation.find(storage_location_ids)
-    @barcodes_for = current_organization.items.barcodes_for(@item)
+    @barcodes_for = BarcodeItem.where(barcodeable_id: @item.id)
   end
 
   def update
diff --git a/app/models/donation.rb b/app/models/donation.rb
@@ -105,13 +105,6 @@ def details
     end
   end
 
-  def remove(item)
-    # doing this will handle either an id or an object
-    item_id = item.to_i
-    line_item = line_items.find_by(item_id: item_id)
-    line_item&.destroy
-  end
-
   def money_raised_in_dollars
     money_raised.to_d / 100
   end
diff --git a/app/models/errors.rb b/app/models/errors.rb
@@ -43,18 +43,6 @@ def message
     end
   end
 
-  class StorageLocationDoesNotMatch < StandardError
-    def message
-      "Storage location kit doesn't match"
-    end
-  end
-
-  class KitAllocationNotExists < StandardError
-    def message
-      "KitAllocation not found for given kit"
-    end
-  end
-
   class InventoryAlreadyHasItems < StandardError
     def message
       "Could not complete action: inventory already has items stored"
diff --git a/app/models/item.rb b/app/models/item.rb
@@ -106,14 +106,6 @@ class Item < ApplicationRecord
 
   before_destroy :validate_destroy, prepend: true
 
-  def self.barcoded_items
-    joins(:barcode_items).order(:name).group(:id)
-  end
-
-  def self.barcodes_for(item)
-    BarcodeItem.where(barcodeable_id: item.id)
-  end
-
   def self.reactivate(item_ids)
     item_ids = Array.wrap(item_ids)
     Item.where(id: item_ids).find_each { |item| item.update(active: true) }
@@ -175,16 +167,8 @@ def other?
     partner_key == "other"
   end
 
-  def self.gather_items(current_organization, global = false)
-    if global
-      where(id: current_organization.barcode_items.all.pluck(:barcodeable_id))
-    else
-      where(id: current_organization.barcode_items.pluck(:barcodeable_id))
-    end
-  end
   # Convenience method so that other methods can be simplified to
   # expect an id or an Item object
-
   def to_i
     id
   end
diff --git a/app/models/kit_allocation.rb b/app/models/kit_allocation.rb
diff --git a/app/models/manufacturer.rb b/app/models/manufacturer.rb
@@ -37,10 +37,4 @@ def self.by_donation_date(count = 10, date_range = nil)
       .order('donation_date DESC')
       .limit(count)
   end
-
-  private
-
-  def exists_in_org?
-    organization.manufacturers.where(name: name).exists?
-  end
 end
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
@@ -79,13 +79,6 @@ def amount_spent_in_dollars
     amount_spent.dollars.to_f
   end
 
-  def remove(item)
-    # doing this will handle either an id or an object
-    item_id = item.to_i
-    line_item = line_items.find_by(item_id: item_id)
-    line_item&.destroy
-  end
-
   def self.organization_summary_by_dates(organization, date_range)
     purchases = where(organization: organization).during(date_range)
 
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
@@ -42,7 +42,6 @@ class StorageLocation < ApplicationRecord
                           inverse_of: :to,
                           foreign_key: :to_id,
                           dependent: :destroy
-  has_many :kit_allocations, dependent: :destroy
 
   validates :name, :address, presence: true
   validates :warehouse_type, inclusion: { in: WAREHOUSE_TYPES },
diff --git a/spec/factories/kit_allocations.rb b/spec/factories/kit_allocations.rb
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
@@ -148,27 +148,6 @@
   end
 
   context "Methods >" do
-    describe "remove" do
-      let!(:donation) { create(:donation, :with_items) }
-
-      it "removes the item from the donation" do
-        item_id = donation.line_items.last.item_id
-        expect do
-          donation.remove(item_id)
-        end.to change { donation.line_items.count }.by(-1)
-      end
-
-      it "works with either an id or an object" do
-      end
-
-      it "fails gracefully if the item doesn't exist" do
-        item_id = create(:item).id
-        expect do
-          donation.remove(item_id)
-        end.not_to change { donation.line_items.count }
-      end
-    end
-
     describe "money_raised" do
       it "tracks the money raised in a donation" do
         donation = create(:donation, :with_items, money_raised: 100)
diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb
@@ -223,23 +223,6 @@
   end
 
   context "Methods >" do
-    describe "barcodes_for" do
-      it "retrieves all BarcodeItems associated with an item" do
-        item = create(:item)
-        barcode_item = create(:barcode_item, barcodeable: item)
-        create(:barcode_item)
-        expect(Item.barcodes_for(item).first).to eq(barcode_item)
-      end
-    end
-    describe "barcoded_items >" do
-      it "returns a collection of items that have barcodes associated with them" do
-        create_list(:item, 3)
-        create(:barcode_item, item: Item.first)
-        create(:barcode_item, item: Item.last)
-        expect(Item.barcoded_items.length).to eq(2)
-      end
-    end
-
     describe '#can_deactivate_or_delete?' do
       let(:item) { create(:item, organization: organization) }
       let(:storage_location) { create(:storage_location, organization: organization) }
diff --git a/spec/models/kit_allocation_spec.rb b/spec/models/kit_allocation_spec.rb
diff --git a/spec/models/manufacturer_spec.rb b/spec/models/manufacturer_spec.rb
@@ -89,17 +89,6 @@
     end
   end
 
-  context "Private Methods" do
-    describe "#exists_in_org?" do
-      let(:organization) { create(:organization) }
-
-      it "returns true if manufacturer exists in an organization" do
-        manufacturer = create(:manufacturer, organization_id: organization.id)
-        expect(manufacturer.send(:exists_in_org?)).to eq(true)
-      end
-    end
-  end
-
   describe "versioning" do
     it { is_expected.to be_versioned }
   end
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
@@ -125,24 +125,6 @@
   end
 
   context "Methods >" do
-    describe "remove" do
-      let!(:purchase) { create(:purchase, :with_items) }
-
-      it "removes the item from the purchase" do
-        item_id = purchase.line_items.last.item_id
-        expect do
-          purchase.remove(item_id)
-        end.to change { purchase.line_items.count }.by(-1)
-      end
-
-      it "fails gracefully if the item doesn't exist" do
-        item_id = create(:item).id
-        expect do
-          purchase.remove(item_id)
-        end.not_to change { purchase.line_items.count }
-      end
-    end
-
     describe "storage_view" do
       let!(:purchase) { create(:purchase, :with_items) }
       it "returns name of storage location" do
