Commit 7e508fba authored by Nick Thomas's avatar Nick Thomas

Merge branch '49796-project-deletion-may-not-log-audit-events-during-group-deletion' into 'master'

Resolve "Project deletion may not log audit events during group deletion"

Closes #49796

See merge request gitlab-org/gitlab-ce!21162
parents 9939b613 5fbb6ddf
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Groups module Groups
class DestroyService < Groups::BaseService class DestroyService < Groups::BaseService
DestroyError = Class.new(StandardError)
def async_execute def async_execute
job_id = GroupDestroyWorker.perform_async(group.id, current_user.id) job_id = GroupDestroyWorker.perform_async(group.id, current_user.id)
Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}")
...@@ -12,9 +14,8 @@ module Groups ...@@ -12,9 +14,8 @@ module Groups
group.projects.each do |project| group.projects.each do |project|
# Execute the destruction of the models immediately to ensure atomic cleanup. # Execute the destruction of the models immediately to ensure atomic cleanup.
# Skip repository removal because we remove directory with namespace success = ::Projects::DestroyService.new(project, current_user).execute
# that contain all these repositories raise DestroyError, "Project #{project.id} can't be deleted" unless success
::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute
end end
group.children.each do |group| group.children.each do |group|
......
---
title: 'Fix: Project deletion may not log audit events during group deletion'
merge_request: 21162
author:
type: fixed
...@@ -135,6 +135,17 @@ describe Groups::DestroyService do ...@@ -135,6 +135,17 @@ describe Groups::DestroyService do
it_behaves_like 'group destruction', false it_behaves_like 'group destruction', false
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
expect { destroy_group(group, user, false) }
.to raise_error(Groups::DestroyService::DestroyError, "Project #{project.id} can't be deleted" )
end
end
describe 'repository removal' do describe 'repository removal' do
before do before do
destroy_group(group, user, false) destroy_group(group, user, false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment