Commit 99f08b3f authored by Douwe Maan's avatar Douwe Maan

Merge branch 'feature/cross-project-labels' into 'master'

Add support for cross project references for labels

## Summary

Support for cross project references for labels.

## Rationale

1.   Cross project label references are currently not supported in GitLab
1.   `to_reference` method signature in `Label` model breaks the abstraction introduced in `Referable`.

      `concerns/referable.rb:  def to_reference(_from_project = nil)`

      Signatures:

      ```
      label.rb:           def to_reference(format = :id)

      commit_range.rb:    def to_reference(from_project = nil)
      commit.rb:          def to_reference(from_project = nil)
      external_issue.rb:  def to_reference(_from_project = nil)
      group.rb:           def to_reference(_from_project = nil)
      issue.rb:           def to_reference(from_project = nil)
      merge_request.rb:   def to_reference(from_project = nil)
      milestone.rb:       def to_reference(from_project = nil)
      project.rb:         def to_reference(_from_project = nil)
      snippet.rb:         def to_reference(from_project = nil)
      user.rb:            def to_reference(_from_project = nil)
      ```

     This MR suggests using `def to_reference(from_project = nil, format: :id)` which makes use of keyword arguments and preserves abstract interface.

1.   We need support for cross project label references when we want to move issue to another project

     It may happen that issue description, system notes or comments contain reference to label and this reference will be invalid after moving issue to another project and will not be displayed correctly unless we have support for cross project references.

     Merge request that needs this feature: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2831


I think that cross project label references may be useful, (example: `Hey, see our issues for CI in GitLab CE! - gitab-org/gitlab-ce~"CI"`).

cc @JobV @DouweM @rspeicher 

See merge request !2966
parents eab7892d b3f533c3
...@@ -9,6 +9,7 @@ v 8.6.0 (unreleased) ...@@ -9,6 +9,7 @@ v 8.6.0 (unreleased)
- Indicate how much an MR diverged from the target branch (Pierre de La Morinerie) - Indicate how much an MR diverged from the target branch (Pierre de La Morinerie)
- Strip leading and trailing spaces in URL validator (evuez) - Strip leading and trailing spaces in URL validator (evuez)
- Return empty array instead of 404 when commit has no statuses in commit status API - Return empty array instead of 404 when commit has no statuses in commit status API
- Add support for cross-project label references
- Update documentation to reflect Guest role not being enforced on internal projects - Update documentation to reflect Guest role not being enforced on internal projects
- Allow search for logged out users - Allow search for logged out users
- Don't show Issues/MRs from archived projects in Groups view - Don't show Issues/MRs from archived projects in Groups view
......
...@@ -50,19 +50,25 @@ module LabelsHelper ...@@ -50,19 +50,25 @@ module LabelsHelper
@project.labels.pluck(:title) @project.labels.pluck(:title)
end end
def render_colored_label(label) def render_colored_label(label, label_suffix = '')
label_color = label.color || Label::DEFAULT_COLOR label_color = label.color || Label::DEFAULT_COLOR
text_color = text_color_for_bg(label_color) text_color = text_color_for_bg(label_color)
# Intentionally not using content_tag here so that this method can be called # Intentionally not using content_tag here so that this method can be called
# by LabelReferenceFilter # by LabelReferenceFilter
span = %(<span class="label color-label") + span = %(<span class="label color-label") +
%( style="background-color: #{label_color}; color: #{text_color}">) + %(style="background-color: #{label_color}; color: #{text_color}">) +
escape_once(label.name) + '</span>' %(#{escape_once(label.name)}#{label_suffix}</span>)
span.html_safe span.html_safe
end end
def render_colored_cross_project_label(label)
label_suffix = label.project.name_with_namespace
label_suffix = " <i>in #{escape_once(label_suffix)}</i>"
render_colored_label(label, label_suffix)
end
def suggested_colors def suggested_colors
[ [
'#0033CC', '#0033CC',
...@@ -119,5 +125,6 @@ module LabelsHelper ...@@ -119,5 +125,6 @@ module LabelsHelper
end end
# Required for Banzai::Filter::LabelReferenceFilter # Required for Banzai::Filter::LabelReferenceFilter
module_function :render_colored_label, :text_color_for_bg, :escape_once module_function :render_colored_label, :render_colored_cross_project_label,
:text_color_for_bg, :escape_once
end end
...@@ -48,10 +48,15 @@ class Label < ActiveRecord::Base ...@@ -48,10 +48,15 @@ class Label < ActiveRecord::Base
'~' '~'
end end
##
# Pattern used to extract label references from text # Pattern used to extract label references from text
#
# This pattern supports cross-project references.
#
def self.reference_pattern def self.reference_pattern
%r{ %r{
#{reference_prefix} (#{Project.reference_pattern})?
#{Regexp.escape(reference_prefix)}
(?: (?:
(?<label_id>\d+) | # Integer-based label ID, or (?<label_id>\d+) | # Integer-based label ID, or
(?<label_name> (?<label_name>
...@@ -62,24 +67,31 @@ class Label < ActiveRecord::Base ...@@ -62,24 +67,31 @@ class Label < ActiveRecord::Base
}x }x
end end
def self.link_reference_pattern
nil
end
##
# Returns the String necessary to reference this Label in Markdown # Returns the String necessary to reference this Label in Markdown
# #
# format - Symbol format to use (default: :id, optional: :name) # format - Symbol format to use (default: :id, optional: :name)
# #
# Note that its argument differs from other objects implementing Referable. If
# a non-Symbol argument is given (such as a Project), it will default to :id.
#
# Examples: # Examples:
# #
# Label.first.to_reference # => "~1" # Label.first.to_reference # => "~1"
# Label.first.to_reference(:name) # => "~\"bug\"" # Label.first.to_reference(format: :name) # => "~\"bug\""
# Label.first.to_reference(project) # => "gitlab-org/gitlab-ce~1"
# #
# Returns a String # Returns a String
def to_reference(format = :id) #
if format == :name && !name.include?('"') def to_reference(from_project = nil, format: :id)
%(#{self.class.reference_prefix}"#{name}") format_reference = label_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}"
if cross_project_reference?(from_project)
project.to_reference + reference
else else
"#{self.class.reference_prefix}#{id}" reference
end end
end end
...@@ -98,4 +110,16 @@ class Label < ActiveRecord::Base ...@@ -98,4 +110,16 @@ class Label < ActiveRecord::Base
def template? def template?
template template
end end
private
def label_format_reference(format = :id)
raise StandardError, 'Unknown format' unless [:id, :name].include?(format)
if format == :name && !name.include?('"')
%("#{name}")
else
id
end
end
end end
...@@ -66,7 +66,7 @@ class SystemNoteService ...@@ -66,7 +66,7 @@ class SystemNoteService
def self.change_label(noteable, project, author, added_labels, removed_labels) def self.change_label(noteable, project, author, added_labels, removed_labels)
labels_count = added_labels.count + removed_labels.count labels_count = added_labels.count + removed_labels.count
references = ->(label) { label.to_reference(:id) } references = ->(label) { label.to_reference(format: :id) }
added_labels = added_labels.map(&references).join(' ') added_labels = added_labels.map(&references).join(' ')
removed_labels = removed_labels.map(&references).join(' ') removed_labels = removed_labels.map(&references).join(' ')
......
...@@ -207,6 +207,7 @@ GFM also recognizes certain cross-project references: ...@@ -207,6 +207,7 @@ GFM also recognizes certain cross-project references:
| `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 |
| `namespace/project~"Some label"` | issues with given label |
## Task Lists ## Task Lists
......
...@@ -94,6 +94,8 @@ module Banzai ...@@ -94,6 +94,8 @@ module Banzai
object_link_filter(link, object_class.link_reference_pattern, link_text: text) object_link_filter(link, object_class.link_reference_pattern, link_text: text)
end end
end end
doc
end end
# Replace references (like `!123` for merge requests) in text with links # Replace references (like `!123` for merge requests) in text with links
......
module Banzai module Banzai
module Filter module Filter
# HTML filter that replaces label references with links. # HTML filter that replaces label references with links.
class LabelReferenceFilter < ReferenceFilter class LabelReferenceFilter < AbstractReferenceFilter
# Public: Find label references in text def self.object_class
# Label
# LabelReferenceFilter.references_in(text) do |match, id, name|
# "<a href=...>#{Label.find(id)}</a>"
# end
#
# text - String text to search.
#
# Yields the String match, an optional Integer label ID, and an optional
# String label name.
#
# Returns a String replaced with the return of the block.
def self.references_in(text)
text.gsub(Label.reference_pattern) do |match|
yield match, $~[:label_id].to_i, $~[:label_name]
end
end
def self.referenced_by(node)
{ label: LazyReference.new(Label, node.attr("data-label")) }
end end
def call def find_object(project, id)
replace_text_nodes_matching(Label.reference_pattern) do |content| project.labels.find(id)
label_link_filter(content)
end end
replace_link_nodes_with_href(Label.reference_pattern) do |link, text| def self.references_in(text, pattern = Label.reference_pattern)
label_link_filter(link, link_text: text) text.gsub(pattern) do |match|
yield match, $~[:label_id].to_i, $~[:label_name], $~[:project], $~
end end
end end
# Replace label references in text with links to the label specified. def references_in(text, pattern = Label.reference_pattern)
# text.gsub(pattern) do |match|
# text - String text to replace references in. project = project_from_ref($~[:project])
# params = label_params($~[:label_id].to_i, $~[:label_name])
# Returns a String with label references replaced with links. All links label = project.labels.find_by(params)
# have `gfm` and `gfm-label` class names attached for styling.
def label_link_filter(text, link_text: nil)
project = context[:project]
self.class.references_in(text) do |match, id, name|
params = label_params(id, name)
if label = project.labels.find_by(params)
url = url_for_label(project, label)
klass = reference_class(:label)
data = data_attribute(
original: link_text || match,
project: project.id,
label: label.id
)
text = link_text || render_colored_label(label) if label
yield match, label.id, $~[:project], $~
%(<a href="#{url}" #{data}
class="#{klass}">#{escape_once(text)}</a>)
else else
match match
end end
end end
end end
def url_for_label(project, label) def url_for_object(label, project)
h = Gitlab::Application.routes.url_helpers h = Gitlab::Application.routes.url_helpers
h.namespace_project_issues_url( project.namespace, project, label_name: label.name, h.namespace_project_issues_url(project.namespace, project, label_name: label.name,
only_path: context[:only_path]) only_path: context[:only_path])
end end
def render_colored_label(label) def object_link_text(object, matches)
LabelsHelper.render_colored_label(label) if context[:project] == object.project
LabelsHelper.render_colored_label(object)
else
LabelsHelper.render_colored_cross_project_label(object)
end
end end
# Parameters to pass to `Label.find_by` based on the given arguments # Parameters to pass to `Label.find_by` based on the given arguments
......
...@@ -209,7 +209,7 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e ...@@ -209,7 +209,7 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e
- Label by ID: <%= simple_label.to_reference %> - Label by ID: <%= simple_label.to_reference %>
- Label by name: <%= Label.reference_prefix %><%= simple_label.name %> - Label by name: <%= Label.reference_prefix %><%= simple_label.name %>
- Label by name in quotes: <%= label.to_reference(:name) %> - Label by name in quotes: <%= label.to_reference(format: :name) %>
- Ignored in code: `<%= simple_label.to_reference %>` - Ignored in code: `<%= simple_label.to_reference %>`
- Ignored in links: [Link to <%= simple_label.to_reference %>](#label-link) - Ignored in links: [Link to <%= simple_label.to_reference %>](#label-link)
- Link to label by reference: [Label](<%= label.to_reference %>) - Link to label by reference: [Label](<%= label.to_reference %>)
......
...@@ -111,7 +111,7 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do ...@@ -111,7 +111,7 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
context 'String-based multi-word references in quotes' do context 'String-based multi-word references in quotes' do
let(:label) { create(:label, name: 'gfm references', project: project) } let(:label) { create(:label, name: 'gfm references', project: project) }
let(:reference) { label.to_reference(:name) } let(:reference) { label.to_reference(format: :name) }
it 'links to a valid reference' do it 'links to a valid reference' do
doc = reference_filter("See #{reference}") doc = reference_filter("See #{reference}")
...@@ -176,4 +176,29 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do ...@@ -176,4 +176,29 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do
expect(result[:references][:label]).to eq [label] expect(result[:references][:label]).to eq [label]
end end
end end
describe 'cross project label references' do
let(:another_project) { create(:empty_project, :public) }
let(:project_name) { another_project.name_with_namespace }
let(:label) { create(:label, project: another_project, color: '#00ff00') }
let(:reference) { label.to_reference(project) }
let!(:result) { reference_filter("See #{reference}") }
it 'points to referenced project issues page' do
expect(result.css('a').first.attr('href'))
.to eq urls.namespace_project_issues_url(another_project.namespace,
another_project,
label_name: label.name)
end
it 'has valid color' do
expect(result.css('a span').first.attr('style'))
.to match /background-color: #00ff00/
end
it 'contains cross project content' do
expect(result.css('a').first.text).to eq "#{label.name} in #{project_name}"
end
end
end end
...@@ -59,18 +59,42 @@ describe Label, models: true do ...@@ -59,18 +59,42 @@ describe Label, models: true do
context 'using id' do context 'using id' do
it 'returns a String reference to the object' do it 'returns a String reference to the object' do
expect(label.to_reference).to eq "~#{label.id}" expect(label.to_reference).to eq "~#{label.id}"
expect(label.to_reference(double('project'))).to eq "~#{label.id}"
end end
end end
context 'using name' do context 'using name' do
it 'returns a String reference to the object' do it 'returns a String reference to the object' do
expect(label.to_reference(:name)).to eq %(~"#{label.name}") expect(label.to_reference(format: :name)).to eq %(~"#{label.name}")
end end
it 'uses id when name contains double quote' do it 'uses id when name contains double quote' do
label = create(:label, name: %q{"irony"}) label = create(:label, name: %q{"irony"})
expect(label.to_reference(:name)).to eq "~#{label.id}" expect(label.to_reference(format: :name)).to eq "~#{label.id}"
end
end
context 'using invalid format' do
it 'raises error' do
expect { label.to_reference(format: :invalid) }
.to raise_error StandardError, /Unknown format/
end
end
context 'cross project reference' do
let(:project) { create(:project) }
context 'using name' do
it 'returns cross reference with label name' do
expect(label.to_reference(project, format: :name))
.to eq %Q(#{label.project.to_reference}~"#{label.name}")
end
end
context 'using id' do
it 'returns cross reference with label id' do
expect(label.to_reference(project, format: :id))
.to eq %Q(#{label.project.to_reference}~#{label.id})
end
end end
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