From b94e6c97f94797e0f125797f7eafd9be52324a9a Mon Sep 17 00:00:00 2001 From: Bob Holt Date: Fri, 13 Nov 2015 12:07:13 -0500 Subject: [PATCH 1/5] fixes #52 When calling the `save()` method with `wait: true` and `patch: true`, the current implmentation assumes a basic data structure and ignored the structure generated by an overridden `serialize` method. This fix invokes the `serialize()` method to ensure the user's data structure is respected. In order for this to work with `PATCH`, we have to inspect that structure to find where the attributes have been placed, and then limit the properties of those object to only the ones changed. This still makes some assumptions about the data structure (namely that all attributes appear within a single object within the serialized structure), but hopefully less restrictive ones than the current implementation. --- ampersand-model.js | 40 ++++++++++++++++++++++++++++++++++++++-- package.json | 6 +++++- test/backbone.js | 15 +++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/ampersand-model.js b/ampersand-model.js index 22cb315..4dc8f05 100755 --- a/ampersand-model.js +++ b/ampersand-model.js @@ -5,6 +5,10 @@ var assign = require('lodash.assign'); var isObject = require('lodash.isobject'); var clone = require('lodash.clone'); var result = require('lodash.result'); +var transform = require('lodash.transform'); +var keys = require('lodash.keys'); +var intersection = require('lodash.intersection'); +var reduce = require('lodash.reduce'); // Throw an error when a URL is needed, and none is supplied. var urlError = function () { @@ -20,6 +24,30 @@ var wrapError = function (model, options) { }; }; +// recursive function to find the attrs in the serialized object +function transformForPatch(obj, attrs) { + var attrKeys = keys(attrs); + + if (!isObject(obj)) { + return obj; + } + + return transform(obj, function(result, val, key) { + // If all attributes are within the object + if (intersection(keys(val), attrKeys).length === attrKeys.length) { + // set this object to the subset of passed-in attrs + result[key] = reduce(attrKeys, function(redux, attrKey) { + redux[attrKey] = attrs[attrKey]; + return redux; + }, {}); + } else { + // run transformForPatch on the next level + result[key] = transformForPatch(val, attrs); + } + }); +} + + var Model = State.extend({ save: function (key, val, options) { var attrs, method; @@ -60,10 +88,18 @@ var Model = State.extend({ wrapError(this, options); method = this.isNew() ? 'create' : (options.patch ? 'patch' : 'update'); - if (method === 'patch') options.attrs = attrs; + + if (method === 'patch') { + options.attrs = transformForPatch(new this.constructor(attrs).serialize(), attrs); + } // if we're waiting we haven't actually set our attributes yet so // we need to do make sure we send right data - if (options.wait && method !== 'patch') options.attrs = assign(model.serialize(), attrs); + if (options.wait && method !== 'patch') { + var clonedModel = new this.constructor(this.getAttributes({props: true})); + clonedModel.set(attrs); + + options.attrs = assign(model.serialize(), attrs); + } var sync = this.sync(method, this, options); // Make the request available on the options object so it can be accessed diff --git a/package.json b/package.json index bcbbcda..4c0326a 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,12 @@ "ampersand-version": "^1.0.2", "lodash.assign": "^3.2.0", "lodash.clone": "^3.0.3", + "lodash.intersection": "^3.2.0", "lodash.isobject": "^3.0.2", - "lodash.result": "^3.1.2" + "lodash.keys": "^3.1.2", + "lodash.reduce": "^3.1.2", + "lodash.result": "^3.1.2", + "lodash.transform": "^3.0.4" }, "devDependencies": { "browserify": "^8.1.0", diff --git a/test/backbone.js b/test/backbone.js index da12d4e..744d689 100755 --- a/test/backbone.js +++ b/test/backbone.js @@ -870,6 +870,21 @@ var Backbone = { t.equal(env.syncArgs.options.attrs.a, undefined); }); + test("save with PATCH and `wait` with a custom serialize method only sends specified attributes in custom format", function (t) { + t.plan(4); + var Model = Backbone.Model.extend({ + id: 1, + serialize: function() {return {data: {attributes: this.getAttributes({props: true}, true)}};} + }); + var model = new Model({parent_id: 1}); + model.set({a: 1, b: 2, d: 4, e: 5}); + model.save({b: 2, d: 4}, {patch: true, wait: true }); + t.equal(env.syncArgs.method, 'patch'); + t.equal(_.size(env.syncArgs.options.attrs.data.attributes), 2); + t.equal(env.syncArgs.options.attrs.data.attributes.d, 4); + t.equal(env.syncArgs.options.attrs.data.attributes.a, undefined); + }); + test.skip("save doesn't validate twice", function (t) { t.plan(1); var model = new Backbone.Model(); From c421529d5b783df6fdd192e629b42fd3d82637ae Mon Sep 17 00:00:00 2001 From: Bob Holt Date: Fri, 13 Nov 2015 12:31:10 -0500 Subject: [PATCH 2/5] make `transformForPatch` a function expression --- ampersand-model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ampersand-model.js b/ampersand-model.js index 4dc8f05..e03df23 100755 --- a/ampersand-model.js +++ b/ampersand-model.js @@ -25,7 +25,7 @@ var wrapError = function (model, options) { }; // recursive function to find the attrs in the serialized object -function transformForPatch(obj, attrs) { +var transformForPatch = function transformForPatch(obj, attrs) { var attrKeys = keys(attrs); if (!isObject(obj)) { @@ -45,7 +45,7 @@ function transformForPatch(obj, attrs) { result[key] = transformForPatch(val, attrs); } }); -} +}; var Model = State.extend({ From 71c0a5c1bc6514c4d2f60e6966d85a69bda6a7d3 Mon Sep 17 00:00:00 2001 From: Bob Holt Date: Fri, 13 Nov 2015 13:01:42 -0500 Subject: [PATCH 3/5] remove unnecessary instantiation of constructor and fix transformForPatch` --- ampersand-model.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/ampersand-model.js b/ampersand-model.js index e03df23..7ae7751 100755 --- a/ampersand-model.js +++ b/ampersand-model.js @@ -26,24 +26,25 @@ var wrapError = function (model, options) { // recursive function to find the attrs in the serialized object var transformForPatch = function transformForPatch(obj, attrs) { + var attrKeys = keys(attrs); if (!isObject(obj)) { return obj; } + // If all attributes are within the object + if (intersection(keys(obj), attrKeys).length === attrKeys.length) { + // return the subset of passed-in attrs + return reduce(attrKeys, function(redux, attrKey) { + redux[attrKey] = attrs[attrKey]; + return redux; + }, {}); + } + return transform(obj, function(result, val, key) { - // If all attributes are within the object - if (intersection(keys(val), attrKeys).length === attrKeys.length) { - // set this object to the subset of passed-in attrs - result[key] = reduce(attrKeys, function(redux, attrKey) { - redux[attrKey] = attrs[attrKey]; - return redux; - }, {}); - } else { - // run transformForPatch on the next level - result[key] = transformForPatch(val, attrs); - } + // run transformForPatch on the next level + result[key] = transformForPatch(val, attrs); }); }; @@ -90,7 +91,7 @@ var Model = State.extend({ method = this.isNew() ? 'create' : (options.patch ? 'patch' : 'update'); if (method === 'patch') { - options.attrs = transformForPatch(new this.constructor(attrs).serialize(), attrs); + options.attrs = transformForPatch(this.serialize(), attrs); } // if we're waiting we haven't actually set our attributes yet so // we need to do make sure we send right data From 78d60981500b33de5b15a589101fc317cc28d879 Mon Sep 17 00:00:00 2001 From: Bob Holt Date: Fri, 13 Nov 2015 13:04:55 -0500 Subject: [PATCH 4/5] serialize clonedModel in `wait:true` --- ampersand-model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ampersand-model.js b/ampersand-model.js index 7ae7751..a88fd93 100755 --- a/ampersand-model.js +++ b/ampersand-model.js @@ -99,7 +99,7 @@ var Model = State.extend({ var clonedModel = new this.constructor(this.getAttributes({props: true})); clonedModel.set(attrs); - options.attrs = assign(model.serialize(), attrs); + options.attrs = clonedModel.serialize(); } var sync = this.sync(method, this, options); From 6e885a8b90112b80ad092a5881ca2dc0dd869e71 Mon Sep 17 00:00:00 2001 From: Bob Holt Date: Fri, 13 Nov 2015 13:17:27 -0500 Subject: [PATCH 5/5] return early if not an object in `transformForPatch` --- ampersand-model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ampersand-model.js b/ampersand-model.js index a88fd93..2c8bf8d 100755 --- a/ampersand-model.js +++ b/ampersand-model.js @@ -27,12 +27,12 @@ var wrapError = function (model, options) { // recursive function to find the attrs in the serialized object var transformForPatch = function transformForPatch(obj, attrs) { - var attrKeys = keys(attrs); - if (!isObject(obj)) { return obj; } + var attrKeys = keys(attrs); + // If all attributes are within the object if (intersection(keys(obj), attrKeys).length === attrKeys.length) { // return the subset of passed-in attrs