Perf: Reduce number of queries and object allocations in request index page (#4920)

Author: coalest

app/controllers/requests_controller.rb

@@ -9,12 +9,12 @@ def index
                 .during(helpers.selected_range)
                 .class_filter(filter_params)
     @unfulfilled_requests_count = current_organization.requests.where(status: [:pending, :started]).count
-    @paginated_requests = @requests.page(params[:page])
+    @paginated_requests = @requests.includes(:partner).page(params[:page])
     @calculate_product_totals = RequestsTotalItemsService.new(requests: @requests).calculate
-    @items = current_organization.items.alphabetized
-    @partners = current_organization.partners.order(:name)
+    @items = current_organization.items.alphabetized.select(:id, :name)
+    @partners = current_organization.partners.alphabetized.select(:id, :name)
     @statuses = Request.statuses.transform_keys(&:humanize)
-    @partner_users = User.where(id: @paginated_requests.pluck(:partner_user_id))
+    @partner_users = User.where(id: @paginated_requests.map(&:partner_user_id)).select(:id, :name, :email)
     @request_types = Request.request_types.transform_keys(&:humanize)
     @selected_request_type = filter_params[:by_request_type]
     @selected_request_item = filter_params[:by_request_item_id]

app/models/partners/item_request.rb

@@ -37,12 +37,10 @@ def request_unit_is_supported
     end
 
     def name_with_unit(quantity_override = nil)
-      if item
-        if Flipper.enabled?(:enable_packs) && request_unit.present?
-          "#{name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
-        else
-          name
-        end
+      if Flipper.enabled?(:enable_packs) && request_unit.present?
+        "#{name} - #{request_unit.pluralize(quantity_override || quantity.to_i)}"
+      else
+        name
       end
     end
   end

app/services/exports/export_request_service.rb

@@ -97,7 +97,7 @@ def build_row_data(request)
       row += Array.new(item_headers.size, 0)
 
       request.item_requests.each do |item_request|
-        item_name = item_request.name_with_unit(0) || DELETED_ITEMS_COLUMN_HEADER
+        item_name = item_request.item.present? ? item_request.name_with_unit(0) : DELETED_ITEMS_COLUMN_HEADER
         item_column_idx = headers_with_indexes[item_name]
         row[item_column_idx] ||= 0
         row[item_column_idx] += item_request.quantity.to_i

app/services/requests_total_items_service.rb

@@ -1,11 +1,9 @@
 class RequestsTotalItemsService
   def initialize(requests:)
-    @requests = requests.includes(item_requests: {item: :request_units})
+    @requests = requests
   end
 
   def calculate
-    return unless requests
-
     totals = Hash.new(0)
     item_requests.each do |item_request|
       totals[item_request.name_with_unit] += item_request.quantity.to_i
@@ -16,9 +14,7 @@ def calculate
 
   private
 
-  attr_accessor :requests
-
   def item_requests
-    @item_requests ||= requests.flat_map(&:item_requests)
+    @item_requests ||= Partners::ItemRequest.where(request: @requests)
   end
 end

spec/services/requests_total_items_service_spec.rb

@@ -73,5 +73,21 @@
 
       it { is_expected.to be_blank }
     end
+
+    context 'when request item belongs to deleted item' do
+      let(:item) { create(:item, :with_unit, name: "Diaper", organization:, unit: "pack") }
+      let!(:requests) do
+        request = create(:request, :with_item_requests, request_items: [{"item_id" => item.id, "quantity" => 10, "request_unit" => "pack"}])
+        Request.where(id: request.id)
+      end
+
+      before do
+        item.destroy
+      end
+
+      it 'returns item with correct quantity calculated' do
+        expect(subject).to eq({"Diaper" => 10})
+      end
+    end
   end
 end