Skip to content

[EagerLoadingExtension] Remove unset attributes key in normalization context #2279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 29, 2018
Merged

[EagerLoadingExtension] Remove unset attributes key in normalization context #2279

merged 1 commit into from
Oct 29, 2018

Conversation

ArnoudThibaut
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR ~

When an entity had more that one relation, if the first relation was not present in the attributes array in the normalization context the whole array was removed so they was no test for other relations on the next loop iteration.

The aim of this pull request is to fix this behavior and to let the chance to other relations to be eager loaded.

@@ -167,8 +167,6 @@ private function joinRelations(QueryBuilder $queryBuilder, QueryNameGeneratorInt
if ($inAttributes = isset($normalizationContext[AbstractNormalizer::ATTRIBUTES][$association])) {
// prepare the child context
$normalizationContext[AbstractNormalizer::ATTRIBUTES] = $normalizationContext[AbstractNormalizer::ATTRIBUTES][$association];
} else {
unset($normalizationContext[AbstractNormalizer::ATTRIBUTES]);
Copy link
Member

Choose a reason for hiding this comment

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

@dunglas I think you added this but it does not look useful to me, I'd be 👍 for this patch, any reason we should keep this?

@dunglas dunglas merged commit 91e5bb2 into api-platform:2.3 Oct 29, 2018
@dunglas
Copy link
Member

dunglas commented Oct 29, 2018

Thanks Thibaut!

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.

3 participants