Merge branch 'sh-move-delete-groups-api-async' into 'master'

Move API group deletion to Sidekiq

See merge request gitlab-org/gitlab-ce!18626

6 files changed, 25 insertions, 9 deletions

diff --git a/changelogs/unreleased/sh-move-delete-groups-api-async.yml b/changelogs/unreleased/sh-move-delete-groups-api-async.yml
new file mode 100644
index 00000000000..1b200cac5c5
--- /dev/null
+++ b/changelogs/unreleased/sh-move-delete-groups-api-async.yml
@@ -0,0 +1,5 @@
+---
+title: Move API group deletion to Sidekiq
+merge_request:
+author:
+type: changed
diff --git a/doc/api/groups.md b/doc/api/groups.md
index 923fd662a5b..96842ef330f 100644
--- a/doc/api/groups.md
+++ b/doc/api/groups.md
@@ -487,6 +487,9 @@ Parameters:
 
 - `id` (required) - The ID or path of a user group
 
+This will queue a background job to delete all projects in the group. The
+response will be a 202 Accepted if the user has authorization.
+
 ## Search for group
 
 Get all groups that match your string in their name or path.
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 0d125cd7831..03b6b30a0d8 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -167,8 +167,10 @@ module API
 
         Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
         destroy_conditionally!(group) do |group|
-          ::Groups::DestroyService.new(group, current_user).execute
+          ::Groups::DestroyService.new(group, current_user).async_execute
         end
+
+        accepted!
       end
 
       desc 'Get a list of projects in this group.' do
diff --git a/lib/api/v3/groups.rb b/lib/api/v3/groups.rb
index 3844fd4810d..4fa7d196e50 100644
--- a/lib/api/v3/groups.rb
+++ b/lib/api/v3/groups.rb
@@ -131,8 +131,9 @@ module API
         delete ":id" do
           group = find_group!(params[:id])
           authorize! :admin_group, group
-          Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285')
-          present ::Groups::DestroyService.new(group, current_user).execute, with: Entities::GroupDetail, current_user: current_user
+          ::Groups::DestroyService.new(group, current_user).async_execute
+
+          accepted!
         end
 
         desc 'Get a list of projects in this group.' do
diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb
index bb0034e3237..7d923932309 100644
--- a/spec/requests/api/groups_spec.rb
+++ b/spec/requests/api/groups_spec.rb
@@ -738,13 +738,16 @@ describe API::Groups do
   describe "DELETE /groups/:id" do
     context "when authenticated as user" do
       it "removes group" do
-        delete api("/groups/#{group1.id}", user1)
+        Sidekiq::Testing.fake! do
+          expect { delete api("/groups/#{group1.id}", user1) }.to change(GroupDestroyWorker.jobs, :size).by(1)
+        end
 
-        expect(response).to have_gitlab_http_status(204)
+        expect(response).to have_gitlab_http_status(202)
       end
 
       it_behaves_like '412 response' do
         let(:request) { api("/groups/#{group1.id}", user1) }
+        let(:success_status) { 202 }
       end
 
       it "does not remove a group if not an owner" do
@@ -773,7 +776,7 @@ describe API::Groups do
       it "removes any existing group" do
         delete api("/groups/#{group2.id}", admin)
 
-        expect(response).to have_gitlab_http_status(204)
+        expect(response).to have_gitlab_http_status(202)
       end
 
       it "does not remove a non existing group" do
diff --git a/spec/requests/api/v3/groups_spec.rb b/spec/requests/api/v3/groups_spec.rb
index a1cdf583de3..34d4b8e9565 100644
--- a/spec/requests/api/v3/groups_spec.rb
+++ b/spec/requests/api/v3/groups_spec.rb
@@ -458,9 +458,11 @@ describe API::V3::Groups do
   describe "DELETE /groups/:id" do
     context "when authenticated as user" do
       it "removes group" do
-        delete v3_api("/groups/#{group1.id}", user1)
+        Sidekiq::Testing.fake! do
+          expect { delete v3_api("/groups/#{group1.id}", user1) }.to change(GroupDestroyWorker.jobs, :size).by(1)
+        end
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(202)
       end
 
       it "does not remove a group if not an owner" do
@@ -489,7 +491,7 @@ describe API::V3::Groups do
       it "removes any existing group" do
         delete v3_api("/groups/#{group2.id}", admin)
 
-        expect(response).to have_gitlab_http_status(200)
+        expect(response).to have_gitlab_http_status(202)
       end
 
       it "does not remove a non existing group" do