From 013f77cef573161517465f98fc9fd160342d365f Mon Sep 17 00:00:00 2001
From: Brian Bonus <bsbonus@gmail.com>
Date: Thu, 22 May 2025 12:13:23 -0700
Subject: [PATCH 1/3] 
 Bug-5165-update-purchase-export-with-inactive-unpurchased-items

---
 app/controllers/purchases_controller.rb       |  2 +-
 .../exports/export_purchases_csv_service.rb   | 23 +----
 .../export_purchases_csv_service_spec.rb      | 91 ++++++++++++++++++-
 3 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index e7fb50d167..33c39232c5 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -32,7 +32,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..f42890255a 100644
--- a/app/services/exports/export_purchases_csv_service.rb
+++ b/app/services/exports/export_purchases_csv_service.rb
@@ -1,16 +1,18 @@
 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(
+      @organization = organization
+      @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
@@ -34,7 +36,7 @@ def generate_csv_data
 
     private
 
-    attr_reader :purchases
+    attr_reader :purchases, :item_headers
 
     def headers
       # Build the headers in the correct order
@@ -60,7 +62,6 @@ def headers_with_indexes
     # (or on the order of the literal).
     def base_table
       {
-
         "Purchases from" => ->(purchase) {
           purchase.vendor.try(:business_name)
         },
@@ -101,20 +102,6 @@ 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) }
 
diff --git a/spec/services/exports/export_purchases_csv_service_spec.rb b/spec/services/exports/export_purchases_csv_service_spec.rb
index 2293379363..7f8a8a155f 100644
--- a/spec/services/exports/export_purchases_csv_service_spec.rb
+++ b/spec/services/exports/export_purchases_csv_service_spec.rb
@@ -1,10 +1,11 @@
 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) }
+    subject { described_class.new(purchase_ids: purchase_ids, organization: organization).generate_csv_data }
     let(:purchase_ids) { purchases.map(&:id) }
     let(:duplicate_item) do
       FactoryBot.create(
-        :item, name: Faker::Appliance.unique.equipment
+        :item, name: Faker::Appliance.unique.equipment, organization: organization
       )
     end
     let(:items_lists) do
@@ -13,7 +14,7 @@
           [duplicate_item, 5],
           [
             FactoryBot.create(
-              :item, name: Faker::Appliance.unique.equipment
+              :item, name: Faker::Appliance.unique.equipment, organization: organization
             ),
             7
           ],
@@ -21,7 +22,7 @@
         ],
         *(Array.new(3) do |i|
           [[FactoryBot.create(
-            :item, name: Faker::Appliance.unique.equipment
+            :item, name: Faker::Appliance.unique.equipment, organization: organization
           ), i + 1]]
         end)
       ]
@@ -35,8 +36,9 @@
       items_lists.each_with_index.map do |items, i|
         purchase = create(
           :purchase,
+          organization: organization,
           vendor: create(
-            :vendor, business_name: "Vendor Name #{i}"
+            :vendor, business_name: "Vendor Name #{i}", organization: organization
           ),
           issued_at: start_time + i.days,
           comment: "This is the #{i}-th purchase in the test.",
@@ -114,5 +116,84 @@
         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
+        # Force unused_item to be created first
+        unused_item
+        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
+        # Force inactive_item to be created first
+        inactive_item
+        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 generating CSV output" 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
+
+    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
 end

From 00c0accddc41f379b864132cdbd43a028dfa8815 Mon Sep 17 00:00:00 2001
From: Brian Bonus <bsbonus@gmail.com>
Date: Wed, 28 May 2025 11:10:25 -0700
Subject: [PATCH 2/3] Bug-5152 cleans up class to not use instance vars

---
 app/services/exports/export_purchases_csv_service.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/services/exports/export_purchases_csv_service.rb b/app/services/exports/export_purchases_csv_service.rb
index f42890255a..aad5b256e0 100644
--- a/app/services/exports/export_purchases_csv_service.rb
+++ b/app/services/exports/export_purchases_csv_service.rb
@@ -4,7 +4,6 @@ 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!
-      @organization = organization
       @purchases = organization.purchases.includes(
         :storage_location,
         :vendor,
@@ -12,7 +11,7 @@ def initialize(purchase_ids:, organization:)
       ).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)
+      @item_headers = organization.items.select("DISTINCT ON (LOWER(name)) items.name").order("LOWER(name) ASC").map(&:name)
     end
 
     def generate_csv
@@ -36,11 +35,11 @@ def generate_csv_data
 
     private
 
-    attr_reader :purchases, :item_headers
+    attr_reader :purchases
 
     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
@@ -105,7 +104,7 @@ def base_headers
     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

From f71fca35e50106b24dc83bbd8943bed03f045021 Mon Sep 17 00:00:00 2001
From: Brian Bonus <bsbonus@gmail.com>
Date: Wed, 28 May 2025 11:10:44 -0700
Subject: [PATCH 3/3] Bug-5152 Cleans up spec file a bit

---
 .../export_purchases_csv_service_spec.rb      | 210 +++++++++---------
 1 file changed, 103 insertions(+), 107 deletions(-)

diff --git a/spec/services/exports/export_purchases_csv_service_spec.rb b/spec/services/exports/export_purchases_csv_service_spec.rb
index 7f8a8a155f..e7607a00fc 100644
--- a/spec/services/exports/export_purchases_csv_service_spec.rb
+++ b/spec/services/exports/export_purchases_csv_service_spec.rb
@@ -1,97 +1,97 @@
 RSpec.describe Exports::ExportPurchasesCSVService do
-  describe "#generate_csv_data" do
-    let(:organization) { create(:organization) }
-    subject { described_class.new(purchase_ids: purchase_ids, organization: organization).generate_csv_data }
-    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
-            ),
-            7
-          ],
-          [duplicate_item, 3]
-        ],
-        *(Array.new(3) do |i|
-          [[FactoryBot.create(
-            :item, name: Faker::Appliance.unique.equipment, organization: organization
-          ), 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,
-          organization: organization,
-          vendor: create(
-            :vendor, business_name: "Vendor Name #{i}", organization: organization
+  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
-        )
+          7
+        ],
+        [duplicate_item, 3]
+      ],
+      *(Array.new(3) do |i|
+        [[FactoryBot.create(
+          :item, name: Faker::Appliance.unique.equipment, organization: organization
+        ), i + 1]]
+      end)
+    ]
+  end
 
-        items.each do |(item, quantity)|
-          purchase.line_items << create(
-            :line_item, quantity: quantity, item: item
-          )
-        end
+  let(:item_names) { items_lists.flatten(1).map(&:first).map(&:name).sort.uniq }
+
+  let(:purchases) do
+    start_time = Time.current
+
+    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
+      )
 
-        purchase
+      items.each do |(item, quantity)|
+        purchase.line_items << create(
+          :line_item, quantity: quantity, item: item
+        )
       end
-    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
-
-    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
+      purchase
     end
+  end
 
-    let(:expected_item_headers) do
-      expect(item_names).not_to be_empty
-
-      item_names
-    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)
@@ -118,10 +118,8 @@
     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!(:unused_item) { create(:item, name: "Unused Item", organization: organization) }
       let(:generated_csv_data) do
-        # Force unused_item to be created first
-        unused_item
         described_class.new(purchase_ids: purchases.map(&:id), organization: organization).generate_csv_data
       end
 
@@ -137,10 +135,8 @@
     end
 
     context "when an organization's item is inactive" do
-      let(:inactive_item) { create(:item, name: "Inactive Item", active: false, organization: organization) }
+      let!(:inactive_item) { create(:item, name: "Inactive Item", active: false, organization: organization) }
       let(:generated_csv_data) do
-        # Force inactive_item to be created first
-        inactive_item
         described_class.new(purchase_ids: purchases.map(&:id), organization: organization).generate_csv_data
       end
 
@@ -155,25 +151,6 @@
       end
     end
 
-    context "when generating CSV output" 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
-
     context "when items have different cases" do
       let(:item_names) { ["Zebra", "apple", "Banana"] }
       let(:expected_order) { ["apple", "Banana", "Zebra"] }
@@ -196,4 +173,23 @@
       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