Allow secondary emails in user search

User searching only happened with an equality check for the primary
email. We already had a search_with_secondary_emails helper however
inefficient because it was using an IN condition.

Since there is a unique constraint on secondary emails as well,
that can be updated with a strict equality check and make the query
perform similarly to the one without secondary emails check.

That allows us to default to searching through secondary emails as well
(when using the API, project member list etc).

app/models/user.rb

@@ -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,7 +630,7 @@ 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))
 
@@ -624,7 +638,7 @@ class User < ApplicationRecord
         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
 

changelogs/unreleased/cat-user-search-secondary-emails.yml

+---
+title: Allow secondary emails in user search
+merge_request: 47587
+author:
+type: added

config/feature_flags/development/user_search_secondary_email.yml

+---
+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.6'
+type: development
+group: group::access
+default_enabled: false

spec/models/user_spec.rb

@@ -2027,6 +2027,9 @@ RSpec.describe User 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!(:email) do
+      create(:email, user: user, email: 'alias@example.com')
+    end
 
     describe 'name matching' do
       it 'returns users with a matching name with exact match first' do
@@ -2064,6 +2067,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..4])).not_to include(email.user)
+        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..4])).not_to include(email.user)
+        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,11 +2136,74 @@ RSpec.describe User do
     end
   end
 
+  describe '.search_without_secondary_emails' do
+    delegate :search_without_secondary_emails, to: :described_class
+
+    let!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) }
+    let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) }
+    let!(:email) do
+      create(:email, user: another_user, email: 'alias@example.com')
+    end
+
+    it 'returns users with a matching name' do
+      expect(search_without_secondary_emails(user.name)).to eq([user])
+    end
+
+    it 'returns users with a partially matching name' do
+      expect(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(search_without_secondary_emails(user.name.upcase)).to eq([user])
+    end
+
+    it 'returns users with a matching email' do
+      expect(search_without_secondary_emails(user.email)).to eq([user])
+    end
+
+    it 'does not return users with a partially matching email' do
+      expect(search_without_secondary_emails(user.email[0..2])).not_to include(user)
+    end
+
+    it 'returns users with a matching email regardless of the casing' do
+      expect(search_without_secondary_emails(user.email.upcase)).to eq([user])
+    end
+
+    it 'returns users with a matching username' do
+      expect(search_without_secondary_emails(user.username)).to eq([user])
+    end
+
+    it 'returns users with a partially matching username' do
+     expect(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(search_without_secondary_emails(user.username.upcase)).to eq([user])
+    end
+
+    it 'does not return users with a matching whole secondary email' do
+      expect(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(search_without_secondary_emails(email.email[1..4])).not_to include(email.user)
+    end
+
+    it 'returns no matches for an empty string' do
+      expect(search_without_secondary_emails('')).to be_empty
+    end
+
+    it 'returns no matches for nil' do
+      expect(search_without_secondary_emails(nil)).to be_empty
+    end
+  end
+
+
   describe '.search_with_secondary_emails' do
     delegate :search_with_secondary_emails, to: :described_class
 
-    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!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'someone.1@example.com' ) }
+    let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'another.2@example.com' ) }
     let!(:email) do
       create(:email, user: another_user, email: 'alias@example.com')
     end
@@ -2129,7 +2225,7 @@ RSpec.describe User do
     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(search_with_secondary_emails(user.email[0..2])).not_to include(user)
     end
 
     it 'returns users with a matching email regardless of the casing' do
@@ -2153,7 +2249,7 @@ RSpec.describe User do
     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(search_with_secondary_emails(email.email[1..4])).not_to include(email.user)
     end
 
     it 'returns no matches for an empty string' do