Commit 55f2425a authored by Robert Schilling's avatar Robert Schilling

API: Make subscription API more RESTfuL

parent 316a7312
---
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
...@@ -29,4 +29,5 @@ changes are in V4: ...@@ -29,4 +29,5 @@ changes are in V4:
- Return pagination headers for all endpoints that return an array - 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 - 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)` - Moved `PUT /users/:id/(block|unblock)` to `POST /users/:id/(block|unblock)`
- Make subscription API more RESTful. Use `post ":id/#{type}/:subscribable_id/subscribe"` to subscribe and `post ":id/#{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) - 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 ...@@ -17,6 +17,7 @@ module API
mount ::API::V3::Projects mount ::API::V3::Projects
mount ::API::V3::ProjectSnippets mount ::API::V3::ProjectSnippets
mount ::API::V3::Repositories mount ::API::V3::Repositories
mount ::API::V3::Subscriptions
mount ::API::V3::SystemHooks mount ::API::V3::SystemHooks
mount ::API::V3::Tags mount ::API::V3::Tags
mount ::API::V3::Todos mount ::API::V3::Todos
......
...@@ -21,7 +21,7 @@ module API ...@@ -21,7 +21,7 @@ module API
desc 'Subscribe to a resource' do desc 'Subscribe to a resource' do
success entity_class success entity_class
end end
post ":id/#{type}/:subscribable_id/subscription" do post ":id/#{type}/:subscribable_id/subscribe" do
resource = instance_exec(params[:subscribable_id], &finder) resource = instance_exec(params[:subscribable_id], &finder)
if resource.subscribed?(current_user, user_project) if resource.subscribed?(current_user, user_project)
...@@ -35,7 +35,7 @@ module API ...@@ -35,7 +35,7 @@ module API
desc 'Unsubscribe from a resource' do desc 'Unsubscribe from a resource' do
success entity_class success entity_class
end end
delete ":id/#{type}/:subscribable_id/subscription" do post ":id/#{type}/:subscribable_id/unsubscribe" do
resource = instance_exec(params[:subscribable_id], &finder) resource = instance_exec(params[:subscribable_id], &finder)
if !resource.subscribed?(current_user, user_project) 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
...@@ -1232,55 +1232,55 @@ describe API::Issues, api: true do ...@@ -1232,55 +1232,55 @@ describe API::Issues, api: true do
end end
end end
describe 'POST :id/issues/:issue_id/subscription' do describe 'POST :id/issues/:issue_id/subscribe' do
it 'subscribes to an issue' 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(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(true) expect(json_response['subscribed']).to eq(true)
end end
it 'returns 304 if already subscribed' do 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) expect(response).to have_http_status(304)
end end
it 'returns 404 if the issue is not found' do 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) expect(response).to have_http_status(404)
end end
it 'returns 404 if the issue is confidential' do 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) expect(response).to have_http_status(404)
end end
end end
describe 'DELETE :id/issues/:issue_id/subscription' do describe 'POST :id/issues/:issue_id/unsubscribe' do
it 'unsubscribes from an issue' 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) expect(json_response['subscribed']).to eq(false)
end end
it 'returns 304 if not subscribed' do 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) expect(response).to have_http_status(304)
end end
it 'returns 404 if the issue is not found' do 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) expect(response).to have_http_status(404)
end end
it 'returns 404 if the issue is confidential' do 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) expect(response).to have_http_status(404)
end end
......
...@@ -318,10 +318,10 @@ describe API::Labels, api: true do ...@@ -318,10 +318,10 @@ describe API::Labels, api: true do
end end
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 context "when label_id is a label title" do
it "subscribes to the label" 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(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title) expect(json_response["name"]).to eq(label1.title)
...@@ -331,7 +331,7 @@ describe API::Labels, api: true do ...@@ -331,7 +331,7 @@ describe API::Labels, api: true do
context "when label_id is a label ID" do context "when label_id is a label ID" do
it "subscribes to the label" 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(response).to have_http_status(201)
expect(json_response["name"]).to eq(label1.title) expect(json_response["name"]).to eq(label1.title)
...@@ -343,7 +343,7 @@ describe API::Labels, api: true do ...@@ -343,7 +343,7 @@ describe API::Labels, api: true do
before { label1.subscribe(user, project) } before { label1.subscribe(user, project) }
it "returns 304" do 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) expect(response).to have_http_status(304)
end end
...@@ -351,21 +351,21 @@ describe API::Labels, api: true do ...@@ -351,21 +351,21 @@ describe API::Labels, api: true do
context "when label ID is not found" do context "when label ID is not found" do
it "returns 404 error" 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) expect(response).to have_http_status(404)
end end
end 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) } before { label1.subscribe(user, project) }
context "when label_id is a label title" do context "when label_id is a label title" do
it "unsubscribes from the label" 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["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey expect(json_response["subscribed"]).to be_falsey
end end
...@@ -373,9 +373,9 @@ describe API::Labels, api: true do ...@@ -373,9 +373,9 @@ describe API::Labels, api: true do
context "when label_id is a label ID" do context "when label_id is a label ID" do
it "unsubscribes from the label" 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["name"]).to eq(label1.title)
expect(json_response["subscribed"]).to be_falsey expect(json_response["subscribed"]).to be_falsey
end end
...@@ -385,7 +385,7 @@ describe API::Labels, api: true do ...@@ -385,7 +385,7 @@ describe API::Labels, api: true do
before { label1.unsubscribe(user, project) } before { label1.unsubscribe(user, project) }
it "returns 304" do 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) expect(response).to have_http_status(304)
end end
...@@ -393,7 +393,7 @@ describe API::Labels, api: true do ...@@ -393,7 +393,7 @@ describe API::Labels, api: true do
context "when label ID is not found" do context "when label ID is not found" do
it "returns 404 error" 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) expect(response).to have_http_status(404)
end end
......
...@@ -662,22 +662,22 @@ describe API::MergeRequests, api: true do ...@@ -662,22 +662,22 @@ describe API::MergeRequests, api: true do
end end
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 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(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(true) expect(json_response['subscribed']).to eq(true)
end end
it 'returns 304 if already subscribed' do 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) expect(response).to have_http_status(304)
end end
it 'returns 404 if the merge request is not found' do 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) expect(response).to have_http_status(404)
end end
...@@ -686,28 +686,28 @@ describe API::MergeRequests, api: true do ...@@ -686,28 +686,28 @@ describe API::MergeRequests, api: true do
guest = create(:user) guest = create(:user)
project.team << [guest, :guest] 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) expect(response).to have_http_status(403)
end end
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 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) expect(json_response['subscribed']).to eq(false)
end end
it 'returns 304 if not subscribed' do 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) expect(response).to have_http_status(304)
end end
it 'returns 404 if the merge request is not found' do 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) expect(response).to have_http_status(404)
end end
...@@ -716,7 +716,7 @@ describe API::MergeRequests, api: true do ...@@ -716,7 +716,7 @@ describe API::MergeRequests, api: true do
guest = create(:user) guest = create(:user)
project.team << [guest, :guest] 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) expect(response).to have_http_status(403)
end end
......
...@@ -67,4 +67,86 @@ describe API::V3::Labels, api: true do ...@@ -67,4 +67,86 @@ describe API::V3::Labels, api: true do
expect(priority_label_response['subscribed']).to be_falsey expect(priority_label_response['subscribed']).to be_falsey
end end
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 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