From 8cc49d9797722cce4af400838bafef1fcb714d6d Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 2 Aug 2013 10:55:02 -0400
Subject: [PATCH] Don't send more pm's after a user has been blocked from
 making posts. This can happen if a staff member flags posts that are already
 hidden.

---
 app/services/spam_rules_enforcer.rb       |  5 +--
 app/services/user_blocker.rb              | 10 ++++--
 spec/services/spam_rules_enforcer_spec.rb | 42 +++++++++++++++--------
 spec/services/user_blocker_spec.rb        |  7 ++++
 4 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/app/services/spam_rules_enforcer.rb b/app/services/spam_rules_enforcer.rb
index e48b7853b06..abc78bd5620 100644
--- a/app/services/spam_rules_enforcer.rb
+++ b/app/services/spam_rules_enforcer.rb
@@ -48,8 +48,9 @@ class SpamRulesEnforcer
 
   def punish_user
     Post.transaction do
-      UserBlocker.block(@user, nil, {message: :too_many_spam_flags})
-      GroupMessage.create(Group[:moderators].name, :user_automatically_blocked, {user: @user, limit_once_per: false})
+      if UserBlocker.block(@user, nil, {message: :too_many_spam_flags})
+        GroupMessage.create(Group[:moderators].name, :user_automatically_blocked, {user: @user, limit_once_per: false})
+      end
     end
   end
 
diff --git a/app/services/user_blocker.rb b/app/services/user_blocker.rb
index 355952ea9ee..6106080e093 100644
--- a/app/services/user_blocker.rb
+++ b/app/services/user_blocker.rb
@@ -14,9 +14,13 @@ class UserBlocker
 
   def block
     hide_posts
-    @user.blocked = true
-    if @user.save
-      SystemMessage.create(@user, @opts[:message] || :blocked_by_staff)
+    unless @user.blocked?
+      @user.blocked = true
+      if @user.save
+        SystemMessage.create(@user, @opts[:message] || :blocked_by_staff)
+      end
+    else
+      false
     end
   end
 
diff --git a/spec/services/spam_rules_enforcer_spec.rb b/spec/services/spam_rules_enforcer_spec.rb
index ff410cdad64..92e373b129f 100644
--- a/spec/services/spam_rules_enforcer_spec.rb
+++ b/spec/services/spam_rules_enforcer_spec.rb
@@ -111,22 +111,34 @@ describe SpamRulesEnforcer do
       subject.stubs(:block?).returns(true)
     end
 
-    it "blocks the user" do
-      UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags))
-      subject.punish_user
-    end
-
-    it 'prevents the user from making new posts' do
-      subject.punish_user
-      expect(Guardian.new(user).can_create_post?(nil)).to be_false
-    end
-
-    it 'sends private message to moderators' do
-      moderator = Fabricate(:moderator)
-      GroupMessage.expects(:create).with do |group, msg_type, params|
-        group == Group[:moderators].name and msg_type == :user_automatically_blocked and params[:user].id == user.id
+    context 'user is not blocked' do
+      before do
+        UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(true)
+      end
+
+      it 'prevents the user from making new posts' do
+        subject.punish_user
+        expect(Guardian.new(user).can_create_post?(nil)).to be_false
+      end
+
+      it 'sends private message to moderators' do
+        moderator = Fabricate(:moderator)
+        GroupMessage.expects(:create).with do |group, msg_type, params|
+          group == Group[:moderators].name and msg_type == :user_automatically_blocked and params[:user].id == user.id
+        end
+        subject.punish_user
+      end
+    end
+
+    context 'user is already blocked' do
+      before do
+        UserBlocker.expects(:block).with(user, nil, has_entries(message: :too_many_spam_flags)).returns(false)
+      end
+
+      it "doesn't send a pm to moderators if the user is already blocked" do
+        GroupMessage.expects(:create).never
+        subject.punish_user
       end
-      subject.punish_user
     end
   end
 
diff --git a/spec/services/user_blocker_spec.rb b/spec/services/user_blocker_spec.rb
index 232b2b380d2..52c87c43459 100644
--- a/spec/services/user_blocker_spec.rb
+++ b/spec/services/user_blocker_spec.rb
@@ -53,6 +53,13 @@ describe UserBlocker do
       SystemMessage.expects(:create).never
       block_user
     end
+
+    it "doesn't send a pm if the user is already blocked" do
+      user.stubs(:blocked?).returns(true)
+      SystemMessage.unstub(:create)
+      SystemMessage.expects(:create).never
+      block_user.should == false
+    end
   end
 
   describe 'unblock' do