diff --git a/CHANGELOG b/CHANGELOG index efb6dc6f610..aedd9f8ebdf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -47,6 +47,7 @@ v 8.8.0 (unreleased) - Bump ace-rails-ap gem version from 2.0.1 to 4.0.2 which upgrades Ace Editor from 1.1.2 to 1.2.3 - Total method execution timings are no longer tracked - Allow Admins to remove the Login with buttons for OAuth services and still be able to import !4034. (Andrei Gliga) + - Add API endpoints for un/subscribing from/to a label. !4051 (Ahmad Sherif) v 8.7.5 - Fix relative links in wiki pages. !4050 diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb index d5a881b2445..083257f1005 100644 --- a/app/models/concerns/subscribable.rb +++ b/app/models/concerns/subscribable.rb @@ -36,6 +36,12 @@ module Subscribable update(subscribed: !subscribed?(user)) end + def subscribe(user) + subscriptions. + find_or_initialize_by(user_id: user.id). + update(subscribed: true) + end + def unsubscribe(user) subscriptions. find_or_initialize_by(user_id: user.id). diff --git a/doc/api/labels.md b/doc/api/labels.md index 3730c07c5a7..b857d81768e 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -165,3 +165,73 @@ Example response: "description": "Documentation" } ``` + +## Subscribe to a label + +Subscribes the authenticated user to a label to receive notifications. If the +operation is successful, status code `201` together with the updated label is +returned. If the user is already subscribed to the label, the status code `304` +is returned. If the project or label is not found, status code `404` is +returned. + +``` +POST /projects/:id/labels/:label_id/subscription +``` + +| Attribute | Type | Required | Description | +| ---------- | ----------------- | -------- | ------------------------------------ | +| `id` | integer | yes | The ID of a project | +| `label_id` | integer or string | yes | The ID or title of a project's label | + +```bash +curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/subscription +``` + +Example response: + +```json +{ + "name": "Docs", + "color": "#cc0033", + "description": "", + "open_issues_count": 0, + "closed_issues_count": 0, + "open_merge_requests_count": 0, + "subscribed": true +} +``` + +## Unsubscribe from a label + +Unsubscribes the authenticated user from a label to not receive notifications +from it. If the operation is successful, status code `200` together with the +updated label is returned. If the user is not subscribed to the label, the +status code `304` is returned. If the project or label is not found, status code +`404` is returned. + +``` +DELETE /projects/:id/labels/:label_id/subscription +``` + +| Attribute | Type | Required | Description | +| ---------- | ----------------- | -------- | ------------------------------------ | +| `id` | integer | yes | The ID of a project | +| `label_id` | integer or string | yes | The ID or title of a project's label | + +```bash +curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/subscription +``` + +Example response: + +```json +{ + "name": "Docs", + "color": "#cc0033", + "description": "", + "open_issues_count": 0, + "closed_issues_count": 0, + "open_merge_requests_count": 0, + "subscribed": false +} +``` diff --git a/lib/api/api.rb b/lib/api/api.rb index 5fd9c30cb42..360fb41a721 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -57,5 +57,6 @@ module API mount ::API::Variables mount ::API::Runners mount ::API::Licenses + mount ::API::Subscriptions end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 2870a6a40ef..406f5ea9139 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -307,6 +307,10 @@ module API class Label < Grape::Entity expose :name, :color, :description expose :open_issues_count, :closed_issues_count, :open_merge_requests_count + + expose :subscribed do |label, options| + label.subscribed?(options[:current_user]) + end end class Compare < Grape::Entity diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 40c967453fb..5e638dbe16a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -95,6 +95,17 @@ module API end end + def find_project_label(id) + label = user_project.labels.find_by_id(id) || user_project.labels.find_by_title(id) + label || not_found!('Label') + end + + def find_project_issue(id) + issue = user_project.issues.find(id) + not_found! unless can?(current_user, :read_issue, issue) + issue + end + def paginate(relation) relation.page(params[:page]).per(params[:per_page].to_i).tap do |data| add_pagination_headers(data) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 40928749481..f59a4d6c012 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -103,8 +103,7 @@ module API # Example Request: # GET /projects/:id/issues/:issue_id get ":id/issues/:issue_id" do - @issue = user_project.issues.find(params[:issue_id]) - not_found! unless can?(current_user, :read_issue, @issue) + @issue = find_project_issue(params[:issue_id]) present @issue, with: Entities::Issue, current_user: current_user end @@ -234,42 +233,6 @@ module API authorize!(:destroy_issue, issue) issue.destroy 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/subscription - post ':id/issues/:issue_id/subscription' do - issue = user_project.issues.find(params[:issue_id]) - - if issue.subscribed?(current_user) - not_modified! - else - issue.toggle_subscription(current_user) - present issue, with: Entities::Issue, current_user: current_user - end - end - - # Unsubscribes from a project issue - # - # Parameters: - # id (required) - The ID of a project - # issue_id (required) - The ID of a project issue - # Example Request: - # DELETE /projects/:id/issues/:issue_id/subscription - delete ':id/issues/:issue_id/subscription' do - issue = user_project.issues.find(params[:issue_id]) - - if issue.subscribed?(current_user) - issue.unsubscribe(current_user) - present issue, with: Entities::Issue, current_user: current_user - else - not_modified! - end - end end end end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 4af6bef0fa7..c806829d69e 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -11,7 +11,7 @@ module API # Example Request: # GET /projects/:id/labels get ':id/labels' do - present user_project.labels, with: Entities::Label + present user_project.labels, with: Entities::Label, current_user: current_user end # Creates a new label @@ -36,7 +36,7 @@ module API label = user_project.labels.create(attrs) if label.valid? - present label, with: Entities::Label + present label, with: Entities::Label, current_user: current_user else render_validation_error!(label) end @@ -90,7 +90,7 @@ module API attrs[:name] = attrs.delete(:new_name) if attrs.key?(:new_name) if label.update(attrs) - present label, with: Entities::Label + present label, with: Entities::Label, current_user: current_user else render_validation_error!(label) end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 7e78609ecb9..4e7de8867b4 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -327,42 +327,6 @@ module API issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user)) present paginate(issues), with: Entities::Issue, current_user: current_user end - - # Subscribes to a merge request - # - # Parameters: - # 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/subscription - post "#{path}/subscription" do - merge_request = user_project.merge_requests.find(params[:merge_request_id]) - - 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 - end - end - - # Unsubscribes from a merge request - # - # Parameters: - # id (required) - The ID of a project - # merge_request_id (required) - The ID of a merge request - # Example Request: - # 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) - merge_request.unsubscribe(current_user) - present merge_request, with: Entities::MergeRequest, current_user: current_user - else - not_modified! - end - end end end end diff --git a/lib/api/subscriptions.rb b/lib/api/subscriptions.rb new file mode 100644 index 00000000000..c49e2a21b82 --- /dev/null +++ b/lib/api/subscriptions.rb @@ -0,0 +1,60 @@ +module API + class Subscriptions < Grape::API + before { authenticate! } + + subscribable_types = { + 'merge_request' => proc { |id| user_project.merge_requests.find(id) }, + 'merge_requests' => proc { |id| user_project.merge_requests.find(id) }, + 'issues' => proc { |id| find_project_issue(id) }, + 'labels' => proc { |id| find_project_label(id) }, + } + + resource :projects do + subscribable_types.each do |type, finder| + type_singularized = type.singularize + type_id_str = :"#{type_singularized}_id" + entity_class = Entities.const_get(type_singularized.camelcase) + + # Subscribe to a resource + # + # Parameters: + # id (required) - The ID of a project + # subscribable_id (required) - The ID of a resource + # Example Request: + # POST /projects/:id/labels/:subscribable_id/subscription + # POST /projects/:id/issues/:subscribable_id/subscription + # POST /projects/:id/merge_requests/:subscribable_id/subscription + post ":id/#{type}/:#{type_id_str}/subscription" do + resource = instance_exec(params[type_id_str], &finder) + + if resource.subscribed?(current_user) + not_modified! + else + resource.subscribe(current_user) + present resource, with: entity_class, current_user: current_user + end + end + + # Unsubscribe from a resource + # + # Parameters: + # id (required) - The ID of a project + # subscribable_id (required) - The ID of a resource + # Example Request: + # DELETE /projects/:id/labels/:subscribable_id/subscription + # DELETE /projects/:id/issues/:subscribable_id/subscription + # DELETE /projects/:id/merge_requests/:subscribable_id/subscription + delete ":id/#{type}/:#{type_id_str}/subscription" do + resource = instance_exec(params[type_id_str], &finder) + + if !resource.subscribed?(current_user) + not_modified! + else + resource.unsubscribe(current_user) + present resource, with: entity_class, current_user: current_user + end + end + end + end + end +end diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index e31fdb0bffb..b7fc5a92497 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -44,6 +44,16 @@ describe Subscribable, 'Subscribable' do end end + describe '#subscribe' do + it 'subscribes the given user' do + expect(resource.subscribed?(user)).to be_falsey + + resource.subscribe(user) + + expect(resource.subscribed?(user)).to be_truthy + end + end + describe '#unsubscribe' do it 'unsubscribes the given current user' do resource.subscriptions.create(user: user, subscribed: true) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 9dd43f4fab3..37ab9cc8cfe 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -623,6 +623,12 @@ describe API::API, api: true do expect(response.status).to eq(404) end + + it 'returns 404 if the issue is confidential' do + post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member) + + expect(response.status).to eq(404) + end end describe 'DELETE :id/issues/:issue_id/subscription' do @@ -644,5 +650,11 @@ describe API::API, api: true do expect(response.status).to eq(404) end + + it 'returns 404 if the issue is confidential' do + delete api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscription", non_member) + + expect(response.status).to eq(404) + end end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 6943ff9d26c..b2c7f8d9acb 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -190,4 +190,86 @@ describe API::API, api: true do expect(json_response['message']['color']).to eq(['must be a valid color code']) end end + + describe "POST /projects/:id/labels/:label_id/subscription" do + context "when label_id is a label title" do + it "should subscribe to the label" do + post api("/projects/#{project.id}/labels/#{label1.title}/subscription", user) + + expect(response.status).to eq(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 "should subscribe to the label" do + post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(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) } + + it "should return 304" do + post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(304) + end + end + + context "when label ID is not found" do + it "should a return 404 error" do + post api("/projects/#{project.id}/labels/1234/subscription", user) + + expect(response.status).to eq(404) + end + end + end + + describe "DELETE /projects/:id/labels/:label_id/subscription" do + before { label1.subscribe(user) } + + context "when label_id is a label title" do + it "should unsubscribe from the label" do + delete api("/projects/#{project.id}/labels/#{label1.title}/subscription", user) + + expect(response.status).to eq(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 "should unsubscribe from the label" do + delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(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) } + + it "should return 304" do + delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) + + expect(response.status).to eq(304) + end + end + + context "when label ID is not found" do + it "should a return 404 error" do + delete api("/projects/#{project.id}/labels/1234/subscription", user) + + expect(response.status).to eq(404) + end + end + end end