Match Jira users by email and username/name

- add scope for matching by name OR username to User

app/models/user.rb

@@ -509,6 +509,38 @@ class User < ApplicationRecord
       by_any_email(email, confirmed: confirmed).take
     end
 
+    # Returns a relation containing all users with emails for given emails
+    # where non-confirmed emails are included and private commit emails are skiped
+    # or names where username and name is searched for exact match
+    #
+    #
+    # @param emails [String, Array<String>] email addresses to check
+    # @param names [String, Array<String>] names to check in username and name
+    def by_emails_or_names(jira_pairs)
+      sql = <<~SQL
+        WITH jira_users(name, email) AS (VALUES #{jira_pairs})
+          SELECT DISTINCT users.id AS user_id,
+            (CASE
+             WHEN ((jira_users.email = users.email) OR (jira_users.email = emails.email))
+             THEN 3
+             WHEN (jira_users.name = lower(users.username))
+             THEN 2
+             WHEN (jira_users.name = lower(users.name))
+             THEN 1
+            END)
+            AS match_score, jira_users.name as jira_name, jira_users.email as jira_email
+          FROM users
+          LEFT JOIN emails ON (users.id = emails.user_id)
+          JOIN jira_users ON (jira_users.name = lower(users.username))
+            OR (jira_users.name = lower(users.name))
+            OR (jira_users.email = users.email)
+            OR (jira_users.email = emails.email)
+          ORDER BY match_score DESC;
+      SQL
+
+      ActiveRecord::Base.connection.execute(sql)
+    end
+
     # Returns a relation containing all the users for the given email addresses
     #
     # @param emails [String, Array<String>] email addresses to check

app/services/jira_import/users_importer.rb

@@ -2,8 +2,6 @@
 
 module JiraImport
   class UsersImporter
-    attr_reader :user, :project, :start_at
-
     def initialize(user, project, start_at)
       @project = project
       @start_at = start_at
@@ -23,6 +21,8 @@ module JiraImport
 
     private
 
+    attr_reader :user, :project, :start_at
+
     def mapped_users
       users_mapper_service.execute
     end
@@ -44,9 +44,9 @@ module JiraImport
       # TODO: use deployment_type enum from jira service when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
       case deployment_type.upcase
       when JiraService::DEPLOYMENT_TYPES[:server]
-        ServerUsersMapperService.new(project.jira_service, start_at)
+        ServerUsersMapperService.new(user, project, start_at)
       when JiraService::DEPLOYMENT_TYPES[:cloud]
-        CloudUsersMapperService.new(project.jira_service, start_at)
+        CloudUsersMapperService.new(user, project, start_at)
       else
         raise ArgumentError
       end

app/services/jira_import/users_mapper_service.rb

@@ -2,30 +2,37 @@
 
 module JiraImport
   class UsersMapperService
+    include Gitlab::Utils::StrongMemoize
+
     # MAX_USERS must match the pageSize value in app/assets/javascripts/jira_import/utils/constants.js
     MAX_USERS = 50
 
-    attr_reader :jira_service, :start_at
-
-    def initialize(jira_service, start_at)
-      @jira_service = jira_service
+    # The class is called from UsersImporter and small batches of users are expected
+    # In case the mapping of a big batch of users is expected to be passed here
+    # the implementation needs to change here and handles the matching in batches
+    def initialize(current_user, project, start_at)
+      @current_user = current_user
+      @project = project
+      @jira_service = project.jira_service
       @start_at = start_at
     end
 
     def execute
-      users.to_a.map do |jira_user|
+      jira_users.to_a.map do |jira_user|
         {
           jira_account_id: jira_user_id(jira_user),
           jira_display_name: jira_user_name(jira_user),
           jira_email: jira_user['emailAddress']
-        }.merge(match_user(jira_user))
+        }.merge(gitlab_id: find_gitlab_id(jira_user))
       end
     end
 
     private
 
-    def users
-      @users ||= client.get(url)
+    attr_reader :current_user, :project, :jira_service, :start_at
+
+    def jira_users
+      @jira_users ||= client.get(url)
     end
 
     def client
@@ -44,10 +51,35 @@ module JiraImport
       raise NotImplementedError
     end
 
-    # TODO: Matching user by email and displayName will be done as the part
-    # of follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/219023
-    def match_user(jira_user)
-      { gitlab_id: nil, gitlab_username: nil, gitlab_name: nil }
+    def matched_users
+      strong_memoize(:matched_users) do
+        pairs_to_match = jira_users.map do |user|
+          "('#{jira_user_name(user)&.downcase}', '#{user['emailAddress']&.downcase}')"
+        end.join(',')
+
+        User.by_emails_or_names(pairs_to_match)
+      end
+    end
+
+    def find_gitlab_id(jira_user)
+      user = matched_users.find do |matched_user|
+        matched_user['jira_email'] == jira_user['emailAddress']&.downcase ||
+          matched_user['jira_name'].downcase == jira_user_name(jira_user)&.downcase
+      end
+
+      return unless user
+
+      user_id = user['user_id']
+
+      return unless project_member_ids.include?(user_id)
+
+      user_id
+    end
+
+    def project_member_ids
+      # rubocop: disable CodeReuse/ActiveRecord
+      @project_member_ids ||= MembersFinder.new(project, current_user).execute.pluck(:user_id)
+      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end

spec/services/jira_import/cloud_users_mapper_service_spec.rb

@@ -5,15 +5,44 @@ require 'spec_helper'
 RSpec.describe JiraImport::CloudUsersMapperService do
   let(:start_at) { 7 }
   let(:url) { "/rest/api/2/users?maxResults=50&startAt=#{start_at}" }
+
+  let_it_be(:user_1) { create(:user, username: 'randomuser', name: 'USER-name1', email: 'uji@example.com') }
+  let_it_be(:user_2) { create(:user, username: 'username2') }
+  let_it_be(:user_5) { create(:user, username: 'username5') }
+  let_it_be(:user_4) { create(:user, email: 'user4@example.com') }
+  let_it_be(:user_6) { create(:user, email: 'user6@example.com') }
+  let_it_be(:user_7) { create(:user, username: 'username7') }
+  let_it_be(:user_8) do
+    create(:user).tap { |user| create(:email, user: user, email: 'user8_email@example.com') }
+  end
+
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:group)        { create(:group) }
+  let_it_be(:project)      { create(:project, group: group) }
+
   let(:jira_users) do
     [
-      { 'accountId' => 'abcd', 'displayName' => 'user1' },
-      { 'accountId' => 'efg' },
-      { 'accountId' => 'hij', 'displayName' => 'user3', 'emailAddress' => 'user3@example.com' }
+      { 'accountId' => 'abcd', 'displayName' => 'User-Name1' }, # matched by name
+      { 'accountId' => 'efg', 'displayName' => 'username2' }, # matcher by username
+      { 'accountId' => 'hij' }, # no match
+      { 'accountId' => '123', 'displayName' => 'user4', 'emailAddress' => 'user4@example.com' }, # matched by email
+      { 'accountId' => '456', 'displayName' => 'username5foo', 'emailAddress' => 'user5@example.com' }, # no match
+      { 'accountId' => '789', 'displayName' => 'user6', 'emailAddress' => 'user6@example.com' }, # matched by email, no project member
+      { 'accountId' => 'xyz', 'displayName' => 'username7', 'emailAddress' => 'user7@example.com' }, # matched by username, no project member
+      { 'accountId' => 'vhk', 'displayName' => 'user8', 'emailAddress' => 'user8_email@example.com' }, # matched by secondary email
+      { 'accountId' => 'uji', 'displayName' => 'user9', 'emailAddress' => 'uji@example.com' } # matched by email, same as user_1
     ]
   end
 
   describe '#execute' do
+    before do
+      project.add_developer(current_user)
+      project.add_developer(user_1)
+      project.add_developer(user_2)
+      group.add_developer(user_4)
+      group.add_guest(user_8)
+    end
+
     it_behaves_like 'mapping jira users'
   end
 end

spec/services/jira_import/server_users_mapper_service_spec.rb

@@ -5,15 +5,44 @@ require 'spec_helper'
 RSpec.describe JiraImport::ServerUsersMapperService do
   let(:start_at) { 7 }
   let(:url) { "/rest/api/2/user/search?username=''&maxResults=50&startAt=#{start_at}" }
+
+  let_it_be(:user_1) { create(:user, username: 'randomuser', name: 'USER-name1', email: 'uji@example.com') }
+  let_it_be(:user_2) { create(:user, username: 'username2') }
+  let_it_be(:user_5) { create(:user, username: 'username5') }
+  let_it_be(:user_4) { create(:user, email: 'user4@example.com') }
+  let_it_be(:user_6) { create(:user, email: 'user6@example.com') }
+  let_it_be(:user_7) { create(:user, username: 'username7') }
+  let_it_be(:user_8) do
+    create(:user).tap { |user| create(:email, user: user, email: 'user8_email@example.com') }
+  end
+
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:group)        { create(:group) }
+  let_it_be(:project)      { create(:project, group: group) }
+
   let(:jira_users) do
     [
-      { 'key' => 'abcd', 'name' => 'user1' },
-      { 'key' => 'efg' },
-      { 'key' => 'hij', 'name' => 'user3', 'emailAddress' => 'user3@example.com' }
+      { 'key' => 'abcd', 'name' => 'User-Name1' }, # matched by name
+      { 'key' => 'efg', 'name' => 'username2' }, # matcher by username
+      { 'key' => 'hij' }, # no match
+      { 'key' => '123', 'name' => 'user4', 'emailAddress' => 'user4@example.com' }, # matched by email
+      { 'key' => '456', 'name' => 'username5foo', 'emailAddress' => 'user5@example.com' }, # no match
+      { 'key' => '789', 'name' => 'user6', 'emailAddress' => 'user6@example.com' }, # matched by email, no project member
+      { 'key' => 'xyz', 'name' => 'username7', 'emailAddress' => 'user7@example.com' }, # matched by username, no project member
+      { 'key' => 'vhk', 'name' => 'user8', 'emailAddress' => 'user8_email@example.com' }, # matched by secondary email
+      { 'key' => 'uji', 'name' => 'user9', 'emailAddress' => 'uji@example.com' } # matched by email, same as user_1
     ]
   end
 
   describe '#execute' do
+    before do
+      project.add_developer(current_user)
+      project.add_developer(user_1)
+      project.add_developer(user_2)
+      group.add_developer(user_4)
+      group.add_guest(user_8)
+    end
+
     it_behaves_like 'mapping jira users'
   end
 end

spec/services/jira_import/users_importer_spec.rb

@@ -6,7 +6,8 @@ RSpec.describe JiraImport::UsersImporter do
   include JiraServiceHelper
 
   let_it_be(:user) { create(:user) }
-  let_it_be(:project, reload: true) { create(:project) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project, reload: true) { create(:project, group: group) }
   let_it_be(:start_at) { 7 }
 
   let(:importer) { described_class.new(user, project, start_at) }
@@ -18,19 +19,15 @@ RSpec.describe JiraImport::UsersImporter do
       [
         {
           jira_account_id: 'acc1',
-          jira_display_name: 'user1',
+          jira_display_name: 'user-name1',
           jira_email: 'sample@jira.com',
-          gitlab_id: nil,
-          gitlab_username: nil,
-          gitlab_name: nil
+          gitlab_id: project_member.id
         },
         {
           jira_account_id: 'acc2',
-          jira_display_name: 'user2',
+          jira_display_name: 'user-name2',
           jira_email: nil,
-          gitlab_id: nil,
-          gitlab_username: nil,
-          gitlab_name: nil
+          gitlab_id: group_member.id
         }
       ]
     end
@@ -69,13 +66,22 @@ RSpec.describe JiraImport::UsersImporter do
           context 'when jira client returns an empty array' do
             let(:jira_users) { [] }
 
-            it 'retturns nil payload' do
+            it 'returns nil payload' do
               expect(subject.success?).to be_truthy
               expect(subject.payload).to be_empty
             end
           end
 
           context 'when jira client returns an results' do
+            let_it_be(:project_member) { create(:user, email: 'sample@jira.com') }
+            let_it_be(:group_member) { create(:user, name: 'user-name2') }
+            let_it_be(:other_user) { create(:user) }
+
+            before do
+              project.add_developer(project_member)
+              group.add_developer(group_member)
+            end
+
             it 'returns the mapped users' do
               expect(subject.success?).to be_truthy
               expect(subject.payload).to eq(mapped_users)
@@ -90,8 +96,8 @@ RSpec.describe JiraImport::UsersImporter do
       let(:url) { "/rest/api/2/user/search?username=''&maxResults=50&startAt=#{start_at}" }
       let(:jira_users) do
         [
-          { 'key' => 'acc1', 'name' => 'user1', 'emailAddress' => 'sample@jira.com' },
-          { 'key' => 'acc2', 'name' => 'user2' }
+          { 'key' => 'acc1', 'name' => 'user-name1', 'emailAddress' => 'sample@jira.com' },
+          { 'key' => 'acc2', 'name' => 'user-name2' }
         ]
       end
 
@@ -110,8 +116,8 @@ RSpec.describe JiraImport::UsersImporter do
       let(:url) { "/rest/api/2/users?maxResults=50&startAt=#{start_at}" }
       let(:jira_users) do
         [
-          { 'accountId' => 'acc1', 'displayName' => 'user1', 'emailAddress' => 'sample@jira.com' },
-          { 'accountId' => 'acc2', 'displayName' => 'user2' }
+          { 'accountId' => 'acc1', 'displayName' => 'user-name1', 'emailAddress' => 'sample@jira.com' },
+          { 'accountId' => 'acc2', 'displayName' => 'user-name2' }
         ]
       end
 

spec/support/shared_examples/services/jira_import/user_mapper_services_shared_examples.rb

@@ -3,7 +3,6 @@
 RSpec.shared_examples 'mapping jira users' do
   let(:client) { double }
 
-  let_it_be(:project) { create(:project) }
   let_it_be(:jira_service) { create(:jira_service, project: project, active: true) }
 
   before do
@@ -11,7 +10,7 @@ RSpec.shared_examples 'mapping jira users' do
     allow(client).to receive(:get).with(url).and_return(jira_users)
   end
 
-  subject { described_class.new(jira_service, start_at) }
+  subject { described_class.new(current_user, project, start_at) }
 
   context 'jira_users is nil' do
     let(:jira_users) { nil }
@@ -22,18 +21,27 @@ RSpec.shared_examples 'mapping jira users' do
   end
 
   context 'when jira_users is present' do
-    # TODO: now we only create an array in a proper format
-    # mapping is tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/219023
     let(:mapped_users) do
       [
-        { jira_account_id: 'abcd', jira_display_name: 'user1', jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
-        { jira_account_id: 'efg', jira_display_name: nil, jira_email: nil, gitlab_id: nil, gitlab_username: nil, gitlab_name: nil },
-        { jira_account_id: 'hij', jira_display_name: 'user3', jira_email: 'user3@example.com', gitlab_id: nil, gitlab_username: nil, gitlab_name: nil }
+        { jira_account_id: 'abcd', jira_display_name: 'User-Name1', jira_email: nil, gitlab_id: user_1.id },
+        { jira_account_id: 'efg', jira_display_name: 'username2', jira_email: nil, gitlab_id: user_2.id },
+        { jira_account_id: 'hij', jira_display_name: nil, jira_email: nil, gitlab_id: nil },
+        { jira_account_id: '123', jira_display_name: 'user4', jira_email: 'user4@example.com', gitlab_id: user_4.id },
+        { jira_account_id: '456', jira_display_name: 'username5foo', jira_email: 'user5@example.com', gitlab_id: nil },
+        { jira_account_id: '789', jira_display_name: 'user6', jira_email: 'user6@example.com', gitlab_id: nil },
+        { jira_account_id: 'xyz', jira_display_name: 'username7', jira_email: 'user7@example.com', gitlab_id: nil },
+        { jira_account_id: 'vhk', jira_display_name: 'user8', jira_email: 'user8_email@example.com', gitlab_id: user_8.id },
+        { jira_account_id: 'uji', jira_display_name: 'user9', jira_email: 'uji@example.com', gitlab_id: user_1.id }
       ]
     end
 
     it 'returns users mapped to Gitlab' do
       expect(subject.execute).to eq(mapped_users)
     end
+
+    # 1 query for getting matched users, 3 queries for MembersFinder
+    it 'runs only 4 queries' do
+      expect { subject }.not_to exceed_query_limit(4)
+    end
   end
 end