diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 65166cf7ce..7b8054dd93 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -26,12 +26,19 @@ def index affiliations: :organization, event_registrations: :event, categorizable_items: { category: :category_type } - ).references(:user)) + )) filters = authorized_scope(params, as: :index).to_unsafe_h + name_email = filters.delete(:name_email) + contact_info = filters.delete(:contact_info) filtered = base_scope.search_by_params(filters) - .order(:first_name, :last_name) - @count_display = count_display(filtered.count) - @people = filtered.paginate(page: params[:page], per_page: per_page) + filtered = filtered.search_name_email(name_email) + filtered = filtered.search_contact(contact_info) + filtered = filtered.distinct if contact_info.present? + filtered = filtered.order(:first_name, :last_name) + + total = filtered.count + @count_display = count_display(total) + @people = filtered.paginate(page: params[:page], per_page: per_page, total_entries: total) render :people_results else @@ -41,7 +48,13 @@ def index def email_addresses authorize! Person, to: :manage? - people = authorized_scope(Person.includes(:user)).search_by_params(params.to_unsafe_h) + filters = params.to_unsafe_h + name_email = filters.delete(:name_email) + contact_info = filters.delete(:contact_info) + people = Person.includes(:user).search_by_params(filters) + people = people.search_name_email(name_email) + people = people.search_contact(contact_info) + people = people.distinct if contact_info.present? @email_addresses = people.filter_map { |person| person.preferred_email.presence }.uniq.sort end diff --git a/app/models/person.rb b/app/models/person.rb index c91a334c19..466222c046 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -207,6 +207,27 @@ def known_email_2_type? = email_2_type.in?(CONTACT_TYPES) options :all, type: :text, default: true, default_operator: :or end + search_scope :search_name_email do + attributes :first_name, :legal_first_name, :last_name, :email, :email_2 + attributes all: [ :first_name, :legal_first_name, :last_name, :email, :email_2 ] + options :all, type: :text, default: true, default_operator: :or + end + + search_scope :search_contact do + scope { left_joins(:contact_methods, :addresses) } + attributes phone: "contact_methods.value" + attributes street: "addresses.street_address" + attributes city: "addresses.city" + attributes state: "addresses.state" + attributes zip: "addresses.zip_code" + attributes county: "addresses.county" + attributes country: "addresses.country" + attributes all: [ "contact_methods.value", + "addresses.street_address", "addresses.city", "addresses.state", + "addresses.zip_code", "addresses.county", "addresses.country" ] + options :all, type: :text, default: true, default_operator: :or + end + FIRST_NAME_HIDDEN_PREFERENCES = %w[last_name_only].freeze LAST_NAME_HIDDEN_PREFERENCES = %w[first_name_last_initial first_name_only].freeze @@ -214,7 +235,6 @@ def known_email_2_type? = email_2_type.in?(CONTACT_TYPES) (people.first_name LIKE :pattern AND COALESCE(people.display_name_preference, '') NOT IN (:first_name_hidden)) OR (people.last_name LIKE :pattern AND COALESCE(people.display_name_preference, '') NOT IN (:last_name_hidden)) OR (people.profile_show_email AND (people.email LIKE :pattern OR users.email LIKE :pattern)) - OR (people.profile_show_phone AND (users.phone LIKE :pattern OR contact_methods.value LIKE :pattern)) SQL # Spaces and periods are dropped on both sides so an initialled name ("Cathy S.") finds itself. @@ -247,7 +267,7 @@ def known_email_2_type? = email_2_type.in?(CONTACT_TYPES) def self.shared_contact_search(query) terms = query.scan(SEARCH_TERM_PATTERN).map { |groups| groups.compact.first }.compact_blank terms.reduce(all) do |relation, term| - matches = unscoped.left_joins(:user, :active_phones).where( + matches = unscoped.left_joins(:user).where( SHARED_CONTACT_MATCH_SQL, pattern: "%#{sanitize_sql_like(term)}%", first_name_hidden: FIRST_NAME_HIDDEN_PREFERENCES, diff --git a/app/policies/person_policy.rb b/app/policies/person_policy.rb index a60a03ee11..aabf1ecdb3 100644 --- a/app/policies/person_policy.rb +++ b/app/policies/person_policy.rb @@ -20,11 +20,11 @@ class PersonPolicy < ApplicationPolicy # behind the same admin gate the :index params_filter enforces. ADMIN_ONLY_FILTERS = %i[ role facilitator_status membership_status - staff_tag_ids topic_subscription_type_id + staff_tag_ids topic_subscription_type_id contact_info ].freeze SHARED_ONLY_FILTERS = { - contact_info: :shared_contact_info, + name_email: :shared_contact_info, sector_ids: :shared_sector_ids, age_range_ids: :shared_age_range_ids, organization_name: :shared_organization_name, diff --git a/app/views/people/_search_boxes.html.erb b/app/views/people/_search_boxes.html.erb index c893543c1d..59fca28040 100644 --- a/app/views/people/_search_boxes.html.erb +++ b/app/views/people/_search_boxes.html.erb @@ -7,8 +7,8 @@ class: "space-y-4") do |f| %> <%= hidden_field_tag "organization_id", params[:organization_id] if params[:organization_id].present? %>
- - <%= render "shared/filter_text", param: :contact_info, label: "Name, email, or phone (if shared)", submit: true, + + <%= render "shared/filter_text", param: :name_email, label: "Name or email", submit: true, wrapper_class: "w-full md:flex-1" %> @@ -36,7 +36,7 @@ <% if allowed_to?(:manage?, Person) %>

Admin-only filters

-
+
<%= render "shared/filter_select", param: :role, label: "Role", options: Person::ROLE_FILTER_OPTIONS, selected: params[:role], @@ -70,6 +70,11 @@ selected: Array(params[:topic_subscription_type_id]).map(&:to_i), placeholder: "Any topic", wrapper_class: "w-full", hint: link_to("Manage topics", topic_subscription_types_path, class: eyebrow_link_class, target: "_blank", rel: "noopener") %> + + + <%= render "shared/filter_text", param: :contact_info, label: "Contact info", submit: true, + wrapper_class: "w-full", + tooltip: "Prefixes: phone:, street:, city:, state:, zip:, county:, country:" %>
<% end %> diff --git a/app/views/shared/_filter_text.html.erb b/app/views/shared/_filter_text.html.erb index de7964b522..f380591174 100644 --- a/app/views/shared/_filter_text.html.erb +++ b/app/views/shared/_filter_text.html.erb @@ -2,6 +2,7 @@ bars, so every free-text filter renders the same markup. Locals: param – input name/id (Symbol or String) label – optional field label (omitted when nil) + tooltip – optional help text shown as a title on an info icon beside the label placeholder – optional input placeholder value – optional input value (defaults to params[param]) submit – true adds the magnifier search button inside the field @@ -17,7 +18,14 @@ placeholder: local_assigns[:placeholder], **local_assigns.fetch(:input_options, {}) %> <%= tag.div class: local_assigns.fetch(:wrapper_class, "w-full md:w-48") do %> - <%= label_tag param, label_text, class: local_assigns.fetch(:label_class, search_label_class) if label_text %> + <% if label_text %> + <%= label_tag param, nil, class: local_assigns.fetch(:label_class, search_label_class) do %> + <%= label_text %> + <% if local_assigns[:tooltip].present? %> + + <% end %> + <% end %> + <% end %> <% if submit %>
<%= input %> diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 6a5f2d83db..acfac6717a 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -700,6 +700,11 @@ def phone_numbers expect(results).not_to include(person_alice) end + it 'search_name_email matches names and emails' do + expect(Person.search_name_email('Alice')).to include(person_alice) + expect(Person.search_name_email('alice@example')).to include(person_alice) + end + it 'filters by shared_contact_info matching a full name' do results = Person.search_by_params(shared_contact_info: 'Alice Smith') expect(results).to include(person_alice) @@ -714,20 +719,32 @@ def phone_numbers expect(Person.search_by_params(shared_contact_info: 'alice@example')).not_to include(person_alice) end - it 'filters by shared_contact_info matching a shared login phone' do - person_bob.update!(profile_show_phone: true) - person_bob.user.update!(phone: '555-0199') + it 'search_contact matches a contact method' do + create(:contact_method, contactable: person_alice, value: '555-0199') - expect(Person.search_by_params(shared_contact_info: '555-0199')).to include(person_bob) + expect(Person.search_contact('555-0199')).to include(person_alice) + expect(Person.search_contact('555-0199')).not_to include(person_bob) end - it 'filters by shared_contact_info matching terms across a name and a shared phone' do - person_alice.update!(profile_show_phone: true) - person_alice.user.update!(phone: '555-0199') + it 'search_contact matches an inactive contact method' do + create(:contact_method, contactable: person_alice, value: '555-0199', inactive: true) - results = Person.search_by_params(shared_contact_info: 'Alice 555-0199') - expect(results).to include(person_alice) - expect(results).not_to include(person_bob) + expect(Person.search_contact('555-0199')).to include(person_alice) + end + + it 'search_contact matches an address' do + create(:address, addressable: person_bob, street_address: '123 Hiddenway', city: 'Portland') + + expect(Person.search_contact('Hiddenway')).to include(person_bob) + end + + it 'search_contact supports attribute prefixes' do + create(:address, addressable: person_bob, street_address: '123 Hiddenway', city: 'Portland') + create(:contact_method, contactable: person_alice, value: '555-0199') + + expect(Person.search_contact('city:portland')).to include(person_bob) + expect(Person.search_contact('city:portland')).not_to include(person_alice) + expect(Person.search_contact('phone:555')).to include(person_alice) end it 'filters by shared_contact_info treating a quoted phrase as one term' do diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index 9a75f946df..95232c6b14 100644 --- a/spec/policies/person_policy_spec.rb +++ b/spec/policies/person_policy_spec.rb @@ -412,7 +412,8 @@ def in_production describe "params_filter(:index)" do let(:filters) do ActionController::Parameters.new( - contact_info: "ada", role: "story_authors", staff_tag_ids: [ "1" ], + name_email: "ada", contact_info: "555-0142", + role: "story_authors", staff_tag_ids: [ "1" ], topic_subscription_type_id: [ "2" ], facilitator_status: "active", membership_status: "paid", sector_ids: [ "4" ], age_range_ids: [ "5" ], organization_name: "Acme", organization_id: "3" diff --git a/spec/requests/people_email_addresses_spec.rb b/spec/requests/people_email_addresses_spec.rb index a0c4189a1f..8421d88f48 100644 --- a/spec/requests/people_email_addresses_spec.rb +++ b/spec/requests/people_email_addresses_spec.rb @@ -12,13 +12,23 @@ create(:person, first_name: "Bob", last_name: "Builder", email: "bob@example.com", user: nil) - get email_addresses_people_path(contact_info: "Alice") + get email_addresses_people_path(name_email: "Alice") expect(response).to have_http_status(:success) expect(response.body).to include("alice@example.com") expect(response.body).not_to include("bob@example.com") end + it "matches contact details like the people index" do + person = create(:person, first_name: "Carol", last_name: "Phoner", + email: "carol@example.com", user: nil) + create(:contact_method, contactable: person, value: "555-0142") + + get email_addresses_people_path(contact_info: "555-0142") + + expect(response.body).to include("carol@example.com") + end + it "falls back to the person's preferred email when the email column is blank" do create(:person, email: nil, user: create(:user, email: "dana@example.com")) create(:person, email: nil, email_2: "rio@example.com", user: nil) @@ -30,7 +40,7 @@ end it "shows the empty state when no people match" do - get email_addresses_people_path(contact_info: "nobody-matches-this") + get email_addresses_people_path(name_email: "nobody-matches-this") expect(response.body).to include("No email addresses found.") end @@ -47,10 +57,10 @@ describe "Email addresses link on the filtered index frame" do it "carries the active filter into the Email addresses link" do - get people_path(contact_info: "Alice"), headers: { "Turbo-Frame" => "people_results" } + get people_path(name_email: "Alice"), headers: { "Turbo-Frame" => "people_results" } expect(response).to have_http_status(:success) - expect(response.body).to include(email_addresses_people_path(contact_info: "Alice")) + expect(response.body).to include(email_addresses_people_path(name_email: "Alice")) end end end diff --git a/spec/requests/people_search_spec.rb b/spec/requests/people_search_spec.rb index cb2903d3ae..78786ce669 100644 --- a/spec/requests/people_search_spec.rb +++ b/spec/requests/people_search_spec.rb @@ -15,6 +15,11 @@ expect(response).to have_http_status(:ok) end + it "shows the admin-only contact info filter" do + get people_path + expect(response.body).to include('name="contact_info"') + end + it "breaks the person profile button out of the results frame" do get people_path, headers: turbo_headers expect_frame_breakout(response.body, person_path(person_alice)) @@ -27,8 +32,8 @@ expect(response.body).to include("Bob") end - it "filters by contact_info" do - get people_path, params: { contact_info: "Alice" }, headers: turbo_headers + it "filters by name_email" do + get people_path, params: { name_email: "Alice" }, headers: turbo_headers expect(response).to have_http_status(:ok) expect(response.body).to include("Alice") expect(response.body).not_to include("Bob") @@ -162,6 +167,7 @@ def published_person(first_name) get people_path expect(response.body).not_to include("Admin-only filters") expect(response.body).not_to include("staff_tag_ids") + expect(response.body).not_to include('name="contact_info"') end it "ignores a staff tag filter passed by URL" do @@ -181,7 +187,7 @@ def published_person(first_name) end it "still applies the filters the bar does offer them" do - get people_path, params: { contact_info: "Markedone" }, headers: turbo_headers + get people_path, params: { name_email: "Markedone" }, headers: turbo_headers expect(response.body).to include("Markedone") expect(response.body).not_to include("Plainone") @@ -189,7 +195,7 @@ def published_person(first_name) describe "contact info search" do def search(term) - get people_path, params: { contact_info: term }, headers: turbo_headers + get people_path, params: { name_email: term }, headers: turbo_headers response.body end @@ -218,21 +224,11 @@ def search(term) expect(search("Hiddenway")).not_to include("Markedone") end - it "matches a shared phone" do + it "ignores a contact details filter passed by URL" do marked.update!(profile_show_phone: true) create(:contact_method, contactable: marked, value: "555-0142") - expect(search("555-0142")).to include("Markedone") - end - - it "skips a phone the person hides" do - marked.update!(profile_show_phone: false) - create(:contact_method, contactable: marked, value: "555-0142") - expect(search("555-0142")).not_to include("Markedone") - end - - it "skips an inactive phone" do - create(:contact_method, contactable: marked, value: "555-0142", inactive: true) - expect(search("555-0142")).not_to include("Markedone") + get people_path, params: { contact_info: "555-0142" }, headers: turbo_headers + expect(response.body).to include("Markedone") end %w[first_name_last_initial first_name_only].each do |preference| @@ -344,6 +340,11 @@ def search(term) let!(:person) { create(:person, first_name: "Hiddenfacts", profile_show_email: false, profile_show_phone: false) } def search(term) + get people_path, params: { name_email: term }, headers: turbo_headers + response.body + end + + def search_contact_details(term) get people_path, params: { contact_info: term }, headers: turbo_headers response.body end @@ -363,19 +364,19 @@ def search(term) expect(search("Legalname")).to include("Hiddenfacts") end - it "matches addresses" do - create(:address, addressable: person, street_address: "123 Hiddenway") - expect(search("Hiddenway")).to include("Hiddenfacts") - end - - it "matches a phone the person hides" do + it "matches a contact method via the contact info filter" do create(:contact_method, contactable: person, value: "555-0142") - expect(search("555-0142")).to include("Hiddenfacts") + expect(search_contact_details("555-0142")).to include("Hiddenfacts") end - it "matches an inactive phone" do + it "matches an inactive contact method via the contact info filter" do create(:contact_method, contactable: person, value: "555-0142", inactive: true) - expect(search("555-0142")).to include("Hiddenfacts") + expect(search_contact_details("555-0142")).to include("Hiddenfacts") + end + + it "matches an address via the contact info filter" do + create(:address, addressable: person, street_address: "123 Hiddenway") + expect(search_contact_details("Hiddenway")).to include("Hiddenfacts") end %w[first_name_last_initial first_name_only].each do |preference|