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 more re-used embedded docs issues #1550

Merged
merged 1 commit into from
Jan 10, 2017

Conversation

malarzm
Copy link
Member

@malarzm malarzm commented Jan 9, 2017

I'll comment on changes separately but as for now failing test case @redthor has provided in #1525 is fixed

@malarzm malarzm added the Bug label Jan 9, 2017
@malarzm malarzm added this to the 1.1.3 milestone Jan 9, 2017
@@ -745,16 +745,6 @@ private function computeOrRecomputeChangeSet(ClassMetadata $class, $document, $r
if ($orgValue !== null) {
$this->scheduleOrphanRemoval($orgValue);
}

if ($actualValue !== null) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is #1542 removed because it was good only for updates (it's in the path when document is not new) while one of tests I've added was interested in fixing inserts

@@ -998,6 +988,10 @@ private function computeAssociationChanges($parentDocument, array $assoc, $value
if ($assoc['type'] === ClassMetadata::ONE) {
$class->setFieldValue($parentDocument, $assoc['fieldName'], $entry);
$this->setOriginalDocumentProperty(spl_object_hash($parentDocument), $assoc['fieldName'], $entry);
$poid = spl_object_hash($parentDocument);
if (isset($this->documentChangeSets[$poid][$assoc['fieldName']])) {
$this->documentChangeSets[$poid][$assoc['fieldName']][1] = $entry;
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixed #1542's failing test case and new one with early persist

if ($knownParent && $knownParent !== $document) {
$relatedDocuments = clone $relatedDocuments;
$class->setFieldValue($document, $mapping['fieldName'], $relatedDocuments);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This however I fail to see why I need to bring back but without this change set test with late persist is failing for reason I don't see yet

@malarzm malarzm requested a review from alcaeus January 9, 2017 21:18
@redthor
Copy link

redthor commented Jan 9, 2017

Thanks @malarzm ! I've removed the clone() I had added to our models and then pulled in your branch and ran the particular integration test that was failing and it passed 😎

@malarzm malarzm merged commit 758c29b into doctrine:1.1.x Jan 10, 2017
@malarzm malarzm deleted the gh1525-yet-again branch January 10, 2017 07:49
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.

3 participants