fix: accessibility for filter labels

app/helpers/filter_helper.rb

@@ -1,12 +1,16 @@
 module FilterHelper
   def filter_select(label: nil, scope:, collection:, key: :id, value: :name, selected: nil)
-    label ||= "Filter #{scope.to_s.gsub("_", ' ')}"
-    label_tag(label) + collection_select(:filters, scope, collection || {}, key, value, { include_blank: true, selected: selected }, class: "form-control")
+    sanitize_scope = scope.to_s.gsub("_", " ")
+    label ||= "Filter #{sanitize_scope}"
+    id = "filters_#{scope}_#{SecureRandom.uuid}"
+    label_tag(id, label) + collection_select(:filters, scope, collection || {}, key, value, { include_blank: true, selected: selected }, {class: "form-control", id: id})
   end
 
   def filter_text(label: nil, scope:, selected: nil)
-    label ||= "Filter #{scope.to_s.gsub("_", ' ')}"
-    label_tag(label) + text_field(:filters, scope, class: "form-control", value: selected)
+    sanitize_scope = scope.to_s.gsub("_", " ")
+    label ||= "Filter #{sanitize_scope}"
+    id = "filters_#{scope}_#{SecureRandom.uuid}"
+    label_tag(id, label) + text_field(:filters, scope, class: "form-control", id: id, value: selected)
   end
 
   def filter_checkbox(label: nil, scope:, selected: nil)

app/views/donations/index.html.erb

@@ -43,8 +43,11 @@
                 <% end %>
                 <% if @sources.present? %>
                   <div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
-                    <%= label_tag "by Source" %>
-                    <%= select_tag "filters[by_source]", options_for_select(@sources, @selected_source), {include_blank: true, class: "form-control"} %>
+                    <% id = "filter_#{SecureRandom.uuid}" %>
+                    <%= label_tag id, "Filter by Source" %>
+                    <%= select_tag "filters[by_source]",
+                    options_for_select(@sources, @selected_source),
+                    { include_blank: true, class: "form-control", id: id } %>
                   </div>
                 <% end %>
                 <% if @product_drives.present? %>
@@ -144,4 +147,4 @@
       </div>
     </div>
   </div>
-</section>
+</section>

spec/system/adjustment_system_spec.rb

@@ -132,7 +132,7 @@
     create(:adjustment, organization: @organization, storage_location: storage_location2)
 
     visit subject
-    select storage_location.name, from: "filters_at_location"
+    select storage_location.name, from: "filters[at_location]"
     click_on "Filter"
 
     expect(page).to have_css("table tr", count: 2)
@@ -144,7 +144,7 @@
     create(:adjustment, organization: @organization, storage_location: storage_location2, user_id: @organization_admin.id)
 
     visit subject
-    select @user.name, from: "filters_by_user"
+    select @user.name, from: "filters[by_user]"
     click_on "Filter"
 
     expect(page).to have_css("table tr", count: 2)

spec/system/audit_system_spec.rb

@@ -71,7 +71,7 @@
         create(:audit, organization: @organization, storage_location: storage_location2)
 
         visit subject
-        select storage_location.name, from: "filters_at_location"
+        select storage_location.name, from: "filters[at_location]"
         click_button "Filter"
 
         expect(page).to have_css("table tr", count: 2)

spec/system/barcode_item_system_spec.rb

@@ -22,7 +22,7 @@
       b = create(:barcode_item, organization: @organization)
       create(:barcode_item, organization: @organization)
       visit subject
-      fill_in "filters_by_value", with: b.value
+      fill_in "filters[by_value]", with: b.value
       click_button "Filter"
 
       expect(page).to have_css("table tbody tr", count: 1)
@@ -39,15 +39,15 @@
       create(:barcode_item, barcodeable: item1)
       visit subject
 
-      expect(page.all("select#filters_barcodeable_id option").map(&:text)).to eq(expected_order)
-      expect(page.all("select#filters_barcodeable_id option").map(&:text)).not_to eq(expected_order.reverse)
+      expect(page.all('select[name="filters[barcodeable_id]"] option').map(&:text)).to eq(expected_order)
+      expect(page.all('select[name="filters[barcodeable_id]"] option').map(&:text)).not_to eq(expected_order.reverse)
     end
 
     it "can have a user filter the #index by item type" do
       b = create(:barcode_item, organization: @organization)
       create(:barcode_item, organization: @organization)
       visit subject
-      select b.item.name, from: "filters_barcodeable_id"
+      select b.item.name, from: "filters[barcodeable_id]"
       click_button "Filter"
 
       expect(page).to have_css("table tbody tr", count: 1)
@@ -59,7 +59,7 @@
       create(:barcode_item, organization: @organization, barcodeable: item)
       create(:barcode_item, organization: @organization, barcodeable: item2)
       visit subject
-      select BaseItem.first.name, from: "filters_by_item_partner_key"
+      select BaseItem.first.name, from: "filters[by_item_partner_key]"
       click_button "Filter"
 
       expect(page).to have_css("table tbody tr", count: 2)

spec/system/distribution_system_spec.rb

@@ -523,7 +523,7 @@
       # check for all distributions
       expect(page).to have_css("table tbody tr", count: 2)
       # filter
-      select(item1.name, from: "filters_by_item_id")
+      select(item1.name, from: "filters[by_item_id]")
       click_button("Filter")
       # check for filtered distributions
       expect(page).to have_css("table tbody tr", count: 1)
@@ -544,7 +544,7 @@
       # check for all distributions
       expect(page).to have_css("table tbody tr", count: 2)
       # filter
-      select(item_category.name, from: "filters_by_item_category_id")
+      select(item_category.name, from: "filters[by_item_category_id]")
       click_button("Filter")
       # check for filtered distributions
       expect(page).to have_css("table tbody tr", count: 1)
@@ -564,7 +564,7 @@
       # check for all distributions
       expect(page).to have_css("table tbody tr", count: 2)
       # filter
-      select(partner1.name, from: "filters_by_partner")
+      select(partner1.name, from: "filters[by_partner]")
       click_button("Filter")
       # check for filtered distributions
       expect(page).to have_css("table tbody tr", count: 1)
@@ -578,7 +578,7 @@
       # check for all distributions
       expect(page).to have_css("table tbody tr", count: 2)
       # filter
-      select(distribution1.state.humanize, from: "filters_by_state")
+      select(distribution1.state.humanize, from: "filters[by_state]")
       click_button("Filter")
       # check for filtered distributions
       expect(page).to have_css("table tbody tr", count: 1)

spec/system/donation_system_spec.rb

@@ -52,7 +52,7 @@
         create(:donation_site_donation)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select Donation::SOURCES[:misc], from: "filters_by_source"
+        select Donation::SOURCES[:misc], from: "filters[by_source]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -65,7 +65,7 @@
         create(:product_drive_donation, product_drive: b, product_drive_participant: x)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select a.name, from: "filters_by_product_drive"
+        select a.name, from: "filters[by_product_drive]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -78,7 +78,7 @@
         create(:product_drive_donation, product_drive: x, product_drive_participant: b)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select a.business_name, from: "filters_by_product_drive_participant"
+        select a.business_name, from: "filters[by_product_drive_participant]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -91,9 +91,9 @@
         create(:product_drive_donation, product_drive: x, product_drive_participant: b)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select a.business_name, from: "filters_by_product_drive_participant"
+        select a.business_name, from: "filters[by_product_drive_participant]"
         click_button "Filter"
-        expect(page).to have_select("filters_by_product_drive_participant", selected: a.business_name)
+        expect(page).to have_select("filters[by_product_drive_participant]", selected: a.business_name)
       end
 
       it "Filters by manufacturer" do
@@ -103,7 +103,7 @@
         create(:manufacturer_donation, manufacturer: b)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select a.name, from: "filters_from_manufacturer"
+        select a.name, from: "filters[from_manufacturer]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -113,7 +113,7 @@
         create(:donation, donation_site: location1)
         create(:donation, donation_site: location2)
         visit subject
-        select location1.name, from: "filters_from_donation_site"
+        select location1.name, from: "filters[from_donation_site]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -124,7 +124,7 @@
         create(:donation, storage_location: storage2)
         visit subject
         expect(page).to have_css("table tbody tr", count: 2)
-        select storage1.name, from: "filters_at_storage_location"
+        select storage1.name, from: "filters[at_storage_location]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end
@@ -139,10 +139,10 @@
         create(:donation_site_donation, storage_location: storage1)
         visit subject
         expect(page).to have_css("table tbody tr", count: 3)
-        select Donation::SOURCES[:misc], from: "filters_by_source"
+        select Donation::SOURCES[:misc], from: "filters[by_source]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 2)
-        select storage1.name, from: "filters_at_storage_location"
+        select storage1.name, from: "filters[at_storage_location]"
         click_button "Filter"
         expect(page).to have_css("table tbody tr", count: 1)
       end

spec/system/item_system_spec.rb

@@ -70,7 +70,7 @@
     create(:item, base_item: BaseItem.first)
     create(:item, base_item: BaseItem.last)
     visit url_prefix + "/items"
-    select BaseItem.first.name, from: "filters_by_base_item"
+    select BaseItem.first.name, from: "filters[by_base_item]"
     click_button "Filter"
     within "#items-table" do
       expect(page).to have_css("tbody tr", count: 1)

spec/system/product_drive_system_spec.rb

@@ -24,7 +24,7 @@
     end
 
     it "Shows the expected filters with the expected values" do
-      expect(page.has_select?('filters_by_name', with_options: @product_drives.map(&:name))).to be true
+      expect(page.has_select?('filters[by_name]', with_options: @product_drives.map(&:name))).to be true
       expect(page.has_field?('filters_date_range', with: this_year))
     end
 

spec/system/purchase_system_spec.rb

@@ -61,7 +61,7 @@
           create(:purchase, storage_location: storage2)
           visit subject
           expect(page).to have_css("table tbody tr", count: 2)
-          select storage1.name, from: "filters_at_storage_location"
+          select storage1.name, from: "filters[at_storage_location]"
           click_button "Filter"
           expect(page).to have_css("table tbody tr", count: 1)
         end
@@ -73,7 +73,7 @@
           create(:purchase, vendor: vendor2)
           visit subject
           expect(page).to have_css("table tbody tr", count: 2)
-          select vendor1.business_name, from: "filters_from_vendor"
+          select vendor1.business_name, from: "filters[from_vendor]"
           click_button "Filter"
           expect(page).to have_css("table tbody tr", count: 1)
         end

spec/system/request_system_spec.rb

@@ -59,7 +59,7 @@
         it "constrains the list" do
           visit subject
           expect(page).to have_css("table tbody tr", count: 5)
-          select(item2.name, from: "filters_by_request_item_id")
+          select(item2.name, from: "filters[by_request_item_id]")
           click_on "Filter"
           expect(page).to have_css("table tbody tr", count: 1)
         end
@@ -69,7 +69,7 @@
         it "constrains the list" do
           visit subject
           expect(page).to have_css("table tbody tr", count: 5)
-          select(partner2.name, from: "filters_by_partner")
+          select(partner2.name, from: "filters[by_partner]")
           click_on 'Filter'
           expect(page).to have_css("table tbody tr", count: 1)
         end
@@ -81,7 +81,7 @@
           # check for all requests
           expect(page).to have_css("table tbody tr", count: 5)
           # filter
-          select('Fulfilled', from: "filters_by_status")
+          select('Fulfilled', from: "filters[by_status]")
           click_on 'Filter'
           # check for filtered requests
           expect(page).to have_css("table tbody tr", count: 1)
@@ -92,7 +92,7 @@
         it "respects the applied filters" do
           visit subject
           expect(page).to have_css("table tbody tr", count: 5)
-          select(item2.name, from: "filters_by_request_item_id")
+          select(item2.name, from: "filters[by_request_item_id]")
           click_on 'Filter'
           expect(page).to have_css("table tbody tr", count: 1)
           click_on 'Export Requests'

spec/system/storage_location_system_spec.rb

@@ -98,7 +98,7 @@
       location3 = create(:storage_location, :with_items, item: item, item_quantity: 10, name: "Baz", discarded_at: rand(2.years).seconds.ago)
       visit subject
 
-      select item.name, from: "filters_containing"
+      select item.name, from: "filters[containing]"
       click_button "Filter"
 
       expect(page).to have_css("table tr", count: 2)
@@ -160,8 +160,8 @@
       create(:storage_location, :with_items, item: item3, item_quantity: 10, name: "Baz")
       visit subject
 
-      expect(page.all("select#filters_containing option").map(&:text).select(&:present?)).to eq(expected_order)
-      expect(page.all("select#filters_containing option").map(&:text).select(&:present?)).not_to eq(expected_order.reverse)
+      expect(page.all('select[name="filters[containing]"] option').map(&:text).select(&:present?)).to eq(expected_order)
+      expect(page.all('select[name="filters[containing]"] option').map(&:text).select(&:present?)).not_to eq(expected_order.reverse)
     end
   end
 

spec/system/transfer_system_spec.rb

@@ -122,13 +122,13 @@ def create_transfer(amount, from_name, to_name)
       create(:transfer, organization: @organization, from: to_storage_location, to: from_storage_location)
 
       visit subject
-      select to_storage_location.name, from: "filters_to_location"
+      select to_storage_location.name, from: "filters[to_location]"
       click_button "Filter"
 
       expect(page).to have_css("table tr", count: 2)
 
       visit subject
-      select from_storage_location.name, from: "filters_from_location"
+      select from_storage_location.name, from: "filters[from_location]"
       click_button "Filter"
 
       expect(page).to have_css("table tr", count: 2)