Test Jira connection before running import

- also reorganise specs a bit

app/models/project.rb

@@ -870,10 +870,12 @@ class Project < ApplicationRecord
     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
+    if 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
 
-    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)
+    raise Projects::ImportService::Error, _('Unable to connect to the Jira instance. Please check your Jira integration configuration.') unless jira_service.test(nil)[:success]
   end
 
   def human_import_status_name

changelogs/unreleased/211984-jira-import-connection-test.yml

+---
+title: Test Jira connection before running import
+merge_request: 29926
+author:
+type: changed

locale/gitlab.pot

@@ -22221,6 +22221,9 @@ msgstr ""
 msgid "Unable to connect to server: %{error}"
 msgstr ""
 
+msgid "Unable to connect to the Jira instance. Please check your Jira integration configuration."
+msgstr ""
+
 msgid "Unable to convert Kubernetes logs encoding to UTF-8"
 msgstr ""
 

spec/controllers/projects/import/jira_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Projects::Import::JiraController do
+  include JiraServiceHelper
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
   let_it_be(:jira_project_key) { 'Test' }
@@ -61,6 +63,7 @@ describe Projects::Import::JiraController do
       before do
         stub_feature_flags(jira_issue_import: true)
         stub_feature_flags(jira_issue_import_vue: false)
+        stub_jira_service_test
       end
 
       context 'when Jira service is enabled for the project' do

spec/lib/gitlab/jira_import/base_importer_spec.rb

@@ -3,12 +3,17 @@
 require 'spec_helper'
 
 describe Gitlab::JiraImport::BaseImporter do
+  include JiraServiceHelper
+
   let(:project) { create(:project) }
 
   describe 'with any inheriting class' do
-    context 'when feature flag disabled' do
+    context 'when an error is returned from the project validation' do
       before do
         stub_feature_flags(jira_issue_import: false)
+
+        allow(project).to receive(:validate_jira_import_settings!)
+          .and_raise(Projects::ImportService::Error, 'Jira import feature is disabled.')
       end
 
       it 'raises exception' do
@@ -16,20 +21,17 @@ describe Gitlab::JiraImport::BaseImporter do
       end
     end
 
-    context 'when feature flag enabled' do
+    context 'when project validation is ok' do
+      let!(:jira_service) { create(:jira_service, project: project) }
+
       before do
         stub_feature_flags(jira_issue_import: true)
-      end
+        stub_jira_service_test
 
-      context 'when Jira service was not setup' do
-        it 'raises exception' do
-          expect { described_class.new(project) }.to raise_error(Projects::ImportService::Error, 'Jira integration not configured.')
-        end
+        allow(project).to receive(:validate_jira_import_settings!)
       end
 
       context 'when Jira service exists' do
-        let!(:jira_service) { create(:jira_service, project: project) }
-
         context 'when Jira import data is not present' do
           it 'raises exception' do
             expect { described_class.new(project) }.to raise_error(Projects::ImportService::Error, 'Unable to find Jira project to import data from.')

spec/lib/gitlab/jira_import/issues_importer_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::JiraImport::IssuesImporter do
+  include JiraServiceHelper
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
   let_it_be(:jira_import) { create(:jira_import_state, project: project) }
@@ -12,6 +14,7 @@ describe Gitlab::JiraImport::IssuesImporter do
 
   before do
     stub_feature_flags(jira_issue_import: true)
+    stub_jira_service_test
   end
 
   describe '#imported_items_cache_key' do

spec/lib/gitlab/jira_import/labels_importer_spec.rb

@@ -3,9 +3,11 @@
 require 'spec_helper'
 
 describe Gitlab::JiraImport::LabelsImporter do
-  let_it_be(:user)         { create(:user) }
-  let_it_be(:group)        { create(:group) }
-  let_it_be(:project)      { create(:project, group: group) }
+  include JiraServiceHelper
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
   let_it_be(:jira_service) { create(:jira_service, project: project) }
 
   subject { described_class.new(project).execute }
@@ -13,13 +15,14 @@ describe Gitlab::JiraImport::LabelsImporter do
   before do
     stub_feature_flags(jira_issue_import: true)
     stub_const('Gitlab::JiraImport::LabelsImporter::MAX_LABELS', 2)
-
-    WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo')
-      .to_return(body: { url: 'http://url' }.to_json )
   end
 
   describe '#execute', :clean_gitlab_redis_cache do
-    context 'when jira import label is missing from jira import' do
+    before do
+      stub_jira_service_test
+    end
+
+    context 'when label is missing from jira import' do
       let_it_be(:no_label_jira_import) { create(:jira_import_state, label: nil, project: project) }
 
       it 'raises error' do

spec/models/project_spec.rb

@@ -5944,6 +5944,119 @@ describe Project do
     end
   end
 
+  describe '#validate_jira_import_settings!' do
+    include JiraServiceHelper
+
+    let_it_be(:project, reload: true) { create(:project) }
+
+    shared_examples 'raise Jira import error' do |message|
+      it 'returns error' do
+        expect { subject }.to raise_error(Projects::ImportService::Error, message)
+      end
+    end
+
+    shared_examples 'jira configuration base checks' do
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(jira_issue_import: false)
+        end
+
+        it_behaves_like 'raise Jira import error', 'Jira import feature is disabled.'
+      end
+
+      context 'when feature flag is enabled' do
+        before do
+          stub_feature_flags(jira_issue_import: true)
+        end
+
+        context 'when Jira service was not setup' do
+          it_behaves_like 'raise Jira import 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 connection is not valid' do
+            before do
+              WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo')
+                .to_raise(JIRA::HTTPError.new(double(message: 'Some failure.')))
+            end
+
+            it_behaves_like 'raise Jira import error', 'Unable to connect to the Jira instance. Please check your Jira integration configuration.'
+          end
+        end
+      end
+    end
+
+    before do
+      stub_jira_service_test
+    end
+
+    context 'without user param' do
+      subject { project.validate_jira_import_settings! }
+
+      it_behaves_like 'jira configuration base checks'
+
+      context 'when jira connection is valid' do
+        let!(:jira_service) { create(:jira_service, project: project, active: true) }
+
+        it 'does not return any error' do
+          expect { subject }.not_to raise_error
+        end
+      end
+    end
+
+    context 'with user param provided' do
+      let_it_be(:user) { create(:user) }
+
+      subject { project.validate_jira_import_settings!(user: user) }
+
+      context 'when user has permission to run import' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        it_behaves_like 'jira configuration base checks'
+      end
+
+      context 'when feature flag is 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
+            create(:jira_service, project: project, active: true)
+
+            project.add_developer(user)
+          end
+
+          it_behaves_like 'raise Jira import 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
+
+          let!(:jira_service) { create(:jira_service, project: project, active: true) }
+
+          context 'when issues feature is disabled' do
+            let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+
+            it_behaves_like 'raise Jira import error', 'Cannot import because issues are not available in this project.'
+          end
+
+          context 'when everything is ok' do
+            it 'does not return any error' do
+              expect { subject }.not_to raise_error
+            end
+          end
+        end
+      end
+    end
+  end
+
   def finish_job(export_job)
     export_job.start
     export_job.finish

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

@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 describe 'Starting a Jira Import' do
+  include JiraServiceHelper
   include GraphqlHelpers
 
   let_it_be(:user) { create(:user) }
@@ -104,6 +105,8 @@ describe 'Starting a Jira Import' do
 
           before do
             project.reload
+
+            stub_jira_service_test
           end
 
           context 'when issues feature are disabled' do

spec/services/jira_import/start_import_service_spec.rb

@@ -3,113 +3,89 @@
 require 'spec_helper'
 
 describe JiraImport::StartImportService do
+  include JiraServiceHelper
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project, reload: true) { create(:project) }
   let(:key) { 'KEY' }
 
   subject { described_class.new(user, project, key).execute }
 
-  context 'when feature flag disabled' do
+  context 'when an error is returned from the project validation' do
     before do
-      stub_feature_flags(jira_issue_import: false)
+      allow(project).to receive(:validate_jira_import_settings!)
+        .and_raise(Projects::ImportService::Error, 'Jira import feature is disabled.')
     end
 
     it_behaves_like 'responds with error', 'Jira import feature is disabled.'
   end
 
-  context 'when feature flag enabled' do
+  context 'when project validation is ok' do
+    let!(:jira_service) { create(:jira_service, project: project, active: true) }
+
     before do
-      stub_feature_flags(jira_issue_import: true)
+      stub_jira_service_test
+      allow(project).to receive(:validate_jira_import_settings!)
     end
 
-    context 'when user does not have permissions to run the import' do
-      before do
-        create(:jira_service, project: project, active: true)
+    context 'when Jira project key is not provided' do
+      let(:key) { '' }
 
-        project.add_developer(user)
-      end
-
-      it_behaves_like 'responds with error', 'You do not have permissions to run the import.'
+      it_behaves_like 'responds with error', 'Unable to find Jira project to import data from.'
     end
 
-    context 'when user has permission to run import' do
-      before do
-        project.add_maintainer(user)
-      end
+    context 'when correct data provided' do
+      let(:fake_key)  { 'some-key' }
 
-      context 'when Jira service was not setup' do
-        it_behaves_like 'responds with error', 'Jira integration not configured.'
-      end
+      subject { described_class.new(user, project, fake_key).execute }
 
-      context 'when Jira service exists' do
-        let!(:jira_service) { create(:jira_service, project: project, active: true) }
+      context 'when import is already running' do
+        let_it_be(:jira_import_state) { create(:jira_import_state, :started, project: project) }
 
-        context 'when Jira project key is not provided' do
-          let(:key) { '' }
+        it_behaves_like 'responds with error', 'Jira import is already running.'
+      end
 
-          it_behaves_like 'responds with error', 'Unable to find Jira project to import data from.'
+      context 'when everything is ok' do
+        it 'returns success response' do
+          expect(subject).to be_a(ServiceResponse)
+          expect(subject).to be_success
         end
 
-        context 'when issues feature are disabled' do
-          let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+        it 'schedules Jira import' do
+          subject
 
-          it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.'
+          expect(project.latest_jira_import).to be_scheduled
         end
 
-        context 'when correct data provided' do
-          let(:fake_key)  { 'some-key' }
-
-          subject { described_class.new(user, project, fake_key).execute }
-
-          context 'when import is already running' do
-            let_it_be(:jira_import_state) { create(:jira_import_state, :started, project: project) }
+        it 'creates Jira import data' do
+          jira_import = subject.payload[:import_data]
 
-            it_behaves_like 'responds with error', 'Jira import is already running.'
-          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
-
-              expect(project.latest_jira_import).to be_scheduled
-            end
-          end
-
-          it 'creates Jira import data' do
-            jira_import = subject.payload[:import_data]
-
-            expect(jira_import.jira_project_xid).to eq(0)
-            expect(jira_import.jira_project_name).to eq(fake_key)
-            expect(jira_import.jira_project_key).to eq(fake_key)
-            expect(jira_import.user).to eq(user)
-          end
+          expect(jira_import.jira_project_xid).to eq(0)
+          expect(jira_import.jira_project_name).to eq(fake_key)
+          expect(jira_import.jira_project_key).to eq(fake_key)
+          expect(jira_import.user).to eq(user)
+        end
 
-          it 'creates Jira import label' do
-            expect { subject }.to change { Label.count }.by(1)
-          end
+        it 'creates Jira import label' do
+          expect { subject }.to change { Label.count }.by(1)
+        end
 
-          it 'creates Jira label title with correct number' do
-            jira_import = subject.payload[:import_data]
+        it 'creates Jira label title with correct number' do
+          jira_import = subject.payload[:import_data]
 
-            label_title = "jira-import::#{jira_import.jira_project_key}-1"
-            expect(jira_import.label.title).to eq(label_title)
-          end
+          label_title = "jira-import::#{jira_import.jira_project_key}-1"
+          expect(jira_import.label.title).to eq(label_title)
+        end
+      end
 
-          context 'when multiple Jira imports for same Jira project' do
-            let!(:jira_imports) { create_list(:jira_import_state, 3, :finished, project: project, jira_project_key: fake_key)}
+      context 'when multiple Jira imports for same Jira project' do
+        let!(:jira_imports) { create_list(:jira_import_state, 3, :finished, project: project, jira_project_key: fake_key)}
 
-            it 'creates Jira label title with correct number' do
-              jira_import = subject.payload[:import_data]
+        it 'creates Jira label title with correct number' do
+          jira_import = subject.payload[:import_data]
 
-              label_title = "jira-import::#{jira_import.jira_project_key}-4"
-              expect(jira_import.label.title).to eq(label_title)
-            end
-          end
+          label_title = "jira-import::#{jira_import.jira_project_key}-4"
+          expect(jira_import.label.title).to eq(label_title)
         end
       end
     end

spec/support/helpers/jira_service_helper.rb

@@ -78,6 +78,11 @@ module JiraServiceHelper
     JIRA_API + "/issue/#{issue_id}"
   end
 
+  def stub_jira_service_test
+    WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo')
+      .to_return(body: { url: 'http://url' }.to_json)
+  end
+
   def stub_jira_urls(issue_id)
     WebMock.stub_request(:get, jira_project_url)
     WebMock.stub_request(:get, jira_api_comment_url(issue_id)).to_return(body: jira_issue_comments)

spec/workers/gitlab/jira_import/stage/import_issues_worker_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::JiraImport::Stage::ImportIssuesWorker do
+  include JiraServiceHelper
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, import_type: 'jira') }
 
@@ -25,6 +27,7 @@ describe Gitlab::JiraImport::Stage::ImportIssuesWorker do
 
       before do
         stub_feature_flags(jira_issue_import: true)
+        stub_jira_service_test
       end
 
       context 'when import did not start' do

spec/workers/gitlab/jira_import/stage/import_labels_worker_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::JiraImport::Stage::ImportLabelsWorker do
+  include JiraServiceHelper
+
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, import_type: 'jira') }
 
@@ -36,6 +38,8 @@ describe Gitlab::JiraImport::Stage::ImportLabelsWorker do
         let!(:jira_service) { create(:jira_service, project: project) }
 
         before do
+          stub_jira_service_test
+
           jira_import.start!
 
           WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/label?maxResults=500&startAt=0')