Skip to content
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

Fix skipping properties if they are listed after a not loaded relation. #1387

Closed

Conversation

lenardpalko
Copy link

This issue fixes an issue that occurs when merging entites, when the entity that is being merged has some other properties after a association type field.

Fixes the following scenario :
Your entity extends a parent entity and when it is merged by the entity manager first its fields are computed, this is done correctly bby ReflectionPropertiesGetter::getProperties(), but in the mergeEntityStateIntoManagedCopy method the iteration is stopped when it gets to a field that is a relationship and it is Proxy and was not loaded yet.
The order in which the fields are computed is : current class properties and then the parent properties, so if the current class has a lazy loaded relationship then the properties of the parent are not merged.

So the fix doesn't stop the iteration it just skips the current property that is not loaded yet and goes to the next property.

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3699

We use Jira to track the state of pull requests and the versions they got
included in.

@DHager
Copy link
Contributor

DHager commented Apr 17, 2015

This sounds like one of those fiddly edge-cases where a test may be useful.

@deeky666
Copy link
Member

yeah requires a test case indeed

@lenardpalko
Copy link
Author

I've added some tests too. Not sure if the location is appropriate, let me know and I move them. I've added them under the namespace Doctrine/Tests/ORM/Functional/Ticket.

@Ocramius
Copy link
Member

@lenardpalko merged, thank you!

master: 06a00cf
2.5: 4ca00f7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants