Merge branch 'open-project-integration-1' into 'master'

Extract methods to issue tracker services

See merge request gitlab-org/gitlab!26773

app/models/project.rb

@@ -1190,6 +1190,10 @@ class Project < ApplicationRecord
     update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) if Gitlab::Database.read_write?
   end
 
+  def external_references_supported?
+    external_issue_tracker&.support_cross_reference?
+  end
+
   def has_wiki?
     wiki_enabled? || has_external_wiki?
   end

app/models/project_services/issue_tracker_service.rb

@@ -151,6 +151,14 @@ class IssueTrackerService < Service
     result
   end
 
+  def support_close_issue?
+    false
+  end
+
+  def support_cross_reference?
+    false
+  end
+
   private
 
   def enabled_in_gitlab_config

app/models/project_services/jira_service.rb

 # frozen_string_literal: true
 
 class JiraService < IssueTrackerService
+  extend ::Gitlab::Utils::Override
   include Gitlab::Routing
   include ApplicationHelper
   include ActionView::Helpers::AssetUrlHelper
@@ -205,6 +206,16 @@ class JiraService < IssueTrackerService
     nil
   end
 
+  override :support_close_issue?
+  def support_close_issue?
+    true
+  end
+
+  override :support_cross_reference?
+  def support_cross_reference?
+    true
+  end
+
   private
 
   def test_settings

app/services/issues/close_service.rb

@@ -18,9 +18,9 @@ module Issues
     # The code calling this method is responsible for ensuring that a user is
     # allowed to close the given issue.
     def close_issue(issue, closed_via: nil, notifications: true, system_note: true)
-      if project.jira_tracker_active? && issue.is_a?(ExternalIssue)
-        project.jira_service.close_issue(closed_via, issue)
-        todo_service.close_issue(issue, current_user)
+      if issue.is_a?(ExternalIssue)
+        close_external_issue(issue, closed_via)
+
         return issue
       end
 
@@ -47,6 +47,13 @@ module Issues
 
     private
 
+    def close_external_issue(issue, closed_via)
+      return unless project.external_issue_tracker&.support_close_issue?
+
+      project.external_issue_tracker.close_issue(closed_via, issue)
+      todo_service.close_issue(issue, current_user)
+    end
+
     def create_note(issue, current_commit)
       SystemNoteService.change_status(issue, issue.project, current_user, issue.state, current_commit)
     end

app/services/system_notes/issuables_service.rb

@@ -144,7 +144,7 @@ module SystemNotes
     #
     # Returns Boolean
     def cross_reference_disallowed?(mentioner)
-      return true if noteable.is_a?(ExternalIssue) && !noteable.project.jira_tracker_active?
+      return true if noteable.is_a?(ExternalIssue) && !noteable.project&.external_references_supported?
       return false unless mentioner.is_a?(MergeRequest)
       return false unless noteable.is_a?(Commit)
 

lib/gitlab/reference_extractor.rb

@@ -44,7 +44,7 @@ module Gitlab
     end
 
     def issues
-      if project && project.jira_tracker?
+      if project&.external_references_supported?
         if project.issues_enabled?
           @references[:all_issues] ||= references(:external_issue) + references(:issue)
         else

spec/lib/gitlab/reference_extractor_spec.rb

@@ -225,6 +225,24 @@ describe Gitlab::ReferenceExtractor do
     end
   end
 
+  context 'with an inactive external issue tracker' do
+    let(:project) { create(:project) }
+    let!(:jira_service) { create(:jira_service, project: project, active: false) }
+    let(:issue)   { create(:issue, project: project) }
+
+    context 'when GitLab issues are enabled' do
+      it 'returns only internal issue' do
+        subject.analyze("JIRA-123 and FOOBAR-4567 and #{issue.to_reference}")
+        expect(subject.issues).to eq([issue])
+      end
+
+      it 'does not return any issue if the internal one does not exists' do
+        subject.analyze("JIRA-123 and FOOBAR-4567 and #999")
+        expect(subject.issues).to be_empty
+      end
+    end
+  end
+
   context 'with a project with an underscore' do
     let(:other_project) { create(:project, path: 'test_project') }
     let(:issue) { create(:issue, project: other_project) }

spec/services/issues/close_service_spec.rb

@@ -70,6 +70,38 @@ describe Issues::CloseService do
   end
 
   describe '#close_issue' do
+    context 'with external issue' do
+      context 'with an active external issue tracker supporting close_issue' do
+        let!(:external_issue_tracker) { create(:jira_service, project: project) }
+
+        it 'closes the issue on the external issue tracker' do
+          expect(project.external_issue_tracker).to receive(:close_issue)
+
+          described_class.new(project, user).close_issue(external_issue)
+        end
+      end
+
+      context 'with innactive external issue tracker supporting close_issue' do
+        let!(:external_issue_tracker) { create(:jira_service, project: project, active: false) }
+
+        it 'does not close the issue on the external issue tracker' do
+          expect(project.external_issue_tracker).not_to receive(:close_issue)
+
+          described_class.new(project, user).close_issue(external_issue)
+        end
+      end
+
+      context 'with an active external issue tracker not supporting close_issue' do
+        let!(:external_issue_tracker) { create(:bugzilla_service, project: project) }
+
+        it 'does not close the issue on the external issue tracker' do
+          expect(project.external_issue_tracker).not_to receive(:close_issue)
+
+          described_class.new(project, user).close_issue(external_issue)
+        end
+      end
+    end
+
     context "closed by a merge request", :sidekiq_might_not_need_inline do
       it 'mentions closure via a merge request' do
         perform_enqueued_jobs do

spec/services/merge_requests/merge_service_spec.rb

@@ -158,7 +158,7 @@ describe MergeRequests::MergeService do
           end
 
           it 'does not close issue' do
-            allow(jira_tracker).to receive_messages(jira_issue_transition_id: nil)
+            jira_tracker.update(jira_issue_transition_id: nil)
 
             expect_any_instance_of(JiraService).not_to receive(:transition_issue)
 

spec/services/system_notes/issuables_service_spec.rb

@@ -598,8 +598,8 @@ describe ::SystemNotes::IssuablesService do
     context 'when mentioner is not a MergeRequest' do
       it 'is falsey' do
         mentioner = noteable.dup
-        expect(service.cross_reference_disallowed?(mentioner))
-          .to be_falsey
+
+        expect(service.cross_reference_disallowed?(mentioner)).to be_falsey
       end
     end
 
@@ -609,24 +609,35 @@ describe ::SystemNotes::IssuablesService do
 
       it 'is truthy when noteable is in commits' do
         expect(mentioner).to receive(:commits).and_return([noteable])
-        expect(service.cross_reference_disallowed?(mentioner))
-          .to be_truthy
+
+        expect(service.cross_reference_disallowed?(mentioner)).to be_truthy
       end
 
       it 'is falsey when noteable is not in commits' do
         expect(mentioner).to receive(:commits).and_return([])
-        expect(service.cross_reference_disallowed?(mentioner))
-          .to be_falsey
+
+        expect(service.cross_reference_disallowed?(mentioner)).to be_falsey
       end
     end
 
     context 'when notable is an ExternalIssue' do
+      let(:project) { create(:project) }
       let(:noteable) { ExternalIssue.new('EXT-1234', project) }
 
-      it 'is truthy' do
-        mentioner = noteable.dup
-        expect(service.cross_reference_disallowed?(mentioner))
-          .to be_truthy
+      it 'is false with issue tracker supporting referencing' do
+        create(:jira_service, project: project)
+
+        expect(service.cross_reference_disallowed?(noteable)).to be_falsey
+      end
+
+      it 'is true with issue tracker not supporting referencing' do
+        create(:bugzilla_service, project: project)
+
+        expect(service.cross_reference_disallowed?(noteable)).to be_truthy
+      end
+
+      it 'is true without issue tracker' do
+        expect(service.cross_reference_disallowed?(noteable)).to be_truthy
       end
     end
   end