Skip to content

Commit

Permalink
FEATURE: the ability to search users by custom fields (discourse#12762)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lis2 authored Apr 27, 2021
1 parent 8aeeadd commit e29605b
Show file tree
Hide file tree
Showing 16 changed files with 154 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ");
},
Expand All @@ -78,6 +78,7 @@ export default Component.extend(bufferedProperty("userField"), {
"required",
"show_on_profile",
"show_on_user_card",
"searchable",
"options"
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}} <span>{{i18n "admin.user_fields.editable.title"}}</span>
{{/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}} <span>{{i18n "admin.user_fields.required.title"}}</span>
{{/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}} <span>{{i18n "admin.user_fields.show_on_profile.title"}}</span>
{{/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}} <span>{{i18n "admin.user_fields.show_on_user_card.title"}}</span>
{{/admin-form-row}}

{{#admin-form-row wrapLabel="true"}}
{{input type="checkbox" checked=buffered.searchable}} <span>{{i18n "admin.user_fields.searchable.title"}}</span>
{{/admin-form-row}}

{{#admin-form-row}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions app/assets/stylesheets/common/base/search-menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/user_fields_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1421,7 +1425,8 @@ def expire_old_email_tokens
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
Expand Down
2 changes: 2 additions & 0 deletions app/models/user_custom_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions app/models/user_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,4 +37,5 @@ def self.max_length
# show_on_user_card :boolean default(FALSE), not null
# external_name :string
# external_type :string
# searchable :boolean default(FALSE), not null
#
1 change: 1 addition & 0 deletions app/serializers/search_result_user_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@

class SearchResultUserSerializer < BasicUserSerializer
attributes :name
attributes :custom_data
end
1 change: 1 addition & 0 deletions app/serializers/user_field_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class UserFieldSerializer < ApplicationSerializer
:required,
:show_on_profile,
:show_on_user_card,
:searchable,
:position,
:options

Expand Down
20 changes: 17 additions & 3 deletions app/services/search_indexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,13 @@ def self.update_posts_index(post_id:, topic_title:, category_name:, topic_tags:,
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

Expand Down Expand Up @@ -165,6 +166,16 @@ def self.queue_category_posts_reindex(category_id)
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

Expand Down Expand Up @@ -222,7 +233,10 @@ def self.index(obj, force: false)
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)
Expand Down
4 changes: 4 additions & 0 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions db/migrate/20210420015635_add_searchable_to_user_fields.rb
Original file line number Diff line number Diff line change
@@ -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
20 changes: 19 additions & 1 deletion lib/search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,8 @@ def category_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)
Expand All @@ -849,7 +850,24 @@ def user_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
Expand Down
41 changes: 41 additions & 0 deletions spec/lib/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,45 @@
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
19 changes: 19 additions & 0 deletions spec/services/search_indexer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,23 @@ def scrub(html, strip_diacritics: false)
)
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

0 comments on commit e29605b

Please sign in to comment.