From c95d9775e76d7750124368d0d4ba84e348c19c78 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 2 Jun 2015 13:55:21 -0400 Subject: [PATCH 1/3] Revert "Merge pull request #3644 from jridgewell/collection-parse" This reverts commit ff57b54cbb1b1c0d297d25bf673c8adc7023dd69, reversing changes made to 5ea158506ff65def13b132b30f0b0c9ecb0d31a2. --- backbone.js | 10 ++-- test/collection.js | 123 ++++++++++++++++++++------------------------- 2 files changed, 60 insertions(+), 73 deletions(-) diff --git a/backbone.js b/backbone.js index 35dd30860..53fb08729 100644 --- a/backbone.js +++ b/backbone.js @@ -739,7 +739,6 @@ if (options.comparator !== void 0) this.comparator = options.comparator; this._reset(); this.initialize.apply(this, arguments); - if (models && options.parse) models = this.parse(models, options); if (models) this.reset(models, _.extend({silent: true}, options)); }; @@ -790,6 +789,7 @@ // the core operation for updating the data contained by the collection. set: function(models, options) { options = _.defaults({}, options, setOptions); + if (options.parse) models = this.parse(models, options); var singular = !_.isArray(models); models = singular ? (models ? [models] : []) : models.slice(); var id, model, attrs, existing, sort; @@ -983,13 +983,13 @@ // collection when they arrive. If `reset: true` is passed, the response // data will be passed through the `reset` method instead of `set`. fetch: function(options) { - options = _.extend({parse: true}, options); - var collection = this; + options = options ? _.clone(options) : {}; + if (options.parse === void 0) options.parse = true; var success = options.success; + var collection = this; options.success = function(resp) { var method = options.reset ? 'reset' : 'set'; - var models = options.parse ? collection.parse(resp, options) : resp; - collection[method](models, options); + collection[method](resp, options); if (success) success.call(options.context, collection, resp, options); collection.trigger('sync', collection, resp, options); }; diff --git a/test/collection.js b/test/collection.js index 2cee05d40..49c36c689 100644 --- a/test/collection.js +++ b/test/collection.js @@ -252,6 +252,19 @@ equal(col.at(0).get('value'), 2); }); + test("add with parse and merge", function() { + var collection = new Backbone.Collection(); + collection.parse = function(attrs) { + return _.map(attrs, function(model) { + if (model.model) return model.model; + return model; + }); + }; + collection.add({id: 1}); + collection.add({model: {id: 1, name: 'Alf'}}, {parse: true, merge: true}); + equal(collection.first().get('name'), 'Alf'); + }); + test("add model to collection with sort()-style comparator", 3, function() { var col = new Backbone.Collection; col.comparator = function(a, b) { @@ -965,6 +978,29 @@ equal(c.at(0).get('name'), 'test'); }); + test("#1407 parse option on reset parses collection and models", 2, function() { + var model = { + namespace : [{id: 1}, {id:2}] + }; + var Collection = Backbone.Collection.extend({ + model: Backbone.Model.extend({ + parse: function(model) { + model.name = 'test'; + return model; + } + }), + parse: function(model) { + return model.namespace; + } + }); + var c = new Collection(); + c.reset(model, {parse:true}); + + equal(c.length, 2); + equal(c.at(0).get('name'), 'test'); + }); + + test("Reset includes previous models in triggered event.", 1, function() { var model = new Backbone.Model(); var collection = new Backbone.Collection([model]) @@ -1087,21 +1123,18 @@ }); test("`set` and model level `parse`", function() { - var Model = Backbone.Model.extend({ - parse: function(model) { - return model.model; - } - }); + var Model = Backbone.Model.extend({}); var Collection = Backbone.Collection.extend({ - model: Model + model: Model, + parse: function (res) { return _.pluck(res.models, 'model'); } }); var model = new Model({id: 1}); var collection = new Collection(model); - collection.set([ - {model: {id: 1, attr: 'test'}}, - {model: {id: 2, attr: 'test'}} - ], {parse: true}); - deepEqual(collection.pluck('attr'), ['test', 'test']); + collection.set({models: [ + {model: {id: 1}}, + {model: {id: 2}} + ]}, {parse: true}); + equal(collection.first(), model); }); test("`set` data is only parsed once", function() { @@ -1167,6 +1200,17 @@ new Collection().add({id: 1}, {sort: false}); }); + test("#1915 - `parse` data in the right order in `set`", function() { + var collection = new (Backbone.Collection.extend({ + parse: function (data) { + strictEqual(data.status, 'ok'); + return data.data; + } + })); + var res = {status: 'ok', data:[{id: 1}]}; + collection.set(res, {parse: true}); + }); + asyncTest("#1939 - `parse` is passed `options`", 1, function () { var collection = new (Backbone.Collection.extend({ url: '/', @@ -1578,61 +1622,4 @@ collection.invoke('method', 1, 2, 3); }); - test("set, add, and reset do not parse at collection level", 0, function() { - var Collection = Backbone.Collection.extend({ - parse: function(models) { - ok(false); - } - }); - var c = new Collection(); - c.set({id: 1}, {parse: true}); - c.add([{id: 2}, {id: 3}], {parse: true}); - c.reset([{id: 4}, {id: 5}], {parse: true}); - }); - - test("#3636 - create does not parse at collection level", 4, function() { - var Collection = Backbone.Collection.extend({ - url: '/test', - model: Backbone.Model.extend({ - parse: function(model) { - ok(true); - return model.model; - } - }), - parse: function(models) { - ok(false); - } - }); - var c = new Collection(); - c.create({}, {parse: true}); - this.ajaxSettings.success({model: {id: 1}}); - c.create(c.first(), {parse: true}); - this.ajaxSettings.success({model: {attr: 'test'}}); - equal(c.get(1).get('attr'), 'test'); - }); - - test("fetch data is only parsed once", 3, function() { - var modelParse = 0; - var collectionParse = 0; - var Collection = Backbone.Collection.extend({ - url: '/test', - model: Backbone.Model.extend({ - parse: function(model) { - modelParse++; - return model.model; - } - }), - parse: function(models) { - collectionParse++; - return models.models; - } - }); - var c = new Collection(); - c.fetch(); - this.ajaxSettings.success({models: [{model: {id: 1, attr: 'test'} }] }); - equal(modelParse, 1); - equal(collectionParse, 1); - equal(c.get(1).get('attr'), 'test'); - }); - })(); From ffa4fa597de154d704161a5df584e0458fa5f1a5 Mon Sep 17 00:00:00 2001 From: Andrey Kuzmin Date: Tue, 26 May 2015 22:58:02 +0200 Subject: [PATCH 2/3] Added failing test case --- test/collection.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/collection.js b/test/collection.js index 49c36c689..507191e3e 100644 --- a/test/collection.js +++ b/test/collection.js @@ -561,6 +561,34 @@ }); + test("create with wait:true should not call collection.parse", 1, function() { + var collectionParseCalled = false; + + var Model = Backbone.Model.extend({ + sync: function (method, model, options) { + _.extend(options, {specialSync: true}); + return Backbone.Model.prototype.sync.call(this, method, model, options); + } + }); + + var Collection = Backbone.Collection.extend({ + model: Model, + url: '/test', + parse: function () { + collectionParseCalled = true; + } + }); + + var collection = new Collection; + + var success = function (model, response, options) { + equal(collectionParseCalled, false); + }; + + collection.create({}, {wait: true, success: success}); + this.ajaxSettings.success(); + }); + test("a failing create returns model with errors", function() { var ValidatingModel = Backbone.Model.extend({ validate: function(attrs) { From fdea29dfc812af03da22c13702f418b7c4d3af13 Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Tue, 2 Jun 2015 14:06:50 -0400 Subject: [PATCH 3/3] Collection#add shouldn't parse a model instance Fixes #3636, but in a backwards compatible way. --- backbone.js | 6 +++--- test/collection.js | 20 +++----------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/backbone.js b/backbone.js index 53fb08729..945a5458d 100644 --- a/backbone.js +++ b/backbone.js @@ -789,7 +789,7 @@ // the core operation for updating the data contained by the collection. set: function(models, options) { options = _.defaults({}, options, setOptions); - if (options.parse) models = this.parse(models, options); + if (options.parse && !this._isModel(models)) models = this.parse(models, options); var singular = !_.isArray(models); models = singular ? (models ? [models] : []) : models.slice(); var id, model, attrs, existing, sort; @@ -1776,7 +1776,7 @@ this._updateHash(this.location, fragment, options.replace); if (this.iframe && (fragment !== this.getHash(this.iframe.contentWindow))) { var iWindow = this.iframe.contentWindow; - + // Opening and closing the iframe tricks IE7 and earlier to push a // history entry on hash-tag change. When replace is true, we don't // want this. @@ -1784,7 +1784,7 @@ iWindow.document.open(); iWindow.document.close(); } - + this._updateHash(iWindow.location, fragment, options.replace); } diff --git a/test/collection.js b/test/collection.js index 507191e3e..fa8481861 100644 --- a/test/collection.js +++ b/test/collection.js @@ -561,31 +561,17 @@ }); - test("create with wait:true should not call collection.parse", 1, function() { - var collectionParseCalled = false; - - var Model = Backbone.Model.extend({ - sync: function (method, model, options) { - _.extend(options, {specialSync: true}); - return Backbone.Model.prototype.sync.call(this, method, model, options); - } - }); - + test("create with wait:true should not call collection.parse", 0, function() { var Collection = Backbone.Collection.extend({ - model: Model, url: '/test', parse: function () { - collectionParseCalled = true; + ok(false); } }); var collection = new Collection; - var success = function (model, response, options) { - equal(collectionParseCalled, false); - }; - - collection.create({}, {wait: true, success: success}); + collection.create({}, {wait: true}); this.ajaxSettings.success(); });