Add page-based pagination to GraphQL #3175

raoulclais · 2019-10-15T09:00:44Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tickets	fixes #2762, api-platform/api-platform#1152
License	MIT
Doc PR	N/A

This PR allows to use a page-based pagination using GraphQL instead of the default cursor-based pagination. This choice is made at operation/resource level.

alanpoulain · 2019-10-21T15:34:55Z

Nice feature!
Could you add an entry in the CHANGELOG.md file please (in 2.6.0 with the PR number)?

alanpoulain · 2019-10-21T15:48:26Z

features/graphql/collection.feature

+        collection {
+          id
+        }
+        paginationMetadata {


paginationInfo to be coherent with the pageInfo?

alanpoulain · 2019-10-21T15:59:51Z

features/graphql/collection.feature

+    """
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON node "data.fooDummies.collection" should have 1 elements


Suggested change

And the JSON node "data.fooDummies.collection" should have 1 elements

And the JSON node "data.fooDummies.collection" should have 1 element

alanpoulain · 2019-10-21T16:00:56Z

features/graphql/collection.feature

+    And the JSON node "data.fooDummies.collection" should have 0 elements
+
+  @createSchema
+  Scenario: Retrieve a paginated collection using page-based pagination and client defined limit


Suggested change

Scenario: Retrieve a paginated collection using page-based pagination and client defined limit

Scenario: Retrieve a paginated collection using page-based pagination and client-defined limit

alanpoulain · 2019-10-21T16:05:57Z

src/DataProvider/Pagination.php

+    public function getGraphQlPaginationType(string $resourceClass = null, string $operationName = null): string
+    {
+        $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
+
+        return (string) $resourceMetadata->getGraphqlAttribute($operationName, 'paginationType', 'cursor', true);
+    }


An if $resourceClass or $operationName is null? Maybe they should not be nullable?

alanpoulain · 2019-10-21T16:07:27Z

src/GraphQl/Resolver/Stage/SerializeStage.php

@@ -91,7 +93,10 @@ public function __invoke($itemOrCollection, string $resourceClass, string $opera
                    $data[$index] = $this->normalizer->normalize($object, ItemNormalizer::FORMAT, $normalizationContext);
                }
            } else {
-                $data = $this->serializePaginatedCollection($itemOrCollection, $normalizationContext, $context);
+                $paginationType = $this->pagination->getGraphQlPaginationType($resourceClass, $operationName);


Like above, this variable is not needed.

alanpoulain · 2019-10-21T16:10:33Z

src/GraphQl/Resolver/Stage/SerializeStage.php

+        $info = $context['info'];
+
+        if (!($collection instanceof PaginatorInterface)) {
+            throw Error::createLocatedError(sprintf('Collection returned by the collection data provider must implement %s', PaginatorInterface::class), $info->fieldNodes, $info->path);


Please throw a \LogicException instead (see: https://github.com/api-platform/core/pull/3063/files).

alanpoulain · 2019-10-21T16:11:35Z

src/GraphQl/Type/FieldsBuilder.php

+                        $args = [
+                            'page' => [
+                                'type' => GraphQLType::int(),
+                                'description' => 'The current page.',


Suggested change

'description' => 'The current page.',

'description' => 'Returns the current page.',

alanpoulain · 2019-10-21T16:12:00Z

src/GraphQl/Type/FieldsBuilder.php

+                        if ($itemsPerPageOptions['client_items_per_page']) {
+                            $args[$itemsPerPageOptions['items_per_page_parameter_name']] = [
+                                'type' => GraphQLType::int(),
+                                'description' => 'The number of items per page.',


Suggested change

'description' => 'The number of items per page.',

'description' => 'Returns the number of items per page.',

alanpoulain · 2019-10-21T16:13:08Z

src/GraphQl/Type/FieldsBuilder.php

-                            'description' => 'Returns the elements in the list that come after the specified cursor.',
-                        ],
-                    ];
+                    $paginationType = $this->pagination->getGraphQlPaginationType($resourceClass, $queryName);


Maybe we should introduce a private method here (like getFilterArgs).

mahmoodbazdar · 2019-11-04T12:37:41Z

@raoulclais Any news on this?

alanpoulain · 2019-11-19T13:45:55Z

Thank you @raoulclais! 🙂

* Add page-based pagination to GraphQL * Use page_parameter_name Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

jpierront · 2020-04-23T13:09:12Z

@raoulclais @alanpoulain Hum, any object properties except "id" return a "null" value. If the property is nullable, it return null. If the property is not nullable it crash.

The query:

query getItems {
  items(page: 1, itemsPerPage: 12) {
    collection {
      id
      name
    }
  }
}

With a not null but nullable property:

{
  "data": {
    "items": {
      "collection": [
        {
          "id": "/api/items/1",
          "name": null
        }
      ]
    }
  }
}

With a not null and not nullable property:

  "errors": [
    {
      "debugMessage": "Cannot return null for non-nullable field Item.name.",
      "path": [
        "items",
        "collection",
        0,
        "name"
      ],
      "trace": [
        {
          "file": ".../vendor/webonyx/graphql-php/src/Executor/ReferenceExecutor.php",
          "line": 723,
          "call": "GraphQL\\Executor\\ReferenceExecutor::completeValue(GraphQLType: String, instance of ArrayObject(1), instance of GraphQL\\Type\\Definition\\ResolveInfo, array(4), null)"
        },
    }
  ]

alanpoulain · 2020-04-23T14:00:20Z

@jpierront you're right, I fixed it there: #3517.

raoulclais force-pushed the graphql-page-based-pagination branch from faf63b2 to 5493107 Compare October 16, 2019 10:11

alanpoulain reviewed Oct 21, 2019

View reviewed changes

alanpoulain mentioned this pull request Nov 6, 2019

graphql pagination last/before/startCursor not enabled api-platform/api-platform#1327

Closed

raoulclais force-pushed the graphql-page-based-pagination branch from 538e997 to 3acedd1 Compare November 7, 2019 16:02

alanpoulain force-pushed the graphql-page-based-pagination branch from 6820239 to 9609944 Compare November 19, 2019 12:30

raoulclais and others added 2 commits November 19, 2019 14:22

Add page-based pagination to GraphQL

4ceb0d0

Use page_parameter_name

6653917

alanpoulain force-pushed the graphql-page-based-pagination branch from 9609944 to 6653917 Compare November 19, 2019 13:22

alanpoulain merged commit bf8afd3 into api-platform:master Nov 19, 2019

norkunas pushed a commit to norkunas/core that referenced this pull request Dec 2, 2019

Add page-based pagination to GraphQL (api-platform#3175)

5820262

* Add page-based pagination to GraphQL * Use page_parameter_name Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

alanpoulain mentioned this pull request Apr 23, 2020

[GraphQL] Fix serializer context attributes for page-based pagination #3517

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add page-based pagination to GraphQL #3175

Add page-based pagination to GraphQL #3175

raoulclais commented Oct 15, 2019 •

edited by alanpoulain

Loading

alanpoulain commented Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

alanpoulain Oct 21, 2019

mahmoodbazdar commented Nov 4, 2019

alanpoulain commented Nov 19, 2019

jpierront commented Apr 23, 2020

alanpoulain commented Apr 23, 2020

	And the JSON node "data.fooDummies.collection" should have 1 elements
	And the JSON node "data.fooDummies.collection" should have 1 element

	Scenario: Retrieve a paginated collection using page-based pagination and client defined limit
	Scenario: Retrieve a paginated collection using page-based pagination and client-defined limit

	'description' => 'The current page.',
	'description' => 'Returns the current page.',

	'description' => 'The number of items per page.',
	'description' => 'Returns the number of items per page.',

Add page-based pagination to GraphQL #3175

Add page-based pagination to GraphQL #3175

Conversation

raoulclais commented Oct 15, 2019 • edited by alanpoulain Loading

alanpoulain commented Oct 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mahmoodbazdar commented Nov 4, 2019

alanpoulain commented Nov 19, 2019

jpierront commented Apr 23, 2020

alanpoulain commented Apr 23, 2020

raoulclais commented Oct 15, 2019 •

edited by alanpoulain

Loading