Permalink
Browse files

Refactor some repeated code into 'Backbone.Store.resolveIdForItem'; a…

…ccept collections that are passed straight into 'attributes' as relations when creating a new model. Fixes #96.
  • Loading branch information...
1 parent e7d0ac3 commit 64311e3c9736247ea5e2f613a79a1475bb6f09a5 @PaulUithol committed Apr 11, 2012
Showing with 98 additions and 31 deletions.
  1. +68 −30 backbone-relational.js
  2. +30 −1 test/tests.js
@@ -4,7 +4,7 @@
*
* Backbone-relational may be freely distributed under the MIT license.
* For details and documentation: https://github.com/PaulUithol/Backbone-relational.
- * Depends on (as in, compeletely useless without) Backbone: https://github.com/documentcloud/backbone.
+ * Depends on Backbone: https://github.com/documentcloud/backbone.
*/
( function( undefined ) {
"use strict";
@@ -211,8 +211,34 @@
return coll;
},
-
- find: function( type, id ) {
+
+ /**
+ * Find an id
+ * @param type
+ * @param {String|Number|Object|Backbone.RelationalModel} item
+ */
+ resolveIdForItem: function( type, item ) {
+ var id = _.isString( item ) || _.isNumber( item ) ? item : null;
+
+ if ( id == null ) {
+ if ( item instanceof Backbone.RelationalModel ) {
+ id = item.id;
+ }
+ else if ( _.isObject( item ) ) {
+ id = item[ type.prototype.idAttribute ];
@DouweM
DouweM Apr 12, 2012 Collaborator

Are the type.prototype and therefore the type argument really necessary here? Can't we just use item.idAttribute?

@PaulUithol
PaulUithol Apr 12, 2012 Owner

I've thought about that, but then it would have to be called like Backbone.Relational.store.resolveIdForItem( this.relatedModel.prototype.idAttribute, item );. Since it's called about four times, it seemed nicer to make it part of this method.

@DouweM
DouweM Apr 12, 2012 Collaborator

Well, type.prototype.idAttribute === item.idAttribute, so as we're already passing in item, we don't need to pass in type as well. Right?

Couldn't we just replace type.prototype.idAttribute with item.idAttribute, and get rid of the type parameter?

@PaulUithol
PaulUithol Apr 12, 2012 Owner

I think I misunderstood you. item will in many cases just be a plain JSON object from somewhere, without an idAttribute property. I'd use $.isPlainObject for the check to clarify that, but adding a jQuery dependency here is not such a good idea ;).

@DouweM
DouweM Apr 12, 2012 Collaborator

Aah, you're right, I was assuming we were handling the case where item was a Backbone.RelationalModel here, but that's handled 3 lines up. My mistake!

+ }
+ }
+
+ return id;
+ },
+
+ /**
+ *
+ * @param type
+ * @param {String|Number|Object|Backbone.RelationalModel} item
+ */
+ find: function( type, item ) {
+ var id = this.resolveIdForItem( type, item );
var coll = this.getCollection( type );
return coll && coll.get( id );
},
@@ -538,10 +564,10 @@
if ( item instanceof this.relatedModel ) {
model = item;
}
- else if ( item && ( _.isString( item ) || _.isNumber( item ) || typeof( item ) === 'object' ) ) {
+ else if ( item ) {
// Try to find an instance of the appropriate 'relatedModel' in the store, or create it
- var id = _.isString( item ) || _.isNumber( item ) ? item : item[ this.relatedModel.prototype.idAttribute ];
- model = Backbone.Relational.store.find( this.relatedModel, id );
+ model = Backbone.Relational.store.find( this.relatedModel, item );
+
if ( model && _.isObject( item ) ) {
model.set( item, options );
}
@@ -620,8 +646,8 @@
options = this.sanitizeOptions( options );
var item = this.keyContents;
- if ( item && ( _.isString( item ) || _.isNumber( item ) || typeof( item ) === 'object' ) ) {
- var id = _.isString( item ) || _.isNumber( item ) ? item : item[ this.relatedModel.prototype.idAttribute ];
+ if ( item ) {
+ var id = Backbone.Relational.store.resolveIdForItem( this.relatedModel, item );
if ( model.id === id ) {
this.addRelated( model, options );
}
@@ -672,7 +698,14 @@
throw new Error( 'collectionType must inherit from Backbone.Collection' );
}
- this.setRelated( this._prepareCollection() );
+ // Handle cases where a model/relation is created with a collection passed straight into 'attributes'
+ if ( this.keyContents instanceof Backbone.Collection ) {
+ this.setRelated( this._prepareCollection( this.keyContents ) );
+ }
+ else {
+ this.setRelated( this._prepareCollection() );
+ }
+
this.findRelated( { silent: true } );
},
@@ -684,6 +717,7 @@
/**
* Bind events and setup collectionKeys for a collection that is to be used as the backing store for a HasMany.
+ * If no 'collection' is supplied, a new collection will be created of the specified 'collectionType' option.
* @param {Backbone.Collection} [collection]
*/
_prepareCollection: function( collection ) {
@@ -723,27 +757,31 @@
findRelated: function( options ) {
if ( this.keyContents ) {
- // Handle cases the an API/user supplies just an Object/id instead of an Array
- this.keyContents = _.isArray( this.keyContents ) ? this.keyContents : [ this.keyContents ];
-
var models = [];
- // Try to find instances of the appropriate 'relatedModel' in the store
- _.each( this.keyContents, function( item ) {
- var id = _.isString( item ) || _.isNumber( item ) ? item : item[ this.relatedModel.prototype.idAttribute ];
-
- var model = Backbone.Relational.store.find( this.relatedModel, id );
- if ( model && _.isObject( item ) ) {
- model.set( item, options );
- }
- else if ( !model ) {
- model = this.createModel( item );
- }
-
- if ( model && !this.related.getByCid( model ) && !this.related.get( model ) ) {
- models.push( model );
- }
- }, this );
+ if ( this.keyContents instanceof Backbone.Collection ) {
+ models = this.keyContents.models;
+ }
+ else {
+ // Handle cases the an API/user supplies just an Object/id instead of an Array
+ this.keyContents = _.isArray( this.keyContents ) ? this.keyContents : [ this.keyContents ];
+
+ // Try to find instances of the appropriate 'relatedModel' in the store
+ _.each( this.keyContents, function( item ) {
+ var model = Backbone.Relational.store.find( this.relatedModel, item );
+
+ if ( model && _.isObject( item ) ) {
+ model.set( item, options );
+ }
+ else if ( !model ) {
+ model = this.createModel( item );
+ }
+
+ if ( model && !this.related.getByCid( model ) && !this.related.get( model ) ) {
+ models.push( model );
+ }
+ }, this );
+ }
// Add all found 'models' in on go, so 'add' will only be called once (and thus 'sort', etc.)
if ( models.length ) {
@@ -804,7 +842,7 @@
if ( !this.related.getByCid( model ) && !this.related.get( model ) ) {
// Check if this new model was specified in 'this.keyContents'
var item = _.any( this.keyContents, function( item ) {
- var id = _.isString( item ) || _.isNumber( item ) ? item : item[ this.relatedModel.prototype.idAttribute ];
+ var id = Backbone.Relational.store.resolveIdForItem( this.relatedModel, item );
return id && id === model.id;
}, this );
@@ -1044,7 +1082,7 @@
rel = this.getRelation( key ),
keyContents = rel && rel.keyContents,
toFetch = keyContents && _.select( _.isArray( keyContents ) ? keyContents : [ keyContents ], function( item ) {
- var id = _.isString( item ) || _.isNumber( item ) ? item : item[ rel.relatedModel.prototype.idAttribute ];
+ var id = Backbone.Relational.store.resolveIdForItem( rel.relatedModel, item );
return id && !Backbone.Relational.store.find( rel.relatedModel, id );
}, this );
View
@@ -1012,7 +1012,36 @@ $(document).ready(function() {
equal( zoo.get( 'animals' ).length, 1, "Still just 1 elephant in the zoo" );
});
-
+
+ test( "collections can also be passed as attributes on creation", function() {
+ var animals = new AnimalCollection([
+ { id: 1, species: 'Lion' },
+ { id: 2 ,species: 'Zebra' }
+ ]);
+
+ var zoo = new Zoo( { animals: animals } );
+ console.log( zoo.get( 'animals' ), zoo.get( 'animals' ).models );
+
+ equal( zoo.get( 'animals' ), animals, "The 'animals' collection has been set as the zoo's animals" );
+ equal( zoo.get( 'animals' ).length, 2, "Two animals in 'zoo'" );
+
+ zoo.destroy();
+
+ var newZoo = new Zoo( { animals: animals.models } );
+
+ ok( newZoo.get( 'animals' ).length === 2, "Two animals in the 'newZoo'" );
+ });
+
+ test( "models can also be passed as attributes on creation", function() {
+ var artis = new Zoo( { name: 'Artis' } );
+
+ var animal = new Animal( { species: 'Hippo', livesIn: artis });
+
+ equal( artis.get( 'animals' ).at( 0 ), animal, "Artis has a Hippo" );
+ equal( animal.get( 'livesIn' ), artis, "The Hippo is in Artis" );
+ });
+
+
module( "Backbone.HasOne", { setup: initObjects } );

0 comments on commit 64311e3

Please sign in to comment.