diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 71b946b9ce8..f12165e2c94 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -120,21 +120,21 @@ module Jobs if type == "digest" return if user.staged - - if user.last_seen_at - if user.last_seen_at > + 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 - end + return end end seen_recently = - user.last_seen_at && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago - + ( + user.last_seen_at.present? && + 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) || diff --git a/app/jobs/scheduled/enqueue_digest_emails.rb b/app/jobs/scheduled/enqueue_digest_emails.rb index c5793cdc030..f700c31025f 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) - return if SiteSetting.disable_emails == "yes" - return if SiteSetting.disable_digest_emails? - return if SiteSetting.private_email? - - target_user_ids.each do |user_id| - ::Jobs.enqueue(:user_email, type: "digest", user_id: user_id) + if SiteSetting.disable_digest_emails? || SiteSetting.private_email? || + SiteSetting.disable_emails == "yes" + return end + users = target_user_ids + + users.each { |user_id| ::Jobs.enqueue(:user_email, type: "digest", user_id: user_id) } end def target_user_ids @@ -21,11 +21,14 @@ module Jobs .real .activated .not_suspended - .not_staged + .where(staged: false) .joins(:user_option, :user_stat, :user_emails) .where("user_options.email_digests") .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)", + ) .where( "COALESCE(user_stats.digest_attempted_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)", ) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 14cee52d9c5..c839780d434 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -224,7 +224,7 @@ class UserNotifications < ActionMailer::Base build_summary_for(user) @unsubscribe_key = UnsubscribeKey.create_key_for(@user, UnsubscribeKey::DIGEST_TYPE) - min_date = opts[:since] || user.last_seen_at || 1.month.ago + min_date = opts[:since] || user.last_emailed_at || user.last_seen_at || 1.month.ago # Fetch some topics and posts to show digest_opts = { diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index 71a5ff6dfb8..2d50068c62f 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -68,20 +68,6 @@ 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([]) @@ -283,8 +269,8 @@ RSpec.describe Jobs::UserEmail do it "creates an email log when the mail is sent (via Email::Sender)" do freeze_time - last_seen_at = 7.days.ago - user.update!(last_seen_at: last_seen_at) + last_emailed_at = 7.days.ago + user.update!(last_emailed_at: last_emailed_at) Topic.last.update(created_at: 1.minute.ago) expect do Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) end.to change { @@ -296,7 +282,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_seen_at) + expect(email_log.user.last_emailed_at).to_not eq_time(last_emailed_at) end it "creates a skipped email log when the mail is skipped" do