From 19f6942aadf1b2e5bd992b319919f630ab50f029 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Tue, 18 Jul 2023 21:00:12 +1000
Subject: [PATCH] Remove restrict_merge_status_recheck feature flag
It had been default enabled for a couple months now so it's safe to
remove it now.
Changelog: removed
---
.../mergeability_check_batch_worker.rb | 3 +-
.../restrict_merge_status_recheck.yml | 8 -----
lib/api/entities/merge_request_basic.rb | 2 --
lib/api/merge_requests.rb | 3 +-
spec/requests/api/merge_requests_spec.rb | 36 -------------------
.../mergeability_check_batch_worker_spec.rb | 19 ----------
6 files changed, 2 insertions(+), 69 deletions(-)
delete mode 100644 config/feature_flags/development/restrict_merge_status_recheck.yml
diff --git a/app/workers/merge_requests/mergeability_check_batch_worker.rb b/app/workers/merge_requests/mergeability_check_batch_worker.rb
index f48e9c234abd0..e95c3952c8ca9 100644
--- a/app/workers/merge_requests/mergeability_check_batch_worker.rb
+++ b/app/workers/merge_requests/mergeability_check_batch_worker.rb
@@ -40,8 +40,7 @@ def perform(merge_request_ids, user_id)
private
def merge_status_recheck_not_allowed?(merge_request, user)
- ::Feature.enabled?(:restrict_merge_status_recheck, merge_request.project) &&
- !Ability.allowed?(user, :update_merge_request, merge_request.project)
+ !Ability.allowed?(user, :update_merge_request, merge_request.project)
end
end
end
diff --git a/config/feature_flags/development/restrict_merge_status_recheck.yml b/config/feature_flags/development/restrict_merge_status_recheck.yml
deleted file mode 100644
index 8b7da32677382..0000000000000
--- a/config/feature_flags/development/restrict_merge_status_recheck.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: restrict_merge_status_recheck
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115948
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/404567
-milestone: '15.11'
-type: development
-group: group::code review
-default_enabled: true
diff --git a/lib/api/entities/merge_request_basic.rb b/lib/api/entities/merge_request_basic.rb
index adff7f87cd351..56519e2bf0824 100644
--- a/lib/api/entities/merge_request_basic.rb
+++ b/lib/api/entities/merge_request_basic.rb
@@ -107,8 +107,6 @@ def detailed_merge_status
end
def can_check_mergeability?(project)
- return true if ::Feature.disabled?(:restrict_merge_status_recheck, project)
-
Ability.allowed?(options[:current_user], :update_merge_request, project)
end
end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index ff9d0e2c37113..03b9ee03b464b 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -118,8 +118,7 @@ def authorize_merge_request_rebase!(merge_request)
end
def recheck_mergeability_of(merge_requests:)
- return if ::Feature.enabled?(:restrict_merge_status_recheck, user_project) &&
- !can?(current_user, :update_merge_request, user_project)
+ return unless can?(current_user, :update_merge_request, user_project)
merge_requests.each { |mr| mr.check_mergeability(async: true) }
end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 0ca621fd9ea73..d7d09ec704f4c 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -166,42 +166,6 @@
expect(merge_request.reload.merge_status).to eq('unchecked')
end
end
-
- context 'when restrict_merge_status_recheck FF is disabled' do
- before do
- stub_feature_flags(restrict_merge_status_recheck: false)
- end
-
- context 'with batched_api_mergeability_checks FF on' do
- it 'checks mergeability asynchronously in batch', :sidekiq_inline do
- get(api(endpoint_path, user2), params: { with_merge_status_recheck: true })
-
- expect_successful_response_with_paginated_array
-
- expect(merge_request.reload.merge_status).to eq('can_be_merged')
- end
- end
-
- context 'with batched_api_mergeability_checks FF off' do
- before do
- stub_feature_flags(batched_api_mergeability_checks: false)
- end
-
- context 'with merge status recheck projection' do
- it 'does enqueue a merge status recheck' do
- expect_next_instances_of(check_service_class, (1..2)) do |service|
- expect(service).not_to receive(:execute)
- expect(service).to receive(:async_execute).and_call_original
- end
-
- get(api(endpoint_path, user2), params: { with_merge_status_recheck: true })
-
- expect_successful_response_with_paginated_array
- expect(mr_entity['merge_status']).to eq('checking')
- end
- end
- end
- end
end
end
diff --git a/spec/workers/merge_requests/mergeability_check_batch_worker_spec.rb b/spec/workers/merge_requests/mergeability_check_batch_worker_spec.rb
index 828ffb0c8111c..35d06ea6e8696 100644
--- a/spec/workers/merge_requests/mergeability_check_batch_worker_spec.rb
+++ b/spec/workers/merge_requests/mergeability_check_batch_worker_spec.rb
@@ -40,25 +40,6 @@
subject.perform([merge_request_1.id, merge_request_2.id, merge_request_3.id, 1234], user.id)
end
- context 'when restrict_merge_status_recheck FF is off' do
- before do
- stub_feature_flags(restrict_merge_status_recheck: false)
- end
-
- it 'executes MergeabilityCheckService on merge requests that needs to be checked' do
- expect_next_instance_of(MergeRequests::MergeabilityCheckService, merge_request_1) do |service|
- expect(service).to receive(:execute).and_return(ServiceResponse.success)
- end
- expect_next_instance_of(MergeRequests::MergeabilityCheckService, merge_request_2) do |service|
- expect(service).to receive(:execute).and_return(ServiceResponse.success)
- end
- expect(MergeRequests::MergeabilityCheckService).not_to receive(:new).with(merge_request_3.id)
- expect(MergeRequests::MergeabilityCheckService).not_to receive(:new).with(1234)
-
- subject.perform([merge_request_1.id, merge_request_2.id, merge_request_3.id, 1234], user.id)
- end
- end
-
it 'structurally logs a failed mergeability check' do
expect_next_instance_of(MergeRequests::MergeabilityCheckService, merge_request_1) do |service|
expect(service).to receive(:execute).and_return(ServiceResponse.error(message: "solar flares"))
--
GitLab