Merge branch '7341-epics-extractor' into 'master'

Include (closed) for closed epics in parsed text

Closes #7341

See merge request gitlab-org/gitlab-ee!7946

ee/changelogs/unreleased/7341-epics-extractor.yml

+---
+title: Include (closed) for closed epics in parsed text
+merge_request: 7946
+author:
+type: fixed

ee/lib/ee/banzai/issuable_extractor.rb

+# frozen_string_literal: true
+
+module EE
+  module Banzai
+    module IssuableExtractor
+      EPIC_REFERENCE_TYPE = '@data-reference-type="epic"'.freeze
+
+      private
+
+      def reference_types
+        super.push(EPIC_REFERENCE_TYPE)
+      end
+
+      def parsers
+        super.push(::Banzai::ReferenceParser::EpicParser.new(context))
+      end
+    end
+  end
+end

ee/spec/lib/banzai/filter/issuable_state_filter_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Banzai::Filter::IssuableStateFilter do
+  include ActionView::Helpers::UrlHelper
+  include FilterSpecHelper
+
+  let(:user) { create(:user) }
+  let(:context) { { current_user: user, issuable_state_filter_enabled: true, group: group } }
+  let(:epic) { create(:epic, :opened, group: group) }
+  let(:closed_epic) { create(:epic, :closed, group: group) }
+  let(:group) { create(:group) }
+  let(:other_group) { create(:group) }
+
+  def create_link(text, data)
+    link_to(text, '', class: 'gfm has-tooltip', data: data)
+  end
+
+  it 'ignores open epic references' do
+    link = create_link(epic.to_reference, epic: epic.id, reference_type: 'epic')
+
+    doc = filter(link, context)
+
+    expect(doc.css('a').last.text).to eq(epic.to_reference)
+  end
+
+  it 'appends state to closed epic references' do
+    link = create_link(closed_epic.to_reference, epic: closed_epic.id, reference_type: 'epic')
+
+    doc = filter(link, context)
+
+    expect(doc.css('a').last.text).to eq("#{closed_epic.to_reference} (closed)")
+  end
+
+  it 'skips cross references if the user cannot read cross group' do
+    expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false }
+
+    link = create_link(closed_epic.to_reference(other_group), epic: closed_epic.id, reference_type: 'epic')
+
+    doc = filter(link, context.merge(group: other_group))
+
+    expect(doc.css('a').last.text).to eq("#{closed_epic.to_reference(other_group)}")
+  end
+end

ee/spec/lib/banzai/issuable_extractor_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Banzai::IssuableExtractor do
+  it 'returns an instance of an epic for the node with reference' do
+    epic = create(:epic)
+    user = create(:user)
+    epic_link = Nokogiri::HTML.fragment(
+      "<a href='' data-epic='#{epic.id}' data-reference-type='epic' class='gfm'>text</a>"
+    ).children[0]
+
+    result = described_class.new(Banzai::RenderContext.new(nil, user)).extract([epic_link])
+
+    expect(result).to eq(epic_link => epic)
+  end
+end

lib/banzai/filter/issuable_state_filter.rb

@@ -18,7 +18,7 @@ module Banzai
         issuables = extractor.extract([doc])
 
         issuables.each do |node, issuable|
-          next if !can_read_cross_project? && issuable.project != project
+          next if !can_read_cross_project? && cross_reference?(issuable)
 
           if VISIBLE_STATES.include?(issuable.state) && issuable_reference?(node.inner_html, issuable)
             node.content += " (#{issuable.state})"
@@ -31,7 +31,14 @@ module Banzai
       private
 
       def issuable_reference?(text, issuable)
-        text == issuable.reference_link_text(project || group)
+        CGI.unescapeHTML(text) == issuable.reference_link_text(project || group)
+      end
+
+      def cross_reference?(issuable)
+        return true if issuable.project != project
+        return true if issuable.respond_to?(:group) && issuable.group != group
+
+        false
       end
 
       def can_read_cross_project?

lib/banzai/issuable_extractor.rb

@@ -9,13 +9,13 @@ module Banzai
   # so we can avoid N+1 queries problem
 
   class IssuableExtractor
-    QUERY = %q(
-      descendant-or-self::a[contains(concat(" ", @class, " "), " gfm ")]
-      [@data-reference-type="issue" or @data-reference-type="merge_request"]
-    ).freeze
+    prepend EE::Banzai::IssuableExtractor
 
     attr_reader :context
 
+    ISSUE_REFERENCE_TYPE = '@data-reference-type="issue"'.freeze
+    MERGE_REQUEST_REFERENCE_TYPE = '@data-reference-type="merge_request"'.freeze
+
     # context - An instance of Banzai::RenderContext.
     def initialize(context)
       @context = context
@@ -24,21 +24,38 @@ module Banzai
     # Returns Hash in the form { node => issuable_instance }
     def extract(documents)
       nodes = documents.flat_map do |document|
-        document.xpath(QUERY)
+        document.xpath(query)
       end
 
-      issue_parser = Banzai::ReferenceParser::IssueParser.new(context)
+      # The project or group for the issuable might be pending for deletion!
+      # Filter them out because we don't care about them.
+      issuables_for_nodes(nodes).select { |node, issuable| issuable.project || issuable.group }
+    end
+
+    private
+
+    def issuables_for_nodes(nodes)
+      parsers.each_with_object({}) do |parser, result|
+        result.merge!(parser.records_for_nodes(nodes))
+      end
+    end
 
-      merge_request_parser =
+    def parsers
+      [
+        Banzai::ReferenceParser::IssueParser.new(context),
         Banzai::ReferenceParser::MergeRequestParser.new(context)
+      ]
+    end
 
-      issuables_for_nodes = issue_parser.records_for_nodes(nodes).merge(
-        merge_request_parser.records_for_nodes(nodes)
+    def query
+      %Q(
+        descendant-or-self::a[contains(concat(" ", @class, " "), " gfm ")]
+        [#{reference_types.join(' or ')}]
       )
+    end
 
-      # The project for the issue/MR might be pending for deletion!
-      # Filter them out because we don't care about them.
-      issuables_for_nodes.select { |node, issuable| issuable.project }
+    def reference_types
+      [ISSUE_REFERENCE_TYPE, MERGE_REQUEST_REFERENCE_TYPE]
     end
   end
 end