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

[2.0] Change Sets #14

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

[2.0] Change Sets #14

wants to merge 26 commits into from

Conversation

malarzm
Copy link
Owner

@malarzm malarzm commented Jun 17, 2016

BC Breaks in BCish introduction of ObjectChangeSet

  • UnitOfWork::getDocumentChangeSet is now returning ObjectChangeSet|null. This will make empty($changeSet) checks still valid however all is_array or comparing anything to an array will break.

BC Breaks in Use ObjectChangeSet for modified embedded documents

  • change sets for embedded objects are instances of ObjectChangeSet which does not (yet?) provides BC for accessing [0] and [1] for old and new value respectively.

* @param PersistentCollectionInterface $collection
* @param ObjectChangeSet[] $changes
*/
public function __construct(PersistentCollectionInterface $collection, array $changes)
Copy link

Choose a reason for hiding this comment

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

One thing we'll have to think of is the ability to replace collections:

$object->embedMany = []

This won't just be a collection changeset but instead the entire collection gets replaced with a new collection. In order to properly populate insertedDocuments and deletedDocuments we'd have to know about both collections.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmmm somehow I'd see FieldChange for such action, what do you think?

Copy link

Choose a reason for hiding this comment

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

So FieldChange(oldCollection, newArray)? That could work - I was just wondering how we could accomodate this.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done like this, it's also working like this for exchanged embedded documents :)

@malarzm malarzm force-pushed the changesets branch 4 times, most recently from 8f69bdc to 1e3f9bb Compare June 18, 2016 12:25
@malarzm malarzm force-pushed the changesets branch 2 times, most recently from f1b9da8 to 1f11137 Compare July 30, 2016 16:53
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.

2 participants