FIX: don't send emails from muted users in mailing list mode

This commit is contained in:
Régis Hanol 2016-12-12 15:28:26 +01:00
parent 405ffbf5a4
commit 664feca199
3 changed files with 164 additions and 149 deletions

View File

@ -11,27 +11,26 @@ module Jobs
post = post_id ? Post.with_deleted.find_by(id: post_id) : nil
raise Discourse::InvalidParameters.new(:post_id) unless post
return if post.trashed? || post.user_deleted? || (!post.topic)
return if post.trashed? || post.user_deleted? || !post.topic
users =
User.activated.not_blocked.not_suspended.real
.joins(:user_option)
.where('user_options.mailing_list_mode AND user_options.mailing_list_mode_frequency > 0')
.where('NOT EXISTS(
.where('NOT EXISTS (
SELECT 1
FROM muted_users mu
WHERE mu.muted_user_id = ? AND mu.user_id = users.id
)', post.user_id)
.where('NOT EXISTS (
SELECT 1
FROM topic_users tu
WHERE
tu.topic_id = ? AND
tu.user_id = users.id AND
tu.notification_level = ?
WHERE tu.topic_id = ? AND tu.user_id = users.id AND tu.notification_level = ?
)', post.topic_id, TopicUser.notification_levels[:muted])
.where('NOT EXISTS(
.where('NOT EXISTS (
SELECT 1
FROM category_users cu
WHERE
cu.category_id = ? AND
cu.user_id = users.id AND
cu.notification_level = ?
WHERE cu.category_id = ? AND cu.user_id = users.id AND cu.notification_level = ?
)', post.topic.category_id, CategoryUser.notification_levels[:muted])
users.each do |user|

View File

@ -13,9 +13,11 @@ module Jobs
def target_user_ids
# Users who want to receive daily mailing list emails
User.real
.activated
.not_suspended
.not_blocked
.joins(:user_option)
.where(active: true, staged: false, user_options: {mailing_list_mode: true, mailing_list_mode_frequency: 0})
.where(staged: false, user_options: { mailing_list_mode: true, mailing_list_mode_frequency: 0 })
.where("#{!SiteSetting.must_approve_users?} OR approved OR moderator OR admin")
.where("date_part('hour', first_seen_at) = date_part('hour', CURRENT_TIMESTAMP)") # where the hour of first_seen_at is the same as the current hour
.where("COALESCE(first_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - '23 HOURS'::INTERVAL") # don't send unless you've been around for a day already

View File

@ -2,148 +2,162 @@ require "rails_helper"
describe Jobs::NotifyMailingListSubscribers do
context "with mailing list on" do
let(:mailing_list_user) { Fabricate(:user) }
let(:user) { Fabricate(:user) }
before { mailing_list_user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1) }
before do
SiteSetting.default_email_mailing_list_mode = true
SiteSetting.default_email_mailing_list_mode_frequency = 1
end
let(:user) { Fabricate(:user) }
let(:post) { Fabricate(:post, user: user) }
context "SiteSetting.max_emails_per_day_per_user" do
it 'stops sending mail once limit is reached' do
SiteSetting.max_emails_per_day_per_user = 2
post = Fabricate(:post)
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
user.email_logs.create(email_type: 'blah', to_address: user.email, user_id: user.id)
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
end
end
context "SiteSetting.bounce_score_threshold" do
it "stops sending mail once bounce threshold is reached" do
user.user_stat.update_columns(bounce_score: SiteSetting.bounce_score_threshold + 1)
post = Fabricate(:post)
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
expect(EmailLog.where(user_id: user.id, skipped: true).count).to eq(1)
end
end
context "totally skipped if mailing list mode disabled" do
it "sends no email to the user" do
SiteSetting.disable_mailing_list_mode = true
post = Fabricate(:post)
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
expect(EmailLog.count).to eq(0)
end
end
context "with a valid post authored by same user" do
let!(:post) { Fabricate(:post, user: user) }
it "doesn't send the email to the user if the frequency is set to 'always with no echo'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2)
UserNotifications.expects(:mailing_list_notify).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "sends the email to the user if the frequency is set to 'always'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1)
UserNotifications.expects(:mailing_list_notify).with(user, post).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "does not send the email to the user if the frequency is set to 'daily'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
UserNotifications.expects(:mailing_list_notify).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "with a valid post created by someone other than the user" do
let!(:post) { Fabricate(:post, user: user, user_id: 'different') }
it "sends the email to the user if the frequency is set to 'always with no echo'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 2)
UserNotifications.expects(:mailing_list_notify).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "sends the email to the user if the frequency is set to 'always'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 1)
UserNotifications.expects(:mailing_list_notify).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
it "does not send the email to the user if the frequency is set to 'daily'" do
user.user_option.update(mailing_list_mode: true, mailing_list_mode_frequency: 0)
UserNotifications.expects(:mailing_list_notify).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "with a deleted post" do
let!(:post) { Fabricate(:post, user: user, deleted_at: Time.now) }
it "doesn't send the email to the user" do
UserNotifications.expects(:mailing_list_notify).with(user, post).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "with a user_deleted post" do
let!(:post) { Fabricate(:post, user: user, user_deleted: true) }
it "doesn't send the email to the user" do
UserNotifications.expects(:mailing_list_notify).with(user, post).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "with a deleted topic" do
let!(:post) { Fabricate(:post, user: user) }
before do
post.topic.update_column(:deleted_at, Time.now)
end
it "doesn't send the email to the user" do
UserNotifications.expects(:mailing_list_notify).with(user, post).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "to an anonymous user" do
let(:user) { Fabricate(:anonymous) }
let!(:post) { Fabricate(:post, user: user) }
it "doesn't send the email to the user" do
UserNotifications.expects(:mailing_list_notify).with(user, post).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
end
context "with mailing list off" do
before { SiteSetting.stubs(:default_email_mailing_list_mode).returns(false) }
let(:user) { Fabricate(:user) }
let!(:post) { Fabricate(:post, user: user) }
it "doesn't send the email to the user" do
UserNotifications.expects(:mailing_list_notify).never
shared_examples "no emails" do
it "doesn't send any emails" do
UserNotifications.expects(:mailing_list_notify).with(mailing_list_user, post).never
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
shared_examples "one email" do
it "sends the email" do
UserNotifications.expects(:mailing_list_notify).with(mailing_list_user, post).once
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
end
end
context "when mailing list mode is globally disabled" do
before { SiteSetting.disable_mailing_list_mode = true }
include_examples "no emails"
end
context "when mailing list mode is globally enabled" do
before { SiteSetting.disable_mailing_list_mode = false }
context "with an invalid post_id" do
it "throws an error" do
expect { Jobs::NotifyMailingListSubscribers.new.execute(post_id: -1) }.to raise_error(Discourse::InvalidParameters)
end
end
context "with a deleted post" do
before { post.update(deleted_at: Time.now) }
include_examples "no emails"
end
context "with a user_deleted post" do
before { post.update(user_deleted: true) }
include_examples "no emails"
end
context "with a deleted topic" do
before { post.topic.update(deleted_at: Time.now) }
include_examples "no emails"
end
context "with a valid post from another user" do
context "to an inactive user" do
before { mailing_list_user.update(active: false) }
include_examples "no emails"
end
context "to a blocked user" do
before { mailing_list_user.update(blocked: true) }
include_examples "no emails"
end
context "to a suspended user" do
before { mailing_list_user.update(suspended_till: 1.day.from_now) }
include_examples "no emails"
end
context "to an anonymous user" do
let(:mailing_list_user) { Fabricate(:anonymous) }
include_examples "no emails"
end
context "to an user who has disabled mailing list mode" do
before { mailing_list_user.user_option.update(mailing_list_mode: false) }
include_examples "no emails"
end
context "to an user who has frequency set to 'daily'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 0) }
include_examples "no emails"
end
context "to an user who has frequency set to 'always'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 1) }
include_examples "one email"
end
context "to an user who has frequency set to 'no echo'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 2) }
include_examples "one email"
end
context "from a muted user" do
before { MutedUser.create(user: mailing_list_user, muted_user: user) }
include_examples "no emails"
end
context "from a muted topic" do
before { TopicUser.create(user: mailing_list_user, topic: post.topic, notification_level: TopicUser.notification_levels[:muted]) }
include_examples "no emails"
end
context "from a muted category" do
before { CategoryUser.create(user: mailing_list_user, category: post.topic.category, notification_level: CategoryUser.notification_levels[:muted]) }
include_examples "no emails"
end
context "max emails per day was reached" do
before { SiteSetting.max_emails_per_day_per_user = 2 }
it "doesn't send any emails" do
(SiteSetting.max_emails_per_day_per_user + 1).times {
mailing_list_user.email_logs.create(email_type: 'foobar', to_address: mailing_list_user.email)
}
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
UserNotifications.expects(:mailing_list_notify).with(mailing_list_user, post).never
expect(EmailLog.where(user: mailing_list_user, skipped: true).count).to eq(1)
end
end
context "bounce score was reached" do
it "doesn't send any emails" do
mailing_list_user.user_stat.update(bounce_score: SiteSetting.bounce_score_threshold + 1)
Jobs::NotifyMailingListSubscribers.new.execute(post_id: post.id)
UserNotifications.expects(:mailing_list_notify).with(mailing_list_user, post).never
expect(EmailLog.where(user: mailing_list_user, skipped: true).count).to eq(1)
end
end
end
context "with a valid post from same user" do
let(:post) { Fabricate(:post, user: mailing_list_user) }
context "to an user who has frequency set to 'daily'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 0) }
include_examples "no emails"
end
context "to an user who has frequency set to 'always'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 1) }
include_examples "one email"
end
context "to an user who has frequency set to 'no echo'" do
before { mailing_list_user.user_option.update(mailing_list_mode_frequency: 2) }
include_examples "no emails"
end
end
end
end