Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixes #3028 assign collection in _prepareModel #3036

Merged
merged 1 commit into from

4 participants

@tgriesser
Collaborator

So looking at it again, the _addReference is more about events and adding to the internal reference hashes than the collection property, so I still think this is alright... it reverts to the previous behavior for this case, but also removes model.collection in cases of failure with wait:true.

Let me know if you disagree on that second piece, but I would assume that attempting to add a model to a collection shouldn't set the "collection" attribute on the model if it's not actually added.

@lfac-pt

Awesome! :+1:

@jashkenas
Owner

@braddunbar Any thoughts on this?

@braddunbar
Collaborator

Moving collection assignment back to _prepareModel seems good to me. I'm not sure about the error handler for {wait: true} though. That behavior has been around for some time without any issues reported and as far as I can tell #3028 doesn't mention it.

@tgriesser
Collaborator

Fair enough, ditched that part of it.

@braddunbar braddunbar merged commit 28ac874 into from
@braddunbar
Collaborator

Thanks @tgriesser!

@tgriesser tgriesser deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 18, 2014
  1. @tgriesser
This page is out of date. Refresh to see the latest.
Showing with 14 additions and 2 deletions.
  1. +4 −2 backbone.js
  2. +10 −0 test/collection.js
View
6 backbone.js
@@ -939,7 +939,10 @@
// Prepare a hash of attributes (or other model) to be added to this
// collection.
_prepareModel: function(attrs, options) {
- if (attrs instanceof Model) return attrs;
+ if (attrs instanceof Model) {
+ if (!attrs.collection) attrs.collection = this;
+ return attrs;
+ }
options = options ? _.clone(options) : {};
options.collection = this;
var model = new this.model(attrs, options);
@@ -952,7 +955,6 @@
_addReference: function(model, options) {
this._byId[model.cid] = model;
if (model.id != null) this._byId[model.id] = model;
- if (!model.collection) model.collection = this;
model.on('all', this._onModelEvent, this);
},
View
10 test/collection.js
@@ -1416,4 +1416,14 @@
ok(collection.at(1) instanceof B);
equal(collection.at(1).id, 'b-1');
});
+
+ test("create with wait, model instance, #3028", 1, function() {
+ var collection = new Backbone.Collection();
+ var model = new Backbone.Model({id: 1});
+ model.sync = function(){
+ equal(this.collection, collection);
+ };
+ collection.create(model, {wait: true});
+ });
+
})();
Something went wrong with that request. Please try again.