Get modelName from collection when collection uses BaseModel

[shebson]

When model.constructor.id isn't set (such as when a collection uses generic BaseModel), get modelName from model.collection.constructor.id.

Fixes #151

[lo1tuma]

• The tests failed on Travis CI
• You should write/adapt a test for the new behavior ;)

[lo1tuma]

Setting an id on the BaseModel should solve this problem too.

[lo1tuma]

This line is way too long and has to much logic in it which makes it error-prone and realy hard to understand.

[lo1tuma]

Do you mean modelOrCollectionClass.collection instead of this.collection?

[lo1tuma]

If I understand this correctly then the modelName method can return the underscorized id of the collection from a model. How do you reference the BaseModel with the underscorized collectionId?

[shebson]

Yes, you understand this correctly. If I understand #151 correctly, this is Spike's proposed solution to ModelStore#set breaking on collections that use the generic BaseModel.

I'm not sure I totally understand your question about how to reference BaseModel with the underscorized collectionId, so let me know if this is unclear or doesn't answer your question.

My understanding is that modelName and id are serialized to form a key in the local ModelStore. For example, if you have a model whose constructor id is "book" with id "8", the key is book:8.

But if you have a collection "Books" that doesn't specify a model, each model does not currently have a modelName. So instead of book:8 you would now have a key of simply :8, which could be the same key as another collection. It also seriously breaks on IE, where instead of getting a key of :8, it throws an error.

With this change, we get the modelName from the collection constructor if model.constructor.id isn't set. So in the example above, the key would be books:8.

The end result is that the ModelStore and fetcher can handle collections that don't specify a model.

That said, it's still failing Travis. I'll submit a fix this weekend.

[shebson]

After looking into this more, I think attempting to fix this behavior by making changes to the .modelName method isn't the best approach. I'm going to close this pull request for now and submit a new one if I make headway with a different approach. Thanks for your help @lo1tuma !

[lo1tuma]

According to your example where modelName would return books:8 for a model. books is still the name of a collection and not the model. This might be work for the ModelStore but would break something else. For example you try to load a model class based on the result of modelName.

I think the problem should be fixed in ModelStore itself. Using the name of a model as a key is probably not the best solution.

BTW: We should also try to refactor out all the collection stuff from modelUtils. This is the most confusing code part in rendr.