From 3eea900729b218a36ef7f9e8af84b80cc6fae214 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". `NaturallySortable.natural_order` builds an ORDER BY expression that lowercases the value 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. `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/concerns/naturally_sortable.rb | 28 +++++++++++++++++++ app/models/product_drive_participant.rb | 3 +- 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 ++++++++++++ 6 files changed, 51 insertions(+), 4 deletions(-) create mode 100644 app/models/concerns/naturally_sortable.rb diff --git a/app/models/concerns/naturally_sortable.rb b/app/models/concerns/naturally_sortable.rb new file mode 100644 index 0000000000..e3df34a0a3 --- /dev/null +++ b/app/models/concerns/naturally_sortable.rb @@ -0,0 +1,28 @@ +# Sorts records the way a person reads a list rather than the way Postgres +# compares text: case-insensitively, and with runs of digits compared by value +# so that "Item 9" comes before "Item 10". +module NaturallySortable + extend ActiveSupport::Concern + + # Digit runs are zero padded to this width before they are compared, so any + # number shorter than this sorts by value. Longer numbers are truncated by + # `lpad` and fall back to comparing their first digits. + DIGIT_PADDING = 20 + + class_methods do + # Returns an ORDER BY expression that splits +sql_expression+ into runs of + # digits and non-digits, zero pads the digit runs and lowercases the rest. + # + # The expression is interpolated verbatim, so only pass a literal written + # here - never user input. + def natural_order(sql_expression) + Arel.sql(<<~SQL.squish) + (SELECT string_agg( + CASE WHEN chunk[1] ~ '^[0-9]' THEN lpad(chunk[1], #{DIGIT_PADDING}, '0') ELSE chunk[1] END, + '' ORDER BY idx) + FROM regexp_matches(lower(coalesce(#{sql_expression}, '')), '[0-9]+|[^0-9]+', 'g') + WITH ORDINALITY AS chunks(chunk, idx)) + SQL + end + end +end diff --git a/app/models/product_drive_participant.rb b/app/models/product_drive_participant.rb index dd2d1c656e..43c2c69a53 100644 --- a/app/models/product_drive_participant.rb +++ b/app/models/product_drive_participant.rb @@ -20,6 +20,7 @@ class ProductDriveParticipant < ApplicationRecord has_paper_trail include Filterable include Geocodable + include NaturallySortable include Provideable has_many :donations, inverse_of: :product_drive_participant, dependent: :destroy @@ -30,7 +31,7 @@ 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) } + scope :alphabetized, -> { order(natural_order("COALESCE(NULLIF(business_name, ''), contact_name)")) } 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