Send a message to moderators when a newuser_spam_host_threshold is exceeded. Send it no more than once per day per user.

app/services/group_message.rb

@@ -0,0 +1,66 @@
+# GroupMessage sends a private message to a group.
+# It will also avoid sending the same message repeatedly, which can happen with
+# notifications to moderators when spam is detected.
+#
+# Options:
+#
+#   user: (User) If the message is about a user, pass the user object.
+#   limit_once_per: (seconds) Limit sending the given type of message once every X seconds.
+#                   The default is 24 hours. Set to false to always send the message.
+
+require_dependency 'post_creator'
+require_dependency 'topic_subtype'
+require_dependency 'discourse'
+
+class GroupMessage
+
+  include Rails.application.routes.url_helpers
+
+  def self.create(group_name, message_type, opts={})
+    GroupMessage.new(group_name, message_type, opts).create
+  end
+
+  def initialize(group_name, message_type, opts={})
+    @group_name = group_name
+    @message_type = message_type
+    @opts = opts
+  end
+
+  def create
+    unless sent_recently?
+      post = PostCreator.create( Discourse.system_user,
+                          target_group_names: [@group_name],
+                          archetype: Archetype.private_message,
+                          subtype: TopicSubtype.system_message,
+                          title: I18n.t("system_messages.#{@message_type}.subject_template", message_params),
+                          raw: I18n.t("system_messages.#{@message_type}.text_body_template", message_params) )
+      remember_message_sent
+      post
+    else
+      false
+    end
+  end
+
+  def message_params
+    @message_params ||= {
+      username: @opts[:user].username,
+      user_url: admin_user_path(@opts[:user].username)
+    }
+  end
+
+  def sent_recently?
+    return false if @opts[:limit_once_per] == false
+    $redis.get(sent_recently_key).present?
+  end
+
+  # default is to send no more than once every 24 hours (24 * 60 * 60 = 86,400 seconds)
+  def remember_message_sent
+    $redis.setex(sent_recently_key, @opts[:limit_once_per].try(:to_i) || 86_400, 1) unless @opts[:limit_once_per] == false
+  end
+
+  private
+
+    def sent_recently_key
+      "grpmsg:#{@group_name}:#{@message_type}:#{@opts[:user] ? @opts[:user].username : ''}"
+    end
+end

app/services/spam_rules_enforcer.rb

@@ -53,7 +53,7 @@ class SpamRulesEnforcer
   def punish_user
     Post.update_all(["hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)", Post.hidden_reasons[:new_user_spam_threshold_reached]], user_id: @user.id)
     SystemMessage.create(@user, :too_many_spam_flags)
-    notify_moderators
+    GroupMessage.create(Group[:moderators].name, :user_automatically_blocked, {user: @user, limit_once_per: false})
     @user.blocked = true
     @user.save
   end
@@ -64,18 +64,4 @@ class SpamRulesEnforcer
     @user.save
   end
 
-
-  private
-
-    def notify_moderators
-      title = I18n.t("system_messages.user_automatically_blocked.subject_template", {username: @user.username})
-      raw_body = I18n.t("system_messages.user_automatically_blocked.text_body_template", {username: @user.username, blocked_user_url: admin_user_path(@user.username)})
-      PostCreator.create( Discourse.system_user,
-                          target_group_names: [Group[:moderators].name],
-                          archetype: Archetype.private_message,
-                          subtype: TopicSubtype.system_message,
-                          title: title,
-                          raw: raw_body )
-    end
-
 end

config/locales/server.en.yml

@@ -858,9 +858,16 @@ en:
     user_automatically_blocked:
       subject_template: "User %{username} was automatically blocked"
       text_body_template: |
-        This is an automated message to inform you that [%{username}](%{blocked_user_url}) has been automatically blocked because too many people submitted flags against %{username}'s post(s).
+        This is an automated message to inform you that [%{username}](%{user_url}) has been automatically blocked because too many people submitted flags against %{username}'s post(s).
 
-        Please [review the flags](/admin/flags). If %{username} should not be blocked from posting, click the unblock button on [the admin user page](%{blocked_user_url}).
+        Please [review the flags](/admin/flags). If %{username} should not be blocked from posting, click the unblock button on [the admin user page](%{user_url}).
+
+    spam_post_blocked:
+      subject_template: "Spam was detected in a post by %{username}"
+      text_body_template: |
+        This is an automated message to inform you that [%{username}](%{user_url}) tried to make a post with links, but it was stopped as spam based on the newuser_spam_host_threshold site setting.
+
+        Please [review the user](%{user_url}).
 
     unblocked:
       subject_template: "Account unblocked"

lib/post_creator.rb

@@ -69,6 +69,10 @@ class PostCreator
       @post.save_reply_relationships
     end
 
+    if @spam
+      GroupMessage.create( Group[:moderators].name, :spam_post_blocked, {user: @user, limit_once_per: 24.hours} )
+    end
+
     enqueue_jobs
     @post
   end
@@ -261,7 +265,6 @@ class PostCreator
 
   def enqueue_jobs
     if @post && !@post.errors.present?
-
       # We need to enqueue jobs after the transaction. Otherwise they might begin before the data has
       # been comitted.
       topic_id = @opts[:topic_id] || @topic.try(:id)

spec/components/post_creator_spec.rb

@@ -216,11 +216,19 @@ describe PostCreator do
     end
 
     it "does not create the post" do
+      GroupMessage.stubs(:create)
       creator.create
       creator.errors.should be_present
       creator.spam?.should be_true
     end
 
+    it "sends a message to moderators" do
+      GroupMessage.expects(:create).with do |group_name, msg_type, params|
+        group_name == Group[:moderators].name and msg_type == :spam_post_blocked and params[:user].id == user.id
+      end
+      creator.create
+    end
+
   end
 
   # more integration testing ... maximise our testing

spec/services/group_message_spec.rb

@@ -0,0 +1,119 @@
+require 'spec_helper'
+
+describe GroupMessage do
+
+  let(:moderators_group) { Group[:moderators].name }
+
+  let!(:admin)     { Fabricate.build(:admin, id: 999) }
+  let!(:user)      { Fabricate.build(:user, id: 111) }
+
+  before do
+    Discourse.stubs(:system_user).returns(admin)
+  end
+
+  subject(:send_group_message) { GroupMessage.create(moderators_group, :user_automatically_blocked, {user: user}) }
+
+  describe 'not sent recently' do
+    before { GroupMessage.any_instance.stubs(:sent_recently?).returns(false) }
+
+    it 'should send a private message to the given group' do
+      PostCreator.expects(:create).with do |from_user, opts|
+          from_user.id == admin.id and
+            opts[:target_group_names] and opts[:target_group_names].include?(Group[:moderators].name) and
+            opts[:archetype] == Archetype.private_message and
+            opts[:title].present? and
+            opts[:raw].present?
+        end.returns(stub_everything)
+      send_group_message
+    end
+
+    it 'returns whatever PostCreator returns' do
+      the_output = stub_everything
+      PostCreator.stubs(:create).returns(the_output)
+      expect(send_group_message).to eq(the_output)
+    end
+
+    it "remembers that it was sent so it doesn't spam the group with the same message" do
+      PostCreator.stubs(:create).returns(stub_everything)
+      GroupMessage.any_instance.expects(:remember_message_sent)
+      send_group_message
+    end
+  end
+
+  describe 'sent recently' do
+    before  { GroupMessage.any_instance.stubs(:sent_recently?).returns(true) }
+    subject { GroupMessage.create(moderators_group, :user_automatically_blocked, {user: user}) }
+
+    it { should eq(false) }
+
+    it 'should not send the same notification again' do
+      PostCreator.expects(:create).never
+      subject
+    end
+  end
+
+  describe 'message_params' do
+    let(:user) { Fabricate.build(:user, id: 123123) }
+    shared_examples 'common message params for group messages' do
+      it 'returns the correct params' do
+        expect(subject[:username]).to eq(user.username)
+        expect(subject[:user_url]).to be_present
+      end
+    end
+
+    context 'user_automatically_blocked' do
+      subject { GroupMessage.new(moderators_group, :user_automatically_blocked, {user: user}).message_params }
+      include_examples 'common message params for group messages'
+    end
+
+    context 'spam_post_blocked' do
+      subject { GroupMessage.new(moderators_group, :spam_post_blocked, {user: user}).message_params }
+      include_examples 'common message params for group messages'
+    end
+  end
+
+  describe 'methods that use redis' do
+    let(:user)              { Fabricate.build(:user, id: 123123) }
+    subject(:group_message) { GroupMessage.new(moderators_group, :user_automatically_blocked, {user: user}) }
+    before do
+      PostCreator.stubs(:create).returns(stub_everything)
+      group_message.stubs(:sent_recently_key).returns('the_key')
+    end
+
+    describe 'sent_recently?' do
+      it 'returns true if redis says so' do
+        $redis.stubs(:get).with(group_message.sent_recently_key).returns('1')
+        expect(group_message.sent_recently?).to be_true
+      end
+
+      it 'returns false if redis returns nil' do
+        $redis.stubs(:get).with(group_message.sent_recently_key).returns(nil)
+        expect(group_message.sent_recently?).to be_false
+      end
+
+      it 'always returns false if limit_once_per is false' do
+        gm = GroupMessage.new(moderators_group, :user_automatically_blocked, {user: user, limit_once_per: false})
+        gm.stubs(:sent_recently_key).returns('the_key')
+        $redis.stubs(:get).with(gm.sent_recently_key).returns('1')
+        expect(gm.sent_recently?).to be_false
+      end
+    end
+
+    describe 'remember_message_sent' do
+      it 'stores a key in redis that expires after 24 hours' do
+        $redis.expects(:setex).with(group_message.sent_recently_key, 24 * 60 * 60, anything).returns('OK')
+        group_message.remember_message_sent
+      end
+
+      it 'can use a given expiry time' do
+        $redis.expects(:setex).with(anything, 30 * 60, anything).returns('OK')
+        GroupMessage.new(moderators_group, :user_automatically_blocked, {user: user, limit_once_per: 30.minutes}).remember_message_sent
+      end
+
+      it 'can be disabled' do
+        $redis.expects(:setex).never
+        GroupMessage.new(moderators_group, :user_automatically_blocked, {user: user, limit_once_per: false}).remember_message_sent
+      end
+    end
+  end
+end

spec/services/spam_rules_enforcer_spec.rb

@@ -120,11 +120,9 @@ describe SpamRulesEnforcer do
     it 'sends private messages to the user and to moderators' do
       SystemMessage.expects(:create).with(user, anything, anything)
       moderator = Fabricate(:moderator)
-      PostCreator.expects(:create).with do |from_user, opts|
-        from_user.id == admin.id &&
-          opts[:target_group_names] && opts[:target_group_names].include?(Group[:moderators].name) &&
-          opts[:archetype] == Archetype.private_message
-      end.returns(stub_everything)
+      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