Revert "Fix: Project deletion may not log audit events during user deletion"

This reverts commit ad2af8f8.

app/services/projects/destroy_service.rb

@@ -84,6 +84,9 @@ module Projects
     end
 
     def remove_repository(path)
+      # Skip repository removal. We use this flag when remove user or group
+      return true if params[:skip_repo] == true
+
       # There is a possibility project does not have repository or wiki
       return true unless repo_exists?(path)
 

app/services/users/destroy_service.rb

@@ -4,8 +4,6 @@ module Users
   class DestroyService
     prepend ::EE::Users::DestroyService
 
-    DestroyError = Class.new(StandardError)
-
     attr_accessor :current_user
 
     def initialize(current_user)
@@ -50,8 +48,9 @@ module Users
       namespace.prepare_for_destroy
 
       user.personal_projects.each do |project|
-        success = ::Projects::DestroyService.new(project, current_user).execute
-        raise DestroyError, "Project #{project.id} can't be deleted" unless success
+        # Skip repository removal because we remove directory with namespace
+        # that contain all this repositories
+        ::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute
       end
 
       yield(user) if block_given?

changelogs/unreleased/49796-project-deletion-may-not-log-audit-events-during-user-deletion.yml

----
-title: 'Fix: Project deletion may not log audit events during user deletion'
-merge_request:
-author:
-type: fixed

spec/services/users/destroy_service_spec.rb

@@ -20,7 +20,7 @@ describe Users::DestroyService do
 
       it 'will delete the project' do
         expect_next_instance_of(Projects::DestroyService) do |destroy_service|
-          expect(destroy_service).to receive(:execute).once.and_return(true)
+          expect(destroy_service).to receive(:execute).once
         end
 
         service.execute(user)
@@ -35,7 +35,7 @@ describe Users::DestroyService do
 
       it 'destroys a project in pending_delete' do
         expect_next_instance_of(Projects::DestroyService) do |destroy_service|
-          expect(destroy_service).to receive(:execute).once.and_return(true)
+          expect(destroy_service).to receive(:execute).once
         end
 
         service.execute(user)
@@ -172,36 +172,23 @@ describe Users::DestroyService do
     end
 
     describe "user personal's repository removal" do
-      context 'storages' do
-        before do
-          perform_enqueued_jobs { service.execute(user) }
-        end
-
-        context 'legacy storage' do
-          let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) }
-
-          it 'removes repository' do
-            expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
-          end
-        end
+      before do
+        perform_enqueued_jobs { service.execute(user) }
+      end
 
-        context 'hashed storage' do
-          let!(:project) { create(:project, :empty_repo, namespace: user.namespace) }
+      context 'legacy storage' do
+        let!(:project) { create(:project, :empty_repo, :legacy_storage, namespace: user.namespace) }
 
-          it 'removes repository' do
-            expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
-          end
+        it 'removes repository' do
+          expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
         end
       end
 
-      context 'repository removal status is taken into account' do
-        it 'raises exception' do
-          expect_next_instance_of(::Projects::DestroyService) do |destroy_service|
-            expect(destroy_service).to receive(:execute).and_return(false)
-          end
+      context 'hashed storage' do
+        let!(:project) { create(:project, :empty_repo, namespace: user.namespace) }
 
-          expect { service.execute(user) }
-            .to raise_error(Users::DestroyService::DestroyError, "Project #{project.id} can't be deleted" )
+        it 'removes repository' do
+          expect(gitlab_shell.exists?(project.repository_storage, "#{project.disk_path}.git")).to be_falsey
         end
       end
     end

spec/workers/project_destroy_worker_spec.rb

@@ -18,6 +18,13 @@ describe ProjectDestroyWorker do
       expect(Dir.exist?(path)).to be_falsey
     end
 
+    it 'deletes the project but skips repo deletion' do
+      subject.perform(project.id, project.owner.id, { "skip_repo" => true })
+
+      expect(Project.all).not_to include(project)
+      expect(Dir.exist?(path)).to be_truthy
+    end
+
     it 'does not raise error when project could not be found' do
       expect do
         subject.perform(-1, project.owner.id, {})