Cleanup backend code

Removed EE::Banzai::Filter::LabelReferenceFilter and
handled all the EE-specific logic in the EE overrides
of LabelHelper

app/helpers/labels_helper.rb

@@ -42,52 +42,36 @@ module LabelsHelper
     if block_given?
       link_to link, class: 'gl-link gl-label-link', &block
     else
-      render_label(label, tooltip: tooltip, link: link, small: small)
+      render_label(label, link: link, tooltip: tooltip, small: small)
     end
   end
 
-  def render_label(label, tooltip: true, link: nil, dataset: nil, small: false, wrapper_class: nil, wrapper_style: nil, extra: nil)
-    # if scoped label is used then EE wraps label tag with scoped label
-    # doc link
+  def render_label(label, link: nil, tooltip: true, dataset: nil, small: false)
     html = render_colored_label(label)
 
     if link
-      html = link_to(
-        html,
-        link,
-        class: label_css_classes(tooltip),
-        data: label_dataset(label, dataset, tooltip)
-      )
+      title = label_tooltip_title(label) if tooltip
+      html = render_label_link(html, link: link, title: title, dataset: dataset)
     end
 
-    html += extra.html_safe if extra
-
-    wrapper_classes = Array.wrap(wrapper_class)
-    wrapper_classes << 'gl-label'
-    wrapper_classes << 'gl-label-sm' if small
-
-    content_tag(:span, html.html_safe, class: wrapper_classes.join(' '), style: wrapper_style)
+    wrap_label_html(html, small: small, label: label)
   end
 
-  def render_colored_label(label, label_suffix: '')
-    render_partial_label(
-      label,
-      label_suffix: label_suffix,
-      label_name: label.name,
+  def render_colored_label(label, suffix: '')
+    render_label_text(
+      label.name,
+      suffix: suffix,
       css_class: text_color_class_for_bg(label.color),
       bg_color: label.color
     )
   end
 
-  def render_partial_label(label, label_suffix: '', label_name: nil, css_class: nil, bg_color: nil)
-    # Intentionally not using content_tag here so that this method can be called
-    # by LabelReferenceFilter
-    span = %(<span class="gl-label-text #{css_class}" ) +
-      %(data-html="true" #{"style=\"background-color: #{bg_color}\"" if bg_color} ) +
-      %(data-container="body">) +
-      %(#{ERB::Util.html_escape_once(label_name)}#{label_suffix}</span>)
+  # We need the `label` argument here for EE
+  def wrap_label_html(label_html, small:, label:)
+    wrapper_classes = %w(gl-label)
+    wrapper_classes << 'gl-label-sm' if small
 
-    span.html_safe
+    %(<span class="#{wrapper_classes.join(' ')}">#{label_html}</span>).html_safe
   end
 
   def label_tooltip_title(label)
@@ -284,14 +268,27 @@ module LabelsHelper
 
   private
 
-  def label_dataset(label, dataset, tooltip)
+  def render_label_link(label_html, link:, title:, dataset:)
+    classes = %w(gl-link gl-label-link)
     dataset ||= {}
-    dataset.merge!(html: true, title: label_tooltip_title(label)) if tooltip
+
+    if title.present?
+      classes << 'has-tooltip'
+      dataset.merge!(html: true, title: title)
+    end
+
+    link_to(label_html, link, class: classes.join(' '), data: dataset)
   end
 
-  def label_css_classes(tooltip)
-    css = 'gl-link gl-label-link'
-    tooltip ? "#{css} has-tooltip" : css
+  def render_label_text(name, suffix: '', css_class: nil, bg_color: nil)
+    <<~HTML.chomp.html_safe
+      <span
+        class="gl-label-text #{css_class}"
+        data-container="body"
+        data-html="true"
+        #{"style=\"background-color: #{bg_color}\"" if bg_color}
+      >#{ERB::Util.html_escape_once(name)}#{suffix}</span>
+    HTML
   end
 end
 

ee/app/helpers/ee/labels_helper.rb

@@ -8,44 +8,32 @@ module EE
       singleton_class.prepend self
     end
 
-    def render_label(label, tooltip: true, link: nil, dataset: nil, small: false)
+    def render_colored_label(label, suffix: '')
       return super unless label.scoped_label?
 
-      scoped_label_wrapper(
-        super(
-          label,
-          tooltip: tooltip,
-          link: link,
-          dataset: dataset,
-          small: small,
-          wrapper_class: 'gl-label-scoped',
-          wrapper_style: "color: #{label.color}",
-          extra: scoped_labels_doc_link(label)
-        ),
-        label
-      )
-    end
-
-    def render_colored_label(label, label_suffix: '')
-      return super unless label.scoped_label?
-
-      text_color_class = text_color_class_for_bg(label.color)
       scope_name, label_name = label.name.split(Label::SCOPED_LABEL_SEPARATOR)
 
-      # Tooltip is omitted as it's attached to the link containing label title on scoped labels
-      render_partial_label(
-        label, label_suffix: label_suffix,
-        label_name: scope_name,
-        css_class: text_color_class,
+      render_label_text(
+        scope_name,
+        css_class: text_color_class_for_bg(label.color),
         bg_color: label.color
-      ) + render_partial_label(
-        label, label_suffix: label_suffix,
-        label_name: label_name
+      ) + render_label_text(
+        label_name,
+        suffix: suffix
       )
     end
 
-    def scoped_label_wrapper(link, label)
-      %(<span class="d-inline-block position-relative scoped-label-wrapper">#{link}</span>).html_safe
+    def wrap_label_html(label_html, small:, label:)
+      return super unless label.scoped_label?
+
+      wrapper_classes = %w(gl-label gl-label-scoped)
+      wrapper_classes << 'gl-label-sm' if small
+
+      <<~HTML.chomp.html_safe
+        <span class="d-inline-block position-relative scoped-label-wrapper">
+          <span class="#{wrapper_classes.join(' ')}" style="color: #{label.color}">#{label_html + scoped_labels_doc_link}</span>
+        </span>
+      HTML
     end
 
     def label_tooltip_title(label)
@@ -84,11 +72,12 @@ module EE
       super + ['epics']
     end
 
-    def scoped_labels_doc_link(label)
-      content = %(<i class="fa fa-question-circle"></i>)
-      help_url = ::Gitlab::Routing.url_helpers.help_page_url('user/project/labels.md', anchor: 'scoped-labels')
+    private
+
+    def scoped_labels_doc_link
+      help_url = ::Gitlab::Routing.url_helpers.help_page_path('user/project/labels.md', anchor: 'scoped-labels')
 
-      %(<a href="#{help_url}" class="gl-link gl-label-icon" target="_blank" rel="noopener">#{content}</a>)
+      %(<a href="#{help_url}" class="gl-link gl-label-icon" target="_blank" rel="noopener"><i class="fa fa-question-circle"></i></a>).html_safe
     end
   end
 end

ee/lib/ee/banzai/filter/label_reference_filter.rb

-# frozen_string_literal: true
-
-module EE
-  module Banzai
-    module Filter
-      module LabelReferenceFilter
-        extend ::Gitlab::Utils::Override
-
-        override :wrap_link
-        def wrap_link(link, label)
-          content = super
-          parent = project || group
-
-          if label.scoped_label? && parent && parent.feature_available?(:scoped_labels)
-            presenter = label.present(issuable_parent: parent)
-            doc_link = ::LabelsHelper.scoped_labels_doc_link(label)
-            content = %(<span class="gl-label gl-label-scoped gl-label-sm" style="color: #{label.color}">#{link}#{doc_link}</span>)
-            content = ::LabelsHelper.scoped_label_wrapper(content, presenter)
-          end
-
-          content
-        end
-
-        def tooltip_title(label)
-          ::LabelsHelper.label_tooltip_title(label)
-        end
-      end
-    end
-  end
-end

ee/spec/features/boards/sidebar_spec.rb

@@ -263,8 +263,8 @@ describe 'Issue Boards', :js do
     end
 
     it 'removes existing scoped label' do
-      scoped1 = scoped_label_1.title.split('::')
-      scoped2 = scoped_label_2.title.split('::')
+      scoped1 = scoped_label_1.title.split(Label::SCOPED_LABEL_SEPARATOR)
+      scoped2 = scoped_label_2.title.split(Label::SCOPED_LABEL_SEPARATOR)
 
       click_card(card1)
 

ee/spec/helpers/ee/labels_helper_spec.rb

@@ -14,12 +14,13 @@ describe LabelsHelper do
       end
 
       it 'includes link to scoped labels documentation' do
-        scoped = scoped_label.title.split('::')
-        expect(render_label(scoped_label)).to match(%r(<span.+><span.+><span.+>#{scoped.first}</span><span.+>#{scoped.last}</span><a.+>.*question-circle.*</a></span></span>$))
+        scope, name = scoped_label.title.split(Label::SCOPED_LABEL_SEPARATOR)
+
+        expect(render_label(scoped_label)).to match(%r(<span.+>#{scope}</span><span.+>#{name}</span><a.+>.*question-circle.*</a>)m)
       end
 
       it 'does not include link to scoped label documentation for common labels' do
-        expect(render_label(label)).to match(%r(<span.+><span.+>#{label.name}</span></span>$))
+        expect(render_label(label)).to match(%r(<span.+><span.+>#{label.name}</span></span>$)m)
       end
     end
 
@@ -29,7 +30,7 @@ describe LabelsHelper do
       end
 
       it 'does not include link to scoped documentation' do
-        expect(render_label(scoped_label)).to match(%r(<span.+><span.+>#{scoped_label.name}</span></span>$))
+        expect(render_label(scoped_label)).to match(%r(<span.+><span.+>#{scoped_label.name}</span></span>$)m)
       end
     end
   end

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

@@ -16,9 +16,9 @@ describe Banzai::Filter::LabelReferenceFilter do
 
     it 'includes link to scoped documentation' do
       doc = reference_filter("See #{scoped_label.to_reference}")
-      scoped = scoped_label.name.split('::')
+      scope, name = scoped_label.name.split(Label::SCOPED_LABEL_SEPARATOR)
 
-      expect(doc.to_html).to match(%r(<span.+><a.+><span.+>#{scoped.first}</span><span.+>#{scoped.last}</span></a><a.+>.*question-circle.*</a></span>))
+      expect(doc.to_html).to match(%r(<span.+><a.+><span.+>#{scope}</span><span.+>#{name}</span></a><a.+>.*question-circle.*</a></span>))
     end
 
     it 'does not include link to scoped documentation for common labels' do

lib/banzai/filter/label_reference_filter.rb

@@ -93,14 +93,12 @@ module Banzai
         end
 
         presenter = object.present(issuable_subject: parent)
-        LabelsHelper.render_colored_label(presenter, label_suffix: label_suffix)
+        LabelsHelper.render_colored_label(presenter, suffix: label_suffix)
       end
 
       def wrap_link(link, label)
-        content = super
-        content = %(<span class="gl-label gl-label-sm">#{content}</span>)
-
-        content
+        presenter = label.present(issuable_subject: project || group)
+        LabelsHelper.wrap_label_html(link, small: true, label: presenter)
       end
 
       def full_path_ref?(matches)
@@ -112,10 +110,9 @@ module Banzai
       end
 
       def object_link_title(object, matches)
-        LabelsHelper.label_tooltip_title(object)
+        presenter = object.present(issuable_subject: project || group)
+        LabelsHelper.label_tooltip_title(presenter)
       end
     end
   end
 end
-
-Banzai::Filter::LabelReferenceFilter.prepend_if_ee('EE::Banzai::Filter::LabelReferenceFilter')

lib/gitlab/markdown_cache.rb

@@ -3,7 +3,7 @@
 module Gitlab
   module MarkdownCache
     # Increment this number every time the renderer changes its output
-    CACHE_COMMONMARK_VERSION        = 18
+    CACHE_COMMONMARK_VERSION        = 19
     CACHE_COMMONMARK_VERSION_START  = 10
 
     BaseError = Class.new(StandardError)

spec/helpers/labels_helper_spec.rb

@@ -56,7 +56,7 @@ describe LabelsHelper do
 
     context 'without subject' do
       it "uses the label's project" do
-        expect(link_to_label(label_presenter)).to match %r{<a.*href="/#{label.project.full_path}/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}
+        expect(link_to_label(label_presenter)).to match %r{<a.*href="/#{label.project.full_path}/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}m
       end
     end
 
@@ -65,7 +65,7 @@ describe LabelsHelper do
       let(:subject) { build(:project, namespace: namespace, name: 'bar3') }
 
       it 'links to project issues page' do
-        expect(link_to_label(label_presenter)).to match %r{<a.*href="/foo3/bar3/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}
+        expect(link_to_label(label_presenter)).to match %r{<a.*href="/foo3/bar3/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}m
       end
     end
 
@@ -73,7 +73,7 @@ describe LabelsHelper do
       let(:subject) { build(:group, name: 'bar') }
 
       it 'links to group issues page' do
-        expect(link_to_label(label_presenter)).to match %r{<a.*href="/groups/bar/-/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}
+        expect(link_to_label(label_presenter)).to match %r{<a.*href="/groups/bar/-/issues\?label_name%5B%5D=#{label.name}".*>.*</a>}m
       end
     end
 
@@ -81,7 +81,7 @@ describe LabelsHelper do
       ['issue', :issue].each do |type|
         context "set to #{type}" do
           it 'links to correct page' do
-            expect(link_to_label(label_presenter, type: type)).to match %r{<a.*href="/#{label.project.full_path}/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}".*>.*</a>}
+            expect(link_to_label(label_presenter, type: type)).to match %r{<a.*href="/#{label.project.full_path}/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}".*>.*</a>}m
           end
         end
       end
@@ -89,7 +89,7 @@ describe LabelsHelper do
       ['merge_request', :merge_request].each do |type|
         context "set to #{type}" do
           it 'links to correct page' do
-            expect(link_to_label(label_presenter, type: type)).to match %r{<a.*href="/#{label.project.full_path}/-/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}".*>.*</a>}
+            expect(link_to_label(label_presenter, type: type)).to match %r{<a.*href="/#{label.project.full_path}/-/#{type.to_s.pluralize}\?label_name%5B%5D=#{label.name}".*>.*</a>}m
           end
         end
       end