Add pipeline lists to GraphQL

This adds Keyset pagination to GraphQL lists. PoC for that is
pipelines on merge requests and projects.

When paginating a list, the base-64 encoded id of the ordering
field (in most cases the primary key) can be passed in the `before` or
`after` GraphQL argument.

app/controllers/projects/pipelines_controller.rb

@@ -13,7 +13,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   def index
     @scope = params[:scope]
     @pipelines = PipelinesFinder
-      .new(project, scope: @scope)
+      .new(project, current_user, scope: @scope)
       .execute
       .page(params[:page])
       .per(30)
@@ -178,7 +178,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   end
 
   def limited_pipelines_count(project, scope = nil)
-    finder = PipelinesFinder.new(project, scope: scope)
+    finder = PipelinesFinder.new(project, current_user, scope: scope)
 
     view_context.limited_counter_with_delimiter(finder.execute)
   end

app/finders/pipelines_finder.rb

@@ -1,15 +1,20 @@
 class PipelinesFinder
-  attr_reader :project, :pipelines, :params
+  attr_reader :project, :pipelines, :params, :current_user
 
   ALLOWED_INDEXED_COLUMNS = %w[id status ref user_id].freeze
 
-  def initialize(project, params = {})
+  def initialize(project, current_user, params = {})
     @project = project
+    @current_user = current_user
     @pipelines = project.pipelines
     @params = params
   end
 
   def execute
+    unless Ability.allowed?(current_user, :read_pipeline, project)
+      return Ci::Pipeline.none
+    end
+
     items = pipelines
     items = by_scope(items)
     items = by_status(items)

app/graphql/gitlab_schema.rb

@@ -2,7 +2,10 @@ class GitlabSchema < GraphQL::Schema
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize
   use Gitlab::Graphql::Present
+  use Gitlab::Graphql::Connections
 
   query(Types::QueryType)
+
+  default_max_page_size 100
   # mutation(Types::MutationType)
 end

app/graphql/resolvers/concerns/resolves_pipelines.rb

@@ -0,0 +1,23 @@
+module ResolvesPipelines
+  extend ActiveSupport::Concern
+
+  included do
+    type [Types::Ci::PipelineType], null: false
+    argument :status,
+             Types::Ci::PipelineStatusEnum,
+             required: false,
+             description: "Filter pipelines by their status"
+    argument :ref,
+             GraphQL::STRING_TYPE,
+             required: false,
+             description: "Filter pipelines by the ref they are run for"
+    argument :sha,
+             GraphQL::STRING_TYPE,
+             required: false,
+             description: "Filter pipelines by the sha of the commit they are run for"
+  end
+
+  def resolve_pipelines(project, params = {})
+    PipelinesFinder.new(project, context[:current_user], params).execute
+  end
+end

app/graphql/resolvers/merge_request_pipelines_resolver.rb

@@ -0,0 +1,16 @@
+module Resolvers
+  class MergeRequestPipelinesResolver < BaseResolver
+    include ::ResolvesPipelines
+
+    alias_method :merge_request, :object
+
+    def resolve(**args)
+      resolve_pipelines(project, args)
+        .merge(merge_request.all_pipelines)
+    end
+
+    def project
+      merge_request.source_project
+    end
+  end
+end

app/graphql/resolvers/project_pipelines_resolver.rb

@@ -0,0 +1,11 @@
+module Resolvers
+  class ProjectPipelinesResolver < BaseResolver
+    include ResolvesPipelines
+
+    alias_method :project, :object
+
+    def resolve(**args)
+      resolve_pipelines(project, args)
+    end
+  end
+end

app/graphql/types/ci/pipeline_status_enum.rb

@@ -0,0 +1,9 @@
+module Types
+  module Ci
+    class PipelineStatusEnum < BaseEnum
+      ::Ci::Pipeline.all_state_names.each do |state_symbol|
+        value state_symbol.to_s.upcase, value: state_symbol.to_s
+      end
+    end
+  end
+end

app/graphql/types/ci/pipeline_type.rb

@@ -0,0 +1,31 @@
+module Types
+  module Ci
+    class PipelineType < BaseObject
+      expose_permissions Types::PermissionTypes::Ci::Pipeline
+
+      graphql_name 'Pipeline'
+
+      field :id, GraphQL::ID_TYPE, null: false
+      field :iid, GraphQL::ID_TYPE, null: false
+
+      field :sha, GraphQL::STRING_TYPE, null: false
+      field :before_sha, GraphQL::STRING_TYPE, null: true
+      field :status, PipelineStatusEnum, null: false
+      field :duration,
+            GraphQL::INT_TYPE,
+            null: true,
+            description: "Duration of the pipeline in seconds"
+      field :coverage,
+            GraphQL::FLOAT_TYPE,
+            null: true,
+            description: "Coverage percentage"
+      field :created_at, Types::TimeType, null: false
+      field :updated_at, Types::TimeType, null: false
+      field :started_at, Types::TimeType, null: true
+      field :finished_at, Types::TimeType, null: true
+      field :committed_at, Types::TimeType, null: true
+
+      # TODO: Add triggering user as a type
+    end
+  end
+end

app/graphql/types/merge_request_type.rb

@@ -45,5 +45,11 @@ module Types
     field :upvotes, GraphQL::INT_TYPE, null: false
     field :downvotes, GraphQL::INT_TYPE, null: false
     field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false
+
+    field :head_pipeline, Types::Ci::PipelineType, null: true, method: :actual_head_pipeline do
+      authorize :read_pipeline
+    end
+    field :pipelines, Types::Ci::PipelineType.connection_type,
+          resolver: Resolvers::MergeRequestPipelinesResolver
   end
 end

app/graphql/types/permission_types/ci/pipeline.rb

@@ -0,0 +1,11 @@
+module Types
+  module PermissionTypes
+    module Ci
+      class Pipeline < BasePermissionType
+        graphql_name 'PipelinePermissions'
+
+        abilities :update_pipeline, :admin_pipeline, :destroy_pipeline
+      end
+    end
+  end
+end

app/graphql/types/project_type.rb

@@ -70,5 +70,10 @@ module Types
           resolver: Resolvers::MergeRequestResolver do
       authorize :read_merge_request
     end
+
+    field :pipelines,
+          Types::Ci::PipelineType.connection_type,
+          null: false,
+          resolver: Resolvers::ProjectPipelinesResolver
   end
 end

changelogs/unreleased/bvl-graphql-pipeline-lists.yml

@@ -0,0 +1,5 @@
+---
+title: Add pipeline lists to GraphQL
+merge_request: 20249
+author:
+type: added

config/application.rb

@@ -45,7 +45,8 @@ module Gitlab
                                      #{config.root}/app/workers/concerns
                                      #{config.root}/app/services/concerns
                                      #{config.root}/app/serializers/concerns
-                                     #{config.root}/app/finders/concerns])
+                                     #{config.root}/app/finders/concerns
+                                     #{config.root}/app/graphql/resolvers/concerns])
 
     config.generators.templates.push("#{config.root}/generator_templates")
 

doc/development/api_graphql_styleguide.md

@@ -54,6 +54,94 @@ a new presenter specifically for GraphQL.
 The presenter is initialized using the object resolved by a field, and
 the context.
 
+### Connection Types
+
+GraphQL uses [cursor based
+pagination](https://graphql.org/learn/pagination/#pagination-and-edges)
+to expose collections of items. This provides the clients with a lot
+of flexibility while also allowing the backend to use different
+pagination models.
+
+To expose a collection of resources we can use a connection type. This wraps the array with default pagination fields. For example a query for project-pipelines could look like this:
+
+```
+query($project_path: ID!) {
+  project(fullPath: $project_path) {
+    pipelines(first: 2) {
+      pageInfo {
+        hasNextPage
+        hasPreviousPage
+      }
+      edges {
+        cursor
+        node {
+          id
+          status
+        }
+      }
+    }
+  }
+}
+```
+
+This would return the first 2 pipelines of a project and related
+pagination info., ordered by descending ID. The returned data would
+look like this:
+
+```json
+{
+  "data": {
+    "project": {
+      "pipelines": {
+        "pageInfo": {
+          "hasNextPage": true,
+          "hasPreviousPage": false
+        },
+        "edges": [
+          {
+            "cursor": "Nzc=",
+            "node": {
+              "id": "77",
+              "status": "FAILED"
+            }
+          },
+          {
+            "cursor": "Njc=",
+            "node": {
+              "id": "67",
+              "status": "FAILED"
+            }
+          }
+        ]
+      }
+    }
+  }
+}
+```
+
+To get the next page, the cursor of the last known element could be
+passed:
+
+```
+query($project_path: ID!) {
+  project(fullPath: $project_path) {
+    pipelines(first: 2, after: "Njc=") {
+      pageInfo {
+        hasNextPage
+        hasPreviousPage
+      }
+      edges {
+        cursor
+        node {
+          id
+          status
+        }
+      }
+    }
+  }
+}
+```
+
 ### Exposing permissions for a type
 
 To expose permissions the current user has on a resource, you can call

lib/api/pipelines.rb

@@ -31,7 +31,7 @@ module API
       get ':id/pipelines' do
         authorize! :read_pipeline, user_project
 
-        pipelines = PipelinesFinder.new(user_project, params).execute
+        pipelines = PipelinesFinder.new(user_project, current_user, params).execute
         present paginate(pipelines), with: Entities::PipelineBasic
       end
 

lib/gitlab/graphql/connections.rb

@@ -0,0 +1,12 @@
+module Gitlab
+  module Graphql
+    module Connections
+      def self.use(_schema)
+        GraphQL::Relay::BaseConnection.register_connection_implementation(
+          ActiveRecord::Relation,
+          Gitlab::Graphql::Connections::KeysetConnection
+        )
+      end
+    end
+  end
+end

lib/gitlab/graphql/connections/keyset_connection.rb

@@ -0,0 +1,73 @@
+module Gitlab
+  module Graphql
+    module Connections
+      class KeysetConnection < GraphQL::Relay::BaseConnection
+        def cursor_from_node(node)
+          encode(node[order_field].to_s)
+        end
+
+        def sliced_nodes
+          @sliced_nodes ||=
+            begin
+              sliced = nodes
+
+              sliced = sliced.where(before_slice) if before.present?
+              sliced = sliced.where(after_slice) if after.present?
+
+              sliced
+            end
+        end
+
+        def paged_nodes
+          if first && last
+            raise Gitlab::Graphql::Errors::ArgumentError.new("Can only provide either `first` or `last`, not both")
+          end
+
+          if last
+            sliced_nodes.last(limit_value)
+          else
+            sliced_nodes.limit(limit_value)
+          end
+        end
+
+        private
+
+        def before_slice
+          if sort_direction == :asc
+            table[order_field].lt(decode(before))
+          else
+            table[order_field].gt(decode(before))
+          end
+        end
+
+        def after_slice
+          if sort_direction == :asc
+            table[order_field].gt(decode(after))
+          else
+            table[order_field].lt(decode(after))
+          end
+        end
+
+        def limit_value
+          @limit_value ||= [first, last, max_page_size].compact.min
+        end
+
+        def table
+          nodes.arel_table
+        end
+
+        def order_info
+          @order_info ||= nodes.order_values.first
+        end
+
+        def order_field
+          @order_field ||= order_info&.expr&.name || nodes.primary_key
+        end
+
+        def sort_direction
+          @order_direction ||= order_info&.direction || :desc
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/errors.rb

@@ -0,0 +1,8 @@
+module Gitlab
+  module Graphql
+    module Errors
+      BaseError = Class.new(GraphQL::ExecutionError)
+      ArgumentError = Class.new(BaseError)
+    end
+  end
+end

lib/gitlab/graphql/present/instrumentation.rb

@@ -3,6 +3,8 @@ module Gitlab
     module Present
       class Instrumentation
         def instrument(type, field)
+          return field unless field.metadata[:type_class]
+
           presented_in = field.metadata[:type_class].owner
           return field unless presented_in.respond_to?(:presenter_class)
           return field unless presented_in.presenter_class

spec/controllers/projects/pipelines_controller_spec.rb

@@ -4,7 +4,7 @@ describe Projects::PipelinesController do
   include ApiHelpers
 
   set(:user) { create(:user) }
-  set(:project) { create(:project, :public, :repository) }
+  let(:project) { create(:project, :public, :repository) }
   let(:feature) { ProjectFeature::DISABLED }
 
   before do
@@ -91,6 +91,24 @@ describe Projects::PipelinesController do
       end
     end
 
+    context 'when the project is private' do
+      let(:project) { create(:project, :private, :repository) }
+
+      it 'returns `not_found` when the user does not have access' do
+        sign_in(create(:user))
+
+        get_pipelines_index_json
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      it 'returns the pipelines when the user has access' do
+        get_pipelines_index_json
+
+        expect(json_response['pipelines'].size).to eq(5)
+      end
+    end
+
     def get_pipelines_index_json
       get :index, namespace_id: project.namespace,
                   project_id: project,

spec/finders/pipelines_finder_spec.rb

@@ -1,9 +1,10 @@
 require 'spec_helper'
 
 describe PipelinesFinder do
-  let(:project) { create(:project, :repository) }
-
-  subject { described_class.new(project, params).execute }
+  let(:project) { create(:project, :public, :repository) }
+  let(:current_user) { nil }
+  let(:params) { {} }
+  subject { described_class.new(project, current_user, params).execute }
 
   describe "#execute" do
     context 'when params is empty' do
@@ -223,5 +224,27 @@ describe PipelinesFinder do
         end
       end
     end
+
+    context 'when the project has limited access to piplines' do
+      let(:project) { create(:project, :private, :repository) }
+      let(:current_user) { create(:user) }
+      let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) }
+
+      context 'when the user has access' do
+        before do
+          project.add_developer(current_user)
+        end
+
+        it 'is expected to return pipelines' do
+          is_expected.to contain_exactly(*pipelines)
+        end
+      end
+
+      context 'the user is not allowed to read pipelines' do
+        it 'returns empty' do
+          is_expected.to be_empty
+        end
+      end
+    end
   end
 end

spec/graphql/gitlab_schema_spec.rb

@@ -27,6 +27,12 @@ describe GitlabSchema do
     expect(described_class.query).to eq(::Types::QueryType.to_graphql)
   end
 
+  it 'paginates active record relations using `Gitlab::Graphql::Connections::KeysetConnection`' do
+    connection = GraphQL::Relay::BaseConnection::CONNECTION_IMPLEMENTATIONS[ActiveRecord::Relation.name]
+
+    expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection)
+  end
+
   def field_instrumenters
     described_class.instrumenters[:field]
   end

spec/graphql/resolvers/concerns/resolves_pipelines_spec.rb

@@ -0,0 +1,52 @@
+require 'spec_helper'
+
+describe ResolvesPipelines do
+  include GraphqlHelpers
+
+  subject(:resolver) do
+    Class.new(Resolvers::BaseResolver) do
+      include ResolvesPipelines
+
+      def resolve(**args)
+        resolve_pipelines(object, args)
+      end
+    end
+  end
+
+  let(:current_user) { create(:user) }
+  set(:project) { create(:project, :private) }
+  set(:pipeline) { create(:ci_pipeline, project: project) }
+  set(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) }
+  set(:ref_pipeline) { create(:ci_pipeline, project: project, ref: 'awesome-feature') }
+  set(:sha_pipeline) { create(:ci_pipeline, project: project, sha: 'deadbeef') }
+
+  before do
+    project.add_developer(current_user)
+  end
+
+  it { is_expected.to have_graphql_arguments(:status, :ref, :sha) }
+
+  it 'finds all pipelines' do
+    expect(resolve_pipelines).to contain_exactly(pipeline, failed_pipeline, ref_pipeline, sha_pipeline)
+  end
+
+  it 'allows filtering by status' do
+    expect(resolve_pipelines(status: 'failed')).to contain_exactly(failed_pipeline)
+  end
+
+  it 'allows filtering by ref' do
+    expect(resolve_pipelines(ref: 'awesome-feature')).to contain_exactly(ref_pipeline)
+  end
+
+  it 'allows filtering by sha' do
+    expect(resolve_pipelines(sha: 'deadbeef')).to contain_exactly(sha_pipeline)
+  end
+
+  it 'does not return any pipelines if the user does not have access' do
+    expect(resolve_pipelines({}, {})).to be_empty
+  end
+
+  def resolve_pipelines(args = {}, context = { current_user: current_user })
+    resolve(resolver, obj: project, args: args, ctx: context)
+  end
+end

spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe Resolvers::MergeRequestPipelinesResolver do
+  include GraphqlHelpers
+
+  set(:merge_request) { create(:merge_request) }
+  set(:pipeline) do
+    create(
+      :ci_pipeline,
+      project: merge_request.source_project,
+      ref: merge_request.source_branch,
+      sha: merge_request.diff_head_sha
+    )
+  end
+  set(:other_project_pipeline) { create(:ci_pipeline, project: merge_request.source_project) }
+  set(:other_pipeline) { create(:ci_pipeline) }
+  let(:current_user) { create(:user) }
+
+  before do
+    merge_request.project.add_developer(current_user)
+  end
+
+  def resolve_pipelines
+    resolve(described_class, obj: merge_request, ctx: { current_user: current_user })
+  end
+
+  it 'resolves only MRs for the passed merge request' do
+    expect(resolve_pipelines).to contain_exactly(pipeline)
+  end
+end

spec/graphql/resolvers/project_pipelines_resolver_spec.rb

@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe Resolvers::ProjectPipelinesResolver do
+  include GraphqlHelpers
+
+  set(:project) { create(:project) }
+  set(:pipeline) {  create(:ci_pipeline, project: project) }
+  set(:other_pipeline) { create(:ci_pipeline) }
+  let(:current_user) { create(:user) }
+
+  before do
+    project.add_developer(current_user)
+  end
+
+  def resolve_pipelines
+    resolve(described_class, obj: project, ctx: { current_user: current_user })
+  end
+
+  it 'resolves only MRs for the passed merge request' do
+    expect(resolve_pipelines).to contain_exactly(pipeline)
+  end
+end

spec/graphql/types/ci/pipeline_type_spec.rb

@@ -0,0 +1,7 @@
+require 'spec_helper'
+
+describe Types::Ci::PipelineType do
+  it { expect(described_class.graphql_name).to eq('Pipeline') }
+
+  it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Ci::Pipeline) }
+end

spec/graphql/types/merge_request_type_spec.rb

@@ -1,5 +1,16 @@
 require 'spec_helper'
 
-describe Types::MergeRequestType do
+describe GitlabSchema.types['MergeRequest'] do
   it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::MergeRequest) }
+
+  describe 'head pipeline' do
+    it 'has a head pipeline field' do
+      expect(described_class).to have_graphql_field(:head_pipeline)
+    end
+
+    it 'authorizes the field' do
+      expect(described_class.fields['headPipeline'])
+        .to require_graphql_authorizations(:read_pipeline)
+    end
+  end
 end

spec/graphql/types/project_type_spec.rb

@@ -13,4 +13,6 @@ describe GitlabSchema.types['Project'] do
         .to  require_graphql_authorizations(:read_merge_request)
     end
   end
+
+  it { is_expected.to have_graphql_field(:pipelines) }
 end

spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb

@@ -0,0 +1,112 @@
+require 'spec_helper'
+
+describe Gitlab::Graphql::Connections::KeysetConnection do
+  let(:nodes) { Project.all.order(id: :asc) }
+  let(:arguments) {  {} }
+  subject(:connection) do
+    described_class.new(nodes, arguments, max_page_size: 3)
+  end
+
+  def encoded_property(value)
+    Base64.strict_encode64(value.to_s)
+  end
+
+  describe '#cursor_from_nodes' do
+    let(:project) { create(:project) }
+
+    it 'returns an encoded ID' do
+      expect(connection.cursor_from_node(project))
+        .to eq(encoded_property(project.id))
+    end
+
+    context 'when an order was specified' do
+      let(:nodes) { Project.order(:updated_at) }
+
+      it 'returns the encoded value of the order' do
+        expect(connection.cursor_from_node(project))
+          .to eq(encoded_property(project.updated_at))
+      end
+    end
+  end
+
+  describe '#sliced_nodes' do
+    let(:projects) { create_list(:project, 4) }
+
+    context 'when before is passed' do
+      let(:arguments) { { before: encoded_property(projects[1].id) } }
+
+      it 'only returns the project before the selected one' do
+        expect(subject.sliced_nodes).to contain_exactly(projects.first)
+      end
+
+      context 'when the sort order is descending' do
+        let(:nodes) { Project.all.order(id: :desc) }
+
+        it 'returns the correct nodes' do
+          expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1])
+        end
+      end
+    end
+
+    context 'when after is passed' do
+      let(:arguments) { { after: encoded_property(projects[1].id) } }
+
+      it 'only returns the project before the selected one' do
+        expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1])
+      end
+
+      context 'when the sort order is descending' do
+        let(:nodes) { Project.all.order(id: :desc) }
+
+        it 'returns the correct nodes' do
+          expect(subject.sliced_nodes).to contain_exactly(projects.first)
+        end
+      end
+    end
+
+    context 'when both before and after are passed' do
+      let(:arguments) do
+        {
+          after: encoded_property(projects[1].id),
+          before: encoded_property(projects[3].id)
+        }
+      end
+
+      it 'returns the expected set' do
+        expect(subject.sliced_nodes).to contain_exactly(projects[2])
+      end
+    end
+  end
+
+  describe '#paged_nodes' do
+    let!(:projects) { create_list(:project, 5) }
+
+    it 'returns the collection limited to max page size' do
+      expect(subject.paged_nodes.size).to eq(3)
+    end
+
+    context 'when `first` is passed' do
+      let(:arguments) { { first: 2 } }
+
+      it 'returns only the first elements' do
+        expect(subject.paged_nodes).to contain_exactly(projects.first, projects.second)
+      end
+    end
+
+    context 'when `last` is passed' do
+      let(:arguments) { { last: 2 } }
+
+      it 'returns only the last elements' do
+        expect(subject.paged_nodes).to contain_exactly(projects[3], projects[4])
+      end
+    end
+
+    context 'when both are passed' do
+      let(:arguments) { { first: 2, last: 2 } }
+
+      it 'raises an error' do
+        expect { subject.paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+      end
+    end
+  end
+end

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

@@ -67,4 +67,28 @@ describe 'getting merge request information nested in a project' do
       expect(merge_request_graphql_data).to be_nil
     end
   end
+
+  context 'when there are pipelines' do
+    before do
+      pipeline = create(
+        :ci_pipeline,
+        project: merge_request.source_project,
+        ref: merge_request.source_branch,
+        sha: merge_request.diff_head_sha
+      )
+      merge_request.update!(head_pipeline: pipeline)
+    end
+
+    it 'has a head pipeline' do
+      post_graphql(query, current_user: current_user)
+
+      expect(merge_request_graphql_data['headPipeline']).to be_present
+    end
+
+    it 'has pipeline connections' do
+      post_graphql(query, current_user: current_user)
+
+      expect(merge_request_graphql_data['pipelines']['edges'].size).to eq(1)
+    end
+  end
 end

spec/requests/api/graphql/project_query_spec.rb

@@ -26,6 +26,18 @@ describe 'getting project information' do
         post_graphql(query, current_user: current_user)
       end
     end
+
+    context 'when there are pipelines present' do
+      before do
+        create(:ci_pipeline, project: project)
+      end
+
+      it 'is included in the pipelines connection' do
+        post_graphql(query, current_user: current_user)
+
+        expect(graphql_data['project']['pipelines']['edges'].size).to eq(1)
+      end
+    end
   end
 
   context 'when the user does not have access to the project' do

spec/support/helpers/graphql_helpers.rb

@@ -57,12 +57,12 @@ module GraphqlHelpers
 
     type.fields.map do |name, field|
       # We can't guess arguments, so skip fields that require them
-      next if field.arguments.any?
+      next if required_arguments?(field)
 
-      if scalar?(field)
-        name
-      else
+      if nested_fields?(field)
         "#{name} { #{all_graphql_fields_for(field_type(field))} }"
+      else
+        name
       end
     end.compact.join("\n")
   end
@@ -85,10 +85,22 @@ module GraphqlHelpers
     json_response['data']
   end
 
+  def nested_fields?(field)
+    !scalar?(field) && !enum?(field)
+  end
+
   def scalar?(field)
     field_type(field).kind.scalar?
   end
 
+  def enum?(field)
+    field_type(field).kind.enum?
+  end
+
+  def required_arguments?(field)
+    field.arguments.values.any? { |argument| argument.type.non_null? }
+  end
+
   def field_type(field)
     if field.type.respond_to?(:of_type)
       field.type.of_type