Using getRelations of SelectFields involves only selecting the requested fields for relationships
illambo opened this issue · 5 comments
Versions:
- graphql-laravel Version: 8.5.0
- Laravel Version: 9.48.0
- PHP Version: 8.1.14
Description:
Taking as an example (taken from the docs):
public function resolve($root, array $args, $context, ResolveInfo $info, Closure $getSelectFields)
{
/** @var SelectFields $fields */
$fields = $getSelectFields();
$select = $fields->getSelect();
$with = $fields->getRelations();
$users = User::select($select)->with($with);
return $users->get();
}
Using $select = $fields->getSelect();
the developer can filter or not only for the fields requested by the query but this logic it is not applicable at the relationship level, $with = $fields->getRelations();
this filter is forced (you do not have the option to select "*").
I apologize if I may not have expressed the problem well.
When nested relations are extracted within the query, only the fields requested for the relation in these are extracted. This leads to problems if there are calculated fields that rely on missing fields from the nested relationship, which are not retrieved by the query and therefore are not available during field processing, generating an incorrect result.
We would like to have the possibility to be able to select the entire recordset for the required relations, maybe with an extra parameters in the getRelations method ?
Having to "play" with the always attribute it doesn't seem like the correct solution to us because involves redundancy of the eloquent model logic inside graphql.
What do you think ? (We are aware of the related discussion)
Thank you
Sorry, same answer as in #979 . I try to keep SelectFields
alive as best as possible, but TBH I've a hard time maintaining and (and also no fun doing so) and I also never truly used it, dataloaders are preferred.
Ok, as soon as I can I elaborate tests and pr.
Does the https://github.com/rebing/graphql-laravel#eager-loading-relationships always-field mentioned here, fixes it for you?
or the query param?
https://github.com/rebing/graphql-laravel#type-relationship-query
Hi @crissi, I played with both ways (always field and type relationship query) to get around the problem but in the end it all turned out to be too complicated and logically incorrect (IMO the graphql layer must not be aware of the fields needed for any computed field logics on the eloquent side), so I chose another solution, extend SelectFields class and overwrite the method handleFields.
I disabled this check, so instead of this:
graphql-laravel/src/Support/SelectFields.php
Lines 254 to 258 in fc0d1d5
I just left this:
$select = [$parentTable.'.*'];
Doing so I "solved" the problem for me, I hope I was clear and helpful.