From 2a6790bf21afe0015d1bf1a4a3791848f0f075d9 Mon Sep 17 00:00:00 2001 From: Alexis Harris Date: Fri, 17 Apr 2026 00:05:39 -0400 Subject: [PATCH] Fix inconsistent item headers in donation and purchase CSV exports --- Gemfile.lock | 2 +- app/controllers/purchases_controller.rb | 6 +++- .../exports/export_donations_csv_service.rb | 11 ++----- .../exports/export_purchases_csv_service.rb | 15 ++++------ db/schema.rb | 2 +- .../export_donations_csv_service_spec.rb | 9 ++++++ .../export_purchases_csv_service_spec.rb | 29 ++++++++++++------- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index b42ed4907f..51d47212c2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -856,4 +856,4 @@ DEPENDENCIES webmock (~> 3.26) BUNDLED WITH - 2.7.1 + 4.0.9 diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb index 9f9a19793d..ff66551d17 100644 --- a/app/controllers/purchases_controller.rb +++ b/app/controllers/purchases_controller.rb @@ -33,8 +33,12 @@ def index respond_to do |format| format.html # format.csv { send_data Purchase.generate_csv(@purchases), filename: "Purchases-#{Time.zone.today}.csv" } + # the fix made in export purchases depends on the use of the current organization 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_donations_csv_service.rb b/app/services/exports/export_donations_csv_service.rb index 9ca5bcb73f..459df11715 100644 --- a/app/services/exports/export_donations_csv_service.rb +++ b/app/services/exports/export_donations_csv_service.rb @@ -95,18 +95,11 @@ def base_headers base_table.keys end + #updated to use organization headers instead of just headers with a value when filtered def item_headers return @item_headers if @item_headers - item_names = Set.new - - donations.each do |donation| - donation.line_items.each do |line_item| - item_names.add(line_item.item.name) - end - end - - @item_headers = item_names.sort + @item_headers = @organization.items.pluck(:name).sort_by(&:downcase) @item_headers = @item_headers.flat_map { |header| [header, "#{header} In-Kind Value"] } if @organization.include_in_kind_values_in_exported_files diff --git a/app/services/exports/export_purchases_csv_service.rb b/app/services/exports/export_purchases_csv_service.rb index 5de939f87e..edb7898a4b 100644 --- a/app/services/exports/export_purchases_csv_service.rb +++ b/app/services/exports/export_purchases_csv_service.rb @@ -1,6 +1,7 @@ module Exports class ExportPurchasesCSVService - def initialize(purchase_ids:) + def initialize(purchase_ids:, organization:) + @organization = 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! @@ -101,18 +102,12 @@ def base_headers base_table.keys end + # updated to use organization headers instead of just headers with a value when filtered + # more updates in purchases controller 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 + @item_headers = @organization.items.pluck(:name).sort_by(&:downcase) end def build_row_data(purchase) diff --git a/db/schema.rb b/db/schema.rb index b1ce1c0540..50b213bbed 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_10_07_141240) do +ActiveRecord::Schema[8.0].define(version: 2025_10_17_194543) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" diff --git a/spec/services/exports/export_donations_csv_service_spec.rb b/spec/services/exports/export_donations_csv_service_spec.rb index 89ec120140..5d2b74a99f 100644 --- a/spec/services/exports/export_donations_csv_service_spec.rb +++ b/spec/services/exports/export_donations_csv_service_spec.rb @@ -71,6 +71,15 @@ def source_name(donation) CSV expect(subject).to eq(csv) end + # new donations export regression test + it "includes all organization item headers even when exported donations only use a subset of items" do + unused_item = create(:item, name: "Z Unused Item", organization: organization) + + csv_rows = CSV.parse(subject, headers: true) + headers = csv_rows.headers + + expect(headers).to include("A Item", "B Item", "C Item", "Dupe Item", "E Item", "Z Unused Item") + end end context 'while "Include in-kind value in donation and distribution exports?" is set to yes' do diff --git a/spec/services/exports/export_purchases_csv_service_spec.rb b/spec/services/exports/export_purchases_csv_service_spec.rb index 2293379363..7406840a5e 100644 --- a/spec/services/exports/export_purchases_csv_service_spec.rb +++ b/spec/services/exports/export_purchases_csv_service_spec.rb @@ -1,20 +1,21 @@ RSpec.describe Exports::ExportPurchasesCSVService do describe "#generate_csv_data" do - subject { described_class.new(purchase_ids: purchase_ids).generate_csv_data } + let(:organization) { create(:organization) } let(:purchase_ids) { purchases.map(&:id) } + let(:storage_location) { create(:storage_location, organization: organization) } + subject { described_class.new(purchase_ids: purchase_ids, organization: organization).generate_csv_data } + let(:duplicate_item) do - FactoryBot.create( - :item, name: Faker::Appliance.unique.equipment - ) + 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 - ), + FactoryBot.create(:item, name: Faker::Appliance.unique.equipment, + organization: organization), 7 ], [duplicate_item, 3] @@ -35,9 +36,9 @@ items_lists.each_with_index.map do |items, i| purchase = create( :purchase, - vendor: create( - :vendor, business_name: "Vendor Name #{i}" - ), + organization: organization, + storage_location: storage_location, + 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, @@ -114,5 +115,13 @@ expect(subject[idx + 1]).to eq(row) end end + # new purchase export regression test + it "includes all organization item headers even when exported purchases only use a subset of items" do + unused_item = create(:item, name: "Z Unused Item", organization: organization) + + headers = subject[0] + + expect(headers).to include("Z Unused Item") + end end end