Added indication for closed issues in notesadam_1369_strikethrough_closed_issues

For issues that are closed, the links will now show '[closed]' following
the issue number. This is done as post-process after the markdown has
been loaded from the cache as the status of the issue may change between
the cache being populated and the note being displayed.

Also as part of this change, the Banzai
BaseParser#grouped_objects_for_nodes method has been refactored to return a
Hash utilising the node itself as the key, since this was a common
pattern of usage for this method.

13 files changed, 201 insertions, 29 deletions

diff --git a/lib/banzai/filter/issuable_state_filter.rb b/lib/banzai/filter/issuable_state_filter.rb
new file mode 100644
index 00000000000..75d23c11dd7
--- /dev/null
+++ b/lib/banzai/filter/issuable_state_filter.rb
@@ -0,0 +1,41 @@
+module Banzai
+  module Filter
+    # HTML filter that appends state information to issuable links.
+    # Runs as a post-process filter as issuable state might change whilst
+    # Markdown is in the cache.
+    #
+    # This filter supports cross-project references.
+    class IssuableStateFilter < HTML::Pipeline::Filter
+      def call
+        nodes  = Querying.css(doc, 'a.gfm[data-reference-type=merge_request]')
+        nodes += Querying.css(doc, 'a.gfm[data-reference-type=issue]')
+        return doc unless nodes.count > 0
+
+        issue_parser = Banzai::ReferenceParser::IssueParser.new(project, current_user)
+        mr_parser = Banzai::ReferenceParser::MergeRequestParser.new(project, current_user)
+
+        issuables = issue_parser.issues_for_nodes(nodes)
+        issuables = issuables.merge(mr_parser.merge_requests_for_nodes(nodes))
+
+        nodes.each do |node|
+          issuable = issuables[node]
+          if issuable && issuable.closed?
+            node.children.last.content += ' [closed]'
+          end
+        end
+
+        doc
+      end
+
+      private
+
+      def current_user
+        context[:current_user]
+      end
+
+      def project
+        context[:project]
+      end
+    end
+  end
+end
diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb
index c59a80dd1c7..9f9882b3b40 100644
--- a/lib/banzai/filter/redactor_filter.rb
+++ b/lib/banzai/filter/redactor_filter.rb
@@ -7,7 +7,7 @@ module Banzai
     #
     class RedactorFilter < HTML::Pipeline::Filter
       def call
-        Redactor.new(project, current_user).redact([doc])
+        Redactor.new(project, current_user).redact([doc]) unless context[:skip_redaction]
 
         doc
       end
diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb
index 9f8eb0931b8..10e7ab996e8 100644
--- a/lib/banzai/object_renderer.rb
+++ b/lib/banzai/object_renderer.rb
@@ -58,7 +58,7 @@ module Banzai
     # Returns a Banzai context for the given object and attribute.
     def context_for(object, attribute)
       context = base_context.dup
-      context = context.merge(object.banzai_render_context(attribute))
+      context = context.merge(object.banzai_render_context(attribute)).merge(skip_redaction: true)
       context
     end
 
@@ -70,7 +70,7 @@ module Banzai
         string = Banzai.render_field(object, attribute)
         context = context_for(object, attribute)
 
-        Banzai::Pipeline[:relative_link].to_document(string, context)
+        Banzai::Pipeline[:post_process].to_document(string, context)
       end
     end
 
diff --git a/lib/banzai/pipeline/post_process_pipeline.rb b/lib/banzai/pipeline/post_process_pipeline.rb
index ecff094b1e5..a22b750041c 100644
--- a/lib/banzai/pipeline/post_process_pipeline.rb
+++ b/lib/banzai/pipeline/post_process_pipeline.rb
@@ -4,7 +4,8 @@ module Banzai
       def self.filters
         FilterArray[
           Filter::RelativeLinkFilter,
-          Filter::RedactorFilter
+          Filter::RedactorFilter,
+          Filter::IssuableStateFilter
         ]
       end
 
diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb
index d8a855ec1fe..f61013904f9 100644
--- a/lib/banzai/reference_parser/base_parser.rb
+++ b/lib/banzai/reference_parser/base_parser.rb
@@ -62,8 +62,7 @@ module Banzai
 
         nodes.select do |node|
           if node.has_attribute?(project_attr)
-            node_id = node.attr(project_attr).to_i
-            can_read_reference?(user, projects[node_id])
+            can_read_reference?(user, projects[node])
           else
             true
           end
@@ -112,12 +111,12 @@ module Banzai
         per_project
       end
 
-      # Returns a Hash containing objects for an attribute grouped per their
-      # IDs.
+      # Returns a Hash containing objects for an attribute grouped per the
+      # nodes that reference them.
       #
       # The returned Hash uses the following format:
       #
-      #     { id value => row }
+      #     { node => row }
       #
       # nodes - An Array of HTML nodes to process.
       #
@@ -131,11 +130,14 @@ module Banzai
       def grouped_objects_for_nodes(nodes, collection, attribute)
         return {} if nodes.empty?
 
-        ids = unique_attribute_values(nodes, attribute)
-        rows = collection_objects_for_ids(collection, ids)
+        ids = unique_attribute_values(nodes, attribute).sort_by(&:to_i)
+        rows = collection_objects_for_ids(collection, ids).sort_by(&:id)
+        objects_by_id = Hash[ids.zip(rows)]
 
-        rows.each_with_object({}) do |row, hash|
-          hash[row.id] = row
+        nodes.each_with_object({}) do |node, hash|
+          if node.has_attribute?(attribute)
+            hash[node] = objects_by_id[node.attr(attribute)]
+          end
         end
       end
 
diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb
index 6c20dec5734..4196145749c 100644
--- a/lib/banzai/reference_parser/issue_parser.rb
+++ b/lib/banzai/reference_parser/issue_parser.rb
@@ -13,14 +13,14 @@ module Banzai
           issues_readable_by_user(issues.values, user).to_set
 
         nodes.select do |node|
-          readable_issues.include?(issue_for_node(issues, node))
+          readable_issues.include?(issues[node])
         end
       end
 
       def referenced_by(nodes)
         issues = issues_for_nodes(nodes)
 
-        nodes.map { |node| issue_for_node(issues, node) }.uniq
+        nodes.map { |node| issues[node] }.uniq
       end
 
       def issues_for_nodes(nodes)
@@ -44,12 +44,6 @@ module Banzai
           self.class.data_attribute
         )
       end
-
-      private
-
-      def issue_for_node(issues, node)
-        issues[node.attr(self.class.data_attribute).to_i]
-      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb
index 40451947e6c..8ce6618b835 100644
--- a/lib/banzai/reference_parser/merge_request_parser.rb
+++ b/lib/banzai/reference_parser/merge_request_parser.rb
@@ -3,6 +3,26 @@ module Banzai
     class MergeRequestParser < BaseParser
       self.reference_type = :merge_request
 
+      def merge_requests_for_nodes(nodes)
+        @merge_requests_for_nodes ||= grouped_objects_for_nodes(
+          nodes,
+          MergeRequest.all.includes(
+            :author,
+            :assignee,
+            # These associations are primarily used for checking permissions.
+            # Eager loading these ensures we don't end up running dozens of
+            # queries in this process.
+            target_project: [
+              { namespace: :owner },
+              { group: [:owners, :group_members] },
+              :invited_groups,
+              :project_members
+            ]
+          ),
+          self.class.data_attribute
+        )
+      end
+
       def references_relation
         MergeRequest.includes(:author, :assignee, :target_project)
       end
diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb
index 7adaffa19c1..09b66cbd8fb 100644
--- a/lib/banzai/reference_parser/user_parser.rb
+++ b/lib/banzai/reference_parser/user_parser.rb
@@ -49,7 +49,7 @@ module Banzai
       # Check if project belongs to a group which
       # user can read.
       def can_read_group_reference?(node, user, groups)
-        node_group = groups[node.attr('data-group').to_i]
+        node_group = groups[node]
 
         node_group && can?(user, :read_group, node_group)
       end
@@ -74,8 +74,8 @@ module Banzai
           if project && project_id && project.id == project_id.to_i
             true
           elsif project_id && user_id
-            project = projects[project_id.to_i]
-            user = users[user_id.to_i]
+            project = projects[node]
+            user = users[node]
 
             project && user ? project.team.member?(user) : false
           else
diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
new file mode 100644
index 00000000000..fb01b341866
--- /dev/null
+++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
@@ -0,0 +1,71 @@
+require 'spec_helper'
+
+describe Banzai::Filter::IssuableStateFilter, lib: true do
+  include ActionView::Helpers::UrlHelper
+  include FilterSpecHelper
+
+  let(:user)    { create(:user) }
+
+  def create_link(data)
+    link_to('text', '', class: 'gfm', data: data)
+  end
+
+  it 'ignores non-GFM links' do
+    html = %(See <a href="https://google.com/">Google</a>)
+    doc = filter(html, current_user: user)
+    expect(doc.css('a').last.text).not_to include('[closed]')
+  end
+
+  it 'ignores non-issuable links' do
+    project = create(:empty_project, :public)
+    link = create_link(project: project, reference_type: 'issue')
+    doc = filter(link, current_user: user)
+    expect(doc.css('a').last.text).not_to include('[closed]')
+  end
+
+  context 'for issue references' do
+    it 'ignores open issue references' do
+      issue = create(:issue)
+      link = create_link(issue: issue.id, reference_type: 'issue')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).not_to include('[closed]')
+    end
+
+    it 'ignores reopened issue references' do
+      reopened_issue = create(:issue, :reopened)
+      link = create_link(issue: reopened_issue.id, reference_type: 'issue')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).not_to include('[closed]')
+    end
+
+    it 'appends [closed] to closed issue references' do
+      closed_issue = create(:issue, :closed)
+      link = create_link(issue: closed_issue.id, reference_type: 'issue')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).to include('[closed]')
+    end
+  end
+
+  context 'for merge request references' do
+    it 'ignores open merge request references' do
+      mr = create(:merge_request)
+      link = create_link(merge_request: mr.id, reference_type: 'merge_request')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).not_to include('[closed]')
+    end
+
+    it 'ignores reopened merge request references' do
+      mr = create(:merge_request, :reopened)
+      link = create_link(merge_request: mr.id, reference_type: 'merge_request')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).not_to include('[closed]')
+    end
+
+    it 'appends [closed] to closed merge request references' do
+      mr = create(:merge_request, :closed)
+      link = create_link(merge_request: mr.id, reference_type: 'merge_request')
+      doc = filter(link, current_user: user)
+      expect(doc.css('a').last.text).to include('[closed]')
+    end
+  end
+end
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index 0140a91c7ba..8a6fe1ad6a3 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -15,6 +15,16 @@ describe Banzai::Filter::RedactorFilter, lib: true do
     link_to('text', '', class: 'gfm', data: data)
   end
 
+  it 'skips when the skip_redaction flag is set' do
+    user = create(:user)
+    project = create(:empty_project)
+
+    link = reference_link(project: project.id, reference_type: 'test')
+    doc = filter(link, current_user: user, skip_redaction: true)
+
+    expect(doc.css('a').length).to eq 1
+  end
+
   context 'with data-project' do
     let(:parser_class) do
       Class.new(Banzai::ReferenceParser::BaseParser) do
diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb
index 6bcda87c999..74a964198bb 100644
--- a/spec/lib/banzai/object_renderer_spec.rb
+++ b/spec/lib/banzai/object_renderer_spec.rb
@@ -78,6 +78,29 @@ describe Banzai::ObjectRenderer do
       expect(context).to have_key(:foo)
       expect(context[:foo]).to eq(:bar)
     end
+
+    it 'adds the :skip_redaction flag' do
+      context = renderer.context_for(object, :note)
+      expect(context[:skip_redaction]).to eq(true)
+    end
+
+    context 'when the object responds to "author"' do
+      it 'includes the author in the context' do
+        expect(object).to receive(:author).and_return('Alice')
+
+        context = renderer.context_for(object, :note)
+
+        expect(context[:author]).to eq('Alice')
+      end
+    end
+
+    context 'when the object does not respond to "author"' do
+      it 'does not include the author in the context' do
+        context = renderer.context_for(object, :note)
+
+        expect(context.key?(:author)).to eq(false)
+      end
+    end
   end
 
   describe '#render_attributes' do
diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb
index aa127f0179d..a3141894c74 100644
--- a/spec/lib/banzai/reference_parser/base_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb
@@ -92,16 +92,26 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
   end
 
   describe '#grouped_objects_for_nodes' do
-    it 'returns a Hash grouping objects per ID' do
-      nodes = [double(:node)]
+    it 'returns a Hash grouping objects per node' do
+      link = double(:link)
+
+      expect(link).to receive(:has_attribute?).
+        with('data-user').
+        and_return(true)
+
+      expect(link).to receive(:attr).
+        with('data-user').
+        and_return(user.id.to_s)
+
+      nodes = [link]
 
       expect(subject).to receive(:unique_attribute_values).
         with(nodes, 'data-user').
-        and_return([user.id])
+        and_return([user.id.to_s])
 
       hash = subject.grouped_objects_for_nodes(nodes, User, 'data-user')
 
-      expect(hash).to eq({ user.id => user })
+      expect(hash).to eq({ link => user })
     end
 
     it 'returns an empty Hash when the list of nodes is empty' do
diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
index 6873b7b85f9..f98fa00c418 100644
--- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
@@ -75,7 +75,7 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
       link['data-issue'] = issue.id.to_s
       nodes = [link]
 
-      expect(subject.issues_for_nodes(nodes)).to eq({ issue.id => issue })
+      expect(subject.issues_for_nodes(nodes)).to eq({ link => issue })
     end
   end
 end