Merge branch 'eReGeBe/gitlab-ce-feature/milestone-md' into 'master'

Implement special GitLab markdown reference for milestones

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3453 by @eReGeBe, with two additions:

- Move changelog item to 8.8
- Fix cross-project milestone ref with invalid project, like https://gitlab.com/gitlab-org/gitlab-ce/commit/f7348cd348ad8f4a18d74dd668283a4e236f5790 did for labels

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/13829

See merge request !3897

4 files changed, 144 insertions, 25 deletions

diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb
index 1772cc3f6a4..3e777a5e92b 100644
--- a/spec/fixtures/markdown.md.erb
+++ b/spec/fixtures/markdown.md.erb
@@ -216,10 +216,14 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
 
 #### MilestoneReferenceFilter
 
-- Milestone: <%= milestone.to_reference %>
+- Milestone by ID: <%= simple_milestone.to_reference %>
+- Milestone by name: <%= Milestone.reference_prefix %><%= simple_milestone.name %>
+- Milestone by name in quotes: <%= milestone.to_reference(format: :name) %>
 - Milestone in another project: <%= xmilestone.to_reference(project) %>
-- Ignored in code: `<%= milestone.to_reference %>`
-- Link to milestone by URL: [Milestone](<%= urls.namespace_project_milestone_url(milestone.project.namespace, milestone.project, milestone) %>)
+- Ignored in code: `<%= simple_milestone.to_reference %>`
+- Ignored in links: [Link to <%= simple_milestone.to_reference %>](#milestone-link)
+- Milestone by URL: <%= urls.namespace_project_milestone_url(milestone.project.namespace, milestone.project, milestone) %>
+- Link to milestone by URL: [Milestone](<%= milestone.to_reference %>)
 
 ### Task Lists
 
diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb
index 5beb61dac5c..bdf48eabb0e 100644
--- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb
@@ -3,8 +3,9 @@ require 'spec_helper'
 describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
   include FilterSpecHelper
 
-  let(:project) { create(:project, :public) }
-  let(:milestone)   { create(:milestone, project: project) }
+  let(:project)   { create(:project, :public) }
+  let(:milestone) { create(:milestone, project: project) }
+  let(:reference) { milestone.to_reference }
 
   it 'requires project context' do
     expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
@@ -17,11 +18,42 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
   end
 
-  context 'internal reference' do
-    # Convert the Markdown link to only the URL, since these tests aren't run through the regular Markdown pipeline.
-    # Milestone reference behavior in the full Markdown pipeline is tested elsewhere.
-    let(:reference) { milestone.to_reference.gsub(/\[([^\]]+)\]\(([^)]+)\)/, '\2') }
+  it 'includes default classes' do
+    doc = reference_filter("Milestone #{reference}")
+    expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
+  end
+
+  it 'includes a data-project attribute' do
+    doc = reference_filter("Milestone #{reference}")
+    link = doc.css('a').first
+
+    expect(link).to have_attribute('data-project')
+    expect(link.attr('data-project')).to eq project.id.to_s
+  end
+
+  it 'includes a data-milestone attribute' do
+    doc = reference_filter("See #{reference}")
+    link = doc.css('a').first
+
+    expect(link).to have_attribute('data-milestone')
+    expect(link.attr('data-milestone')).to eq milestone.id.to_s
+  end
+
+  it 'supports an :only_path context' do
+    doc = reference_filter("Milestone #{reference}", only_path: true)
+    link = doc.css('a').first.attr('href')
 
+    expect(link).not_to match %r(https?://)
+    expect(link).to eq urls.
+      namespace_project_milestone_path(project.namespace, project, milestone)
+  end
+
+  it 'adds to the results hash' do
+    result = reference_pipeline_result("Milestone #{reference}")
+    expect(result[:references][:milestone]).to eq [milestone]
+  end
+
+  context 'Integer-based references' do
     it 'links to a valid reference' do
       doc = reference_filter("See #{reference}")
 
@@ -30,29 +62,82 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
 
     it 'links with adjacent text' do
-      doc = reference_filter("milestone (#{reference}.)")
-      expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(milestone.title)}<\/a>\.\)/)
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
     end
 
-    it 'includes a title attribute' do
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.css('a').first.attr('title')).to eq "Milestone: #{milestone.title}"
+    it 'ignores invalid milestone IIDs' do
+      exp = act = "Milestone #{invalidate_reference(reference)}"
+
+      expect(reference_filter(act).to_html).to eq exp
     end
+  end
+
+  context 'String-based single-word references' do
+    let(:milestone) { create(:milestone, name: 'gfm', project: project) }
+    let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
 
-    it 'escapes the title attribute' do
-      milestone.update_attribute(:title, %{"></a>whatever<a title="})
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(project.namespace, project, milestone)
+      expect(doc.text).to eq 'See gfm'
+    end
 
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.text).to eq "milestone \">whatever"
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
     end
 
-    it 'includes default classes' do
-      doc = reference_filter("milestone #{reference}")
-      expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
+    it 'ignores invalid milestone names' do
+      exp = act = "Milestone #{Milestone.reference_prefix}#{milestone.name.reverse}"
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  context 'String-based multi-word references in quotes' do
+    let(:milestone) { create(:milestone, name: 'gfm references', project: project) }
+    let(:reference) { milestone.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_milestone_url(project.namespace, project, milestone)
+      expect(doc.text).to eq 'See gfm references'
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
+    end
+
+    it 'ignores invalid milestone names' do
+      exp = act = %(Milestone #{Milestone.reference_prefix}"#{milestone.name.reverse}")
+
+      expect(reference_filter(act).to_html).to eq exp
+    end
+  end
+
+  describe 'referencing a milestone in a link href' do
+    let(:reference) { %Q{<a href="#{milestone.to_reference}">Milestone</a>} }
+
+    it 'links to a valid reference' do
+      doc = reference_filter("See #{reference}")
+
+      expect(doc.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(project.namespace, project, milestone)
+    end
+
+    it 'links with adjacent text' do
+      doc = reference_filter("Milestone (#{reference}.)")
+      expect(doc.to_html).to match(%r(\(<a.+>Milestone</a>\.\)))
     end
 
     it 'includes a data-project attribute' do
-      doc = reference_filter("milestone #{reference}")
+      doc = reference_filter("Milestone #{reference}")
       link = doc.css('a').first
 
       expect(link).to have_attribute('data-project')
@@ -68,8 +153,34 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
     end
 
     it 'adds to the results hash' do
-      result = reference_pipeline_result("milestone #{reference}")
+      result = reference_pipeline_result("Milestone #{reference}")
       expect(result[:references][:milestone]).to eq [milestone]
     end
   end
+
+  describe 'cross project milestone references' do
+    let(:another_project)  { create(:empty_project, :public) }
+    let(:project_path) { another_project.path_with_namespace }
+    let(:milestone) { create(:milestone, project: another_project) }
+    let(:reference) { milestone.to_reference(project) }
+
+    let!(:result) { reference_filter("See #{reference}") }
+
+    it 'points to referenced project milestone page' do
+      expect(result.css('a').first.attr('href')).to eq urls.
+        namespace_project_milestone_url(another_project.namespace,
+                                        another_project,
+                                        milestone)
+    end
+
+    it 'contains cross project content' do
+      expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_path}"
+    end
+
+    it 'escapes the name attribute' do
+      allow_any_instance_of(Milestone).to receive(:title).and_return(%{"></a>whatever<a title="})
+      doc = reference_filter("See #{reference}")
+      expect(doc.css('a').first.text).to eq "#{milestone.name} in #{project_path}"
+    end
+  end
 end
diff --git a/spec/support/markdown_feature.rb b/spec/support/markdown_feature.rb
index b87cd6bbca2..7fc6d6fcc5e 100644
--- a/spec/support/markdown_feature.rb
+++ b/spec/support/markdown_feature.rb
@@ -63,8 +63,12 @@ class MarkdownFeature
     @label ||= create(:label, name: 'awaiting feedback', project: project)
   end
 
+  def simple_milestone
+    @simple_milestone ||= create(:milestone, name: 'gfm-milestone', project: project)
+  end
+
   def milestone
-    @milestone ||= create(:milestone, project: project)
+    @milestone ||= create(:milestone, name: 'next goal', project: project)
   end
 
   # Cross-references -----------------------------------------------------------
diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb
index 43cb6ef43f2..d921f9bb2bc 100644
--- a/spec/support/matchers/markdown_matchers.rb
+++ b/spec/support/matchers/markdown_matchers.rb
@@ -154,7 +154,7 @@ module MarkdownMatchers
     set_default_markdown_messages
 
     match do |actual|
-      expect(actual).to have_selector('a.gfm.gfm-milestone', count: 3)
+      expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6)
     end
   end