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

Having attribute :name in a serializer causes exceptions #1094

Conversation

whatthewhat
Copy link
Contributor

Having attribute :name in a serializer breaks with NoMethodError: undefined method 'name' for #<AuthorSerializer:0x007ffe5afe6810>.
But attributes :id, :name works..

I've changed one of the test serializers to demonstrate the issue.

I'll try to find time later to investigate some more, unless there is not an obvious fix someone could point me to.

@beauby
Copy link
Contributor

beauby commented Aug 28, 2015

Could you state the version and adapter that lead to the error?

@whatthewhat
Copy link
Contributor Author

@beauby should be clear from the tests when (if) they fail.

The error reproduces on master, the serializer is super simple:

class CompanySerializer < ActiveModel::Serializer
  attribute :id
  attribute :name
end

@beauby
Copy link
Contributor

beauby commented Aug 28, 2015

Sure, but which adapter? (json, flatten_json, json_api?)

@whatthewhat
Copy link
Contributor Author

@beauby oh, sorry :) json

@whatthewhat
Copy link
Contributor Author

Looking at the test failures seems like it's universal for all adapters

@beauby
Copy link
Contributor

beauby commented Aug 28, 2015

Investigating!

@beauby
Copy link
Contributor

beauby commented Aug 28, 2015

Found the cause, but no fix yet.
In attributes we call method_defined?(attr), and in attribute we call respond_to?(key, false). I'm not sure about the rationale behind it, but replacing respond_to?(key, false) with method_defined?(key) messes with the ability of overriding the id attribute.
@joaomdmoura @bf4 ideas?

@beauby
Copy link
Contributor

beauby commented Aug 28, 2015

Hmm actually it's no surprise overriding id fails in that case because of this. @joaomdmoura what's the reason for this id method?

@kayhide
Copy link

kayhide commented Aug 28, 2015

#method_defined? is for a class and looks for a instance method.
#respond_to? is for a object and looks for a method of the receiver object.

So this returns whether the class method exists or not.
This does not check instance methods.

In this case, we are interested in instance methods, so #method_defined? is good to do that.

@beauby
Copy link
Contributor

beauby commented Aug 29, 2015

Thanks a lot for the clarification @kayhide!

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.

3 participants