Read about our upcoming Code of Conduct on this issue

Commit b79f22c7 authored by Sean McGivern's avatar Sean McGivern Committed by Oswaldo Ferreira
Browse files

Merge branch '39461-notes-api-for-issues-no-longer-returns-label-additions-removals' into 'master'

Resolve "Notes API for issues no longer returns label additions/removals"

Closes #39461

See merge request gitlab-org/gitlab-ce!15080

(cherry picked from commit f562e69eab0d7bac8ea20d29c3df981289dd8cdb)

38ffd1bc7b87 fix the commit reference pattern from matching other entities
b44999acfa67 try to fix the failing spec on external refs
445273512a49 reverting to the simpler approach
0922444ffcff fix the linting error
8acad6cd0631 add the missing spec
bc6b9a624669 add `#with_metadata` scope to remove a N+1 from the notes' API
parent 8f7f0ce366e1
......@@ -110,6 +110,7 @@ def values
includes(:author, :noteable, :updated_by,
project: [:project_members, { group: [:group_members] }])
end
scope :with_metadata, -> { includes(:system_note_metadata) }
after_initialize :ensure_discussion_id
before_validation :nullify_blank_type, :nullify_blank_line_code
......@@ -169,7 +170,13 @@ def has_special_role?(role, note)
end
def cross_reference?
system? && matches_cross_reference_regex?
return unless system?
if force_cross_reference_regex_check?
matches_cross_reference_regex?
else
SystemNoteService.cross_reference?(note)
end
end
def diff_note?
......@@ -382,4 +389,10 @@ def ensure_discussion_id
def set_discussion_id
self.discussion_id ||= discussion_class.discussion_id(self)
end
def force_cross_reference_regex_check?
return unless system?
SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.include?(system_note_metadata&.action)
end
end
class SystemNoteMetadata < ActiveRecord::Base
# These notes's action text might contain a reference that is external.
# We should always force a deep validation upon references that are found
# in this note type.
# Other notes can always be safely shown as all its references are
# in the same project (i.e. with the same permissions)
TYPES_WITH_CROSS_REFERENCES = %w[
commit cross_reference
close duplicate
].freeze
ICON_TYPES = %w[
commit description merge confidential visible label assignee cross_reference
title time_tracking branch milestone discussion task moved
......
......@@ -583,6 +583,10 @@ def discussion_lock(issuable, author)
create_note(NoteSummary.new(issuable, issuable.project, author, body, action: action))
end
def cross_reference?(note_text)
note_text =~ /\A#{cross_reference_note_prefix}/i
end
private
def notes_for_mentioner(mentioner, noteable, notes)
......
---
title: Label addition/removal are not going to be redacted wrongfully in the API.
merge_request: 15080
author:
type: fixed
......@@ -33,7 +33,7 @@ class Notes < Grape::API
# paginate() only works with a relation. This could lead to a
# mismatch between the pagination headers info and the actual notes
# array returned, but this is really a edge-case.
paginate(noteable.notes)
paginate(noteable.notes.with_metadata)
.reject { |n| n.cross_reference_not_visible_for?(current_user) }
present notes, with: Entities::Note
else
......@@ -50,7 +50,7 @@ class Notes < Grape::API
end
get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = find_project_noteable(noteables_str, params[:noteable_id])
note = noteable.notes.find(params[:note_id])
note = noteable.notes.with_metadata.find(params[:note_id])
can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
if can_read_note
......
......@@ -231,6 +231,37 @@
end
end
describe '#cross_reference?' do
it 'falsey for user-generated notes' do
note = create(:note, system: false)
expect(note.cross_reference?).to be_falsy
end
context 'when the note might contain cross references' do
SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.each do |type|
let(:note) { create(:note, :system) }
let!(:metadata) { create(:system_note_metadata, note: note, action: type) }
it 'delegates to the cross-reference regex' do
expect(note).to receive(:matches_cross_reference_regex?).and_return(false)
note.cross_reference?
end
end
end
context 'when the note cannot contain cross references' do
let(:commit_note) { build(:note, note: 'mentioned in 1312312313 something else.', system: true) }
let(:label_note) { build(:note, note: 'added ~2323232323', system: true) }
it 'scan for a `mentioned in` prefix' do
expect(commit_note.cross_reference?).to be_truthy
expect(label_note.cross_reference?).to be_falsy
end
end
end
describe 'clear_blank_line_code!' do
it 'clears a blank line code before validation' do
note = build(:note, line_code: ' ')
......
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