diff --git a/changelogs/unreleased/api-todos-restful.yml b/changelogs/unreleased/api-todos-restful.yml new file mode 100644 index 00000000000..dba1350a495 --- /dev/null +++ b/changelogs/unreleased/api-todos-restful.yml @@ -0,0 +1,4 @@ +--- +title: 'API: Use POST requests to mark todos as done' +merge_request: 9410 +author: Robert Schilling diff --git a/doc/api/todos.md b/doc/api/todos.md index a5e81801024..a2fbbc7e1f8 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -184,7 +184,7 @@ Marks a single pending todo given by its ID for the current user as done. The todo marked as done is returned in the response. ``` -DELETE /todos/:id +POST /todos/:id/mark_as_done ``` Parameters: @@ -194,7 +194,7 @@ Parameters: | `id` | integer | yes | The ID of a todo | ```bash -curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/130 +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/130/mark_as_done ``` Example Response: @@ -277,20 +277,15 @@ Example Response: ## Mark all todos as done -Marks all pending todos for the current user as done. It returns the number of marked todos. +Marks all pending todos for the current user as done. It returns the HTTP status code `204` with an empty response. ``` -DELETE /todos +POST /todos/mark_as_done ``` ```bash -curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/donmark_as_donee ``` -Example Response: - -```json -3 -``` [ce-3188]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3188 diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 3f58c098b43..1af124c56b1 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -24,9 +24,9 @@ changes are in V4: - `/gitlab_ci_ymls/:key` - `/dockerfiles/:key` - Moved `/projects/fork/:id` to `/projects/:id/fork` +- Moved `DELETE /todos` to `POST /todos/mark_as_done` and `DELETE /todos/:todo_id` to `POST /todos/:todo_id/mark_as_done` - Endpoints `/projects/owned`, `/projects/visible`, `/projects/starred` & `/projects/all` are consolidated into `/projects` using query parameters - Return pagination headers for all endpoints that return an array - Removed `DELETE projects/:id/deploy_keys/:key_id/disable`. Use `DELETE projects/:id/deploy_keys/:key_id` instead - Moved `PUT /users/:id/(block|unblock)` to `POST /users/:id/(block|unblock)` - Labels filter on `projects/:id/issues` and `/issues` now matches only issues containing all labels (i.e.: Logical AND, not OR) - diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 9bd077263a7..0b9650b296c 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -58,7 +58,7 @@ module API params do requires :id, type: Integer, desc: 'The ID of the todo being marked as done' end - delete ':id' do + post ':id/mark_as_done' do todo = current_user.todos.find(params[:id]) TodoService.new.mark_todos_as_done([todo], current_user) @@ -66,9 +66,11 @@ module API end desc 'Mark all todos as done' - delete do + post '/mark_as_done' do todos = find_todos TodoService.new.mark_todos_as_done(todos, current_user) + + no_content! end end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 2069d2a7c75..f35e963a14b 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -107,46 +107,47 @@ describe API::Todos, api: true do end end - describe 'DELETE /todos/:id' do + describe 'POST /todos/:id/mark_as_done' do context 'when unauthenticated' do it 'returns authentication error' do - delete api("/todos/#{pending_1.id}") + post api("/todos/#{pending_1.id}/mark_as_done") - expect(response.status).to eq(401) + expect(response).to have_http_status(401) end end context 'when authenticated' do it 'marks a todo as done' do - delete api("/todos/#{pending_1.id}", john_doe) + post api("/todos/#{pending_1.id}/mark_as_done", john_doe) - expect(response.status).to eq(200) + expect(response).to have_http_status(201) + expect(json_response['id']).to eq(pending_1.id) + expect(json_response['state']).to eq('done') expect(pending_1.reload).to be_done end it 'updates todos cache' do expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original - delete api("/todos/#{pending_1.id}", john_doe) + post api("/todos/#{pending_1.id}/mark_as_done", john_doe) end end end - describe 'DELETE /todos' do + describe 'POST /mark_as_done' do context 'when unauthenticated' do it 'returns authentication error' do - delete api('/todos') + post api('/todos/mark_as_done') - expect(response.status).to eq(401) + expect(response).to have_http_status(401) end end context 'when authenticated' do it 'marks all todos as done' do - delete api('/todos', john_doe) + post api('/todos/mark_as_done', john_doe) - expect(response.status).to eq(200) - expect(response.body).to eq('3') + expect(response).to have_http_status(204) expect(pending_1.reload).to be_done expect(pending_2.reload).to be_done expect(pending_3.reload).to be_done @@ -155,7 +156,7 @@ describe API::Todos, api: true do it 'updates todos cache' do expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original - delete api("/todos", john_doe) + post api("/todos/mark_as_done", john_doe) end end end