[Pkg-javascript-commits] [backbone] 226/281: Collection#add tweaks

[Jonas Smedegaard]

This is an automated email from the git hooks/post-receive script.

js pushed a commit to tag 0.9.0
in repository backbone.

commit 09ab8cb17da44c5b6675a9629159e445e655580d
Author: Brad Dunbar <dunbarb2 at gmail.com>
Date:   Tue Jan 17 23:37:53 2012 -0500

    Collection#add tweaks
    
    * leave collection in a consistent state after throw
    * correct indices
---
 backbone.js        | 24 +++++++++++++-----------
 test/collection.js | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/backbone.js b/backbone.js
index 5be2187..87daa0d 100644
--- a/backbone.js
+++ b/backbone.js
@@ -470,31 +470,33 @@
     // Add a model, or list of models to the set. Pass **silent** to avoid
     // firing the `add` event for every new model.
     add : function(models, options) {
-      var i, index, length;
+      var i, index, length, model, cids = {};
       options || (options = {});
-      if (!_.isArray(models)) models = [models];
-      models = slice.call(models);
+      models = _.isArray(models) ? models.slice() : [models];
       for (i = 0, length = models.length; i < length; i++) {
-        var model = models[i] = this._prepareModel(models[i], options);
-        if (!model) {
+        if (!(model = models[i] = this._prepareModel(models[i], options))) {
           throw new Error("Can't add an invalid model to a collection");
         }
         var hasId = model.id != null;
         if (this._byCid[model.cid] || (hasId && this._byId[model.id])) {
           throw new Error("Can't add the same model to a collection twice");
         }
+      }
+      for (i = 0; i < length; i++) {
+        (model = models[i]).on('all', this._onModelEvent, this);
         this._byCid[model.cid] = model;
-        if (hasId) this._byId[model.id] = model;
-        model.on('all', this._onModelEvent, this);
+        if (model.id != null) this._byId[model.id] = model;
+        cids[model.cid] = true;
       }
       this.length += length;
       index = options.at != null ? options.at : this.models.length;
       splice.apply(this.models, [index, 0].concat(models));
       if (this.comparator) this.sort({silent: true});
       if (options.silent) return this;
-      for (i = 0; i < length; i++) {
-        options.index = index + i;
-        models[i].trigger('add', models[i], this, options);
+      for (i = 0, length = this.models.length; i < length; i++) {
+        if (!cids[(model = this.models[i]).cid]) continue;
+        options.index = i;
+        model.trigger('add', model, this, options);
       }
       return this;
     },
@@ -504,7 +506,7 @@
     remove : function(models, options) {
       var i, index, model;
       options || (options = {});
-      models = _.isArray(models) ? slice.call(models) : [models];
+      models = _.isArray(models) ? models.slice() : [models];
       for (i = 0, l = models.length; i < l; i++) {
         model = this.getByCid(models[i]) || this.get(models[i]);
         if (!model) continue;
diff --git a/test/collection.js b/test/collection.js
index b836486..057d83d 100644
--- a/test/collection.js
+++ b/test/collection.js
@@ -489,4 +489,37 @@ $(document).ready(function() {
     }, "Can't add an invalid model to a collection");
   });
 
+  test("Collection: index with comparator", function() {
+    expect(4);
+    var counter = 0;
+    var col = new Backbone.Collection([{id: 2}, {id: 4}], {
+      comparator: function(model){ return model.id; }
+    }).on('add', function(model, colleciton, options){
+      if (model.id == 1) {
+        equal(options.index, 0);
+        equal(counter++, 0);
+      }
+      if (model.id == 3) {
+        equal(options.index, 2);
+        equal(counter++, 1);
+      }
+    });
+    col.add([{id: 3}, {id: 1}]);
+  });
+
+  test("Collection: throwing during add leaves consistent state", function() {
+    expect(4);
+    var col = new Backbone.Collection();
+    col.bind('test', function() { ok(false); });
+    col.model = Backbone.Model.extend({
+      validate: function(attrs){ if (!attrs.valid) return 'invalid'; }
+    });
+    var model = new col.model({id: 1, valid: true});
+    raises(function() { col.add([model, {id: 2}]); });
+    model.trigger('test');
+    ok(!col.getByCid(model.cid));
+    ok(!col.get(1));
+    equal(col.length, 0);
+  });
+
 });

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/backbone.git