diff --git a/app/services/exports/export_distributions_csv_service.rb b/app/services/exports/export_distributions_csv_service.rb index 137bb082c8..bbac1a199f 100644 --- a/app/services/exports/export_distributions_csv_service.rb +++ b/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 diff --git a/spec/controllers/distributions_controller_spec.rb b/spec/controllers/distributions_controller_spec.rb index 4a6ae7723e..a355637890 100644 --- a/spec/controllers/distributions_controller_spec.rb +++ b/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 diff --git a/spec/services/exports/export_distributions_csv_service_spec.rb b/spec/services/exports/export_distributions_csv_service_spec.rb index 8d1631f422..6b3f6d48b5 100644 --- a/spec/services/exports/export_distributions_csv_service_spec.rb +++ b/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