Merge branch '7-12-fix-post-receive-external-tracker' into '7-12-stable'

Fix post-receive errors on a push when an external issue tracker is configured

Closes #1700
Closes #1720

See merge request !855

CHANGELOG

@@ -13,6 +13,7 @@ v 7.13.0 (unreleased)
 v 7.12.0 (unreleased)
   - Fix Error 500 when one user attempts to access a personal, internal snippet (Stan Hu)
   - Disable changing of target branch in new merge request page when a branch has already been specified (Stan Hu)
+  - Fix post-receive errors on a push when an external issue tracker is configured (Stan Hu)
   - Update oauth button logos for Twitter and Google to recommended assets
   - Update browser gem to version 0.8.0 for IE11 support (Stan Hu)
   - Fix timeout when rendering file with thousands of lines.

app/services/git_push_service.rb

@@ -88,6 +88,13 @@ class GitPushService
         end
       end
 
+      if project.default_issues_tracker?
+        create_cross_reference_notes(commit, issues_to_close)
+      end
+    end
+  end
+
+  def create_cross_reference_notes(commit, issues_to_close)
     # Create cross-reference notes for any other references. Omit any issues that were referenced in an
     # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
     # being pushed to a different branch).
@@ -102,7 +109,6 @@ class GitPushService
       end
     end
   end
-  end
 
   def build_push_data(oldrev, newrev, ref)
     Gitlab::PushDataBuilder.

app/services/issues/close_service.rb

 module Issues
   class CloseService < Issues::BaseService
     def execute(issue, commit = nil)
-      if issue.close
+      if project.default_issues_tracker? && issue.close
         event_service.close_issue(issue, current_user)
         create_note(issue, commit)
         notification_service.close_issue(issue, current_user)

spec/services/git_push_service_spec.rb

@@ -233,6 +233,15 @@ describe GitPushService do
 
       expect(Issue.find(issue.id)).to be_opened
     end
+
+    it "doesn't close issues when external issue tracker is in use" do
+      allow(project).to receive(:default_issues_tracker?).and_return(false)
+
+      # The push still shouldn't create cross-reference notes.
+      expect {
+        service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
+      }.not_to change { Note.where(project_id: project.id, system: true).count }
+    end
   end
 
   describe "empty project" do

spec/services/issues/close_service_spec.rb

@@ -31,5 +31,15 @@ describe Issues::CloseService do
         expect(note.note).to include "Status changed to closed"
       end
     end
+
+    context "external issue tracker" do
+      before do
+        allow(project).to receive(:default_issues_tracker?).and_return(false)
+        @issue = Issues::CloseService.new(project, user, {}).execute(issue)
+      end
+
+      it { expect(@issue).to be_valid }
+      it { expect(@issue).to be_opened }
+    end
   end
 end