diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb index 9f9a19793d..aa25641441 100644 --- a/app/controllers/purchases_controller.rb +++ b/app/controllers/purchases_controller.rb @@ -34,7 +34,7 @@ def index format.html # format.csv { send_data Purchase.generate_csv(@purchases), filename: "Purchases-#{Time.zone.today}.csv" } format.csv do - send_data Exports::ExportPurchasesCSVService.new(purchase_ids: @purchases.map(&:id)).generate_csv, filename: "Purchases-#{Time.zone.today}.csv" + send_data Exports::ExportPurchasesCSVService.new(purchase_ids: @purchases.map(&:id), organization: current_organization).generate_csv, filename: "Purchases-#{Time.zone.today}.csv" end end end diff --git a/app/services/exports/export_purchases_csv_service.rb b/app/services/exports/export_purchases_csv_service.rb index 5de939f87e..aad5b256e0 100644 --- a/app/services/exports/export_purchases_csv_service.rb +++ b/app/services/exports/export_purchases_csv_service.rb @@ -1,16 +1,17 @@ module Exports class ExportPurchasesCSVService - def initialize(purchase_ids:) + def initialize(purchase_ids:, organization:) # Use a where lookup so that I can eager load all the resources # needed rather than depending on external code to do it for me. # This makes this code more self contained and efficient! - @purchases = Purchase.includes( + @purchases = organization.purchases.includes( :storage_location, :vendor, line_items: [:item] ).where( id: purchase_ids ).order(created_at: :asc) + @item_headers = organization.items.select("DISTINCT ON (LOWER(name)) items.name").order("LOWER(name) ASC").map(&:name) end def generate_csv @@ -38,7 +39,7 @@ def generate_csv_data def headers # Build the headers in the correct order - base_headers + item_headers + base_headers + @item_headers end # Returns a Hash of keys to indexes so that obtaining the index @@ -60,7 +61,6 @@ def headers_with_indexes # (or on the order of the literal). def base_table { - "Purchases from" => ->(purchase) { purchase.vendor.try(:business_name) }, @@ -101,24 +101,10 @@ def base_headers base_table.keys end - def item_headers - return @item_headers if @item_headers - - item_names = Set.new - - purchases.each do |purchase| - purchase.line_items.each do |line_item| - item_names.add(line_item.item.name) - end - end - - @item_headers = item_names.sort - end - def build_row_data(purchase) row = base_table.values.map { |closure| closure.call(purchase) } - row += Array.new(item_headers.size, 0) + row += Array.new(@item_headers.size, 0) purchase.line_items.each do |line_item| item_name = line_item.item.name diff --git a/spec/services/exports/export_purchases_csv_service_spec.rb b/spec/services/exports/export_purchases_csv_service_spec.rb index 2293379363..e7607a00fc 100644 --- a/spec/services/exports/export_purchases_csv_service_spec.rb +++ b/spec/services/exports/export_purchases_csv_service_spec.rb @@ -1,95 +1,97 @@ RSpec.describe Exports::ExportPurchasesCSVService do - describe "#generate_csv_data" do - subject { described_class.new(purchase_ids: purchase_ids).generate_csv_data } - let(:purchase_ids) { purchases.map(&:id) } - let(:duplicate_item) do - FactoryBot.create( - :item, name: Faker::Appliance.unique.equipment - ) - end - let(:items_lists) do - [ - [ - [duplicate_item, 5], - [ - FactoryBot.create( - :item, name: Faker::Appliance.unique.equipment - ), - 7 - ], - [duplicate_item, 3] - ], - *(Array.new(3) do |i| - [[FactoryBot.create( - :item, name: Faker::Appliance.unique.equipment - ), i + 1]] - end) - ] + let(:organization) { create(:organization) } + let(:total_item_quantities) do + template = item_names.index_with(0) + + items_lists.map do |items_list| + row = template.dup + items_list.each do |(item, quantity)| + row[item.name] += quantity + end + row.values end + end - let(:item_names) { items_lists.flatten(1).map(&:first).map(&:name).sort.uniq } + let(:expected_item_headers) do + expect(item_names).not_to be_empty - let(:purchases) do - start_time = Time.current + item_names + end + let(:expected_headers) do + [ + "Purchases from", + "Storage Location", + "Purchased Date", + "Quantity of Items", + "Variety of Items", + "Amount Spent", + "Spent on Diapers", + "Spent on Adult Incontinence", + "Spent on Period Supplies", + "Spent on Other", + "Comment" + ] + expected_item_headers + end - items_lists.each_with_index.map do |items, i| - purchase = create( - :purchase, - vendor: create( - :vendor, business_name: "Vendor Name #{i}" + let(:purchase_ids) { purchases.map(&:id) } + let(:duplicate_item) do + FactoryBot.create( + :item, name: Faker::Appliance.unique.equipment, organization: organization + ) + end + let(:items_lists) do + [ + [ + [duplicate_item, 5], + [ + FactoryBot.create( + :item, name: Faker::Appliance.unique.equipment, organization: organization ), - issued_at: start_time + i.days, - comment: "This is the #{i}-th purchase in the test.", - amount_spent_in_cents: i * 4 + 555, - amount_spent_on_diapers_cents: i + 100, - amount_spent_on_adult_incontinence_cents: i + 125, - amount_spent_on_period_supplies_cents: i + 130, - amount_spent_on_other_cents: i + 200 - ) - - items.each do |(item, quantity)| - purchase.line_items << create( - :line_item, quantity: quantity, item: item - ) - end + 7 + ], + [duplicate_item, 3] + ], + *(Array.new(3) do |i| + [[FactoryBot.create( + :item, name: Faker::Appliance.unique.equipment, organization: organization + ), i + 1]] + end) + ] + end - purchase - end - end + let(:item_names) { items_lists.flatten(1).map(&:first).map(&:name).sort.uniq } - let(:expected_headers) do - [ - "Purchases from", - "Storage Location", - "Purchased Date", - "Quantity of Items", - "Variety of Items", - "Amount Spent", - "Spent on Diapers", - "Spent on Adult Incontinence", - "Spent on Period Supplies", - "Spent on Other", - "Comment" - ] + expected_item_headers - end + let(:purchases) do + start_time = Time.current - let(:total_item_quantities) do - template = item_names.index_with(0) + items_lists.each_with_index.map do |items, i| + purchase = create( + :purchase, + organization: organization, + vendor: create( + :vendor, business_name: "Vendor Name #{i}", organization: organization + ), + issued_at: start_time + i.days, + comment: "This is the #{i}-th purchase in the test.", + amount_spent_in_cents: i * 4 + 555, + amount_spent_on_diapers_cents: i + 100, + amount_spent_on_adult_incontinence_cents: i + 125, + amount_spent_on_period_supplies_cents: i + 130, + amount_spent_on_other_cents: i + 200 + ) - items_lists.map do |items_list| - row = template.dup - items_list.each do |(item, quantity)| - row[item.name] += quantity - end - row.values + items.each do |(item, quantity)| + purchase.line_items << create( + :line_item, quantity: quantity, item: item + ) end - end - - let(:expected_item_headers) do - expect(item_names).not_to be_empty - item_names + purchase end + end + + describe "#generate_csv_data" do + subject { described_class.new(purchase_ids: purchase_ids, organization: organization).generate_csv_data } it "should match the expected content for the csv" do expect(subject[0]).to eq(expected_headers) @@ -114,5 +116,80 @@ expect(subject[idx + 1]).to eq(row) end end + + context "when an organization's item exists but isn't in any purchase" do + let!(:unused_item) { create(:item, name: "Unused Item", organization: organization) } + let(:generated_csv_data) do + described_class.new(purchase_ids: purchases.map(&:id), organization: organization).generate_csv_data + end + + it "should include the unused item as a column with 0 quantities" do + expect(generated_csv_data[0]).to include(unused_item.name) + + purchases.each_with_index do |_, idx| + row = generated_csv_data[idx + 1] + item_column_index = generated_csv_data[0].index(unused_item.name) + expect(row[item_column_index]).to eq(0) + end + end + end + + context "when an organization's item is inactive" do + let!(:inactive_item) { create(:item, name: "Inactive Item", active: false, organization: organization) } + let(:generated_csv_data) do + described_class.new(purchase_ids: purchases.map(&:id), organization: organization).generate_csv_data + end + + it "should include the inactive item as a column with 0 quantities" do + expect(generated_csv_data[0]).to include(inactive_item.name) + + purchases.each_with_index do |_, idx| + row = generated_csv_data[idx + 1] + item_column_index = generated_csv_data[0].index(inactive_item.name) + expect(row[item_column_index]).to eq(0) + end + end + end + + context "when items have different cases" do + let(:item_names) { ["Zebra", "apple", "Banana"] } + let(:expected_order) { ["apple", "Banana", "Zebra"] } + let(:purchase) { create(:purchase, organization: organization) } + let(:case_sensitive_csv_data) do + # Create items in random order to ensure sort is working + item_names.shuffle.each do |name| + create(:item, name: name, organization: organization) + end + + described_class.new(purchase_ids: [purchase.id], organization: organization).generate_csv_data + end + + it "should sort item columns case-insensitively, ASC" do + # Get just the item columns by removing the known base headers + item_columns = case_sensitive_csv_data[0] - expected_headers[0..-4] # plucks out the 3 items at the end + + # Check that the remaining columns match our expected case-insensitive sort + expect(item_columns).to eq(expected_order) + end + end + end + + describe "#generate_csv" do + let(:generated_csv) { described_class.new(purchase_ids: purchase_ids, organization: organization).generate_csv } + + it "returns a valid CSV string" do + expect(generated_csv).to be_a(String) + expect { CSV.parse(generated_csv) }.not_to raise_error + end + + it "includes headers as first row" do + csv_rows = CSV.parse(generated_csv) + expect(csv_rows.first).to eq(expected_headers) + end + + it "includes data for all purchases" do + csv_rows = CSV.parse(generated_csv) + expect(csv_rows.count).to eq(purchases.count + 1) # +1 for headers + end end end