getoutreach / epf

A framework for keeping your Ember.js apps in sync.
http://epf.io
MIT License
369 stars 33 forks source link

Serialize hasmany as ids by default #91

Closed joostdevries closed 10 years ago

joostdevries commented 10 years ago

Changed the default serializer behavior to serialize a hasMany to an array of ids by default. This is definitely not a "must-merge" PR but I want to look into the option of at least providing a config option in the relation setup. IMHO is you define a relation to be bi-directional, it's unexpected behavior if the persistence lib assumes your API to be one-directional.

What do you think?

ghempton commented 10 years ago

I would like to provide a config option for this. Currently there is an adapter-level configuration setting for the "owner" of a relationship (e.g. in a 1-1 this would be the side of the relationship that actually controls the foreign key). It might make sense to use this property in this case?

joostdevries commented 10 years ago

Using owner sounds like a reasonable option although I think a serialize flag on the relation might be even more clear. I've been playing around with this and it's definitely something that requires more changes than I've put in this PR. For example many-to-many relations would are become possible to setup with this config but serializing them correctly would definitely mean one of the tho models needs to be the "owner" (otherwise, updating a many-many would dirty two models at the same time and cause two request).

An even more interesting case would be a hasMany to the same model. In this case I guess the owner flag needs to be set on the model/relation instance.

Maybe it would be even better to use two layers: One in the relationship config which lets you decide whether or not a relationship should be serialized on that specific model (this flag would be true by default on belongsTo). The second layer would be a check during the "dirtying" of a relationship to make sure the same relationship isn't flagged as dirty twice.

Let me know what you think.

joostdevries commented 10 years ago

I've provided a more high-level view in #102

ghempton commented 10 years ago

As noted in the related issue, this PR is now outdated and custom serializers for relationships can be defined.