diff --git a/app/jobs/regular/notify_mailing_list_subscribers.rb b/app/jobs/regular/notify_mailing_list_subscribers.rb index 90cd12d5af1..c8db65406ed 100644 --- a/app/jobs/regular/notify_mailing_list_subscribers.rb +++ b/app/jobs/regular/notify_mailing_list_subscribers.rb @@ -12,9 +12,8 @@ module Jobs raise Discourse::InvalidParameters.new(:post_id) unless post - User.not_suspended - .not_blocked - .real + users = + User.activated.not_blocked.not_suspended.real .where(mailing_list_mode: true) .where('NOT EXISTS( SELECT 1 @@ -32,11 +31,25 @@ module Jobs cu.user_id = users.id AND cu.notification_level = ? )', post.topic.category_id, CategoryUser.notification_levels[:muted]) - .each do |user| - if Guardian.new(user).can_see?(post) - message = UserNotifications.mailing_list_notify(user, post) - Email::Sender.new(message, :mailing_list, user).send + + error_count = 0 + users.each do |user| + if Guardian.new(user).can_see?(post) + begin + message = UserNotifications.mailing_list_notify(user, post) + Email::Sender.new(message, :mailing_list, user).send + rescue => e + Discourse.handle_exception(e, error_context( + args, + "Sending post to mailing list subscribers", { + user_id: user.id, + user_email: user.email + })) + if (++error_count) >= 4 + raise RuntimeError, "ABORTING NotifyMailingListSubscribers due to repeated failures" end + end + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 3c419a70a7a..d5dc7a70212 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -99,13 +99,18 @@ class User < ActiveRecord::Base # set to true to optimize creation and save for imports attr_accessor :import_mode - scope :blocked, -> { where(blocked: true) } # no index - scope :not_blocked, -> { where(blocked: false) } # no index - scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) } # no index - scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) } - # excluding fake users like the community user + # excluding fake users like the system user scope :real, -> { where('id > 0') } + # TODO-PERF: There is no indexes on any of these + # and NotifyMailingListSubscribers does a select-all-and-loop + # may want to create an index on (active, blocked, suspended_till, mailing_list_mode)? + scope :blocked, -> { where(blocked: true) } + scope :not_blocked, -> { where(blocked: false) } + scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) } + scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) } + scope :activated, -> { where(active: true) } + module NewTopicDuration ALWAYS = -1 LAST_VISIT = -2