Merge pull request #4954 from coalest/refactor-total-item-calculations-for-performance

Perf: Refactor total items calculations

Author: cielf

app/controllers/manufacturers_controller.rb

@@ -1,6 +1,6 @@
 class ManufacturersController < ApplicationController
   def index
-    @manufacturers = current_organization.manufacturers.includes(:donations).all.alphabetized
+    @manufacturers = current_organization.manufacturers.with_volumes.alphabetized
   end
 
   def create

app/controllers/product_drive_participants_controller.rb

@@ -6,7 +6,7 @@ class ProductDriveParticipantsController < ApplicationController
   # TODO: Should there be a :destroy action for this?
 
   def index
-    @product_drive_participants = current_organization.product_drive_participants.includes(:donations).all.order(:business_name)
+    @product_drive_participants = current_organization.product_drive_participants.with_volumes.order(:business_name)
 
     respond_to do |format|
       format.html

app/controllers/vendors_controller.rb

@@ -3,7 +3,7 @@ class VendorsController < ApplicationController
   include Importable
 
   def index
-    @vendors = current_organization.vendors.includes(:purchases).all.alphabetized
+    @vendors = current_organization.vendors.with_volumes.alphabetized
 
     respond_to do |format|
       format.html

app/models/manufacturer.rb

@@ -21,10 +21,11 @@ class Manufacturer < ApplicationRecord
 
   scope :alphabetized, -> { order(:name) }
 
-  def volume
-    # returns 0 instead of nil when Manufacturer exists without any donations
-    donations.joins(:line_items).sum(:quantity)
-  end
+  scope :with_volumes, -> {
+    left_joins(donations: :line_items)
+      .select("manufacturers.*, SUM(COALESCE(line_items.quantity, 0)) AS volume")
+      .group(:id)
+  }
 
   def self.by_donation_date(count = 10, date_range = nil)
     # selects manufacturers that have donation qty > 0 in the provided date range

app/models/product_drive_participant.rb

@@ -30,6 +30,11 @@ class ProductDriveParticipant < ApplicationRecord
   validates :comment, length: { maximum: 500 }
 
   scope :alphabetized, -> { order(:contact_name) }
+  scope :with_volumes, -> {
+    left_joins(donations: :line_items)
+      .select("product_drive_participants.*, SUM(COALESCE(line_items.quantity, 0)) AS volume")
+      .group(:id)
+  }
 
   def volume
     donations.map { |d| d.line_items.total }.reduce(:+)

app/models/vendor.rb

@@ -27,7 +27,9 @@ class Vendor < ApplicationRecord
 
   scope :alphabetized, -> { order(:business_name) }
 
-  def volume
-    purchases.map { |d| d.line_items.total }.reduce(:+)
-  end
+  scope :with_volumes, -> {
+    left_joins(purchases: :line_items)
+      .select("vendors.*, SUM(COALESCE(line_items.quantity, 0)) AS volume")
+      .group(:id)
+  }
 end

spec/models/manufacturer_spec.rb

@@ -23,30 +23,37 @@
     end
   end
 
-  context "Methods" do
-    describe "volume" do
+  context "Scopes" do
+    describe "with_volumes" do
+      subject { described_class.with_volumes }
+
       it "retrieves the amount of product that has been donated by manufacturer" do
         mfg = create(:manufacturer)
         create(:donation, :with_items, item_quantity: 15, source: Donation::SOURCES[:manufacturer], manufacturer: mfg)
-        expect(mfg.volume).to eq(15)
+
+        expect(subject.first.volume).to eq(15)
       end
 
       it "retrieves the amount of product that has been donated by manufacturer from multiple donations" do
         mfg = create(:manufacturer)
         create(:donation, :with_items, item_quantity: 15, source: Donation::SOURCES[:manufacturer], manufacturer: mfg)
         create(:donation, :with_items, item_quantity: 10, source: Donation::SOURCES[:manufacturer], manufacturer: mfg)
-        expect(mfg.volume).to eq(25)
+
+        expect(subject.first.volume).to eq(25)
       end
 
       it "ignores the amount of product from other manufacturers" do
         mfg = create(:manufacturer)
         mfg2 = create(:manufacturer)
         create(:donation, :with_items, item_quantity: 5, source: Donation::SOURCES[:manufacturer], manufacturer: mfg)
         create(:donation, :with_items, item_quantity: 10, source: Donation::SOURCES[:manufacturer], manufacturer: mfg2)
-        expect(mfg.volume).to eq(5)
+
+        expect(subject.first.volume).to eq(5)
       end
     end
+  end
 
+  context "Methods" do
     describe "by_donation_date" do
       before do
         # Prepare manufacturers with donations for tests

spec/models/product_drive_participant_spec.rb

@@ -38,6 +38,19 @@
     end
   end
 
+  context "Scopes" do
+    describe "with_volumes" do
+      subject { described_class.with_volumes }
+      it "retrieves the amount of product that has been donated by participant" do
+        dd = create(:product_drive)
+        ddp = create(:product_drive_participant)
+        create(:donation, :with_items, item_quantity: 10, source: Donation::SOURCES[:product_drive], product_drive: dd, product_drive_participant: ddp)
+
+        expect(subject.first.volume).to eq(10)
+      end
+    end
+  end
+
   context "Methods" do
     describe "volume" do
       it "retrieves the amount of product that has been donated by participant" do

spec/models/vendor_spec.rb

@@ -26,12 +26,15 @@
     end
   end
 
-  context "Methods" do
-    describe "volume" do
+  context "Scopes" do
+    describe "with_volumes" do
+      subject { described_class.with_volumes }
+
       it "retrieves the amount of product that has been bought from this vendor" do
         vendor = create(:vendor)
         create(:purchase, :with_items, item_quantity: 10, amount_spent_in_cents: 1, vendor: vendor)
-        expect(vendor.volume).to eq(10)
+
+        expect(subject.first.volume).to eq(10)
       end
     end
   end