Extract embedded belongsTo relationships in DS.EmbeddedRecordsMixin

[emk]

DS.EmbeddedRecordsMixin handles embedded hasMany relationships, but not embedded belongsTo relationships. This patch attempts implements nested belongsTo extraction, with full unit tests, but not the corresponding serialization primitives.

Use case: I'm dealing with a project that has an array of values, each of which contains an embedded array, each value in which contains another embedded belongsTo value, which in turns contains another embedded array. DS.EmbeddedRecordsMixin almost handles this, but it breaks on the embedded belongsTo value.

It would be nice to implement the corresponding serialization support, for the sake of completeness, but I don't know whether anybody would use it.

Please let me know if I can improve this patch in any way. Thank you for all your work on Ember Data!

[alexspeller]

👍 this would be super useful, I was about to submit a similar PR myself

[pixelhandler]

@alexspeller @emk I have a fork of DS.EmbeddedRecordsMixin which was the result of adding support for belongsTo relationships and sat in a PR #1516 for about a couple months. See: https://github.com/pixelhandler/ember-data-extensions

[emk]

@pixelhandler: Oh, nice!

For those who aren't familiar with DS.EmbeddedRecordsMixin, it actually handles multiply-embedded records quite well, except for those that use has_one in ActiveModel::Serializers. Once this piece is in place, everything appears to work nicely.

If @pixelhandler has matching serialization code, then we might want to go ahead and use his version. But it's definitely worth fixing has_one/belongsTo—it's pretty easy to do, and this code seems to do pretty much everything you might want once this one issue is fixed.

If anybody needs use cases, documentation, etc., please let me know. But I think this is worth fixing.

[pixelhandler]

@bradleypriest do you think that DS.EmbeddedRecordsMixin will continue to be enhanced in the activemodel-adapter package?

[bradleypriest]

@pixelhandler I'd like to get the DS.ActiveModelAdapter/Serializer and DS.EmbeddedRecordsMixin supporting everything that the default rails ActiveModelSerializer gem can output.

[bradleypriest]

I think Mongo stuff will probably stay in separate Serializer/Mixin's for the meantime if that's what you're asking 😉

[pixelhandler]

@bradleypriest @emk @alexspeller I'm using mongoDB, so I embed objects using the (Rails) model, not always in an instance of ActiveModel::Serializer.

Can someone tell me if ActiveModel:Serializers can do something like has_one :author, embed: :objects?

I've used embeds_one in my model's but that comes with using Mongoid. I needed more support for using belongsTo than Ember Data's DS.EmbeddedRecordsMixin provided. That is when I created PR #1516 to support belongsTo in the mixin. I've forked the mixin and have been using that.

I am not sure if DS.EmbeddedRecordsMixin is missing support for belongsTo that will eventually be supported, or if DS.EmbeddedRecordsMixin is finished and completely meets the functionality of using the active_model_serializers gem. If the mixin will need additional support for using embeded belongsTo (objects), then I have a working fork and would like to contribute that support to the DS.EmbeddedRecordsMixin.

In my opinion if ActiveModel::Serializers allows something like has_one :author, embed: :objects then the current iteration of DS.EmbeddedRecordsMixin should be extended to support the embedded object for a model using a belongsTo relationship.

I just tried out using embed: :objectsand it did work when using has_one in an instance of ActiveModel::Serializers. So If you all agree that AMS does support embedded objects using has_one then the Ember ActiveModelSerializer should also support that. If that is that case then I'd propose a new PR w/ both the test in this PR #1633 and the mixin from https://github.com/pixelhandler/ember-data-extensions ( an iteration after PR #1516 )

This is the proposal I made for supporting belongsTo using has_one in a serializer: http://discuss.emberjs.com/t/extend-ds-activemodelserializer-support-for-embedded-objects-belongsto-relationship-using-has-one

[emk]

Yes, you can do has_one :author, embed: :objects with ActiveModel::Serializers:

https://github.com/rails-api/active_model_serializers/blob/master/test/unit/active_model/serializer/has_one_test.rb#L91

Note that if you want this to work with Ember Data, you'll need to also use attributes :id on all the models involved.

Here's how DS.EmbeddedRecordsMixin works:

1. Given the original JSON, it walks the entire embed hierarchy, flattening everything as needed.
2. It looks up serializers for each relation, and uses the appropriate primaryKey and attrs information for each type.
3. Once everything is flattened, Ember Data runs the individual normalizers for each type.

The problem occurs in step (1): DS.EmbeddedRecordsMixin walks the entire embed hierarchy in one pass, which means it needs to know about both has_many and has_one embeds, or else it can't flatten everything appropriately. Perhaps in an ideal world. step (1) would rely on each model unpacking its own embeds, so that you could override the behavior at one level in the tree. But that's not how it works.

Anyway, with a patch like this, I think that DS.EmbeddedRecordsMixin will extract anything I've ever seen generated by ActiveModel::Serializers. And please feel free to use my unit test with somebody else's code, of course.

Anyway, please let me know if I can help in any way here. Thank you for your consideration!

[pixelhandler]

@emk I updated pixelhandler/ember-data-extensions with your test and added the support for nested objects (belongsTo / has_one).

@bradleypriest if you think support for belongsTo is needed in DS.EmbeddedRecordsMixin I can open a new PR.

[bradleypriest]

Hi @pixelhandler, if it's handled by the AMS gem then yes, definitely.

I'm just about to start adding integration tests here https://github.com/bradleypriest/ams_integration_app

[pixelhandler]

@bradleypriest I opened PR #1637 which is based on PR #1516.

[radar]

I just came across this issue in my app. It was returning undefined for my association, which is definitely not useful. What's the status on this PR?

[bakura10]

Any idea of when can this be merged? :) Thanks!

[radar]

@bakura10 @bradleypriest said this on Twitter: https://twitter.com/bradleypriest/status/453438310254272512

[bakura10]

Thanks, that's good to hear :).

[pixelhandler]

@emk I've updated #1637 after pairing w/ @igorT addressing this issue. If you're still using embedded records would you mind looking at some of the changes I'm wondering how others are using foreign keys with embedded records.

[igorT]

We merged #1637 so closing this