Merge branch 'sh-fix-project-deletions-not-logged' into 'master'

Fix audit and Geo project deletion events not being logged in certain conditions

Closes gitlab-ce#44378

See merge request gitlab-org/gitlab-ee!5006

app/services/projects/destroy_service.rb

@@ -89,7 +89,10 @@ module Projects
     def attempt_rollback(project, message)
       return unless project
 
-      project.update_attributes(delete_error: message, pending_delete: false)
+      # It's possible that the project was destroyed, but some after_commit
+      # hook failed and caused us to end up here. A destroyed model will be a frozen hash,
+      # which cannot be altered.
+      project.update_attributes(delete_error: message, pending_delete: false) unless project.destroyed?
       log_error("Deletion failed on #{project.full_path} with the following message: #{message}")
     end
 

ee/app/services/ee/projects/destroy_service.rb

@@ -7,7 +7,10 @@ module EE
       def execute
         succeeded = super
 
-        if succeeded
+        # It's possible that some error occurred, but at the end of the day
+        # if the project is destroyed from the database, we should log events
+        # and clean up where we can.
+        if project&.destroyed?
           mirror_cleanup(project)
           log_geo_event(project)
           log_audit_event(project)

ee/changelogs/unreleased/sh-fix-project-deletions-not-logged.yml

+---
+title: Fix audit and Geo project deletion events not being logged under certain conditions
+merge_request:
+author:
+type: fixed

ee/spec/services/projects/destroy_service_spec.rb

@@ -74,4 +74,14 @@ describe Projects::DestroyService do
       end
     end
   end
+
+  context 'system hooks exception' do
+    before do
+      allow_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).and_raise('something went wrong')
+    end
+
+    it 'logs an audit event' do
+      expect { subject.execute }.to change(AuditEvent, :count)
+    end
+  end
 end