diff options
author | Robert Schilling <rschilling@student.tugraz.at> | 2017-02-17 12:45:05 +0300 |
---|---|---|
committer | Robert Schilling <rschilling@student.tugraz.at> | 2017-02-21 19:44:44 +0300 |
commit | 55f2425a678b61178d46e50f2b5a2da929228f52 (patch) | |
tree | 467d77c5a37a2101fda87be193563b11987c4f35 | |
parent | 316a7312341fd2d359b44da3f386c3739c1bdb4d (diff) |
API: Make subscription API more RESTfuL
-rw-r--r-- | changelogs/unreleased/api-subscription-restful.yml | 4 | ||||
-rw-r--r-- | doc/api/v3_to_v4.md | 1 | ||||
-rw-r--r-- | lib/api/api.rb | 1 | ||||
-rw-r--r-- | lib/api/subscriptions.rb | 4 | ||||
-rw-r--r-- | lib/api/v3/subscriptions.rb | 53 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 22 | ||||
-rw-r--r-- | spec/requests/api/labels_spec.rb | 24 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 22 | ||||
-rw-r--r-- | spec/requests/api/v3/labels_spec.rb | 82 |
9 files changed, 177 insertions, 36 deletions
diff --git a/changelogs/unreleased/api-subscription-restful.yml b/changelogs/unreleased/api-subscription-restful.yml new file mode 100644 index 00000000000..95db470e6c9 --- /dev/null +++ b/changelogs/unreleased/api-subscription-restful.yml @@ -0,0 +1,4 @@ +--- +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 diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 1af124c56b1..e24ee0da204 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -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 ":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) diff --git a/lib/api/api.rb b/lib/api/api.rb index e729c07f8c3..2e51be9fff3 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -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 diff --git a/lib/api/subscriptions.rb b/lib/api/subscriptions.rb index e11d7537cc9..acf11dbdf26 100644 --- a/lib/api/subscriptions.rb +++ b/lib/api/subscriptions.rb @@ -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) diff --git a/lib/api/v3/subscriptions.rb b/lib/api/v3/subscriptions.rb new file mode 100644 index 00000000000..02a4157c26e --- /dev/null +++ b/lib/api/v3/subscriptions.rb @@ -0,0 +1,53 @@ +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 diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index ece1b43567d..774a8a1946f 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1232,55 +1232,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 diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 5d7a76cf3be..566d11bba57 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -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 diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index f4dee4a4ca1..c125df8b90b 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -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 diff --git a/spec/requests/api/v3/labels_spec.rb b/spec/requests/api/v3/labels_spec.rb index 18e2c0d40c8..bcb0c6b9449 100644 --- a/spec/requests/api/v3/labels_spec.rb +++ b/spec/requests/api/v3/labels_spec.rb @@ -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 |