-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add option to select adapter by the request header #1082
Conversation
By default, AMS responds to media type application/json and application/vnd.api+json using the FlattenJson and JsonApi adapters respectively. If you want to add a different media type, override the defaults or even create your own, you can do it in your initializer.
@joaomdmoura, I've worked on #1062. We will add more tests if you decide to move on. |
if ActiveModel::Serializer.enabled_adapters_by_media_type && !options.fetch(:adapter, nil) | ||
adapter = ActiveModel::Serializer::Adapter.by_request(request) | ||
options[:adapter] ||= adapter if adapter | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be really great to merge #1017 first to take advantage of the register/get interface cc @joaomdmoura :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check #1017
I made some comments, you might also need to rebase it. I'll check #1017 in order to see if we can get it merged and you can take advantage of it on your PR as well. |
@@ -7,6 +7,8 @@ module Configuration | |||
included do |base| | |||
base.config.array_serializer = ActiveModel::Serializer::ArraySerializer | |||
base.config.adapter = :flatten_json | |||
base.config.enabled_adapters_by_media_type = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be false by default, as the defined serializers may not play well with all possible adapters.
@akaKuruma are you still interested in this PR? A lot has changed in master, but, I think it actually makes this PR simpler. Let me know if I can help. |
@bf4 yes, I'll check what we need to change, thanks |
Yay! B mobile phone
|
+1 on this feature. I'm working on writing an AMS adapter for the Siren hypermedia type. And then I plan to create a second adapter for a flavor of Siren for connected devices called Zetta. I'd like to be able to swap among the adapters for ease of dev and test and to provide API clients more flexibility in production. Right now my controllers are not very DRY. My world looks like this today and will look much better after this PR. Thanks for the great work. Photocells Controller
Mime Type Initializer
|
I wanted to reference related work I'm proposing to Rails rails/rails#21496 (comment) Impl on master here in ams should use serialization_context |
I think we're still open to this feature but this is out of date. If anyone wants to open a new PR for this, the implementation should use |
It resolves #1062