Commit 149528f4 authored by Sean McGivern's avatar Sean McGivern

Support references to group milestones

Group milestones can only be referred to by name, not IID. They also do not
support cross-project references.
parent 03b816f3
......@@ -47,14 +47,6 @@ module GitlabRoutingHelper
project_pipeline_path(pipeline.project, pipeline.id, *args)
end
def milestone_path(entity, *args)
if entity.is_group_milestone?
group_milestone_path(entity.group, entity, *args)
elsif entity.is_project_milestone?
project_milestone_path(entity.project, entity, *args)
end
end
def issue_url(entity, *args)
project_issue_url(entity.project, entity, *args)
end
......@@ -67,14 +59,6 @@ module GitlabRoutingHelper
project_pipeline_url(pipeline.project, pipeline.id, *args)
end
def milestone_url(entity, *args)
if entity.is_group_milestone?
group_milestone_url(entity.group, entity, *args)
elsif entity.is_project_milestone?
project_milestone_url(entity.project, entity, *args)
end
end
def pipeline_job_url(pipeline, build, *args)
project_job_url(pipeline.project, build.id, *args)
end
......
module MilestonesRoutingHelper
def milestone_path(milestone, *args)
if milestone.is_group_milestone?
group_milestone_path(milestone.group, milestone, *args)
elsif milestone.is_project_milestone?
project_milestone_path(milestone.project, milestone, *args)
end
end
def milestone_url(milestone, *args)
if milestone.is_group_milestone?
group_milestone_url(milestone.group, milestone, *args)
elsif milestone.is_project_milestone?
project_milestone_url(milestone.project, milestone, *args)
end
end
end
......@@ -149,7 +149,9 @@ class Milestone < ActiveRecord::Base
end
##
# Returns the String necessary to reference this Milestone in Markdown
# Returns the String necessary to reference this Milestone in Markdown. Group
# milestones only support name references, and do not support cross-project
# references.
#
# format - Symbol format to use (default: :iid, optional: :name)
#
......@@ -161,12 +163,16 @@ class Milestone < ActiveRecord::Base
# Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1"
#
def to_reference(from_project = nil, format: :iid, full: false)
return if is_group_milestone?
return if is_group_milestone? && format != :name
format_reference = milestone_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}"
if project
"#{project.to_reference(from_project, full: full)}#{reference}"
else
reference
end
end
def reference_link_text(from_project = nil)
......
......@@ -181,7 +181,11 @@ module Gitlab
end
end
# We add the MilestonesRoutingHelper because we know that this does not
# conflict with the methods defined in `project_url_helpers`, and we want
# these methods available in the same places.
Gitlab::Routing.add_helpers(project_url_helpers)
Gitlab::Routing.add_helpers(MilestonesRoutingHelper)
end
end
end
......@@ -248,7 +248,7 @@ GFM will recognize the following:
| `~123` | label by ID |
| `~bug` | one-word label by name |
| `~"feature request"` | multi-word label by name |
| `%123` | milestone by ID |
| `%123` | project milestone by ID |
| `%v1.23` | one-word milestone by name |
| `%"release candidate"` | multi-word milestone by name |
| `9ba12248` | specific commit |
......@@ -262,7 +262,7 @@ GFM also recognizes certain cross-project references:
|:----------------------------------------|:------------------------|
| `namespace/project#123` | issue |
| `namespace/project!123` | merge request |
| `namespace/project%123` | milestone |
| `namespace/project%123` | project milestone |
| `namespace/project$123` | snippet |
| `namespace/project@9ba12248` | specific commit |
| `namespace/project@9ba12248...b19a04f5` | commit range comparison |
......@@ -274,7 +274,7 @@ It also has a shorthand version to reference other projects from the same namesp
|:------------------------------|:------------------------|
| `project#123` | issue |
| `project!123` | merge request |
| `project%123` | milestone |
| `project%123` | project milestone |
| `project$123` | snippet |
| `project@9ba12248` | specific commit |
| `project@9ba12248...b19a04f5` | commit range comparison |
......
......@@ -59,6 +59,12 @@ module Banzai
# Example: project.merge_requests.find
end
# Override if the link reference pattern produces a different ID (global
# ID vs internal ID, for instance) to the regular reference pattern.
def find_object_from_link(project, id)
find_object(project, id)
end
def find_object_cached(project, id)
if RequestStore.active?
cache = find_objects_cache[object_class][project.id]
......@@ -69,6 +75,16 @@ module Banzai
end
end
def find_object_from_link_cached(project, id)
if RequestStore.active?
cache = find_objects_from_link_cache[object_class][project.id]
get_or_set_cache(cache, id) { find_object_from_link(project, id) }
else
find_object_from_link(project, id)
end
end
def project_from_ref_cached(ref)
if RequestStore.active?
cache = project_refs_cache
......@@ -120,7 +136,7 @@ module Banzai
if link == inner_html && inner_html =~ /\A#{link_pattern}/
replace_link_node_with_text(node, link) do
object_link_filter(inner_html, link_pattern)
object_link_filter(inner_html, link_pattern, link_reference: true)
end
next
......@@ -128,7 +144,7 @@ module Banzai
if link =~ /\A#{link_pattern}\z/
replace_link_node_with_href(node, link) do
object_link_filter(link, link_pattern, link_content: inner_html)
object_link_filter(link, link_pattern, link_content: inner_html, link_reference: true)
end
next
......@@ -146,15 +162,26 @@ module Banzai
# text - String text to replace references in.
# pattern - Reference pattern to match against.
# link_content - Original content of the link being replaced.
# link_reference - True if this was using the link reference pattern,
# false otherwise.
#
# Returns a String with references replaced with links. All links
# have `gfm` and `gfm-OBJECT_NAME` class names attached for styling.
def object_link_filter(text, pattern, link_content: nil)
def object_link_filter(text, pattern, link_content: nil, link_reference: false)
references_in(text, pattern) do |match, id, project_ref, namespace_ref, matches|
project_path = full_project_path(namespace_ref, project_ref)
project = project_from_ref_cached(project_path)
if project && object = find_object_cached(project, id)
if project
object =
if link_reference
find_object_from_link_cached(project, id)
else
find_object_cached(project, id)
end
end
if object
title = object_link_title(object)
klass = reference_class(object_sym)
......@@ -303,6 +330,12 @@ module Banzai
end
end
def find_objects_from_link_cache
RequestStore[:banzai_find_objects_from_link_cache] ||= Hash.new do |hash, key|
hash[key] = Hash.new { |h, k| h[k] = {} }
end
end
def url_for_object_cache
RequestStore[:banzai_url_for_object] ||= Hash.new do |hash, key|
hash[key] = Hash.new { |h, k| h[k] = {} }
......
......@@ -8,8 +8,15 @@ module Banzai
Milestone
end
# Links to project milestones contain the IID, but when we're handling
# 'regular' references, we need to use the global ID to disambiguate
# between group and project milestones.
def find_object(project, id)
project.milestones.find_by(iid: id)
find_milestone_with_finder(project, id: id)
end
def find_object_from_link(project, iid)
find_milestone_with_finder(project, iid: iid)
end
def references_in(text, pattern = Milestone.reference_pattern)
......@@ -22,7 +29,7 @@ module Banzai
milestone = find_milestone($~[:project], $~[:namespace], $~[:milestone_iid], $~[:milestone_name])
if milestone
yield match, milestone.iid, $~[:project], $~[:namespace], $~
yield match, milestone.id, $~[:project], $~[:namespace], $~
else
match
end
......@@ -36,7 +43,8 @@ module Banzai
return unless project
milestone_params = milestone_params(milestone_id, milestone_name)
project.milestones.find_by(milestone_params)
find_milestone_with_finder(project, milestone_params)
end
def milestone_params(iid, name)
......@@ -47,15 +55,27 @@ module Banzai
end
end
def find_milestone_with_finder(project, params)
finder_params = { project_ids: [project.id], order: nil }
# We don't support IID lookups for group milestones, because IIDs can
# clash between group and project milestones.
if project.group && !params[:iid]
finder_params[:group_ids] = [project.group.id]
end
MilestonesFinder.new(finder_params).execute.find_by(params)
end
def url_for_object(milestone, project)
h = Gitlab::Routing.url_helpers
h.project_milestone_url(project, milestone,
only_path: context[:only_path])
Gitlab::Routing
.url_helpers
.milestone_url(milestone, only_path: context[:only_path])
end
def object_link_text(object, matches)
milestone_link = escape_once(super)
reference = object.project.to_reference(project)
reference = object.project&.to_reference(project)
if reference.present?
"#{milestone_link} <i>in #{reference}</i>".html_safe
......
......@@ -227,8 +227,11 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
- Milestone in another project: <%= xmilestone.to_reference(project) %>
- Ignored in code: `<%= simple_milestone.to_reference %>`
- Ignored in links: [Link to <%= simple_milestone.to_reference %>](#milestone-link)
- Milestone by URL: <%= urls.project_milestone_url(milestone.project, milestone) %>
- Milestone by URL: <%= urls.milestone_url(milestone) %>
- Link to milestone by URL: [Milestone](<%= milestone.to_reference %>)
- Group milestone by name: <%= Milestone.reference_prefix %><%= group_milestone.name %>
- Group milestone by name in quotes: <%= group_milestone.to_reference(format: :name) %>
- Group milestone by URL is ignore: <%= urls.milestone_url(group_milestone) %>
### Task Lists
......
......@@ -63,44 +63,4 @@ describe GitlabRoutingHelper do
it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
end
end
describe '#milestone_path' do
context 'for a group milestone' do
let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) }
it 'links to the group milestone page' do
expect(milestone_path(milestone))
.to eq(group_milestone_path(group, milestone))
end
end
context 'for a project milestone' do
let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) }
it 'links to the project milestone page' do
expect(milestone_path(milestone))
.to eq(project_milestone_path(project, milestone))
end
end
end
describe '#milestone_url' do
context 'for a group milestone' do
let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) }
it 'links to the group milestone page' do
expect(milestone_url(milestone))
.to eq(group_milestone_url(group, milestone))
end
end
context 'for a project milestone' do
let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) }
it 'links to the project milestone page' do
expect(milestone_url(milestone))
.to eq(project_milestone_url(project, milestone))
end
end
end
end
require 'spec_helper'
describe MilestonesRoutingHelper do
let(:project) { build_stubbed(:project) }
let(:group) { build_stubbed(:group) }
describe '#milestone_path' do
context 'for a group milestone' do
let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) }
it 'links to the group milestone page' do
expect(milestone_path(milestone))
.to eq(group_milestone_path(group, milestone))
end
end
context 'for a project milestone' do
let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) }
it 'links to the project milestone page' do
expect(milestone_path(milestone))
.to eq(project_milestone_path(project, milestone))
end
end
end
describe '#milestone_url' do
context 'for a group milestone' do
let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) }
it 'links to the group milestone page' do
expect(milestone_url(milestone))
.to eq(group_milestone_url(group, milestone))
end
end
context 'for a project milestone' do
let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) }
it 'links to the project milestone page' do
expect(milestone_url(milestone))
.to eq(project_milestone_url(project, milestone))
end
end
end
end
......@@ -3,23 +3,24 @@ require 'spec_helper'
describe Banzai::Filter::MilestoneReferenceFilter do
include FilterSpecHelper
let(:project) { create(:project, :public) }
let(:milestone) { create(:milestone, project: project) }
let(:reference) { milestone.to_reference }
let(:group) { create(:group, :public) }
let(:project) { create(:project, :public, group: group) }
it 'requires project context' do
expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
end
shared_examples 'reference parsing' do
%w(pre code a style).each do |elem|
it "ignores valid references contained inside '#{elem}' element" do
exp = act = "<#{elem}>milestone #{milestone.to_reference}</#{elem}>"
exp = act = "<#{elem}>milestone #{reference}</#{elem}>"
expect(reference_filter(act).to_html).to eq exp
end
end
it 'includes default classes' do
doc = reference_filter("Milestone #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone has-tooltip'
end
......@@ -44,16 +45,15 @@ describe Banzai::Filter::MilestoneReferenceFilter do
link = doc.css('a').first.attr('href')
expect(link).not_to match %r(https?://)
expect(link).to eq urls
.project_milestone_path(project, milestone)
expect(link).to eq urls.milestone_path(milestone)
end
end
context 'Integer-based references' do
shared_examples 'Integer-based references' do
it 'links to a valid reference' do
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls
.project_milestone_url(project, milestone)
expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
end
it 'links with adjacent text' do
......@@ -68,15 +68,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
context 'String-based single-word references' do
let(:milestone) { create(:milestone, name: 'gfm', project: project) }
shared_examples 'String-based single-word references' do
let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
before do
milestone.update!(name: 'gfm')
end
it 'links to a valid reference' do
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls
.project_milestone_url(project, milestone)
expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
expect(doc.text).to eq 'See gfm'
end
......@@ -92,15 +94,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
context 'String-based multi-word references in quotes' do
let(:milestone) { create(:milestone, name: 'gfm references', project: project) }
shared_examples 'String-based multi-word references in quotes' do
let(:reference) { milestone.to_reference(format: :name) }
before do
milestone.update!(name: 'gfm references')
end
it 'links to a valid reference' do
doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls
.project_milestone_url(project, milestone)
expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
expect(doc.text).to eq 'See gfm references'
end
......@@ -116,23 +120,27 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
describe 'referencing a milestone in a link href' do
let(:reference) { %Q{<a href="#{milestone.to_reference}">Milestone</a>} }
shared_examples 'referencing a milestone in a link href' do
let(:unquoted_reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
let(:link_reference) { %Q{<a href="#{unquoted_reference}">Milestone</a>} }
before do
milestone.update!(name: 'gfm')
end
it 'links to a valid reference' do
doc = reference_filter("See #{reference}")
doc = reference_filter("See #{link_reference}")
expect(doc.css('a').first.attr('href')).to eq urls
.project_milestone_url(project, milestone)
expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
end
it 'links with adjacent text' do
doc = reference_filter("Milestone (#{reference}.)")
doc = reference_filter("Milestone (#{link_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 #{link_reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-project')
......@@ -140,7 +148,35 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
it 'includes a data-milestone attribute' do
doc = reference_filter("See #{reference}")
doc = reference_filter("See #{link_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
end
shared_examples 'linking to a milestone as the entire link' do
let(:unquoted_reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
let(:link) { urls.milestone_url(milestone) }
let(:link_reference) { %Q{<a href="#{link}">#{link}</a>} }
it 'replaces the link text with the milestone reference' do
doc = reference_filter("See #{link}")
expect(doc.css('a').first.text).to eq(unquoted_reference)
end
it 'includes a data-project attribute' do
doc = reference_filter("Milestone #{link_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 #{link_reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-milestone')
......@@ -148,7 +184,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
describe 'cross-project / cross-namespace complete reference' do
shared_examples 'cross-project / cross-namespace complete reference' do
let(:namespace) { create(:namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) }
let(:milestone) { create(:milestone, project: another_project) }
......@@ -184,7 +220,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
describe 'cross-project / same-namespace complete reference' do
shared_examples 'cross-project / same-namespace complete reference' do
let(:namespace) { create(:namespace) }
let(:project) { create(:project, :public, namespace: namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) }
......@@ -221,7 +257,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
describe 'cross project shorthand reference' do
shared_examples 'cross project shorthand reference' do
let(:namespace) { create(:namespace) }
let(:project) { create(:project, :public, namespace: namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) }
......@@ -258,27 +294,53 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end
end
describe 'cross project milestone references' do
let(:another_project) { create(:project, :public) }
let(:project_path) { another_project.full_path }
let(:milestone) { create(:milestone, project: another_project) }
let(:reference) { milestone.to_reference(project) }
context 'project milestones' do
let(:milestone) { create(:milestone, project: project) }
let(:reference) { milestone.to_reference }
let!(:result) { reference_filter("See #{reference}") }
include_examples 'reference parsing'
it 'points to referenced project milestone page' do
expect(result.css('a').first.attr('href')).to eq urls
.project_milestone_url(another_project, milestone)
it_behaves_like 'Integer-based references'
it_behaves_like 'String-based single-word references'
it_behaves_like 'String-based multi-word references in quotes'
it_behaves_like 'referencing a milestone in a link href'
it_behaves_like 'cross-project / cross-namespace complete reference'
it_behaves_like 'cross-project / same-namespace complete reference'
it_behaves_like 'cross project shorthand reference'
end
it 'contains cross project content' do
expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_path}"
context 'group milestones' do
let(:milestone) { create(:milestone, group: group) }
let(:reference) { milestone.to_reference(format: :name) }
include_examples 'reference parsing'
it_behaves_like 'String-based single-word references'
it_behaves_like 'String-based multi-word references in quotes'
it_behaves_like 'referencing a milestone in a link href'
it 'does not support references by IID' do
doc = reference_filter("See #{Milestone.reference_prefix}#{milestone.iid}")
expect(doc.css('a')).to be_empty
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}"
it 'does not support references by link' do
doc = reference_filter("See #{urls.milestone_url(milestone)}")
expect(doc.css('a').first.text).to eq(urls.milestone_url(milestone))
end
it 'does not support cross-project references' do
another_group = create(:group)
another_project = create(:project, :public, group: group)
project_reference = another_project.to_reference(project)
milestone.update!(group: another_group)
doc = reference_filter("See #{project_reference}#{reference}")
expect(doc.css('a')).to be_empty
end
end
end
......@@ -230,16 +230,40 @@ describe Milestone do
end
describe '#to_reference' do
let(:project) { build(:project, name: 'sample-project') }
let(:milestone) { build(:milestone, iid: 1, project: project) }
let(:group) { build_stubbed(:group) }
let(:project) { build_stubbed(:project, name: 'sample-project') }
let(:another_project) { build_stubbed(:project, name: 'another-project', namespace: project.namespace) }
context 'for a project milestone' do
let(:milestone) { build_stubbed(:milestone, iid: 1, project: project, name: 'milestone') }
it 'returns a String reference to the object' do
expect(milestone.to_reference).to eq "%1"
expect(milestone.to_reference).to eq '%1'
end
it 'returns a reference by name when the format is set to :name' do
expect(milestone.to_reference(format: :name)).to eq '%"milestone"'
end
it 'supports a cross-project reference' do
another_project = build(:project, name: 'another-project', namespace: project.namespace)
expect(milestone.to_reference(another_project)).to eq "sample-project%1"
expect(milestone.to_reference(another_project)).to eq 'sample-project%1'
end
end
context 'for a group milestone' do
let(:milestone) { build_stubbed(:milestone, iid: 1, group: group, name: 'milestone') }
it 'returns nil with the default format' do
expect(milestone.to_reference).to be_nil
end
it 'returns a reference by name when the format is set to :name' do
expect(milestone.to_reference(format: :name)).to eq '%"milestone"'
end
it 'does not supports cross-project references' do
expect(milestone.to_reference(another_project, format: :name)).to eq '%"milestone"'
end
end
end
......
......@@ -23,7 +23,7 @@ class MarkdownFeature
# Direct references ----------------------------------------------------------
def project
@project ||= create(:project, :repository).tap do |project|
@project ||= create(:project, :repository, group: group).tap do |project|
project.team << [user, :master]
end
end
......@@ -75,6 +75,10 @@ class MarkdownFeature
@milestone ||= create(:milestone, name: 'next goal', project: project)
end
def group_milestone
@group_milestone ||= create(:milestone, name: 'group-milestone', group: group)
end
# Cross-references -----------------------------------------------------------
def xproject
......
......@@ -155,7 +155,7 @@ module MarkdownMatchers
set_default_markdown_messages
match do |actual|
expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6)
expect(actual).to have_selector('a.gfm.gfm-milestone', count: 8)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment