From 14000a8bb93b1b0876870b6211bf8fc5113abf37 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 22 Jan 2011 10:15:51 -0600 Subject: [PATCH] Improve multiple collection support Preserve original model collection when adding/removing from another collection Destroying a model removes it from all collections --- backbone.js | 15 +++++++--- test/collection.js | 73 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/backbone.js b/backbone.js index 86214fcbb..e323ed0bf 100644 --- a/backbone.js +++ b/backbone.js @@ -287,7 +287,7 @@ var model = this; var success = options.success; options.success = function(resp) { - if (model.collection) model.collection.remove(model); + model.trigger('destroy', model, model.collection, options); if (success) success(model, resp); }; options.error = wrapError(options.error, model, options); @@ -555,7 +555,9 @@ if (already) throw new Error(["Can't add the same model to a set twice", already.id]); this._byId[model.id] = model; this._byCid[model.cid] = model; - model.collection = this; + if (!model.collection) { + model.collection = this; + } var index = this.comparator ? this.sortedIndex(model, this.comparator) : this.length; this.models.splice(index, 0, model); model.bind('all', this._onModelEvent); @@ -581,7 +583,9 @@ // Internal method to remove a model's ties to a collection. _removeReference : function(model) { - delete model.collection; + if (this == model.collection) { + delete model.collection; + } model.unbind('all', this._onModelEvent); }, @@ -589,8 +593,11 @@ // Sets need to update their indexes when models change ids. All other // events simply proxy through. "add" and "remove" events that originate // in other collections are ignored. - _onModelEvent : function(ev, model, collection) { + _onModelEvent : function(ev, model, collection, options) { if ((ev == 'add' || ev == 'remove') && collection != this) return; + if (ev == 'destroy') { + this._remove(model, options); + } if (ev === 'change:id') { delete this._byId[model.previous('id')]; this._byId[model.id] = model; diff --git a/test/collection.js b/test/collection.js index 8ed704a83..9c23cfdb2 100644 --- a/test/collection.js +++ b/test/collection.js @@ -73,6 +73,34 @@ $(document).ready(function() { ok(opts.amazing); }); + test("Collection: add model to multiple collections", function() { + var counter = 0; + var e = new Backbone.Model({id: 10, label : 'e'}); + e.bind('add', function(model, collection) { + counter++; + equals(e, model); + if (counter > 1) { + equals(collection, colF); + } else { + equals(collection, colE); + } + }); + var colE = new Backbone.Collection([]); + colE.bind('add', function(model, collection) { + equals(e, model); + equals(colE, collection); + }); + var colF = new Backbone.Collection([]); + colF.bind('add', function(model, collection) { + equals(e, model); + equals(colF, collection); + }); + colE.add(e); + equals(e.collection, colE); + colF.add(e); + equals(e.collection, colE); + }); + test("Collection: remove", function() { var removed = otherRemoved = null; col.bind('remove', function(model){ removed = model.get('label'); }); @@ -121,6 +149,51 @@ $(document).ready(function() { equals(passed, true); }); + test("Collection: remove same model in multiple collection", function() { + var counter = 0; + var e = new Backbone.Model({id: 5, title: 'Othello'}); + e.bind('remove', function(model, collection) { + counter++; + equals(e, model); + if (counter > 1) { + equals(collection, colE); + } else { + equals(collection, colF); + } + }); + var colE = new Backbone.Collection([e]); + colE.bind('remove', function(model, collection) { + equals(e, model); + equals(colE, collection); + }); + var colF = new Backbone.Collection([e]); + colF.bind('remove', function(model, collection) { + equals(e, model); + equals(colF, collection); + }); + equals(colE, e.collection); + colF.remove(e); + ok(colF.length == 0); + ok(colE.length == 1); + equals(counter, 1); + equals(colE, e.collection); + colE.remove(e); + equals(null, e.collection); + ok(colE.length == 0); + equals(counter, 2); + }); + + test("Colllection: model destroy removes from all collections", function() { + var e = new Backbone.Model({id: 5, title: 'Othello'}); + e.sync = function(method, model, options) { options.success({}); }; + var colE = new Backbone.Collection([e]); + var colF = new Backbone.Collection([e]); + e.destroy(); + ok(colE.length == 0); + ok(colF.length == 0); + equals(null, e.collection); + }); + test("Collection: fetch", function() { col.fetch(); equals(lastRequest[0], 'read');