Skip to content

fix error when disabling eager_loading #1581

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
Dec 20, 2017

Conversation

jdeniau
Copy link
Contributor

@jdeniau jdeniau commented Dec 20, 2017

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

Introduced by d3b690e

@@ -412,8 +412,10 @@ private function registerDoctrineExtensionConfiguration(ContainerBuilder $contai
return;
}

$container->removeDefinition('api_platform.doctrine.orm.query_extension.eager_loading');
$container->removeDefinition('api_platform.doctrine.orm.query_extension.filter_eager_loading');
$container->removeAlias('api_platform.doctrine.orm.query_extension.eager_loading');
Copy link
Member

Choose a reason for hiding this comment

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

It should be the inverse IIUC.

Copy link
Member

Choose a reason for hiding this comment

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

👍 this is because of #1557 !

You should remove the alias EagerLoadingExtension::class (please use this instead of the string) and remove the definition by it's id indeed.

Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I misread the error message, the problem is on the alias. I'm fixing it right now

@jdeniau jdeniau force-pushed the jd-fix-autowiringEagerLoading branch from 9edb56f to b232a73 Compare December 20, 2017 15:58
@soyuka
Copy link
Member

soyuka commented Dec 20, 2017

I'm merging 2.1 there are a bunch of conflicts I'll let you know when ready.

@jdeniau jdeniau force-pushed the jd-fix-autowiringEagerLoading branch from b232a73 to b9c9fa6 Compare December 20, 2017 16:01
@jdeniau jdeniau force-pushed the jd-fix-autowiringEagerLoading branch from b9c9fa6 to a502622 Compare December 20, 2017 16:06
@jdeniau
Copy link
Contributor Author

jdeniau commented Dec 20, 2017

Well it's fixed @soyuka @dunglas

Let me know if I need to do something after your merge

@soyuka
Copy link
Member

soyuka commented Dec 20, 2017

Though you were on master my bad!

@dunglas dunglas merged commit d5532d5 into api-platform:2.1 Dec 20, 2017
@jdeniau jdeniau deleted the jd-fix-autowiringEagerLoading branch December 21, 2017 08:07
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
…gerLoading

fix error when disabling eager_loading
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