active_model_serializers
active_model_serializers copied to clipboard
Namespace ignored when fetching serializer for parent class
Consider an STI case with only parent class serializers and no separate child serializers. Now throw in different namespaces.
With model's:
class Parent < ActiveRecord::Base
end
class ChildA < Parent
end
class ChildB < Parent
end
Controllers & Serializers with namespaces looks like:
app/controllers/v1/parents_controller.rb
app/serializers/v1/parents_serializer.rb
Expected behavior vs actual behavior
Now when I render child records's from my V1::ParentsController controller, no serializer is being used (ActiveModel::Serializer::Null).
I've tracked down the issue to get_serializer_for method in ActiveModel::Serializer. When recursively calling get_serializer_for for super_class. Here, the namespace needs to be passed in as well.
Environment
Rails: 5
AMS Version: 1.10-stable
Additonal helpful information
Seems to be introduced in https://github.com/rails-api/active_model_serializers/commit/b29395b0ac160c2ee28cc333467954eaafd917bc
Do I understand you're saying that the V1 namespace isn't being used?
Thanks for digging into the code.
You don't include your render code. Have you tried specifying namespace or serializer? I get it would be nice if the namespace lookup worked as expected, but is there anything which makes specifying it inconvenient?
@bf4 Yes the namespace is not respected even though the render is called from a V1 namespaced controller.
I tried specifying namespace but it's the same. But, I've been bypassing this by specifying the serializer.
Did you try the fix you suggested?
@bf4 You mean the fix in AMS itself? No, not yet.
I can try it out and make a PR with tests over the weekend.
@Charizard yeah, fork it, change it, point your gemfile to it, try it.
(Or clone it to a relative dir and point Gemfile to it, same story)
Or just patch it
related I think https://github.com/rails-api/active_model_serializers/issues/2295
@bf4 Yes, it's partly related. When the namespace is explicitly specified like so,
class ResourceNamespace
class PostSerializer < ActiveModel::Serializer
has_many :comments, namespace: ResourceNamespace
end
class CommentSerializer < ActiveModel::Serializer; end
end
then #2295 takes care of it. But, in cases where the namespace is not specified like so,
class ResourceNamespace
class PostSerializer < ActiveModel::Serializer
has_many :comments
end
class CommentSerializer < ActiveModel::Serializer; end
end
don't you think comments should implicitly pickup ResourceNamespace::CommentSerializer?
The lookup chain for this would be,
ResourceNamespace::PostSerializer::CommentSerializer<namespace>::CommentSerializer(if a namespace is specified)ResourceNamespace::CommentSerializer(new item in the lookup chain that I'm proposing) - This would use the namespace of the parent serializer to check for any compatible serializers in it.CommentSerializer
I've added a failing test here for my proposal.