From 3e2f4fea8e4dd4bbc1bd867c7f7f977315cd331c Mon Sep 17 00:00:00 2001 From: John Costa Date: Sat, 7 Feb 2026 23:09:42 -0800 Subject: [PATCH 1/2] Add item filter to donations index page Add by_item_id and by_item_category_id scopes to Donation model, mirroring the existing Distribution filters. Wire up the item filter dropdown on the donations index page and permit the new params in both the controller and the View::Donations data object. Closes #5105 --- app/controllers/donations_controller.rb | 2 +- app/models/donation.rb | 2 ++ app/models/view/donations.rb | 9 ++++++++- app/views/donations/index.html.erb | 5 +++++ spec/models/donation_spec.rb | 22 ++++++++++++++++++++++ spec/system/donation_system_spec.rb | 12 ++++++++++++ 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 74580065f6..bc09bdfffe 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -134,7 +134,7 @@ def donation_item_params def filter_params return {} unless params.key?(:filters) - params.require(:filters).permit(:at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, :from_manufacturer, :by_category) + params.require(:filters).permit(:at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, :from_manufacturer, :by_item_id, :by_item_category_id, :by_category) end # Omits donation_site_id or product_drive_participant_id if those aren't selected as source diff --git a/app/models/donation.rb b/app/models/donation.rb index 7fe6f5cfad..55a08ede10 100644 --- a/app/models/donation.rb +++ b/app/models/donation.rb @@ -52,6 +52,8 @@ class Donation < ApplicationRecord where(manufacturer_id: manufacturer_id) } + scope :by_item_id, ->(item_id) { includes(:items).where(items: { id: item_id }) } + scope :by_item_category_id, ->(item_category_id) { includes(:items).where(items: { item_category_id: item_category_id }) } scope :by_category, ->(item_category) { joins(line_items: {item: :item_category}).where("item_categories.name ILIKE ?", item_category) } diff --git a/app/models/view/donations.rb b/app/models/view/donations.rb index 9518e98047..6116a35a50 100644 --- a/app/models/view/donations.rb +++ b/app/models/view/donations.rb @@ -2,6 +2,7 @@ module View Donations = Data.define( :donations, :filters, + :items, :item_categories, :paginated_donations, :product_drives, @@ -18,7 +19,8 @@ def filter_params(params) params.require(:filters).permit( :at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, - :from_manufacturer, :by_category + :from_manufacturer, :by_item_id, :by_item_category_id, + :by_category ) else {} @@ -49,6 +51,7 @@ def from_params(params:, organization:, helpers:) new( donations: donations, filters: filters, + items: organization.items.alphabetized.select(:id, :name), item_categories: organization.item_categories.pluck(:name).uniq, paginated_donations: paginated_donations, product_drives: organization.product_drives.alphabetized, @@ -68,6 +71,10 @@ def selected_source filters[:by_source] end + def selected_item + filters[:by_item_id].presence + end + def selected_item_category filters[:by_category] end diff --git a/app/views/donations/index.html.erb b/app/views/donations/index.html.erb index c3e551e466..c3347ebe4c 100644 --- a/app/views/donations/index.html.erb +++ b/app/views/donations/index.html.erb @@ -87,6 +87,11 @@ selected: @donation_info.selected_donation_site) %> <% end %> + <% if @donation_info.items.present? %> +
+ <%= filter_select(label: "Filter by Item", scope: :by_item_id, collection: @donation_info.items, selected: @donation_info.selected_item) %> +
+ <% end %> <% if @donation_info.item_categories.present? %>
<% id = "filter_#{SecureRandom.uuid}" %> diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb index 53929bf31e..b23139ba57 100644 --- a/spec/models/donation_spec.rb +++ b/spec/models/donation_spec.rb @@ -156,6 +156,28 @@ end end end + + describe "by_item_id >" do + it "returns only donations with the specified item" do + item1 = create(:item, organization: organization) + item2 = create(:item, organization: organization) + create(:donation, :with_items, item: item1, organization: organization) + create(:donation, :with_items, item: item2, organization: organization) + expect(Donation.by_item_id(item1.id).count).to eq(1) + end + end + + describe "by_item_category_id >" do + it "returns only donations with items in the specified category" do + category = create(:item_category, organization: organization) + other_category = create(:item_category, organization: organization) + item1 = create(:item, item_category: category, organization: organization) + item2 = create(:item, item_category: other_category, organization: organization) + create(:donation, :with_items, item: item1, organization: organization) + create(:donation, :with_items, item: item2, organization: organization) + expect(Donation.by_item_category_id(category.id).count).to eq(1) + end + end end context "Associations >" do diff --git a/spec/system/donation_system_spec.rb b/spec/system/donation_system_spec.rb index 2a158531b2..6576d0eebb 100644 --- a/spec/system/donation_system_spec.rb +++ b/spec/system/donation_system_spec.rb @@ -135,6 +135,18 @@ expect(page).to have_css("table tbody tr", count: 1) end + it "Filters by item" do + item_1 = create(:item, name: "Good item", organization: organization) + item_2 = create(:item, name: "Bad item", organization: organization) + create(:donation, :with_items, item: item_1) + create(:donation, :with_items, item: item_2) + visit subject + expect(page).to have_css("table tbody tr", count: 2) + select item_1.name, from: "filters[by_item_id]" + click_button "Filter" + expect(page).to have_css("table tbody tr", count: 1) + end + it "Filters by category" do category_1 = create(:item_category, name: "Category 1", organization: organization) category_2 = create(:item_category, name: "Category 2", organization: organization) From 434785675cb98c74f4549e7ebdb1603e4e359051 Mon Sep 17 00:00:00 2001 From: John Costa Date: Sat, 21 Feb 2026 21:24:19 -0800 Subject: [PATCH 2/2] Address review feedback: use joins instead of includes, remove redundant scope - Change by_item_id scope from includes to joins with distinct for efficient filtering without eager loading - Remove by_item_category_id scope, controller param, and view model param since the existing by_category scope and "Filter by Category" dropdown already cover category filtering - Remove corresponding by_item_category_id spec Co-Authored-By: Claude Opus 4.6 --- app/controllers/donations_controller.rb | 2 +- app/models/donation.rb | 3 +-- app/models/view/donations.rb | 3 +-- spec/models/donation_spec.rb | 11 ----------- 4 files changed, 3 insertions(+), 16 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index bc09bdfffe..16e5eab444 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -134,7 +134,7 @@ def donation_item_params def filter_params return {} unless params.key?(:filters) - params.require(:filters).permit(:at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, :from_manufacturer, :by_item_id, :by_item_category_id, :by_category) + params.require(:filters).permit(:at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, :from_manufacturer, :by_item_id, :by_category) end # Omits donation_site_id or product_drive_participant_id if those aren't selected as source diff --git a/app/models/donation.rb b/app/models/donation.rb index 55a08ede10..cd94db1386 100644 --- a/app/models/donation.rb +++ b/app/models/donation.rb @@ -52,8 +52,7 @@ class Donation < ApplicationRecord where(manufacturer_id: manufacturer_id) } - scope :by_item_id, ->(item_id) { includes(:items).where(items: { id: item_id }) } - scope :by_item_category_id, ->(item_category_id) { includes(:items).where(items: { item_category_id: item_category_id }) } + scope :by_item_id, ->(item_id) { joins(:items).where(items: { id: item_id }).distinct } scope :by_category, ->(item_category) { joins(line_items: {item: :item_category}).where("item_categories.name ILIKE ?", item_category) } diff --git a/app/models/view/donations.rb b/app/models/view/donations.rb index 6116a35a50..8da6cff511 100644 --- a/app/models/view/donations.rb +++ b/app/models/view/donations.rb @@ -19,8 +19,7 @@ def filter_params(params) params.require(:filters).permit( :at_storage_location, :by_source, :from_donation_site, :by_product_drive, :by_product_drive_participant, - :from_manufacturer, :by_item_id, :by_item_category_id, - :by_category + :from_manufacturer, :by_item_id, :by_category ) else {} diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb index b23139ba57..50edb97a15 100644 --- a/spec/models/donation_spec.rb +++ b/spec/models/donation_spec.rb @@ -167,17 +167,6 @@ end end - describe "by_item_category_id >" do - it "returns only donations with items in the specified category" do - category = create(:item_category, organization: organization) - other_category = create(:item_category, organization: organization) - item1 = create(:item, item_category: category, organization: organization) - item2 = create(:item, item_category: other_category, organization: organization) - create(:donation, :with_items, item: item1, organization: organization) - create(:donation, :with_items, item: item2, organization: organization) - expect(Donation.by_item_category_id(category.id).count).to eq(1) - end - end end context "Associations >" do