Commit 5fc1123b authored by Sean McGivern's avatar Sean McGivern

Merge branch 'cat-user-search-secondary-emails' into 'master'

Allow secondary emails in user search

See merge request gitlab-org/gitlab!47587
parents c45a64fd 863096ba
......@@ -587,11 +587,13 @@ class User < ApplicationRecord
sanitized_order_sql = Arel.sql(sanitize_sql_array([order, query: query]))
where(
fuzzy_arel_match(:name, query, lower_exact_match: true)
.or(fuzzy_arel_match(:username, query, lower_exact_match: true))
.or(arel_table[:email].eq(query))
).reorder(sanitized_order_sql, :name)
search_query = if Feature.enabled?(:user_search_secondary_email)
search_with_secondary_emails(query)
else
search_without_secondary_emails(query)
end
search_query.reorder(sanitized_order_sql, :name)
end
# Limits the result set to users _not_ in the given query/list of IDs.
......@@ -606,6 +608,18 @@ class User < ApplicationRecord
reorder(:name)
end
def search_without_secondary_emails(query)
return none if query.blank?
query = query.downcase
where(
fuzzy_arel_match(:name, query, lower_exact_match: true)
.or(fuzzy_arel_match(:username, query, lower_exact_match: true))
.or(arel_table[:email].eq(query))
)
end
# searches user by given pattern
# it compares name, email, username fields and user's secondary emails with given pattern
# This method uses ILIKE on PostgreSQL.
......@@ -616,15 +630,16 @@ class User < ApplicationRecord
query = query.downcase
email_table = Email.arel_table
matched_by_emails_user_ids = email_table
matched_by_email_user_id = email_table
.project(email_table[:user_id])
.where(email_table[:email].eq(query))
.take(1) # at most 1 record as there is a unique constraint
where(
fuzzy_arel_match(:name, query)
.or(fuzzy_arel_match(:username, query))
.or(arel_table[:email].eq(query))
.or(arel_table[:id].in(matched_by_emails_user_ids))
.or(arel_table[:id].eq(matched_by_email_user_id))
)
end
......
---
title: Allow secondary emails in user search
merge_request: 47587
author:
type: added
---
name: user_search_secondary_email
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47587
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/282137
milestone: '13.7'
type: development
group: group::access
default_enabled: false
......@@ -2024,9 +2024,10 @@ RSpec.describe User do
end
describe '.search' do
let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') }
let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') }
let!(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@gmail.com') }
let_it_be(:user) { create(:user, name: 'user', username: 'usern', email: 'email@example.com') }
let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') }
let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') }
let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') }
describe 'name matching' do
it 'returns users with a matching name with exact match first' do
......@@ -2056,7 +2057,7 @@ RSpec.describe User do
end
it 'does not return users with a partially matching Email' do
expect(described_class.search(user.email[0..2])).not_to include(user, user2)
expect(described_class.search(user.email[1...-1])).to be_empty
end
it 'returns users with a matching Email regardless of the casing' do
......@@ -2064,6 +2065,36 @@ RSpec.describe User do
end
end
describe 'secondary email matching' do
context 'feature flag :user_search_secondary_email is enabled' do
it 'returns users with a matching secondary email' do
expect(described_class.search(email.email)).to include(email.user)
end
it 'does not return users with a matching part of secondary email' do
expect(described_class.search(email.email[1...-1])).to be_empty
end
it 'returns users with a matching secondary email regardless of the casing' do
expect(described_class.search(email.email.upcase)).to include(email.user)
end
end
context 'feature flag :user_search_secondary_email is disabled' do
before do
stub_feature_flags(user_search_secondary_email: false)
end
it 'does not return users with a matching secondary email' do
expect(described_class.search(email.email)).not_to include(email.user)
end
it 'does not return users with a matching part of secondary email' do
expect(described_class.search(email.email[1...-1])).to be_empty
end
end
end
describe 'username matching' do
it 'returns users with a matching username' do
expect(described_class.search(user.username)).to eq([user, user2])
......@@ -2103,65 +2134,119 @@ RSpec.describe User do
end
end
describe '.search_with_secondary_emails' do
delegate :search_with_secondary_emails, to: :described_class
describe '.search_without_secondary_emails' do
let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) }
let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) }
let_it_be(:email) { create(:email, user: another_user, email: 'alias@example.com') }
it 'returns users with a matching name' do
expect(described_class.search_without_secondary_emails(user.name)).to eq([user])
end
it 'returns users with a partially matching name' do
expect(described_class.search_without_secondary_emails(user.name[0..2])).to eq([user])
end
it 'returns users with a matching name regardless of the casing' do
expect(described_class.search_without_secondary_emails(user.name.upcase)).to eq([user])
end
it 'returns users with a matching email' do
expect(described_class.search_without_secondary_emails(user.email)).to eq([user])
end
it 'does not return users with a partially matching email' do
expect(described_class.search_without_secondary_emails(user.email[1...-1])).to be_empty
end
it 'returns users with a matching email regardless of the casing' do
expect(described_class.search_without_secondary_emails(user.email.upcase)).to eq([user])
end
it 'returns users with a matching username' do
expect(described_class.search_without_secondary_emails(user.username)).to eq([user])
end
it 'returns users with a partially matching username' do
expect(described_class.search_without_secondary_emails(user.username[0..2])).to eq([user])
end
it 'returns users with a matching username regardless of the casing' do
expect(described_class.search_without_secondary_emails(user.username.upcase)).to eq([user])
end
it 'does not return users with a matching whole secondary email' do
expect(described_class.search_without_secondary_emails(email.email)).not_to include(email.user)
end
it 'does not return users with a matching part of secondary email' do
expect(described_class.search_without_secondary_emails(email.email[1...-1])).to be_empty
end
let!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'john.doe@example.com' ) }
let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'albert.smith@example.com' ) }
let!(:email) do
create(:email, user: another_user, email: 'alias@example.com')
it 'returns no matches for an empty string' do
expect(described_class.search_without_secondary_emails('')).to be_empty
end
it 'returns no matches for nil' do
expect(described_class.search_without_secondary_emails(nil)).to be_empty
end
end
describe '.search_with_secondary_emails' do
let_it_be(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) }
let_it_be(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) }
let_it_be(:email) { create(:email, user: another_user, email: 'alias@example.com') }
it 'returns users with a matching name' do
expect(search_with_secondary_emails(user.name)).to eq([user])
expect(described_class.search_with_secondary_emails(user.name)).to eq([user])
end
it 'returns users with a partially matching name' do
expect(search_with_secondary_emails(user.name[0..2])).to eq([user])
expect(described_class.search_with_secondary_emails(user.name[0..2])).to eq([user])
end
it 'returns users with a matching name regardless of the casing' do
expect(search_with_secondary_emails(user.name.upcase)).to eq([user])
expect(described_class.search_with_secondary_emails(user.name.upcase)).to eq([user])
end
it 'returns users with a matching email' do
expect(search_with_secondary_emails(user.email)).to eq([user])
expect(described_class.search_with_secondary_emails(user.email)).to eq([user])
end
it 'does not return users with a partially matching email' do
expect(search_with_secondary_emails(user.email[0..2])).not_to include([user])
expect(described_class.search_with_secondary_emails(user.email[1...-1])).to be_empty
end
it 'returns users with a matching email regardless of the casing' do
expect(search_with_secondary_emails(user.email.upcase)).to eq([user])
expect(described_class.search_with_secondary_emails(user.email.upcase)).to eq([user])
end
it 'returns users with a matching username' do
expect(search_with_secondary_emails(user.username)).to eq([user])
expect(described_class.search_with_secondary_emails(user.username)).to eq([user])
end
it 'returns users with a partially matching username' do
expect(search_with_secondary_emails(user.username[0..2])).to eq([user])
expect(described_class.search_with_secondary_emails(user.username[0..2])).to eq([user])
end
it 'returns users with a matching username regardless of the casing' do
expect(search_with_secondary_emails(user.username.upcase)).to eq([user])
expect(described_class.search_with_secondary_emails(user.username.upcase)).to eq([user])
end
it 'returns users with a matching whole secondary email' do
expect(search_with_secondary_emails(email.email)).to eq([email.user])
expect(described_class.search_with_secondary_emails(email.email)).to eq([email.user])
end
it 'does not return users with a matching part of secondary email' do
expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user])
expect(described_class.search_with_secondary_emails(email.email[1...-1])).to be_empty
end
it 'returns no matches for an empty string' do
expect(search_with_secondary_emails('')).to be_empty
expect(described_class.search_with_secondary_emails('')).to be_empty
end
it 'returns no matches for nil' do
expect(search_with_secondary_emails(nil)).to be_empty
expect(described_class.search_with_secondary_emails(nil)).to be_empty
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment