Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Models are now parsed before checking if they exist in a collection #3766

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -844,13 +844,17 @@

// Turn bare objects into model references, and prevent invalid models
// from being added.
var model;
var model, parsedModel;
for (var i = 0; i < models.length; i++) {
model = models[i];

// If a duplicate is found, prevent it from being added and
// optionally merge it into the existing model.
var existing = this.get(model);
if (!existing && !this._isModel(attrs) && options.parse) {
parsedModel = new this.model(model, options);
existing = existing || this.get(parsedModel.attributes);
}
if (existing) {
if (merge && model !== existing) {
var attrs = this._isModel(model) ? model.attributes : model;
Expand All @@ -866,7 +870,7 @@

// If this is a new, valid model, push it to the `toAdd` list.
} else if (add) {
model = models[i] = this._prepareModel(model, options);
model = models[i] = this._prepareModel(parsedModel || model, options);
if (model) {
toAdd.push(model);
this._addReference(model, options);
Expand Down Expand Up @@ -1078,14 +1082,14 @@

// Prepare a hash of attributes (or other model) to be added to this
// collection.
_prepareModel: function(attrs, options) {
if (this._isModel(attrs)) {
if (!attrs.collection) attrs.collection = this;
return attrs;
_prepareModel: function(model, options) {
if (this._isModel(model)) {
model.collection = model.collection || this;
} else {
options = options ? _.clone(options) : {};
options.collection = this;
model = new this.model(model, options);
}
options = options ? _.clone(options) : {};
options.collection = this;
var model = new this.model(attrs, options);
if (!model.validationError) return model;
this.trigger('invalid', this, model.validationError, options);
return false;
Expand Down
23 changes: 22 additions & 1 deletion test/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,36 @@
equal(col.length, 1);
});

test("merge in duplicate models with {merge: true}", 3, function() {
test("merge in duplicate models with {merge: true}", 4, function() {
var col = new Backbone.Collection;
col.add([{id: 1, name: 'Moe'}, {id: 2, name: 'Curly'}, {id: 3, name: 'Larry'}]);
col.add({id: 1, name: 'Moses'});
equal(col.first().get('name'), 'Moe');
col.add({id: 1, name: 'Moses'}, {merge: true});
equal(col.first().get('name'), 'Moses');
var firstModel = col.first();

col.add({id: 1, name: 'Tim'}, {merge: true, silent: true});
var secondModel = col.first();
equal(col.first().get('name'), 'Tim');

strictEqual(firstModel, secondModel);
});

test("collection should merge in duplicate raw objects with {merge: true}", 1, function() {
var Model = Backbone.Model.extend({
parse: function(data) { return data.wrapper; }
});

var Col = Backbone.Collection.extend({model: Model});
var col = new Col;
col.set([{wrapper: {id: 1, name: 'Foo'}}], {parse: true, merge: true});
var firstModel = col.first();

col.set([{wrapper: {id: 1, name: 'Bar'}}], {parse: true, merge: true});
var secondModel = col.first();

strictEqual(firstModel, secondModel);
});

test("add model to multiple collections", 10, function() {
Expand Down