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

Move API group deletion to Sidekiq

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

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

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.

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

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

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

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