delayed ActionMailer fails to deliver multiple recipients

Question

I have an mailer class like this

class Mailer < ActionMailer::Base
  def newsletter_scheduler()
    recipients = User.all.to_a
    Mailer.delay_for(certain_time.days).newsletter(recipients)
  end

  def newsletter(recipients)
    recipients.each do |user|
      Logger.log("sending newsletter to #{user.email}"
      mail(
        to: user.email,
        subject: "What's hot last week",
      ).deliver
  end
end

I'm not looking for alternative to implement this... the problem is that this sends the newsletter to the last recipient only and multiple times So for example in the specs

it 'sends newsletter to all users' do
  Mailer.newsletter_scheduler()
  newsletters = ActionMailer::Base.deliveries

  newsletters.each { |nl| p nl.to }
end

i get this (note: I have 5 users)

["[email protected]"]
["[email protected]"]
["[email protected]"]
["[email protected]"]
["[email protected]"]

can anybody tell me why this is happening while the logger showing the following

sending newsletter to [email protected]
sending newsletter to [email protected]
sending newsletter to [email protected]
sending newsletter to [email protected]
sending newsletter to [email protected]

[EDIT] the following code have the same effect as the code above

class Newsletter
  def self.schedule
    recipients = User.all.map { |u| u.id }
    Mailer.delay_for(certain_time.days).newsletter(recipients)
  end
end

class Mailer < ActionMailer::Base
  def newsletter(recipients)
    recipients.each do |id|
      user = User.find(id)
      Logger.log("sending newsletter to #{user.email}"
      mail(
        to: user.email,
        subject: "What's hot last week",
      )
  end
end

Newsletter.schedule() #produces the same results in the specs

Answer 1

Your code also present another issue. #deliver is expected to be called outside the mail action as I shown you in my code above.

Consider to split your code

class NewsletterScheduler
  def self.execute
    User.all.each do |user|
      Mailer.delay_for(certain_time.days).newsletter(user)
    end
  end
end

class Mailer < ActionMailer::Base
  def newsletter(user)
    Logger.log("sending newsletter to #{user.email}"
    mail(to: user.email, subject: "What's hot last week")
  end
end

NewsletterScheduler.execute

---

Important warning: You should never pass complex object as arguments to a queue. Instead, always pass a reference.

In your case change

def newsletter(recipients)

to accepts an array of IDs instead of an array of User objects. The reason is that objects are complicated and internal state can change, especially if they are stored in a database. Moreover, serializing a primitive value is more efficient than serializing a complex object.

class NewsletterScheduler
  def self.execute
    User.all.each do |user|
      Mailer.delay_for(certain_time.days).newsletter(user_id)
    end
  end
end

class Mailer < ActionMailer::Base
  def newsletter(user_id)
    user = User.find(user_id)
    Logger.log("sending newsletter to #{user.email}"
    mail(to: user.email, subject: "What's hot last week")
  end
end