From ec61503c874ebbb147595e5ded86601164b54002 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 2 Feb 2012 08:53:10 -0500 Subject: [PATCH] fixes #928 - Save sends correct attrs. * Temporarily set model's attrs for `sync`. * Remove cross-module (global) dependencies in Collection, Model, and sync test modules. --- backbone.js | 12 ++++++++--- test/collection.js | 19 +++++++++++----- test/model.js | 54 +++++++++++++++++++++++++++++++++------------- test/sync.js | 23 ++++++++++++++------ 4 files changed, 78 insertions(+), 30 deletions(-) diff --git a/backbone.js b/backbone.js index a44fce30..3dcf3699 100644 --- a/backbone.js +++ b/backbone.js @@ -289,7 +289,7 @@ // If the server returns an attributes hash that differs, the model's // state will be `set` again. save: function(key, value, options) { - var attrs; + var attrs, current; if (_.isObject(key) || key == null) { attrs = key; options = value; @@ -299,7 +299,11 @@ } options = options ? _.clone(options) : {}; - if (attrs && !this[options.wait ? '_validate' : 'set'](attrs, options)) return false; + if (options.wait) current = _.clone(this.attributes); + var silentOptions = _.extend({}, options, {silent: true}); + if (attrs && !this.set(attrs, options.wait ? silentOptions : options)) { + return false; + } var model = this; var success = options.success; options.success = function(resp, status, xhr) { @@ -314,7 +318,9 @@ }; options.error = Backbone.wrapError(options.error, model, options); var method = this.isNew() ? 'create' : 'update'; - return (this.sync || Backbone.sync).call(this, method, this, options); + var xhr = (this.sync || Backbone.sync).call(this, method, this, options); + if (options.wait) this.set(current, silentOptions); + return xhr; }, // Destroy this model on the server if it was already persisted. diff --git a/test/collection.js b/test/collection.js index 9c8a3c32..99759478 100644 --- a/test/collection.js +++ b/test/collection.js @@ -1,12 +1,21 @@ $(document).ready(function() { - module("Backbone.Collection"); + var lastRequest = null; + var sync = Backbone.sync; - window.lastRequest = null; + module("Backbone.Collection", { - Backbone.sync = function() { - lastRequest = _.toArray(arguments); - }; + setup: function() { + Backbone.sync = function() { + lastRequest = _.toArray(arguments); + }; + }, + + teardown: function() { + Backbone.sync = sync; + } + + }); var a = new Backbone.Model({id: 3, label: 'a'}); var b = new Backbone.Model({id: 2, label: 'b'}); diff --git a/test/model.js b/test/model.js index feb24b8e..3ee29425 100644 --- a/test/model.js +++ b/test/model.js @@ -1,16 +1,32 @@ $(document).ready(function() { - module("Backbone.Model"); - // Variable to catch the last request. - window.lastRequest = null; + var lastRequest = null; + // Variable to catch ajax params. + var ajaxParams = null; + var sync = Backbone.sync; + var ajax = $.ajax; + var urlRoot = null; - window.originalSync = Backbone.sync; + module("Backbone.Model", { - // Stub out Backbone.request... - Backbone.sync = function() { - lastRequest = _.toArray(arguments); - }; + setup: function() { + Backbone.sync = function() { + lastRequest = _.toArray(arguments); + sync.apply(this, arguments); + }; + $.ajax = function(params) { ajaxParams = params; }; + urlRoot = Backbone.Model.prototype.urlRoot; + Backbone.Model.prototype.urlRoot = '/'; + }, + + teardown: function() { + Backbone.sync = sync; + $.ajax = ajax; + Backbone.Model.prototype.urlRoot = urlRoot; + } + + }); var attrs = { id : '1-the-tempest', @@ -67,13 +83,8 @@ $(document).ready(function() { doc.collection.url = '/collection/'; equal(doc.url(), '/collection/1-the-tempest'); doc.collection = null; - var failed = false; - try { - doc.url(); - } catch (e) { - failed = true; - } - equal(failed, true); + doc.urlRoot = null; + raises(function() { doc.url(); }); doc.collection = collection; }); @@ -603,4 +614,17 @@ $(document).ready(function() { equal(model.previous(''), true); }); + test("`save` with `wait` sends correct attributes", function() { + var changed = 0; + var model = new Backbone.Model({x: 1, y: 2}); + model.on('change:x', function() { changed++; }); + model.save({x: 3}, {wait: true}); + deepEqual(JSON.parse(ajaxParams.data), {x: 3, y: 2}); + equal(model.get('x'), 1); + equal(changed, 0); + lastRequest[2].success({}); + equal(model.get('x'), 3); + equal(changed, 1); + }); + }); diff --git a/test/sync.js b/test/sync.js index 0059ccca..40e9ac28 100644 --- a/test/sync.js +++ b/test/sync.js @@ -1,11 +1,21 @@ $(document).ready(function() { - module("Backbone.sync", {setup : function() { - window.lastRequest = null; - $.ajax = function(obj) { - lastRequest = obj; - }; - }}); + var ajax = $.ajax + var lastRequest = null; + + module("Backbone.sync", { + + setup : function() { + $.ajax = function(obj) { + lastRequest = obj; + }; + }, + + teardown: function() { + $.ajax = ajax; + } + + }); var Library = Backbone.Collection.extend({ url : function() { return '/library'; } @@ -20,7 +30,6 @@ $(document).ready(function() { }; test("sync: read", function() { - Backbone.sync = originalSync; library.fetch(); equal(lastRequest.url, '/library'); equal(lastRequest.type, 'GET');