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

Empty collection root key from explicit serializer option #1618

Merged
merged 2 commits into from
Mar 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
Breaking changes:

Features:
- [#1618](https://github.com/rails-api/active_model_serializers/issues/1618) Get collection root key for
empty collection from explicit serializer option, when possible. (@bf4)
- [#1574](https://github.com/rails-api/active_model_serializers/pull/1574) Provide key translation. (@remear)
- [#1494](https://github.com/rails-api/active_model_serializers/pull/1494) Make serializers serializalbe
(using the Attributes adapter by default). (@bf4)
Expand Down
2 changes: 1 addition & 1 deletion lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def as_json(adapter_opts = nil)

# Used by adapter as resource root.
def json_key
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it now, shouldn't it be the responsibility of the adapter to determine the json key/root?

Copy link
Member Author

Choose a reason for hiding this comment

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

it does, but it uses information from the serializer....

Copy link
Member

Choose a reason for hiding this comment

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

From what I see, we can set the root as adapter option and call serializer._type and serializer.object.class.model_name.to_s.underscore from the adapter. Same goes for collection_serializer. What do you think?

root || object.class.model_name.to_s.underscore
root || _type || object.class.model_name.to_s.underscore
Copy link
Member Author

Choose a reason for hiding this comment

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

not required in this PR, but makes sense

end

def read_attribute_for_serialization(attr)
Expand Down
37 changes: 25 additions & 12 deletions lib/active_model/serializer/collection_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ class CollectionSerializer
attr_reader :object, :root

def initialize(resources, options = {})
@root = options[:root]
@object = resources
@object = resources
@options = options
@root = options[:root]
serializer_context_class = options.fetch(:serializer_context_class, ActiveModel::Serializer)
@serializers = resources.map do |resource|
serializer_context_class = options.fetch(:serializer_context_class, ActiveModel::Serializer)
serializer_class = options.fetch(:serializer) { serializer_context_class.serializer_for(resource) }

if serializer_class.nil? # rubocop:disable Style/GuardClause
Expand All @@ -26,9 +27,28 @@ def success?
true
end

# TODO: unify naming of root, json_key, and _type. Right now, a serializer's
# json_key comes from the root option or the object's model name, by default.
# But, if a dev defines a custom `json_key` method with an explicit value,
# we have no simple way to know that it is safe to call that instance method.
# (which is really a class property at this point, anyhow).
# rubocop:disable Metrics/CyclomaticComplexity
# Disabling cop since it's good to highlight the complexity of this method by
# including all the logic right here.
def json_key
root || derived_root
return root if root
# 1. get from options[:serializer] for empty resource collection
key = object.empty? &&
(explicit_serializer_class = options[:serializer]) &&
explicit_serializer_class._type
# 2. get from first serializer instance in collection
key ||= (serializer = serializers.first) && serializer.json_key
# 3. get from collection name, if a named collection
key ||= object.respond_to?(:name) ? object.name && object.name.underscore : nil
# 4. key may be nil for empty collection and no serializer option
key && key.pluralize
end
# rubocop:enable Metrics/CyclomaticComplexity

def paginated?
object.respond_to?(:current_page) &&
Expand All @@ -38,14 +58,7 @@ def paginated?

protected

attr_reader :serializers

private

def derived_root
key = serializers.first.try(:json_key) || object.try(:name).try(:underscore)
key.try(:pluralize)
end
attr_reader :serializers, :options
end
end
end
10 changes: 10 additions & 0 deletions test/collection_serializer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
module ActiveModel
class Serializer
class CollectionSerializerTest < ActiveSupport::TestCase
MessagesSerializer = Class.new(ActiveModel::Serializer) do
Copy link
Member

Choose a reason for hiding this comment

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

We should align on Class.new(ActiveModel::Serializer) vs class Name; end.

type 'messages'
end

def setup
@comment = Comment.new
@post = Post.new
Expand Down Expand Up @@ -84,6 +88,12 @@ def test_json_key_with_resource_without_name_and_no_serializers
assert_nil serializer.json_key
end

def test_json_key_with_empty_resources_with_serializer
resource = []
serializer = collection_serializer.new(resource, serializer: MessagesSerializer)
assert_equal 'messages', serializer.json_key
end

def test_json_key_with_root
expected = 'custom_root'
serializer = collection_serializer.new(@resource, root: expected)
Expand Down