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

Save with wait:true, patch: true and custom serialize() method #64

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
41 changes: 39 additions & 2 deletions ampersand-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -20,6 +24,31 @@ 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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nitpicky performance tweak would be to move this before the keys(attrs) line above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing.

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) {
// run transformForPatch on the next level
result[key] = transformForPatch(val, attrs);
});
};


var Model = State.extend({
save: function (key, val, options) {
var attrs, method;
Expand Down Expand Up @@ -60,10 +89,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(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
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}));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this the one that was causing errors when using this.serialize() then?

clonedModel.set(attrs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, this is so you can call .set probably, I should read everything before commenting lol

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, correct.

The issue with this.serialize() was because i was ignoring the top-level object up in transformForPatch. I pulled the intersection stuff out of the transform so that it always runs against the passed-in object first.


options.attrs = clonedModel.serialize();
}
var sync = this.sync(method, this, options);

// Make the request available on the options object so it can be accessed
Expand Down
6 changes: 5 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 15 additions & 0 deletions test/backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down