Merge branch 'sh-async-delete-children' into 'master'

Make nested groups deletion synchronous to avoid potential race

See merge request !9288

2 files changed, 4 insertions, 1 deletions

diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb
index 2e2d7f884ac..497fdb09cdc 100644
--- a/app/services/groups/destroy_service.rb
+++ b/app/services/groups/destroy_service.rb
@@ -18,7 +18,8 @@ module Groups
       end
 
       group.children.each do |group|
-        DestroyService.new(group, current_user).async_execute
+        # This needs to be synchronous since the namespace gets destroyed below
+        DestroyService.new(group, current_user).execute
       end
 
       group.really_destroy!
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index 32c2ed8cae7..98c560ffb26 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -5,6 +5,7 @@ describe Groups::DestroyService, services: true do
 
   let!(:user)         { create(:user) }
   let!(:group)        { create(:group) }
+  let!(:nested_group) { create(:group, parent: group) }
   let!(:project)      { create(:project, namespace: group) }
   let!(:gitlab_shell) { Gitlab::Shell.new }
   let!(:remove_path)  { group.path + "+#{group.id}+deleted" }
@@ -20,6 +21,7 @@ describe Groups::DestroyService, services: true do
       end
 
       it { expect(Group.unscoped.all).not_to include(group) }
+      it { expect(Group.unscoped.all).not_to include(nested_group) }
       it { expect(Project.unscoped.all).not_to include(project) }
     end