How do you return out of an ActionMailer::Base function?
I'm trying to implement an ActionMailer function that will send out a newsletter to a specific user. I want to make sure that the newsletter is only sent to subscribed users. I tried implementing it like so:
class UserMailer < ActionMailer::Base
def newsletter(user)
return unless user.subscribed # This still renders my mailer view
mail(:to => user.email, :subject => "News开发者_Python百科letter")
end
end
The problem is that the return unless user.subscribed
line still appears to be rendering the mailer view and is still sent by the calling code (from a cron job):
task :cron => :environment do
User.where(:subscribed => true).each do |user|
UserMailer.newsletter(user).deliver
end
end
Note that I do have that subscription logic in my cron job as well for performance reasons (shouldn't have to iterate over ALL users, only those that are subscribed). However, it feels like the UserMailer class is the right place for this logic to exist (otherwise any other location that calls the newsletter
method will need to check the subscribed
flag as well.
The Mailer, IMHO, is the wrong place for this logic. The mailer should do nothing but format and send messages. The logic to decide whether or not to send should be within the calling block of code. It's not the right way, but something as simple as:
UserMailer.newsletter(user).deliver if user.subscribed?
Alternately, as you mentioned, you shouldn't have to iterate over all users, just the subscribed. So with a scope
in the User
model called subscribed
:
User.subscribed.each do |user|
UserMailer.newsletter(user).deliver
end
This way you don't need to test on a per-user basis; only the subscribed users are included, and the logic is in the calling block, not in the mailer.
精彩评论