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

field :fields and def fields=(fields) mixing up #15

Closed
minsikzzang opened this issue Aug 15, 2017 · 0 comments
Closed

field :fields and def fields=(fields) mixing up #15

minsikzzang opened this issue Aug 15, 2017 · 0 comments

Comments

@minsikzzang
Copy link

minsikzzang commented Aug 15, 2017

Issue

I've found issue in https://github.com/facebook/facebook-ruby-ads-sdk/blob/master/lib/facebook_ads/ad_object.rb#L48 when using insights creation api.

def initialize(attributes, *args)
   ...
   self.fields = fields + attributes.keys
   ...
end

above self.fields is usually calling def fields=(fields) below when there are no field: fields declared.

https://github.com/facebook/facebook-ruby-ads-sdk/blob/master/lib/facebook_ads/ad_object.rb#L96

But there is one place field: fields declared found below

https://github.com/facebook/facebook-ruby-ads-sdk/blob/master/lib/facebook_ads/ad_objects/ad_report_run.rb#L52

so when it goes to

self.fields = fields + attributes.keys

it tries to access below method with name=fields instead of calling instance method fields=

 def define_writer(name)
    define_method("#{name}=") do |val|
       changes[name] = val
       @fields.add(name.to_s)
    end
end

How to replicate this issue

FacebookAds::AdAccount.get('act_xxxxxx').insights(fields: 'ad_report_run').create(level: 'ad', fields: ['ad_id'])

This will throw exception
NoMethodError: undefined method `add' for nil:NilClass

Suggestion to fix

Just rename instance method def fields=(fields) to either def _fields=(fields) or more explicitly def response_fields=(fields)

@minsikzzang minsikzzang changed the title field :fields and def fields=(fields) mixing up field :fields and def fields=(fields)\ mixing up Aug 16, 2017
@minsikzzang minsikzzang changed the title field :fields and def fields=(fields)\ mixing up field :fields and def fields=(fields) mixing up Aug 16, 2017
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

No branches or pull requests

1 participant