From f48bd7180f99234248b0f7a76e3172897bd42d41 Mon Sep 17 00:00:00 2001 From: Kate Donaldson Date: Tue, 30 Jan 2024 22:46:06 -0600 Subject: [PATCH 1/4] Adds query for all an organization's items to the CSV export service, updates spec to test new behavior --- .../export_distributions_csv_service.rb | 13 ++--- spec/requests/partners_requests_spec.rb | 1 + .../export_distributions_csv_service_spec.rb | 53 ++++++++++++++++--- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/app/services/exports/export_distributions_csv_service.rb b/app/services/exports/export_distributions_csv_service.rb index ce64b90520..376f5ee64c 100644 --- a/app/services/exports/export_distributions_csv_service.rb +++ b/app/services/exports/export_distributions_csv_service.rb @@ -10,6 +10,7 @@ def initialize(distributions:, filters: []) # service object. @distributions = distributions @filters = filters + @organization = @distributions.first.organization end def generate_csv @@ -114,15 +115,7 @@ def base_headers def item_headers return @item_headers if @item_headers - item_names = Set.new - - distributions.each do |distribution| - distribution.line_items.each do |line_item| - item_names.add(line_item.item.name) - end - end - - @item_headers = item_names.sort + @item_headers = @organization.items.uniq.sort_by(&:created_at).pluck(:name) end def build_row_data(distribution) @@ -133,6 +126,8 @@ def build_row_data(distribution) distribution.line_items.each do |line_item| item_name = line_item.item.name item_column_idx = headers_with_indexes[item_name] + next unless item_column_idx + row[item_column_idx] += line_item.quantity end diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb index c7472bb914..8060e6021e 100644 --- a/spec/requests/partners_requests_spec.rb +++ b/spec/requests/partners_requests_spec.rb @@ -141,6 +141,7 @@ context "csv" do let(:response_format) { 'csv' } + before { create(:distribution, partner: partner) } it { is_expected.to be_successful } end diff --git a/spec/services/exports/export_distributions_csv_service_spec.rb b/spec/services/exports/export_distributions_csv_service_spec.rb index accc292b5b..6066168551 100644 --- a/spec/services/exports/export_distributions_csv_service_spec.rb +++ b/spec/services/exports/export_distributions_csv_service_spec.rb @@ -51,9 +51,13 @@ let(:item_id) { distributions.flatten.first.line_items.first.item_id } let(:filters) { {by_item_id: item_id} } let(:item_name) { Item.find(item_id).name } + let(:organization) { distributions.first.organization } + + let(:all_org_items) { Item.where(organization:).uniq.sort_by(&:created_at) } let(:total_item_quantities) do - template = item_names.index_with(0) + # binding.pry + template = all_org_items.pluck(:name).index_with(0) items_lists.map do |items_list| row = template.dup @@ -64,7 +68,7 @@ end end - let(:expected_headers) do + let(:non_item_headers) do [ "Partner", "Date of Distribution", @@ -76,14 +80,10 @@ "State", "Agency Representative", "Comments" - ] + expected_item_headers + ] end - let(:expected_item_headers) do - expect(item_names).not_to be_empty - - item_names - end + let(:expected_headers) { non_item_headers + all_org_items.pluck(:name) } it 'should match the expected content for the csv' do expect(subject[0]).to eq(expected_headers) @@ -107,5 +107,42 @@ expect(subject[idx + 1]).to eq(row) end end + + context 'when a new item is added' do + let!(:original_columns_count) { organization.items.size + non_item_headers.size } + let(:new_item_name) { "new item" } + before do + create(:item, name: new_item_name, organization:) + end + + it 'should add it to the end of the row' do + expect(subject[0].uniq).to eq(expected_headers) + .and end_with(new_item_name) + .and have_attributes(size: original_columns_count + 1) + end + + it 'should show up with a 0 quantity if there are no distributions' do + distributions.zip(total_item_quantities).each_with_index do |(distribution, total_item_quantity), idx| + row = [ + distribution.partner.name, + distribution.issued_at.strftime("%m/%d/%Y"), + distribution.storage_location.name, + distribution.line_items.where(item_id: item_id).total, + distribution.cents_to_dollar(distribution.line_items.total_value), + distribution.delivery_method, + "$#{distribution.shipping_cost.to_f}", + distribution.state, + distribution.agency_rep, + distribution.comment + ] + + row += total_item_quantity + + expect(subject[idx + 1]).to eq(row) + .and end_with(0) + .and have_attributes(size: original_columns_count + 1) + end + end + end end end From 3068bc805a4d5a6194aa3607bcedb8ac3b8874aa Mon Sep 17 00:00:00 2001 From: Kate Donaldson Date: Wed, 31 Jan 2024 17:28:24 -0600 Subject: [PATCH 2/4] Fixes query to run ordering/filtering in the db, adds return if distributions is blank, TODO about error-handling --- app/services/exports/export_distributions_csv_service.rb | 8 ++++++-- spec/requests/partners_requests_spec.rb | 1 - .../exports/export_distributions_csv_service_spec.rb | 1 - 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/services/exports/export_distributions_csv_service.rb b/app/services/exports/export_distributions_csv_service.rb index 376f5ee64c..a8bfff8abc 100644 --- a/app/services/exports/export_distributions_csv_service.rb +++ b/app/services/exports/export_distributions_csv_service.rb @@ -10,10 +10,14 @@ def initialize(distributions:, filters: []) # service object. @distributions = distributions @filters = filters - @organization = @distributions.first.organization + @organization = @distributions&.first&.organization end def generate_csv + # TODO: we may want some error handling here in case there are none, but I'm not sure what the process is; + # should we do a flash[:error]? + return if distributions.blank? + csv_data = generate_csv_data CSV.generate(headers: true) do |csv| @@ -115,7 +119,7 @@ def base_headers def item_headers return @item_headers if @item_headers - @item_headers = @organization.items.uniq.sort_by(&:created_at).pluck(:name) + @item_headers = @organization.items.order(:created_at).distinct.select([:created_at, :name]).map(&:name) end def build_row_data(distribution) diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb index 8060e6021e..c7472bb914 100644 --- a/spec/requests/partners_requests_spec.rb +++ b/spec/requests/partners_requests_spec.rb @@ -141,7 +141,6 @@ context "csv" do let(:response_format) { 'csv' } - before { create(:distribution, partner: partner) } it { is_expected.to be_successful } end diff --git a/spec/services/exports/export_distributions_csv_service_spec.rb b/spec/services/exports/export_distributions_csv_service_spec.rb index 6066168551..16dd8257d2 100644 --- a/spec/services/exports/export_distributions_csv_service_spec.rb +++ b/spec/services/exports/export_distributions_csv_service_spec.rb @@ -56,7 +56,6 @@ let(:all_org_items) { Item.where(organization:).uniq.sort_by(&:created_at) } let(:total_item_quantities) do - # binding.pry template = all_org_items.pluck(:name).index_with(0) items_lists.map do |items_list| From 1ece7e3df554861d72a52e51eff7514165f76af3 Mon Sep 17 00:00:00 2001 From: Kate Donaldson Date: Fri, 2 Feb 2024 16:13:01 -0600 Subject: [PATCH 3/4] Updates the export service to handle zero-cases correctly --- app/controllers/distributions_controller.rb | 2 +- app/controllers/partners_controller.rb | 2 +- app/services/exports/export_distributions_csv_service.rb | 8 ++------ .../exports/export_distributions_csv_service_spec.rb | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb index 2348de0678..55b21e4e34 100644 --- a/app/controllers/distributions_controller.rb +++ b/app/controllers/distributions_controller.rb @@ -66,7 +66,7 @@ def index respond_to do |format| format.html format.csv do - send_data Exports::ExportDistributionsCSVService.new(distributions: @distributions, filters: filter_params).generate_csv, filename: "Distributions-#{Time.zone.today}.csv" + send_data Exports::ExportDistributionsCSVService.new(distributions: @distributions, organization: current_organization, filters: filter_params).generate_csv, filename: "Distributions-#{Time.zone.today}.csv" end end end diff --git a/app/controllers/partners_controller.rb b/app/controllers/partners_controller.rb index ceb8f297d0..44596a2d12 100644 --- a/app/controllers/partners_controller.rb +++ b/app/controllers/partners_controller.rb @@ -59,7 +59,7 @@ def show respond_to do |format| format.html format.csv do - send_data Exports::ExportDistributionsCSVService.new(distributions: @partner_distributions, filters: filter_params).generate_csv, filename: "PartnerDistributions-#{Time.zone.today}.csv" + send_data Exports::ExportDistributionsCSVService.new(distributions: @partner_distributions, organization: current_organization, filters: filter_params).generate_csv, filename: "PartnerDistributions-#{Time.zone.today}.csv" end end end diff --git a/app/services/exports/export_distributions_csv_service.rb b/app/services/exports/export_distributions_csv_service.rb index a8bfff8abc..a0f6569409 100644 --- a/app/services/exports/export_distributions_csv_service.rb +++ b/app/services/exports/export_distributions_csv_service.rb @@ -1,7 +1,7 @@ module Exports class ExportDistributionsCSVService include DistributionHelper - def initialize(distributions:, filters: []) + 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 @@ -10,14 +10,10 @@ def initialize(distributions:, filters: []) # service object. @distributions = distributions @filters = filters - @organization = @distributions&.first&.organization + @organization = organization end def generate_csv - # TODO: we may want some error handling here in case there are none, but I'm not sure what the process is; - # should we do a flash[:error]? - return if distributions.blank? - csv_data = generate_csv_data CSV.generate(headers: true) do |csv| diff --git a/spec/services/exports/export_distributions_csv_service_spec.rb b/spec/services/exports/export_distributions_csv_service_spec.rb index 16dd8257d2..14244d3ac7 100644 --- a/spec/services/exports/export_distributions_csv_service_spec.rb +++ b/spec/services/exports/export_distributions_csv_service_spec.rb @@ -1,6 +1,6 @@ describe Exports::ExportDistributionsCSVService do describe '#generate_csv_data' do - subject { described_class.new(distributions: distributions, filters: filters).generate_csv_data } + subject { described_class.new(distributions: distributions, organization: Organization.first, filters: filters).generate_csv_data } let(:distributions) { distributions } let(:duplicate_item) do From 63633718c90ae259b76265cc800aa44929e33067 Mon Sep 17 00:00:00 2001 From: Kate Donaldson Date: Fri, 2 Feb 2024 16:30:18 -0600 Subject: [PATCH 4/4] Adds a spec for the zero-case --- .../export_distributions_csv_service_spec.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/services/exports/export_distributions_csv_service_spec.rb b/spec/services/exports/export_distributions_csv_service_spec.rb index 14244d3ac7..8f35ac5314 100644 --- a/spec/services/exports/export_distributions_csv_service_spec.rb +++ b/spec/services/exports/export_distributions_csv_service_spec.rb @@ -115,12 +115,12 @@ end it 'should add it to the end of the row' do - expect(subject[0].uniq).to eq(expected_headers) + expect(subject[0]).to eq(expected_headers) .and end_with(new_item_name) .and have_attributes(size: original_columns_count + 1) end - it 'should show up with a 0 quantity if there are no distributions' do + it 'should show up with a 0 quantity if there are none of this item in any distribution' do distributions.zip(total_item_quantities).each_with_index do |(distribution, total_item_quantity), idx| row = [ distribution.partner.name, @@ -143,5 +143,15 @@ end end end + + context 'when there are no distributions but the report is requested' do + subject { described_class.new(distributions: [], organization: Organization.first, filters: filters).generate_csv_data } + it 'returns a csv with only headers and no rows' do + header_row = subject[0] + expect(header_row).to eq(expected_headers) + expect(header_row.last).to eq(all_org_items.last.name) + expect(subject.size).to eq(1) + end + end end end