diff --git a/backbone.js b/backbone.js index c07a0be86..eb589cbe0 100644 --- a/backbone.js +++ b/backbone.js @@ -1084,9 +1084,23 @@ var collection = this; var success = options.success; options.success = function(m, resp, callbackOpts) { - if (wait) collection.add(m, callbackOpts); + if (wait) { + m.off('error', this._forwardPristineError, this); + collection.add(m, callbackOpts); + } if (success) success.call(callbackOpts.context, m, resp, callbackOpts); }; + // In case of wait:true, our collection is not listening to any + // of the model's events yet, so it will not forward the error + // event. In this special case, we need to listen for it + // separately and handle the event just once. + // (The reason we don't need to do this for the sync event is + // in the success handler above: we add the model first, which + // causes the collection to listen, and then invoke the callback + // that triggers the event.) + if (wait) { + model.once('error', this._forwardPristineError, this); + } model.save(null, options); return model; }, @@ -1224,8 +1238,19 @@ } } this.trigger.apply(this, arguments); - } + }, + // Internal callback method used in `create`. It serves as a + // stand-in for the `_onModelEvent` method, which is not yet bound + // during the `wait` period of the `create` call. We still want to + // forward any `'error'` event at the end of the `wait` period, + // hence a customized callback. + _forwardPristineError: function(model, collection, options) { + // Prevent double forward if the model was already in the + // collection before the call to `create`. + if (this.has(model)) return; + this._onModelEvent('error', model, collection, options); + } }); // Defining an @@iterator method implements JavaScript's Iterable protocol. diff --git a/test/collection.js b/test/collection.js index d0890f81f..adf16dfe9 100644 --- a/test/collection.js +++ b/test/collection.js @@ -654,6 +654,47 @@ assert.equal(collection.length, 1); }); + QUnit.test('failing create with wait:true triggers error event (#4262)', function(assert) { + assert.expect(2); + var collection = new Backbone.Collection; + collection.url = '/test'; + collection.on('error', function() { assert.ok(true); }); + var model = collection.create({id: '1'}, {wait: true}); + model.on('error', function() { assert.ok(true); }); + this.ajaxSettings.error(); + }); + + QUnit.test('successful create with wait:true triggers success event (#4262)', function(assert) { + assert.expect(2); + var collection = new Backbone.Collection; + collection.url = '/test'; + collection.on('sync', function() { assert.ok(true); }); + var model = collection.create({id: '1'}, {wait: true}); + model.on('sync', function() { assert.ok(true); }); + this.ajaxSettings.success(); + }); + + QUnit.test('failing create pre-existing with wait:true triggers once (#4262)', function(assert) { + assert.expect(1); + var model = new Backbone.Model; + var collection = new Backbone.Collection([model]); + collection.url = '/test'; + collection.on('error', function() { assert.ok(true); }); + collection.create(model, {wait: true}); + this.ajaxSettings.error(); + }); + + QUnit.test('successful create pre-existing with wait:true preserves other error bindings (#4262)', function(assert) { + assert.expect(1); + var model = new Backbone.Model; + var collection = new Backbone.Collection([model]); + collection.url = '/test'; + model.on('error', function() { assert.ok(true); }); + collection.create(model, {wait: true}); + this.ajaxSettings.success(); + model.trigger('error'); + }); + QUnit.test('initialize', function(assert) { assert.expect(1); var Collection = Backbone.Collection.extend({ diff --git a/test/model.js b/test/model.js index b3092df7f..d5e20080a 100644 --- a/test/model.js +++ b/test/model.js @@ -712,6 +712,15 @@ this.ajaxSettings.success(); }); + QUnit.test('failing save with wait:true triggers error event (#4262)', function(assert) { + assert.expect(1); + var model = new Backbone.Model; + model.urlRoot = '/test'; + model.on('error', function() { assert.ok(true); }); + model.save({id: '1'}, {wait: true}); + this.ajaxSettings.error(); + }); + QUnit.test('fetch', function(assert) { assert.expect(2); doc.fetch();