From d3a1ffb4354de230f959faf3043b99ab68c5d87a Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Mon, 10 Aug 2026 01:58:03 -0400 Subject: [PATCH] Sort product drive participant dropdowns the way a person reads them The `alphabetized` scope ordered by `contact_name`, but every dropdown shows `business_name` and only falls back to `contact_name` when it is blank, so the lists were sorted on a column the user cannot see. Sorting on the displayed name is not enough on its own. `ORDER BY name` uses the database collation, which differs between environments: a `C.UTF-8` cluster puts every capitalised name before every lowercase one, while the `postgres:12.3` image CI runs is initialised with `en_US.utf8` and does not. Plain text ordering also puts "Store 10" before "Store 9". `DISPLAY_NAME_ORDER` is an ORDER BY expression that lowercases the displayed name and zero-pads runs of digits, so the ordering is case-insensitive and natural whatever the cluster's collation is. It is an expression rather than a Postgres function or an ICU collation because the schema is maintained as `schema.rb`, which carries neither. It is written as a literal with no interpolation, so it cannot carry a value in. `create.js.erb` rebuilt the dropdown without the scope at all, so the list lost its order as soon as a participant was added from the modal. It now reuses `display_name`, which is also what the donation form and the donation filter label the options with, so the sort key and the label can no longer drift apart. The donation filter previously labelled options with `business_name` alone, leaving participants who only have a contact name as blank entries. Co-Authored-By: Claude Opus 5 (1M context) --- app/models/product_drive_participant.rb | 22 ++++++++++++++++++- app/views/donations/_donation_form.html.erb | 2 +- app/views/donations/index.html.erb | 2 +- .../product_drive_participants/create.js.erb | 2 +- spec/models/product_drive_participant_spec.rb | 18 +++++++++++++++ 5 files changed, 42 insertions(+), 4 deletions(-) diff --git a/app/models/product_drive_participant.rb b/app/models/product_drive_participant.rb index dd2d1c656e..49967628a6 100644 --- a/app/models/product_drive_participant.rb +++ b/app/models/product_drive_participant.rb @@ -30,7 +30,27 @@ class ProductDriveParticipant < ApplicationRecord validates :business_name, presence: { message: "Must provide a name or a business name" }, if: proc { |pdp| pdp.contact_name.blank? } validates :comment, length: { maximum: 500 } - scope :alphabetized, -> { order(:contact_name) } + # Orders on the name the drop-downs actually show - `display_name`, which is + # `business_name` falling back to `contact_name` - rather than on the database + # collation, which is not the same everywhere: a `C.UTF-8` cluster puts every + # capitalised name before every lowercase one and the `en_US.utf8` image CI + # runs does not. Runs of digits are zero padded so that they compare by value + # and "Store 9" comes before "Store 10". + # + # Written as a literal because the schema is maintained as `schema.rb`, which + # carries neither a Postgres function nor an ICU collation - both would + # disappear on `db:schema:load`. + DISPLAY_NAME_ORDER = Arel.sql(<<~SQL.squish) + (SELECT string_agg( + CASE WHEN chunk[1] ~ '^[0-9]' THEN lpad(chunk[1], 20, '0') ELSE chunk[1] END, + '' ORDER BY idx) + FROM regexp_matches( + lower(coalesce(NULLIF(business_name, ''), contact_name, '')), + '[0-9]+|[^0-9]+', 'g') + WITH ORDINALITY AS chunks(chunk, idx)) + SQL + + scope :alphabetized, -> { order(DISPLAY_NAME_ORDER) } scope :by_business_name, ->(business_name) { where("business_name ILIKE ?", "%#{business_name}%") } scope :by_contact_name, ->(contact_name) { where("contact_name ILIKE ?", "%#{contact_name}%") } scope :with_volumes, -> { diff --git a/app/views/donations/_donation_form.html.erb b/app/views/donations/_donation_form.html.erb index fde46484ac..b43993fdbe 100644 --- a/app/views/donations/_donation_form.html.erb +++ b/app/views/donations/_donation_form.html.erb @@ -43,7 +43,7 @@ collection: @product_drive_participants, selected: donation_form.product_drive_participant_id, include_blank: true, - label_method: lambda { |x| "#{x.try(:business_name).presence || x.try(:contact_name)}" }, + label_method: :display_name, label: "Product Drive Participant", error: "Which product drive participant was this from?", wrapper: :input_group %> diff --git a/app/views/donations/index.html.erb b/app/views/donations/index.html.erb index f128e67b9d..8cb16e204d 100644 --- a/app/views/donations/index.html.erb +++ b/app/views/donations/index.html.erb @@ -65,7 +65,7 @@
<%= filter_select(scope: :by_product_drive_participant, collection: @donation_info.product_drive_participants, - value: :business_name, + value: :display_name, selected: @donation_info.selected_product_drive_participant) %>
<% end %> diff --git a/app/views/product_drive_participants/create.js.erb b/app/views/product_drive_participants/create.js.erb index 9f291950f7..66b7cb9e8c 100644 --- a/app/views/product_drive_participants/create.js.erb +++ b/app/views/product_drive_participants/create.js.erb @@ -2,6 +2,6 @@ $("#modal_new").modal("hide"); $("#donation_product_drive_participant_id").empty(); $("#donation_product_drive_participant_id"). -html('<%= j options_from_collection_for_select(current_organization.product_drive_participants, :id, lambda { |p| p.business_name.present? ? p.business_name : p.contact_name }) %>'); +html('<%= j options_from_collection_for_select(current_organization.product_drive_participants.alphabetized, :id, :display_name) %>'); $("#donation_product_drive_participant_id").append(''); $("#donation_product_drive_participant_id").val('<%= @product_drive_participant[:id] %>'); diff --git a/spec/models/product_drive_participant_spec.rb b/spec/models/product_drive_participant_spec.rb index 6f6c08e6a2..55d7b0fc42 100644 --- a/spec/models/product_drive_participant_spec.rb +++ b/spec/models/product_drive_participant_spec.rb @@ -71,6 +71,24 @@ expect(ProductDriveParticipant.by_contact_name("Shellstrop")).to match_array([eleanor, donna]) end end + + describe ".alphabetized" do + it "orders by the name that is displayed, falling back to contact name" do + zebra = create(:product_drive_participant, business_name: "Zebra Foods", contact_name: "adam") + no_business = create(:product_drive_participant, business_name: nil, contact_name: "molly") + aardvark = create(:product_drive_participant, business_name: "Aardvark Supplies", contact_name: "zoe") + + expect(ProductDriveParticipant.alphabetized).to eq([aardvark, no_business, zebra]) + end + + it "orders numbers by value rather than by digit" do + tenth = create(:product_drive_participant, business_name: "Store 10") + second = create(:product_drive_participant, business_name: "Store 2") + ninth = create(:product_drive_participant, business_name: "Store 9") + + expect(ProductDriveParticipant.alphabetized).to eq([second, ninth, tenth]) + end + end end context "Methods" do