Merge branch '214810-use-users-mapping' into 'master'

Use stored user mapping for Jira importer

See merge request gitlab-org/gitlab!34487

lib/gitlab/jira_import/issue_serializer.rb

@@ -52,7 +52,9 @@ module Gitlab
       end
 
       def map_user_id(jira_user)
-        Gitlab::JiraImport::UserMapper.new(project, jira_user).execute&.id
+        return unless jira_user&.dig('accountId')
+
+        Gitlab::JiraImport.get_user_mapping(project.id, jira_user['accountId'])
       end
 
       def reporter

lib/gitlab/jira_import/user_mapper.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module JiraImport
-    class UserMapper
-      include ::Gitlab::Utils::StrongMemoize
-
-      def initialize(project, jira_user)
-        @project = project
-        @jira_user = jira_user
-      end
-
-      def execute
-        return unless jira_user
-
-        email = jira_user['emailAddress']
-
-        # We also include emails that are not yet confirmed
-        users = User.by_any_email(email).to_a
-
-        user = users.first
-
-        # this event should never happen but we should log it in case we have invalid data
-        log_user_mapping_message('Multiple users found for an email address', email) if users.count > 1
-
-        unless project.project_member(user) || project.group&.group_member(user)
-          log_user_mapping_message('Jira user not found', email)
-
-          return
-        end
-
-        user
-      end
-
-      private
-
-      attr_reader :project, :jira_user, :params
-
-      def log_user_mapping_message(message, email)
-        logger.info(
-          project_id: project.id,
-          project_path: project.full_path,
-          user_email: email,
-          message: message
-        )
-      end
-
-      def logger
-        @logger ||= Gitlab::Import::Logger.build
-      end
-    end
-  end
-end

spec/lib/gitlab/jira_import/issue_serializer_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
     let_it_be(:other_project_label) { create(:label, project: project, title: 'feature') }
     let_it_be(:group_label) { create(:group_label, group: group, title: 'dev') }
     let_it_be(:current_user) { create(:user) }
+    let_it_be(:user) { create(:user) }
 
     let(:iid) { 5 }
     let(:key) { 'PROJECT-5' }
@@ -17,8 +18,8 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
     let(:description) { 'basic description' }
     let(:created_at) { '2020-01-01 20:00:00' }
     let(:updated_at) { '2020-01-10 20:00:00' }
-    let(:assignee) { double(attrs: { 'displayName' => 'Solver', 'emailAddress' => 'assignee@example.com' }) }
-    let(:reporter) { double(attrs: { 'displayName' => 'Reporter', 'emailAddress' => 'reporter@example.com' }) }
+    let(:assignee) { nil }
+    let(:reporter) { nil }
     let(:jira_status) { 'new' }
 
     let(:parent_field) do
@@ -109,11 +110,12 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
       end
 
       context 'author' do
-        context 'when reporter maps to a valid GitLab user' do
-          let!(:user) { create(:user, email: 'reporter@example.com') }
+        let(:reporter) { double(attrs: { 'displayName' => 'Solver', 'accountId' => 'abcd' }) }
 
+        context 'when reporter maps to a valid GitLab user' do
           it 'sets the issue author to the mapped user' do
-            project.add_developer(user)
+            expect(Gitlab::JiraImport).to receive(:get_user_mapping).with(project.id, 'abcd')
+              .and_return(user.id)
 
             expect(subject[:author_id]).to eq(user.id)
           end
@@ -121,6 +123,9 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
 
         context 'when reporter does not map to a valid Gitlab user' do
           it 'defaults the issue author to project creator' do
+            expect(Gitlab::JiraImport).to receive(:get_user_mapping).with(project.id, 'abcd')
+              .and_return(nil)
+
             expect(subject[:author_id]).to eq(current_user.id)
           end
         end
@@ -129,25 +134,30 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
           let(:reporter) { nil }
 
           it 'defaults the issue author to project creator' do
+            expect(Gitlab::JiraImport).not_to receive(:get_user_mapping)
+
             expect(subject[:author_id]).to eq(current_user.id)
           end
         end
 
-        context 'when reporter field is missing email address' do
+        context 'when reporter field is missing accountId' do
           let(:reporter) { double(attrs: { 'displayName' => 'Reporter' }) }
 
           it 'defaults the issue author to project creator' do
+            expect(Gitlab::JiraImport).not_to receive(:get_user_mapping)
+
             expect(subject[:author_id]).to eq(current_user.id)
           end
         end
       end
 
       context 'assignee' do
-        context 'when assignee maps to a valid GitLab user' do
-          let!(:user) { create(:user, email: 'assignee@example.com') }
+        let(:assignee) { double(attrs: { 'displayName' => 'Solver', 'accountId' => '1234' }) }
 
+        context 'when assignee maps to a valid GitLab user' do
           it 'sets the issue assignees to the mapped user' do
-            project.add_developer(user)
+            expect(Gitlab::JiraImport).to receive(:get_user_mapping).with(project.id, '1234')
+              .and_return(user.id)
 
             expect(subject[:assignee_ids]).to eq([user.id])
           end
@@ -155,6 +165,9 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
 
         context 'when assignee does not map to a valid GitLab user' do
           it 'leaves the assignee empty' do
+            expect(Gitlab::JiraImport).to receive(:get_user_mapping).with(project.id, '1234')
+              .and_return(nil)
+
             expect(subject[:assignee_ids]).to be_nil
           end
         end
@@ -163,14 +176,18 @@ RSpec.describe Gitlab::JiraImport::IssueSerializer do
           let(:assignee) { nil }
 
           it 'leaves the assignee empty' do
+            expect(Gitlab::JiraImport).not_to receive(:get_user_mapping)
+
             expect(subject[:assignee_ids]).to be_nil
           end
         end
 
-        context 'when assginee field is missing email address' do
-          let(:assignee) { double(attrs: { 'displayName' => 'Reporter' }) }
+        context 'when assginee field is missing accountId' do
+          let(:assignee) { double(attrs: { 'displayName' => 'Solver' }) }
 
           it 'leaves the assignee empty' do
+            expect(Gitlab::JiraImport).not_to receive(:get_user_mapping)
+
             expect(subject[:assignee_ids]).to be_nil
           end
         end

spec/lib/gitlab/jira_import/user_mapper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::JiraImport::UserMapper do
-  let_it_be(:group)   { create(:group) }
-  let_it_be(:project) { create(:project, group: group) }
-  let_it_be(:user)    { create(:user, email: 'user@example.com') }
-  let_it_be(:email)   { create(:email, user: user, email: 'second_email@example.com', confirmed_at: nil) }
-
-  let(:jira_user) { { 'acountId' => '1a2b', 'emailAddress' => 'user@example.com' } }
-
-  describe '#execute' do
-    subject { described_class.new(project, jira_user).execute }
-
-    context 'when jira_user is nil' do
-      let(:jira_user) { nil }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'when Gitlab user is not found by email' do
-      let(:jira_user) { { 'acountId' => '1a2b', 'emailAddress' => 'other@example.com' } }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'when jira_user emailAddress is nil' do
-      let(:jira_user) { { 'acountId' => '1a2b', 'emailAddress' => nil } }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'when jira_user emailAddress key is missing' do
-      let(:jira_user) { { 'acountId' => '1a2b' } }
-
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'when found user is not a project member' do
-      it 'returns nil' do
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'when found user is a project member' do
-      it 'returns the found user' do
-        project.add_developer(user)
-
-        expect(subject).to eq(user)
-      end
-    end
-
-    context 'when user found by unconfirmd secondary address is a project member' do
-      let(:jira_user) { { 'acountId' => '1a2b', 'emailAddress' => 'second_email@example.com' } }
-
-      it 'returns the found user' do
-        project.add_developer(user)
-
-        expect(subject).to eq(user)
-      end
-    end
-
-    context 'when user is a group member' do
-      it 'returns the found user' do
-        group.add_developer(user)
-
-        expect(subject).to eq(user)
-      end
-    end
-  end
-end