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

Bug fix for ArraySerializer json_key #1007

Merged

Conversation

jiajiawang
Copy link
Contributor

When the resource is a zero result query,
i.e. post_comments = PostComment.where("1=0")
the json_key will become 'postcomments' rather than 'post_comments'.
Using 'underscore' instead of 'downcase' fixes the error.

When the resource is a zero result query,
i.e. post_comments = PostComment.where("1=0")
the json_key will become 'postcomments' rather than 'post_comments'.
Using 'underscore' instead of 'downcase' fixes the error.
@joaomdmoura
Copy link
Member

Awesome @jiajiawang!
Could you add a test to cover it too? So we can make sure it won't easily broke again 😄

@jiajiawang
Copy link
Contributor Author

Sure
On 16/07/2015 8:41 am, "João Moura" notifications@github.com wrote:

Awesome @jiajiawang https://github.com/jiajiawang!
Could you add a test to cover it too? So we can make sure it won't easily
broke again [image: 😄]


Reply to this email directly or view it on GitHub
#1007 (comment)
.

test json key when resource is empty
@joaomdmoura
Copy link
Member

Great! LGTM, I'm merging it.

joaomdmoura added a commit that referenced this pull request Jul 16, 2015
@joaomdmoura joaomdmoura merged commit d714094 into rails-api:master Jul 16, 2015
@jiajiawang jiajiawang deleted the array_serializer_json_key_fix branch July 16, 2015 04:27
pranavpr added a commit to Poncho-Box8/active_model_serializers that referenced this pull request Aug 11, 2015
@@ -38,6 +38,17 @@ def test_meta_and_meta_key_attr_readers
assert_equal @serializer.meta, "the meta"
assert_equal @serializer.meta_key, "the meta key"
end

def test_json_key_when_resource_is_empty
Array.class_eval do
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like it's trying to be a fake AR Relation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants