diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5f38f29fa87442f66004c916a89ba3da65712b53..e873af01877ce2f0ba873c0bb6c083854e51c5c3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -560,11 +560,7 @@ def public_merge_status end scope :without_hidden, -> { - if Feature.enabled?(:hide_merge_requests_from_banned_users) - where_not_exists(Users::BannedUser.where('merge_requests.author_id = banned_users.user_id')) - else - all - end + where_not_exists(Users::BannedUser.where('merge_requests.author_id = banned_users.user_id')) } scope :merged_without_state_event_source, -> { @@ -2439,7 +2435,7 @@ def can_suggest_reviewers? end def hidden? - Feature.enabled?(:hide_merge_requests_from_banned_users) && author&.banned? + author&.banned? end def diffs_batch_cache_with_max_age? diff --git a/config/feature_flags/development/hide_merge_requests_from_banned_users.yml b/config/feature_flags/development/hide_merge_requests_from_banned_users.yml deleted file mode 100644 index 7ba8475e60725d50782500814acac7e9d833ab41..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/hide_merge_requests_from_banned_users.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: hide_merge_requests_from_banned_users -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107836 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/386726 -milestone: "15.8" -type: development -group: group::authorization -default_enabled: false diff --git a/doc/administration/moderate_users.md b/doc/administration/moderate_users.md index 9569e7d4222ad4e5006bc072c5fc1ba16fe27fc2..336f5d1f43f0273271f74d0821a2d689b64b34a6 100644 --- a/doc/administration/moderate_users.md +++ b/doc/administration/moderate_users.md @@ -307,6 +307,7 @@ Users can also be reactivated using the [GitLab API](../api/user_moderation.md#r - Hiding merge requests of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107836) in GitLab 15.8 [with a flag](feature_flags.md) named `hide_merge_requests_from_banned_users`. Disabled by default. - Hiding comments of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112973) in GitLab 15.11 [with a flag](feature_flags.md) named `hidden_notes`. Disabled by default. - Hiding projects of banned users [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121488) in GitLab 16.2 [with a flag](feature_flags.md) named `hide_projects_of_banned_users`. Disabled by default. +- Hiding merge requests of banned users [generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188770) in GitLab 18.0. Feature flag `hide_merge_requests_from_banned_users` removed. {{< /history >}} diff --git a/ee/lib/elastic/latest/merge_request_instance_proxy.rb b/ee/lib/elastic/latest/merge_request_instance_proxy.rb index 8ebccd9c50384d85c3dd06fbe742e42660bb44bc..1b7fa64acce0578301b478f328c9009d9fcc1450 100644 --- a/ee/lib/elastic/latest/merge_request_instance_proxy.rb +++ b/ee/lib/elastic/latest/merge_request_instance_proxy.rb @@ -33,9 +33,7 @@ def as_indexed_json(_options = {}) data['merge_requests_access_level'] = safely_read_project_feature_for_elasticsearch(:merge_requests) data['hashed_root_namespace_id'] = target_project.namespace.hashed_root_namespace_id - # Use target.hidden? once the FF hide_merge_requests_from_banned_users is fully rolled out - # https://gitlab.com/gitlab-org/gitlab/-/issues/410671 - data['hidden'] = target.author&.banned? + data['hidden'] = target.hidden? data['archived'] = target.project.archived? # Schema version. The format is Date.today.strftime('%y_%m') diff --git a/ee/lib/search/elastic/merge_request_query_builder.rb b/ee/lib/search/elastic/merge_request_query_builder.rb index b52751284df511962cf1b5b0c58b3c67c05b5a19..d18f3494b5535f78429b22aef14a2f5052357765 100644 --- a/ee/lib/search/elastic/merge_request_query_builder.rb +++ b/ee/lib/search/elastic/merge_request_query_builder.rb @@ -24,9 +24,7 @@ def build query_hash = ::Search::Elastic::Filters.by_target_branch(query_hash: query_hash, options: options) end - if Feature.enabled?(:hide_merge_requests_from_banned_users) # rubocop: disable Gitlab/FeatureFlagWithoutActor -- existing flag - query_hash = ::Search::Elastic::Filters.by_not_hidden(query_hash: query_hash, options: options) - end + query_hash = ::Search::Elastic::Filters.by_not_hidden(query_hash: query_hash, options: options) return ::Search::Elastic::Aggregations.by_label_ids(query_hash: query_hash) if options[:aggregation] diff --git a/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb b/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb index 775e37a6ac48ee37af127e8d7cb805cf4068cc7a..c08bfbbc891820c5e5b30fdf8b9df95843b328d9 100644 --- a/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb +++ b/ee/spec/lib/elastic/latest/merge_request_class_proxy_spec.rb @@ -58,55 +58,41 @@ end describe 'search on basis of hidden attribute' do - context 'when feature_flag hide_merge_requests_from_banned_users is disabled' do + context 'when current_user is non admin' do let(:current_user) { user } - before do - stub_feature_flags(hide_merge_requests_from_banned_users: false) + it 'does not include merge_requests from the banned authors' do + expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id]) end - it 'includes merge_requests from the banned authors' do - expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id, banned_user_mr.id]) + it 'has the correct named queries' do + result.response + + assert_named_queries( + 'merge_request:match:search_terms', + 'filters:non_archived' + ) end end - context 'when feature_flag hide_merge_requests_from_banned_users is enabled' do - context 'when current_user is non admin' do - let(:current_user) { user } - - it 'does not include merge_requests from the banned authors' do - expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id]) - end - - it 'has the correct named queries' do - result.response + context 'when current_user is anonymous' do + let(:current_user) { nil } - assert_named_queries( - 'merge_request:match:search_terms', - 'filters:non_archived' - ) - end + it 'does not include merge_requests from the banned authors' do + expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id]) end + end - context 'when current_user is anonymous' do - let(:current_user) { nil } + context 'when current_user is admin' do + let_it_be(:admin) { create(:user, :admin) } + let(:current_user) { admin } - it 'does not include merge_requests from the banned authors' do - expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id]) - end + before do + allow(admin).to receive(:can_admin_all_resources?).and_return(true) end - context 'when current_user is admin' do - let_it_be(:admin) { create(:user, :admin) } - let(:current_user) { admin } - - before do - allow(admin).to receive(:can_admin_all_resources?).and_return(true) - end - - it 'includes merge_requests from the banned authors' do - expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id, banned_user_mr.id]) - end + it 'includes merge_requests from the banned authors' do + expect(elasticsearch_hit_ids(result)).to match_array([active_user_mr.id, banned_user_mr.id]) end end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 8458d75a5ae75b4973be62434f6831775f702196..3ea72a66fd83cccfe464d94a32ad1a1d2867ef4d 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -1356,13 +1356,5 @@ def find(label_name) it { is_expected.to include(banned_merge_request) } end - - context 'when the `hide_merge_requests_from_banned_users` feature flag is disabled' do - before do - stub_feature_flags(hide_merge_requests_from_banned_users: false) - end - - it { is_expected.to include(banned_merge_request) } - end end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6155a843cad4722b82393b996ab172e3cedc3c8f..2364abafaa07ab2710acc1609613d56d417a1daa 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -349,16 +349,6 @@ it 'only returns public issuables' do expect(described_class.without_hidden).not_to include(hidden_merge_request) end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(hide_merge_requests_from_banned_users: false) - end - - it 'returns public and hidden issuables' do - expect(described_class.without_hidden).to include(hidden_merge_request) - end - end end describe '.merged_without_state_event_source' do @@ -6568,14 +6558,6 @@ def transition! let_it_be(:author) { create(:user, :banned) } it { is_expected.to eq(true) } - - context 'when the feature flag is disabled' do - before do - stub_feature_flags(hide_merge_requests_from_banned_users: false) - end - - it { is_expected.to eq(false) } - end end end diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index 7f4aad48ae49ae0251d3f3761f964ae681f32b79..642842a0aca71f274b258a7f73bfc21531c11107 100644 --- a/spec/policies/merge_request_policy_spec.rb +++ b/spec/policies/merge_request_policy_spec.rb @@ -632,19 +632,5 @@ def permission_table_for_planner(public_merge_request: false) it 'allows admin to read the merge_request', :enable_admin_mode do expect(permissions(admin, hidden_merge_request)).to be_allowed(:read_merge_request) end - - context 'when the `hide_merge_requests_from_banned_users` feature flag is disabled' do - before do - stub_feature_flags(hide_merge_requests_from_banned_users: false) - end - - it 'allows non-admin users to read the merge_request' do - expect(permissions(user, hidden_merge_request)).to be_allowed(:read_merge_request) - end - - it 'allows admin users to read the merge_request', :enable_admin_mode do - expect(permissions(admin, hidden_merge_request)).to be_allowed(:read_merge_request) - end - end end end