Commit a51ed1c4 authored by Luke Duncalfe's avatar Luke Duncalfe

Fix errors in GraphQL TodosQuery

Fixes missing target types in `TargetTypeEnum` and separates the
EE-specific types.

Adds `calls_gitaly` for the `TodoType#body` field, as an error is raised
when requesting the `todo.body field` for a `Commit` `Todo`:

    RuntimeError (Gitaly is called for field 'body' on Types::TodoType
    - please either specify a constant complexity or add
    `calls_gitaly: true` to the field declaration)

When the `Todo` (a polymorphic model) has a `target_type` of `Commit`,
requesting the `body` field calls Gitaly (`Todo#body` calls
`Todo#target` which fetches the `Commit` from Gitaly) and we get the
above error.

I've set `calls_gitaly: true` for the `TodoType#body` field (affecting
all Todos, regardless of the `target_type`) in order to avoid the error,
while we work out a better solution.

https://gitlab.com/gitlab-org/gitlab/issues/34757
https://gitlab.com/gitlab-org/gitlab/issues/34757#note_234752665
parent 2d0fe31d
...@@ -2,8 +2,10 @@ ...@@ -2,8 +2,10 @@
module Types module Types
class TodoTargetEnum < BaseEnum class TodoTargetEnum < BaseEnum
value 'Issue' value 'COMMIT', value: 'Commit', description: 'A Commit'
value 'MergeRequest' value 'ISSUE', value: 'Issue', description: 'An Issue'
value 'Epic' value 'MERGEREQUEST', value: 'MergeRequest', description: 'A MergeRequest'
end end
end end
Types::TodoTargetEnum.prepend_if_ee('::EE::Types::TodoTargetEnum')
...@@ -40,7 +40,8 @@ module Types ...@@ -40,7 +40,8 @@ module Types
field :body, GraphQL::STRING_TYPE, field :body, GraphQL::STRING_TYPE,
description: 'Body of the todo', description: 'Body of the todo',
null: false null: false,
calls_gitaly: true # TODO This is only true when `target_type` is `Commit`. See https://gitlab.com/gitlab-org/gitlab/issues/34757#note_234752665
field :state, Types::TodoStateEnum, field :state, Types::TodoStateEnum,
description: 'State of the todo', description: 'State of the todo',
......
---
title: Fix errors in GraphQL Todos API due to missing TargetTypeEnum values
merge_request: 19052
author:
type: fixed
# frozen_string_literal: true
module EE
module Types
module TodoTargetEnum
extend ActiveSupport::Concern
prepended do
value 'DESIGN', value: 'DesignManagement::Design', description: 'A Design'
value 'EPIC', value: 'Epic', description: 'An Epic'
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'getting project information' do
include GraphqlHelpers
set(:current_user) { create(:user) }
set(:design_todo) { create(:todo, user: current_user, target: create(:design)) }
set(:epic_todo) { create(:todo, user: current_user, target: create(:epic)) }
let(:fields) do
<<~QUERY
nodes {
#{all_graphql_fields_for('todos'.classify)}
}
QUERY
end
let(:query) do
graphql_query_for('currentUser', {}, query_graphql_field('todos', {}, fields))
end
subject { graphql_data.dig('currentUser', 'todos', 'nodes') }
before do
post_graphql(query, current_user: current_user)
end
it_behaves_like 'a working graphql query'
it 'returns Todos for all target types' do
is_expected.to include(
a_hash_including('targetType' => 'DESIGN'),
a_hash_including('targetType' => 'EPIC')
)
end
end
...@@ -13,7 +13,7 @@ describe 'Query current user todos' do ...@@ -13,7 +13,7 @@ describe 'Query current user todos' do
let(:fields) do let(:fields) do
<<~QUERY <<~QUERY
nodes { nodes {
id #{all_graphql_fields_for('todos'.classify)}
} }
QUERY QUERY
end end
...@@ -28,6 +28,8 @@ describe 'Query current user todos' do ...@@ -28,6 +28,8 @@ describe 'Query current user todos' do
post_graphql(query, current_user: current_user) post_graphql(query, current_user: current_user)
end end
it_behaves_like 'a working graphql query'
it 'contains the expected ids' do it 'contains the expected ids' do
is_expected.to include( is_expected.to include(
a_hash_including('id' => commit_todo.to_global_id.to_s), a_hash_including('id' => commit_todo.to_global_id.to_s),
...@@ -35,4 +37,12 @@ describe 'Query current user todos' do ...@@ -35,4 +37,12 @@ describe 'Query current user todos' do
a_hash_including('id' => merge_request_todo.to_global_id.to_s) a_hash_including('id' => merge_request_todo.to_global_id.to_s)
) )
end end
it 'returns Todos for all target types' do
is_expected.to include(
a_hash_including('targetType' => 'COMMIT'),
a_hash_including('targetType' => 'ISSUE'),
a_hash_including('targetType' => 'MERGEREQUEST')
)
end
end end
# frozen_string_literal: true
require 'spec_helper'
describe 'getting project information' do
include GraphqlHelpers
let(:query) do
graphql_query_for('currentUser', {}, 'name')
end
subject { graphql_data['currentUser'] }
before do
post_graphql(query, current_user: current_user)
end
context 'when there is a current_user' do
set(:current_user) { create(:user) }
it_behaves_like 'a working graphql query'
it { is_expected.to include('name' => current_user.name) }
end
context 'when there is no current_user' do
let(:current_user) { nil }
it_behaves_like 'a working graphql query'
it { is_expected.to be_nil }
end
end
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