diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index d59a6c87e7e4490b6a48d7038b0aabf6ec6a6569..aab8febc452460387ea77dbda7d062a14c0c5112 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -5,13 +5,7 @@ class CloseService < MergeRequests::BaseService include RemovesRefs def execute(merge_request, commit = nil) - if Feature.enabled?(:destroy_fork_network_on_archive, project) - unless params[:skip_authorization].present? || can?(current_user, :update_merge_request, merge_request) - return merge_request - end - else - return merge_request unless can?(current_user, :update_merge_request, merge_request) - end + return merge_request unless authorized_to_close?(merge_request) # If we close MergeRequest we want to ignore validation # so we can close broken one (Ex. fork project removed) @@ -55,6 +49,10 @@ def expire_unapproved_key(merge_request) def trigger_merge_request_merge_status_updated(merge_request) GraphqlTriggers.merge_request_merge_status_updated(merge_request) end + + def authorized_to_close?(merge_request) + params[:skip_authorization].present? || can?(current_user, :update_merge_request, merge_request) + end end end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 1a5d0c061a6870aca55af4d61e5c3497a6cdcc28..eefbce438416097695e05f27da1ac915b8e7cff7 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -37,11 +37,7 @@ def execute update_project! after_update - if Feature.enabled?(:destroy_fork_network_on_archive, project) && - project.previous_changes[:archived] == [false, true] - - UnlinkForkService.new(project, current_user).execute - end + UnlinkForkService.new(project, current_user).execute if archiving_project? success rescue ActiveRecord::ActiveRecordError @@ -60,6 +56,10 @@ def run_auto_devops_pipeline? private + def archiving_project? + project.previous_changes[:archived] == [false, true] + end + def update_project! if Feature.disabled?(:replicate_deletion_schedule_operations, project) return project.update!(params.except(*non_assignable_project_params)) diff --git a/config/feature_flags/beta/destroy_fork_network_on_archive.yml b/config/feature_flags/beta/destroy_fork_network_on_archive.yml deleted file mode 100644 index 972c797ea271c2d92654dbed0055c38ac5cd64b0..0000000000000000000000000000000000000000 --- a/config/feature_flags/beta/destroy_fork_network_on_archive.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: destroy_fork_network_on_archive -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/518708 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/189438 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/538925 -milestone: '18.0' -type: beta -group: group::source code -default_enabled: true diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 55c9b3ff1d0b90d24d793de988d08163354f75ed..0bddbc0bbfbdf6a044c5f4929f0a4c248605f57e 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -171,28 +171,5 @@ def execute end end end - - context 'with feature flag :destroy_fork_network_on_archive disabled' do - before do - stub_feature_flags(destroy_fork_network_on_archive: false) - end - - context 'when user has permission' do - it 'closes MR' do - result = service.execute(merge_request) - - expect(result).to be_closed - end - end - - context 'when user does not have permission' do - it 'does not close MR' do - guest_service = described_class.new(project: project, current_user: guest) - result = guest_service.execute(merge_request) - - expect(result).to be_open - end - end - end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1f566eedbe1b2b47844c35a69e050092ee2a0c44..e7beba07d2e01cc78a102e71cca65ceee309abbf 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -529,35 +529,21 @@ ) end - context 'when the destroy_fork_network_on_archive feature flag is enabled' do - context 'when project is being archived' do - it 'calls UnlinkForkService' do - project.update!(archived: false) + context 'when project is being archived' do + it 'calls UnlinkForkService' do + project.update!(archived: false) - expect(Projects::UnlinkForkService).to receive(:new).with(project, user).and_return(unlink_fork_service) + expect(Projects::UnlinkForkService).to receive(:new).with(project, user).and_return(unlink_fork_service) - update_project(project, user, archived: true) - end - end - - context 'when project is not being archived' do - it 'does not call UnlinkForkService' do - expect(Projects::UnlinkForkService).not_to receive(:new) - - update_project(project, user, archived: false) - end + update_project(project, user, archived: true) end end - context 'when the destroy_fork_network_on_archive feature flag is disabled' do - before do - stub_feature_flags(destroy_fork_network_on_archive: false) - end - + context 'when project is not being archived' do it 'does not call UnlinkForkService' do expect(Projects::UnlinkForkService).not_to receive(:new) - update_project(project, user, archived: true) + update_project(project, user, archived: false) end end end