Merge branch 'refactor-jira-workers' into 'master'

Refactor validation of jira imports

See merge request gitlab-org/gitlab!28704

app/models/project.rb

@@ -866,6 +866,16 @@ class Project < ApplicationRecord
     latest_jira_import&.status || 'initial'
   end
 
+  def validate_jira_import_settings!(user: nil)
+    raise Projects::ImportService::Error, _('Jira import feature is disabled.') unless jira_issues_import_feature_flag_enabled?
+    raise Projects::ImportService::Error, _('Jira integration not configured.') unless jira_service&.active?
+
+    return unless user
+
+    raise Projects::ImportService::Error, _('Cannot import because issues are not available in this project.') unless feature_available?(:issues, user)
+    raise Projects::ImportService::Error, _('You do not have permissions to run the import.') unless user.can?(:admin_project, self)
+  end
+
   def human_import_status_name
     import_state&.human_status_name || 'none'
   end

app/services/jira_import/start_import_service.rb

@@ -62,12 +62,12 @@ module JiraImport
     end
 
     def validate
-      return build_error_response(_('Jira import feature is disabled.')) unless project.jira_issues_import_feature_flag_enabled?
-      return build_error_response(_('You do not have permissions to run the import.')) unless user.can?(:admin_project, project)
-      return build_error_response(_('Cannot import because issues are not available in this project.')) unless project.feature_available?(:issues, user)
-      return build_error_response(_('Jira integration not configured.')) unless project.jira_service&.active?
+      project.validate_jira_import_settings!(user: user)
+
       return build_error_response(_('Unable to find Jira project to import data from.')) if jira_project_key.blank?
       return build_error_response(_('Jira import is already running.')) if import_in_progress?
+    rescue Projects::ImportService::Error => e
+      build_error_response(e.message)
     end
 
     def build_error_response(message)

lib/gitlab/jira_import/base_importer.rb

@@ -6,10 +6,10 @@ module Gitlab
       attr_reader :project, :client, :formatter, :jira_project_key
 
       def initialize(project)
-        raise Projects::ImportService::Error, _('Jira import feature is disabled.') unless project.jira_issues_import_feature_flag_enabled?
-        raise Projects::ImportService::Error, _('Jira integration not configured.') unless project.jira_service&.active?
+        project.validate_jira_import_settings!
 
         @jira_project_key = project.latest_jira_import&.jira_project_key
+
         raise Projects::ImportService::Error, _('Unable to find Jira project to import data from.') unless @jira_project_key
 
         @project = project

spec/requests/api/graphql/mutations/jira_import/start_spec.rb

@@ -99,12 +99,6 @@ describe 'Starting a Jira Import' do
           it_behaves_like 'a mutation that returns errors in the response', errors: ['Jira integration not configured.']
         end
 
-        context 'when issues feature are disabled' do
-          let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
-
-          it_behaves_like 'a mutation that returns errors in the response', errors: ['Cannot import because issues are not available in this project.']
-        end
-
         context 'when when project has Jira service' do
           let!(:service) { create(:jira_service, project: project) }
 
@@ -112,6 +106,12 @@ describe 'Starting a Jira Import' do
             project.reload
           end
 
+          context 'when issues feature are disabled' do
+            let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+
+            it_behaves_like 'a mutation that returns errors in the response', errors: ['Cannot import because issues are not available in this project.']
+          end
+
           context 'when jira_project_key not provided' do
             let(:jira_project_key) { '' }
 

spec/services/jira_import/start_import_service_spec.rb

@@ -5,8 +5,9 @@ require 'spec_helper'
 describe JiraImport::StartImportService do
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project) }
+  let(:key) { 'KEY' }
 
-  subject { described_class.new(user, project, '').execute }
+  subject { described_class.new(user, project, key).execute }
 
   context 'when feature flag disabled' do
     before do
@@ -23,6 +24,8 @@ describe JiraImport::StartImportService do
 
     context 'when user does not have permissions to run the import' do
       before do
+        create(:jira_service, project: project, active: true)
+
         project.add_developer(user)
       end
 
@@ -38,19 +41,21 @@ describe JiraImport::StartImportService do
         it_behaves_like 'responds with error', 'Jira integration not configured.'
       end
 
-      context 'when issues feature are disabled' do
-        let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
-
-        it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.'
-      end
-
       context 'when Jira service exists' do
         let!(:jira_service) { create(:jira_service, project: project, active: true) }
 
         context 'when Jira project key is not provided' do
+          let(:key) { '' }
+
           it_behaves_like 'responds with error', 'Unable to find Jira project to import data from.'
         end
 
+        context 'when issues feature are disabled' do
+          let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+
+          it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.'
+        end
+
         context 'when correct data provided' do
           let(:fake_key)  { 'some-key' }
 
@@ -62,15 +67,17 @@ describe JiraImport::StartImportService do
             it_behaves_like 'responds with error', 'Jira import is already running.'
           end
 
-          it 'returns success response' do
-            expect(subject).to be_a(ServiceResponse)
-            expect(subject).to be_success
-          end
+          context 'when everything is ok' do
+            it 'returns success response' do
+              expect(subject).to be_a(ServiceResponse)
+              expect(subject).to be_success
+            end
 
-          it 'schedules jira import' do
-            subject
+            it 'schedules jira import' do
+              subject
 
-            expect(project.latest_jira_import).to be_scheduled
+              expect(project.latest_jira_import).to be_scheduled
+            end
           end
 
           it 'creates jira import data' do