Permalink
Browse files

Change `findOrCreate` to pass `parsedAttributes` to `build`, and adju…

…st tests to reflect change.
  • Loading branch information...
1 parent 4d0b705 commit b7ac366cbabf274461c5db7271b37bd3f1b1d5c1 @WCPetersen WCPetersen committed Jan 26, 2014
Showing with 6 additions and 5 deletions.
  1. +1 −1 backbone-relational.js
  2. +5 −4 test/tests.js
View
2 backbone-relational.js
@@ -1783,7 +1783,7 @@
model.set( parsedAttributes, options );
}
else if ( !model && options.create !== false ) {
- model = this.build( attributes, options );
+ model = this.build( parsedAttributes, _.defaults( { parse: false }, options ) );
}
}
View
9 test/tests.js
@@ -1196,9 +1196,10 @@ $(document).ready(function() {
ok( animal.get( 'livesIn' ) instanceof Zoo );
ok( animal.get( 'livesIn' ).get( 'animals' ).get( animal ) === animal );
- // `parse` gets called once by `findOrCreate` directly when trying to lookup `1`,
- // once when `build` (called from `findOrCreate`) calls the Zoo constructor with `{ parse: true}`.
- ok( parseCalled === 2, 'parse called 2 times? ' + parseCalled );
+ // `parse` gets called by `findOrCreate` directly when trying to lookup `1`,
+ // and the parsed attributes are passed to `build` (called from `findOrCreate`) with `{ parse: false }`,
+ // rather than having `parse` called again by the Zoo constructor.
+ ok( parseCalled === 1, 'parse called 1 time? ' + parseCalled );
parseCalled = 0;
@@ -2266,7 +2267,7 @@ $(document).ready(function() {
ok( job && job.id === 'e1', 'job exists' );
ok( person && person.id === 'p1', 'person exists' );
- ok( modelParseCalled === 7, 'model.parse called 7 times? ' + modelParseCalled );
+ ok( modelParseCalled === 4, 'model.parse called 4 times? ' + modelParseCalled );
ok( collParseCalled === 0, 'coll.parse called 0 times? ' + collParseCalled );
});

1 comment on commit b7ac366

@philfreo
Collaborator

What's the backstory on this commit? It broke some code of mine... and it looks like it's now impossible to do what used to be easy:

 // using a BaseModel which inherits from Backbone.RelationalModel, and does other magic
    var EmailAccount = BaseModel.extend({
        resource: 'email_account',
        constructor: function() {
            this.smtp = new EmailSMTP(null, { account: this });
            return Model.apply(this, arguments);
        },
        parse: function(data) {
            data = JSON.parse(JSON.stringify(data));
            if (data) {
                if (this instanceof EmailAccount) {
                    if (data.smtp) this.smtp.set(data.smtp);  /// NO LONGER GETS CALLED EVER
                }
                delete data.smtp;
            }
            return data;
        }
    });

// obvious way:
var account = new EmailAccount(accountData, { parse: true });

// but to avoid 'duplicate ID' errors if the data is loaded multiple times, the natural fix is this:
var account = EmailAccount.findOrCreate(accountData, { parse: true });

I understand that there may be some scenarios where parsing before you have an instance is necessary, but that doesn't mean you shouldn't be able to use "parse" in the way that Backbone intends - which means that an instance is present.

See also "parse is now an excellent place to extract and vivify incoming nested JSON into associated submodels." from changelog and jashkenas/backbone#2755

Please sign in to comment.