fix(graphql): Enforce paginated lists to never be null and nodes to never be null

[wucdbm]

Q	A
Branch?	main
Tickets	-
License	MIT
Doc PR	-

PR just for ease of explanation, would resubmit with proper commit message, tests, etc

When is a paginated list returned without the list or with a list of null values? But at the same time, pagination info is always there?! Makes no sense to me, some information on the matter would be appreciated.

The same applies to cursor pagination - edges list can be a null? a node in an edge can be a null? How can that happen?!

My main issue is automatic type extraction for frontend integrations, where I constantly have to check if the list is there, if an item is actually an item and not a null, etc.

If there's a real possibility for these cases, can we somehow make this typing behaviour configurable instead?

Please share your thoughts, I am completely lost on this.

[soyuka]

this is legit can you add a behat test where this is asserted for ?

[wucdbm]

Definitely. The same applies to the cursor pagination. I'll look into it this or the following week, as I'd need to understand how tests are setup in the project, I only went as far as browsing the source code to find what causes this.

[wucdbm]

@soyuka What's the difference between src/Core/GraphQl/Type/TypeBuilder.php and src/GraphQL/Type/TypeBuilder.php? To my understanding, according to the imports, the former is going away in favor of the latter?

[wucdbm]

TypeBuilderTest::testPageBasedGetResourcePaginatedCollectionType passes, but requires more assertions added as it does not assert what the collection consists of, only its presence.

Edit: Done

[wucdbm]

@soyuka Please review. Furthermore, I am completely clueless as to what the semantic pull request wants from me, so please advise on title.

[wucdbm]

    And the JSON node "data.type2.fields" should contain:
    """
    {
      "name":"edges",
      "type":{
        "name":null,
        "kind":"LIST",
        "ofType":{
          "name":"DummyAggregateOfferEdge",
          "kind":"OBJECT"
        }
      }
    }
    """

must become

    And the JSON node "data.type2.fields" should contain:
    """
    {
      "name":"edges",
      "type":{
        "name":null,
        "kind":"NON_NULL",
        "ofType":{
          "name":null,
          "kind":"LIST"
        }
      }
    }
    """

Which sorta can't test that the edge is of type DummyAggregateOfferEdge A bunch of other tests in the behat suite also fail, what to do about them?

[wucdbm]

Anyone? Do we lose some of the strictness of these behat tests since the type is now wrapped in a notNull, or how else should we proceed, can we somehow fetch a third-level child type or something?

[alanpoulain]

You should test the edge type at one more level then.

[wucdbm]

Please review. Also added two more cases to test the $resourceType instanceof NullableType if-statements, in case type hints are added to Type::nonNull Not sure where to place the DummyNotImplementingNullableInterfaceType class

[wucdbm]

I guess noone cares, if anyone cba, feel free to fork

[alanpoulain]

It will be merged eventually. There are even older PR waiting to be reviewed too.