From 077f9a4eeef3c64c5f3e9cc5df5442c8817ee1d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 30 Mar 2016 23:12:34 -0300 Subject: [PATCH 01/15] Implementing special GitLab markdown reference for milestones Using the syntax proposed in #13829 [project_reference]%(milestone_id | milestone_name) to get a link to the referred milestone. --- app/models/milestone.rb | 42 +++++++++++++++---- .../filter/milestone_reference_filter.rb | 26 +++++++++++- spec/fixtures/markdown.md.erb | 9 ++-- spec/support/markdown_feature.rb | 6 ++- spec/support/matchers/markdown_matchers.rb | 2 +- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 986184dd301..39dc8d89614 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -74,8 +74,22 @@ class Milestone < ActiveRecord::Base end end + def self.reference_prefix + '%' + end + def self.reference_pattern - nil + %r{ + (#{Project.reference_pattern})? + #{Regexp.escape(reference_prefix)} + (?: + (?\d+) | # Integer-based milestone ID, or + (? + [A-Za-z0-9_-]+ | # String-based single-word milestone title, or + "[^"]+" # String-based multi-word milestone surrounded in quotes + ) + ) + }x end def self.link_reference_pattern @@ -86,13 +100,15 @@ class Milestone < ActiveRecord::Base self.where('due_date > ?', Time.now).reorder(due_date: :asc).first end - def to_reference(from_project = nil) - escaped_title = self.title.gsub("]", "\\]") + def to_reference(from_project = nil, format: :id) + format_reference = milestone_format_reference(format) + reference = "#{self.class.reference_prefix}#{format_reference}" - h = Gitlab::Routing.url_helpers - url = h.namespace_project_milestone_url(self.project.namespace, self.project, self) - - "[#{escaped_title}](#{url})" + if cross_project_reference?(from_project) + project.to_reference + reference + else + reference + end end def reference_link_text(from_project = nil) @@ -160,4 +176,16 @@ class Milestone < ActiveRecord::Base issues.where(id: ids). update_all(["position = CASE #{conditions} ELSE position END", *pairs]) end + + private + + def milestone_format_reference(format = :id) + raise StandardError, 'Unknown format' unless [:id, :name].include?(format) + + if format == :name && !name.include?('"') + %("#{name}") + else + id + end + end end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 4cb82178024..2c90fd4d385 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -7,14 +7,36 @@ module Banzai end def find_object(project, id) - project.milestones.find_by(iid: id) + project.milestones.find(id) end - def url_for_object(issue, project) + def references_in(text, pattern = Milestone.reference_pattern) + text.gsub(pattern) do |match| + project = project_from_ref($~[:project]) + params = milestone_params($~[:milestone_id].to_i, $~[:milestone_name]) + milestone = project.milestones.find_by(params) + + if milestone + yield match, milestone.id, $~[:project], $~ + else + match + end + end + end + + def url_for_object(milestone, project) h = Gitlab::Routing.url_helpers h.namespace_project_milestone_url(project.namespace, project, milestone, only_path: context[:only_path]) end + + def milestone_params(id, name) + if name + { name: name.tr('"', '') } + else + { id: id } + end + end end end end diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index 1772cc3f6a4..6d3bf810c2c 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -216,10 +216,13 @@ 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) +- Link to milestone by URL: [Milestone](<%= milestone.to_reference %>) ### Task Lists 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..492138716af 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: 5) end end From 375e83bb57dc0143691cf6ef7277bec494f060f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 31 Mar 2016 21:54:00 -0300 Subject: [PATCH 02/15] Consistently using iid when treating milestones as referrables Also, addint a suffix to the reference text when the milestone is in another project --- app/models/milestone.rb | 25 ++- .../filter/milestone_reference_filter.rb | 19 ++- .../filter/milestone_reference_filter_spec.rb | 159 +++++++++++++++--- 3 files changed, 164 insertions(+), 39 deletions(-) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 39dc8d89614..50fa95d4d4b 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -83,10 +83,10 @@ class Milestone < ActiveRecord::Base (#{Project.reference_pattern})? #{Regexp.escape(reference_prefix)} (?: - (?\d+) | # Integer-based milestone ID, or + (?\d+) | # Integer-based milestone iid, or (? - [A-Za-z0-9_-]+ | # String-based single-word milestone title, or - "[^"]+" # String-based multi-word milestone surrounded in quotes + [A-Za-z0-9_-]+ | # String-based single-word milestone title, or + "[^"]+" # String-based multi-word milestone surrounded in quotes ) ) }x @@ -100,7 +100,18 @@ class Milestone < ActiveRecord::Base self.where('due_date > ?', Time.now).reorder(due_date: :asc).first end - def to_reference(from_project = nil, format: :id) + ## + # Returns the String necessary to reference this Milestone in Markdown + # + # format - Symbol format to use (default: :iid, optional: :name) + # + # 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}" @@ -179,13 +190,13 @@ class Milestone < ActiveRecord::Base private - def milestone_format_reference(format = :id) - raise StandardError, 'Unknown format' unless [:id, :name].include?(format) + def milestone_format_reference(format = :iid) + raise StandardError, 'Unknown format' unless [:iid, :name].include?(format) if format == :name && !name.include?('"') %("#{name}") else - id + iid end end end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 2c90fd4d385..419532717f2 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -7,17 +7,17 @@ module Banzai end def find_object(project, id) - project.milestones.find(id) + project.milestones.find_by(iid: id) end def references_in(text, pattern = Milestone.reference_pattern) text.gsub(pattern) do |match| project = project_from_ref($~[:project]) - params = milestone_params($~[:milestone_id].to_i, $~[:milestone_name]) + params = milestone_params($~[:milestone_iid].to_i, $~[:milestone_name]) milestone = project.milestones.find_by(params) if milestone - yield match, milestone.id, $~[:project], $~ + yield match, milestone.iid, $~[:project], $~ else match end @@ -30,11 +30,20 @@ module Banzai only_path: context[:only_path]) end - def milestone_params(id, name) + def object_link_text(object, matches) + if context[:project] == object.project + super + else + "#{super} in #{escape_once(object.project.name_with_namespace)}". + html_safe + end + end + + def milestone_params(iid, name) if name { name: name.tr('"', '') } else - { id: id } + { iid: iid } end end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index ebf3d7489b5..26f87286b2c 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,10 +18,111 @@ 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}") + + 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(\(#{milestone.name}\.\))) + end + + 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 '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 + + it 'links with adjacent text' do + doc = reference_filter("Milestone (#{reference}.)") + expect(doc.to_html).to match(%r(\(#{milestone.name}\.\))) + 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 + + 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(\(#{milestone.name}\.\))) + 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{Milestone} } it 'links to a valid reference' do doc = reference_filter("See #{reference}") @@ -30,29 +132,12 @@ 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(/\(#{Regexp.escape(milestone.title)}<\/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}" - end - - it 'escapes the title attribute' do - milestone.update_attribute(:title, %{">whateverMilestone\.\))) 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,28 @@ 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_name) { another_project.name_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_name}" + end + end end From 6d9794d42a7bea1150374c76fd3ce5521a44e58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 4 Apr 2016 22:20:10 -0300 Subject: [PATCH 03/15] Transforming milestones link references to the short reference form --- lib/banzai/filter/milestone_reference_filter.rb | 5 +++++ spec/fixtures/markdown.md.erb | 1 + spec/support/matchers/markdown_matchers.rb | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 419532717f2..556087c4880 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -11,6 +11,11 @@ module Banzai end 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| project = project_from_ref($~[:project]) params = milestone_params($~[:milestone_iid].to_i, $~[:milestone_name]) diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index 6d3bf810c2c..3e777a5e92b 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -222,6 +222,7 @@ 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.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/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index 492138716af..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: 5) + expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6) end end From 1ff896f2bf5d06d0d772fd0df98bf43edf107373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 4 Apr 2016 23:09:44 -0300 Subject: [PATCH 04/15] Escaping the `object_link_text` on cross project milestone references --- lib/banzai/filter/milestone_reference_filter.rb | 2 +- spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 556087c4880..aea1abf3b8e 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -39,7 +39,7 @@ module Banzai if context[:project] == object.project super else - "#{super} in #{escape_once(object.project.name_with_namespace)}". + "#{escape_once(super)} in #{escape_once(object.project.name_with_namespace)}". html_safe end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 26f87286b2c..ac3e6e4e536 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -176,5 +176,11 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do it 'contains cross project content' do expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_name}" end + + it 'escapes the name attribute' do + allow_any_instance_of(Milestone).to receive(:title).and_return(%{">whatever Date: Mon, 4 Apr 2016 23:37:50 -0300 Subject: [PATCH 05/15] Implementing autocomplete for GFM milestone references --- .../javascripts/gfm_auto_complete.js.coffee | 19 +++++++++++++++++++ app/controllers/projects_controller.rb | 1 + app/services/projects/autocomplete_service.rb | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/app/assets/javascripts/gfm_auto_complete.js.coffee b/app/assets/javascripts/gfm_auto_complete.js.coffee index 61e3f811e73..54d89ef69a1 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.coffee +++ b/app/assets/javascripts/gfm_auto_complete.js.coffee @@ -18,6 +18,10 @@ GitLab.GfmAutoComplete = Issues: template: '
  • ${id} ${title}
    • ' + # Milestones + Milestones: + template: '
  • ${title}
    • ' + # Add GFM auto-completion to all input fields, that accept GFM input. setup: (wrap) -> @input = $('.js-gfm-input') @@ -81,6 +85,19 @@ GitLab.GfmAutoComplete = title: sanitize(i.title) search: "#{i.iid} #{i.title}" + @input.atwho + at: '%' + alias: 'milestones' + searchKey: 'search' + displayTpl: @Milestones.template + insertTpl: '${atwho-at}${id}' + callbacks: + beforeSave: (milestones) -> + $.map milestones, (m) -> + id: m.iid + title: sanitize(m.title) + search: "#{m.title}" + @input.atwho at: '!' alias: 'mergerequests' @@ -105,6 +122,8 @@ GitLab.GfmAutoComplete = @input.atwho 'load', '@', data.members # load issues @input.atwho 'load', 'issues', data.issues + # load milestones + @input.atwho 'load', 'milestones', data.milestones # load merge requests @input.atwho 'load', 'mergerequests', data.mergerequests # load emojis diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3768efe142a..8662de712a1 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -147,6 +147,7 @@ class ProjectsController < Projects::ApplicationController @suggestions = { emojis: AwardEmoji.urls, issues: autocomplete.issues, + milestones: autocomplete.milestones, mergerequests: autocomplete.merge_requests, members: participants } diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index ba50305dbd5..eec38c5c3d8 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -4,6 +4,10 @@ module Projects @project.issues.visible_to_user(current_user).opened.select([:iid, :title]) end + def milestones + @project.milestones.active.select([:iid, :title]) + end + def merge_requests @project.merge_requests.opened.select([:iid, :title]) end From 0ba116a58ed57d760b264c39f241798528f54b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 5 Apr 2016 21:35:43 -0300 Subject: [PATCH 06/15] Matching version-like expressions as `milestone_name`s instead of `milestone_iid`s The changes also account for %2.1. being matched as milestone_name = "2.1" without the word-separating dot. --- app/models/milestone.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 50fa95d4d4b..92c07fd20da 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -79,14 +79,19 @@ class Milestone < ActiveRecord::Base end def self.reference_pattern + # 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. %r{ (#{Project.reference_pattern})? #{Regexp.escape(reference_prefix)} (?: - (?\d+) | # Integer-based milestone iid, or + (? + \d+(?!\S\w)\b # Integer-based milestone iid, or + ) | (? - [A-Za-z0-9_-]+ | # String-based single-word milestone title, or - "[^"]+" # String-based multi-word milestone surrounded in quotes + [^"\s]+\b | # String-based single-word milestone title, or + "[^"]+" # String-based multi-word milestone surrounded in quotes ) ) }x From 4596190a2d1eb6d575b52ff889a20c49fbd8ca2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 5 Apr 2016 21:40:40 -0300 Subject: [PATCH 07/15] Inserting Milestone titles insted of IIDs with GFM auto complete --- app/assets/javascripts/gfm_auto_complete.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/gfm_auto_complete.js.coffee b/app/assets/javascripts/gfm_auto_complete.js.coffee index 54d89ef69a1..0f2c5a6241c 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.coffee +++ b/app/assets/javascripts/gfm_auto_complete.js.coffee @@ -90,7 +90,7 @@ GitLab.GfmAutoComplete = alias: 'milestones' searchKey: 'search' displayTpl: @Milestones.template - insertTpl: '${atwho-at}${id}' + insertTpl: '${atwho-at}${title}' callbacks: beforeSave: (milestones) -> $.map milestones, (m) -> From ec71edfeddc403df5dcff1300e3f4868554c5f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 5 Apr 2016 21:43:26 -0300 Subject: [PATCH 08/15] Sorting Milestones on the auto complete list by due date and title --- app/services/projects/autocomplete_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index eec38c5c3d8..eb73948006e 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -5,7 +5,7 @@ module Projects end def milestones - @project.milestones.active.select([:iid, :title]) + @project.milestones.active.reorder(due_date: :asc, title: :asc).select([:iid, :title]) end def merge_requests From 0f925714d04a4d2e86db3a752fc8c1fc45da2214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 6 Apr 2016 20:35:02 -0300 Subject: [PATCH 09/15] Inserting the Milestone title between quotes on GFM auto complete This is due to the fact that for multiple word titles it might be an invalid reference without the quotes --- app/assets/javascripts/gfm_auto_complete.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/gfm_auto_complete.js.coffee b/app/assets/javascripts/gfm_auto_complete.js.coffee index 0f2c5a6241c..41dba342107 100644 --- a/app/assets/javascripts/gfm_auto_complete.js.coffee +++ b/app/assets/javascripts/gfm_auto_complete.js.coffee @@ -90,7 +90,7 @@ GitLab.GfmAutoComplete = alias: 'milestones' searchKey: 'search' displayTpl: @Milestones.template - insertTpl: '${atwho-at}${title}' + insertTpl: '${atwho-at}"${title}"' callbacks: beforeSave: (milestones) -> $.map milestones, (m) -> From 30d1d47d1da729319a3e71bd5599c473fc926565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Wed, 6 Apr 2016 21:37:56 -0300 Subject: [PATCH 10/15] Using project `path_with_namespace` in milestone's cross project references link text --- lib/banzai/filter/milestone_reference_filter.rb | 2 +- spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index aea1abf3b8e..746e768061c 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -39,7 +39,7 @@ module Banzai if context[:project] == object.project super else - "#{escape_once(super)} in #{escape_once(object.project.name_with_namespace)}". + "#{escape_once(super)} in #{escape_once(object.project.path_with_namespace)}". html_safe end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index ac3e6e4e536..bdf48eabb0e 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -160,7 +160,7 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do describe 'cross project milestone references' do let(:another_project) { create(:empty_project, :public) } - let(:project_name) { another_project.name_with_namespace } + let(:project_path) { another_project.path_with_namespace } let(:milestone) { create(:milestone, project: another_project) } let(:reference) { milestone.to_reference(project) } @@ -174,13 +174,13 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do end it 'contains cross project content' do - expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_name}" + 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(%{">    whatever Date: Fri, 8 Apr 2016 23:03:23 -0300 Subject: [PATCH 11/15] Include Milestone reference syntax in Markdown documentation --- doc/markdown/markdown.md | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md index 4f199b6af6f..1afa1f14067 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -185,20 +185,23 @@ GFM will turn that reference into a link so you can navigate between them easily GFM will recognize the following: -| input | references | -|:-----------------------|:---------------------------| -| `@user_name` | specific user | -| `@group_name` | specific group | -| `@all` | entire team | -| `#123` | issue | -| `!123` | merge request | -| `$123` | snippet | -| `~123` | label by ID | -| `~bug` | one-word label by name | -| `~"feature request"` | multi-word label by name | -| `9ba12248` | specific commit | -| `9ba12248...b19a04f5` | commit range comparison | -| `[README](doc/README)` | repository file references | +| input | references | +|:-----------------------|:--------------------------- | +| `@user_name` | specific user | +| `@group_name` | specific group | +| `@all` | entire team | +| `#123` | issue | +| `!123` | merge request | +| `$123` | snippet | +| `~123` | label by ID | +| `~bug` | one-word label by name | +| `~"feature request"` | multi-word label by name | +| `%123` | milestone by ID | +| `%v1.23` | one-word milestone by name | +| `%"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: @@ -206,6 +209,7 @@ GFM also recognizes certain cross-project references: |:----------------------------------------|:------------------------| | `namespace/project#123` | issue | | `namespace/project!123` | merge request | +| `namespace/project%123` | milestone | | `namespace/project$123` | snippet | | `namespace/project@9ba12248` | specific commit | | `namespace/project@9ba12248...b19a04f5` | commit range comparison | From 7910853368970292eb243ee34072c7f527fa67f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 19 Apr 2016 22:20:43 -0300 Subject: [PATCH 12/15] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index d4b8a509261..b35cd9585dc 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -112,6 +112,7 @@ v 8.7.0 (unreleased) - Fix GitHub project's link in the import page when provider has a custom URL - Add RAW build trace output and button on build page - Add incremental build trace update into CI API + - Implement GFM references for milestones (Alejandro Rodríguez) v 8.6.7 - Fix persistent XSS vulnerability in `commit_person_link` helper From ab1734f9e1e3f07482185c8a4cb168be463fcff5 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 23 Apr 2016 12:27:29 +0200 Subject: [PATCH 13/15] Move changelog item --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index f81435805d3..b1df9145d93 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) + - Implement GFM references for milestones (Alejandro Rodríguez) v 8.7.1 (unreleased) - Use the `can?` helper instead of `current_user.can?` @@ -121,7 +122,6 @@ v 8.7.0 - Fix GitHub project's link in the import page when provider has a custom URL - Add RAW build trace output and button on build page - Add incremental build trace update into CI API - - Implement GFM references for milestones (Alejandro Rodríguez) v 8.6.7 - Fix persistent XSS vulnerability in `commit_person_link` helper From 715959e58190eca661ea377b949af3515d8da913 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Sat, 23 Apr 2016 12:34:09 +0200 Subject: [PATCH 14/15] Fix cross-project milestone ref with invalid project --- .../filter/milestone_reference_filter.rb | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 746e768061c..dad0768f51b 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -17,9 +17,7 @@ module Banzai return super(text, pattern) if pattern != Milestone.reference_pattern text.gsub(pattern) do |match| - project = project_from_ref($~[:project]) - params = milestone_params($~[:milestone_iid].to_i, $~[:milestone_name]) - milestone = project.milestones.find_by(params) + milestone = find_milestone($~[:project], $~[:milestone_iid], $~[:milestone_name]) if milestone yield match, milestone.iid, $~[:project], $~ @@ -29,6 +27,22 @@ module Banzai 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.namespace_project_milestone_url(project.namespace, project, milestone, @@ -43,14 +57,6 @@ module Banzai html_safe end end - - def milestone_params(iid, name) - if name - { name: name.tr('"', '') } - else - { iid: iid } - end - end end end end From 129bb6c2a71c8499daeb5d55f657b0eda8366bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 18 May 2016 23:45:25 -0500 Subject: [PATCH 15/15] Address Yorick's feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 2 -- app/models/milestone.rb | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b00cb9064a7..3466b98c4c2 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,8 +2,6 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) - Implement GFM references for milestones (Alejandro Rodríguez) - -v 8.7.1 (unreleased) - Snippets tab under user profile. !4001 (Long Nguyen) - Fix error when using link to uploads in global snippets - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 31a54f44453..e0c8454a998 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -67,7 +67,7 @@ class Milestone < ActiveRecord::Base # 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. - %r{ + @reference_pattern ||= %r{ (#{Project.reference_pattern})? #{Regexp.escape(reference_prefix)} (?: @@ -195,7 +195,7 @@ class Milestone < ActiveRecord::Base private def milestone_format_reference(format = :iid) - raise StandardError, 'Unknown format' unless [:iid, :name].include?(format) + raise ArgumentError, 'Unknown format' unless [:iid, :name].include?(format) if format == :name && !name.include?('"') %("#{name}")