FIX: don't send emails to anonymous users

Also changes behaviour of real to not return anonymous users.

This means user counts will no longer include them, and the
mailing list system will ignore them even if they somehow end up
with the feature turned on.
This commit is contained in:
Luke Granger-Brown 2015-05-11 00:10:10 +01:00
parent 5329403a71
commit 9f9825bb6b
6 changed files with 73 additions and 2 deletions

View File

@ -16,6 +16,7 @@ module Jobs
# Find the user
@user = User.find_by(id: args[:user_id])
return skip(I18n.t("email_log.no_user", user_id: args[:user_id])) unless @user
return skip(I18n.t("email_log.anonymous_user")) if @user.anonymous?
return skip(I18n.t("email_log.suspended_not_pm")) if @user.suspended? && args[:type] != :user_private_message
seen_recently = (@user.last_seen_at.present? && @user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago)

View File

@ -108,8 +108,15 @@ class User < ActiveRecord::Base
# set to true to optimize creation and save for imports
attr_accessor :import_mode
# excluding fake users like the system user
scope :real, -> { where('id > 0') }
# excluding fake users like the system user or anonymous users
scope :real, -> { where('id > 0').where('NOT EXISTS(
SELECT 1
FROM user_custom_fields ucf
WHERE
ucf.user_id = users.id AND
ucf.name = ? AND
ucf.value::int > 0
)', 'master_id') }
scope :staff, -> { where("admin OR moderator") }

View File

@ -2006,6 +2006,7 @@ en:
email_log:
no_user: "Can't find user with id %{user_id}"
anonymous_user: "User is anonymous"
suspended_not_pm: "User is suspended, not a message"
seen_recently: "User was seen recently"
post_not_found: "Can't find a post with id %{post_id}"

View File

@ -86,3 +86,16 @@ Fabricator(:trust_level_4, from: :user) do
email { sequence(:email) { |i| "tl4#{i}@elderfun.com" } }
trust_level TrustLevel[4]
end
Fabricator(:anonymous, from: :user) do
name ''
username { sequence(:username) { |i| "anonymous#{i}" } }
email { sequence(:email) { |i| "anonymous#{i}@anonymous.com" } }
trust_level TrustLevel[1]
trust_level_locked true
after_create do |user|
user.custom_fields["master_id"] = 1
user.save!
end
end

View File

@ -47,6 +47,16 @@ describe Jobs::NotifyMailingListSubscribers do
end
context "to an anonymous user with mailing list on" do
let(:user) { Fabricate(:anonymous, mailing_list_mode: true) }
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
context "with mailing list off" do
let(:user) { Fabricate(:user, mailing_list_mode: false) }
let!(:post) { Fabricate(:post, user: user) }

View File

@ -9,6 +9,7 @@ describe Jobs::UserEmail do
let(:user) { Fabricate(:user, last_seen_at: 11.minutes.ago ) }
let(:suspended) { Fabricate(:user, last_seen_at: 10.minutes.ago, suspended_at: 5.minutes.ago, suspended_till: 7.days.from_now ) }
let(:anonymous) { Fabricate(:anonymous, last_seen_at: 11.minutes.ago ) }
let(:mailer) { Mail::Message.new(to: user.email) }
it "raises an error when there is no user" do
@ -96,6 +97,22 @@ describe Jobs::UserEmail do
Jobs::UserEmail.new.execute(type: :private_message, user_id: suspended.id, post_id: pm_from_staff.id)
end
end
context 'user is anonymous' do
before { SiteSetting.stubs(:allow_anonymous_posting).returns(true) }
it "doesn't send email for a pm from a regular user" do
Email::Sender.any_instance.expects(:send).never
Jobs::UserEmail.new.execute(type: :private_message, user_id: anonymous.id, post_id: post.id)
end
it "doesn't send email for a pm from a staff user" do
pm_from_staff = Fabricate(:post, user: Fabricate(:moderator))
pm_from_staff.topic.topic_allowed_users.create!(user_id: anonymous.id)
Email::Sender.any_instance.expects(:send).never
Jobs::UserEmail.new.execute(type: :private_message, user_id: anonymous.id, post_id: pm_from_staff.id)
end
end
end
@ -169,6 +186,28 @@ describe Jobs::UserEmail do
end
end
end
context 'user is anonymous' do
before { SiteSetting.stubs(:allow_anonymous_posting).returns(true) }
it "doesn't send email for a pm from a regular user" do
Email::Sender.any_instance.expects(:send).never
Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, notification_id: notification.id)
end
it "doesn't send email for a pm from staff" do
pm_from_staff = Fabricate(:post, user: Fabricate(:moderator))
pm_from_staff.topic.topic_allowed_users.create!(user_id: anonymous.id)
pm_notification = Fabricate(:notification,
user: anonymous,
topic: pm_from_staff.topic,
post_number: pm_from_staff.post_number,
data: { original_post_id: pm_from_staff.id }.to_json
)
Email::Sender.any_instance.expects(:send).never
Jobs::UserEmail.new.execute(type: :user_private_message, user_id: anonymous.id, notification_id: pm_notification.id)
end
end
end
end