Make subscription API more RESTful

7 files changed, 68 insertions, 72 deletions

diff --git a/doc/api/issues.md b/doc/api/issues.md
index 4f7d948eba1..42024becc36 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -408,13 +408,14 @@ Example response:
 
 ## Subscribe to an issue
 
-Subscribes to an issue to receive notifications. If the operation is successful,
-status code `201` together with the updated issue is returned. If the user is
-already subscribed to the issue, the status code `304` is returned. If the
-project or issue is not found, status code `404` is returned.
+Subscribes the authenticated user to an issue to receive notifications. If the
+operation is successful, status code `201` together with the updated issue is
+returned. If the user is already subscribed to the issue, the status code `304`
+is returned. If the project or issue is not found, status code `404` is
+returned.
 
 ```
-POST /projects/:id/issues/:issue_id/subscribe
+POST /projects/:id/issues/:issue_id/subscription
 ```
 
 | Attribute | Type | Required | Description |
@@ -423,7 +424,7 @@ POST /projects/:id/issues/:issue_id/subscribe
 | `issue_id` | integer | yes | The ID of a project's issue |
 
 ```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscribe
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscription
 ```
 
 Example response:
@@ -461,14 +462,14 @@ Example response:
 
 ## Unsubscribe from an issue
 
-Unsubscribes from an issue to not receive notifications from that issue. If the
-operation is successful, status code `201` together with the updated issue is
-returned. If the user is not subscribed to the issue, the status code `304`
-is returned. If the project or issue is not found, status code `404` is
-returned.
+Unsubscribes the authenticated user from the issue to not receive notifications
+from it. If the operation is successful, status code `200` together with the
+updated issue is returned. If the user is not subscribed to the issue, the
+status code `304` is returned. If the project or issue is not found, status code
+`404` is returned.
 
 ```
-POST /projects/:id/issues/:issue_id/unsubscribe
+DELETE /projects/:id/issues/:issue_id/subscription
 ```
 
 | Attribute | Type | Required | Description |
@@ -477,7 +478,7 @@ POST /projects/:id/issues/:issue_id/unsubscribe
 | `issue_id` | integer | yes | The ID of a project's issue |
 
 ```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/unsubscribe
+curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscription
 ```
 
 Example response:
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 16cb8926265..3c18ebfa31e 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -609,14 +609,14 @@ Example response:
 
 ## Subscribe to a merge request
 
-Subscribes to a merge request to receive notification. If the operation is
-successful, status code `201` together with the updated merge request is
-returned. If the user is already subscribed to the merge request, the status
-code `304` is returned. If the project or merge request is not found, status
-code `404` is returned.
+Subscribes the authenticated user to a merge request to receive notification. If
+the operation is successful, status code `201` together with the updated merge
+request is returned. If the user is already subscribed to the merge request, the
+status code `304` is returned. If the project or merge request is not found,
+status code `404` is returned.
 
 ```
-POST /projects/:id/merge_requests/:merge_request_id/subscribe
+POST /projects/:id/merge_requests/:merge_request_id/subscription
 ```
 
 | Attribute | Type | Required | Description |
@@ -625,7 +625,7 @@ POST /projects/:id/merge_requests/:merge_request_id/subscribe
 | `merge_request_id` | integer | yes   | The ID of the merge request |
 
 ```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscribe
+curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscription
 ```
 
 Example response:
@@ -682,14 +682,14 @@ Example response:
 ```
 ## Unsubscribe from a merge request
 
-Unsubscribes from a merge request to not receive notifications from that merge
-request. If the operation is successful, status code `201` together with the
-updated merge request is returned. If the user is not subscribed to the merge
-request, the status code `304` is returned. If the project or merge request is
-not found, status code `404` is returned.
+Unsubscribes the authenticated user from a merge request to not receive
+notifications from that merge request. If the operation is successful, status
+code `200` together with the updated merge request is returned. If the user is
+not subscribed to the merge request, the status code `304` is returned. If the
+project or merge request is not found, status code `404` is returned.
 
 ```
-POST /projects/:id/merge_requests/:merge_request_id/unsubscribe
+DELETE /projects/:id/merge_requests/:merge_request_id/subscription
 ```
 
 | Attribute | Type | Required | Description |
@@ -698,7 +698,7 @@ POST /projects/:id/merge_requests/:merge_request_id/unsubscribe
 | `merge_request_id` | integer | yes   | The ID of the merge request |
 
 ```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscribe
+curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscription
 ```
 
 Example response:
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index aa0597564ed..54452f763a6 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -242,7 +242,7 @@ module API
     end
 
     def not_modified!
-      render_api_error!('304 Not modified', 304)
+      render_api_error!('304 Not Modified', 304)
     end
 
     def render_validation_error!(model)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 049618c00f7..37d25073074 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -238,32 +238,15 @@ module API
       #  id (required)       - The ID of a project
       #  issue_id (required) - The ID of a project issue
       # Example Request:
-      #   POST /projects/:id/issues/:issue_id
-      post ":id/issues/:issue_idsubscribe" do
+      #   POST /projects/:id/issues/:issue_id/subscription
+      post ":id/issues/:issue_id/subscription" do
         issue = user_project.issues.find_by(id: params[:issue_id])
 
-        if !issue.subscribed?(current_user)
-          present issue, with: Entities::Issue, current_user: current_user
-        else
+        if issue.subscribed?(current_user)
           not_modified!
-        end
-      end
-
-      # Subscribes to a project issue
-      #
-      # Parameters:
-      #  id (required) - The ID of a project
-      #  issue_id (required) - The ID of a project issue
-      # Example Request:
-      #   POST /projects/:id/issues/:issue_id/subscribe
-      post ":id/issues/:issue_id/subscribe" do
-        issue = user_project.issues.find_by(id: params[:issue_id])
-
-        if !issue.subscribed?(current_user)
+        else
           issue.toggle_subscription(current_user)
           present issue, with: Entities::Issue, current_user: current_user
-        else
-          not_modified!
         end
       end
 
@@ -273,8 +256,8 @@ module API
       #  id (required)       - The ID of a project
       #  issue_id (required) - The ID of a project issue
       # Example Request:
-      #   POST /projects/:id/issues/:issue_id/unsubscribe
-      post ":id/issues/:issue_id/unsubscribe" do
+      #   DELETE /projects/:id/issues/:issue_id/subscription
+      delete ":id/issues/:issue_id/subscription" do
         issue = user_project.issues.find_by(id: params[:issue_id])
 
         if issue.subscribed?(current_user)
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index d166484ba54..7e78609ecb9 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -334,15 +334,15 @@ module API
         #  id (required)               - The ID of a project
         #  merge_request_id (required) - The ID of a merge request
         # Example Request:
-        #   POST /projects/:id/issues/:merge_request_id/subscribe
-        post "#{path}/subscribe" do
+        #   POST /projects/:id/issues/:merge_request_id/subscription
+        post "#{path}/subscription" do
           merge_request = user_project.merge_requests.find(params[:merge_request_id])
 
-          if !merge_request.subscribed?(current_user)
+          if merge_request.subscribed?(current_user)
+            not_modified!
+          else
             merge_request.toggle_subscription(current_user)
             present merge_request, with: Entities::MergeRequest, current_user: current_user
-          else
-            not_modified!
           end
         end
 
@@ -352,8 +352,8 @@ module API
         #  id (required)               - The ID of a project
         #  merge_request_id (required) - The ID of a merge request
         # Example Request:
-        #   POST /projects/:id/merge_requests/:merge_request_id/unsubscribe
-        post "#{path}/unsubscribe" do
+        #   DELETE /projects/:id/merge_requests/:merge_request_id/subscription
+        delete "#{path}/subscription" do
           merge_request = user_project.merge_requests.find(params[:merge_request_id])
 
           if merge_request.subscribed?(current_user)
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 437caf466b0..8361a1649e0 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -571,33 +571,45 @@ describe API::API, api: true  do
     end
   end
 
-  describe 'POST :id/issues/:issue_id/subscribe' do
+  describe 'POST :id/issues/:issue_id/subscription' do
     it 'subscribes to an issue' do
-      post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user2)
+      post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user2)
 
       expect(response.status).to eq(201)
       expect(json_response['subscribed']).to eq(true)
     end
 
     it 'returns 304 if already subscribed' do
-      post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user)
+      post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user)
 
       expect(response.status).to eq(304)
     end
+
+    it 'returns 404 if the issue is not found' do
+      post api("/projects/#{project.id}/issues/123/subscription", user)
+
+      expect(response.status).to eq(404)
+    end
   end
 
-  describe 'POST :id/issues/:issue_id/unsubscribe' do
+  describe 'DELETE :id/issues/:issue_id/subscription' do
     it 'unsubscribes from an issue' do
-      post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user)
+      post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user)
 
-      expect(response.status).to eq(201)
+      expect(response.status).to eq(200)
       expect(json_response['subscribed']).to eq(false)
     end
 
     it 'returns 304 if not subscribed' do
-      post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user2)
+      post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user2)
 
       expect(response.status).to eq(304)
     end
+
+    it 'returns 404 if the issue is not found' do
+      post api("/projects/#{project.id}/issues/123/subscription", user)
+
+      expect(response.status).to eq(404)
+    end
   end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index b71c72a3829..c247fcf9c96 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -516,31 +516,31 @@ describe API::API, api: true  do
     end
   end
 
-  describe 'POST :id/merge_requests/:merge_request_id/subscribe' do
+  describe 'POST :id/merge_requests/:merge_request_id/subscription' do
     it 'subscribes to a merge request' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", admin)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", admin)
 
       expect(response.status).to eq(201)
       expect(json_response['subscribed']).to eq(true)
     end
 
     it 'returns 304 if already subscribed' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", user)
 
       expect(response.status).to eq(304)
     end
   end
 
-  describe 'POST :id/merge_requests/:merge_request_id/unsubscribe' do
+  describe 'DELETE :id/merge_requests/:merge_request_id/subscription' do
     it 'unsubscribes from a merge request' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user)
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", user)
 
-      expect(response.status).to eq(201)
+      expect(response.status).to eq(200)
       expect(json_response['subscribed']).to eq(false)
     end
 
     it 'returns 304 if not subscribed' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", admin)
+      delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", admin)
 
       expect(response.status).to eq(304)
     end