Merge branch 'ignore-references' into 'master'

Don't notify users mentioned in code blocks or blockquotes.

cc @rspeicher

See merge request !753

7 files changed, 83 insertions, 60 deletions

diff --git a/CHANGELOG b/CHANGELOG
index dbc54021d40..85fa933fa6b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.12.0 (unreleased)
+  - Don't notify users mentioned in code blocks or blockquotes.
   - Disable changing of the source branch in merge request update API (Stan Hu)
   - Shorten merge request WIP text.
   - Add option to disallow users from registering any application to use GitLab as an OAuth provider
diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index 3c207619adf..2777944fc9d 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -41,29 +41,26 @@ module GitlabMarkdownHelper
     fragment.to_html.html_safe
   end
 
+  MARKDOWN_OPTIONS = {
+    no_intra_emphasis:   true,
+    tables:              true,
+    fenced_code_blocks:  true,
+    strikethrough:       true,
+    lax_spacing:         true,
+    space_after_headers: true,
+    superscript:         true,
+    footnotes:           true
+  }.freeze
+
   def markdown(text, options={})
     unless @markdown && options == @options
       @options = options
 
-      options.merge!(
-        # Handled further down the line by Gitlab::Markdown::SanitizationFilter
-        escape_html: false
-      )
-
       # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch
       rend = Redcarpet::Render::GitlabHTML.new(self, user_color_scheme_class, options)
 
       # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
-      @markdown = Redcarpet::Markdown.new(rend,
-        no_intra_emphasis:   true,
-        tables:              true,
-        fenced_code_blocks:  true,
-        strikethrough:       true,
-        lax_spacing:         true,
-        space_after_headers: true,
-        superscript:         true,
-        footnotes:           true
-      )
+      @markdown = Redcarpet::Markdown.new(rend, MARKDOWN_OPTIONS)
     end
 
     @markdown.render(text).html_safe
diff --git a/lib/gitlab/markdown/reference_filter.rb b/lib/gitlab/markdown/reference_filter.rb
index be4d26af0fc..a84bacd3d4f 100644
--- a/lib/gitlab/markdown/reference_filter.rb
+++ b/lib/gitlab/markdown/reference_filter.rb
@@ -25,12 +25,18 @@ module Gitlab
         ERB::Util.html_escape_once(html)
       end
 
-      # Don't look for references in text nodes that are children of these
-      # elements.
-      IGNORE_PARENTS = %w(pre code a style).to_set
+      def ignore_parents
+        @ignore_parents ||= begin
+          # Don't look for references in text nodes that are children of these
+          # elements.
+          parents = %w(pre code a style)
+          parents << 'blockquote' if context[:ignore_blockquotes]
+          parents.to_set
+        end
+      end
 
       def ignored_ancestry?(node)
-        has_ancestor?(node, IGNORE_PARENTS)
+        has_ancestor?(node, ignore_parents)
       end
 
       def project
diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb
index e35f848fa6e..e836b05ff25 100644
--- a/lib/gitlab/reference_extractor.rb
+++ b/lib/gitlab/reference_extractor.rb
@@ -1,7 +1,7 @@
 module Gitlab
   # Extract possible GFM references from an arbitrary String for further processing.
   class ReferenceExtractor
-    attr_accessor :project, :current_user, :references
+    attr_accessor :project, :current_user
 
     def initialize(project, current_user = nil)
       @project = project
@@ -9,48 +9,31 @@ module Gitlab
     end
 
     def analyze(text)
-      @_text = text.dup
+      references.clear
+      @text = markdown.render(text.dup)
     end
 
-    def users
-      result = pipeline_result(:user)
-      result.uniq
+    %i(user label issue merge_request snippet commit commit_range).each do |type|
+      define_method("#{type}s") do
+        references[type]
+      end
     end
 
-    def labels
-      result = pipeline_result(:label)
-      result.uniq
-    end
-
-    def issues
-      # TODO (rspeicher): What about external issues?
-
-      result = pipeline_result(:issue)
-      result.uniq
-    end
-
-    def merge_requests
-      result = pipeline_result(:merge_request)
-      result.uniq
-    end
+    private
 
-    def snippets
-      result = pipeline_result(:snippet)
-      result.uniq
+    def markdown
+      @markdown ||= Redcarpet::Markdown.new(Redcarpet::Render::HTML, GitlabMarkdownHelper::MARKDOWN_OPTIONS)
     end
 
-    def commits
-      result = pipeline_result(:commit)
-      result.uniq
-    end
+    def references
+      @references ||= Hash.new do |references, type|
+        type = type.to_sym
+        return references[type] if references.has_key?(type)
 
-    def commit_ranges
-      result = pipeline_result(:commit_range)
-      result.uniq
+        references[type] = pipeline_result(type).uniq
+      end
     end
 
-    private
-
     # Instantiate and call HTML::Pipeline with a single reference filter type,
     # returning the result
     #
@@ -65,11 +48,12 @@ module Gitlab
         project: project,
         current_user: current_user,
         # We don't actually care about the links generated
-        only_path: true
+        only_path: true,
+        ignore_blockquotes: true
       }
 
       pipeline = HTML::Pipeline.new([filter], context)
-      result = pipeline.call(@_text)
+      result = pipeline.call(@text)
 
       result[:references][filter_type]
     end
diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb
index 7dcecc2ecf6..2f7aff03c2a 100644
--- a/lib/redcarpet/render/gitlab_html.rb
+++ b/lib/redcarpet/render/gitlab_html.rb
@@ -10,6 +10,8 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     @options = options.dup
 
     @options.reverse_merge!(
+      # Handled further down the line by Gitlab::Markdown::SanitizationFilter
+      escape_html: false,
       project: @template.instance_variable_get("@project")
     )
 
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index c14f4ac6bf6..f921dd9cc09 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -16,6 +16,30 @@ describe Gitlab::ReferenceExtractor do
     expect(subject.users).to eq([@u_foo, @u_bar, @u_offteam])
   end
 
+  it 'ignores user mentions inside specific elements' do
+    @u_foo = create(:user, username: 'foo')
+    @u_bar = create(:user, username: 'bar')
+    @u_offteam = create(:user, username: 'offteam')
+
+    project.team << [@u_foo, :reporter]
+    project.team << [@u_bar, :guest]
+
+    subject.analyze(%Q{
+      Inline code: `@foo` 
+
+      Code block:
+
+      ```
+      @bar
+      ```
+
+      Quote: 
+
+      > @offteam
+    })
+    expect(subject.users).to eq([])
+  end
+
   it 'accesses valid issue objects' do
     @i0 = create(:issue, project: project)
     @i1 = create(:issue, project: project)
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index ede62e8f37a..d29c8a55c82 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -107,17 +107,26 @@ shared_examples 'an editable mentionable' do
   it 'creates new cross-reference notes when the mentionable text is edited' do
     subject.save
 
-    new_text = <<-MSG
+    new_text = <<-MSG.strip_heredoc
       These references already existed:
-        Issue:  #{mentioned_issue.to_reference}
-        Commit: #{mentioned_commit.to_reference}
+
+      Issue:  #{mentioned_issue.to_reference}
+
+      Commit: #{mentioned_commit.to_reference}
+
+      ---
 
       This cross-project reference already existed:
-        Issue:  #{ext_issue.to_reference(project)}
+
+      Issue:  #{ext_issue.to_reference(project)}
+
+      ---
 
       These two references are introduced in an edit:
-        Issue: #{new_issues[0].to_reference}
-        Cross: #{new_issues[1].to_reference(project)}
+
+      Issue: #{new_issues[0].to_reference}
+
+      Cross: #{new_issues[1].to_reference(project)}
     MSG
 
     # These three objects were already referenced, and should not receive new