5477 speed up distributions export

app/services/exports/export_distributions_csv_service.rb

@@ -7,11 +7,11 @@ class ExportDistributionsCSVService
     include DistributionHelper
     include ItemsHelper
 
-    def initialize(distributions:, organization:, filters: [])
-      # Currently, the @distributions are already loaded by the controllers that are delegating exporting
-      # to this service object; this is happening within the same request/response cycle, so it's already
-      # in memory, so we can pass that collection in directly. Should this be moved to a background / async
-      # job, we will need to pass in a collection of IDs instead.
+    def initialize(distributions:, organization:, filters: {})
+      # Currently, `distributions` is a ActiveRecord::Relation from the controllers that are delegating exporting
+      # to this service object; this is happening within the same request/response cycle, so we pass the query in
+      # directly. Should this be moved to a background / async job, we will need to pass in a collection of IDs instead.
+      # The distribution model objects are not loaded until the `#find_each` in `generate_csv_data`
       # Also, adding in a `filters` parameter to make the filters that have been used available to this
       # service object.
       @distributions = distributions
@@ -32,7 +32,7 @@ def generate_csv_data
       csv_data = []
 
       csv_data << base_headers + item_headers
-      distributions.each do |distribution|
+      distributions.find_each do |distribution|
         csv_data << build_row_data(distribution)
       end
 
@@ -147,18 +147,26 @@ def item_headers
     end
 
     def build_row_data(distribution)
-      row = base_table.values.map { |closure| closure.call(distribution) }
-
-      item_names.each do |item_name|
-        # We are doing this in-memory so that we can use the already-loaded line item records
-        line_items = distribution.line_items.select { |item| item.name == item_name }
-
-        row << line_items.sum(&:quantity)
-        row << Money.new(line_items.sum(&:value_per_line_item)) if @organization.include_in_kind_values_in_exported_files
-        row << line_items.map(&:has_packages).compact.sum.round(2) if @organization.include_packages_in_distribution_export
+      base_row = base_table.values.map { |closure| closure.call(distribution) }
+
+      grouped_line_items = distribution.line_items.group_by(&:name)
+      base_row + item_names.flat_map do |item_name|
+        line_items = grouped_line_items[item_name]
+        next missing_item unless line_items
+        [
+          line_items.sum(&:quantity),
+          (Money.new(line_items.sum(&:value_per_line_item)) if @organization.include_in_kind_values_in_exported_files),
+          (line_items.map(&:has_packages).compact.sum.round(2) if @organization.include_packages_in_distribution_export)
+        ].compact
       end
+    end
 
-      row
+    def missing_item
+      @missing_item ||= [
+        0,
+        ("0.00".freeze if @organization.include_in_kind_values_in_exported_files),
+        (0 if @organization.include_packages_in_distribution_export)
+      ].compact
     end
   end
 end

spec/controllers/distributions_controller_spec.rb

@@ -439,5 +439,29 @@
         end
       end
     end
+
+    describe "GET #index" do
+      context "when distribution has items updated for minimum quantity" do
+        let(:storage_location) { create(:storage_location, organization: organization) }
+        let(:distribution_count) { 5 }
+        let(:distributions) { create_list(:distribution, distribution_count, :with_items, item: items.sample, storage_location: storage_location, organization: organization) }
+        let(:items) { create_list(:item, 500, organization:, on_hand_minimum_quantity: 5) }
+
+        it "responds 200" do
+          get :index
+
+          expect(response).to have_http_status(:ok)
+        end
+
+        it "downloads a CSV" do
+          distributions
+          get :index, format: :csv
+
+          expect(response.header["Content-Type"]).to include "text/csv"
+          expect(response).to have_http_status(:ok)
+          expect(CSV.parse(response.body).size).to eq(distribution_count + 1)
+        end
+      end
+    end
   end
 end

spec/services/exports/export_distributions_csv_service_spec.rb

@@ -4,9 +4,10 @@
   let(:partner) { create(:partner, name: "first partner", email: "firstpartner@gmail.com", notes: "just a note.", organization_id: organization.id) }
   let(:include_in_kind_values) { false }
   let(:include_packages) { false }
+  let(:distributions_query) { Distribution.where(id: distributions.map(&:id)).includes(:partner, :storage_location, line_items: :item) } # match the query design used in DistributionsController and PartnersController
 
   describe '#generate_csv' do
-    subject { described_class.new(distributions: distributions, organization: organization, filters: filters).generate_csv }
+    subject { described_class.new(distributions: distributions_query, organization: organization, filters: filters).generate_csv }
 
     let(:duplicate_item) { create(:item, name: "Dupe Item", value_in_cents: 300, organization: organization, package_size: 2) }
 
@@ -174,7 +175,7 @@
     end
 
     context 'when there are no distributions but the report is requested' do
-      subject { described_class.new(distributions: [], organization: organization, filters: filters).generate_csv }
+      subject { described_class.new(distributions: Distribution.none, organization: organization, filters: filters).generate_csv }
       it 'returns a csv with only headers and no rows' do
         csv = <<~CSV
           Partner,Initial Allocation,Scheduled for,Source Inventory,Total Number of #{item_name},Total Value of #{item_name},Delivery Method,Shipping Cost,Status,Agency Representative,Comments,Reporting Category,Dupe Item
@@ -183,4 +184,26 @@
       end
     end
   end
+
+  # this is intended for easier performance testing, not to be run with rest of suite
+  # also, better for testing consistency to disable config.order = :random
+  # lastly, test setup time gets long, the printed duration is the actual generation time to pay attention to
+  xcontext "performance timing" do
+    [500, 1000, 2000, 5000, 10_000, 20_000, 30_000].reverse_each do |distribution_count|
+      it "processes #{distribution_count} distributions" do
+        partners = create_list(:partner, 500)
+        items = create_list(:item, 5000, organization:, on_hand_minimum_quantity: 5)
+        create_list(:distribution, distribution_count, :with_items, item: items.sample, storage_location: storage_location, organization: organization, partner: partners.sample)
+        TestInventory.create_inventory(organization, {
+          storage_location.id => items.map { [it.id, rand(20..69)] }.to_h
+        })
+        t = Time.zone.now
+        csv = Exports::ExportDistributionsCSVService.new(distributions: organization.distributions.includes(:partner, :storage_location, line_items: :item), organization:).generate_csv
+        puts "================================================================="
+        puts "Duration: #{(d = Time.zone.now - t).round(4)} for #{distribution_count} distributions: #{(d / distribution_count).round(4)}/dist"
+        File.write("dist-#{distribution_count}.csv", csv)
+        expect(true)
+      end
+    end
+  end
 end