Move TimelineEventPipeline into IncidentManagement namespace

* Move TimelineEventPipeline into IncidentManagement namespace
* Cover additional scenarios in tests

ee/app/models/incident_management/timeline_event.rb

@@ -6,7 +6,7 @@ module IncidentManagement
 
     self.table_name = 'incident_management_timeline_events'
 
-    cache_markdown_field :note, pipeline: :timeline_event, issuable_reference_expansion_enabled: true
+    cache_markdown_field :note, pipeline: :'incident_management/timeline_event', issuable_reference_expansion_enabled: true
 
     belongs_to :project
     belongs_to :author, class_name: 'User', foreign_key: :author_id

ee/lib/banzai/pipeline/incident_management/timeline_event_pipeline.rb

+# frozen_string_literal: true
+
+module Banzai
+  module Pipeline
+    module IncidentManagement
+      class TimelineEventPipeline < PlainMarkdownPipeline
+        ALLOWLIST = Banzai::Filter::SanitizationFilter::LIMITED.deep_dup.merge(
+          elements: %w(p b i strong em pre code a img)
+        ).freeze
+
+        def self.filters
+          @filters ||= FilterArray[
+            *super,
+            *Banzai::Pipeline::GfmPipeline.reference_filters,
+            Filter::EmojiFilter,
+            Filter::ExternalLinkFilter,
+            Filter::ImageLinkFilter,
+            Filter::SanitizationFilter
+          ]
+        end
+
+        def self.transform_context(context)
+          Filter::AssetProxyFilter.transform_context(context).merge(
+            only_path: true,
+            no_sourcepos: true,
+            allowlist: ALLOWLIST,
+            link_replaces_image: true
+          )
+        end
+      end
+    end
+  end
+end

ee/lib/banzai/pipeline/timeline_event_pipeline.rb

-# frozen_string_literal: true
-
-module Banzai
-  module Pipeline
-    class TimelineEventPipeline < PlainMarkdownPipeline
-      ALLOWLIST = Banzai::Filter::SanitizationFilter::LIMITED.deep_dup.merge(
-        elements: %w(p b i strong em pre code a img)
-      )
-
-      def self.filters
-        @filters ||= FilterArray[
-          *super,
-          *reference_filters,
-          Filter::EmojiFilter,
-          Filter::ExternalLinkFilter,
-          Filter::ImageLinkFilter,
-          Filter::SanitizationFilter
-        ]
-      end
-
-      def self.reference_filters
-        Banzai::Pipeline::GfmPipeline.reference_filters
-      end
-
-      def self.transform_context(context)
-        Filter::AssetProxyFilter.transform_context(context).merge(
-          only_path: true,
-          no_sourcepos: true,
-          allowlist: ALLOWLIST,
-          link_replaces_image: true
-        )
-      end
-    end
-  end
-end

ee/spec/lib/banzai/pipeline/timeline_event_pipeline_spec.rb → ee/spec/lib/banzai/pipeline/incident_management/timeline_event_pipeline_spec.rb

@@ -2,13 +2,20 @@
 
 require 'spec_helper'
 
-RSpec.describe Banzai::Pipeline::TimelineEventPipeline do
+RSpec.describe Banzai::Pipeline::IncidentManagement::TimelineEventPipeline do
   let_it_be(:project) { create(:project) }
 
-  describe '.reference_filters' do
-    it 'contains required reference filters' do
-      expect(described_class.reference_filters).to contain_exactly(
-        *Banzai::Pipeline::GfmPipeline.reference_filters
+  describe '.filters' do
+    it 'contains required filters' do
+      expect(described_class.filters).to eq(
+        [
+          *Banzai::Pipeline::PlainMarkdownPipeline.filters,
+          *Banzai::Pipeline::GfmPipeline.reference_filters,
+          Banzai::Filter::EmojiFilter,
+          Banzai::Filter::ExternalLinkFilter,
+          Banzai::Filter::ImageLinkFilter,
+          Banzai::Filter::SanitizationFilter
+        ]
       )
     end
   end
@@ -17,13 +24,13 @@ RSpec.describe Banzai::Pipeline::TimelineEventPipeline do
     subject(:output) { described_class.to_html(markdown, project: project) }
 
     context 'when markdown contains font style transformations' do
-      let(:markdown) { '**bold** _italic_ `code`'}
+      let(:markdown) { '**bold** _italic_ `code`' }
 
       it { is_expected.to eq('<p><strong>bold</strong> <em>italic</em> <code>code</code></p>') }
     end
 
     context 'when markdown contains banned HTML tags' do
-      let(:markdown) { '<div>div</div><h1>h1</h1>'}
+      let(:markdown) { '<div>div</div><h1>h1</h1>' }
 
       it 'filters out banned tags' do
         is_expected.to eq(' div  h1 ')
@@ -40,12 +47,12 @@ RSpec.describe Banzai::Pipeline::TimelineEventPipeline do
       let(:markdown) { '![Name](/path/to/image.png)' }
 
       it 'replaces image with a link to the image' do
-        is_expected.to eq(%q{<p><a class="with-attachment-icon" href="/path/to/image.png" target="_blank">Name</a></p>})
+        is_expected.to eq(%q(<p><a class="with-attachment-icon" href="/path/to/image.png" target="_blank">Name</a></p>))
       end
     end
 
     context 'when markdown contains emojis' do
-      let(:markdown) { ':+1:👍'}
+      let(:markdown) { ':+1:👍' }
 
       it { is_expected.to eq('<p>👍👍</p>') }
     end
@@ -55,7 +62,7 @@ RSpec.describe Banzai::Pipeline::TimelineEventPipeline do
       let(:markdown) { "issue ##{issue.iid}" }
 
       it 'contains a link to the issue' do
-        is_expected.to match(%r(<p>issue <a\shref="[\w\/]+-\/issues\/#{issue.iid}".*>##{issue.iid}<\/a><\/p>))
+        is_expected.to match(%r(<p>issue <a href="[\w/]+-/issues/#{issue.iid}".*>##{issue.iid}</a></p>))
       end
     end
 
@@ -64,7 +71,7 @@ RSpec.describe Banzai::Pipeline::TimelineEventPipeline do
       let(:markdown) { "MR !#{mr.iid}" }
 
       it 'contains a link to the merge request' do
-        is_expected.to match(%r(<p>MR <a\shref="[\w\/]+-\/merge_requests\/#{mr.iid}".*>!#{mr.iid}<\/a><\/p>))
+        is_expected.to match(%r(<p>MR <a href="[\w/]+-/merge_requests/#{mr.iid}".*>!#{mr.iid}</a></p>))
       end
     end
   end

lib/banzai/filter/image_link_filter.rb

@@ -9,7 +9,7 @@ module Banzai
       # a new node (a link to the image source), copy the image as a child
       # of the anchor, and then replace the img with the link-wrapped version.
       #
-      # If `link_replaces_image` context parameter provided, the image is going
+      # If `link_replaces_image` context parameter is provided, the image is going
       # to be replaced with a link to an image.
       def call
         doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |img|

spec/lib/banzai/filter/image_link_filter_spec.rb

@@ -8,29 +8,34 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do
   let(:path) { '/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg' }
   let(:context) { {} }
 
-  def image(path, alt: nil)
+  def image(path, alt: nil, data_src: nil)
     alt_tag = alt ? %Q{alt="#{alt}"} : ""
+    data_src_tag = data_src ? %Q{data-src="#{data_src}"} : ""
 
-    %(<img src="#{path}" #{alt_tag} />)
+    %(<img src="#{path}" #{alt_tag} #{data_src_tag} />)
   end
 
   it 'wraps the image with a link to the image src' do
     doc = filter(image(path), context)
+
     expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href']
   end
 
   it 'does not wrap a duplicate link' do
     doc = filter(%Q(<a href="/whatever">#{image(path)}</a>), context)
+
     expect(doc.to_html).to match %r{^<a href="/whatever"><img[^>]*></a>$}
   end
 
   it 'works with external images' do
     doc = filter(image('https://i.imgur.com/DfssX9C.jpg'), context)
+
     expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href']
   end
 
   it 'works with inline images' do
     doc = filter(%Q(<p>test #{image(path)} inline</p>), context)
+
     expect(doc.to_html).to match %r{^<p>test <a[^>]*><img[^>]*></a> inline</p>$}
   end
 
@@ -41,11 +46,18 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do
     expect(doc.at_css('img')['data-canonical-src']).to eq doc.at_css('a')['data-canonical-src']
   end
 
+  it 'adds no-attachment icon class to the link' do
+    doc = filter(image(path), context)
+
+    expect(doc.at_css('a')['class']).to match(%r{no-attachment-icon})
+  end
+
   context 'when :link_replaces_image is true' do
     let(:context) { { link_replaces_image: true } }
 
     it 'replaces the image with link to image src', :aggregate_failures do
       doc = filter(image(path), context)
+
       expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$})
       expect(doc.at_css('a')['href']).to eq(path)
     end
@@ -57,6 +69,14 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do
       expect(doc.at_css('a')['href']).to eq(path)
     end
 
+    it 'uses image data-src as a link text', :aggregate_failures do
+      data_src = '/uploads/data-src.png'
+      doc = filter(image(path, data_src: data_src), context)
+
+      expect(doc.to_html).to match(%r{^<a[^>]*>#{data_src}</a>$})
+      expect(doc.at_css('a')['href']).to eq(data_src)
+    end
+
     it 'adds attachment icon class to the link' do
       doc = filter(image(path), context)