diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2018-03-30 19:27:03 +0200 |
---|---|---|
committer | Jan Provaznik <jprovaznik@gitlab.com> | 2018-04-04 21:56:39 +0200 |
commit | dbfc9cb6f355d1e1f14adf818647e07a5fa5868c (patch) | |
tree | 5f7ce77f9645d49db0291f33edb4109a82404dc8 | |
parent | 166b4575a6353668c894fea0ba234d0b371dee03 (diff) | |
download | gitlab-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.rb | 4 | ||||
-rw-r--r-- | app/services/notes/post_process_service.rb | 4 | ||||
-rw-r--r-- | app/services/system_note_service.rb | 4 | ||||
-rw-r--r-- | app/workers/new_note_worker.rb | 2 | ||||
-rw-r--r-- | lib/banzai/cross_project_reference.rb | 4 | ||||
-rw-r--r-- | lib/banzai/filter/abstract_reference_filter.rb | 20 | ||||
-rw-r--r-- | lib/banzai/filter/label_reference_filter.rb | 47 | ||||
-rw-r--r-- | spec/lib/banzai/cross_project_reference_spec.rb | 10 | ||||
-rw-r--r-- | spec/lib/banzai/filter/label_reference_filter_spec.rb | 21 |
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) |