Skip to content

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Sep 25, 2013

When using embed :ids, include: true, and the model class being embedded does
not implement active_model_serializer, then a default serialization should
happen.

The else branch in ActiveModel::Serializer::Association#find_serializable did
not yet have a test case, and would cause the following error :

 NoMethodError: undefined method `object' for #<Model:0x123>
    active_model_serializers/lib/active_model/serializer.rb:411:in `block in merge_association'

This is because merge_association calls find_serializable, and expects an
object that responds to #object and #serializable_hash, which is not the
case for the model instance itself. This commit wraps the object in a
DefaultSerializer so it satisfies the interface the caller expects.

…specified

When using `embed :ids, include: true`, and the model class being embedded does
not implement `active_model_serializer`, then a default serialization should
happen.

The else branch in `ActiveModel::Serializer::Association#find_serializable` did
not yet have a test case, and would cause the following error :

   NoMethodError: undefined method `object' for #<Model:0x123>
      active_model_serializers/lib/active_model/serializer.rb:411:in `block in merge_association'

This is because `merge_association` calls `find_serializable`, and expects an
object that responds to `#object` and `#serializable_hash`, which is not the
case for the model instance itself. This commit wraps the object in a
DefaultSerializer so it satisfies the interface the caller expects.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) when pulling 6e81363 on plexus:default_serializer_for_embed_include into 919bb38 on rails-api:master.

@agrobbin
Copy link

I ran into this problem as well when attempting to serialize an Array of Structs (spoofing a has_many relationship). Would be great to get this merged!

@spastorino
Copy link
Contributor

Current master does exactly that. Check it out and if you need something else, just let me know.
Thank for working on this anyway.

@spastorino spastorino closed this Oct 22, 2013
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.

4 participants