Commit c8633f42 authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch 'revert-eed60157' into 'master'

Revert '19706-allow-user-search-less-than-3-chars-when-scoped'

See merge request gitlab-org/gitlab!79806
parents 5df57f2f fad143ba
......@@ -62,7 +62,7 @@ module Autocomplete
find_users
.active
.reorder_by_name
.optionally_search(search, use_minimum_char_limit: use_minimum_char_limit)
.optionally_search(search)
.where_not_in(skip_users)
.limit_to_todo_authors(
user: current_user,
......@@ -99,12 +99,6 @@ module Autocomplete
ActiveRecord::Associations::Preloader.new.preload(items, :status)
end
# rubocop: enable CodeReuse/ActiveRecord
def use_minimum_char_limit
return if project.blank? && group.blank? # We return nil so that we use the default defined in the User model
false
end
end
end
......
......@@ -204,7 +204,7 @@ class Member < ApplicationRecord
class << self
def search(query)
joins(:user).merge(User.search(query, use_minimum_char_limit: false))
joins(:user).merge(User.search(query))
end
def search_invite_email(query)
......
......@@ -667,8 +667,7 @@ class User < ApplicationRecord
sanitized_order_sql = Arel.sql(sanitize_sql_array([order, query: query]))
search_with_secondary_emails(query, use_minimum_char_limit: options[:use_minimum_char_limit])
.reorder(sanitized_order_sql, :name)
search_with_secondary_emails(query).reorder(sanitized_order_sql, :name)
end
# Limits the result set to users _not_ in the given query/list of IDs.
......@@ -683,10 +682,23 @@ 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.
def search_with_secondary_emails(query, use_minimum_char_limit: nil)
def search_with_secondary_emails(query)
return none if query.blank?
query = query.downcase
......@@ -697,11 +709,9 @@ class User < ApplicationRecord
.where(email_table[:email].eq(query))
.take(1) # at most 1 record as there is a unique constraint
use_minimum_char_limit = user_search_minimum_char_limit if use_minimum_char_limit.nil?
where(
fuzzy_arel_match(:name, query, use_minimum_char_limit: use_minimum_char_limit)
.or(fuzzy_arel_match(:username, query, use_minimum_char_limit: use_minimum_char_limit))
fuzzy_arel_match(:name, query, use_minimum_char_limit: user_search_minimum_char_limit)
.or(fuzzy_arel_match(:username, query, use_minimum_char_limit: user_search_minimum_char_limit))
.or(arel_table[:email].eq(query))
.or(arel_table[:id].eq(matched_by_email_user_id))
)
......
......@@ -32,7 +32,7 @@ class UsersStarProject < ApplicationRecord
end
def search(query)
joins(:user).merge(User.search(query, use_minimum_char_limit: false))
joins(:user).merge(User.search(query))
end
end
end
......@@ -23,7 +23,7 @@ module API
def retrieve_members(source, params:, deep: false)
members = deep ? find_all_members(source) : source_members(source).connected_to_user
members = members.includes(:user)
members = members.references(:user).merge(User.search(params[:query], use_minimum_char_limit: false)) if params[:query].present?
members = members.references(:user).merge(User.search(params[:query])) if params[:query].present?
members = members.where(user_id: params[:user_ids]) if params[:user_ids].present?
members
end
......
......@@ -7,16 +7,15 @@ RSpec.describe Autocomplete::UsersFinder do
# https://gitlab.com/gitlab-org/gitlab/-/issues/21432
describe '#execute' do
let_it_be(:user1) { create(:user, name: 'zzzzzname', username: 'johndoe') }
let_it_be(:user2) { create(:user, :blocked, username: 'notsorandom') }
let_it_be(:external_user) { create(:user, :external) }
let_it_be(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') }
let!(:user1) { create(:user, username: 'johndoe') }
let!(:user2) { create(:user, :blocked, username: 'notsorandom') }
let!(:external_user) { create(:user, :external) }
let!(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') }
let(:current_user) { create(:user) }
let(:params) { {} }
let_it_be(:project) { nil }
let_it_be(:group) { nil }
let(:project) { nil }
let(:group) { nil }
subject { described_class.new(params: params, current_user: current_user, project: project, group: group).execute.to_a }
......@@ -27,7 +26,7 @@ RSpec.describe Autocomplete::UsersFinder do
end
context 'when project passed' do
let_it_be(:project) { create(:project) }
let(:project) { create(:project) }
it { is_expected.to match_array([project.first_owner]) }
......@@ -44,36 +43,16 @@ RSpec.describe Autocomplete::UsersFinder do
it { is_expected.to match_array([project.first_owner]) }
end
end
context 'searching with less than 3 characters' do
let(:params) { { search: 'zz' } }
before do
project.add_guest(user1)
end
it 'allows partial matches' do
expect(subject).to contain_exactly(user1)
end
end
end
context 'when group passed and project not passed' do
let_it_be(:group) { create(:group, :public) }
let(:group) { create(:group, :public) }
before_all do
before do
group.add_users([user1], GroupMember::DEVELOPER)
end
it { is_expected.to match_array([user1]) }
context 'searching with less than 3 characters' do
let(:params) { { search: 'zz' } }
it 'allows partial matches' do
expect(subject).to contain_exactly(user1)
end
end
end
context 'when passed a subgroup' do
......@@ -97,14 +76,6 @@ RSpec.describe Autocomplete::UsersFinder do
let(:params) { { search: 'johndoe' } }
it { is_expected.to match_array([user1]) }
context 'searching with less than 3 characters' do
let(:params) { { search: 'zz' } }
it 'does not allow partial matches' do
expect(subject).to be_empty
end
end
end
context 'when filtered by skip_users' do
......
......@@ -2664,12 +2664,6 @@ RSpec.describe User do
it 'returns users with a exact matching username shorter than 3 chars regardless of the casing' do
expect(described_class.search(user3.username.upcase)).to eq([user3])
end
context 'when use_minimum_char_limit is false' do
it 'returns users with a partially matching username' do
expect(described_class.search('u', use_minimum_char_limit: false)).to eq([user3, user, user2])
end
end
end
it 'returns no matches for an empty string' do
......@@ -2681,6 +2675,64 @@ RSpec.describe User do
end
end
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
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' ) }
......
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