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

[FIX] Fetch json key from item serializer if empty collection is pass… #1537

Closed
wants to merge 1 commit into from

Conversation

RomanKapitonov
Copy link
Contributor

…ed to collection serializer and each_searializer is specified.

@RomanKapitonov
Copy link
Contributor Author

Fixes #1536

…ed to collection serializer and each_searializer is specified.
def json_key
'messages'
end
end
Copy link
Member

Choose a reason for hiding this comment

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

would you mind moving this to the test file? poro's is a little cluttered

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -39,8 +45,15 @@ def paginated?
private

def derived_root
key = serializers.first.try(:json_key) || object.try(:name).try(:underscore)
key.try(:pluralize)
serializers.first.try(:json_key).try(:pluralize)
Copy link
Member

Choose a reason for hiding this comment

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

aside: we really need to replace Serializer#json_key with Serializer._type...

Copy link
Member

Choose a reason for hiding this comment

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

Here what about:

def derived_root
  return unless serializers.empty?
  serializers.first.json_key.to_s.pluralize
end

Makes it more readable IMO.

@bf4
Copy link
Member

bf4 commented Mar 7, 2016

@RomanKapitonov are you planning on coming back to this?

@NullVoxPopuli
Copy link
Contributor

@RomanKapitonov are you planning on helping resolve the PR Review comments? :-\

@bf4
Copy link
Member

bf4 commented Mar 25, 2016

Followed up with #1618

@groyoh groyoh closed this in #1618 Mar 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants