diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index f12165e2c94..1f3f7f6801d 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -119,22 +119,36 @@ module Jobs end if type == "digest" - return if user.staged - if user.last_emailed_at && - user.last_emailed_at > - ( - user.user_option&.digest_after_minutes || - SiteSetting.default_email_digest_frequency.to_i - ).minutes.ago - return + # same checks as in the "enqueue_digest_emails" job + # in case something changed since the job was enqueued + return if SiteSetting.disable_digest_emails? || SiteSetting.private_email? + + return if user.bot? || user.anonymous? || !user.active || user.suspended? || user.staged + + return if !user.user_stat + return if user.user_stat.bounce_score >= SiteSetting.bounce_score_threshold + + return if !user.user_option + return if !user.user_option.email_digests + return if user.user_option.mailing_list_mode + + delay = user.user_option.digest_after_minutes || SiteSetting.default_email_digest_frequency + delay = delay.to_i + + return if delay <= 0 # 0 means never send digest + + if user.user_stat.digest_attempted_at + return if user.user_stat.digest_attempted_at > delay.minutes.ago + end + + if user.last_seen_at + return if user.last_seen_at > delay.minutes.ago end end seen_recently = - ( - user.last_seen_at.present? && - user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago - ) + user.last_seen_at && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago + if !args[:force_respect_seen_recently] && ( always_email_regular?(user, type) || always_email_private_message?(user, type) || @@ -164,16 +178,17 @@ module Jobs email_args[:notification_type] = email_args[:notification_type].to_s end - if !SiteSetting.disable_mailing_list_mode && user.user_option.mailing_list_mode? && - user.user_option.mailing_list_mode_frequency > 0 && # don't catch notifications for users on daily mailing list mode - (!post.try(:topic).try(:private_message?)) && - NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type]) - # no need to log a reason when the mail was already sent via the mailing list job - return nil, nil + # don't catch notifications for users on daily mailing list mode + if user.user_option.mailing_list_mode && user.user_option.mailing_list_mode_frequency > 0 + if !post&.topic&.private_message? && + NOTIFICATIONS_SENT_BY_MAILING_LIST.include?(email_args[:notification_type]) + # no need to log a reason when the mail was already sent via the mailing list job + return + end end unless always_email_regular?(user, type) || always_email_private_message?(user, type) - if (notification && notification.read?) || (post && post.seen?(user)) + if notification&.read? || post&.seen?(user) return skip_message(SkippedEmailLog.reason_types[:user_email_notification_already_read]) end end diff --git a/app/jobs/scheduled/enqueue_digest_emails.rb b/app/jobs/scheduled/enqueue_digest_emails.rb index f700c31025f..85cb2945d04 100644 --- a/app/jobs/scheduled/enqueue_digest_emails.rb +++ b/app/jobs/scheduled/enqueue_digest_emails.rb @@ -5,13 +5,13 @@ module Jobs every 30.minutes def execute(args) - if SiteSetting.disable_digest_emails? || SiteSetting.private_email? || - SiteSetting.disable_emails == "yes" - return - end - users = target_user_ids + return if SiteSetting.disable_emails == "yes" + return if SiteSetting.disable_digest_emails? + return if SiteSetting.private_email? - users.each { |user_id| ::Jobs.enqueue(:user_email, type: "digest", user_id: user_id) } + target_user_ids.each do |user_id| + ::Jobs.enqueue(:user_email, type: "digest", user_id: user_id) + end end def target_user_ids @@ -20,20 +20,23 @@ module Jobs User .real .activated + .not_staged .not_suspended - .where(staged: false) .joins(:user_option, :user_stat, :user_emails) .where("user_options.email_digests") + .where( + "COALESCE(user_options.digest_after_minutes, ?) > 0", + SiteSetting.default_email_digest_frequency, + ) .where("user_stats.bounce_score < ?", SiteSetting.bounce_score_threshold) .where("user_emails.primary") .where( - "COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)", + "COALESCE(user_stats.digest_attempted_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * COALESCE(user_options.digest_after_minutes, ?))", + SiteSetting.default_email_digest_frequency, ) .where( - "COALESCE(user_stats.digest_attempted_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)", - ) - .where( - "COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)", + "COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * COALESCE(user_options.digest_after_minutes, ?))", + SiteSetting.default_email_digest_frequency, ) .where( "COALESCE(last_seen_at, '2010-01-01') >= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * ?)", diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index c839780d434..01f90cbc022 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -224,18 +224,18 @@ class UserNotifications < ActionMailer::Base build_summary_for(user) @unsubscribe_key = UnsubscribeKey.create_key_for(@user, UnsubscribeKey::DIGEST_TYPE) - min_date = opts[:since] || user.last_emailed_at || user.last_seen_at || 1.month.ago + @since = opts[:since] || user.last_seen_at || user.user_stat&.digest_attempted_at || 1.month.ago # Fetch some topics and posts to show digest_opts = { limit: SiteSetting.digest_topics + SiteSetting.digest_other_topics, top_order: true, } - topics_for_digest = Topic.for_digest(user, min_date, digest_opts) + topics_for_digest = Topic.for_digest(user, @since, digest_opts) if topics_for_digest.empty? && !user.user_option.try(:include_tl0_in_digests) # Find some topics from new users that are at least 24 hours old topics_for_digest = - Topic.for_digest(user, min_date, digest_opts.merge(include_tl0: true)).where( + Topic.for_digest(user, @since, digest_opts.merge(include_tl0: true)).where( "topics.created_at < ?", 24.hours.ago, ) @@ -257,7 +257,7 @@ class UserNotifications < ActionMailer::Base if SiteSetting.digest_posts > 0 Post .order("posts.score DESC") - .for_mailing_list(user, min_date) + .for_mailing_list(user, @since) .where("posts.post_type = ?", Post.types[:regular]) .where( "posts.deleted_at IS NULL AND posts.hidden = false AND posts.user_deleted = false", @@ -275,20 +275,16 @@ class UserNotifications < ActionMailer::Base @excerpts = {} - @popular_topics.map do |t| - @excerpts[t.first_post.id] = email_excerpt( - t.first_post.cooked, - t.first_post, - ) if t.first_post.present? + @popular_topics.each do |t| + next if t.first_post.blank? + @excerpts[t.first_post.id] = email_excerpt(t.first_post.cooked, t.first_post) end # Try to find 3 interesting stats for the top of the digest - new_topics_count = Topic.for_digest(user, min_date).count + new_topics_count = Topic.for_digest(user, @since).count + # We used topics from new users instead, so count should match + new_topics_count = topics_for_digest.size if new_topics_count == 0 - if new_topics_count == 0 - # We used topics from new users instead, so count should match - new_topics_count = topics_for_digest.size - end @counts = [ { id: "new_topics", @@ -312,7 +308,7 @@ class UserNotifications < ActionMailer::Base end if @counts.size < 3 - value = user.unread_notifications_of_type(Notification.types[:liked], since: min_date) + value = user.unread_notifications_of_type(Notification.types[:liked], since: @since) if value > 0 @counts << { id: "likes_received", @@ -324,7 +320,7 @@ class UserNotifications < ActionMailer::Base end if @counts.size < 3 && user.user_option.digest_after_minutes.to_i >= 1440 - value = summary_new_users_count(min_date) + value = summary_new_users_count(@since) if value > 0 @counts << { id: "new_users", @@ -335,9 +331,7 @@ class UserNotifications < ActionMailer::Base end end - @last_seen_at = short_date(user.last_seen_at || user.created_at) - - @preheader_text = I18n.t("user_notifications.digest.preheader", last_seen_at: @last_seen_at) + @preheader_text = I18n.t("user_notifications.digest.preheader", since: @since) opts = { from_alias: I18n.t("user_notifications.digest.from", site_name: Email.site_title), @@ -841,7 +835,7 @@ class UserNotifications < ActionMailer::Base end def build_summary_for(user) - @site_name = SiteSetting.email_prefix.presence || SiteSetting.title # used by I18n + @site_name = SiteSetting.email_prefix.presence || SiteSetting.title @user = user @date = short_date(Time.now) @base_url = Discourse.base_url @@ -853,24 +847,17 @@ class UserNotifications < ActionMailer::Base @disable_email_custom_styles = !SiteSetting.apply_custom_styles_to_digest end - def self.summary_new_users_count_key(min_date_str) - "summary-new-users:#{min_date_str}" - end - - def summary_new_users_count(min_date) - min_date_str = min_date.is_a?(String) ? min_date : min_date.strftime("%Y-%m-%d") - key = self.class.summary_new_users_count_key(min_date_str) - ((count = Discourse.redis.get(key)) && count.to_i) || - begin - count = - User - .real - .where(active: true, staged: false) - .not_suspended - .where("created_at > ?", min_date_str) - .count - Discourse.redis.setex(key, 1.day, count) - count - end + def summary_new_users_count(since) + date = since.is_a?(String) ? since : since.strftime("%Y-%m-%d") + key = "summary-new-users:#{date}" + Discourse.redis.get(key)&.to_i || + User + .real + .activated + .not_staged + .not_suspended + .where("created_at > ?", date) + .count + .tap { Discourse.redis.setex(key, 1.day, _1) } end end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 0ee8e71472e..60a90e65d94 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -85,14 +85,8 @@ class UserOption < ActiveRecord::Base self.like_notification_frequency = SiteSetting.default_other_like_notification_frequency - if SiteSetting.default_email_digest_frequency.to_i <= 0 - self.email_digests = false - else - self.email_digests = true - end - - self.digest_after_minutes ||= SiteSetting.default_email_digest_frequency.to_i - + self.email_digests = SiteSetting.default_email_digest_frequency.to_i > 0 + self.digest_after_minutes = SiteSetting.default_email_digest_frequency.to_i self.include_tl0_in_digests = SiteSetting.default_include_tl0_in_digests self.text_size = SiteSetting.default_text_size @@ -107,8 +101,7 @@ class UserOption < ActiveRecord::Base end def mailing_list_mode - return false if SiteSetting.disable_mailing_list_mode - super + SiteSetting.disable_mailing_list_mode ? false : super end def redirected_to_top_yet? diff --git a/app/views/user_notifications/digest.text.erb b/app/views/user_notifications/digest.text.erb index 40ce99b8ef3..4aa90877f6d 100644 --- a/app/views/user_notifications/digest.text.erb +++ b/app/views/user_notifications/digest.text.erb @@ -1,7 +1,5 @@ <%- site_link = raw(@markdown_linker.create(@site_name, '/')) %> -<%= raw(t 'user_notifications.digest.why', - site_link: site_link, - last_seen_at: @last_seen_at) %> +<%= raw(t 'user_notifications.digest.why', site_link: site_link, since: @since) %> <%- @counts.each do |count| -%> <%= count[:value] -%> <%=t count[:label_key] %> diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index ff9668126b1..a22f10bf786 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -4251,7 +4251,7 @@ en: If you have any questions, [contact our friendly staff](%{base_url}/about). digest: - why: "A brief summary of %{site_link} since your last visit on %{last_seen_at}" + why: "A brief summary of %{site_link} since %{since}" since_last_visit: "Since your last visit" new_topics: "New Topics" unread_notifications: "Unread Notifications" @@ -4267,7 +4267,7 @@ en: your_email_settings: "your email settings" click_here: "click here" from: "%{site_name}" - preheader: "A brief summary since your last visit on %{last_seen_at}" + preheader: "A brief summary since %{since}" custom: html: header: "" diff --git a/spec/jobs/enqueue_digest_emails_spec.rb b/spec/jobs/enqueue_digest_emails_spec.rb index abe1802d2d2..b126805578e 100644 --- a/spec/jobs/enqueue_digest_emails_spec.rb +++ b/spec/jobs/enqueue_digest_emails_spec.rb @@ -1,220 +1,149 @@ # frozen_string_literal: true RSpec.describe Jobs::EnqueueDigestEmails do - describe "#target_users" do - context "with disabled digests" do - before { SiteSetting.default_email_digest_frequency = 0 } - let!(:user_no_digests) do - Fabricate(:active_user, last_emailed_at: 8.days.ago, last_seen_at: 10.days.ago) - end - - it "doesn't return users with email disabled" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(user_no_digests.id)).to eq( - false, - ) - end - end - - context "with unapproved users" do - before { SiteSetting.must_approve_users = true } - - let!(:unapproved_user) do - Fabricate( - :active_user, - approved: false, - last_emailed_at: 8.days.ago, - last_seen_at: 10.days.ago, - ) - end - - it "should enqueue the right digest emails" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(unapproved_user.id)).to eq( - false, - ) - - # As a moderator - unapproved_user.update_column(:moderator, true) - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(unapproved_user.id)).to eq( - true, - ) - - # As an admin - unapproved_user.update(admin: true, moderator: false) - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(unapproved_user.id)).to eq( - true, - ) - - # As an approved user - unapproved_user.update(admin: false, moderator: false, approved: true) - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(unapproved_user.id)).to eq( - true, - ) - end - end - - context "with staged users" do - let!(:staged_user) do - Fabricate(:active_user, staged: true, last_emailed_at: 1.year.ago, last_seen_at: 1.year.ago) - end - - it "doesn't return staged users" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(staged_user.id)).to eq(false) - end - end - - context "when recently emailed" do - let!(:user_emailed_recently) { Fabricate(:active_user, last_emailed_at: 6.days.ago) } - - it "doesn't return users who have been emailed recently" do - expect( - Jobs::EnqueueDigestEmails.new.target_user_ids.include?(user_emailed_recently.id), - ).to eq(false) - end - end - - context "with inactive user" do - let!(:inactive_user) { Fabricate(:user, active: false) } - - it "doesn't return users who have been emailed recently" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(inactive_user.id)).to eq( - false, - ) - end - end - - context "with suspended user" do - let!(:suspended_user) do - Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) - end - - it "doesn't return users who are suspended" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(suspended_user.id)).to eq( - false, - ) - end - end - - context "when visited the site this week" do - let(:user_visited_this_week) { Fabricate(:active_user, last_seen_at: 6.days.ago) } - - it "doesn't return users who have been emailed recently" do - user = user_visited_this_week - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(user.id)).to eq(false) - end - end - - context "when visited the site a year ago" do - let!(:user_visited_a_year_ago) { Fabricate(:active_user, last_seen_at: 370.days.ago) } - - it "doesn't return the user who have not visited the site for more than 365 days" do - expect( - Jobs::EnqueueDigestEmails.new.target_user_ids.include?(user_visited_a_year_ago.id), - ).to eq(false) - end - end - - context "with regular users" do - let!(:user) do - Fabricate( - :active_user, - last_seen_at: (SiteSetting.suppress_digest_email_after_days - 1).days.ago, - ) - end - - it "returns the user" do - expect(Jobs::EnqueueDigestEmails.new.target_user_ids).to eq([user.id]) - end - end - - context "with too many bounces" do - let!(:bounce_user) { Fabricate(:active_user, last_seen_at: 6.month.ago) } - - it "doesn't return users with too many bounces" do - bounce_user.user_stat.update(bounce_score: SiteSetting.bounce_score_threshold + 1) - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(bounce_user.id)).to eq(false) - end - end - - context "with no primary email" do - let!(:user) { Fabricate(:active_user, last_seen_at: 2.months.ago) } - - it "doesn't return users with no primary emails" do - UserEmail.where(user: user).delete_all - expect(Jobs::EnqueueDigestEmails.new.target_user_ids.include?(user.id)).to eq(false) - end - end - end + let(:job) { described_class.new } describe "#execute" do - let(:user) { Fabricate(:user) } + fab!(:user) - it "limits jobs enqueued per max_digests_enqueued_per_30_mins_per_site" do - user1 = Fabricate(:user, last_seen_at: 2.months.ago, last_emailed_at: 2.months.ago) - user2 = Fabricate(:user, last_seen_at: 2.months.ago, last_emailed_at: 2.months.ago) + context "when all emails are disabled" do + before { SiteSetting.disable_emails = "yes" } - user1.user_stat.update(digest_attempted_at: 2.week.ago) - user2.user_stat.update(digest_attempted_at: 3.weeks.ago) - - global_setting :max_digests_enqueued_per_30_mins_per_site, 1 - - expect_enqueued_with(job: :user_email, args: { type: :digest, user_id: user2.id }) do - expect { Jobs::EnqueueDigestEmails.new.execute(nil) }.to change( - Jobs::UserEmail.jobs, - :size, - ).by(1) - end - - # The job didn't actually run, so fake the user_stat update - user2.user_stat.update(digest_attempted_at: Time.zone.now) - - expect_enqueued_with(job: :user_email, args: { type: :digest, user_id: user1.id }) do - expect { Jobs::EnqueueDigestEmails.new.execute(nil) }.to change( - Jobs::UserEmail.jobs, - :size, - ).by(1) - end - - user1.user_stat.update(digest_attempted_at: Time.zone.now) - - expect_not_enqueued_with(job: :user_email) { Jobs::EnqueueDigestEmails.new.execute(nil) } - end - - context "when digest emails are enabled" do - before { Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).returns([user.id]) } - - it "enqueues the digest email job" do - SiteSetting.disable_digest_emails = false - - expect_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do - Jobs::EnqueueDigestEmails.new.execute({}) - end - end - end - - context "with private email" do - before do + it "does not enqueue the digest email job" do Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never - SiteSetting.private_email = true - end - it "doesn't return users with email disabled" do expect_not_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do - Jobs::EnqueueDigestEmails.new.execute({}) + job.execute({}) end end end context "when digest emails are disabled" do - before do - Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never - SiteSetting.disable_digest_emails = true - end + before { SiteSetting.disable_digest_emails = true } it "does not enqueue the digest email job" do + Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never + expect_not_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do - Jobs::EnqueueDigestEmails.new.execute({}) + job.execute({}) + end + end + end + + context "when emails are private" do + before { SiteSetting.private_email = true } + + it "does not enqueue the digest email job" do + Jobs::EnqueueDigestEmails.any_instance.expects(:target_user_ids).never + + expect_not_enqueued_with(job: :user_email, args: { type: :digest, user_id: user.id }) do + job.execute({}) end end end end + + describe "#target_user_ids" do + fab!(:user) { Fabricate(:active_user, last_seen_at: 10.days.ago) } + fab!(:bot) { Fabricate(:bot, last_seen_at: 10.days.ago) } + fab!(:anon) { Fabricate(:anonymous, last_seen_at: 10.days.ago) } + + it "never returns bots" do + expect(job.target_user_ids).not_to include(bot.id) + end + + it "never returns anonymous users" do + expect(job.target_user_ids).not_to include(anon.id) + end + + it "never returns inactive users" do + user.update!(active: false) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns staged users" do + user.update!(staged: true) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns suspended users" do + user.update!(suspended_till: 1.day.from_now) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have not opted in to digest emails" do + user.user_option.update!(email_digests: false) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have set a digest frequency to 'never" do + user.user_option.update!(digest_after_minutes: 0) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have not set a digest frequency and it's disabled globally" do + SiteSetting.default_email_digest_frequency = 0 + user.user_option.update!(digest_after_minutes: nil) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have a bounce score above the threshold" do + user.user_stat.update!(bounce_score: SiteSetting.bounce_score_threshold + 1) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who doesn't have a primary email" do + user.user_emails.update_all(primary: false) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have received a digest email too recently" do + user.user_stat.update!(digest_attempted_at: 1.minute.ago) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have been seen too recently" do + user.update!(last_seen_at: 1.minute.ago) + expect(job.target_user_ids).not_to include(user.id) + end + + it "never returns users who have been seen too long ago" do + user.update!(last_seen_at: (SiteSetting.suppress_digest_email_after_days + 1).days.ago) + expect(job.target_user_ids).not_to include(user.id) + end + + context "when the site requires user approval" do + before { SiteSetting.must_approve_users = true } + + it "never returns users who have not been approved" do + user.update!(approved: false) + expect(job.target_user_ids).not_to include(user.id) + end + + it "returns users who have been approved" do + user.update!(approved: true) + expect(job.target_user_ids).to include(user.id) + end + + it "always returns moderators" do + user.update!(approved: false, moderator: true) + expect(job.target_user_ids).to include(user.id) + end + + it "always returns admins" do + user.update!(approved: false, admin: true) + expect(job.target_user_ids).to include(user.id) + end + end + + it "limits the number of users returned" do + global_setting :max_digests_enqueued_per_30_mins_per_site, 1 + 2.times { Fabricate(:active_user, last_seen_at: 10.days.ago) } + expect(job.target_user_ids.size).to eq(1) + end + + it "returns the user ids of users who want to receive digest emails" do + expect(job.target_user_ids).to eq([user.id]) + end + end end diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index 2d50068c62f..b05d5d5366a 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -48,6 +48,37 @@ RSpec.describe Jobs::UserEmail do expect(ActionMailer::Base.deliveries).to eq([]) end + it "doesn't call the mailer when the user is suspended" do + suspended.update!(last_seen_at: 8.days.ago, last_emailed_at: 8.days.ago) + Jobs::UserEmail.new.execute(type: :digest, user_id: suspended.id) + expect(ActionMailer::Base.deliveries).to eq([]) + end + + it "doesn't call the mailer when the user is not active" do + user.update!(active: false) + Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) + expect(ActionMailer::Base.deliveries).to eq([]) + end + + it "doesn't call the mailer when the user has disabled email digests" do + user.user_option.update!(email_digests: false) + Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) + expect(ActionMailer::Base.deliveries).to eq([]) + end + + it "doesn't call the mailer when the user has enabled mailing list mode" do + SiteSetting.disable_mailing_list_mode = false + user.user_option.update!(mailing_list_mode: true) + Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) + expect(ActionMailer::Base.deliveries).to eq([]) + end + + it "doesn't call the mailer when the user's digest_after_minute is 0" do + user.user_option.update!(digest_after_minutes: 0) + Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) + expect(ActionMailer::Base.deliveries).to eq([]) + end + context "when not emailed recently" do before do freeze_time @@ -68,6 +99,20 @@ RSpec.describe Jobs::UserEmail do user.user_option.update!(digest_after_minutes: 1.day.to_i / 60) end + it "still sends the digest email" do + Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) + expect(ActionMailer::Base.deliveries).to_not be_empty + expect(user.user_stat.reload.digest_attempted_at).to eq_time(Time.zone.now) + end + end + + context "when recently seen" do + before do + freeze_time + user.update!(last_seen_at: 2.hours.ago) + user.user_option.update!(digest_after_minutes: 1.day.to_i / 60) + end + it "skips sending digest email" do Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) expect(ActionMailer::Base.deliveries).to eq([]) @@ -269,8 +314,8 @@ RSpec.describe Jobs::UserEmail do it "creates an email log when the mail is sent (via Email::Sender)" do freeze_time - last_emailed_at = 7.days.ago - user.update!(last_emailed_at: last_emailed_at) + last_seen_at = 7.days.ago + user.update!(last_seen_at: last_seen_at) Topic.last.update(created_at: 1.minute.ago) expect do Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) end.to change { @@ -282,7 +327,7 @@ RSpec.describe Jobs::UserEmail do expect(email_log.user).to eq(user) expect(email_log.post).to eq(nil) # last_emailed_at should have changed - expect(email_log.user.last_emailed_at).to_not eq_time(last_emailed_at) + expect(email_log.user.last_emailed_at).to_not eq_time(last_seen_at) end it "creates a skipped email log when the mail is skipped" do