Fix system note permissions check
What does this MR do and why?
Fixes Note#all_referenced_mentionables_allowed?
to account for cases when the note contains a reference to a non-existing resource.
For this edge case refs.all_visible?
returns true but refs.all.any?
would return false, resulting in a false negative.
In this new approach, we call refs.all
to generate the references and use refs.all_visible?
as the return value for the method.
How to set up and validate locally
- Open rails console and create a note referencing a non-existing MR:
author = User.first
other_user = User.last
project = Project.create!(title: 'New Test Project', path: 'new-test-project', namespace: author.namespace, creator: author)
issue = Issue.create!(project: project, title: 'Test Issue 1', description: 'test description', author: author)
note = Note.create!(project: project, noteable: issue, system: true, author: author, note: 'mentioned in merge request !1') # take note of its ID
# Check note visibility
note.system_note_visible_for?(author) # => false
note.system_note_visible_for?(other_user) # => false
-
Verify that the note is not visible
-
Checkout branch
eg-fix-system-notes-permissions-check
and check the note’s visibility again
author = User.first
other_user = User.last
note = Note.find(<note_id>)
note.system_note_visible_for?(author) # => true
note.system_note_visible_for?(other_user) # => true
- Verify that the note is visible for both users.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Eugenia Grieff