Merge branch 'fix-label-reference-filter' into 'master'

Fix markdown rendering for label references

Fixes #14424, #19753, #19760

See merge request !5224

CHANGELOG

@@ -90,6 +90,7 @@ v 8.10.0 (unreleased)
   - Optimistic locking for Issues and Merge Requests (Title and description overriding prevention)
   - Redesign Builds and Pipelines pages
   - Change status color and icon for running builds
+  - Fix markdown rendering for: consecutive labels references, label references that begin with a digit or contains `.`
 
 v 8.9.6
   - Fix importing of events under notes for GitLab projects. !5154

app/models/label.rb

@@ -52,14 +52,17 @@ class Label < ActiveRecord::Base
   # This pattern supports cross-project references.
   #
   def self.reference_pattern
+    # NOTE: The id pattern only matches when all characters on the expression
+    # are digits, so it will match ~2 but not ~2fa because that's probably a
+    # label name and we want it to be matched as such.
     @reference_pattern ||= %r{
       (#{Project.reference_pattern})?
       #{Regexp.escape(reference_prefix)}
       (?:
-        (?<label_id>\d+) | # Integer-based label ID, or
+        (?<label_id>\d+(?!\S\w)\b) | # Integer-based label ID, or
         (?<label_name>
-          [A-Za-z0-9_\-\?&]+ | # String-based single-word label title, or
-          "[^,]+"              # String-based multi-word label surrounded in quotes
+          [A-Za-z0-9_\-\?\.&]+ | # String-based single-word label title, or
+          ".+?"                  # String-based multi-word label surrounded in quotes
         )
       )
     }x

spec/lib/banzai/filter/label_reference_filter_spec.rb

@@ -93,8 +93,8 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
     end
 
     it 'links with adjacent text' do
-      doc = reference_filter("Label (#{reference}.)")
-      expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\)))
+      doc = reference_filter("Label (#{reference}).")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\)\.))
     end
 
     it 'ignores invalid label names' do
@@ -104,8 +104,32 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
     end
   end
 
+  context 'String-based single-word references that begin with a digit' do
+    let(:label)     { create(:label, name: '2fa', project: project) }
+    let(:reference) { "#{Label.reference_prefix}#{label.name}" }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_issues_url(project.namespace, project, label_name: label.name)
+      expect(doc.text).to eq 'See 2fa'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Label (#{reference}).")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\)\.))
+    end
+
+    it 'ignores invalid label names' do
+      exp = act = "Label #{Label.reference_prefix}#{label.id}#{label.name.reverse}"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
   context 'String-based single-word references with special characters' do
-    let(:label)     { create(:label, name: '?gfm&', project: project) }
+    let(:label)     { create(:label, name: '?g.fm&', project: project) }
     let(:reference) { "#{Label.reference_prefix}#{label.name}" }
 
     it 'links to a valid reference' do
@@ -113,17 +137,17 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
 
       expect(doc.css('a').first.attr('href')).to eq urls.
         namespace_project_issues_url(project.namespace, project, label_name: label.name)
-      expect(doc.text).to eq 'See ?gfm&'
+      expect(doc.text).to eq 'See ?g.fm&'
     end
 
     it 'links with adjacent text' do
-      doc = reference_filter("Label (#{reference}.)")
-      expect(doc.to_html).to match(%r(\(<a.+><span.+>\?gfm&amp;</span></a>\.\)))
+      doc = reference_filter("Label (#{reference}).")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>\?g\.fm&amp;</span></a>\)\.))
     end
 
     it 'ignores invalid label names' do
       act = "Label #{Label.reference_prefix}#{label.name.reverse}"
-      exp = "Label #{Label.reference_prefix}&amp;mfg?"
+      exp = "Label #{Label.reference_prefix}&amp;mf.g?"
 
       expect(reference_filter(act).to_html).to eq exp
     end
@@ -153,8 +177,32 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
     end
   end
 
+  context 'String-based multi-word references that begin with a digit' do
+    let(:label)     { create(:label, name: '2 factor authentication', project: project) }
+    let(:reference) { label.to_reference(format: :name) }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_issues_url(project.namespace, project, label_name: label.name)
+      expect(doc.text).to eq 'See 2 factor authentication'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Label (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>#{label.name}</span></a>\.\)))
+    end
+
+    it 'ignores invalid label names' do
+      exp = act = "Label #{Label.reference_prefix}#{label.id}#{label.name.reverse}"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
   context 'String-based multi-word references with special characters in quotes' do
-    let(:label)     { create(:label, name: 'gfm & references?', project: project) }
+    let(:label)     { create(:label, name: 'g.fm & references?', project: project) }
     let(:reference) { label.to_reference(format: :name) }
 
     it 'links to a valid reference' do
@@ -162,22 +210,62 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
 
       expect(doc.css('a').first.attr('href')).to eq urls.
         namespace_project_issues_url(project.namespace, project, label_name: label.name)
-      expect(doc.text).to eq 'See gfm & references?'
+      expect(doc.text).to eq 'See g.fm & references?'
     end
 
     it 'links with adjacent text' do
       doc = reference_filter("Label (#{reference}.)")
-      expect(doc.to_html).to match(%r(\(<a.+><span.+>gfm &amp; references\?</span></a>\.\)))
+      expect(doc.to_html).to match(%r(\(<a.+><span.+>g\.fm &amp; references\?</span></a>\.\)))
     end
 
     it 'ignores invalid label names' do
       act = %(Label #{Label.reference_prefix}"#{label.name.reverse}")
-      exp = %(Label #{Label.reference_prefix}"?secnerefer &amp; mfg\")
+      exp = %(Label #{Label.reference_prefix}"?secnerefer &amp; mf.g\")
 
       expect(reference_filter(act).to_html).to eq exp
     end
   end
 
+  describe 'consecutive references' do
+    let(:bug) { create(:label, name: 'bug', project: project) }
+    let(:feature_proposal) { create(:label, name: 'feature proposal', project: project) }
+    let(:technical_debt) { create(:label, name: 'technical debt', project: project) }
+
+    let(:bug_reference) { "#{Label.reference_prefix}#{bug.name}" }
+    let(:feature_proposal_reference) { feature_proposal.to_reference(format: :name) }
+    let(:technical_debt_reference) { technical_debt.to_reference(format: :name) }
+
+    context 'separated with a comma' do
+      let(:references) { "#{bug_reference}, #{feature_proposal_reference}, #{technical_debt_reference}" }
+
+      it 'links to valid references' do
+        doc = reference_filter("See #{references}")
+
+        expect(doc.css('a').map { |a| a.attr('href') }).to match_array([
+          urls.namespace_project_issues_url(project.namespace, project, label_name: bug.name),
+          urls.namespace_project_issues_url(project.namespace, project, label_name: feature_proposal.name),
+          urls.namespace_project_issues_url(project.namespace, project, label_name: technical_debt.name)
+        ])
+        expect(doc.text).to eq 'See bug, feature proposal, technical debt'
+      end
+    end
+
+    context 'separated with a space' do
+      let(:references) { "#{bug_reference} #{feature_proposal_reference} #{technical_debt_reference}" }
+
+      it 'links to valid references' do
+        doc = reference_filter("See #{references}")
+
+        expect(doc.css('a').map { |a| a.attr('href') }).to match_array([
+          urls.namespace_project_issues_url(project.namespace, project, label_name: bug.name),
+          urls.namespace_project_issues_url(project.namespace, project, label_name: feature_proposal.name),
+          urls.namespace_project_issues_url(project.namespace, project, label_name: technical_debt.name)
+        ])
+        expect(doc.text).to eq 'See bug feature proposal technical debt'
+      end
+    end
+  end
+
   describe 'edge cases' do
     it 'gracefully handles non-references matching the pattern' do
       exp = act = '(format nil "~0f" 3.0) ; 3.0'