-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Controller namespace lookup. #1436
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
Conversation
Can we discuss other impls? I like the 0.9 one |
Note also that #1196 solves a different problem. |
@@ -56,7 +56,7 @@ def use_adapter? | |||
|
|||
[:_render_option_json, :_render_with_renderer_json].each do |renderer_method| | |||
define_method renderer_method do |resource, options| | |||
options.fetch(:serialization_context) { options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(request) } | |||
options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(controller: self) unless options.key?(:serialization_context) |
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'd like to see this broken up in to multiple lines
unless options.key?(:serialization_context)
options[:serialization_context] = ActiveModelSerializers::SerializationContext.new(controller: self)
end
serializer = ResourceNamespace::PostSerializer.serializer_for(comment) | ||
assert_equal ResourceNamespace::PostSerializer::CommentSerializer, serializer | ||
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.
This test did not make sense anymore with the new lookup convention.
Any progress here? |
Closing this in favor of RFC #1442. |
Just took a stab at it. Basically, it allows for inferring the serializer in the following scenario:
Ref #1265, #886.