diff options
author | Robert Speicher <robert@gitlab.com> | 2016-07-13 19:59:55 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-07-13 19:59:55 +0000 |
commit | 7968484dfa363537e6e7822ca1ec100bcd0ec4f8 (patch) | |
tree | 5a956fda0531940e2a6de08cfba28febd362c108 | |
parent | 997e2d1a65e738253b327f1b21389c42ca5a3e5f (diff) | |
parent | a4147af674e851d50c71e982475f04195d1a3015 (diff) | |
download | gitlab-ce-7968484dfa363537e6e7822ca1ec100bcd0ec4f8.tar.gz |
Merge branch 'fix-label-reference-filter' into 'master'
Fix markdown rendering for label references
Fixes #14424, #19753, #19760
See merge request !5224
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/label.rb | 9 | ||||
-rw-r--r-- | spec/lib/banzai/filter/label_reference_filter_spec.rb | 110 |
3 files changed, 106 insertions, 14 deletions
diff --git a/CHANGELOG b/CHANGELOG index a965c51498c..19286abd74f 100644 --- a/CHANGELOG +++ b/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 diff --git a/app/models/label.rb b/app/models/label.rb index dc5586f5756..35e678001dc 100644 --- a/app/models/label.rb +++ b/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 diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index 9e3d2f5825d..9276a154007 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/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&</span></a>\.\))) + doc = reference_filter("Label (#{reference}).") + expect(doc.to_html).to match(%r(\(<a.+><span.+>\?g\.fm&</span></a>\)\.)) end it 'ignores invalid label names' do act = "Label #{Label.reference_prefix}#{label.name.reverse}" - exp = "Label #{Label.reference_prefix}&mfg?" + exp = "Label #{Label.reference_prefix}&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 & references\?</span></a>\.\))) + expect(doc.to_html).to match(%r(\(<a.+><span.+>g\.fm & references\?</span></a>\.\))) end it 'ignores invalid label names' do act = %(Label #{Label.reference_prefix}"#{label.name.reverse}") - exp = %(Label #{Label.reference_prefix}"?secnerefer & mfg\") + exp = %(Label #{Label.reference_prefix}"?secnerefer & 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' |