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 changeset missing old value for exchanged associations #1437

Merged
merged 1 commit into from
Jun 28, 2016

Conversation

malarzm
Copy link
Member

@malarzm malarzm commented Jun 18, 2016

Found this bug while working on new change sets; in #1339 I was so happy that collections are in the change set that I haven't checked what was in the change set :) By the way fixed the same thing for when embedded document is exchanged.

@malarzm malarzm added the Bug label Jun 18, 2016
@malarzm malarzm added this to the 1.1.1 milestone Jun 18, 2016
@malarzm
Copy link
Member Author

malarzm commented Jun 18, 2016

By the way

Collection won't be in changeset if only one of its element was modified.

from original PR seem to no longer be true however I haven't tested that

@alcaeus
Copy link
Member

alcaeus commented Jun 27, 2016

from original PR seem to no longer be true however I haven't tested that

If that's something we want, I'd add a quick test to it, if not I'd also add a quick test to make sure it isn't so ;)

@malarzm
Copy link
Member Author

malarzm commented Jun 27, 2016

As much as I admire the idea I'm pretty sure we'll get into troubles soon, for example I have quite bad feelings about including clear()ed collection into the changeset or detecting deeper in-collection changes... I'd leave straightening this out to change set overhaul that is yet to come :)

@alcaeus
Copy link
Member

alcaeus commented Jun 28, 2016

Sounds like a plan 👍

@alcaeus alcaeus merged commit 1d44ecc into doctrine:master Jun 28, 2016
@malarzm malarzm deleted the changeset-fix branch March 9, 2017 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants