Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds documentation for overriding default serializer based on conditions #1730

Merged
merged 1 commit into from
May 17, 2016
Merged

Adds documentation for overriding default serializer based on conditions #1730

merged 1 commit into from
May 17, 2016

Conversation

cgmckeever
Copy link
Contributor

Purpose

Conversation on 'dynamic serializers' spawned an interesting solution via @groyoh
Just documenting it here as its a 💯 #protip

Changes

Updated serializer docs

Related GitHub issues

#1719

## Overriding default serializer

You can use the `serializer_for` method to return a different serializer based on
use/scope specific conditions.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wording welcome!

@groyoh
Copy link
Member

groyoh commented May 17, 2016

@cgmckeever thanks for this PR! 💯

end

# the rest of the serializer
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@groyoh much better suggestion - updated

@NullVoxPopuli
Copy link
Contributor

This is super handy to document!

@NullVoxPopuli
Copy link
Contributor

LGTM

@@ -19,6 +19,7 @@ Fixes:

Misc:
- [#1673](https://github.com/rails-api/active_model_serializers/pull/1673) Adds "How to" guide on using AMS with POROs (@DrSayre)
- [#1730](https://github.com/rails-api/active_model_serializers/pull/1730) Adds documentation for overriding default serializer based on conditions (@groyoh/@cgmckeever)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wooooo

@NullVoxPopuli NullVoxPopuli merged commit ec15fa9 into rails-api:master May 17, 2016
@KevinColemanInc
Copy link

KevinColemanInc commented Jun 1, 2016

edit: the issue is we are on v0.8.3. oops

@groyoh hey! so I don't think this documentation is correct. I tried implementing this and it doesn't seem to have the desired effect.

class MySerializer < ActiveModel::Serializer
  def self.serializer_for(model, options)

Override the serializer_for method on MySerializer doesn't do anything, because the code always uses the definition of serializer_for in ActiveModel::Serializer not whatever model itself is referencing.

@serializer ||= ActiveModel::Serializer.serializer_for(resource)

So I think the way it is currently structured, it is impossible to have custom serializers for polymorphic associations in a given parent serializer.

edit: I spoke too soon... so I guess it should be using the calling class's definition of serializer_for:

serializer_class = subject.class.serializer_for(association_value, reflection_options)

but I am still not able to get the described behavior...

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