Merge branch 'fix-ext-issue-webbased-hooks' into 'master'

Fix hooks for web based events with external issue references

The creation of cross references for external issues (which would fail)
is now prevented. This fixes a 500 error and the execution of hooks when
creating a merge request or commenting on one.

Fixes #1650, GH-9333.

This regression might have been introduced by
8f8a8ab32bca8fdc79d7a5115eabbd015dd44c02. There is actually a TODO by
@rspeicher mentioning external issues which is lost (unhandled) in a
later commit.

This fix is related to !766 and !804 which fix a similar issue for hooks for
Git based events.

See merge request !794

3 files changed, 13 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 9b6a3055397..e881fa42a20 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v 7.13.0 (unreleased)
 v 7.12.0 (unreleased)
   - 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
+  - Fix hooks for web based events with external issue references (Daniel Gerhardt)
   - Update browser gem to version 0.8.0 for IE11 support (Stan Hu)
   - Fix timeout when rendering file with thousands of lines.
   - Add "Remember me" checkbox to LDAP signin form.
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index b6801a92330..8253c1f780d 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -212,13 +212,15 @@ class SystemNoteService
   # Check if a cross-reference is disallowed
   #
   # This method prevents adding a "mentioned in !1" note on every single commit
-  # in a merge request.
+  # in a merge request. Additionally, it prevents the creation of references to
+  # external issues (which would fail).
   #
   # noteable  - Noteable object being referenced
   # mentioner - Mentionable object
   #
   # Returns Boolean
   def self.cross_reference_disallowed?(noteable, mentioner)
+    return true if noteable.is_a?(ExternalIssue)
     return false unless mentioner.is_a?(MergeRequest)
     return false unless noteable.is_a?(Commit)
 
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 700286b585a..2658576640c 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -338,6 +338,15 @@ describe SystemNoteService do
           to be_falsey
       end
     end
+
+    context 'when notable is an ExternalIssue' do
+      let(:noteable) { ExternalIssue.new('EXT-1234', project) }
+      it 'is truthy' do
+        mentioner = noteable.dup
+        expect(described_class.cross_reference_disallowed?(noteable, mentioner)).
+          to be_truthy
+      end
+    end
   end
 
   describe '.cross_reference_exists?' do