Fix updates to embedded documents after removal #2768
Conversation
After a parent document and its embeds are removed and then subsequently persisted, the embedded documents didn't get updated. This happened because the parent document goes through an upsert operation, but `PersistenceBuilder->prepareUpsertData()` did not handle embedded associations in the same way that `PersistenceBuilder->prepareUpdateData()` does. This fixes that issue so that an upserted parent document handles embed-many's the same was as an updated parent document. Fixes doctrine#2767
There was a problem hiding this comment.
Thanks @jfortunato for opening this PR with a perfect test case.
I added a prefix to the test classes to prevent name conflict, and refactored the change on PersistenceBuilder to not have nested if blocks.
Could you add a test that remove an embedded document? I'm no expert on this code, but from the comments I get the impression that the change should be made in CollectionPersister.
| $update = $this->prepareUpsertData($embeddedDoc); | ||
| foreach ($update as $cmd => $values) { | ||
| foreach ($values as $name => $value) { | ||
| $updateData[$cmd][$mapping['name'] . '.' . $key . '.' . $name] = $value; |
There was a problem hiding this comment.
This modifies the object at a given position. I'm not sure about the compatibility of deleting documents from the embedded collection.
|
Oups, the last commit broke |
| ) { | ||
| $updateData['$set'][$mapping['name']] = $this->prepareAssociatedCollectionValue($new, true); | ||
| } | ||
| // @EmbedMany and @ReferenceMany are handled by CollectionPersister | ||
|
|
||
| // @ReferenceMany is handled by CollectionPersister |
There was a problem hiding this comment.
This comment was referring to @ReferenceMany which is already handled previously. I wonder if it's really relevant.
Summary
After a parent document and its embeds are removed and then subsequently persisted, the embedded documents didn't get updated. This happened because the parent document goes through an upsert operation, but
PersistenceBuilder->prepareUpsertData()did not handle embedded associations in the same way thatPersistenceBuilder->prepareUpdateData()does. This fixes that issue so that an upserted parent document handles embed-many's the same was as an updated parent document.Fixes #2767