From deff340560a2090001172c23830691684552ca79 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Tue, 16 Apr 2013 13:44:00 -0700 Subject: [PATCH 1/6] fix #2345 return a rejected promise instead of false on invalid model save --- backbone.js | 6 +++--- test/model.js | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/backbone.js b/backbone.js index 7333a278d..6b2445ad1 100644 --- a/backbone.js +++ b/backbone.js @@ -445,7 +445,7 @@ // If the server returns an attributes hash that differs, the model's // state will be `set` again. save: function(key, val, options) { - var attrs, method, xhr, attributes = this.attributes; + var attrs, method, xhr, dfd = new $.Deferred, attributes = this.attributes; // Handle both `"key", value` and `{key: value}` -style arguments. if (key == null || typeof key === 'object') { @@ -461,9 +461,9 @@ // `set(attr).save(null, opts)` with validation. Otherwise, check if // the model will be valid when the attributes, if any, are set. if (attrs && !options.wait) { - if (!this.set(attrs, options)) return false; + if (!this.set(attrs, options)) return dfd.rejectWith(this); } else { - if (!this._validate(attrs, options)) return false; + if (!this._validate(attrs, options)) return dfd.rejectWith(this); } // Set temporary attributes if `{wait: true}`. diff --git a/test/model.js b/test/model.js index faede68a8..59680877b 100644 --- a/test/model.js +++ b/test/model.js @@ -959,10 +959,16 @@ $(document).ready(function() { var model = new Backbone.Model; model.validate = function(){ return 'invalid'; }; model.sync = function(){ ok(false); }; - strictEqual(model.save(), false); + model.save() + .done(function() { + ok(false); + }) + .fail(function() { + ok(true); + }); }); - test("#1377 - Save without attrs triggers 'error'.", 1, function() { + test("#1377 - Save without attrs triggers 'error'.", 2, function() { var Model = Backbone.Model.extend({ url: '/test/', sync: function(method, model, options){ options.success(); }, @@ -970,7 +976,13 @@ $(document).ready(function() { }); var model = new Model({id: 1}); model.on('invalid', function(){ ok(true); }); - model.save(); + model.save() + .done(function() { + ok(false); + }) + .fail(function() { + ok(true); + }); }); test("#1545 - `undefined` can be passed to a model constructor without coersion", function() { From b201d1a1ea3951d8a6d5cf0850a1ee43b35935c1 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Thu, 18 Apr 2013 13:10:14 -0700 Subject: [PATCH 2/6] allow Backbone.Deferred to be overwritten on a framework level. Also changes the `rejectWith` calls to just `reject` since most promise implementations dont have this method --- backbone.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/backbone.js b/backbone.js index 6b2445ad1..543c4de32 100644 --- a/backbone.js +++ b/backbone.js @@ -445,7 +445,7 @@ // If the server returns an attributes hash that differs, the model's // state will be `set` again. save: function(key, val, options) { - var attrs, method, xhr, dfd = new $.Deferred, attributes = this.attributes; + var attrs, method, xhr, dfd = Backbone.Deferred(), attributes = this.attributes; // Handle both `"key", value` and `{key: value}` -style arguments. if (key == null || typeof key === 'object') { @@ -461,9 +461,9 @@ // `set(attr).save(null, opts)` with validation. Otherwise, check if // the model will be valid when the attributes, if any, are set. if (attrs && !options.wait) { - if (!this.set(attrs, options)) return dfd.rejectWith(this); + if (!this.set(attrs, options)) return dfd.reject(); } else { - if (!this._validate(attrs, options)) return dfd.rejectWith(this); + if (!this._validate(attrs, options)) return dfd.reject(); } // Set temporary attributes if `{wait: true}`. @@ -1204,6 +1204,10 @@ return Backbone.$.ajax.apply(Backbone.$, arguments); }; + // Set the default promises library to proxy to `$`. + // Override this if you'd like to use a different library. + Backbone.Deferred = Backbone.$.Deferred; + // Backbone.Router // --------------- From 988b4f0d68ee9148b50678ad461f3def247d4396 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Thu, 18 Apr 2013 13:28:09 -0700 Subject: [PATCH 3/6] add backbone.Deferred sync tests --- test/sync.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/sync.js b/test/sync.js index 8fddb47fa..57dcf0988 100644 --- a/test/sync.js +++ b/test/sync.js @@ -155,6 +155,21 @@ $(document).ready(function() { Backbone.sync('create', model); }); + test("Backbone.Deferred", 2, function() { + Backbone.sync = function(settings){ + return Backbone.Deferred().resolve(); + }; + var model = new Backbone.Model(); + model.url = '/test'; + model.validate = function(attrs) { + return !attrs.accept; + } + result = model.save({accept: false}); + strictEqual(result.state(), 'rejected'); + result = model.save({accept: true}); + strictEqual(result.state(), 'resolved'); + }); + test("Call provided error callback on error.", 1, function() { var model = new Backbone.Model; model.url = '/test'; From 4bde8154887b6c6f25058b939a13be0607c39f45 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Thu, 18 Apr 2013 13:36:20 -0700 Subject: [PATCH 4/6] return a rejected promise when Model#destroy fails --- backbone.js | 2 +- test/model.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backbone.js b/backbone.js index 543c4de32..7ec4b62f9 100644 --- a/backbone.js +++ b/backbone.js @@ -519,7 +519,7 @@ if (this.isNew()) { options.success(); - return false; + return Backbone.Deferred().reject(); } wrapError(this, options); diff --git a/test/model.js b/test/model.js index 59680877b..10c91e7f9 100644 --- a/test/model.js +++ b/test/model.js @@ -489,7 +489,7 @@ $(document).ready(function() { ok(_.isEqual(this.syncArgs.model, doc)); var newModel = new Backbone.Model; - equal(newModel.destroy(), false); + equal(newModel.destroy().state(), 'rejected'); }); test("non-persisted destroy", 1, function() { From 91636355b0ada9c57590aad35644e15c22981d43 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Thu, 18 Apr 2013 13:47:32 -0700 Subject: [PATCH 5/6] functional consistency, fix global variable, refactor .done .fail to .then --- backbone.js | 4 +++- test/model.js | 24 ++++++++++-------------- test/sync.js | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/backbone.js b/backbone.js index 7ec4b62f9..870a18d1e 100644 --- a/backbone.js +++ b/backbone.js @@ -1206,7 +1206,9 @@ // Set the default promises library to proxy to `$`. // Override this if you'd like to use a different library. - Backbone.Deferred = Backbone.$.Deferred; + Backbone.Deferred = function() { + return Backbone.$.Deferred.apply(Backbone.$, arguments); + } // Backbone.Router // --------------- diff --git a/test/model.js b/test/model.js index 10c91e7f9..c15167a82 100644 --- a/test/model.js +++ b/test/model.js @@ -959,13 +959,11 @@ $(document).ready(function() { var model = new Backbone.Model; model.validate = function(){ return 'invalid'; }; model.sync = function(){ ok(false); }; - model.save() - .done(function() { - ok(false); - }) - .fail(function() { - ok(true); - }); + model.save().then(function() { + ok(false); + }, function() { + ok(true); + }); }); test("#1377 - Save without attrs triggers 'error'.", 2, function() { @@ -976,13 +974,11 @@ $(document).ready(function() { }); var model = new Model({id: 1}); model.on('invalid', function(){ ok(true); }); - model.save() - .done(function() { - ok(false); - }) - .fail(function() { - ok(true); - }); + model.save().then(function() { + ok(false); + }, function() { + ok(true); + }); }); test("#1545 - `undefined` can be passed to a model constructor without coersion", function() { diff --git a/test/sync.js b/test/sync.js index 57dcf0988..14f408548 100644 --- a/test/sync.js +++ b/test/sync.js @@ -164,7 +164,7 @@ $(document).ready(function() { model.validate = function(attrs) { return !attrs.accept; } - result = model.save({accept: false}); + var result = model.save({accept: false}); strictEqual(result.state(), 'rejected'); result = model.save({accept: true}); strictEqual(result.state(), 'resolved'); From 247f560a1953bd02074f9e16349859da0d82fbd2 Mon Sep 17 00:00:00 2001 From: Adam Krebs Date: Thu, 6 Jun 2013 16:43:29 -0400 Subject: [PATCH 6/6] destroying unpersisted model should resolve promise, not reject --- backbone.js | 2 +- test/model.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backbone.js b/backbone.js index 870a18d1e..5709b4b35 100644 --- a/backbone.js +++ b/backbone.js @@ -519,7 +519,7 @@ if (this.isNew()) { options.success(); - return Backbone.Deferred().reject(); + return Backbone.Deferred().resolve(); } wrapError(this, options); diff --git a/test/model.js b/test/model.js index c15167a82..c82506841 100644 --- a/test/model.js +++ b/test/model.js @@ -489,7 +489,7 @@ $(document).ready(function() { ok(_.isEqual(this.syncArgs.model, doc)); var newModel = new Backbone.Model; - equal(newModel.destroy().state(), 'rejected'); + equal(newModel.destroy().state(), 'resolved'); }); test("non-persisted destroy", 1, function() {