Commit 99d0bc3d authored by Sean McGivern's avatar Sean McGivern

Merge branch 'api-subscription-restful' into 'master'

API: Make subscription API more RESTfuL

Closes #28327

See merge request !9325
parents 7bf28a4a ca68c817
---
title: 'API: - Make subscription API more RESTful. Use `post ":project_id/:subscribable_type/:subscribable_id/subscribe"` to subscribe and `post ":project_id/:subscribable_type/:subscribable_id/unsubscribe"` to unsubscribe from a resource.'
merge_request: 9325
author: Robert Schilling
......@@ -514,7 +514,7 @@ If the user is already subscribed to the issue, the status code `304`
is returned.
```
POST /projects/:id/issues/:issue_id/subscription
POST /projects/:id/issues/:issue_id/subscribe
```
| Attribute | Type | Required | Description |
......@@ -523,7 +523,7 @@ POST /projects/:id/issues/:issue_id/subscription
| `issue_id` | integer | yes | The ID of a project's issue |
```bash
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscribe
```
Example response:
......@@ -569,7 +569,7 @@ from it. If the user is not subscribed to the issue, the
status code `304` is returned.
```
DELETE /projects/:id/issues/:issue_id/subscription
DELETE /projects/:id/issues/:issue_id/unsubscribe
```
| Attribute | Type | Required | Description |
......@@ -578,7 +578,7 @@ DELETE /projects/:id/issues/:issue_id/subscription
| `issue_id` | integer | yes | The ID of a project's issue |
```bash
curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/unsubscribe
```
Example response:
......
......@@ -188,12 +188,12 @@ Example response:
## Subscribe to a label
Subscribes the authenticated user to a label to receive notifications.
Subscribes the authenticated user to a label to receive notifications.
If the user is already subscribed to the label, the status code `304`
is returned.
```
POST /projects/:id/labels/:label_id/subscription
POST /projects/:id/labels/:label_id/subscribe
```
| Attribute | Type | Required | Description |
......@@ -202,7 +202,7 @@ POST /projects/:id/labels/:label_id/subscription
| `label_id` | integer or string | yes | The ID or title of a project's label |
```bash
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/subscribe
```
Example response:
......@@ -228,7 +228,7 @@ from it. If the user is not subscribed to the label, the
status code `304` is returned.
```
DELETE /projects/:id/labels/:label_id/subscription
DELETE /projects/:id/labels/:label_id/unsubscribe
```
| Attribute | Type | Required | Description |
......@@ -237,7 +237,7 @@ DELETE /projects/:id/labels/:label_id/subscription
| `label_id` | integer or string | yes | The ID or title of a project's label |
```bash
curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/unsubscribe
```
Example response:
......
......@@ -667,7 +667,7 @@ Subscribes the authenticated user to a merge request to receive notification. If
status code `304` is returned.
```
POST /projects/:id/merge_requests/:merge_request_id/subscription
POST /projects/:id/merge_requests/:merge_request_id/subscribe
```
| Attribute | Type | Required | Description |
......@@ -676,7 +676,7 @@ POST /projects/:id/merge_requests/:merge_request_id/subscription
| `merge_request_id` | integer | yes | The ID of the merge request |
```bash
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscribe
```
Example response:
......@@ -741,7 +741,7 @@ notifications from that merge request. If the user is
not subscribed to the merge request, the status code `304` is returned.
```
DELETE /projects/:id/merge_requests/:merge_request_id/subscription
DELETE /projects/:id/merge_requests/:merge_request_id/unsubscribe
```
| Attribute | Type | Required | Description |
......@@ -750,7 +750,7 @@ DELETE /projects/:id/merge_requests/:merge_request_id/subscription
| `merge_request_id` | integer | yes | The ID of the merge request |
```bash
curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/subscription
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/17/unsubscribe
```
Example response:
......
......@@ -29,4 +29,5 @@ changes are in V4:
- 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)`
- Make subscription API more RESTful. Use `post ":project_id/:subscribable_type/:subscribable_id/subscribe"` to subscribe and `post ":project_id/:subscribable_type/:subscribable_id/unsubscribe"` to unsubscribe from a resource.
- Labels filter on `projects/:id/issues` and `/issues` now matches only issues containing all labels (i.e.: Logical AND, not OR)
......@@ -17,6 +17,7 @@ module API
mount ::API::V3::Projects
mount ::API::V3::ProjectSnippets
mount ::API::V3::Repositories
mount ::API::V3::Subscriptions
mount ::API::V3::SystemHooks
mount ::API::V3::Tags
mount ::API::V3::Todos
......
......@@ -21,7 +21,7 @@ module API
desc 'Subscribe to a resource' do
success entity_class
end
post ":id/#{type}/:subscribable_id/subscription" do
post ":id/#{type}/:subscribable_id/subscribe" do
resource = instance_exec(params[:subscribable_id], &finder)
if resource.subscribed?(current_user, user_project)
......@@ -35,7 +35,7 @@ module API
desc 'Unsubscribe from a resource' do
success entity_class
end
delete ":id/#{type}/:subscribable_id/subscription" do
post ":id/#{type}/:subscribable_id/unsubscribe" do
resource = instance_exec(params[:subscribable_id], &finder)
if !resource.subscribed?(current_user, user_project)
......
module API
module V3
class Subscriptions < Grape::API
before { authenticate! }
subscribable_types = {
'merge_request' => proc { |id| find_merge_request_with_access(id, :update_merge_request) },
'merge_requests' => proc { |id| find_merge_request_with_access(id, :update_merge_request) },
'issues' => proc { |id| find_project_issue(id) },
'labels' => proc { |id| find_project_label(id) },
}
params do
requires :id, type: String, desc: 'The ID of a project'
requires :subscribable_id, type: String, desc: 'The ID of a resource'
end
resource :projects do
subscribable_types.each do |type, finder|
type_singularized = type.singularize
entity_class = ::API::Entities.const_get(type_singularized.camelcase)
desc 'Subscribe to a resource' do
success entity_class
end
post ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder)
if resource.subscribed?(current_user, user_project)
not_modified!
else
resource.subscribe(current_user, user_project)
present resource, with: entity_class, current_user: current_user, project: user_project
end
end
desc 'Unsubscribe from a resource' do
success entity_class
end
delete ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder)
if !resource.subscribed?(current_user, user_project)
not_modified!
else
resource.unsubscribe(current_user, user_project)
present resource, with: entity_class, current_user: current_user, project: user_project
end
end
end
end
end
end
end
......@@ -1259,55 +1259,55 @@ describe API::Issues, api: true do
end
end
describe 'POST :id/issues/:issue_id/subscription' do
describe 'POST :id/issues/:issue_id/subscribe' do
it 'subscribes to an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user2)
post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user2)
expect(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(true)
end
it 'returns 304 if already subscribed' do
post api("/projects/#{project.id}/issues/#{issue.id}/subscription", user)
post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user)
expect(response).to have_http_status(304)
end
it 'returns 404 if the issue is not found' do
post api("/projects/#{project.id}/issues/123/subscription", user)
post api("/projects/#{project.id}/issues/123/subscribe", user)
expect(response).to have_http_status(404)
end
it 'returns 404 if the issue is confidential' do
post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member)
post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscribe", non_member)
expect(response).to have_http_status(404)
end
end
describe 'DELETE :id/issues/:issue_id/subscription' do
describe 'POST :id/issues/:issue_id/unsubscribe' do
it 'unsubscribes from an issue' do
delete api("/projects/#{project.id}/issues/#{issue.id}/subscription", user)
post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user)
expect(response).to have_http_status(200)
expect(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(false)
end
it 'returns 304 if not subscribed' do
delete api("/projects/#{project.id}/issues/#{issue.id}/subscription", user2)
post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user2)
expect(response).to have_http_status(304)
end
it 'returns 404 if the issue is not found' do
delete api("/projects/#{project.id}/issues/123/subscription", user)
post api("/projects/#{project.id}/issues/123/unsubscribe", user)
expect(response).to have_http_status(404)
end
it 'returns 404 if the issue is confidential' do
delete api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member)
post api("/projects/#{project.id}/issues/#{confidential_issue.id}/unsubscribe", non_member)
expect(response).to have_http_status(404)
end
......
......@@ -318,10 +318,10 @@ describe API::Labels, api: true do
end
end
describe "POST /projects/:id/labels/:label_id/subscription" do
describe "POST /projects/:id/labels/:label_id/subscribe" do
context "when label_id is a label title" do
it "subscribes to the label" do
post api("/projects/#{project.id}/labels/#{label1.title}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.title}/subscribe", user)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
......@@ -331,7 +331,7 @@ describe API::Labels, api: true do
context "when label_id is a label ID" do
it "subscribes to the label" do
post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.id}/subscribe", user)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
......@@ -343,7 +343,7 @@ describe API::Labels, api: true do
before { label1.subscribe(user, project) }
it "returns 304" do
post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.id}/subscribe", user)
expect(response).to have_http_status(304)
end
......@@ -351,21 +351,21 @@ describe API::Labels, api: true do
context "when label ID is not found" do
it "returns 404 error" do
post api("/projects/#{project.id}/labels/1234/subscription", user)
post api("/projects/#{project.id}/labels/1234/subscribe", user)
expect(response).to have_http_status(404)
end
end
end
describe "DELETE /projects/:id/labels/:label_id/subscription" do
describe "POST /projects/:id/labels/:label_id/unsubscribe" do
before { label1.subscribe(user, project) }
context "when label_id is a label title" do
it "unsubscribes from the label" do
delete api("/projects/#{project.id}/labels/#{label1.title}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.title}/unsubscribe", user)
expect(response).to have_http_status(200)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey
end
......@@ -373,9 +373,9 @@ describe API::Labels, api: true do
context "when label_id is a label ID" do
it "unsubscribes from the label" do
delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.id}/unsubscribe", user)
expect(response).to have_http_status(200)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey
end
......@@ -385,7 +385,7 @@ describe API::Labels, api: true do
before { label1.unsubscribe(user, project) }
it "returns 304" do
delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
post api("/projects/#{project.id}/labels/#{label1.id}/unsubscribe", user)
expect(response).to have_http_status(304)
end
......@@ -393,7 +393,7 @@ describe API::Labels, api: true do
context "when label ID is not found" do
it "returns 404 error" do
delete api("/projects/#{project.id}/labels/1234/subscription", user)
post api("/projects/#{project.id}/labels/1234/unsubscribe", user)
expect(response).to have_http_status(404)
end
......
......@@ -662,22 +662,22 @@ describe API::MergeRequests, api: true do
end
end
describe 'POST :id/merge_requests/:merge_request_id/subscription' do
describe 'POST :id/merge_requests/:merge_request_id/subscribe' do
it 'subscribes to a merge request' do
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", admin)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", admin)
expect(response).to have_http_status(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}/subscription", user)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user)
expect(response).to have_http_status(304)
end
it 'returns 404 if the merge request is not found' do
post api("/projects/#{project.id}/merge_requests/123/subscription", user)
post api("/projects/#{project.id}/merge_requests/123/subscribe", user)
expect(response).to have_http_status(404)
end
......@@ -686,28 +686,28 @@ describe API::MergeRequests, api: true do
guest = create(:user)
project.team << [guest, :guest]
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", guest)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", guest)
expect(response).to have_http_status(403)
end
end
describe 'DELETE :id/merge_requests/:merge_request_id/subscription' do
describe 'POST :id/merge_requests/:merge_request_id/unsubscribe' do
it 'unsubscribes from a merge request' do
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", user)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user)
expect(response).to have_http_status(200)
expect(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(false)
end
it 'returns 304 if not subscribed' do
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", admin)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", admin)
expect(response).to have_http_status(304)
end
it 'returns 404 if the merge request is not found' do
post api("/projects/#{project.id}/merge_requests/123/subscription", user)
post api("/projects/#{project.id}/merge_requests/123/unsubscribe", user)
expect(response).to have_http_status(404)
end
......@@ -716,7 +716,7 @@ describe API::MergeRequests, api: true do
guest = create(:user)
project.team << [guest, :guest]
delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscription", guest)
post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", guest)
expect(response).to have_http_status(403)
end
......
......@@ -67,4 +67,86 @@ describe API::V3::Labels, api: true do
expect(priority_label_response['subscribed']).to be_falsey
end
end
describe "POST /projects/:id/labels/:label_id/subscription" do
context "when label_id is a label title" do
it "subscribes to the label" do
post v3_api("/projects/#{project.id}/labels/#{label1.title}/subscription", user)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_truthy
end
end
context "when label_id is a label ID" do
it "subscribes to the label" do
post v3_api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
expect(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_truthy
end
end
context "when user is already subscribed to label" do
before { label1.subscribe(user, project) }
it "returns 304" do
post v3_api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
expect(response).to have_http_status(304)
end
end
context "when label ID is not found" do
it "returns 404 error" do
post v3_api("/projects/#{project.id}/labels/1234/subscription", user)
expect(response).to have_http_status(404)
end
end
end
describe "DELETE /projects/:id/labels/:label_id/subscription" do
before { label1.subscribe(user, project) }
context "when label_id is a label title" do
it "unsubscribes from the label" do
delete v3_api("/projects/#{project.id}/labels/#{label1.title}/subscription", user)
expect(response).to have_http_status(200)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey
end
end
context "when label_id is a label ID" do
it "unsubscribes from the label" do
delete v3_api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
expect(response).to have_http_status(200)
expect(json_response["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey
end
end
context "when user is already unsubscribed from label" do
before { label1.unsubscribe(user, project) }
it "returns 304" do
delete v3_api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
expect(response).to have_http_status(304)
end
end
context "when label ID is not found" do
it "returns 404 error" do
delete v3_api("/projects/#{project.id}/labels/1234/subscription", user)
expect(response).to have_http_status(404)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment