Merge branch '2954-api-expose-issue-user_notes_count' into 'master'

API: Expose Issue#user_notes_count and MergeRequest#user_notes_count

_Originally opened at !2954 by @cnam812._

- - -

Expose `Issue#user_notes_count` and `MergeRequest#user_notes_count` through the API.

See merge request !3126

9 files changed, 89 insertions, 34 deletions

diff --git a/CHANGELOG b/CHANGELOG
index b07b7da3300..aeade934b46 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 8.8.0 (unreleased)
   - Added button to toggle whitespaces changes on diff view
   - Backport GitHub Enterprise import support from EE
   - Create tags using Rugged for performance reasons. !3745
+  - API: Expose Issue#user_notes_count. !3126 (Anton Popov)
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
   - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
   - Added multiple colors for labels in dropdowns when dups happen.
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 2e4efc4e8d8..7a3db742030 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -160,6 +160,10 @@ module Issuable
     notes.awards.where(note: "thumbsup").count
   end
 
+  def user_notes_count
+    notes.user.count
+  end
+
   def subscribed_without_subscriptions?(user)
     participants(user).include?(user)
   end
diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index 9ad86ed71c9..5cf70ea3bb7 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -28,16 +28,10 @@
           = downvotes
 
       - note_count = issue.notes.user.nonawards.count
-      - if note_count > 0
-        %li
-          = link_to issue_path(issue) + "#notes" do
-            = icon('comments')
-            = note_count
-      - else
-        %li
-          = link_to issue_path(issue) + "#notes", class: "issue-no-comments" do
-            = icon('comments')
-            = note_count
+      %li
+        = link_to issue_path(issue, anchor: 'notes'), class: ('issue-no-comments' if note_count.zero?) do
+          = icon('comments')
+          = note_count
 
   .issue-info
     #{issue.to_reference} ·
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index e740fe8c84d..73c6a95f5ca 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -36,16 +36,10 @@
           = downvotes
 
       - note_count = merge_request.mr_and_commit_notes.user.nonawards.count
-      - if note_count > 0
-        %li
-          = link_to merge_request_path(merge_request) + "#notes" do
-            = icon('comments')
-            = note_count
-      - else
-        %li
-          = link_to merge_request_path(merge_request) + "#notes", class: "merge-request-no-comments" do
-            = icon('comments')
-            = note_count
+      %li
+        = link_to merge_request_path(merge_request, anchor: 'notes'), class: ('merge-request-no-comments' if note_count.zero?) do
+          = icon('comments')
+          = note_count
 
   .merge-request-info
     #{merge_request.to_reference} ·
diff --git a/doc/api/issues.md b/doc/api/issues.md
index 3e78149f442..fc7a7ae0c0c 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -77,7 +77,8 @@ Example response:
       "created_at" : "2016-01-04T15:31:51.081Z",
       "iid" : 6,
       "labels" : [],
-      "subscribed" : false
+      "subscribed" : false,
+      "user_notes_count": 1
    }
 ]
 ```
@@ -154,7 +155,8 @@ Example response:
       "title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
       "updated_at" : "2016-01-04T15:31:46.176Z",
       "created_at" : "2016-01-04T15:31:46.176Z",
-      "subscribed" : false
+      "subscribed" : false,
+      "user_notes_count": 1
    }
 ]
 ```
@@ -216,7 +218,8 @@ Example response:
    "title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
    "updated_at" : "2016-01-04T15:31:46.176Z",
    "created_at" : "2016-01-04T15:31:46.176Z",
-   "subscribed": false
+   "subscribed": false,
+   "user_notes_count": 1
 }
 ```
 
@@ -271,7 +274,8 @@ Example response:
    "description" : null,
    "updated_at" : "2016-01-07T12:44:33.959Z",
    "milestone" : null,
-   "subscribed" : true
+   "subscribed" : true,
+   "user_notes_count": 0
 }
 ```
 
@@ -329,7 +333,8 @@ Example response:
    "id" : 85,
    "assignee" : null,
    "milestone" : null,
-   "subscribed" : true
+   "subscribed" : true,
+   "user_notes_count": 0
 }
 ```
 
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 2057f9d77aa..8217e30fe25 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -67,7 +67,8 @@ Parameters:
     },
     "merge_when_build_succeeds": true,
     "merge_status": "can_be_merged",
-    "subscribed" : false
+    "subscribed" : false,
+    "user_notes_count": 1
   }
 ]
 ```
@@ -130,7 +131,8 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
-  "subscribed" : true
+  "subscribed" : true,
+  "user_notes_count": 1
 }
 ```
 
@@ -230,6 +232,7 @@ Parameters:
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
   "subscribed" : true,
+  "user_notes_count": 1,
   "changes": [
     {
     "old_path": "VERSION",
@@ -308,7 +311,8 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
-  "subscribed" : true
+  "subscribed" : true,
+  "user_notes_count": 0
 }
 ```
 
@@ -378,7 +382,8 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
-  "subscribed" : true
+  "subscribed" : true,
+  "user_notes_count": 1
 }
 ```
 
@@ -472,7 +477,8 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
-  "subscribed" : true
+  "subscribed" : true,
+  "user_notes_count": 1
 }
 ```
 
@@ -537,7 +543,8 @@ Parameters:
   },
   "merge_when_build_succeeds": true,
   "merge_status": "can_be_merged",
-  "subscribed" : true
+  "subscribed" : true,
+  "user_notes_count": 1
 }
 ```
 
@@ -602,7 +609,8 @@ Example response:
       "title" : "Consequatur vero maxime deserunt laboriosam est voluptas dolorem.",
       "created_at" : "2016-01-04T15:31:51.081Z",
       "iid" : 6,
-      "labels" : []
+      "labels" : [],
+      "user_notes_count": 1
    },
 ]
 ```
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 716ca6f7ed9..2870a6a40ef 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -170,10 +170,10 @@ module API
       expose :label_names, as: :labels
       expose :milestone, using: Entities::Milestone
       expose :assignee, :author, using: Entities::UserBasic
-
       expose :subscribed do |issue, options|
         issue.subscribed?(options[:current_user])
       end
+      expose :user_notes_count
     end
 
     class MergeRequest < ProjectEntity
@@ -187,10 +187,10 @@ module API
       expose :milestone, using: Entities::Milestone
       expose :merge_when_build_succeeds
       expose :merge_status
-
       expose :subscribed do |merge_request, options|
         merge_request.subscribed?(options[:current_user])
       end
+      expose :user_notes_count
     end
 
     class MergeRequestChanges < MergeRequest
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index f88e39cad9e..9dd43f4fab3 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -39,6 +39,7 @@ describe API::API, api: true  do
   let!(:empty_milestone) do
     create(:milestone, title: '2.0.0', project: project)
   end
+  let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) }
 
   before { project.team << [user, :reporter] }
 
@@ -232,8 +233,28 @@ describe API::API, api: true  do
   end
 
   describe "GET /projects/:id/issues/:issue_id" do
+    it 'exposes known attributes' do
+      get api("/projects/#{project.id}/issues/#{issue.id}", user)
+
+      expect(response.status).to eq(200)
+      expect(json_response['id']).to eq(issue.id)
+      expect(json_response['iid']).to eq(issue.iid)
+      expect(json_response['project_id']).to eq(issue.project.id)
+      expect(json_response['title']).to eq(issue.title)
+      expect(json_response['description']).to eq(issue.description)
+      expect(json_response['state']).to eq(issue.state)
+      expect(json_response['created_at']).to be_present
+      expect(json_response['updated_at']).to be_present
+      expect(json_response['labels']).to eq(issue.label_names)
+      expect(json_response['milestone']).to be_a Hash
+      expect(json_response['assignee']).to be_a Hash
+      expect(json_response['author']).to be_a Hash
+      expect(json_response['user_notes_count']).to be(1)
+    end
+
     it "should return a project issue by id" do
       get api("/projects/#{project.id}/issues/#{issue.id}", user)
+
       expect(response.status).to eq(200)
       expect(json_response['title']).to eq(issue.title)
       expect(json_response['iid']).to eq(issue.iid)
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 1fa7e76894f..4b0111df149 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -113,6 +113,34 @@ describe API::API, api: true  do
   end
 
   describe "GET /projects/:id/merge_requests/:merge_request_id" do
+    it 'exposes known attributes' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+      expect(response.status).to eq(200)
+      expect(json_response['id']).to eq(merge_request.id)
+      expect(json_response['iid']).to eq(merge_request.iid)
+      expect(json_response['project_id']).to eq(merge_request.project.id)
+      expect(json_response['title']).to eq(merge_request.title)
+      expect(json_response['description']).to eq(merge_request.description)
+      expect(json_response['state']).to eq(merge_request.state)
+      expect(json_response['created_at']).to be_present
+      expect(json_response['updated_at']).to be_present
+      expect(json_response['labels']).to eq(merge_request.label_names)
+      expect(json_response['milestone']).to be_nil
+      expect(json_response['assignee']).to be_a Hash
+      expect(json_response['author']).to be_a Hash
+      expect(json_response['target_branch']).to eq(merge_request.target_branch)
+      expect(json_response['source_branch']).to eq(merge_request.source_branch)
+      expect(json_response['upvotes']).to eq(0)
+      expect(json_response['downvotes']).to eq(0)
+      expect(json_response['source_project_id']).to eq(merge_request.source_project.id)
+      expect(json_response['target_project_id']).to eq(merge_request.target_project.id)
+      expect(json_response['work_in_progress']).to be_falsy
+      expect(json_response['merge_when_build_succeeds']).to be_falsy
+      expect(json_response['merge_status']).to eq('can_be_merged')
+      expect(json_response['user_notes_count']).to be(2)
+    end
+
     it "should return merge_request" do
       get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
       expect(response.status).to eq(200)