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