diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb
index 54542cdace6c0d08035cb311b277ae95f4c82ec0..889cd63fb7e68bfee3002e9d36e7bada179efa20 100644
--- a/app/graphql/resolvers/issues_resolver.rb
+++ b/app/graphql/resolvers/issues_resolver.rb
@@ -35,7 +35,8 @@ def continue_issue_resolve(parent, finder, **args)
def preloads
{
alert_management_alert: [:alert_management_alert],
- labels: [:labels]
+ labels: [:labels],
+ assignees: [:assignees]
}
end
diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb
index 37af317996dca9eadb810a03e7d7cb181b01203d..df789f3cf479a49c1250533dbefad871ec27b7b7 100644
--- a/app/graphql/types/issue_type.rb
+++ b/app/graphql/types/issue_type.rb
@@ -38,8 +38,7 @@ class IssueType < BaseObject
description: 'User that created the issue',
resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.author_id).find }
- # Remove complexity when BatchLoader is used
- field :assignees, Types::UserType.connection_type, null: true, complexity: 5,
+ field :assignees, Types::UserType.connection_type, null: true,
description: 'Assignees of the issue'
field :labels, Types::LabelType.connection_type, null: true,
diff --git a/changelogs/unreleased/11496-issue-type-nplus1.yml b/changelogs/unreleased/11496-issue-type-nplus1.yml
new file mode 100644
index 0000000000000000000000000000000000000000..09fcf3d551376fa551bc2842e8dc27d5dfdfdec3
--- /dev/null
+++ b/changelogs/unreleased/11496-issue-type-nplus1.yml
@@ -0,0 +1,5 @@
+---
+title: Graphql Issues - Fix N+1 for Assignees
+merge_request: 41233
+author:
+type: performance
diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb
index 89b3ecdabb9d1da7c477ef37d2b1cd0122ef3ab8..5d4276f47ca0fe6c7993d7111d5ee1ff878d6a3b 100644
--- a/spec/requests/api/graphql/project/issues_spec.rb
+++ b/spec/requests/api/graphql/project/issues_spec.rb
@@ -5,10 +5,11 @@
RSpec.describe 'getting an issue list for a project' do
include GraphqlHelpers
- let(:project) { create(:project, :repository, :public) }
- let(:current_user) { create(:user) }
let(:issues_data) { graphql_data['project']['issues']['edges'] }
- let!(:issues) 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
@@ -85,7 +86,7 @@
end
context 'when there is a confidential issue' do
- let!(:confidential_issue) do
+ let_it_be(:confidential_issue) do
create(:issue, :confidential, project: project)
end
@@ -309,23 +310,12 @@ def pagination_results_data(data)
QUERY
end
- let(:query) do
- graphql_query_for(
- 'project',
- { 'fullPath' => project.full_path },
- query_graphql_field('issues', {}, fields)
- )
- end
-
- let_it_be(:project) { create(:project, :public) }
- let_it_be(:label1) { create(:label, project: project) }
- let_it_be(:label2) { create(:label, project: project) }
- let_it_be(:issue1) { create(:issue, project: project, labels: [label1]) }
- let_it_be(:issue2) { create(:issue, project: project, labels: [label2]) }
- let_it_be(:issues) { [issue1, issue2] }
-
before do
- stub_feature_flags(graphql_lookahead_support: true)
+ issues.each do |issue|
+ # create a label for each issue we have to properly test N+1
+ label = create(:label, project: project)
+ issue.update!(labels: [label])
+ end
end
def response_label_ids(response_data)
@@ -343,14 +333,64 @@ def labels_as_global_ids(issues)
expect(issues_data.count).to eq(2)
expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(issues))
- issues.append create(:issue, project: project, labels: [create(:label, project: project)])
+ new_issues = issues + [create(:issue, project: project, labels: [create(:label, project: project)])]
+
+ expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control)
+ # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb)
+ # so we have to parse the body ourselves the second time
+ issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges']
+ expect(issues_data.count).to eq(3)
+ expect(response_label_ids(issues_data)).to match_array(labels_as_global_ids(new_issues))
+ end
+ end
+
+ context 'fetching assignees' do
+ let(:fields) do
+ <<~QUERY
+ edges {
+ node {
+ id
+ assignees {
+ nodes {
+ id
+ }
+ }
+ }
+ }
+ QUERY
+ end
+
+ before do
+ issues.each do |issue|
+ # create an assignee for each issue we have to properly test N+1
+ assignee = create(:user)
+ issue.update!(assignees: [assignee])
+ end
+ end
+
+ def response_assignee_ids(response_data)
+ response_data.map do |edge|
+ edge['node']['assignees']['nodes'].map { |node| node['id'] }
+ end.flatten
+ end
+
+ def assignees_as_global_ids(issues)
+ issues.map(&:assignees).flatten.map(&:to_global_id).map(&:to_s)
+ end
+
+ it 'avoids N+1 queries', :aggregate_failures do
+ control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) }
+ expect(issues_data.count).to eq(2)
+ expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(issues))
+
+ new_issues = issues + [create(:issue, project: project, assignees: [create(:user)])]
expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control)
- # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb:271)
+ # graphql_data is memoized (see spec/support/helpers/graphql_helpers.rb)
# so we have to parse the body ourselves the second time
- response_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges']
- expect(response_data.count).to eq(3)
- expect(response_label_ids(response_data)).to match_array(labels_as_global_ids(issues))
+ issues_data = Gitlab::Json.parse(response.body)['data']['project']['issues']['edges']
+ expect(issues_data.count).to eq(3)
+ expect(response_assignee_ids(issues_data)).to match_array(assignees_as_global_ids(new_issues))
end
end
end