Merge pull request #136 from costa/idAttribute-support

idAttribute support + a fix for ignoring pre-defined id when adding to collection

src/backbonefire.js

@@ -164,7 +164,7 @@
    *    primitive - Throw error, primitives cannot be synced
    *    null      - Create blank object and assign id
    */
-  Backbone.Firebase._checkId = function(snap) {
+  Backbone.Firebase._checkId = function(snap, idAttribute) {
     var model = snap.val();
 
     // if the model is a primitive throw an error
@@ -180,7 +180,7 @@
     }
 
     // set the id to the snapshot's key
-    model.id = Backbone.Firebase._getKey(snap);
+    model[idAttribute] = Backbone.Firebase._getKey(snap);
 
     return model;
   };
@@ -348,7 +348,7 @@
       }
 
     },
-    
+
     sync: function(method, model, options) {
       Backbone.Firebase.sync(method, model, options);
     },
@@ -359,7 +359,7 @@
     _setId: function(snap) {
       // if the item new set the name to the id
       if(this.isNew()) {
-        this.set('id', Backbone.Firebase._getKey(snap), { silent: true });
+        this.set(this.idAttribute, Backbone.Firebase._getKey(snap), { silent: true });
       }
     },
 
@@ -376,7 +376,7 @@
      * by comparing the keys that have been removed.
      */
     _unsetAttributes: function(snap) {
-      var newModel = Backbone.Firebase._checkId(snap);
+      var newModel = Backbone.Firebase._checkId(snap, this.idAttribute);
 
       if (typeof newModel === 'object' && newModel !== null) {
         var diff = _.difference(_.keys(this.attributes), _.keys(newModel));
@@ -399,7 +399,7 @@
       var modelObj = model.changedAttributes();
       _.each(model.changed, function(value, key) {
         if (typeof value === 'undefined' || value === null) {
-          if (key == 'id') {
+          if (key == model.idAttribute) {
             delete modelObj[key];
           } else {
             modelObj[key] = null;
@@ -438,7 +438,8 @@
        * Backbone.Firebase.sync with the correct method.
        */
       create: function(model, options) {
-        model.id = Backbone.Firebase._getKey(this.firebase.push());
+        // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+        model[this.idAttribute] = model[this.idAttribute] || Backbone.Firebase._getKey(this.firebase.push());
         options = _.extend({ autoSync: false }, options);
         return Backbone.Collection.prototype.create.call(this, model, options);
       },
@@ -448,7 +449,8 @@
        * Backbone.Firebase.sync with the correct method.
        */
       add: function(model, options) {
-        model.id = Backbone.Firebase._getKey(this.firebase.push());
+        // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+        model[this.idAttribute] = model[this.idAttribute] || Backbone.Firebase._getKey(this.firebase.push());
         options = _.extend({ autoSync: false }, options);
         return Backbone.Collection.prototype.add.call(this, model, options);
       },
@@ -503,7 +505,6 @@
       // Defer the listener incase the data is cached, because
       // then the once call would be synchronous
       _.defer(_.bind(function() {
-
         this.firebase.once('value', function() {
           // indicate that the call has been received from the server
           // and the data has successfully loaded
@@ -541,7 +542,8 @@
             this._suppressEvent = true;
           }
 
-          var childRef = this.firebase.ref().child(model.id);
+          // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+          var childRef = this.firebase.ref().child(model[this.idAttribute]);
           childRef.set(model, _.bind(options.success, model));
         }
 
@@ -568,7 +570,8 @@
 
         for (var i = 0; i < parsed.length; i++) {
           var model = parsed[i];
-          var childRef = this.firebase.child(model.id);
+          // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+          var childRef = this.firebase.child(model[this.idAttribute]);
           if (options.silent === true) {
             this._suppressEvent = true;
           }
@@ -628,9 +631,8 @@
         for (var i = 0; i < models.length; i++) {
           var model = models[i];
 
-          if (!model.id) {
-            model.id = Backbone.Firebase._getKey(this.firebase.push());
-          }
+          // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+          model[this.idAttribute] = model[this.idAttribute] || Backbone.Firebase._getKey(this.firebase.push());
 
           // call Backbone's prepareModel to apply options
           model = Backbone.Collection.prototype._prepareModel.call(
@@ -649,15 +651,16 @@
       },
 
       _childAdded: function(snap) {
-        var model = Backbone.Firebase._checkId(snap);
+        // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+        var model = Backbone.Firebase._checkId(snap, this.idAttribute);
 
         if (this._suppressEvent === true) {
           this._suppressEvent = false;
           Backbone.Collection.prototype.add.call(this, [model], {silent: true});
         } else {
           Backbone.Collection.prototype.add.call(this, [model]);
         }
-        this.get(model.id)._remoteAttributes = model;
+        this.get(model[this.idAttribute])._remoteAttributes = model;
       },
 
       // TODO: child_moved is emitted when the priority for a child is changed, so it
@@ -669,10 +672,12 @@
       // when a model has changed remotely find differences between the
       // local and remote data and apply them to the local model
       _childChanged: function(snap) {
-        var model = Backbone.Firebase._checkId(snap);
+        // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+        var idAttribute = this.idAttribute;
+        var model = Backbone.Firebase._checkId(snap, idAttribute);
 
         var item = _.find(this.models, function(child) {
-          return child.id == model.id;
+          return child.id == model[idAttribute];
         });
 
         if (!item) {
@@ -701,7 +706,8 @@
       // remove an item from the collection when removed remotely
       // provides the ability to remove siliently
       _childRemoved: function(snap) {
-        var model = Backbone.Firebase._checkId(snap);
+        // XXX model prototype broken: this.model.prototype.idAttribute worked around as this.idAttribute
+        var model = Backbone.Firebase._checkId(snap, this.idAttribute);
 
         if (this._suppressEvent === true) {
           this._suppressEvent = false;
@@ -827,6 +833,10 @@
         SyncCollection.apply(this, arguments);
       }
 
+      // XXX before breaking model prototype: worked around this.model.prototype.idAttribute with this.idAttribute
+      this.idAttribute = this.idAttribute || BaseModel.prototype.idAttribute;
+
+      // XXX breaking the model prototype
       // Intercept the given model and give it a firebase ref.
       // Have it listen to local changes silently. When attributes
       // are unset, the callback will set them to null so that they

test/karma.conf.js

@@ -1,9 +1,19 @@
 module.exports = function(config) {
+  var srcPreprocessors = 'coverage';
+  var reporters = ['spec', 'failed'];
+  function isDebug(arg) {
+    return arg === '--debug';
+  }
+  if (process.argv.some(isDebug)) {
+    sourcePreprocessors = [];
+  } else {
+    reporters.push('coverage');
+  }
   config.set({
     frameworks: ['mocha', 'chai', 'sinon'],
 
     preprocessors: {
-      '../src/*.js': 'coverage'
+      '../src/*.js': srcPreprocessors
     },
 
     files: [
@@ -15,7 +25,7 @@ module.exports = function(config) {
       './specs/*_test.js'
     ],
 
-    reporters: ['spec', 'failed', 'coverage'],
+    reporters: reporters,
     coverageReporter: {
       reporters: [
         {

test/specs/collection_test.js

@@ -194,6 +194,7 @@ describe('Backbone.Firebase.Collection', function() {
         User = Backbone.Model.extend({}),
         Users = Backbone.Firebase.Collection.extend({
           url: 'Mock://',
+          idAttribute: User.prototype.idAttribute,
           initialize: function(models, options) {
             this.model = function(attrs, opts) {
               return new User(_.extend(attrs, { addedFromCollection: true}), opts);
@@ -227,6 +228,7 @@ describe('Backbone.Firebase.Collection', function() {
       User = Backbone.Model.extend({}),
       Users = Backbone.Firebase.Collection.extend({
         url: 'Mock://',
+        idAttribute: User.prototype.idAttribute,
         initialize: function(models, options) {
           this.model = function(attrs, opts) {
             return new User(_.extend(attrs, { addedFromCollection: true}), opts);
@@ -451,6 +453,7 @@ describe('Backbone.Firebase.Collection', function() {
       beforeEach(function() {
         var Collection = Backbone.Firebase.Collection.extend({
           url: 'Mock://',
+          idAttribute: Backbone.Model.prototype.idAttribute,
           autoSync: true
         });
 

test/specs/prototype_test.js

@@ -105,7 +105,7 @@ describe('Backbone.Firebase', function() {
         }
       });
 
-      var model = Backbone.Firebase._checkId(mockSnap);
+      var model = Backbone.Firebase._checkId(mockSnap, 'id');
 
       expect(model.id).to.be.ok;
       model.id.should.equal(mockSnap.name());
@@ -118,7 +118,7 @@ describe('Backbone.Firebase', function() {
         val: 'hello'
       });
       try {
-        var model = Backbone.Firebase._checkId(1);
+        var model = Backbone.Firebase._checkId(1, 'id');
       } catch (err) {
         expect(err).to.be.ok
       }
@@ -130,7 +130,7 @@ describe('Backbone.Firebase', function() {
         name: '1',
         val: null
       });
-      var model = Backbone.Firebase._checkId(mockSnap);
+      var model = Backbone.Firebase._checkId(mockSnap, 'id');
       expect(model.id).to.be.ok;
     });