mirror of
https://github.com/discourse/discourse.git
synced 2025-03-03 01:39:19 +00:00
FIX: send activity summaries based on "last seen" (#27035)
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
Improvement over 95885645d9
This commit is contained in:
parent
3d4d21693b
commit
958437e7dd
@ -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
|
||||
|
@ -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 * ?)",
|
||||
|
@ -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
|
||||
|
@ -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?
|
||||
|
@ -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] %>
|
||||
|
@ -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: ""
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
Loading…
x
Reference in New Issue
Block a user