Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger error from failing Collection.create with wait:true #4265

Merged
merged 8 commits into from
Jul 23, 2023
29 changes: 27 additions & 2 deletions backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -1080,9 +1080,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) {
jgonggrijp marked this conversation as resolved.
Show resolved Hide resolved
model.once('error', this._forwardPristineError, this);
}
model.save(null, options);
return model;
},
Expand Down Expand Up @@ -1220,8 +1234,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.
Expand Down
41 changes: 41 additions & 0 deletions test/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

jgonggrijp marked this conversation as resolved.
Show resolved Hide resolved
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({
Expand Down
9 changes: 9 additions & 0 deletions test/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading