Skip to content

Conversation

uorbe001
Copy link

This adds support for has_many polymorphic associations to active_model_serializers.

For an Attachment model with many Attachable models associated, where an Email extends the Attachable model, the output would be something like this:

{
  "attachment":  {
     "name": 'logo.png',
      "url": 'http://example.com/logo.png',
      "attachable_ids": [{
         "type": "email",
          "id": 1
       }]
    },
    "emails": [{
        "id": 1,
        "subject":  "foo",
        "body": "bar"
    }]
}

@coveralls
Copy link

Coverage Status

Coverage increased (+97.4%) when pulling 841692f on projectenthuse:feature/add-has-many-polyorphic-support into 8ac4bf9 on rails-api:master.

@mikegee
Copy link
Contributor

mikegee commented Aug 25, 2013

Looks reasonable to me. Anybody have concerns about this?

@jonah-williams
Copy link

I'm biased but I prefer #367. A polymorphic has_one uses the name of the association and does not append the _id suffix. I would expect a polymorphic has_many to be similar and use the plural association name also without the _ids suffix.

In addition to being consistent I've found it convenient to be able to write clients which can work with association names as provided by the API without needing to strip _id from the given name when re-establishing associations between objects.

@matthewrudy
Copy link

@uorbe001 it seems that Ember Data doesn't use _ids for polymorphic cases.

One of their tests;

env.store.push('user', { id: 1, messages: [ {id: 1, type: 'post'}, {id: 3, type: 'comment'} ] });
env.store.push('post', { id: 1 });
env.store.push('comment', { id: 3 });

@cyril-sf
Copy link

@matthewrudy What you show in that test is the normalized version of the payload, not what is expected from the server. I believe it comes from the RESTAdapter and the Rails default.

Not appending _ids as suggests @jonah-carbonfive makes it straight forward, but I like the _ids version as it helps to understand that the representation isn't necessarily the object itself and that it requires some work to be used.

@matthewrudy
Copy link

my point was more so that @uorbe001 was trying to target Ember-data, and ember-data now expects associations with no _ids.

EmberData now has an adapter for activemodelserializers that does the conversion.

To be consistent with the existing format it should keep the _ids.

Ongoing I expect it'll all be jsonapi anyway.

@spastorino
Copy link
Contributor

Polymorphic associations are not any longer on master and we would need to reimplement them before releasing 0.9. If some of you want to take a look at that, please go ahead. Feel free to ping me on twitter also to get some help. I'm closing this for now. Thanks for doing this work anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants