getoutreach / epf

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

Gotcha / inconsistency for model injection #95

Closed heartsentwined closed 10 years ago

heartsentwined commented 10 years ago

18 describes the current canonical / hacky way of injecting models into epf:

adapter.didReceiveDataForFind(json, App.Foo)

Here, unlike the session.* family of methods, the second argument must be a model class, passing 'foo' will not work.

adapter.didReceiveDataForFind(json, 'foo') // fails silently

Then again this is probably not meant to be a public API. I'm still reporting this for current googlers, and as a note for designing the public API.

ghempton commented 10 years ago

I want to add an API for loading data directly from JSON. What do you think about:

adapter.merge 'foo', json

or if you are merging an entire payload (e.g. something from active model serializers that has multiple types and root keys):

adapter.mergePayload json

The above mergePayload method would be specific to the rest adapter.

Of course session already has a merge method that only takes instantiated models. The implicit line in the sand here is that the adapter would provide API's that deal with raw data and the session would still deal entirely with instantiated models.

Thoughts?

heartsentwined commented 10 years ago

I wonder if there is even any need to separate the hooks.

adapter.merge 'foo', json

should expect

'{ foos: [{"id":1, "value":"a"}, {"id":2, "value":"b"}] }'
# or 
'{ foo: [{"id":1, "value":"a"}] }'

The root key should still be necessary, because foo might want to sideload bar - in that case the payload will look like

'{ 
  foos: [{"id":1, "value":"a", "bar_id": 1}, {"id":2, "value":"b"}],
  bars: [{"id":1, "value":"c", "foo_id": 1}] 
}'

The other method

adapter.mergePayload json

should expect

'{ 
  foos: [{"id":1, "value":"a"}, {"id":2, "value":"b"}],
  bars: [{"id":1, "value":"c"}, {"id":2, "value":"d"}] 
}'

... which is basically the same as the second case above.

Either epf can figure out the foos and bars from the full payload by itself (in which case we can omit the type argument and just have adapter.merge json), or the "load-all" api is ambiguous and impossible (e.g. name clashes between relationship names and top-level model names?). I'm not familiar enough with the internal workings of epf to say which is the correct interpretation, but that's my thoughts.

ghempton commented 10 years ago

I have finally added an api for this. On the session you can call session.mergeData hash, type to accomplish this.