Merge branch 'add-user-discussions-count-to-issues-and-merge-requests' into 'master'

Add userDiscussionsCount to issues and merge requests GraphQL

See merge request gitlab-org/gitlab!46311

app/graphql/types/issue_type.rb

@@ -62,6 +62,8 @@ module Types
           description: 'Number of downvotes the issue has received'
     field :user_notes_count, GraphQL::INT_TYPE, null: false,
           description: 'Number of user notes of the issue'
+    field :user_discussions_count, GraphQL::INT_TYPE, null: false,
+          description: 'Number of user discussions in the issue'
     field :web_path, GraphQL::STRING_TYPE, null: false, method: :issue_path,
           description: 'Web path of the issue'
     field :web_url, GraphQL::STRING_TYPE, null: false,
@@ -113,6 +115,26 @@ module Types
     field :severity, Types::IssuableSeverityEnum, null: true,
           description: 'Severity level of the incident'
 
+    def user_notes_count
+      BatchLoader::GraphQL.for(object.id).batch(key: :issue_user_notes_count) do |ids, loader, args|
+        counts = Note.count_for_collection(ids, 'Issue').index_by(&:noteable_id)
+
+        ids.each do |id|
+          loader.call(id, counts[id]&.count || 0)
+        end
+      end
+    end
+
+    def user_discussions_count
+      BatchLoader::GraphQL.for(object.id).batch(key: :issue_user_discussions_count) do |ids, loader, args|
+        counts = Note.count_for_collection(ids, 'Issue', 'COUNT(DISTINCT discussion_id) as count').index_by(&:noteable_id)
+
+        ids.each do |id|
+          loader.call(id, counts[id]&.count || 0)
+        end
+      end
+    end
+
     def author
       Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
     end

app/graphql/types/merge_request_type.rb

@@ -68,6 +68,8 @@ module Types
           description: 'SHA of the merge request commit (set once merged)'
     field :user_notes_count, GraphQL::INT_TYPE, null: true,
           description: 'User notes count of the merge request'
+    field :user_discussions_count, GraphQL::INT_TYPE, null: true,
+          description: 'Number of user discussions in the merge request'
     field :should_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :should_remove_source_branch?, null: true,
           description: 'Indicates if the source branch of the merge request will be deleted after merge'
     field :force_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :force_remove_source_branch?, null: true,
@@ -158,17 +160,25 @@ module Types
       object.approved_by_users
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
     def user_notes_count
       BatchLoader::GraphQL.for(object.id).batch(key: :merge_request_user_notes_count) do |ids, loader, args|
-        counts = Note.where(noteable_type: 'MergeRequest', noteable_id: ids).user.group(:noteable_id).count
+        counts = Note.count_for_collection(ids, 'MergeRequest').index_by(&:noteable_id)
 
         ids.each do |id|
-          loader.call(id, counts[id] || 0)
+          loader.call(id, counts[id]&.count || 0)
+        end
+      end
+    end
+
+    def user_discussions_count
+      BatchLoader::GraphQL.for(object.id).batch(key: :merge_request_user_discussions_count) do |ids, loader, args|
+        counts = Note.count_for_collection(ids, 'MergeRequest', 'COUNT(DISTINCT discussion_id) as count').index_by(&:noteable_id)
+
+        ids.each do |id|
+          loader.call(id, counts[id]&.count || 0)
         end
       end
     end
-    # rubocop: enable CodeReuse/ActiveRecord
 
     def diff_stats(path: nil)
       stats = Array.wrap(object.diff_stats&.to_a)

app/models/note.rb

@@ -197,8 +197,8 @@ class Note < ApplicationRecord
         .map(&:position)
     end
 
-    def count_for_collection(ids, type)
-      user.select('noteable_id', 'COUNT(*) as count')
+    def count_for_collection(ids, type, count_column = 'COUNT(*) as count')
+      user.select(:noteable_id, count_column)
         .group(:noteable_id)
         .where(noteable_type: type, noteable_id: ids)
     end

changelogs/unreleased/add-user-discussions-count-to-issues-and-merge-requests.yml

+---
+title: Add userDiscussionsCount to issues and merge requests GraphQL
+merge_request: 46311
+author:
+type: added

doc/api/graphql/reference/gitlab_schema.graphql

@@ -7495,6 +7495,11 @@ type EpicIssue implements CurrentUserTodos & Noteable {
   """
   upvotes: Int!
 
+  """
+  Number of user discussions in the issue
+  """
+  userDiscussionsCount: Int!
+
   """
   Number of user notes of the issue
   """
@@ -9959,6 +9964,11 @@ type Issue implements CurrentUserTodos & Noteable {
   """
   upvotes: Int!
 
+  """
+  Number of user discussions in the issue
+  """
+  userDiscussionsCount: Int!
+
   """
   Number of user notes of the issue
   """
@@ -12000,6 +12010,11 @@ type MergeRequest implements CurrentUserTodos & Noteable {
   """
   upvotes: Int!
 
+  """
+  Number of user discussions in the merge request
+  """
+  userDiscussionsCount: Int
+
   """
   User notes count of the merge request
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -20682,6 +20682,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "userDiscussionsCount",
+              "description": "Number of user discussions in the issue",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "userNotesCount",
               "description": "Number of user notes of the issue",
@@ -27153,6 +27171,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "userDiscussionsCount",
+              "description": "Number of user discussions in the issue",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "userNotesCount",
               "description": "Number of user notes of the issue",
@@ -32838,6 +32874,20 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "userDiscussionsCount",
+              "description": "Number of user discussions in the merge request",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "Int",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "userNotesCount",
               "description": "User notes count of the merge request",

doc/api/graphql/reference/index.md

@@ -1191,6 +1191,7 @@ Relationship between an epic and an issue.
 | `updatedAt` | Time! | Timestamp of when the issue was last updated |
 | `updatedBy` | User | User that last updated the issue |
 | `upvotes` | Int! | Number of upvotes the issue has received |
+| `userDiscussionsCount` | Int! | Number of user discussions in the issue |
 | `userNotesCount` | Int! | Number of user notes of the issue |
 | `userPermissions` | IssuePermissions! | Permissions for the current user on the resource |
 | `webPath` | String! | Web path of the issue |
@@ -1426,6 +1427,7 @@ Represents a recorded measurement (object count) for the Admins.
 | `updatedAt` | Time! | Timestamp of when the issue was last updated |
 | `updatedBy` | User | User that last updated the issue |
 | `upvotes` | Int! | Number of upvotes the issue has received |
+| `userDiscussionsCount` | Int! | Number of user discussions in the issue |
 | `userNotesCount` | Int! | Number of user notes of the issue |
 | `userPermissions` | IssuePermissions! | Permissions for the current user on the resource |
 | `webPath` | String! | Web path of the issue |
@@ -1728,6 +1730,7 @@ Autogenerated return type of MarkAsSpamSnippet.
 | `totalTimeSpent` | Int! | Total time reported as spent on the merge request |
 | `updatedAt` | Time! | Timestamp of when the merge request was last updated |
 | `upvotes` | Int! | Number of upvotes for the merge request |
+| `userDiscussionsCount` | Int | Number of user discussions in the merge request |
 | `userNotesCount` | Int | User notes count of the merge request |
 | `userPermissions` | MergeRequestPermissions! | Permissions for the current user on the resource |
 | `webUrl` | String | Web URL of the merge request |

spec/graphql/types/issue_type_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe GitlabSchema.types['Issue'] do
 
   it 'has specific fields' do
     fields = %i[id iid title description state reference author assignees updated_by participants labels milestone due_date
-                confidential discussion_locked upvotes downvotes user_notes_count web_path web_url relative_position
+                confidential discussion_locked upvotes downvotes user_notes_count user_discussions_count web_path web_url relative_position
                 subscribed time_estimate total_time_spent human_time_estimate human_total_time_spent closed_at created_at updated_at task_completion_status
                 designs design_collection alert_management_alert severity current_user_todos]
 

spec/graphql/types/merge_request_type_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
       description_html state created_at updated_at source_project target_project
       project project_id source_project_id target_project_id source_branch
       target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha
-      merge_commit_sha user_notes_count should_remove_source_branch
+      merge_commit_sha user_notes_count user_discussions_count should_remove_source_branch
       diff_refs diff_stats diff_stats_summary
       force_remove_source_branch merge_status in_progress_merge_commit_sha
       merge_error allow_collaboration should_be_rebased rebase_commit_sha

spec/requests/api/graphql/project/issues_spec.rb

@@ -9,10 +9,9 @@ RSpec.describe 'getting an issue list for a project' do
 
   let_it_be(:project) { create(:project, :repository, :public) }
   let_it_be(:current_user) { create(:user) }
-  let_it_be(:issues, reload: true) do
-    [create(:issue, project: project, discussion_locked: true),
-     create(:issue, :with_alert, project: project)]
-  end
+  let_it_be(:issue_a, reload: true) { create(:issue, project: project, discussion_locked: true) }
+  let_it_be(:issue_b, reload: true) { create(:issue, :with_alert, project: project) }
+  let_it_be(:issues, reload: true) { [issue_a, issue_b] }
 
   let(:fields) do
     <<~QUERY
@@ -414,4 +413,42 @@ RSpec.describe 'getting an issue list for a project' do
       expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(new_issues))
     end
   end
+
+  describe 'N+1 query checks' do
+    let(:extra_iid_for_second_query) { issue_b.iid.to_s }
+    let(:search_params) { { iids: [issue_a.iid.to_s] } }
+
+    def execute_query
+      query = graphql_query_for(
+        :project,
+        { full_path: project.full_path },
+        query_graphql_field(:issues, search_params, [
+          query_graphql_field(:nodes, nil, requested_fields)
+        ])
+      )
+      post_graphql(query, current_user: current_user)
+    end
+
+    context 'when requesting `user_notes_count`' do
+      let(:requested_fields) { [:user_notes_count] }
+
+      before do
+        create_list(:note_on_issue, 2, noteable: issue_a, project: project)
+        create(:note_on_issue, noteable: issue_b, project: project)
+      end
+
+      include_examples 'N+1 query check'
+    end
+
+    context 'when requesting `user_discussions_count`' do
+      let(:requested_fields) { [:user_discussions_count] }
+
+      before do
+        create_list(:note_on_issue, 2, noteable: issue_a, project: project)
+        create(:note_on_issue, noteable: issue_b, project: project)
+      end
+
+      include_examples 'N+1 query check'
+    end
+  end
 end

spec/requests/api/graphql/project/merge_requests_spec.rb

@@ -243,6 +243,17 @@ RSpec.describe 'getting merge request listings nested in a project' do
 
       include_examples 'N+1 query check'
     end
+
+    context 'when requesting `user_discussions_count`' do
+      let(:requested_fields) { [:user_discussions_count] }
+
+      before do
+        create_list(:note_on_merge_request, 2, noteable: merge_request_a, project: project)
+        create(:note_on_merge_request, noteable: merge_request_c, project: project)
+      end
+
+      include_examples 'N+1 query check'
+    end
   end
 
   describe 'sorting and pagination' do