Commit ba5b47c2 authored by Douwe Maan's avatar Douwe Maan

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
parents 983b592c 129bb6c2
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.8.0 (unreleased) v 8.8.0 (unreleased)
- Implement GFM references for milestones (Alejandro Rodríguez)
- Snippets tab under user profile. !4001 (Long Nguyen) - Snippets tab under user profile. !4001 (Long Nguyen)
- Fix error when using link to uploads in global snippets - Fix error when using link to uploads in global snippets
- Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref - Fix Error 500 when attempting to retrieve project license when HEAD points to non-existent ref
......
...@@ -18,6 +18,10 @@ GitLab.GfmAutoComplete = ...@@ -18,6 +18,10 @@ GitLab.GfmAutoComplete =
Issues: Issues:
template: '<li><small>${id}</small> ${title}</li>' template: '<li><small>${id}</small> ${title}</li>'
# Milestones
Milestones:
template: '<li>${title}</li>'
# Add GFM auto-completion to all input fields, that accept GFM input. # Add GFM auto-completion to all input fields, that accept GFM input.
setup: (wrap) -> setup: (wrap) ->
@input = $('.js-gfm-input') @input = $('.js-gfm-input')
...@@ -81,6 +85,19 @@ GitLab.GfmAutoComplete = ...@@ -81,6 +85,19 @@ GitLab.GfmAutoComplete =
title: sanitize(i.title) title: sanitize(i.title)
search: "#{i.iid} #{i.title}" search: "#{i.iid} #{i.title}"
@input.atwho
at: '%'
alias: 'milestones'
searchKey: 'search'
displayTpl: @Milestones.template
insertTpl: '${atwho-at}"${title}"'
callbacks:
beforeSave: (milestones) ->
$.map milestones, (m) ->
id: m.iid
title: sanitize(m.title)
search: "#{m.title}"
@input.atwho @input.atwho
at: '!' at: '!'
alias: 'mergerequests' alias: 'mergerequests'
...@@ -105,6 +122,8 @@ GitLab.GfmAutoComplete = ...@@ -105,6 +122,8 @@ GitLab.GfmAutoComplete =
@input.atwho 'load', '@', data.members @input.atwho 'load', '@', data.members
# load issues # load issues
@input.atwho 'load', 'issues', data.issues @input.atwho 'load', 'issues', data.issues
# load milestones
@input.atwho 'load', 'milestones', data.milestones
# load merge requests # load merge requests
@input.atwho 'load', 'mergerequests', data.mergerequests @input.atwho 'load', 'mergerequests', data.mergerequests
# load emojis # load emojis
......
...@@ -141,6 +141,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -141,6 +141,7 @@ class ProjectsController < Projects::ApplicationController
@suggestions = { @suggestions = {
emojis: AwardEmoji.urls, emojis: AwardEmoji.urls,
issues: autocomplete.issues, issues: autocomplete.issues,
milestones: autocomplete.milestones,
mergerequests: autocomplete.merge_requests, mergerequests: autocomplete.merge_requests,
members: participants members: participants
} }
......
...@@ -59,8 +59,27 @@ class Milestone < ActiveRecord::Base ...@@ -59,8 +59,27 @@ class Milestone < ActiveRecord::Base
end end
end end
def self.reference_prefix
'%'
end
def self.reference_pattern def self.reference_pattern
nil # NOTE: The iid pattern only matches when all characters on the expression
# are digits, so it will match %2 but not %2.1 because that's probably a
# milestone name and we want it to be matched as such.
@reference_pattern ||= %r{
(#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}
(?:
(?<milestone_iid>
\d+(?!\S\w)\b # Integer-based milestone iid, or
) |
(?<milestone_name>
[^"\s]+\b | # String-based single-word milestone title, or
"[^"]+" # String-based multi-word milestone surrounded in quotes
)
)
}x
end end
def self.link_reference_pattern def self.link_reference_pattern
...@@ -81,13 +100,26 @@ class Milestone < ActiveRecord::Base ...@@ -81,13 +100,26 @@ class Milestone < ActiveRecord::Base
end end
end end
def to_reference(from_project = nil) ##
escaped_title = self.title.gsub("]", "\\]") # Returns the String necessary to reference this Milestone in Markdown
#
h = Gitlab::Routing.url_helpers # format - Symbol format to use (default: :iid, optional: :name)
url = h.namespace_project_milestone_url(self.project.namespace, self.project, self) #
# Examples:
#
# Milestone.first.to_reference # => "%1"
# Milestone.first.to_reference(format: :name) # => "%\"goal\""
# Milestone.first.to_reference(project) # => "gitlab-org/gitlab-ce%1"
#
def to_reference(from_project = nil, format: :iid)
format_reference = milestone_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}"
"[#{escaped_title}](#{url})" if cross_project_reference?(from_project)
project.to_reference + reference
else
reference
end
end end
def reference_link_text(from_project = nil) def reference_link_text(from_project = nil)
...@@ -159,4 +191,16 @@ class Milestone < ActiveRecord::Base ...@@ -159,4 +191,16 @@ class Milestone < ActiveRecord::Base
issues.where(id: ids). issues.where(id: ids).
update_all(["position = CASE #{conditions} ELSE position END", *pairs]) update_all(["position = CASE #{conditions} ELSE position END", *pairs])
end end
private
def milestone_format_reference(format = :iid)
raise ArgumentError, 'Unknown format' unless [:iid, :name].include?(format)
if format == :name && !name.include?('"')
%("#{name}")
else
iid
end
end
end end
...@@ -4,6 +4,10 @@ module Projects ...@@ -4,6 +4,10 @@ module Projects
@project.issues.visible_to_user(current_user).opened.select([:iid, :title]) @project.issues.visible_to_user(current_user).opened.select([:iid, :title])
end end
def milestones
@project.milestones.active.reorder(due_date: :asc, title: :asc).select([:iid, :title])
end
def merge_requests def merge_requests
@project.merge_requests.opened.select([:iid, :title]) @project.merge_requests.opened.select([:iid, :title])
end end
......
...@@ -185,20 +185,23 @@ GFM will turn that reference into a link so you can navigate between them easily ...@@ -185,20 +185,23 @@ GFM will turn that reference into a link so you can navigate between them easily
GFM will recognize the following: GFM will recognize the following:
| input | references | | input | references |
|:-----------------------|:---------------------------| |:-----------------------|:--------------------------- |
| `@user_name` | specific user | | `@user_name` | specific user |
| `@group_name` | specific group | | `@group_name` | specific group |
| `@all` | entire team | | `@all` | entire team |
| `#123` | issue | | `#123` | issue |
| `!123` | merge request | | `!123` | merge request |
| `$123` | snippet | | `$123` | snippet |
| `~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 |
| `9ba12248` | specific commit | | `%123` | milestone by ID |
| `9ba12248...b19a04f5` | commit range comparison | | `%v1.23` | one-word milestone by name |
| `[README](doc/README)` | repository file references | | `%"release candidate"` | multi-word milestone by name |
| `9ba12248` | specific commit |
| `9ba12248...b19a04f5` | commit range comparison |
| `[README](doc/README)` | repository file references |
GFM also recognizes certain cross-project references: GFM also recognizes certain cross-project references:
...@@ -206,6 +209,7 @@ GFM also recognizes certain cross-project references: ...@@ -206,6 +209,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` | 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 |
......
...@@ -10,11 +10,53 @@ module Banzai ...@@ -10,11 +10,53 @@ module Banzai
project.milestones.find_by(iid: id) project.milestones.find_by(iid: id)
end end
def url_for_object(issue, project) def references_in(text, pattern = Milestone.reference_pattern)
# We'll handle here the references that follow the `reference_pattern`.
# Other patterns (for example, the link pattern) are handled by the
# default implementation.
return super(text, pattern) if pattern != Milestone.reference_pattern
text.gsub(pattern) do |match|
milestone = find_milestone($~[:project], $~[:milestone_iid], $~[:milestone_name])
if milestone
yield match, milestone.iid, $~[:project], $~
else
match
end
end
end
def find_milestone(project_ref, milestone_id, milestone_name)
project = project_from_ref(project_ref)
return unless project
milestone_params = milestone_params(milestone_id, milestone_name)
project.milestones.find_by(milestone_params)
end
def milestone_params(iid, name)
if name
{ name: name.tr('"', '') }
else
{ iid: iid.to_i }
end
end
def url_for_object(milestone, project)
h = Gitlab::Routing.url_helpers h = Gitlab::Routing.url_helpers
h.namespace_project_milestone_url(project.namespace, project, milestone, h.namespace_project_milestone_url(project.namespace, project, milestone,
only_path: context[:only_path]) only_path: context[:only_path])
end end
def object_link_text(object, matches)
if context[:project] == object.project
super
else
"#{escape_once(super)} <i>in #{escape_once(object.project.path_with_namespace)}</i>".
html_safe
end
end
end end
end end
end end
...@@ -216,10 +216,14 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e ...@@ -216,10 +216,14 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
#### MilestoneReferenceFilter #### 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) %> - Milestone in another project: <%= xmilestone.to_reference(project) %>
- Ignored in code: `<%= milestone.to_reference %>` - Ignored in code: `<%= simple_milestone.to_reference %>`
- Link to milestone by URL: [Milestone](<%= urls.namespace_project_milestone_url(milestone.project.namespace, milestone.project, milestone) %>) - 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 ### Task Lists
......
...@@ -3,8 +3,9 @@ require 'spec_helper' ...@@ -3,8 +3,9 @@ require 'spec_helper'
describe Banzai::Filter::MilestoneReferenceFilter, lib: true do describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
include FilterSpecHelper include FilterSpecHelper
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
let(:milestone) { create(:milestone, project: project) } let(:milestone) { create(:milestone, project: project) }
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/)
...@@ -17,11 +18,42 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do ...@@ -17,11 +18,42 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
end end
end end
context 'internal reference' do it 'includes default classes' do
# Convert the Markdown link to only the URL, since these tests aren't run through the regular Markdown pipeline. doc = reference_filter("Milestone #{reference}")
# Milestone reference behavior in the full Markdown pipeline is tested elsewhere. expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
let(:reference) { milestone.to_reference.gsub(/\[([^\]]+)\]\(([^)]+)\)/, '\2') } 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 it 'links to a valid reference' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{reference}")
...@@ -30,29 +62,82 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do ...@@ -30,29 +62,82 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
end end
it 'links with adjacent text' do it 'links with adjacent text' do
doc = reference_filter("milestone (#{reference}.)") doc = reference_filter("Milestone (#{reference}.)")
expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(milestone.title)}<\/a>\.\)/) expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
end end
it 'includes a title attribute' do it 'ignores invalid milestone IIDs' do
doc = reference_filter("milestone #{reference}") exp = act = "Milestone #{invalidate_reference(reference)}"
expect(doc.css('a').first.attr('title')).to eq "Milestone: #{milestone.title}"
expect(reference_filter(act).to_html).to eq exp
end 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 it 'links to a valid reference' do
milestone.update_attribute(:title, %{"></a>whatever<a title="}) 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}") it 'links with adjacent text' do
expect(doc.text).to eq "milestone \">whatever" doc = reference_filter("Milestone (#{reference}.)")
expect(doc.to_html).to match(%r(\(<a.+>#{milestone.name}</a>\.\)))
end end
it 'includes default classes' do it 'ignores invalid milestone names' do
doc = reference_filter("milestone #{reference}") exp = act = "Milestone #{Milestone.reference_prefix}#{milestone.name.reverse}"
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone'
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 end
it 'includes a data-project attribute' do it 'includes a data-project attribute' do
doc = reference_filter("milestone #{reference}") doc = reference_filter("Milestone #{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')
...@@ -68,8 +153,34 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do ...@@ -68,8 +153,34 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do
end end
it 'adds to the results hash' do 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] expect(result[:references][:milestone]).to eq [milestone]
end end
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 end
...@@ -63,8 +63,12 @@ class MarkdownFeature ...@@ -63,8 +63,12 @@ class MarkdownFeature
@label ||= create(:label, name: 'awaiting feedback', project: project) @label ||= create(:label, name: 'awaiting feedback', project: project)
end end
def simple_milestone
@simple_milestone ||= create(:milestone, name: 'gfm-milestone', project: project)
end
def milestone def milestone
@milestone ||= create(:milestone, project: project) @milestone ||= create(:milestone, name: 'next goal', project: project)
end end
# Cross-references ----------------------------------------------------------- # Cross-references -----------------------------------------------------------
......
...@@ -154,7 +154,7 @@ module MarkdownMatchers ...@@ -154,7 +154,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: 3) expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6)
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