mailjet / mailjet-gem

[API v3] Mailjet official Ruby GEM
https://dev.mailjet.com
Other
130 stars 72 forks source link

Resource instances don't hit API? #96

Closed excid3 closed 7 years ago

excid3 commented 7 years ago

Was doing some debugging today and noticed that resource instance methods like update_attributes don't hit the API. Since instance methods don't call the change_resource_path like the class methods do, they don't get the correct endpoint, version, and so on.

# class method, correctly hits API
c = Mailjet::Contactdata.find("name@address.com") 

# instance method, immediately returns true without hitting the API
c.update_attributes("data"=>[{"Name"=>"plan", "Value"=>"monthly"}])

Should these methods be updated in Resource to call that? For example, modifying the method like this works:

    def update_attributes(attribute_hash = {}, options = {})
      self.attributes = attribute_hash
      opts = self.class.change_resource_path(options) # this was added to get the default config
      save(opts)
    end
Zhivko-Mailjet commented 7 years ago

Hi @excid3

Thanks for spotting and reporting the problem. We will investigate further on our end.

In the meantime, we will be more than happy to review your contribution

Lorel commented 7 years ago

Hi @excid3

Indeed, nice catch! 2fc4bccc6f016adb3b30e0984dae59b35497bdd9 should fix that

arnaudbreton commented 7 years ago

Hi @excid3, I've just published v1.5.1 which contains the fix @Lorel mentioned just above. It's available on Rubygems.

Hope it works for you, please let us know. Anyway, many thanks for reporting it and for having chosen Mailjet to power your emails.

excid3 commented 7 years ago

Fantastic, thanks guys! Will update to that and let you know if I run into any issues.

arnaudbreton commented 7 years ago

Awesome, thanks @excid3 for your feedback and time. Looking forward to hear back!