API: Use POST requests to mark todos as done

5 files changed, 28 insertions, 26 deletions

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