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

Updated options fetchRelated() passed to Collection#fetch() #271

Merged
merged 1 commit into from
Feb 15, 2013
Merged

Updated options fetchRelated() passed to Collection#fetch() #271

merged 1 commit into from
Feb 15, 2013

Conversation

jamesshannon
Copy link
Contributor

add: true didn't seem to do anything. Collection.fetch() only checks for updated. Only if it exists, does Collection.update() get called, which then checks for add option... but it defaults to true anyways. I'm presuming that this is a bug and update: true was intended.

Additionally, I added emove: false to prevent Collection#update() from removing existing models. This is important if some models were grabbed from cache and other models relied on fetchRelated() calling fetch(). If remove != false, then the cached models would be removed

For example, if you do:

ParentCollection.reset([{id: 1, children: [{id: 1}, {id: 2}]}, {id: 2, children: [{id: 1}, {id: 2}, {id: 3}]}]);

where:

ParentCollection.model = Parent

Parent.relations = {type: hasmany, key: 'children', relatedModel: Child, createModels: false, fetchRelated: true, collectionType: Children}

Children.url = function (multipleModels) {}

Then, BB Relational will, eventually:

Create Child objects for id 1 and id 2, but not add them to the Parent[id=1].children collection, then send a Parent[id=1].children.fetch() request off. The return will be parsed and added to the collection.

Asynchronously, fetchRelated() is called for Parent[id=2], which already has children id=1, and id=2 added (though pre-fetch). It loops through and determines that only child id=3 needs to be fetched, so it sends the fetch request off. But, without update: true, then fetch() calls reset(), which will wipe everything out. Without remove: false, then fetc() calls update() which will remove all items not added, essentialy acting like reset.

add: true didn't do anything, updated to update: true, which causes Collection#update() to run instead of Collection#reset()
remove: false added to prevent Collection#update() to not remove existing models. This is important if some models were grabbed from cache and other models relied on fetchRelated() calling fetch(). If remove != false, then the cached models would be removed
@jamesshannon
Copy link
Contributor Author

Even with this, I realized that a combination of previously-fetched (and thus cached) children and new children will screw up the order, with the new children all coming after the cached children.

I'll leave it to smarter people than myself to figure this out. (Though I can't help but think that the cache/store has a tendency to screw things up...)

@PaulUithol
Copy link
Owner

add: true was indeed replaced with more granular options around Backbone 0.9.9, I think. Thanks, merging.

PaulUithol added a commit that referenced this pull request Feb 15, 2013
Updated options fetchRelated() passed to Collection#fetch().

Replace `add:true` by `update:true, remove:false`.
@PaulUithol PaulUithol merged commit 3da2d83 into PaulUithol:master Feb 15, 2013
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