Merge branch 'sh-fix-mark-for-deletion-service' into 'master'

Gracefully handle marking a project deletion multiple times

Closes #121730

See merge request gitlab-org/gitlab!22949

changelogs/unreleased/sh-fix-mark-for-deletion-service.yml

+---
+title: Gracefully handle marking a project deletion multiple times
+merge_request: 22949
+author:
+type: fixed

ee/app/services/projects/mark_for_deletion_service.rb

@@ -3,8 +3,8 @@
 module Projects
   class MarkForDeletionService < BaseService
     def execute
-      return if project.marked_for_deletion_at?
-      return unless project.feature_available?(:marking_project_for_deletion)
+      return success if project.marked_for_deletion_at?
+      return error('Cannot mark project for deletion: feature not supported') unless project.feature_available?(:marking_project_for_deletion)
 
       result = ::Projects::UpdateService.new(
         project,

ee/spec/services/projects/mark_for_deletion_service_spec.rb

@@ -18,13 +18,11 @@ describe Projects::MarkForDeletionService do
     end
 
     context 'marking project for deletion' do
-      before do
-        described_class.new(project, user).execute
-      end
-
       it 'marks project as archived and marked for deletion' do
-        expect(Project.unscoped.all).to include(project)
+        result = described_class.new(project, user).execute
 
+        expect(result[:status]).to eq(:success)
+        expect(Project.unscoped.all).to include(project)
         expect(project.archived).to eq(true)
         expect(project.marked_for_deletion_at).not_to be_nil
         expect(project.deleting_user).to eq(user)
@@ -34,11 +32,10 @@ describe Projects::MarkForDeletionService do
     context 'marking project for deletion once again' do
       let(:marked_for_deletion_at) { 2.days.ago }
 
-      before do
-        described_class.new(project, user).execute
-      end
-
       it 'does not change original date' do
+        result = described_class.new(project, user).execute
+
+        expect(result[:status]).to eq(:success)
         expect(project.marked_for_deletion_at).to eq(marked_for_deletion_at.to_date)
       end
     end
@@ -58,6 +55,9 @@ describe Projects::MarkForDeletionService do
       end
 
       it 'does not change project attributes' do
+        result = described_class.new(project, user).execute
+
+        expect(result[:status]).to eq(:error)
         expect(Project.all).to include(project)
 
         expect(project.archived).to eq(false)