NonNull type for data in pagination and NonNull ListOf type

[do6po]

Versions:

• graphql-laravel Version: all with current
• Laravel Version: 8.77.1
• PHP Version:8.0

Description:

I want to make the list (data) and type inside the paginator to be required. But there is a problem with the possible selectable fields. This is due to the lack of a recursion parameter when calling the getWrappedType method in the SelectFields class on line 188. If you pass an argument to the getWrappedType method to retrieve data recursively, everything will work correctly.

like this

                if (is_a($parentType, Config::get('graphql.pagination_type', PaginationType::class)) ||
                    is_a($parentType, Config::get('graphql.simple_pagination_type', SimplePaginationType::class))) {
                    /* @var GraphqlType $fieldType */
                    $fieldType = $fieldObject->config['type'];
                    static::handleFields(
                        $queryArgs,
                        $field,
                        $fieldType->getWrappedType(true), //<---
                        $select,
                        $with,
                        $ctx
                    );
                }

Steps To Reproduce:

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

and inside extends PaginationType

    protected function getPaginationFields(string $typeName): array
    {
        return [
            'data' => [
                'type' => Type::nonNull(
                    Type::listOf(
                        GraphQL::type($typeName)
                    )
                ),
...

[mfn]

I'm not using the paginator in my projects. Do you have a suggestion how to fix this, preferable with tests?

[do6po]

I'm not using the paginator in my projects. Do you have a suggestion how to fix this, preferable with tests?

#878

[jacobdekeizer]

I have the same problem, would be nice if that PR could be merged 😄

[do6po]

I have the same problem, would be nice if that PR could be merged smile

Hi. I am from Ukraine. We have some a little troubles now. When we will got
a clear sky on our head, i fix all troubles with PR and merge it.
Cheers.

[lamtranb]

Hi. I am from Ukraine. We have some a little troubles now. When we will got a clear sky on our head, i fix all troubles with PR and merge it. Cheers.

Hope the best for you.

Could we have any update on this ?

[mfn]

Could we have any update on this ?

Not sure if you mean on this issue or …

A PR was provided but there where issues, so it was rejected. Other than that, we're waiting on OP or someone else to provide a fix.

[lamtranb]

Could we have any update on this ?

Not sure if you mean on this issue or …

A PR was provided but there where issues, so it was rejected. Other than that, we're waiting on OP or someone else to provide a fix.

Sorry for late response. I just have the same problem.
I found the PR. If I follow correctly, do we just need to add the tests you mentioned earlier in #878 so the PR can be merged?

[mfn]

do we just need to add the tests you mentioned

Basically "yes" AFAIR, if they're sufficiently covering the changes and show all is working correctly, we would be back on track.