diff --git a/changelogs/unreleased/3874-correctly-return-json-on-delete-responses.yml b/changelogs/unreleased/3874-correctly-return-json-on-delete-responses.yml new file mode 100644 index 00000000000..4a4932288b4 --- /dev/null +++ b/changelogs/unreleased/3874-correctly-return-json-on-delete-responses.yml @@ -0,0 +1,4 @@ +--- +title: Return 202 with JSON body on async removals on V4 API +merge_request: +author: diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 9a48d63c117..e141723b580 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -41,3 +41,5 @@ changes are in V4: - Renamed `branch_name` to `branch` on DELETE `id/repository/branches/:branch` response [!8936](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8936) - Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736) - Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384) +- Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449) + diff --git a/lib/api/branches.rb b/lib/api/branches.rb index c65de90cca2..34f136948c2 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -137,7 +137,7 @@ module API delete ":id/repository/merged_branches" do DeleteMergedBranchesService.new(user_project, current_user).async_execute - status(200) + accepted! end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index d0efa7b993b..72d2b320077 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -209,6 +209,10 @@ module API render_api_error!('204 No Content', 204) end + def accepted! + render_api_error!('202 Accepted', 202) + end + def render_validation_error!(model) if model.errors.any? render_api_error!(model.errors.messages || '400 Bad Request', 400) diff --git a/lib/api/projects.rb b/lib/api/projects.rb index e7b891bd92e..b89bddc7e29 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -282,6 +282,8 @@ module API delete ":id" do authorize! :remove_project, user_project ::Projects::DestroyService.new(user_project, current_user, {}).async_execute + + accepted! end desc 'Mark this project as forked from another' diff --git a/lib/api/v3/branches.rb b/lib/api/v3/branches.rb index 733c6b21be5..51eb566cf7d 100644 --- a/lib/api/v3/branches.rb +++ b/lib/api/v3/branches.rb @@ -18,6 +18,13 @@ module API present branches, with: ::API::Entities::RepoBranch, project: user_project end + + desc 'Delete all merged branches' + delete ":id/repository/merged_branches" do + DeleteMergedBranchesService.new(user_project, current_user).async_execute + + status(200) + end end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 5571f6cc107..cacdb21c692 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -360,9 +360,11 @@ describe API::Branches, api: true do allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) end - it 'returns 200' do + it 'returns 202 with json body' do delete api("/projects/#{project.id}/repository/merged_branches", user) - expect(response).to have_http_status(200) + + expect(response).to have_http_status(202) + expect(json_response['message']).to eql('202 Accepted') end it 'returns a 403 error if guest' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8d139782fdf..5de4426f3bd 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1263,7 +1263,9 @@ describe API::Projects, api: true do context 'when authenticated as user' do it 'removes project' do delete api("/projects/#{project.id}", user) - expect(response).to have_http_status(200) + + expect(response).to have_http_status(202) + expect(json_response['message']).to eql('202 Accepted') end it 'does not remove a project if not an owner' do @@ -1287,7 +1289,9 @@ describe API::Projects, api: true do context 'when authenticated as admin' do it 'removes any existing project' do delete api("/projects/#{project.id}", admin) - expect(response).to have_http_status(200) + + expect(response).to have_http_status(202) + expect(json_response['message']).to eql('202 Accepted') end it 'does not remove a non existing project' do diff --git a/spec/requests/api/v3/branches_spec.rb b/spec/requests/api/v3/branches_spec.rb index 0e4c6bc3bc6..a3e1581fcc5 100644 --- a/spec/requests/api/v3/branches_spec.rb +++ b/spec/requests/api/v3/branches_spec.rb @@ -20,4 +20,25 @@ describe API::V3::Branches, api: true do expect(branch_names).to match_array(project.repository.branch_names) end end + + describe "DELETE /projects/:id/repository/merged_branches" do + before do + allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) + end + + it 'returns 200' do + delete v3_api("/projects/#{project.id}/repository/merged_branches", user) + + expect(response).to have_http_status(200) + end + + it 'returns a 403 error if guest' do + user_b = create :user + create(:project_member, :guest, user: user_b, project: project) + + delete v3_api("/projects/#{project.id}/repository/merged_branches", user_b) + + expect(response).to have_http_status(403) + end + end end