Merge branch '3874-correctly-return-json-on-delete-responses' into 'master'

Return 202 with JSON body on async removals on V4 API

Closes #3874

See merge request !9449
This commit is contained in:
Sean McGivern 2017-02-24 11:25:50 +00:00
commit f40403cce1
9 changed files with 51 additions and 5 deletions

View file

@ -0,0 +1,4 @@
---
title: Return 202 with JSON body on async removals on V4 API
merge_request:
author:

View file

@ -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) - 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) - 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) - 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)

View file

@ -137,7 +137,7 @@ module API
delete ":id/repository/merged_branches" do delete ":id/repository/merged_branches" do
DeleteMergedBranchesService.new(user_project, current_user).async_execute DeleteMergedBranchesService.new(user_project, current_user).async_execute
status(200) accepted!
end end
end end
end end

View file

@ -209,6 +209,10 @@ module API
render_api_error!('204 No Content', 204) render_api_error!('204 No Content', 204)
end end
def accepted!
render_api_error!('202 Accepted', 202)
end
def render_validation_error!(model) def render_validation_error!(model)
if model.errors.any? if model.errors.any?
render_api_error!(model.errors.messages || '400 Bad Request', 400) render_api_error!(model.errors.messages || '400 Bad Request', 400)

View file

@ -282,6 +282,8 @@ module API
delete ":id" do delete ":id" do
authorize! :remove_project, user_project authorize! :remove_project, user_project
::Projects::DestroyService.new(user_project, current_user, {}).async_execute ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
accepted!
end end
desc 'Mark this project as forked from another' desc 'Mark this project as forked from another'

View file

@ -18,6 +18,13 @@ module API
present branches, with: ::API::Entities::RepoBranch, project: user_project present branches, with: ::API::Entities::RepoBranch, project: user_project
end 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 end
end end

View file

@ -360,9 +360,11 @@ describe API::Branches, api: true do
allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true)
end end
it 'returns 200' do it 'returns 202 with json body' do
delete api("/projects/#{project.id}/repository/merged_branches", user) 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 end
it 'returns a 403 error if guest' do it 'returns a 403 error if guest' do

View file

@ -1263,7 +1263,9 @@ describe API::Projects, api: true do
context 'when authenticated as user' do context 'when authenticated as user' do
it 'removes project' do it 'removes project' do
delete api("/projects/#{project.id}", user) 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 end
it 'does not remove a project if not an owner' do 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 context 'when authenticated as admin' do
it 'removes any existing project' do it 'removes any existing project' do
delete api("/projects/#{project.id}", admin) 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 end
it 'does not remove a non existing project' do it 'does not remove a non existing project' do

View file

@ -20,4 +20,25 @@ describe API::V3::Branches, api: true do
expect(branch_names).to match_array(project.repository.branch_names) expect(branch_names).to match_array(project.repository.branch_names)
end end
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 end