FIX: send activity summaries based on "last seen"

instead of "last emailed" so that people getting email notifications (from a watched topic for example) also get the activity summaries.

Context - https://meta.discourse.org/t/activity-summary-not-sent-if-other-emails-are-sent/293040

Internal Ref - t//125582
This commit is contained in:
Régis Hanol 2024-05-06 13:14:10 +02:00
parent 22db0b56e6
commit 95885645d9
4 changed files with 32 additions and 21 deletions

View File

@ -120,21 +120,21 @@ module Jobs
if type == "digest" if type == "digest"
return if user.staged return if user.staged
if user.last_emailed_at &&
user.last_emailed_at > if user.last_seen_at
if user.last_seen_at >
( (
user.user_option&.digest_after_minutes || user.user_option&.digest_after_minutes ||
SiteSetting.default_email_digest_frequency.to_i SiteSetting.default_email_digest_frequency.to_i
).minutes.ago ).minutes.ago
return return
end
end end
end end
seen_recently = 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] && if !args[:force_respect_seen_recently] &&
( (
always_email_regular?(user, type) || always_email_private_message?(user, type) || always_email_regular?(user, type) || always_email_private_message?(user, type) ||

View File

@ -5,13 +5,13 @@ module Jobs
every 30.minutes every 30.minutes
def execute(args) def execute(args)
if SiteSetting.disable_digest_emails? || SiteSetting.private_email? || return if SiteSetting.disable_emails == "yes"
SiteSetting.disable_emails == "yes" return if SiteSetting.disable_digest_emails?
return return if SiteSetting.private_email?
end
users = target_user_ids
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 end
def target_user_ids def target_user_ids
@ -21,14 +21,11 @@ module Jobs
.real .real
.activated .activated
.not_suspended .not_suspended
.where(staged: false) .not_staged
.joins(:user_option, :user_stat, :user_emails) .joins(:user_option, :user_stat, :user_emails)
.where("user_options.email_digests") .where("user_options.email_digests")
.where("user_stats.bounce_score < ?", SiteSetting.bounce_score_threshold) .where("user_stats.bounce_score < ?", SiteSetting.bounce_score_threshold)
.where("user_emails.primary") .where("user_emails.primary")
.where(
"COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 MINUTE'::INTERVAL * user_options.digest_after_minutes)",
)
.where( .where(
"COALESCE(user_stats.digest_attempted_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 * user_options.digest_after_minutes)",
) )

View File

@ -224,7 +224,7 @@ class UserNotifications < ActionMailer::Base
build_summary_for(user) build_summary_for(user)
@unsubscribe_key = UnsubscribeKey.create_key_for(@user, UnsubscribeKey::DIGEST_TYPE) @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 min_date = opts[:since] || user.last_seen_at || 1.month.ago
# Fetch some topics and posts to show # Fetch some topics and posts to show
digest_opts = { digest_opts = {

View File

@ -68,6 +68,20 @@ RSpec.describe Jobs::UserEmail do
user.user_option.update!(digest_after_minutes: 1.day.to_i / 60) user.user_option.update!(digest_after_minutes: 1.day.to_i / 60)
end 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 it "skips sending digest email" do
Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) Jobs::UserEmail.new.execute(type: :digest, user_id: user.id)
expect(ActionMailer::Base.deliveries).to eq([]) expect(ActionMailer::Base.deliveries).to eq([])
@ -269,8 +283,8 @@ RSpec.describe Jobs::UserEmail do
it "creates an email log when the mail is sent (via Email::Sender)" do it "creates an email log when the mail is sent (via Email::Sender)" do
freeze_time freeze_time
last_emailed_at = 7.days.ago last_seen_at = 7.days.ago
user.update!(last_emailed_at: last_emailed_at) user.update!(last_seen_at: last_seen_at)
Topic.last.update(created_at: 1.minute.ago) Topic.last.update(created_at: 1.minute.ago)
expect do Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) end.to change { expect do Jobs::UserEmail.new.execute(type: :digest, user_id: user.id) end.to change {
@ -282,7 +296,7 @@ RSpec.describe Jobs::UserEmail do
expect(email_log.user).to eq(user) expect(email_log.user).to eq(user)
expect(email_log.post).to eq(nil) expect(email_log.post).to eq(nil)
# last_emailed_at should have changed # 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 end
it "creates a skipped email log when the mail is skipped" do it "creates a skipped email log when the mail is skipped" do