Merge pull request #1599 from braddunbar/sync

Consolidate sync/error events in Backbone.sync.

backbone.js

@@ -336,9 +336,7 @@
       options.success = function(resp, status, xhr) {
         if (!model.set(model.parse(resp, xhr), options)) return false;
         if (success) success(model, resp, options);
-        model.trigger('sync', model, resp, options);
       };
-      options.error = Backbone.wrapError(options.error, model, options);
       return this.sync('read', this, options);
     },
 
@@ -383,11 +381,9 @@
         if (options.wait) serverAttrs = _.extend(attrs || {}, serverAttrs);
         if (!model.set(serverAttrs, options)) return false;
         if (success) success(model, resp, options);
-        model.trigger('sync', model, resp, options);
       };
 
       // Finish configuring and sending the Ajax request.
-      options.error = Backbone.wrapError(options.error, model, options);
       var xhr = this.sync(this.isNew() ? 'create' : 'update', this, options);
 
       // When using `wait`, reset attributes to original values unless
@@ -415,7 +411,6 @@
       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()) {
@@ -423,7 +418,6 @@
         return false;
       }
 
-      options.error = Backbone.wrapError(options.error, model, options);
       var xhr = this.sync('delete', this, options);
       if (!options.wait) destroy();
       return xhr;
@@ -778,9 +772,7 @@
       options.success = function(resp, status, xhr) {
         collection[options.add ? 'add' : 'reset'](collection.parse(resp, xhr), options);
         if (success) success(collection, resp, options);
-        collection.trigger('sync', collection, resp, options);
       };
-      options.error = Backbone.wrapError(options.error, collection, options);
       return this.sync('read', this, options);
     },
 
@@ -1382,6 +1374,18 @@
       params.processData = false;
     }
 
+    var success = options.success;
+    options.success = function(resp, status, xhr) {
+      if (success) success(resp, status, xhr);
+      model.trigger('sync', model, resp, options);
+    };
+
+    var error = options.error;
+    options.error = function(xhr, status, error) {
+      if (error) error(model, xhr, options);
+      model.trigger('error', model, xhr, options);
+    };
+
     // Make the request, allowing the user to override any Ajax options.
     return Backbone.ajax(_.extend(params, options));
   };
@@ -1391,15 +1395,6 @@
     return Backbone.$.ajax.apply(Backbone.$, arguments);
   };
 
-  // Wrap an optional error callback with a fallback error event.
-  Backbone.wrapError = function(onError, originalModel, options) {
-    return function(model, resp) {
-      resp = model === originalModel ? resp : model;
-      if (onError) onError(originalModel, resp, options);
-      originalModel.trigger('error', originalModel, resp, options);
-    };
-  };
-
   // Helpers
   // -------
 

test/collection.js

@@ -2,6 +2,8 @@ $(document).ready(function() {
 
   var lastRequest = null;
   var sync = Backbone.sync;
+  var ajaxParams;
+  var ajax = Backbone.ajax;
 
   var a, b, c, d, e, col, otherCol;
 
@@ -22,11 +24,15 @@ $(document).ready(function() {
           model: model,
           options: options
         };
+        sync.apply(this, arguments);
       };
+
+      Backbone.ajax = function(params) { ajaxParams = params; };
     },
 
     teardown: function() {
       Backbone.sync = sync;
+      Backbone.ajax = ajax;
     }
 
   });
@@ -379,21 +385,25 @@ $(document).ready(function() {
   });
 
   test("Collection: fetch", 4, function() {
-    col.fetch();
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
+    collection.fetch();
     equal(lastRequest.method, 'read');
-    equal(lastRequest.model, col);
+    equal(lastRequest.model, collection);
     equal(lastRequest.options.parse, true);
 
-    col.fetch({parse: false});
+    collection.fetch({parse: false});
     equal(lastRequest.options.parse, false);
   });
 
   test("Collection: create", 4, function() {
-    var model = col.create({label: 'f'}, {wait: true});
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
+    var model = collection.create({label: 'f'}, {wait: true});
     equal(lastRequest.method, 'create');
     equal(lastRequest.model, model);
     equal(model.get('label'), 'f');
-    equal(model.collection, col);
+    equal(model.collection, collection);
   });
 
   test("Collection: create enforces validation", 1, function() {
@@ -524,16 +534,17 @@ $(document).ready(function() {
   });
 
   test("#714: access `model.collection` in a brand new model.", 2, function() {
-    var col = new Backbone.Collection;
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
     var Model = Backbone.Model.extend({
       set: function(attrs) {
         equal(attrs.prop, 'value');
-        equal(this.collection, col);
+        equal(this.collection, collection);
         return this;
       }
     });
-    col.model = Model;
-    col.create({prop: 'value'});
+    collection.model = Model;
+    collection.create({prop: 'value'});
   });
 
   test("#574, remove its own reference to the .models array.", 2, function() {
@@ -659,15 +670,10 @@ $(document).ready(function() {
   });
 
   test("#1412 - Trigger 'sync' event.", 2, function() {
-    var collection = new Backbone.Collection([], {
-      model: Backbone.Model.extend({
-        sync: function(method, model, options) {
-          options.success();
-        }
-      })
-    });
-    collection.sync = function(method, model, options) { options.success(); };
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
     collection.on('sync', function() { ok(true); });
+    Backbone.ajax = function(settings){ settings.success(); };
     collection.fetch();
     collection.create({id: 1});
   });

test/model.js

@@ -776,24 +776,6 @@ $(document).ready(function() {
     model.set({a: true});
   });
 
-  test("Backbone.wrapError triggers `'error'`", 18, function() {
-    var resp = {};
-    var options = {};
-    var model = new Backbone.Model();
-    model.on('error', error);
-    var callback = Backbone.wrapError(null, model, options);
-    callback(model, resp);
-    callback(resp);
-    callback = Backbone.wrapError(error, model, options);
-    callback(model, resp);
-    callback(resp);
-    function error(_model, _resp, _options) {
-      ok(model === _model);
-      ok(resp === _resp);
-      ok(options === _options);
-    }
-  });
-
   test("#1179 - isValid returns true in the absence of validate.", 1, function() {
     var model = new Backbone.Model();
     model.validate = null;
@@ -831,8 +813,8 @@ $(document).ready(function() {
 
   test("#1412 - Trigger 'sync' event.", 3, function() {
     var model = new Backbone.Model({id: 1});
-    model.sync = function(method, model, options) { options.success(); };
-    model.on('sync', function() { ok(true); });
+    model.on('sync', function(){ ok(true); });
+    Backbone.ajax = function(settings){ settings.success(); };
     model.fetch();
     model.save();
     model.destroy();