From f44bd4ec2890e392761576b1e5df1c3afa5e7075 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Tue, 6 May 2014 15:01:19 -0400
Subject: [PATCH] Don't allow sending private messages to suspended users.
 Emails to suspended users should tell them how to respond, since they can't.

---
 app/mailers/user_notifications.rb       |  3 ++-
 lib/email/message_builder.rb            | 18 +++++++++++-------
 lib/guardian.rb                         |  4 +++-
 spec/components/guardian_spec.rb        | 12 ++++++++++++
 spec/mailers/user_notifications_spec.rb |  9 +++++++++
 5 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb
index 2cd39794a..ab71e381f 100644
--- a/app/mailers/user_notifications.rb
+++ b/app/mailers/user_notifications.rb
@@ -157,7 +157,7 @@ class UserNotifications < ActionMailer::Base
        ["replied", "mentioned", "quoted", "posted"].include?(notification_type)
 
     title = @notification.data_hash[:topic_title]
-    allow_reply_by_email = opts[:allow_reply_by_email]
+    allow_reply_by_email = opts[:allow_reply_by_email] unless user.suspended?
 
     send_notification_email(
       title: title,
@@ -213,6 +213,7 @@ class UserNotifications < ActionMailer::Base
       username: from_alias,
       add_unsubscribe_link: true,
       allow_reply_by_email: allow_reply_by_email,
+      include_respond_instructions: !user.suspended?,
       template: template,
       html_override: html,
       style: :notification
diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index 172e69d90..130d3a7cf 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -1,4 +1,4 @@
-# Builds a Mail::Mesage we can use for sending. Optionally supports using a template
+# Builds a Mail::Message we can use for sending. Optionally supports using a template
 # for the body and subject
 module Email
 
@@ -26,12 +26,16 @@ module Email
                         user_preferences_url: "#{Discourse.base_url}/my/preferences" }.merge!(@opts)
 
       if @template_args[:url].present?
-        @template_args[:respond_instructions] =
-          if allow_reply_by_email?
-            I18n.t('user_notifications.reply_by_email', @template_args)
-          else
-            I18n.t('user_notifications.visit_link_to_respond', @template_args)
-          end
+        if @opts[:include_respond_instructions] == false
+          @template_args[:respond_instructions] = ''
+        else
+          @template_args[:respond_instructions] =
+            if allow_reply_by_email?
+              I18n.t('user_notifications.reply_by_email', @template_args)
+            else
+              I18n.t('user_notifications.visit_link_to_respond', @template_args)
+            end
+        end
       end
     end
 
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 607e79dee..9ee8cb409 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -215,7 +215,9 @@ class Guardian
     # PMs are enabled
     (SiteSetting.enable_private_messages ||
       @user.username == SiteSetting.site_contact_username ||
-      @user == Discourse.system_user)
+      @user == Discourse.system_user) &&
+    # Can't send PMs to suspended users
+    (is_staff? || target.is_a?(Group) || !target.suspended?)
   end
 
   private
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index d2a83480b..0a89456be 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -107,6 +107,7 @@ describe Guardian do
   describe 'can_send_private_message' do
     let(:user) { Fabricate(:user) }
     let(:another_user) { Fabricate(:user) }
+    let(:suspended_user) { Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) }
 
     it "returns false when the user is nil" do
       Guardian.new(nil).can_send_private_message?(user).should be_false
@@ -142,6 +143,17 @@ describe Guardian do
         Guardian.new(Discourse.system_user).can_send_private_message?(another_user).should be_true
       end
     end
+
+    context "target user is suspended" do
+      it "returns true for staff" do
+        Guardian.new(admin).can_send_private_message?(suspended_user).should be_true
+        Guardian.new(moderator).can_send_private_message?(suspended_user).should be_true
+      end
+
+      it "returns false for regular users" do
+        Guardian.new(user).can_send_private_message?(suspended_user).should be_false
+      end
+    end
   end
 
   describe 'can_reply_as_new_topic' do
diff --git a/spec/mailers/user_notifications_spec.rb b/spec/mailers/user_notifications_spec.rb
index bcb5b29f6..d7bb4b458 100644
--- a/spec/mailers/user_notifications_spec.rb
+++ b/spec/mailers/user_notifications_spec.rb
@@ -176,6 +176,15 @@ describe UserNotifications do
       it "has a from alias" do
         expects_build_with(has_entry(:from_alias, "#{username}"))
       end
+
+      it "should explain how to respond" do
+        expects_build_with(Not(has_entry(:include_respond_instructions, false)))
+      end
+
+      it "should not explain how to respond if the user is suspended" do
+        User.any_instance.stubs(:suspended?).returns(true)
+        expects_build_with(has_entry(:include_respond_instructions, false))
+      end
     end
   end