Changed the getBootstrappedData function to add each model of a collection to the modelStore #380
Conversation
| _.each(list, function (value, key) { | ||
| if (app.modelUtils.isModel(value) || app.modelUtils.isCollection(value)) { | ||
| var tempObject = {}, | ||
| key = (key !== value.cid && isNaN(parseInt(key))) ? key : value.cid; |
There was a problem hiding this comment.
I think I see what this line is doing, but it's pretty opaque and took me a long time to figure out.
I'm reading it as... if the key is neither a number nor the model cid, use it as is. Otherwise use the cid.
So the case in which we used the cid is either the key is a number or it is the cid... So basically, in the case of a collection, where our keys are numbers, store the model at the root level of the hash using it's cid as a key. Otherwise you could see overwriting among collections.
Are there any other cases this is happening in? If not, I'd prefer it be made more explicit, something like
if (app.modelUtils.isCollection(modelOrCollection) {key = value.cid;}
There was a problem hiding this comment.
The idea here was that if you have a named field you'd want it to appear in the cache as such. Best course of action might be to change the key to be model_name:id: the same way as everything is currently cached.
…he cacheKey for the object constructor:id
|
Looks like a pretty straightforward extension of #372 for collections. 👍 |
|
This PR is no longer required because we are parsing the objects client-side, which is also setting up the cache as we would expect. |
Added each model that's part of a collection to the bootstrappedData - this way we can access them singularly without having to request the single model inside of the collection.
This also fixes an issue if there were two models with a sub-model of the same key.