Merge pull request #3445 from lukegb/bug/anonymous-emails

Don't send emails to anonymous users

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)

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") }
 

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}"

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

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) }

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