Commit c1946364 authored by Jan Provaznik's avatar Jan Provaznik

Better group support notes-related code

Updates notes-related services and rendering so this code can be
easily used for group-scoped resources (specifically Epics).

Related to gitlab-ee!5205
parent 166b4575
...@@ -313,6 +313,10 @@ class Note < ActiveRecord::Base ...@@ -313,6 +313,10 @@ class Note < ActiveRecord::Base
!system? && !for_snippet? !system? && !for_snippet?
end end
def can_create_notification?
true
end
def discussion_class(noteable = nil) def discussion_class(noteable = nil)
# When commit notes are rendered on an MR's Discussion page, they are # When commit notes are rendered on an MR's Discussion page, they are
# displayed in one discussion instead of individually. # displayed in one discussion instead of individually.
......
...@@ -11,7 +11,7 @@ module Notes ...@@ -11,7 +11,7 @@ module Notes
unless @note.system? unless @note.system?
EventCreateService.new.leave_note(@note, @note.author) EventCreateService.new.leave_note(@note, @note.author)
return unless @note.for_project_noteable? return if @note.for_personal_snippet?
@note.create_cross_references! @note.create_cross_references!
execute_note_hooks execute_note_hooks
...@@ -23,6 +23,8 @@ module Notes ...@@ -23,6 +23,8 @@ module Notes
end end
def execute_note_hooks def execute_note_hooks
return unless @note.project
note_data = hook_data note_data = hook_data
@note.project.execute_hooks(note_data, :note_hooks) @note.project.execute_hooks(note_data, :note_hooks)
@note.project.execute_services(note_data, :note_hooks) @note.project.execute_services(note_data, :note_hooks)
......
...@@ -429,7 +429,7 @@ module SystemNoteService ...@@ -429,7 +429,7 @@ module SystemNoteService
def cross_reference(noteable, mentioner, author) def cross_reference(noteable, mentioner, author)
return if cross_reference_disallowed?(noteable, mentioner) 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) body = cross_reference_note_content(gfm_reference)
if noteable.is_a?(ExternalIssue) if noteable.is_a?(ExternalIssue)
...@@ -582,7 +582,7 @@ module SystemNoteService ...@@ -582,7 +582,7 @@ module SystemNoteService
text = "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}" text = "#{cross_reference_note_prefix}%#{mentioner.to_reference(nil)}"
notes.where('(note LIKE ? OR note LIKE ?)', text, text.capitalize) notes.where('(note LIKE ? OR note LIKE ?)', text, text.capitalize)
else else
gfm_reference = mentioner.gfm_reference(noteable.project) gfm_reference = mentioner.gfm_reference(noteable.project || noteable.group)
text = cross_reference_note_content(gfm_reference) text = cross_reference_note_content(gfm_reference)
notes.where(note: [text, text.capitalize]) notes.where(note: [text, text.capitalize])
end end
......
...@@ -5,7 +5,7 @@ class NewNoteWorker ...@@ -5,7 +5,7 @@ class NewNoteWorker
# old `NewNoteWorker` jobs (can remove later) # old `NewNoteWorker` jobs (can remove later)
def perform(note_id, _params = {}) def perform(note_id, _params = {})
if note = Note.find_by(id: note_id) 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 Notes::PostProcessService.new(note).execute
else else
Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job")
......
...@@ -4,7 +4,7 @@ module Banzai ...@@ -4,7 +4,7 @@ module Banzai
module CrossProjectReference module CrossProjectReference
# Given a cross-project reference string, get the Project record # 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 # * No reference is given OR
# * Reference given doesn't exist # * Reference given doesn't exist
# #
...@@ -12,7 +12,7 @@ module Banzai ...@@ -12,7 +12,7 @@ module Banzai
# #
# Returns a Project, or nil if the reference can't be found # Returns a Project, or nil if the reference can't be found
def parent_from_ref(ref) def parent_from_ref(ref)
return context[:project] unless ref return context[:project] || context[:group] unless ref
Project.find_by_full_path(ref) Project.find_by_full_path(ref)
end end
......
...@@ -196,12 +196,14 @@ module Banzai ...@@ -196,12 +196,14 @@ module Banzai
end end
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( data_attribute(
original: text, original: text,
link: link_content, link: link_content,
link_reference: link_reference, link_reference: link_reference,
project: project.id, object_parent_type => parent.id,
object_sym => object.id object_sym => object.id
) )
end end
...@@ -337,6 +339,12 @@ module Banzai ...@@ -337,6 +339,12 @@ module Banzai
def parent def parent
parent_type == :project ? project : group parent_type == :project ? project : group
end end
def full_group_path(group_ref)
return current_parent_path unless group_ref
group_ref
end
end end
end end
end end
...@@ -32,16 +32,25 @@ module Banzai ...@@ -32,16 +32,25 @@ module Banzai
end end
end end
def find_label(project_ref, label_id, label_name) def find_label(parent_ref, label_id, label_name)
project = parent_from_ref(project_ref) parent = parent_from_ref(parent_ref)
return unless project return unless parent
label_params = label_params(label_id, label_name) label_params = label_params(label_id, label_name)
find_labels(project).find_by(label_params) find_labels(parent).find_by(label_params)
end end
def find_labels(project) def find_labels(parent)
LabelsFinder.new(nil, project_id: project.id, include_ancestor_groups: true).execute(skip_authorization: true) 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 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
...@@ -59,20 +68,34 @@ module Banzai ...@@ -59,20 +68,34 @@ module Banzai
end end
end end
def url_for_object(label, project) def url_for_object(label, parent)
h = Gitlab::Routing.url_helpers 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_method]
h.public_send(context[:label_url_method], parent, label_name: label.name, only_path: context[:only_path]) # rubocop:disable GitlabSecurity/PublicSend
end
end end
def object_link_text(object, matches) def object_link_text(object, matches)
label_suffix = ''
if project || full_path_ref?(matches)
project_path = full_project_path(matches[:namespace], matches[:project]) project_path = full_project_path(matches[:namespace], matches[:project])
project_from_ref = from_ref_cached(project_path) parent_from_ref = from_ref_cached(project_path)
reference = project_from_ref.to_human_reference(project) reference = parent_from_ref.to_human_reference(project || group)
label_suffix = " <i>in #{reference}</i>" if reference.present? label_suffix = " <i>in #{reference}</i>" if reference.present?
end
LabelsHelper.render_colored_label(object, label_suffix) LabelsHelper.render_colored_label(object, label_suffix)
end end
def full_path_ref?(matches)
matches[:namespace] && matches[:project]
end
def unescape_html_entities(text) def unescape_html_entities(text)
CGI.unescapeHTML(text.to_s) CGI.unescapeHTML(text.to_s)
end end
......
...@@ -14,6 +14,16 @@ describe Banzai::CrossProjectReference do ...@@ -14,6 +14,16 @@ describe Banzai::CrossProjectReference do
end end
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 context 'when referenced project does not exist' do
it 'returns nil' do it 'returns nil' do
expect(parent_from_ref('invalid/reference')).to be_nil expect(parent_from_ref('invalid/reference')).to be_nil
......
...@@ -596,6 +596,27 @@ describe Banzai::Filter::LabelReferenceFilter do ...@@ -596,6 +596,27 @@ describe Banzai::Filter::LabelReferenceFilter do
end end
describe 'group context' do describe 'group context' do
it 'points to the page defined in label_url_method' do
group = create(:group)
label = create(:group_label, group: group)
reference = "~#{label.name}"
result = reference_filter("See #{reference}", { project: nil, group: group, label_url_method: :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_method: :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 it 'points to referenced project issues page' do
project = create(:project) project = create(:project)
label = create(:label, project: project) label = create(:label, project: project)
...@@ -604,6 +625,7 @@ describe Banzai::Filter::LabelReferenceFilter do ...@@ -604,6 +625,7 @@ describe Banzai::Filter::LabelReferenceFilter do
result = reference_filter("See #{reference}", { project: nil, group: create(:group) } ) result = reference_filter("See #{reference}", { project: nil, group: create(:group) } )
expect(result.css('a').first.attr('href')).to eq(urls.project_issues_url(project, label_name: label.name)) expect(result.css('a').first.attr('href')).to eq(urls.project_issues_url(project, label_name: label.name))
expect(result.css('a').first.text).to eq "#{label.name} in #{project.full_name}"
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