Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cascading Pagination #17

Closed
rambii opened this issue Jun 14, 2017 · 16 comments
Closed

Cascading Pagination #17

rambii opened this issue Jun 14, 2017 · 16 comments

Comments

@rambii
Copy link
Contributor

rambii commented Jun 14, 2017

Another issue with the same setup as in: #16

I have pagination working correctly and it's and awesome feature!
It works for users and devices separately.

But now I want to query all the devices of a user - with pagination. Like this:

{
  users {
    data {
      id
      devices {
        data {
          id
        }
        total
      }
    }
    total
  }
}

Here you can see an example in the graphQL docs.

I tried to change the relation in the UserType

  • from 'type' => Type::listOf(GraphQL::type('Device'))
  • to 'type' => GraphQL::paginate('Device').

This gives me the error because I use it in the DeviceType Definition
Schema must contain unique named types but contains multiple types named \"Device_pagination\".

I also tried to change it

  • to 'type' => GraphQL::type('Device_pagination')

But this gives me the error: Type Device_pagination not found.

Is there a way to enable those kind of queries?
Thanks for your help.

@rebing
Copy link
Owner

rebing commented Jun 14, 2017

You can set a custom name with the second argument, i.e GraphQL::paginate('device', 'user_devices')

@rambii
Copy link
Contributor Author

rambii commented Jun 15, 2017

When I changed it in the UserType to:

'devices' => [
    'type' => GraphQL::paginate('Device', 'user_devices'),
],

it gives me the error of 'account_id' is not defined for type 'user_devices' as soon as I query any information about user.devices.
I also tried to register this type in the config.graphql.php, but it this wont work neither.

@rebing
Copy link
Owner

rebing commented Jun 15, 2017

Make sure you have 'account_id' defined in your DeviceType class

@vestervang
Copy link

vestervang commented Jun 15, 2017

I have the same problem.

Here are my Models:

NewsArticle

namespace App;

use Illuminate\Database\Eloquent\Model;

class NewsArticle extends Model
{
    protected $table = 'news';

    protected $fillable = [
        'author', 'headline', 'slug', 'article',
    ];

    protected $hidden = [
    ];

    public function author()
    {
        return $this->belongsTo('App\User', 'user_id');
    }
}

User

namespace App;

use Illuminate\Notifications\Notifiable;
use Illuminate\Foundation\Auth\User as Authenticatable;

class User extends Authenticatable
{
    use Notifiable;

    protected $fillable = [
        'name', 'email', 'password',
    ];

    protected $hidden = [
        'password', 'remember_token',
    ];

    public function news()
    {
      return $this->hasMany('App\NewsArticle', 'user_id');
    }
}

Here are my GraphQL types:

NewsArticleType

namespace App\GraphQL\Type;

use GraphQL;
use App\NewsArticle;
use GraphQL\Type\Definition\Type;
use Rebing\GraphQL\Support\Type as GraphQLType;
class NewsArticleType extends GraphQLType {
				
	protected $attributes = [
		'name'          => 'news_article',
		'description'   => 'A news article',
		'model'         => NewsArticle::class,
	];
		
	public function fields()
	{
		return [
			'id' => [
				'type' => Type::nonNull(Type::string()),
				'description' => 'The id of the user',
			],
			'headline' => [
				'type' => Type::string(),
				'description' => 'The headline of the news article',
			],
			'slug' => [
				'type' => Type::string(),
				'description' => 'The slug of the news article'
			],
			'article' => [
				'type' => Type::string(),
				'description' => 'The body of the news article'
			],
			'author' => [
				'type' => GraphQL::type('user'),
				'description' => 'The author of the news article'
			],
		];
	}		
}

UserType

namespace App\GraphQL\Type;

use GraphQL;
use GraphQL\Type\Definition\Type;
use Rebing\GraphQL\Support\Type as GraphQLType;
use App\User;

class UserType extends GraphQLType {
				
	protected $attributes = [
		'name'          => 'user',
		'description'   => 'A user',
		'model'         => User::class,
	];
		
	public function fields()
	{
		return [
			'id' => [
				'type' => Type::nonNull(Type::string()),
				'description' => 'The id of the user',
			],
			'email' => [
				'type' => Type::string(),
				'description' => 'The email of the user',
			],
			'username' => [
				'type' => Type::string(),
				'description' => 'The username of the user'
			],
			'runescape_name' => [
				'type' => Type::string(),
				'description' => 'The runescape name of the user'
			],
			'member' => [
				'type' => Type::boolean(),
				'description' => 'The member status of the user'
			],
			'news' => [
				'type' => GraphQL::paginate('news_article', 'user_news_articles'),
				'description' => 'All the news articles the user has written'
			],
		];
	}
						
	// If you want to resolve the field yourself, you can declare a method
	// with the following format resolve[FIELD_NAME]Field()
	protected function resolveEmailField($root, $args)
	{
		return strtolower($root->email);
	}			
}

Last thing; Here is the query i try to execute.

{
  users{
    total
    per_page
    current_page
    from
    to
    data{
      id
      email
      username
      runescape_name
      member
      news{
        total
        data{
          id
          headline
          slug
          article
        }
      }
    }
  }
}

I get this error message:

Field 'user_id' is not defined for type 'user_news_articles'

I don't know if that clarifies the problem. It's essentially the same problem and now you have some code that produces that error.

Edit
Added The query producing the error

@rebing
Copy link
Owner

rebing commented Jun 15, 2017

@vestervang You should add a user_id field to NewsArticleType

@rambii
Copy link
Contributor Author

rambii commented Jun 15, 2017

@rebing The account_id is set in the DeviceType. Without pagination accessing the account_id for a device will work.

@vestervang
Copy link

vestervang commented Jun 16, 2017

Even if i add the 'user_id' field it says it's not there. My NewsArticleType looks like this now:

namespace App\GraphQL\Type;

use GraphQL;
use App\NewsArticle;
use GraphQL\Type\Definition\Type;
use Rebing\GraphQL\Support\Type as GraphQLType;
class NewsArticleType extends GraphQLType {
				
	protected $attributes = [
		'name'          => 'news_article',
		'description'   => 'A news article',
		'model'         => NewsArticle::class,
	];
		
	public function fields()
	{
		return [
			'id' => [
				'type' => Type::nonNull(Type::string()),
				'description' => 'The id of the user',
			],
			'headline' => [
				'type' => Type::string(),
				'description' => 'The headline of the news article',
			],
			'slug' => [
				'type' => Type::string(),
				'description' => 'The slug of the news article'
			],
			'article' => [
				'type' => Type::string(),
				'description' => 'The body of the news article'
			],
			'author' => [
				'type' => GraphQL::type('user'),
				'description' => 'The author of the news article'
			],
			'user_id' => [
				'type' => Type::int(),
				'description' => 'User id of the author'
			]
		];
	}		
}

It produces the same error which i find a little odd.

Everything is working if i do not nest the pagination.

@vestervang
Copy link

vestervang commented Jul 8, 2017

This issue has been solve as of PR #23.

Edit:

I can't close the issue so someone needs to do that.

@rebing
Copy link
Owner

rebing commented Jul 8, 2017

#23

@rebing rebing closed this as completed Jul 8, 2017
@rambii
Copy link
Contributor Author

rambii commented Jul 24, 2017

After an update the setup still does not work for me.

The query

{
  users {
    data {
      id
      devices {
        data {
          id
        }
        total
      }
    }
    total
  }
}

returns null for every device and the error:

    {
      "message": "Argument 1 passed to Rebing\\GraphQL\\Support\\PaginationType::Rebing\\GraphQL\\Support\\{closure}() must be an instance of Illuminate\\Pagination\\LengthAwarePaginator, array given, called in /var/www/html/vendor/webonyx/graphql-php/src/Executor/Executor.php on line 680",
      "locations": [
        {
          "line": 6,
          "column": 9
        }
      ]
    },
    {
      "message": "Argument 1 passed to Rebing\\GraphQL\\Support\\PaginationType::Rebing\\GraphQL\\Support\\{closure}() must be an instance of Illuminate\\Pagination\\LengthAwarePaginator, array given, called in /var/www/html/vendor/webonyx/graphql-php/src/Executor/Executor.php on line 680",
      "locations": [
        {
          "line": 9,
          "column": 9
        }
      ]
    },

The UserType defines devices like so:

	'devices' => [
                'type' => GraphQL::paginate('Device', 'user_devices'),
	],

Do you know what is wrong?

@rebing
Copy link
Owner

rebing commented Jul 24, 2017

Are you returning a LengthAwarePaginator in the resolve function of your Query, i.e $query->paginate(...)

@rambii
Copy link
Contributor Author

rambii commented Jul 24, 2017

Thanks, that fixed it.

I also needed to use $root->devices() instead of $root->devices to have access to the pagination function.

@PayteR
Copy link

PayteR commented Apr 15, 2019

I have same problem

Argument 1 passed to Rebing\\GraphQL\\Support\\PaginationType::Rebing\\GraphQL\\Support\\{closure}() must be an instance of Illuminate\\Pagination\\LengthAwarePaginator, instance of App\\Models\\Taxonomy given, called in D:\\projects\\test\\api\\vendor\\webonyx\\graphql-php\\src\\Executor\\ReferenceExecutor.php on line 613

Could you please explain where to resolve this pagination? Because if you mean here, then there is already too late, because resolver of main query get's all my posts by using with relations
image

@georgeboot
Copy link
Contributor

@rebing can you please elaborate? I'm also having the same issue.

I've tried adding a custom resolve function (just like @PayteR) but that gives a N+1 problem.

What is the proper way of doing this?

My code:

<?php

declare(strict_types=1);

namespace App\GraphQL\Queries;

use App\Models\Event;
use Closure;
use GraphQL\Type\Definition\ResolveInfo;
use GraphQL\Type\Definition\Type;
use Rebing\GraphQL\Support\Facades\GraphQL;
use Rebing\GraphQL\Support\Query;

class EventsQuery extends Query
{
    protected $attributes = [
        'name' => 'events',
        'description' => 'A query'
    ];

    public function type(): Type
    {
        return GraphQL::paginate('event');
    }

    public function args(): array
    {
        return [
            'limit' => [
                'type' => Type::int(),
                'defaultValue' => 100,
            ],
            'page' => [
                'type' => Type::int(),
                'defaultValue' => 1,
            ],
        ];
    }

    public function resolve($root, $args, $context, ResolveInfo $resolveInfo, Closure $getSelectFields)
    {
        $fields = $getSelectFields();

        return Event::query()
            ->with($fields->getRelations())
            ->select($fields->getSelect())
            ->paginate($args['limit'], ['*'], 'page', $args['page']);
    }
}
<?php

declare(strict_types=1);

namespace App\GraphQL\Types;

use App\Models\Event as EventModel;
use GraphQL\Type\Definition\Type;
use Rebing\GraphQL\Support\Facades\GraphQL;
use Rebing\GraphQL\Support\Type as GraphQLType;

class Event extends GraphQLType
{
    protected $attributes = [
        'name' => 'Event',
        'description' => 'A type',
        'model' => EventModel::class,
    ];

    public function fields(): array
    {
        return [
            'id' => [
                'type' => Type::nonNull(Type::id()),
            ],
            'name' => [
                'type' => Type::string(),
            ],
            'createdAt' => [
                'type' => Type::id(),
                'alias' => 'created_at',
            ],
            'ticketTypes' => [
                'type' => GraphQL::paginate('ticketType'),
                'resolve' => function($root, $args) {
                    // this gives n+1 problem
                    return $root->ticketTypes()->paginate(100);
                },
            ],
        ];
    }
}
<?php

declare(strict_types=1);

namespace App\GraphQL\Types;

use App\Models\TicketType as TicketTypeModel;
use Rebing\GraphQL\Support\Type as GraphQLType;
use GraphQL\Type\Definition\Type;

class TicketType extends GraphQLType
{
    protected $attributes = [
        'name' => 'Ticket',
        'description' => 'A type',
        'model' => TicketTypeModel::class,
    ];

    public function fields(): array
    {
        return [
            'id' => [
                'type' => Type::nonNull(Type::id()),
            ],
            'name' => [
                'type' => Type::string(),
            ],
            'createdAt' => [
                'type' => Type::id(),
                'alias' => 'created_at',
            ],
        ];
    }
}

@rebing
Copy link
Owner

rebing commented Oct 30, 2019

@georgeboot Sorry for the late reply.

I've never tested the pagination with Field resolves. It should work in your EventsQuery example though.

You can try to make it work with fields as well and create a PR.

@aakarim
Copy link

aakarim commented Oct 21, 2020

Hi, any update here on field pagination whilst avoiding the n+1 problem? This is fairly common in GraphQL: https://graphql.org/learn/pagination/

Is simply adding the SelectFields closure to the field resolve function? I could put in a PR for that, if so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants