Merge pull request #2221 from tgriesser/sync-revert2

Making error/success handlers promise compatible

backbone.js

@@ -420,11 +420,14 @@
     fetch: function(options) {
       options = options ? _.clone(options) : {};
       if (options.parse === void 0) options.parse = true;
+      var model = this;
       var success = options.success;
-      options.success = function(model, resp, options) {
+      options.success = function(resp) {
         if (!model.set(model.parse(resp, options), options)) return false;
         if (success) success(model, resp, options);
+        model.trigger('sync', model, resp, options);
       };
+      wrapError(this, options);
       return this.sync('read', this, options);
     },
 
@@ -432,7 +435,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, success, method, xhr, attributes = this.attributes;
+      var attrs, method, xhr, attributes = this.attributes;
 
       // Handle both `"key", value` and `{key: value}` -style arguments.
       if (key == null || typeof key === 'object') {
@@ -458,8 +461,9 @@
       // After a successful server-side save, the client is (optionally)
       // updated with the server-side state.
       if (options.parse === void 0) options.parse = true;
-      success = options.success;
-      options.success = function(model, resp, options) {
+      var model = this;
+      var success = options.success;
+      options.success = function(resp) {
         // Ensure attributes are restored during synchronous saves.
         model.attributes = attributes;
         var serverAttrs = model.parse(resp, options);
@@ -468,9 +472,10 @@
           return false;
         }
         if (success) success(model, resp, options);
+        model.trigger('sync', model, resp, options);
       };
+      wrapError(this, options);
 
-      // Finish configuring and sending the Ajax request.
       method = this.isNew() ? 'create' : (options.patch ? 'patch' : 'update');
       if (method === 'patch') options.attrs = attrs;
       xhr = this.sync(method, this, options);
@@ -493,15 +498,17 @@
         model.trigger('destroy', model, model.collection, options);
       };
 
-      options.success = function(model, resp, options) {
+      options.success = function(resp) {
         if (options.wait || model.isNew()) destroy();
         if (success) success(model, resp, options);
+        if (!model.isNew()) model.trigger('sync', model, resp, options);
       };
 
       if (this.isNew()) {
-        options.success(this, null, options);
+        options.success();
         return false;
       }
+      wrapError(this, options);
 
       var xhr = this.sync('delete', this, options);
       if (!options.wait) destroy();
@@ -823,11 +830,14 @@
       options = options ? _.clone(options) : {};
       if (options.parse === void 0) options.parse = true;
       var success = options.success;
-      options.success = function(collection, resp, options) {
+      var collection = this;
+      options.success = function(resp) {
         var method = options.update ? 'update' : 'reset';
         collection[method](resp, options);
         if (success) success(collection, resp, options);
+        collection.trigger('sync', collection, resp, options);
       };
+      wrapError(this, options);
       return this.sync('read', this, options);
     },
 
@@ -840,7 +850,7 @@
       if (!options.wait) this.add(model, options);
       var collection = this;
       var success = options.success;
-      options.success = function(model, resp, options) {
+      options.success = function(resp) {
         if (options.wait) collection.add(model, options);
         if (success) success(model, resp, options);
       };
@@ -1439,18 +1449,6 @@
       params.processData = false;
     }
 
-    var success = options.success;
-    options.success = function(resp) {
-      if (success) success(model, resp, options);
-      model.trigger('sync', model, resp, options);
-    };
-
-    var error = options.error;
-    options.error = function(xhr) {
-      if (error) error(model, xhr, options);
-      model.trigger('error', model, xhr, options);
-    };
-
     // Make the request, allowing the user to override any Ajax options.
     var xhr = options.xhr = Backbone.ajax(_.extend(params, options));
     model.trigger('request', model, xhr, options);
@@ -1509,4 +1507,13 @@
     throw new Error('A "url" property or function must be specified');
   };
 
+  // Wrap an optional error callback with a fallback error event.
+  var wrapError = function (model, options) {
+    var error = options.error;
+    options.error = function(resp) {
+      if (error) error(model, resp, options);
+      model.trigger('error', model, resp, options);
+    };
+  };
+
 }).call(this);

test/collection.js

@@ -362,9 +362,7 @@ $(document).ready(function() {
 
   test("model destroy removes from all collections", 3, function() {
     var e = new Backbone.Model({id: 5, title: 'Othello'});
-    e.sync = function(method, model, options) {
-      options.success(model, [], options);
-    };
+    e.sync = function(method, model, options) { options.success(); };
     var colE = new Backbone.Collection([e]);
     var colF = new Backbone.Collection([e]);
     e.destroy();
@@ -396,6 +394,15 @@ $(document).ready(function() {
     equal(this.syncArgs.options.parse, false);
   });
 
+  test("fetch with an error response triggers an error event", 1, function () {
+    var collection = new Backbone.Collection();
+    collection.on('error', function () {
+      ok(true);
+    });
+    collection.sync = function (method, model, options) { options.error(); };
+    collection.fetch();
+  });
+
   test("ensure fetch only parses once", 1, function() {
     var collection = new Backbone.Collection;
     var counter = 0;
@@ -405,7 +412,7 @@ $(document).ready(function() {
     };
     collection.url = '/test';
     collection.fetch();
-    this.syncArgs.options.success([]);
+    this.syncArgs.options.success();
     equal(counter, 1);
   });
 
@@ -705,9 +712,7 @@ $(document).ready(function() {
   test("#1447 - create with wait adds model.", 1, function() {
     var collection = new Backbone.Collection;
     var model = new Backbone.Model;
-    model.sync = function(method, model, options){
-      options.success(model, [], options);
-    };
+    model.sync = function(method, model, options){ options.success(); };
     collection.on('add', function(){ ok(true); });
     collection.create(model, {wait: true});
   });

test/model.js

@@ -401,7 +401,7 @@ $(document).ready(function() {
       if (attrs.admin) return "Can't change admin status.";
     };
     model.sync = function(method, model, options) {
-      options.success.call(this, this, {admin: true}, options);
+      options.success.call(this, {admin: true});
     };
     model.on('invalid', function(model, error) {
       lastError = error;
@@ -418,6 +418,19 @@ $(document).ready(function() {
     ok(_.isEqual(this.syncArgs.model, doc));
   });
 
+  test("save, fetch, destroy triggers error event when an error occurs", 3, function () {
+    var model = new Backbone.Model();
+    model.on('error', function () {
+      ok(true);
+    });
+    model.sync = function (method, model, options) {
+      options.error();
+    };
+    model.save({data: 2, id: 1});
+    model.fetch();
+    model.destroy();
+  });
+
   test("save with PATCH", function() {
     doc.clear().set({id: 1, a: 1, b: 2, c: 3, d: 4});
     doc.save();
@@ -435,7 +448,7 @@ $(document).ready(function() {
   test("save in positional style", 1, function() {
     var model = new Backbone.Model();
     model.sync = function(method, model, options) {
-      options.success(model, {}, options);
+      options.success();
     };
     model.save('title', 'Twelfth Night');
     equal(model.get('title'), 'Twelfth Night');
@@ -444,8 +457,8 @@ $(document).ready(function() {
   test("save with non-object success response", 2, function () {
     var model = new Backbone.Model();
     model.sync = function(method, model, options) {
-      options.success(model, '', options);
-      options.success(model, null, options);
+      options.success('', options);
+      options.success(null, options);
     };
     model.save({testing:'empty'}, {
       success: function (model) {
@@ -720,7 +733,7 @@ $(document).ready(function() {
   test("#1030 - `save` with `wait` results in correct attributes if success is called during sync", 2, function() {
     var model = new Backbone.Model({x: 1, y: 2});
     model.sync = function(method, model, options) {
-      options.success(model, {}, options);
+      options.success();
     };
     model.on("change:x", function() { ok(true); });
     model.save({x: 3}, {wait: true});
@@ -893,7 +906,7 @@ $(document).ready(function() {
       }
     };
     model.sync = function(method, model, options) {
-      options.success(model, {}, options);
+      options.success();
     };
     model.save({id: 1}, opts);
     model.fetch(opts);
@@ -902,9 +915,8 @@ $(document).ready(function() {
 
   test("#1412 - Trigger 'sync' event.", 3, function() {
     var model = new Backbone.Model({id: 1});
-    model.url = '/test';
+    model.sync = function (method, model, options) { options.success(); };
     model.on('sync', function(){ ok(true); });
-    Backbone.ajax = function(settings){ settings.success(); };
     model.fetch();
     model.save();
     model.destroy();
@@ -950,7 +962,7 @@ $(document).ready(function() {
     var Model = Backbone.Model.extend({
       sync: function(method, model, options) {
         setTimeout(function(){
-          options.success(model, {}, options);
+          options.success();
           start();
         }, 0);
       }