Move Transfer.storage_locations_transferred_to/from_in to StorageLoca… (

#4733)

* Move Transfer.storage_locations_transferred_to/from_in to StorageLocation model

* Change with_transfers_to/from class methods to be scopes

app/controllers/transfers_controller.rb

@@ -11,8 +11,8 @@ def index
                                      .during(helpers.selected_range)
     @selected_from = filter_params[:from_location]
     @selected_to = filter_params[:to_location]
-    @from_storage_locations = Transfer.storage_locations_transferred_from_in(current_organization)
-    @to_storage_locations = Transfer.storage_locations_transferred_to_in(current_organization)
+    @from_storage_locations = StorageLocation.with_transfers_from(current_organization)
+    @to_storage_locations = StorageLocation.with_transfers_to(current_organization)
     respond_to do |format|
       format.html
       format.csv { send_data Transfer.generate_csv(@transfers), filename: "Transfers-#{Time.zone.today}.csv" }

app/models/storage_location.rb

@@ -34,11 +34,11 @@ class StorageLocation < ApplicationRecord
   has_many :distributions, dependent: :destroy
   has_many :transfers_from, class_name: "Transfer",
                             inverse_of: :from,
-                            foreign_key: :id,
+                            foreign_key: :from_id,
                             dependent: :destroy
   has_many :transfers_to, class_name: "Transfer",
                           inverse_of: :to,
-                          foreign_key: :id,
+                          foreign_key: :to_id,
                           dependent: :destroy
   has_many :kit_allocations, dependent: :destroy
 
@@ -55,6 +55,12 @@ class StorageLocation < ApplicationRecord
   scope :alphabetized, -> { order(:name) }
   scope :for_csv_export, ->(organization, *) { where(organization: organization) }
   scope :active_locations, -> { where(discarded_at: nil) }
+  scope :with_transfers_to, ->(organization) {
+    joins(:transfers_to).where(organization_id: organization.id).distinct.order(:name)
+  }
+  scope :with_transfers_from, ->(organization) {
+    joins(:transfers_from).where(organization_id: organization.id).distinct.order(:name)
+  }
 
   # @param organization [Organization]
   # @param inventory [View::Inventory]

app/models/transfer.rb

@@ -29,14 +29,6 @@ class Transfer < ApplicationRecord
   }
   scope :during, ->(range) { where(created_at: range) }
 
-  def self.storage_locations_transferred_to_in(organization)
-    includes(:to).where(organization_id: organization.id).distinct(:to_id).collect(&:to).uniq.sort_by(&:name)
-  end
-
-  def self.storage_locations_transferred_from_in(organization)
-    includes(:from).where(organization_id: organization.id).distinct(:from_id).collect(&:from).uniq.sort_by(&:name)
-  end
-
   validates :from, :to, :organization, presence: true
   validate :storage_locations_belong_to_organization
   validate :storage_locations_must_be_different

spec/models/storage_location_spec.rb

@@ -52,6 +52,32 @@
       expect(results.length).to eq(1)
       expect(results.first.discarded_at).to be_nil
     end
+
+    it "->with_transfers_to yields storage locations with transfers to an organization" do
+      storage_location1 = create(:storage_location, name: "loc1", organization: organization)
+      storage_location2 = create(:storage_location, name: "loc2", organization: organization)
+      storage_location3 = create(:storage_location, name: "loc3", organization: organization)
+      storage_location4 = create(:storage_location, name: "loc4", organization: create(:organization))
+      storage_location5 = create(:storage_location, name: "loc5", organization: storage_location4.organization)
+      create(:transfer, from: storage_location3, to: storage_location1, organization: organization)
+      create(:transfer, from: storage_location3, to: storage_location2, organization: organization)
+      create(:transfer, from: storage_location5, to: storage_location4, organization: storage_location4.organization)
+
+      expect(StorageLocation.with_transfers_to(organization).to_a).to match_array([storage_location1, storage_location2])
+    end
+
+    it "->with_transfers_from yields storage locations with transfers from an organization" do
+      storage_location1 = create(:storage_location, name: "loc1", organization: organization)
+      storage_location2 = create(:storage_location, name: "loc2", organization: organization)
+      storage_location3 = create(:storage_location, name: "loc3", organization: organization)
+      storage_location4 = create(:storage_location, name: "loc4", organization: create(:organization))
+      storage_location5 = create(:storage_location, name: "loc5", organization: storage_location4.organization)
+      create(:transfer, from: storage_location3, to: storage_location1, organization: organization)
+      create(:transfer, from: storage_location3, to: storage_location2, organization: organization)
+      create(:transfer, from: storage_location5, to: storage_location4, organization: storage_location4.organization)
+
+      expect(StorageLocation.with_transfers_from(organization).to_a).to match_array([storage_location3])
+    end
   end
 
   context "Methods >" do

spec/models/transfer_spec.rb

@@ -69,21 +69,6 @@
     end
   end
 
-  context "Methods >" do
-    it "`self.storage_locations_transferred_to` and `..._from` constrains appropriately" do
-      storage_location1 = create(:storage_location, name: "loc1", organization: organization)
-      storage_location2 = create(:storage_location, name: "loc2", organization: organization)
-      storage_location3 = create(:storage_location, name: "loc3", organization: organization)
-      storage_location4 = create(:storage_location, name: "loc4", organization: create(:organization))
-      storage_location5 = create(:storage_location, name: "loc5", organization: storage_location4.organization)
-      create(:transfer, from: storage_location3, to: storage_location1, organization: organization)
-      create(:transfer, from: storage_location3, to: storage_location2, organization: organization)
-      create(:transfer, from: storage_location5, to: storage_location4, organization: storage_location4.organization)
-      expect(Transfer.storage_locations_transferred_to_in(organization).to_a).to match_array([storage_location1, storage_location2])
-      expect(Transfer.storage_locations_transferred_from_in(organization).to_a).to match_array([storage_location3])
-    end
-  end
-
   describe "versioning" do
     it { is_expected.to be_versioned }
   end