Skip to content

Conversation

jotwea
Copy link
Contributor

@jotwea jotwea commented Jan 5, 2024

Q A
Branch? main
Tickets Closes #6083
License MIT
Doc PR -

This PR introduces a new the test case especially for properties and Many-To-One-Relations being null. It fixes the issues relevant for 3.2 and additionally provides a fix for the nested collection feature which was recently introduced on the main branch. There is another PR #6092 that contains only the fixes relevant for 3.2.

@jotwea jotwea force-pushed the resolver-factory branch 2 times, most recently from 1beabf6 to 6e82a2e Compare January 7, 2024 13:39
}

if (null === $resourceClass && \array_key_exists($info->fieldName, $source ?? [])) {
if ((null === $resourceClass || !is_iterable($body)) && \array_key_exists($info->fieldName, $source ?? [])) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meanwhile I found a working solution: this tiny change fixes the issue with null values.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we check the PropertyMetadata type for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored

@soyuka
Copy link
Member

soyuka commented Jan 8, 2024

you can target the lowest branch, 3.x are merged back

public function __invoke(string $resourceClass = null, string $rootClass = null, Operation $operation = null): callable
{
return function (?array $source, array $args, $context, ResolveInfo $info) use ($resourceClass, $rootClass, $operation) {
// special treatment for nested resources without a resolver/provider
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mhh this line is from the other PR right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It was introduced by other PR. I moved this block one level up, though it also gets called when nested collection is null

Copy link
Contributor Author

@jotwea jotwea Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created another PR based on 3.2 that only contains the bugfix relevant for 3.2. This one here also contains the null-fix for the recently introduced "nested collection" feature

@jotwea jotwea force-pushed the resolver-factory branch from dfa8a54 to aafb9c6 Compare April 3, 2024 12:08
@soyuka
Copy link
Member

soyuka commented Apr 5, 2024

thanks cherry-picked

@soyuka soyuka closed this Apr 5, 2024
@soyuka soyuka mentioned this pull request Apr 5, 2024
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

Successfully merging this pull request may close these issues.

GraphQl: Errors with nullish ManyToOne-Relation when using new ResolverFactory

2 participants