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.

Sign up for GitHub

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

Jump to bottom

"api is not defined as an attribute on" warnings on enumeration #188

Closed
timdorr opened this issue Jul 27, 2018 · 5 comments · Fixed by #187
Closed

"api is not defined as an attribute on" warnings on enumeration #188

timdorr opened this issue Jul 27, 2018 · 5 comments · Fixed by #187

Comments

@timdorr
Copy link

timdorr commented Jul 27, 2018

I'm seeing errors like this when using the .each iterator on any Collection object:

W, [2018-07-27T13:56:02.919881 #31343]  WARN -- : api is not defined as an attribute on Nylas::Message

And just to confirm that I'm not totally crazy, it looks like you are too: https://travis-ci.org/nylas/nylas-ruby/jobs/406871743

The source of the issue is merging in the api instance to the model initialize method:

nylas-ruby/lib/nylas/collection.rb

Line 65 in b8623d4

yield(model.new(result.merge(api: api)))

During the model initialize method in the Attributable decorator, it checks for that field as an attribute definition:

nylas-ruby/lib/nylas/model/attributable.rb

Lines 9 to 17 in b8623d4

def initialize(**initial_data)
initial_data.each do |attribute_name, value|
if self.class.attribute_definitions.key?(attribute_name)
send(:"#{attribute_name}=", value)
else
Logging.logger.warn("#{attribute_name} is not defined as an attribute on #{self.class.name}")
end
end
end

While that's an attr_accessor on Model, it's not an attribute class definition and fails that check.

I'm not sure what the best course of action would be here (hence the issue instead of PR). You could whitelist that attribute, define it on the Model as a default for all models, or not merge it and pass it in some other way. I'm not sure what's best, but if you have some opinion about it, I can put together a PR.
@zspencer
Copy link
Contributor

Good catch @timdorr! Maybe instead of checking attribute_definitions.key? we should be checking respond_to?(:"#{key}="). I've got a PR open that I'm resolving today that I can add that into as well.

@zspencer zspencer mentioned this issue Jul 28, 2018
Merged
zspencer added a commit that referenced this issue Jul 28, 2018
@zspencer
API no longer gets dropped when iterating over a collection
fe0e9e9 
Resolves #188
zspencer added a commit that referenced this issue Jul 28, 2018
@zspencer
API no longer gets dropped when iterating over a collection
4894e1e 
Resolves #188

* Tests will fail if model initialize discards API again
@HayleyCAnderson
Copy link
Contributor

Thanks for this excellent bug report @timdorr! I merged the PR from @zspencer and released 4.2.3 which should fix the issue.

@timdorr
Copy link
Author

timdorr commented Aug 7, 2018

@HayleyCAnderson It looks like 4.2.3 was published prior to that fix going in: https://rubygems.org/gems/nylas/versions

Looks like someone bumped the version without committing/pushing the change back to this repo. Not sure what you want to do, but it looks like a 4.2.4 release would be warranted.

@HayleyCAnderson
Copy link
Contributor

Wow, thanks for catching that. Sorry about that; just put up 4.2.4 https://rubygems.org/gems/nylas/versions/4.2.4

@timdorr
Copy link
Author

timdorr commented Aug 7, 2018

No worries. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Finding a model works when new fields are added to the API nylas/nylas-ruby
3 participants
@timdorr @zspencer @HayleyCAnderson