summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2018-03-30 19:27:03 +0200
committerJan Provaznik <jprovaznik@gitlab.com>2018-04-04 21:56:39 +0200
commitdbfc9cb6f355d1e1f14adf818647e07a5fa5868c (patch)
tree5f7ce77f9645d49db0291f33edb4109a82404dc8
parent166b4575a6353668c894fea0ba234d0b371dee03 (diff)
downloadgitlab-ce-jprovazn-comment-refs.tar.gz
Better group support notes-related codejprovazn-comment-refs
Updates notes-related services and rendering so this code can be easily used for group-scoped resources (specifically Epics). Related to gitlab-ee!5205
-rw-r--r--app/models/note.rb4
-rw-r--r--app/services/notes/post_process_service.rb4
-rw-r--r--app/services/system_note_service.rb4
-rw-r--r--app/workers/new_note_worker.rb2
-rw-r--r--lib/banzai/cross_project_reference.rb4
-rw-r--r--lib/banzai/filter/abstract_reference_filter.rb20
-rw-r--r--lib/banzai/filter/label_reference_filter.rb47
-rw-r--r--spec/lib/banzai/cross_project_reference_spec.rb10
-rw-r--r--spec/lib/banzai/filter/label_reference_filter_spec.rb21
9 files changed, 92 insertions, 24 deletions
diff --git a/app/models/note.rb b/app/models/note.rb
index 0f5fb529a87..19bd41a34be 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -313,6 +313,10 @@ class Note < ActiveRecord::Base
!system? && !for_snippet?
end
+ def can_create_notification?
+ true
+ end
+
def discussion_class(noteable = nil)
# When commit notes are rendered on an MR's Discussion page, they are
# displayed in one discussion instead of individually.
diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb
index ad3dcc5010b..27ac68c3c82 100644
--- a/app/services/notes/post_process_service.rb
+++ b/app/services/notes/post_process_service.rb
@@ -11,7 +11,7 @@ module Notes
unless @note.system?
EventCreateService.new.leave_note(@note, @note.author)
- return unless @note.for_project_noteable?
+ return if @note.for_personal_snippet?
@note.create_cross_references!
execute_note_hooks
@@ -23,6 +23,8 @@ module Notes
end
def execute_note_hooks
+ return unless @note.project
+
note_data = hook_data
@note.project.execute_hooks(note_data, :note_hooks)
@note.project.execute_services(note_data, :note_hooks)
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 2253d638e93..00bf5434b7f 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -429,7 +429,7 @@ module SystemNoteService
def cross_reference(noteable, mentioner, author)
return if cross_reference_disallowed?(noteable, mentioner)
- gfm_reference = mentioner.gfm_reference(noteable.project)
+ gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group)
body = cross_reference_note_content(gfm_reference)
if noteable.is_a?(ExternalIssue)
@@ -582,7 +582,7 @@ module SystemNoteService
text = "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}"
notes.where('(note LIKE ? OR note LIKE ?)', text, text.capitalize)
else
- gfm_reference = mentioner.gfm_reference(noteable.project)
+ gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group)
text = cross_reference_note_content(gfm_reference)
notes.where(note: [text, text.capitalize])
end
diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb
index 67c54fbf10e..b925741934a 100644
--- a/app/workers/new_note_worker.rb
+++ b/app/workers/new_note_worker.rb
@@ -5,7 +5,7 @@ class NewNoteWorker
# old `NewNoteWorker` jobs (can remove later)
def perform(note_id, _params = {})
if note = Note.find_by(id: note_id)
- NotificationService.new.new_note(note)
+ NotificationService.new.new_note(note) if note.can_create_notification?
Notes::PostProcessService.new(note).execute
else
Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job")
diff --git a/lib/banzai/cross_project_reference.rb b/lib/banzai/cross_project_reference.rb
index d8fb7705b2a..3f1e95d4cc0 100644
--- a/lib/banzai/cross_project_reference.rb
+++ b/lib/banzai/cross_project_reference.rb
@@ -4,7 +4,7 @@ module Banzai
module CrossProjectReference
# Given a cross-project reference string, get the Project record
#
- # Defaults to value of `context[:project]` if:
+ # Defaults to value of `context[:project]`, or `context[:group]` if:
# * No reference is given OR
# * Reference given doesn't exist
#
@@ -12,7 +12,7 @@ module Banzai
#
# Returns a Project, or nil if the reference can't be found
def parent_from_ref(ref)
- return context[:project] unless ref
+ return context[:project] || context[:group] unless ref
Project.find_by_full_path(ref)
end
diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index c3a03f13306..6efaed7e624 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -196,13 +196,15 @@ module Banzai
end
end
- def data_attributes_for(text, project, object, link_content: false, link_reference: false)
+ def data_attributes_for(text, parent, object, link_content: false, link_reference: false)
+ object_parent_type = parent.is_a?(Group) ? :group : :project
+
data_attribute(
- original: text,
- link: link_content,
- link_reference: link_reference,
- project: project.id,
- object_sym => object.id
+ original: text,
+ link: link_content,
+ link_reference: link_reference,
+ object_parent_type => parent.id,
+ object_sym => object.id
)
end
@@ -337,6 +339,12 @@ module Banzai
def parent
parent_type == :project ? project : group
end
+
+ def full_group_path(group_ref)
+ return current_parent_path unless group_ref
+
+ group_ref
+ end
end
end
end
diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb
index faa5b344e6f..4efa3995e8b 100644
--- a/lib/banzai/filter/label_reference_filter.rb
+++ b/lib/banzai/filter/label_reference_filter.rb
@@ -32,16 +32,25 @@ module Banzai
end
end
- def find_label(project_ref, label_id, label_name)
- project = parent_from_ref(project_ref)
- return unless project
+ def find_label(parent_ref, label_id, label_name)
+ parent = parent_from_ref(parent_ref)
+ return unless parent
label_params = label_params(label_id, label_name)
- find_labels(project).find_by(label_params)
+ find_labels(parent).find_by(label_params)
end
- def find_labels(project)
- LabelsFinder.new(nil, project_id: project.id, include_ancestor_groups: true).execute(skip_authorization: true)
+ def find_labels(parent)
+ params = if parent.is_a?(Group)
+ { group_id: parent.id,
+ include_ancestor_groups: true,
+ only_group_labels: true }
+ else
+ { project_id: parent.id,
+ include_ancestor_groups: true }
+ end
+
+ LabelsFinder.new(nil, params).execute(skip_authorization: true)
end
# Parameters to pass to `Label.find_by` based on the given arguments
@@ -59,20 +68,34 @@ module Banzai
end
end
- def url_for_object(label, project)
+ def url_for_object(label, parent)
h = Gitlab::Routing.url_helpers
- h.project_issues_url(project, label_name: label.name, only_path: context[:only_path])
+
+ if parent.is_a?(Project)
+ h.project_issues_url(parent, label_name: label.name, only_path: context[:only_path])
+ elsif context[:label_url]
+ h.public_send(context[:label_url], parent, label_name: label.name, only_path: context[:only_path]) # rubocop:disable GitlabSecurity/PublicSend
+ end
end
def object_link_text(object, matches)
- project_path = full_project_path(matches[:namespace], matches[:project])
- project_from_ref = from_ref_cached(project_path)
- reference = project_from_ref.to_human_reference(project)
- label_suffix = " <i>in #{reference}</i>" if reference.present?
+ label_suffix = ''
+
+ if project || full_path_ref?(matches)
+ project_path = full_project_path(matches[:namespace], matches[:project])
+ parent_from_ref = from_ref_cached(project_path)
+ reference = parent_from_ref.to_human_reference(project || group)
+
+ label_suffix = " <i>in #{reference}</i>" if reference.present?
+ end
LabelsHelper.render_colored_label(object, label_suffix)
end
+ def full_path_ref?(matches)
+ matches[:namespace] && matches[:project]
+ end
+
def unescape_html_entities(text)
CGI.unescapeHTML(text.to_s)
end
diff --git a/spec/lib/banzai/cross_project_reference_spec.rb b/spec/lib/banzai/cross_project_reference_spec.rb
index 68ca960caab..aadfe7637dd 100644
--- a/spec/lib/banzai/cross_project_reference_spec.rb
+++ b/spec/lib/banzai/cross_project_reference_spec.rb
@@ -14,6 +14,16 @@ describe Banzai::CrossProjectReference do
end
end
+ context 'when no project was referenced in group context' do
+ it 'returns the group from context' do
+ group = double
+
+ allow(self).to receive(:context).and_return({ group: group })
+
+ expect(parent_from_ref(nil)).to eq group
+ end
+ end
+
context 'when referenced project does not exist' do
it 'returns nil' do
expect(parent_from_ref('invalid/reference')).to be_nil
diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb
index 0c524a1551f..81d5448673e 100644
--- a/spec/lib/banzai/filter/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb
@@ -596,6 +596,27 @@ describe Banzai::Filter::LabelReferenceFilter do
end
describe 'group context' do
+ it 'points to the page defined in label_url' do
+ group = create(:group)
+ label = create(:group_label, group: group)
+ reference = "~#{label.name}"
+
+ result = reference_filter("See #{reference}", { project: nil, group: group, label_url: :group_url } )
+
+ expect(result.css('a').first.attr('href')).to eq(urls.group_url(group, label_name: label.name))
+ end
+
+ it 'finds labels also in ancestor groups' do
+ group = create(:group)
+ label = create(:group_label, group: group)
+ subgroup = create(:group, parent: group)
+ reference = "~#{label.name}"
+
+ result = reference_filter("See #{reference}", { project: nil, group: subgroup, label_url: :group_url } )
+
+ expect(result.css('a').first.attr('href')).to eq(urls.group_url(subgroup, label_name: label.name))
+ end
+
it 'points to referenced project issues page' do
project = create(:project)
label = create(:label, project: project)