diff --git a/Gemfile b/Gemfile index 9050c36deb..cf644d001e 100644 --- a/Gemfile +++ b/Gemfile @@ -33,6 +33,8 @@ gem "paper_trail" gem "rolify", "~> 6.0" # Enforces "safe" migrations. gem "strong_migrations" +# Manages Postgres functions and triggers as versioned files. +gem "fx" # used in events gem 'dry-struct' # Use solid_cache as a cache store diff --git a/Gemfile.lock b/Gemfile.lock index bdf547325f..63fbc6626e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -300,6 +300,9 @@ GEM foreman (0.90.0) thor (~> 1.4) formatador (1.1.0) + fx (0.11.0) + activerecord (>= 7.2) + railties (>= 7.2) geocoder (1.8.6) base64 (>= 0.1.0) csv (>= 3.0.0) @@ -803,6 +806,7 @@ DEPENDENCIES flipper-active_record flipper-ui foreman + fx geocoder guard-rspec icalendar diff --git a/app/models/product_drive_participant.rb b/app/models/product_drive_participant.rb index dd2d1c656e..613fa5b3d5 100644 --- a/app/models/product_drive_participant.rb +++ b/app/models/product_drive_participant.rb @@ -30,7 +30,22 @@ 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. + # + # `leading_digit_sort_key` (db/functions/leading_digit_sort_key_v01.sql, added + # via the `fx` gem) zero-pads a leading run of digits so "2" sorts before + # "10" - it does not natural-sort a digit run in the middle of the name. That + # scope was the deliberate tradeoff, over a fuller tokenizing version, agreed + # on in https://github.com/rubyforgood/human-essentials/pull/5656. + DISPLAY_NAME_ORDER = Arel.sql( + "leading_digit_sort_key(coalesce(NULLIF(business_name, ''), contact_name, ''))" + ) + + 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/db/functions/leading_digit_sort_key_v01.sql b/db/functions/leading_digit_sort_key_v01.sql new file mode 100644 index 0000000000..c28cba94a3 --- /dev/null +++ b/db/functions/leading_digit_sort_key_v01.sql @@ -0,0 +1,15 @@ +-- Sort key that treats a run of digits *at the start* of the string as a +-- number instead of a sequence of characters, so "2" sorts before "10". +-- Anything past the leading digits (or the whole string, if it doesn't start +-- with a digit) is compared as plain lowercased text - a digit run elsewhere +-- in the string (e.g. "Store 9" vs. "Store 10") is not natural-sorted. That +-- narrower scope, instead of full natural sort, was the deliberate tradeoff +-- settled on in https://github.com/rubyforgood/human-essentials/pull/5656. +CREATE FUNCTION leading_digit_sort_key(value text) RETURNS text AS $$ + SELECT CASE + WHEN lower(coalesce(value, '')) ~ '^[0-9]+' + THEN lpad(substring(lower(value) from '^[0-9]+'), 20, '0') + || substring(lower(value) from '^[0-9]+(.*)$') + ELSE lower(coalesce(value, '')) + END; +$$ LANGUAGE sql IMMUTABLE PARALLEL SAFE; diff --git a/db/migrate/20260921234706_create_function_leading_digit_sort_key.rb b/db/migrate/20260921234706_create_function_leading_digit_sort_key.rb new file mode 100644 index 0000000000..f4d789641b --- /dev/null +++ b/db/migrate/20260921234706_create_function_leading_digit_sort_key.rb @@ -0,0 +1,5 @@ +class CreateFunctionLeadingDigitSortKey < ActiveRecord::Migration[8.1] + def change + create_function :leading_digit_sort_key + end +end diff --git a/db/schema.rb b/db/schema.rb index c9e1d95553..e653cfb10a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_08_29_112930) do +ActiveRecord::Schema[8.1].define(version: 2026_09_21_234706) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -898,4 +898,19 @@ add_foreign_key "tags", "organizations" add_foreign_key "units", "organizations" add_foreign_key "users", "users_roles", column: "last_role_id", on_delete: :nullify + + create_function :leading_digit_sort_key, sql_definition: <<-'SQL' + CREATE OR REPLACE FUNCTION public.leading_digit_sort_key(value text) + RETURNS text + LANGUAGE sql + IMMUTABLE PARALLEL SAFE + AS $function$ + SELECT CASE + WHEN lower(coalesce(value, '')) ~ '^[0-9]+' + THEN lpad(substring(lower(value) from '^[0-9]+'), 20, '0') + || substring(lower(value) from '^[0-9]+(.*)$') + ELSE lower(coalesce(value, '')) + END; + $function$ + SQL end diff --git a/spec/models/product_drive_participant_spec.rb b/spec/models/product_drive_participant_spec.rb index 6f6c08e6a2..7b2cf51c25 100644 --- a/spec/models/product_drive_participant_spec.rb +++ b/spec/models/product_drive_participant_spec.rb @@ -71,6 +71,32 @@ 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 a leading number by value rather than by digit" do + tenth = create(:product_drive_participant, business_name: "10 Warehouse Way") + second = create(:product_drive_participant, business_name: "2 Warehouse Way") + ninth = create(:product_drive_participant, business_name: "9 Warehouse Way") + + expect(ProductDriveParticipant.alphabetized).to eq([second, ninth, tenth]) + end + + it "does not natural-sort a number in the middle of the name (leading-digit only, by design - see PR #5656)" 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([tenth, second, ninth]) + end + end end context "Methods" do