Crash in Resource#save

[zedalaye]

My "unsubscribe from lists" code fails since I updated to mailjet-gem 1.5, the problem is still present with 1.5.1 :

l = ::Mailjet::Contactslist.all(name: "MyNewsletter", limit: 1).first
lr = ::Mailjet::Listrecipient.all('ContactsList' => l.id, 'Email' => 'me@some.where', limit: 1).first
lr.is_unsubscribed # false
lr.is_unsubscribed = true
lr.is_active = false
lr.save

ArgumentError: wrong number of arguments (given 0, expected 1)
from /home/pierre/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/gems/mailjet-1.5.1/lib/mailjet/resource.rb:246:in `save'

[Zhivko-Mailjet]

@zedalaye
Thanks for reporting the problem. We will look into it and will get back to you once we have any news.

[arnaudbreton]

Hi @zedalaye, apologies for the issue. I've just pushed a branch to stage v1.5.2. Commit ce2877b should fix your issue.

Can you please confirm? I'll then release v1.5.2 with it to Rubygems. Thanks!

Apologies once more for the issue.

[zedalaye]

As far as I can understand the mailjet-gem code, it looks to me quite impossible for the gem to hit the API when options = {} because of :

      def self.default_connection(options = {})
        Mailjet::Connection.new(
          "#{options[:url]}/#{options[:version]}/#{resource_path}",
          options[:api_key] || Mailjet.config.api_key,
          options[:secret_key] || Mailjet.config.secret_key,
          public_operations: public_operations,
          read_only: read_only)
      end

Using the debugger, in Mailjet::Connection.initialize, endpoint is "//REST/listrecipient" so it looks like :url and :version must, at least, be specified in order to build a valid URL.

Don't you think this code from Mailjet::Resource#update_attributes should be moved to #save ?

    opts = self.class.change_resource_path(options)
    save(opts)

[arnaudbreton]

@zedalaye thanks for your feedback.

Could you show us a full extract of your code (minus your API credentials)? The sample you provided above works me with when I use it this way:

require 'mailjet'

Mailjet.configure do |config|
  config.api_key = ENV['MJ_APIKEY_PUBLIC']
  config.secret_key = ENV['MJ_APIKEY_PRIVATE']
  config.default_from = 'my_registered_mailjet_email@domain.com'
end

l = ::Mailjet::Contactslist.all(limit: 1).first
lr = ::Mailjet::Listrecipient.all('ContactsList' => l.id, 'Email' => 'me@some.where', limit: 1).first
lr.is_unsubscribed # false
lr.is_unsubscribed = true
lr.is_active = false
lr.save

[zedalaye]

All Mailjet subscription/unsubscription code is handled by https://github.com/zedalaye/devise_mailjet/blob/212f7c72ea83e22a9a5936fd56c46dc9f2d43fea/lib/devise_mailjet/mailjet_list_api_mapper.rb#L33-L59 later I switched to lr.update_attributes(is_unsubscribed: true, is_active: false) (https://github.com/zedalaye/devise_mailjet/blob/master/lib/devise_mailjet/mailjet_list_api_mapper.rb) that works as expetected.

[arnaudbreton]

@zedalaye thanks for the details, however I'm not sure I'm following if the issue is fixed following my commit mentioned above?

[zedalaye]

Sorry for not being as clear as you would have expected. No, your commit has not solved my issue. The API call from lr.save do not hit the API (because in subsequent Mailjet::Connection.initialize, endpoint is //REST/listrecipient) so, for now I have to stick with the update_attributes version instead of save. Can you tell me if I'm clear enough ?

[arnaudbreton]

@zedalaye now it's clear, thanks! Apologies for the misunderstanding. You say it's only subsequent calls which are failing. Does it mean the first call work?

[zedalaye]

Still not that clear :)

From my devise_mailjet/mailjet_api_mapper.rb

The first call to the API is : ::Mailjet::Contactslist.all(...), it works. These subsenquent calls works too : ::Mailjet::Contactslist.create(...), ::Mailjet::Contact.find(email), ::Mailjet::Contact.create(email: email) as do lr = ::Mailjet::Listrecipient.all('ContactsList' => list_id, 'Contact' => contact_id, limit: 1).first

What don't work is the final lr.save from https://github.com/zedalaye/devise_mailjet/blob/212f7c72ea83e22a9a5936fd56c46dc9f2d43fea/lib/devise_mailjet/mailjet_list_api_mapper.rb#L33-L59

I had to switch to lr.update_attributes() to make it work.

[arnaudbreton]

Thanks. Are you sure you're using the wrapper at the right revision? I tested your sample by myself and it now works fine (when I switch back to master branch, it raises the error than yours)

[zedalaye]

I just built this straight sample code : https://gist.github.com/zedalaye/3307cca6a94de28f180570fccd55533f

If you run the code test-with-save.rb several times, you'll show that no error is raised BUT, the output is always : Subscribed : Unsubscribe ! instead of swapping from Subscribe <-> Unsubscribe each time the code is run as it happens with test-with-update-attributes.rb

[arnaudbreton]

@zedalaye thanks, your sample code has been indeed very helpful to debug what's going on. If I didn't mess up, 886a46a should fix your issue

[zedalaye]

It seems to work as expected right now. Thank you very much !

[zedalaye]

By the way, this may not be the right place to ask for such a question but... is this workflow the most efficient way to subscribe/unsubscribe a contact (that may exist, or not) to a list (that may exist, or not) ?

[arnaudbreton]

@zedalaye awesome, glad to hear. I'm one who should thank you, your sample saved me sometimes to troubleshoot efficiently.

I believe the /contactslist/:id/managecontact endpoint might be a bit more efficient by allowing to merge the contact creation and subscription step in 1 call: https://dev.mailjet.com/guides/?ruby#create-contact-and-subscribe-at-once

Also, it'll allow you to use the same methods for different action (subscription, resubscription, unsubscription, removal).

I'm closing this issue now and will proceed with v1.5.2 release.