Merge pull request #4939 from coalest/cache-inventory-calc-on-dashboard

Perf: Cache inventory calculation on dashboard

Author: cielf

app/controllers/dashboard_controller.rb

@@ -3,7 +3,7 @@ class DashboardController < ApplicationController
   respond_to :html, :js
 
   def index
-    @org_stats = OrganizationStats.new(current_organization)
+    @org_stats = OrganizationStats.new(current_organization, inventory)
     @partners_awaiting_review = current_organization.partners.awaiting_review
     @outstanding_requests = current_organization
       .ordered_requests
@@ -12,9 +12,15 @@ def index
       .order(:created_at)
       .limit(25)
 
-    @low_inventory_report = LowInventoryQuery.call(current_organization)
+    @low_inventory_report = LowInventoryQuery.call(current_organization, inventory)
 
     # passing nil here filters the announcements that didn't come from an organization
     @broadcast_announcements = BroadcastAnnouncement.filter_announcements(nil)
   end
+
+  private
+
+  def inventory
+    @inventory ||= View::Inventory.new(current_organization.id)
+  end
 end

app/models/organization_stats.rb

@@ -7,8 +7,9 @@ class OrganizationStats
            :donation_sites,
            to: :current_organization, allow_nil: true
 
-  def initialize(organization)
+  def initialize(organization, inventory)
     @current_organization = organization
+    @inventory = inventory
   end
 
   def partners_added
@@ -23,14 +24,13 @@ def donation_sites_added
     donation_sites&.length || 0
   end
 
-  def locations_with_inventory
-    return [] unless storage_locations
+  def num_locations_with_inventory
+    return 0 unless storage_locations
 
-    inventory = View::Inventory.new(current_organization.id)
-    storage_locations.select { |loc| inventory.quantity_for(storage_location: loc.id).positive? }
+    storage_locations.count { |loc| inventory.quantity_for(storage_location: loc.id).positive? }
   end
 
   private
 
-  attr_reader :current_organization
+  attr_reader :current_organization, :inventory
 end

app/queries/low_inventory_query.rb

@@ -1,8 +1,9 @@
 class LowInventoryQuery
   LowInventoryItem = Data.define(:id, :name, :on_hand_minimum_quantity, :on_hand_recommended_quantity, :total_quantity)
 
-  def self.call(organization)
-    inventory = View::Inventory.new(organization.id)
+  def self.call(organization, inventory = View::Inventory.new(organization.id))
+    return [] if organization.blank?
+
     items = inventory.all_items.uniq(&:item_id)
 
     low_inventory_items = []

app/views/dashboard/_getting_started_prompt.html.erb

@@ -2,7 +2,7 @@
 <% partner_criteria_met = org_stats.partners_added > 0 %>
 <% location_criteria_met = org_stats.storage_locations_added > 0 %>
 <% donation_criteria_met = org_stats.donation_sites_added > 0 %>
-<% inventory_criteria_met = org_stats.locations_with_inventory.length > 0 %>
+<% inventory_criteria_met = org_stats.num_locations_with_inventory > 0 %>
 <% criterias = [ location_criteria_met, partner_criteria_met, donation_criteria_met, inventory_criteria_met ] %>
 <% current_step = criterias.find_index(false) %>
 
@@ -116,7 +116,7 @@
                     <% else %>
                       <i class="fa fa-pie-chart fa-2x" aria-hidden="true"></i>
                     <% end %>
-                    <h4><%= pluralize(org_stats.locations_with_inventory.length, 'Storage Location') %> with Inventory</h4>
+                    <h4><%= pluralize(org_stats.num_locations_with_inventory, 'Storage Location') %> with Inventory</h4>
                   </div>
 
                   <div class="org-stats-desc">

spec/models/organization_stats_spec.rb

@@ -2,12 +2,14 @@
 #
 RSpec.describe OrganizationStats, type: :model do
   let(:current_org) { create(:organization) }
+  let(:inventory) { View::Inventory.new(current_org.id) }
 
-  subject { described_class.new(current_org) }
+  subject { described_class.new(current_org, inventory) }
 
   describe "partners_added method >" do
     context "current org is nil >" do
       let(:current_org) { nil }
+      let(:inventory) { nil }
 
       it "should return 0" do
         expect(subject.partners_added).to eq(0)
@@ -28,6 +30,7 @@
   describe "storage_locations_added method >" do
     context "current org is nil >" do
       let(:current_org) { nil }
+      let(:inventory) { nil }
 
       it "should return 0" do
         expect(subject.storage_locations_added).to eq(0)
@@ -48,6 +51,7 @@
   describe "donation_sites_added method >" do
     context "current org is nil >" do
       let(:current_org) { nil }
+      let(:inventory) { nil }
 
       it "should return 0" do
         expect(subject.donation_sites_added).to eq(0)
@@ -65,12 +69,13 @@
     end
   end
 
-  describe "locations_with_inventory method >" do
+  describe "num_locations_with_inventory method >" do
     context "current org is nil >" do
       let(:current_org) { nil }
+      let(:inventory) { nil }
 
       it "should return an empty array" do
-        expect(subject.locations_with_inventory).to eq([])
+        expect(subject.num_locations_with_inventory).to eq(0)
       end
     end
 
@@ -87,7 +92,7 @@
       end
 
       it "should return storage location" do
-        expect(subject.locations_with_inventory).to include(storage_location_1)
+        expect(subject.num_locations_with_inventory).to eq(1)
       end
     end
 
@@ -96,7 +101,7 @@
       let(:storage_locations) { [storage_location_1] }
 
       it "should return an empty array" do
-        expect(subject.locations_with_inventory).to eq([])
+        expect(subject.num_locations_with_inventory).to eq(0)
       end
     end
   end