diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb index 67f299eabda..0ef4811ea12 100644 --- a/app/jobs/regular/user_email.rb +++ b/app/jobs/regular/user_email.rb @@ -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) diff --git a/app/models/user.rb b/app/models/user.rb index e9f1ebbb856..0874f3cf672 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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") } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index f6fbe1eb82b..28c064ccff6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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}" diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index de200ec4c2a..13e0ce61c6c 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -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 diff --git a/spec/jobs/notify_mailing_list_subscribers_spec.rb b/spec/jobs/notify_mailing_list_subscribers_spec.rb index c7b9923b3c0..bd70b7d3d92 100644 --- a/spec/jobs/notify_mailing_list_subscribers_spec.rb +++ b/spec/jobs/notify_mailing_list_subscribers_spec.rb @@ -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) } diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb index a36fff4018b..d9ac680445e 100644 --- a/spec/jobs/user_email_spec.rb +++ b/spec/jobs/user_email_spec.rb @@ -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