From e29605b79f25afac2d506c6371e1b528f120b670 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 27 Apr 2021 15:52:45 +1000 Subject: [PATCH] FEATURE: the ability to search users by custom fields (#12762) When the admin creates a new custom field they can specify if that field should be searchable or not. That setting is taken into consideration for quick search results. --- .../addon/components/admin-user-field-item.js | 19 +++++---- .../components/admin-user-field-item.hbs | 12 ++++-- .../app/widgets/search-menu-results.js | 6 +++ .../stylesheets/common/base/search-menu.scss | 5 +++ .../admin/user_fields_controller.rb | 2 +- app/models/user.rb | 7 +++- app/models/user_custom_field.rb | 2 + app/models/user_field.rb | 7 ++++ .../search_result_user_serializer.rb | 1 + app/serializers/user_field_serializer.rb | 1 + app/services/search_indexer.rb | 20 +++++++-- config/locales/client.en.yml | 4 ++ ...420015635_add_searchable_to_user_fields.rb | 7 ++++ lib/search.rb | 20 ++++++++- spec/lib/search_spec.rb | 41 +++++++++++++++++++ spec/services/search_indexer_spec.rb | 19 +++++++++ 16 files changed, 154 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20210420015635_add_searchable_to_user_fields.rb diff --git a/app/assets/javascripts/admin/addon/components/admin-user-field-item.js b/app/assets/javascripts/admin/addon/components/admin-user-field-item.js index c9642e3c94d..be2674ec6c4 100644 --- a/app/assets/javascripts/admin/addon/components/admin-user-field-item.js +++ b/app/assets/javascripts/admin/addon/components/admin-user-field-item.js @@ -44,25 +44,25 @@ export default Component.extend(bufferedProperty("userField"), { }, @discourseComputed( - "userField.editable", - "userField.required", - "userField.show_on_profile", - "userField.show_on_user_card" + "userField.{editable,required,show_on_profile,show_on_user_card,searchable}" ) - flags(editable, required, showOnProfile, showOnUserCard) { + flags(userField) { const ret = []; - if (editable) { + if (userField.editable) { ret.push(I18n.t("admin.user_fields.editable.enabled")); } - if (required) { + if (userField.required) { ret.push(I18n.t("admin.user_fields.required.enabled")); } - if (showOnProfile) { + if (userField.showOnProfile) { ret.push(I18n.t("admin.user_fields.show_on_profile.enabled")); } - if (showOnUserCard) { + if (userField.showOnUserCard) { ret.push(I18n.t("admin.user_fields.show_on_user_card.enabled")); } + if (userField.searchable) { + ret.push(I18n.t("admin.user_fields.searchable.enabled")); + } return ret.join(", "); }, @@ -78,6 +78,7 @@ export default Component.extend(bufferedProperty("userField"), { "required", "show_on_profile", "show_on_user_card", + "searchable", "options" ); diff --git a/app/assets/javascripts/admin/addon/templates/components/admin-user-field-item.hbs b/app/assets/javascripts/admin/addon/templates/components/admin-user-field-item.hbs index 211cc00b516..06ab7a286e2 100644 --- a/app/assets/javascripts/admin/addon/templates/components/admin-user-field-item.hbs +++ b/app/assets/javascripts/admin/addon/templates/components/admin-user-field-item.hbs @@ -22,19 +22,23 @@ {{/if}} {{#admin-form-row wrapLabel="true"}} - {{input type="checkbox" checked=buffered.editable}} {{i18n "admin.user_fields.editable.title"}} + {{input type="checkbox" checked=buffered.editable}} {{i18n "admin.user_fields.editable.title"}} {{/admin-form-row}} {{#admin-form-row wrapLabel="true"}} - {{input type="checkbox" checked=buffered.required}} {{i18n "admin.user_fields.required.title"}} + {{input type="checkbox" checked=buffered.required}} {{i18n "admin.user_fields.required.title"}} {{/admin-form-row}} {{#admin-form-row wrapLabel="true"}} - {{input type="checkbox" checked=buffered.show_on_profile}} {{i18n "admin.user_fields.show_on_profile.title"}} + {{input type="checkbox" checked=buffered.show_on_profile}} {{i18n "admin.user_fields.show_on_profile.title"}} {{/admin-form-row}} {{#admin-form-row wrapLabel="true"}} - {{input type="checkbox" checked=buffered.show_on_user_card}} {{i18n "admin.user_fields.show_on_user_card.title"}} + {{input type="checkbox" checked=buffered.show_on_user_card}} {{i18n "admin.user_fields.show_on_user_card.title"}} + {{/admin-form-row}} + + {{#admin-form-row wrapLabel="true"}} + {{input type="checkbox" checked=buffered.searchable}} {{i18n "admin.user_fields.searchable.title"}} {{/admin-form-row}} {{#admin-form-row}} diff --git a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js index aa22d040162..57b89ef8943 100644 --- a/app/assets/javascripts/discourse/app/widgets/search-menu-results.js +++ b/app/assets/javascripts/discourse/app/widgets/search-menu-results.js @@ -129,6 +129,12 @@ createSearchResult({ userTitles.push(h("span.username", formatUsername(u.username))); + if (u.custom_data) { + u.custom_data.forEach((row) => + userTitles.push(h("span.custom-field", `${row.name}: ${row.value}`)) + ); + } + const userResultContents = [ avatarImg("small", { template: u.avatar_template, diff --git a/app/assets/stylesheets/common/base/search-menu.scss b/app/assets/stylesheets/common/base/search-menu.scss index 0929477ee16..4076e019d97 100644 --- a/app/assets/stylesheets/common/base/search-menu.scss +++ b/app/assets/stylesheets/common/base/search-menu.scss @@ -222,6 +222,11 @@ font-size: $font-down-1; } + .custom-field { + color: var(--primary-high-or-secondary-low); + font-size: $font-down-2; + } + .name { color: var(--primary-high-or-secondary-low); font-size: $font-0; diff --git a/app/controllers/admin/user_fields_controller.rb b/app/controllers/admin/user_fields_controller.rb index 26fa6a51137..939ba7c69d4 100644 --- a/app/controllers/admin/user_fields_controller.rb +++ b/app/controllers/admin/user_fields_controller.rb @@ -3,7 +3,7 @@ class Admin::UserFieldsController < Admin::AdminController def self.columns - [:name, :field_type, :editable, :description, :required, :show_on_profile, :show_on_user_card, :position] + %i(name field_type editable description required show_on_profile show_on_user_card position searchable) end def create diff --git a/app/models/user.rb b/app/models/user.rb index 349cedf1776..e9c7af53654 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -37,6 +37,7 @@ class User < ActiveRecord::Base has_many :targeted_group_histories, dependent: :destroy, foreign_key: :target_user_id, class_name: 'GroupHistory' has_many :reviewable_scores, dependent: :destroy has_many :invites, foreign_key: :invited_by_id, dependent: :destroy + has_many :user_custom_fields, dependent: :destroy has_one :user_option, dependent: :destroy has_one :user_avatar, dependent: :destroy @@ -183,6 +184,9 @@ class User < ActiveRecord::Base # set to true to optimize creation and save for imports attr_accessor :import_mode + # Cache for user custom fields. Currently it is used to display quick search results + attr_accessor :custom_data + scope :with_email, ->(email) do joins(:user_emails).where("lower(user_emails.email) IN (?)", email) end @@ -1421,7 +1425,8 @@ class User < ActiveRecord::Base end def index_search - SearchIndexer.index(self) + # force is needed as user custom fields are updated using SQL and after_save callback is not triggered + SearchIndexer.index(self, force: true) end def clear_global_notice_if_needed diff --git a/app/models/user_custom_field.rb b/app/models/user_custom_field.rb index c38e65467e0..22f4d6d55e8 100644 --- a/app/models/user_custom_field.rb +++ b/app/models/user_custom_field.rb @@ -2,6 +2,8 @@ class UserCustomField < ActiveRecord::Base belongs_to :user + + scope :searchable, -> { joins("INNER JOIN user_fields ON user_fields.id = REPLACE(user_custom_fields.name, 'user_field_', '')::INTEGER AND user_fields.searchable IS TRUE AND user_custom_fields.name like 'user_field_%'") } end # == Schema Information diff --git a/app/models/user_field.rb b/app/models/user_field.rb index 1fcbd0ed564..9adc5b3131d 100644 --- a/app/models/user_field.rb +++ b/app/models/user_field.rb @@ -9,9 +9,15 @@ class UserField < ActiveRecord::Base has_many :user_field_options, dependent: :destroy accepts_nested_attributes_for :user_field_options + after_save :queue_index_search + def self.max_length 2048 end + + def queue_index_search + SearchIndexer.queue_users_reindex(UserCustomField.where(name: "user_field_#{self.id}").pluck(:user_id)) + end end # == Schema Information @@ -31,4 +37,5 @@ end # show_on_user_card :boolean default(FALSE), not null # external_name :string # external_type :string +# searchable :boolean default(FALSE), not null # diff --git a/app/serializers/search_result_user_serializer.rb b/app/serializers/search_result_user_serializer.rb index 9104e53ad1e..322a4c4319f 100644 --- a/app/serializers/search_result_user_serializer.rb +++ b/app/serializers/search_result_user_serializer.rb @@ -2,4 +2,5 @@ class SearchResultUserSerializer < BasicUserSerializer attributes :name + attributes :custom_data end diff --git a/app/serializers/user_field_serializer.rb b/app/serializers/user_field_serializer.rb index 0da8f956f4f..d897a13e443 100644 --- a/app/serializers/user_field_serializer.rb +++ b/app/serializers/user_field_serializer.rb @@ -9,6 +9,7 @@ class UserFieldSerializer < ApplicationSerializer :required, :show_on_profile, :show_on_user_card, + :searchable, :position, :options diff --git a/app/services/search_indexer.rb b/app/services/search_indexer.rb index 45d423925db..d8a970f1bea 100644 --- a/app/services/search_indexer.rb +++ b/app/services/search_indexer.rb @@ -126,12 +126,13 @@ class SearchIndexer end end - def self.update_users_index(user_id, username, name) + def self.update_users_index(user_id, username, name, custom_fields) update_index( table: 'user', id: user_id, a_weight: username, - b_weight: name + b_weight: name, + c_weight: custom_fields ) end @@ -165,6 +166,16 @@ class SearchIndexer SQL end + def self.queue_users_reindex(user_ids) + return if @disabled + + DB.exec(<<~SQL, user_ids: user_ids, version: REINDEX_VERSION) + UPDATE user_search_data + SET version = :version + WHERE user_search_data.user_id IN (:user_ids) + SQL + end + def self.queue_post_reindex(topic_id) return if @disabled @@ -222,7 +233,10 @@ class SearchIndexer end if User === obj && (obj.saved_change_to_username? || obj.saved_change_to_name? || force) - SearchIndexer.update_users_index(obj.id, obj.username_lower || '', obj.name ? obj.name.downcase : '') + SearchIndexer.update_users_index(obj.id, + obj.username_lower || '', + obj.name ? obj.name.downcase : '', + obj.user_custom_fields.searchable.map(&:value).join(" ")) end if Topic === obj && (obj.saved_change_to_title? || force) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 40213e18423..b84d8d71dc3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5008,6 +5008,10 @@ en: title: "Show on user card?" enabled: "shown on user card" disabled: "not shown on user card" + searchable: + title: "Searchable?" + enabled: "searchable" + disabled: "not searchable" field_types: text: "Text Field" diff --git a/db/migrate/20210420015635_add_searchable_to_user_fields.rb b/db/migrate/20210420015635_add_searchable_to_user_fields.rb new file mode 100644 index 00000000000..7634f987e3e --- /dev/null +++ b/db/migrate/20210420015635_add_searchable_to_user_fields.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSearchableToUserFields < ActiveRecord::Migration[6.0] + def change + add_column :user_fields, :searchable, :boolean, default: :false, null: false + end +end diff --git a/lib/search.rb b/lib/search.rb index aa7b1cafbc6..fdc064bfd3d 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -840,7 +840,8 @@ class Search def user_search return if SiteSetting.hide_user_profiles_from_public && !@guardian.user - users = User.includes(:user_search_data) + users = User + .includes(:user_search_data) .references(:user_search_data) .where(active: true) .where(staged: false) @@ -849,7 +850,24 @@ class Search .order("last_posted_at DESC") .limit(limit) + users_custom_data_query = DB.query(<<~SQL, user_ids: users.pluck(:id), term: "%#{@original_term.downcase}%") + SELECT user_custom_fields.user_id, user_fields.name, user_custom_fields.value FROM user_custom_fields + INNER JOIN user_fields ON user_fields.id = REPLACE(user_custom_fields.name, 'user_field_', '')::INTEGER AND user_fields.searchable IS TRUE + WHERE user_id IN (:user_ids) + AND user_custom_fields.name LIKE 'user_field_%' + AND user_custom_fields.value ILIKE :term + SQL + users_custom_data = users_custom_data_query.reduce({}) do |acc, row| + acc[row.user_id] = + Array.wrap(acc[row.user_id]) << { + name: row.name, + value: row.value + } + acc + end + users.each do |user| + user.custom_data = users_custom_data[user.id] || [] @results.add(user) end end diff --git a/spec/lib/search_spec.rb b/spec/lib/search_spec.rb index 13ca42582d9..5e5cec530aa 100644 --- a/spec/lib/search_spec.rb +++ b/spec/lib/search_spec.rb @@ -124,4 +124,45 @@ describe Search do end end end + + context "users" do + fab!(:user) { Fabricate(:user, username: "DonaldDuck") } + fab!(:user2) { Fabricate(:user) } + + before do + SearchIndexer.enable + SearchIndexer.index(user, force: true) + end + + it "finds users by their names or custom fields" do + result = Search.execute("donaldduck", guardian: Guardian.new(user2)) + expect(result.users).to contain_exactly(user) + + user_field = Fabricate(:user_field, name: "custom field") + UserCustomField.create!(user: user, value: "test", name: "user_field_#{user_field.id}") + Jobs::ReindexSearch.new.execute({}) + result = Search.execute("test", guardian: Guardian.new(user2)) + expect(result.users).to be_empty + + user_field.update!(searchable: true) + Jobs::ReindexSearch.new.execute({}) + result = Search.execute("test", guardian: Guardian.new(user2)) + expect(result.users).to contain_exactly(user) + + user_field2 = Fabricate(:user_field, name: "another custom field", searchable: true) + UserCustomField.create!(user: user, value: "longer test", name: "user_field_#{user_field2.id}") + UserCustomField.create!(user: user2, value: "second user test", name: "user_field_#{user_field2.id}") + SearchIndexer.index(user, force: true) + SearchIndexer.index(user2, force: true) + result = Search.execute("test", guardian: Guardian.new(user2)) + + expect(result.users.first.custom_data).to eq([ + { name: "custom field", value: "test" }, + { name: "another custom field", value: "longer test" } + ]) + expect(result.users.last.custom_data).to eq([ + { name: "another custom field", value: "second user test" } + ]) + end + end end diff --git a/spec/services/search_indexer_spec.rb b/spec/services/search_indexer_spec.rb index 659ef8cd4f6..6774543c69b 100644 --- a/spec/services/search_indexer_spec.rb +++ b/spec/services/search_indexer_spec.rb @@ -292,4 +292,23 @@ describe SearchIndexer do ) end end + + describe '.queue_users_reindex' do + let!(:user) { Fabricate(:user) } + let!(:user2) { Fabricate(:user) } + + it 'should reset the version of search data for all users' do + SearchIndexer.index(user, force: true) + SearchIndexer.index(user2, force: true) + SearchIndexer.queue_users_reindex([user.id]) + + expect(user.reload.user_search_data.version).to eq( + SearchIndexer::REINDEX_VERSION + ) + + expect(user2.reload.user_search_data.version).to eq( + SearchIndexer::USER_INDEX_VERSION + ) + end + end end