diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 81017290f12af1b3b7087b1f8172b28bab84889d..3d764f679906159a85ba645ff13e7b4504977c99 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -31,6 +31,7 @@ def execute notes = since_fetch_at(notes) notes = notes.with_notes_filter(@params[:notes_filter]) if notes_filter? notes = redact_internal(notes) + notes = notes.without_hidden if without_hidden_notes? sort(notes) end @@ -189,6 +190,13 @@ def redact_internal(notes) notes.not_internal end + + def without_hidden_notes? + return false unless Feature.enabled?(:hidden_notes) + return false if @current_user&.can_admin_all_resources? + + true + end end NotesFinder.prepend_mod_with('NotesFinder') diff --git a/app/models/note.rb b/app/models/note.rb index b9b884b88c5ccb26736e3a2100a5c989593571a9..13fff9520b7ab54c25d441a1d093b7060b378ed5 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -173,6 +173,14 @@ class Note < ApplicationRecord end scope :with_metadata, -> { includes(:system_note_metadata) } + scope :without_hidden, -> { + if Feature.enabled?(:hidden_notes) + where_not_exists(Users::BannedUser.where('notes.author_id = banned_users.user_id')) + else + all + end + } + scope :for_note_or_capitalized_note, ->(text) { where(note: [text, text.capitalize]) } scope :like_note_or_capitalized_note, ->(text) { where('(note LIKE ? OR note LIKE ?)', text, text.capitalize) } diff --git a/config/feature_flags/development/hidden_notes.yml b/config/feature_flags/development/hidden_notes.yml new file mode 100644 index 0000000000000000000000000000000000000000..239234d696037dfcfd6b21e9dc69a54da400290e --- /dev/null +++ b/config/feature_flags/development/hidden_notes.yml @@ -0,0 +1,8 @@ +--- +name: hidden_notes +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112973 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/405148 +milestone: '15.11' +type: development +group: group::anti-abuse +default_enabled: false diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 1df0aa411d69e105cc096c53e3d411e0f5975d71..e93c0c790c2a792be12acdafb1eb158f5479135b 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -128,6 +128,51 @@ end end + context 'for notes from users who have been banned', :enable_admin_mode, feature_category: :instance_resiliency do + subject(:finder) { described_class.new(user, project: project).execute } + + let_it_be(:banned_user) { create(:banned_user).user } + let!(:banned_note) { create(:note_on_issue, project: project, author: banned_user) } + + context 'when :hidden_notes feature is not enabled' do + before do + stub_feature_flags(hidden_notes: false) + end + + context 'when user is not an admin' do + it { is_expected.to include(banned_note) } + end + + context 'when @current_user is nil' do + let(:user) { nil } + + it { is_expected.to be_empty } + end + end + + context 'when :hidden_notes feature is enabled' do + before do + stub_feature_flags(hidden_notes: true) + end + + context 'when user is an admin' do + let(:user) { create(:admin) } + + it { is_expected.to include(banned_note) } + end + + context 'when user is not an admin' do + it { is_expected.not_to include(banned_note) } + end + + context 'when @current_user is nil' do + let(:user) { nil } + + it { is_expected.to be_empty } + end + end + end + context 'for target type' do let(:project) { create(:project, :repository) } let!(:note1) { create :note_on_issue, project: project } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index c1de8125c0d3133b55a6cf975417c88aa6b2f6bb..ed2bf26e1297a5df626b8f153dcadf36a1dde5b9 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1666,6 +1666,32 @@ def expect_expiration(noteable) end end end + + describe '.without_hidden' do + subject { described_class.without_hidden } + + context 'when a note with a banned author exists' do + let_it_be(:banned_user) { create(:banned_user).user } + let_it_be(:banned_note) { create(:note, author: banned_user) } + + context 'when the :hidden_notes feature is disabled' do + before do + stub_feature_flags(hidden_notes: false) + end + + it { is_expected.to include(banned_note, note1) } + end + + context 'when the :hidden_notes feature is enabled' do + before do + stub_feature_flags(hidden_notes: true) + end + + it { is_expected.not_to include(banned_note) } + it { is_expected.to include(note1) } + end + end + end end describe 'banzai_render_context' do