Commit 39bd776a authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Rémy Coutable

Add userDiscussionsCount to issues and merge requests GraphQL

This change adds new field userDiscussionsCount to issues and merge
request GraphQL API to fetch count of threads initiated or continued by
user.
parent b45d04a0
...@@ -62,6 +62,8 @@ module Types ...@@ -62,6 +62,8 @@ module Types
description: 'Number of downvotes the issue has received' description: 'Number of downvotes the issue has received'
field :user_notes_count, GraphQL::INT_TYPE, null: false, field :user_notes_count, GraphQL::INT_TYPE, null: false,
description: 'Number of user notes of the issue' 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, field :web_path, GraphQL::STRING_TYPE, null: false, method: :issue_path,
description: 'Web path of the issue' description: 'Web path of the issue'
field :web_url, GraphQL::STRING_TYPE, null: false, field :web_url, GraphQL::STRING_TYPE, null: false,
...@@ -113,6 +115,26 @@ module Types ...@@ -113,6 +115,26 @@ module Types
field :severity, Types::IssuableSeverityEnum, null: true, field :severity, Types::IssuableSeverityEnum, null: true,
description: 'Severity level of the incident' 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 def author
Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
end end
......
...@@ -68,6 +68,8 @@ module Types ...@@ -68,6 +68,8 @@ module Types
description: 'SHA of the merge request commit (set once merged)' description: 'SHA of the merge request commit (set once merged)'
field :user_notes_count, GraphQL::INT_TYPE, null: true, field :user_notes_count, GraphQL::INT_TYPE, null: true,
description: 'User notes count of the merge request' 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, 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' 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, field :force_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :force_remove_source_branch?, null: true,
...@@ -158,17 +160,25 @@ module Types ...@@ -158,17 +160,25 @@ module Types
object.approved_by_users object.approved_by_users
end end
# rubocop: disable CodeReuse/ActiveRecord
def user_notes_count def user_notes_count
BatchLoader::GraphQL.for(object.id).batch(key: :merge_request_user_notes_count) do |ids, loader, args| 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| 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 end
end end
# rubocop: enable CodeReuse/ActiveRecord
def diff_stats(path: nil) def diff_stats(path: nil)
stats = Array.wrap(object.diff_stats&.to_a) stats = Array.wrap(object.diff_stats&.to_a)
......
...@@ -197,8 +197,8 @@ class Note < ApplicationRecord ...@@ -197,8 +197,8 @@ class Note < ApplicationRecord
.map(&:position) .map(&:position)
end end
def count_for_collection(ids, type) def count_for_collection(ids, type, count_column = 'COUNT(*) as count')
user.select('noteable_id', 'COUNT(*) as count') user.select(:noteable_id, count_column)
.group(:noteable_id) .group(:noteable_id)
.where(noteable_type: type, noteable_id: ids) .where(noteable_type: type, noteable_id: ids)
end end
......
---
title: Add userDiscussionsCount to issues and merge requests GraphQL
merge_request: 46311
author:
type: added
...@@ -7495,6 +7495,11 @@ type EpicIssue implements CurrentUserTodos & Noteable { ...@@ -7495,6 +7495,11 @@ type EpicIssue implements CurrentUserTodos & Noteable {
""" """
upvotes: Int! upvotes: Int!
"""
Number of user discussions in the issue
"""
userDiscussionsCount: Int!
""" """
Number of user notes of the issue Number of user notes of the issue
""" """
...@@ -9959,6 +9964,11 @@ type Issue implements CurrentUserTodos & Noteable { ...@@ -9959,6 +9964,11 @@ type Issue implements CurrentUserTodos & Noteable {
""" """
upvotes: Int! upvotes: Int!
"""
Number of user discussions in the issue
"""
userDiscussionsCount: Int!
""" """
Number of user notes of the issue Number of user notes of the issue
""" """
...@@ -12000,6 +12010,11 @@ type MergeRequest implements CurrentUserTodos & Noteable { ...@@ -12000,6 +12010,11 @@ type MergeRequest implements CurrentUserTodos & Noteable {
""" """
upvotes: Int! upvotes: Int!
"""
Number of user discussions in the merge request
"""
userDiscussionsCount: Int
""" """
User notes count of the merge request User notes count of the merge request
""" """
......
...@@ -20682,6 +20682,24 @@ ...@@ -20682,6 +20682,24 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "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", "name": "userNotesCount",
"description": "Number of user notes of the issue", "description": "Number of user notes of the issue",
...@@ -27153,6 +27171,24 @@ ...@@ -27153,6 +27171,24 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "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", "name": "userNotesCount",
"description": "Number of user notes of the issue", "description": "Number of user notes of the issue",
...@@ -32838,6 +32874,20 @@ ...@@ -32838,6 +32874,20 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "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", "name": "userNotesCount",
"description": "User notes count of the merge request", "description": "User notes count of the merge request",
...@@ -1191,6 +1191,7 @@ Relationship between an epic and an issue. ...@@ -1191,6 +1191,7 @@ Relationship between an epic and an issue.
| `updatedAt` | Time! | Timestamp of when the issue was last updated | | `updatedAt` | Time! | Timestamp of when the issue was last updated |
| `updatedBy` | User | User that last updated the issue | | `updatedBy` | User | User that last updated the issue |
| `upvotes` | Int! | Number of upvotes the issue has received | | `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 | | `userNotesCount` | Int! | Number of user notes of the issue |
| `userPermissions` | IssuePermissions! | Permissions for the current user on the resource | | `userPermissions` | IssuePermissions! | Permissions for the current user on the resource |
| `webPath` | String! | Web path of the issue | | `webPath` | String! | Web path of the issue |
...@@ -1426,6 +1427,7 @@ Represents a recorded measurement (object count) for the Admins. ...@@ -1426,6 +1427,7 @@ Represents a recorded measurement (object count) for the Admins.
| `updatedAt` | Time! | Timestamp of when the issue was last updated | | `updatedAt` | Time! | Timestamp of when the issue was last updated |
| `updatedBy` | User | User that last updated the issue | | `updatedBy` | User | User that last updated the issue |
| `upvotes` | Int! | Number of upvotes the issue has received | | `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 | | `userNotesCount` | Int! | Number of user notes of the issue |
| `userPermissions` | IssuePermissions! | Permissions for the current user on the resource | | `userPermissions` | IssuePermissions! | Permissions for the current user on the resource |
| `webPath` | String! | Web path of the issue | | `webPath` | String! | Web path of the issue |
...@@ -1728,6 +1730,7 @@ Autogenerated return type of MarkAsSpamSnippet. ...@@ -1728,6 +1730,7 @@ Autogenerated return type of MarkAsSpamSnippet.
| `totalTimeSpent` | Int! | Total time reported as spent on the merge request | | `totalTimeSpent` | Int! | Total time reported as spent on the merge request |
| `updatedAt` | Time! | Timestamp of when the merge request was last updated | | `updatedAt` | Time! | Timestamp of when the merge request was last updated |
| `upvotes` | Int! | Number of upvotes for the merge request | | `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 | | `userNotesCount` | Int | User notes count of the merge request |
| `userPermissions` | MergeRequestPermissions! | Permissions for the current user on the resource | | `userPermissions` | MergeRequestPermissions! | Permissions for the current user on the resource |
| `webUrl` | String | Web URL of the merge request | | `webUrl` | String | Web URL of the merge request |
......
...@@ -15,7 +15,7 @@ RSpec.describe GitlabSchema.types['Issue'] do ...@@ -15,7 +15,7 @@ RSpec.describe GitlabSchema.types['Issue'] do
it 'has specific fields' do it 'has specific fields' do
fields = %i[id iid title description state reference author assignees updated_by participants labels milestone due_date 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 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] designs design_collection alert_management_alert severity current_user_todos]
......
...@@ -17,7 +17,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do ...@@ -17,7 +17,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
description_html state created_at updated_at source_project target_project description_html state created_at updated_at source_project target_project
project project_id source_project_id target_project_id source_branch project project_id source_project_id target_project_id source_branch
target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha 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 diff_refs diff_stats diff_stats_summary
force_remove_source_branch merge_status in_progress_merge_commit_sha force_remove_source_branch merge_status in_progress_merge_commit_sha
merge_error allow_collaboration should_be_rebased rebase_commit_sha merge_error allow_collaboration should_be_rebased rebase_commit_sha
......
...@@ -9,10 +9,9 @@ RSpec.describe 'getting an issue list for a project' do ...@@ -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(:project) { create(:project, :repository, :public) }
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:issues, reload: true) do let_it_be(:issue_a, reload: true) { create(:issue, project: project, discussion_locked: true) }
[create(:issue, project: project, discussion_locked: true), let_it_be(:issue_b, reload: true) { create(:issue, :with_alert, project: project) }
create(:issue, :with_alert, project: project)] let_it_be(:issues, reload: true) { [issue_a, issue_b] }
end
let(:fields) do let(:fields) do
<<~QUERY <<~QUERY
...@@ -414,4 +413,42 @@ RSpec.describe 'getting an issue list for a project' do ...@@ -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)) expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(new_issues))
end end
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 end
...@@ -243,6 +243,17 @@ RSpec.describe 'getting merge request listings nested in a project' do ...@@ -243,6 +243,17 @@ RSpec.describe 'getting merge request listings nested in a project' do
include_examples 'N+1 query check' include_examples 'N+1 query check'
end 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 end
describe 'sorting and pagination' do describe 'sorting and pagination' do
......
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