0

Users are either readers or subscribers. When a new article is published, subscribers get a full text e-mail, readers get a teaser text. Assuming scopes in the model for readers/subscribers, is this right or do I need another conditional for each type of user?

if @article.valid?
  User.subscribers.each do |user|
    ArticleMailer.send_article_full(@article, user).deliver_now
  end
  User.readers.each do |user|
    ArticleMailer.send_article_teaser(@article, user).deliver_now
  end
  redirect_to :root, notice: "Article sent"
else
  render :new, notice: "There was an error"
end
Matthew Bennett
  • 303
  • 2
  • 12

1 Answers1

1

Given the fact, every user can either be a reader or subscriber, but can't be both simultaneously, and if you have correct scopes defined for readers and subscribers in the User model, then it should be fine. You don't need another conditional for each type of user because that would be redundant.

K M Rakibul Islam
  • 33,760
  • 12
  • 89
  • 110
  • Right, thank you K M. I've been playing around with it this week and it works. The current versions have an extra scope for "wants e-mail", to weed out those users who don't first: `User.wantsarticles.readers.each do |user|`. There's perhaps another issue now with running the background job, but I'll ask that in another question. – Matthew Bennett Aug 26 '15 at 06:30
  • Here's the [next, related, question](http://stackoverflow.com/questions/32219759/is-this-expected-behaviour-for-delayed-job-rails-and-mandrill) on this, about how Delayed Job processes it all. – Matthew Bennett Aug 26 '15 at 06:50