Extract scheduling a Jira import to a service

This change moves the Jira import scheduling to a service,
so that it can be easily reused in api endpoints

app/controllers/projects/import/jira_controller.rb

@@ -16,9 +16,8 @@ module Projects
       end
 
       def import
-        import_state = @project.import_state || @project.create_import_state
-
-        schedule_import(jira_import_params) unless import_state.in_progress?
+        response = ::JiraImport::StartImportService.new(current_user, @project, jira_import_params[:jira_project_key]).execute
+        flash[:notice] = response.message if response.message.present?
 
         redirect_to project_import_jira_path(@project)
       end
@@ -39,21 +38,6 @@ module Projects
         redirect_to project_issues_path(@project)
       end
 
-      def schedule_import(params)
-        import_data = @project.create_or_update_import_data(data: {}).becomes(JiraImportData)
-
-        jira_project_details = JiraImportData::JiraProjectDetails.new(
-          params[:jira_project_key],
-          Time.now.strftime('%Y-%m-%d %H:%M:%S'),
-          { user_id: current_user.id, name: current_user.name }
-        )
-        import_data << jira_project_details
-        import_data.force_import!
-
-        @project.import_type = 'jira'
-        @project.import_state.schedule if @project.save
-      end
-
       def jira_import_params
         params.permit(:jira_project_key)
       end

app/services/jira_import/start_import_service.rb

+# frozen_string_literal: true
+
+module JiraImport
+  class StartImportService
+    attr_reader :user, :project, :jira_project_key
+
+    def initialize(user, project, jira_project_key)
+      @user = user
+      @project = project
+      @jira_project_key = jira_project_key
+    end
+
+    def execute
+      validation_response = validate
+      return validation_response if validation_response&.error?
+
+      create_and_schedule_import
+    end
+
+    private
+
+    def create_and_schedule_import
+      import_data = project.create_or_update_import_data(data: {}).becomes(JiraImportData)
+      jira_project_details = JiraImportData::JiraProjectDetails.new(
+        jira_project_key,
+        Time.now.strftime('%Y-%m-%d %H:%M:%S'),
+        { user_id: user.id, name: user.name }
+      )
+      import_data << jira_project_details
+      import_data.force_import!
+
+      project.import_type = 'jira'
+      project.import_state.schedule if project.save!
+
+      ServiceResponse.success(payload: { import_data: import_data } )
+    rescue => ex
+      # in case project.save! raises an erorr
+      Gitlab::ErrorTracking.track_exception(ex, project_id: project.id)
+      build_error_response(ex.message)
+    end
+
+    def validate
+      return build_error_response(_('Jira import feature is disabled.')) unless Feature.enabled?(:jira_issue_import, project)
+      return build_error_response(_('You do not have permissions to run the import.')) unless user.can?(:admin_project, project)
+      return build_error_response(_('Jira integration not configured.')) unless project.jira_service&.active?
+      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?
+    end
+
+    def build_error_response(message)
+      import_data = JiraImportData.new(project: project)
+      import_data.errors.add(:base, message)
+      ServiceResponse.error(
+        message: import_data.errors.full_messages.to_sentence,
+        http_status: 400,
+        payload: { import_data: import_data }
+      )
+    end
+
+    def import_in_progress?
+      import_state = project.import_state || project.create_import_state
+      import_state.in_progress?
+    end
+  end
+end

locale/gitlab.pot

@@ -11305,6 +11305,9 @@ msgstr ""
 msgid "Jira import feature is disabled."
 msgstr ""
 
+msgid "Jira import is already running."
+msgstr ""
+
 msgid "Jira integration not configured."
 msgstr ""
 
@@ -23157,6 +23160,9 @@ msgstr ""
 msgid "You do not have permission to run the Web Terminal. Please contact a project administrator."
 msgstr ""
 
+msgid "You do not have permissions to run the import."
+msgstr ""
+
 msgid "You do not have the correct permissions to override the settings from the LDAP group sync."
 msgstr ""
 

spec/controllers/projects/import/jira_controller_spec.rb

@@ -118,10 +118,9 @@ describe Projects::Import::JiraController do
             end
 
             it 'uses the existing import data' do
-              expect(controller).not_to receive(:schedule_import)
-
               post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: 'New Project' }
 
+              expect(flash[:notice]).to eq('Jira import is already running.')
               expect(response).to redirect_to(project_import_jira_path(project))
             end
           end
@@ -153,8 +152,6 @@ describe Projects::Import::JiraController do
             end
 
             it 'uses the existing import data' do
-              expect(controller).to receive(:schedule_import).and_call_original
-
               post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: 'New Project' }
 
               project.reload

spec/services/jira_import/start_import_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe JiraImport::StartImportService do
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  subject { described_class.new(user, project, '').execute }
+
+  context 'when feature flag disabled' do
+    before do
+      stub_feature_flags(jira_issue_import: false)
+    end
+
+    it_behaves_like 'responds with error', 'Jira import feature is disabled.'
+  end
+
+  context 'when feature flag enabled' do
+    before do
+      stub_feature_flags(jira_issue_import: true)
+    end
+
+    context 'when user does not have permissions to run the import' do
+      before do
+        project.add_developer(user)
+      end
+
+      it_behaves_like 'responds with error', 'You do not have permissions to run the import.'
+    end
+
+    context 'when user has permission to run import' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context 'when Jira service was not setup' do
+        it_behaves_like 'responds with error', 'Jira integration not configured.'
+      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
+          it_behaves_like 'responds with error', 'Unable to find Jira project to import data from.'
+        end
+
+        context 'when correct data provided' do
+          subject { described_class.new(user, project, 'some-key').execute }
+
+          context 'when import is already running' do
+            let!(:import_state) { create(:import_state, project: project, status: :started) }
+
+            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
+
+          it 'schedules jira import' do
+            subject
+
+            expect(project.import_state.status).to eq('scheduled')
+          end
+
+          it 'creates jira import data' do
+            subject
+
+            jira_import_data = project.import_data.becomes(JiraImportData)
+            expect(jira_import_data.force_import?).to be true
+            imported_project_data = jira_import_data.projects.last
+            expect(imported_project_data.key).to eq('some-key')
+            expect(imported_project_data.scheduled_by['user_id']).to eq(user.id)
+          end
+        end
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+shared_examples 'responds with error' do |message|
+  it 'returns error' do
+    expect(subject).to be_a(ServiceResponse)
+    expect(subject).to be_error
+    expect(subject.message).to eq(message)
+  end
+end