Commit 4e2b630d authored by Robert Speicher's avatar Robert Speicher

Merge branch 'group-milestone-references-system-notes' into 'master'

Support group milestone references

Closes #34778

See merge request !13289
parents bc648ae5 df00ebde
...@@ -47,14 +47,6 @@ module GitlabRoutingHelper ...@@ -47,14 +47,6 @@ module GitlabRoutingHelper
project_pipeline_path(pipeline.project, pipeline.id, *args) project_pipeline_path(pipeline.project, pipeline.id, *args)
end 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) def issue_url(entity, *args)
project_issue_url(entity.project, entity, *args) project_issue_url(entity.project, entity, *args)
end end
...@@ -67,14 +59,6 @@ module GitlabRoutingHelper ...@@ -67,14 +59,6 @@ module GitlabRoutingHelper
project_pipeline_url(pipeline.project, pipeline.id, *args) project_pipeline_url(pipeline.project, pipeline.id, *args)
end 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) def pipeline_job_url(pipeline, build, *args)
project_job_url(pipeline.project, build.id, *args) project_job_url(pipeline.project, build.id, *args)
end 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 ...@@ -149,7 +149,9 @@ class Milestone < ActiveRecord::Base
end 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) # format - Symbol format to use (default: :iid, optional: :name)
# #
...@@ -161,12 +163,16 @@ class Milestone < ActiveRecord::Base ...@@ -161,12 +163,16 @@ class Milestone < ActiveRecord::Base
# Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1" # Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1"
# #
def to_reference(from_project = nil, format: :iid, full: false) 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) format_reference = milestone_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}" reference = "#{self.class.reference_prefix}#{format_reference}"
"#{project.to_reference(from_project, full: full)}#{reference}" if project
"#{project.to_reference(from_project, full: full)}#{reference}"
else
reference
end
end end
def reference_link_text(from_project = nil) def reference_link_text(from_project = nil)
......
...@@ -2,11 +2,8 @@ class IssuableBaseService < BaseService ...@@ -2,11 +2,8 @@ class IssuableBaseService < BaseService
private private
def create_milestone_note(issuable) def create_milestone_note(issuable)
milestone = issuable.milestone
return if milestone && milestone.is_group_milestone?
SystemNoteService.change_milestone( SystemNoteService.change_milestone(
issuable, issuable.project, current_user, milestone) issuable, issuable.project, current_user, issuable.milestone)
end end
def create_labels_note(issuable, old_labels) def create_labels_note(issuable, old_labels)
......
...@@ -5,7 +5,15 @@ module Projects ...@@ -5,7 +5,15 @@ module Projects
end end
def milestones def milestones
@project.milestones.active.reorder(due_date: :asc, title: :asc).select([:iid, :title]) finder_params = {
project_ids: [@project.id],
state: :active,
order: { due_date: :asc, title: :asc }
}
finder_params[:group_ids] = [@project.group.id] if @project.group
MilestonesFinder.new(finder_params).execute.select([:iid, :title])
end end
def merge_requests def merge_requests
......
...@@ -142,7 +142,8 @@ module SystemNoteService ...@@ -142,7 +142,8 @@ module SystemNoteService
# #
# Returns the created Note object # Returns the created Note object
def change_milestone(noteable, project, author, milestone) def change_milestone(noteable, project, author, milestone)
body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project)}" format = milestone&.is_group_milestone? ? :name : :iid
body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project, format: format)}"
create_note(NoteSummary.new(noteable, project, author, body, action: 'milestone')) create_note(NoteSummary.new(noteable, project, author, body, action: 'milestone'))
end end
......
---
title: Support Markdown references, autocomplete, and quick actions for group milestones
merge_request:
author:
...@@ -181,7 +181,11 @@ module Gitlab ...@@ -181,7 +181,11 @@ module Gitlab
end end
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(project_url_helpers)
Gitlab::Routing.add_helpers(MilestonesRoutingHelper)
end end
end end
end end
...@@ -248,7 +248,7 @@ GFM will recognize the following: ...@@ -248,7 +248,7 @@ GFM will recognize the following:
| `~123` | label by ID | | `~123` | label by ID |
| `~bug` | one-word label by name | | `~bug` | one-word label by name |
| `~"feature request"` | multi-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 | | `%v1.23` | one-word milestone by name |
| `%"release candidate"` | multi-word milestone by name | | `%"release candidate"` | multi-word milestone by name |
| `9ba12248` | specific commit | | `9ba12248` | specific commit |
...@@ -262,7 +262,7 @@ GFM also recognizes certain cross-project references: ...@@ -262,7 +262,7 @@ GFM also recognizes certain cross-project references:
|:----------------------------------------|:------------------------| |:----------------------------------------|:------------------------|
| `namespace/project#123` | issue | | `namespace/project#123` | issue |
| `namespace/project!123` | merge request | | `namespace/project!123` | merge request |
| `namespace/project%123` | milestone | | `namespace/project%123` | project milestone |
| `namespace/project$123` | snippet | | `namespace/project$123` | snippet |
| `namespace/project@9ba12248` | specific commit | | `namespace/project@9ba12248` | specific commit |
| `namespace/project@9ba12248...b19a04f5` | commit range comparison | | `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 ...@@ -274,7 +274,7 @@ It also has a shorthand version to reference other projects from the same namesp
|:------------------------------|:------------------------| |:------------------------------|:------------------------|
| `project#123` | issue | | `project#123` | issue |
| `project!123` | merge request | | `project!123` | merge request |
| `project%123` | milestone | | `project%123` | project milestone |
| `project$123` | snippet | | `project$123` | snippet |
| `project@9ba12248` | specific commit | | `project@9ba12248` | specific commit |
| `project@9ba12248...b19a04f5` | commit range comparison | | `project@9ba12248...b19a04f5` | commit range comparison |
......
...@@ -56,4 +56,5 @@ total merge requests and issues. ...@@ -56,4 +56,5 @@ total merge requests and issues.
## Quick actions ## Quick actions
[Quick actions](../quick_actions.md) are available for assigning and removing project milestones only. [In the future](https://gitlab.com/gitlab-org/gitlab-ce/issues/34778), this will also apply to group milestones. [Quick actions](../quick_actions.md) are available for assigning and removing
project and group milestones.
...@@ -54,42 +54,42 @@ module Banzai ...@@ -54,42 +54,42 @@ module Banzai
self.class.references_in(*args, &block) self.class.references_in(*args, &block)
end end
# Implement in child class
# Example: project.merge_requests.find
def find_object(project, id) def find_object(project, id)
# Implement in child class
# Example: project.merge_requests.find
end end
def find_object_cached(project, id) # Override if the link reference pattern produces a different ID (global
if RequestStore.active? # ID vs internal ID, for instance) to the regular reference pattern.
cache = find_objects_cache[object_class][project.id] def find_object_from_link(project, id)
find_object(project, id)
end
get_or_set_cache(cache, id) { find_object(project, id) } # Implement in child class
else # Example: project_merge_request_url
def url_for_object(object, project)
end
def find_object_cached(project, id)
cached_call(:banzai_find_object, id, path: [object_class, project.id]) do
find_object(project, id) find_object(project, id)
end end
end end
def project_from_ref_cached(ref) def find_object_from_link_cached(project, id)
if RequestStore.active? cached_call(:banzai_find_object_from_link, id, path: [object_class, project.id]) do
cache = project_refs_cache find_object_from_link(project, id)
get_or_set_cache(cache, ref) { project_from_ref(ref) }
else
project_from_ref(ref)
end end
end end
def url_for_object(object, project) def project_from_ref_cached(ref)
# Implement in child class cached_call(:banzai_project_refs, ref) do
# Example: project_merge_request_url project_from_ref(ref)
end
end end
def url_for_object_cached(object, project) def url_for_object_cached(object, project)
if RequestStore.active? cached_call(:banzai_url_for_object, object, path: [object_class, project.id]) do
cache = url_for_object_cache[object_class][project.id]
get_or_set_cache(cache, object) { url_for_object(object, project) }
else
url_for_object(object, project) url_for_object(object, project)
end end
end end
...@@ -120,7 +120,7 @@ module Banzai ...@@ -120,7 +120,7 @@ module Banzai
if link == inner_html && inner_html =~ /\A#{link_pattern}/ if link == inner_html && inner_html =~ /\A#{link_pattern}/
replace_link_node_with_text(node, link) do 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 end
next next
...@@ -128,7 +128,7 @@ module Banzai ...@@ -128,7 +128,7 @@ module Banzai
if link =~ /\A#{link_pattern}\z/ if link =~ /\A#{link_pattern}\z/
replace_link_node_with_href(node, link) do 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 end
next next
...@@ -146,15 +146,26 @@ module Banzai ...@@ -146,15 +146,26 @@ module Banzai
# text - String text to replace references in. # text - String text to replace references in.
# pattern - Reference pattern to match against. # pattern - Reference pattern to match against.
# link_content - Original content of the link being replaced. # 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 # Returns a String with references replaced with links. All links
# have `gfm` and `gfm-OBJECT_NAME` class names attached for styling. # 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| references_in(text, pattern) do |match, id, project_ref, namespace_ref, matches|
project_path = full_project_path(namespace_ref, project_ref) project_path = full_project_path(namespace_ref, project_ref)
project = project_from_ref_cached(project_path) 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) title = object_link_title(object)
klass = reference_class(object_sym) klass = reference_class(object_sym)
...@@ -297,15 +308,17 @@ module Banzai ...@@ -297,15 +308,17 @@ module Banzai
RequestStore[:banzai_project_refs] ||= {} RequestStore[:banzai_project_refs] ||= {}
end end
def find_objects_cache def cached_call(request_store_key, cache_key, path: [])
RequestStore[:banzai_find_objects_cache] ||= Hash.new do |hash, key| if RequestStore.active?
hash[key] = Hash.new { |h, k| h[k] = {} } cache = RequestStore[request_store_key] ||= Hash.new do |hash, key|
end hash[key] = Hash.new { |h, k| h[k] = {} }
end end
def url_for_object_cache cache = cache.dig(*path) if path.any?
RequestStore[:banzai_url_for_object] ||= Hash.new do |hash, key|
hash[key] = Hash.new { |h, k| h[k] = {} } get_or_set_cache(cache, cache_key) { yield }
else
yield
end end
end end
......
...@@ -8,8 +8,15 @@ module Banzai ...@@ -8,8 +8,15 @@ module Banzai
Milestone Milestone
end 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) 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 end
def references_in(text, pattern = Milestone.reference_pattern) def references_in(text, pattern = Milestone.reference_pattern)
...@@ -22,7 +29,7 @@ module Banzai ...@@ -22,7 +29,7 @@ module Banzai
milestone = find_milestone($~[:project], $~[:namespace], $~[:milestone_iid], $~[:milestone_name]) milestone = find_milestone($~[:project], $~[:namespace], $~[:milestone_iid], $~[:milestone_name])
if milestone if milestone
yield match, milestone.iid, $~[:project], $~[:namespace], $~ yield match, milestone.id, $~[:project], $~[:namespace], $~
else else
match match
end end
...@@ -36,7 +43,8 @@ module Banzai ...@@ -36,7 +43,8 @@ module Banzai
return unless project return unless project
milestone_params = milestone_params(milestone_id, milestone_name) milestone_params = milestone_params(milestone_id, milestone_name)
project.milestones.find_by(milestone_params)
find_milestone_with_finder(project, milestone_params)
end end
def milestone_params(iid, name) def milestone_params(iid, name)
...@@ -47,15 +55,27 @@ module Banzai ...@@ -47,15 +55,27 @@ module Banzai
end end
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) def url_for_object(milestone, project)
h = Gitlab::Routing.url_helpers Gitlab::Routing
h.project_milestone_url(project, milestone, .url_helpers
only_path: context[:only_path]) .milestone_url(milestone, only_path: context[:only_path])
end end
def object_link_text(object, matches) def object_link_text(object, matches)
milestone_link = escape_once(super) milestone_link = escape_once(super)
reference = object.project.to_reference(project) reference = object.project&.to_reference(project)
if reference.present? if reference.present?
"#{milestone_link} <i>in #{reference}</i>".html_safe "#{milestone_link} <i>in #{reference}</i>".html_safe
......
...@@ -227,8 +227,11 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e ...@@ -227,8 +227,11 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
- Milestone in another project: <%= xmilestone.to_reference(project) %> - Milestone in another project: <%= xmilestone.to_reference(project) %>
- Ignored in code: `<%= simple_milestone.to_reference %>` - Ignored in code: `<%= simple_milestone.to_reference %>`
- Ignored in links: [Link to <%= simple_milestone.to_reference %>](#milestone-link) - 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 %>) - 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 ### Task Lists
......
...@@ -63,44 +63,4 @@ describe GitlabRoutingHelper do ...@@ -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) } it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
end end
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 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,57 +3,57 @@ require 'spec_helper' ...@@ -3,57 +3,57 @@ require 'spec_helper'
describe Banzai::Filter::MilestoneReferenceFilter do describe Banzai::Filter::MilestoneReferenceFilter do
include FilterSpecHelper include FilterSpecHelper
let(:project) { create(:project, :public) } let(:group) { create(:group, :public) }
let(:milestone) { create(:milestone, project: project) } let(:project) { create(:project, :public, group: group) }
let(:reference) { milestone.to_reference }
it 'requires project context' do it 'requires project context' do
expect { described_class.call('') }.to raise_error(ArgumentError, /:project/) expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
end end
%w(pre code a style).each do |elem| shared_examples 'reference parsing' do
it "ignores valid references contained inside '#{elem}' element" do %w(pre code a style).each do |elem|
exp = act = "<#{elem}>milestone #{milestone.to_reference}</#{elem}>" it "ignores valid references contained inside '#{elem}' element" do
expect(reference_filter(act).to_html).to eq exp exp = act = "<#{elem}>milestone #{reference}</#{elem}>"
expect(reference_filter(act).to_html).to eq exp
end
end end
end
it 'includes default classes' do it 'includes default classes' do
doc = reference_filter("Milestone #{reference}") doc = reference_filter("Milestone #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone has-tooltip'
end
it 'includes a data-project attribute' do expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone has-tooltip'
doc = reference_filter("Milestone #{reference}") end
link = doc.css('a').first
expect(link).to have_attribute('data-project') it 'includes a data-project attribute' do
expect(link.attr('data-project')).to eq project.id.to_s doc = reference_filter("Milestone #{reference}")
end link = doc.css('a').first
it 'includes a data-milestone attribute' do expect(link).to have_attribute('data-project')
doc = reference_filter("See #{reference}") expect(link.attr('data-project')).to eq project.id.to_s
link = doc.css('a').first end
expect(link).to have_attribute('data-milestone') it 'includes a data-milestone attribute' do
expect(link.attr('data-milestone')).to eq milestone.id.to_s doc = reference_filter("See #{reference}")
end 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 it 'supports an :only_path context' do
doc = reference_filter("Milestone #{reference}", only_path: true) doc = reference_filter("Milestone #{reference}", only_path: true)
link = doc.css('a').first.attr('href') link = doc.css('a').first.attr('href')
expect(link).not_to match %r(https?://) expect(link).not_to match %r(https?://)
expect(link).to eq urls expect(link).to eq urls.milestone_path(milestone)
.project_milestone_path(project, milestone) end
end end
context 'Integer-based references' do shared_examples 'Integer-based references' do
it 'links to a valid reference' do it 'links to a valid reference' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
.project_milestone_url(project, milestone)
end end
it 'links with adjacent text' do it 'links with adjacent text' do
...@@ -68,15 +68,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -68,15 +68,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
context 'String-based single-word references' do shared_examples 'String-based single-word references' do
let(:milestone) { create(:milestone, name: 'gfm', project: project) }
let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" } let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" }
before do
milestone.update!(name: 'gfm')
end
it 'links to a valid reference' do it 'links to a valid reference' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
.project_milestone_url(project, milestone)
expect(doc.text).to eq 'See gfm' expect(doc.text).to eq 'See gfm'
end end
...@@ -92,15 +94,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -92,15 +94,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
context 'String-based multi-word references in quotes' do shared_examples 'String-based multi-word references in quotes' do
let(:milestone) { create(:milestone, name: 'gfm references', project: project) }
let(:reference) { milestone.to_reference(format: :name) } let(:reference) { milestone.to_reference(format: :name) }
before do
milestone.update!(name: 'gfm references')
end
it 'links to a valid reference' do it 'links to a valid reference' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{reference}")
expect(doc.css('a').first.attr('href')).to eq urls expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
.project_milestone_url(project, milestone)
expect(doc.text).to eq 'See gfm references' expect(doc.text).to eq 'See gfm references'
end end
...@@ -116,23 +120,27 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -116,23 +120,27 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
describe 'referencing a milestone in a link href' do shared_examples 'referencing a milestone in a link href' do
let(:reference) { %Q{<a href="#{milestone.to_reference}">Milestone</a>} } 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 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 expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone)
.project_milestone_url(project, milestone)
end end
it 'links with adjacent text' do 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>\.\))) expect(doc.to_html).to match(%r(\(<a.+>Milestone</a>\.\)))
end end
it 'includes a data-project attribute' do it 'includes a data-project attribute' do
doc = reference_filter("Milestone #{reference}") doc = reference_filter("Milestone #{link_reference}")
link = doc.css('a').first link = doc.css('a').first
expect(link).to have_attribute('data-project') expect(link).to have_attribute('data-project')
...@@ -140,7 +148,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -140,7 +148,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
it 'includes a data-milestone attribute' do it 'includes a data-milestone attribute' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{link_reference}")
link = doc.css('a').first link = doc.css('a').first
expect(link).to have_attribute('data-milestone') expect(link).to have_attribute('data-milestone')
...@@ -148,7 +156,35 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -148,7 +156,35 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
describe 'cross-project / cross-namespace complete reference' do 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')
expect(link.attr('data-milestone')).to eq milestone.id.to_s
end
end
shared_examples 'cross-project / cross-namespace complete reference' do
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) } let(:another_project) { create(:project, :public, namespace: namespace) }
let(:milestone) { create(:milestone, project: another_project) } let(:milestone) { create(:milestone, project: another_project) }
...@@ -184,7 +220,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -184,7 +220,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
describe 'cross-project / same-namespace complete reference' do shared_examples 'cross-project / same-namespace complete reference' do
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
let(:project) { create(:project, :public, namespace: namespace) } let(:project) { create(:project, :public, namespace: namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) } let(:another_project) { create(:project, :public, namespace: namespace) }
...@@ -221,7 +257,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -221,7 +257,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
describe 'cross project shorthand reference' do shared_examples 'cross project shorthand reference' do
let(:namespace) { create(:namespace) } let(:namespace) { create(:namespace) }
let(:project) { create(:project, :public, namespace: namespace) } let(:project) { create(:project, :public, namespace: namespace) }
let(:another_project) { create(:project, :public, namespace: namespace) } let(:another_project) { create(:project, :public, namespace: namespace) }
...@@ -258,27 +294,53 @@ describe Banzai::Filter::MilestoneReferenceFilter do ...@@ -258,27 +294,53 @@ describe Banzai::Filter::MilestoneReferenceFilter do
end end
end end
describe 'cross project milestone references' do context 'project milestones' do
let(:another_project) { create(:project, :public) } let(:milestone) { create(:milestone, project: project) }
let(:project_path) { another_project.full_path } let(:reference) { milestone.to_reference }
let(:milestone) { create(:milestone, project: another_project) }
let(:reference) { milestone.to_reference(project) }
let!(:result) { reference_filter("See #{reference}") } include_examples 'reference parsing'
it 'points to referenced project milestone page' do it_behaves_like 'Integer-based references'
expect(result.css('a').first.attr('href')).to eq urls it_behaves_like 'String-based single-word references'
.project_milestone_url(another_project, milestone) 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
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 end
it 'contains cross project content' do it 'does not support references by link' do
expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_path}" doc = reference_filter("See #{urls.milestone_url(milestone)}")
expect(doc.css('a').first.text).to eq(urls.milestone_url(milestone))
end end
it 'escapes the name attribute' do it 'does not support cross-project references' do
allow_any_instance_of(Milestone).to receive(:title).and_return(%{"></a>whatever<a title="}) another_group = create(:group)
doc = reference_filter("See #{reference}") another_project = create(:project, :public, group: group)
expect(doc.css('a').first.text).to eq "#{milestone.name} in #{project_path}" 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 end
end end
...@@ -230,16 +230,40 @@ describe Milestone do ...@@ -230,16 +230,40 @@ describe Milestone do
end end
describe '#to_reference' do describe '#to_reference' do
let(:project) { build(:project, name: 'sample-project') } let(:group) { build_stubbed(:group) }
let(:milestone) { build(:milestone, iid: 1, project: project) } 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'
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 'returns a String reference to the object' do it 'supports a cross-project reference' do
expect(milestone.to_reference).to eq "%1" expect(milestone.to_reference(another_project)).to eq 'sample-project%1'
end
end end
it 'supports a cross-project reference' do context 'for a group milestone' do
another_project = build(:project, name: 'another-project', namespace: project.namespace) let(:milestone) { build_stubbed(:milestone, iid: 1, group: group, name: 'milestone') }
expect(milestone.to_reference(another_project)).to eq "sample-project%1"
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
end end
......
...@@ -88,4 +88,31 @@ describe Projects::AutocompleteService do ...@@ -88,4 +88,31 @@ describe Projects::AutocompleteService do
end end
end end
end end
describe '#milestones' do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let!(:group_milestone) { create(:milestone, group: group) }
let!(:project_milestone) { create(:milestone, project: project) }
let(:milestone_titles) { described_class.new(project, user).milestones.map(&:title) }
it 'includes project and group milestones' do
expect(milestone_titles).to eq([group_milestone.title, project_milestone.title])
end
it 'does not include closed milestones' do
group_milestone.close
expect(milestone_titles).to eq([project_milestone.title])
end
it 'does not include milestones from other projects in the group' do
other_project = create(:project, group: group)
project_milestone.update!(project: other_project)
expect(milestone_titles).to eq([group_milestone.title])
end
end
end end
...@@ -3,7 +3,8 @@ require 'spec_helper' ...@@ -3,7 +3,8 @@ require 'spec_helper'
describe SystemNoteService do describe SystemNoteService do
include Gitlab::Routing include Gitlab::Routing
let(:project) { create(:project) } let(:group) { create(:group) }
let(:project) { create(:project, group: group) }
let(:author) { create(:user) } let(:author) { create(:user) }
let(:noteable) { create(:issue, project: project) } let(:noteable) { create(:issue, project: project) }
let(:issue) { noteable } let(:issue) { noteable }
...@@ -242,25 +243,51 @@ describe SystemNoteService do ...@@ -242,25 +243,51 @@ describe SystemNoteService do
end end
describe '.change_milestone' do describe '.change_milestone' do
subject { described_class.change_milestone(noteable, project, author, milestone) } context 'for a project milestone' do
subject { described_class.change_milestone(noteable, project, author, milestone) }
let(:milestone) { create(:milestone, project: project) } let(:milestone) { create(:milestone, project: project) }
it_behaves_like 'a system note' do it_behaves_like 'a system note' do
let(:action) { 'milestone' } let(:action) { 'milestone' }
end end
context 'when milestone added' do context 'when milestone added' do
it 'sets the note text' do it 'sets the note text' do
expect(subject.note).to eq "changed milestone to #{milestone.to_reference}" expect(subject.note).to eq "changed milestone to #{milestone.to_reference}"
end
end
context 'when milestone removed' do
let(:milestone) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'removed milestone'
end
end end
end end
context 'when milestone removed' do context 'for a group milestone' do
let(:milestone) { nil } subject { described_class.change_milestone(noteable, project, author, milestone) }
it 'sets the note text' do let(:milestone) { create(:milestone, group: group) }
expect(subject.note).to eq 'removed milestone'
it_behaves_like 'a system note' do
let(:action) { 'milestone' }
end
context 'when milestone added' do
it 'sets the note text to use the milestone name' do
expect(subject.note).to eq "changed milestone to #{milestone.to_reference(format: :name)}"
end
end
context 'when milestone removed' do
let(:milestone) { nil }
it 'sets the note text' do
expect(subject.note).to eq 'removed milestone'
end
end end
end end
end end
......
...@@ -21,15 +21,15 @@ shared_examples 'system notes for milestones' do ...@@ -21,15 +21,15 @@ shared_examples 'system notes for milestones' do
create(:group_member, group: group, user: user) create(:group_member, group: group, user: user)
end end
it 'does not create system note' do it 'creates a system note' do
expect do expect do
update_issuable(milestone: group_milestone) update_issuable(milestone: group_milestone)
end.not_to change { Note.system.count } end.to change { Note.system.count }.by(1)
end end
end end
context 'project milestones' do context 'project milestones' do
it 'creates system note' do it 'creates a system note' do
expect do expect do
update_issuable(milestone: create(:milestone)) update_issuable(milestone: create(:milestone))
end.to change { Note.system.count }.by(1) end.to change { Note.system.count }.by(1)
......
...@@ -23,7 +23,7 @@ class MarkdownFeature ...@@ -23,7 +23,7 @@ class MarkdownFeature
# Direct references ---------------------------------------------------------- # Direct references ----------------------------------------------------------
def project def project
@project ||= create(:project, :repository).tap do |project| @project ||= create(:project, :repository, group: group).tap do |project|
project.team << [user, :master] project.team << [user, :master]
end end
end end
...@@ -75,6 +75,10 @@ class MarkdownFeature ...@@ -75,6 +75,10 @@ class MarkdownFeature
@milestone ||= create(:milestone, name: 'next goal', project: project) @milestone ||= create(:milestone, name: 'next goal', project: project)
end end
def group_milestone
@group_milestone ||= create(:milestone, name: 'group-milestone', group: group)
end
# Cross-references ----------------------------------------------------------- # Cross-references -----------------------------------------------------------
def xproject def xproject
......
...@@ -155,7 +155,7 @@ module MarkdownMatchers ...@@ -155,7 +155,7 @@ module MarkdownMatchers
set_default_markdown_messages set_default_markdown_messages
match do |actual| 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
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