FEATURE: Warn a user when they're replying to the same user too much

app/models/user_history.rb

@@ -61,7 +61,8 @@ class UserHistory < ActiveRecord::Base
                           activate_user: 43,
                           change_readonly_mode: 44,
                           backup_download: 45,
-                          backup_destroy: 46)
+                          backup_destroy: 46,
+                          notified_about_get_a_room: 47)
   end
 
   # Staff actions is a subset of all actions, used to audit actions taken by staff users.

config/locales/server.en.yml

@@ -316,6 +316,13 @@ en:
 
       Are you sure you're providing adequate time for other people to share their points of view, too?
 
+    get_a_room: |
+      ### You're replying a lot to the same person
+
+      You've replied %{count} times to that person in a row already.
+
+      Perhaps you're making things too personal, rather than allowing for a community discussion? You could try sending them a message instead.
+
     too_many_replies: |
       ### You have reached the reply limit for this topic
 
@@ -1359,6 +1366,8 @@ en:
 
     sequential_replies_threshold: "Number of posts a user has to make in a row in a topic before being reminded about too many sequential replies."
 
+    get_a_room_threshold: "Number of posts a user has to make to the same person in the same topic before being warned."
+
     enable_mobile_theme: "Mobile devices use a mobile-friendly theme, with the ability to switch to the full site. Disable this if you want to use a custom stylesheet that is fully responsive."
 
     dominating_topic_minimum_percent: "What percentage of posts a user has to make in a topic before being reminded about overly dominating a topic."

config/site_settings.yml

@@ -1212,6 +1212,7 @@ uncategorized:
   # Warnings
   educate_until_posts: 2
   sequential_replies_threshold: 2
+  get_a_room_threshold: 4
   dominating_topic_minimum_percent: 20
   disable_avatar_education_message: false
 

lib/composer_messages_finder.rb

@@ -6,13 +6,16 @@ class ComposerMessagesFinder
     @topic = Topic.find_by(id: details[:topic_id]) if details[:topic_id]
   end
 
+  def self.check_methods
+    @check_methods ||= instance_methods.find_all {|m| m =~ /^check\_/}
+  end
+
   def find
-    check_reviving_old_topic    ||
+    self.class.check_methods.each do |m|
-    check_education_message     ||
+      msg = send(m)
-    check_new_user_many_replies ||
+      return msg if msg.present?
-    check_avatar_notification   ||
+    end
-    check_sequential_replies    ||
+    nil
-    check_dominating_topic
   end
 
   # Determines whether to show the user education text
@@ -79,18 +82,7 @@ class ComposerMessagesFinder
 
   # Is a user replying too much in succession?
   def check_sequential_replies
-
+    return unless educate_reply?(:notified_about_sequential_replies)
-    # We only care about replies to topics
-    return unless replying? && @details[:topic_id] &&
-
-                  # And who have posted enough
-                  (@user.post_count >= SiteSetting.educate_until_posts) &&
-
-                  # And it's not a message
-                  (@topic.present? && !@topic.private_message?) &&
-
-                  # And who haven't been notified about sequential replies already
-                  !UserHistory.exists_for_user?(@user, :notified_about_sequential_replies, topic_id: @details[:topic_id])
 
     # Count the topics made by this user in the last day
     recent_posts_user_ids = Post.where(topic_id: @details[:topic_id])
@@ -118,12 +110,7 @@ class ComposerMessagesFinder
   end
 
   def check_dominating_topic
-
+    return unless educate_reply?(:notified_about_dominating_topic)
-    # We only care about replies to topics for a user who has posted enough
-    return unless replying? &&
-                  @details[:topic_id] &&
-                  (@user.post_count >= SiteSetting.educate_until_posts) &&
-                  !UserHistory.exists_for_user?(@user, :notified_about_dominating_topic, topic_id: @details[:topic_id])
 
     return if @topic.blank? ||
               @topic.user_id == @user.id ||
@@ -149,6 +136,37 @@ class ComposerMessagesFinder
     }
   end
 
+  def check_get_a_room
+    return unless educate_reply?(:notified_about_get_a_room)
+    return unless @details[:post_id].present?
+
+    reply_to_user_id = Post.where(id: @details[:post_id]).pluck(:user_id)[0]
+
+    # Users's last x posts in the topic
+    last_x_replies = @topic.
+      posts.
+      where(user_id: @user.id).
+      limit(SiteSetting.get_a_room_threshold).
+      pluck(:reply_to_user_id).
+      find_all {|uid| uid != @user.id && uid == reply_to_user_id}
+
+    return unless last_x_replies.size == SiteSetting.get_a_room_threshold
+
+    UserHistory.create!(action: UserHistory.actions[:notified_about_get_a_room],
+                        target_user_id: @user.id,
+                        topic_id: @details[:topic_id])
+
+    {
+      id: 'get_a_room',
+      templateName: 'education',
+      wait_for_typing: true,
+      extraClass: 'education-message',
+      body: PrettyText.cook(
+        I18n.t('education.get_a_room', count: SiteSetting.get_a_room_threshold)
+      )
+    }
+  end
+
   def check_reviving_old_topic
     return unless replying?
     return if @topic.nil? ||
@@ -167,6 +185,14 @@ class ComposerMessagesFinder
 
   private
 
+    def educate_reply?(type)
+      replying? &&
+      @details[:topic_id] &&
+      (@topic.present? && !@topic.private_message?) &&
+      (@user.post_count >= SiteSetting.educate_until_posts) &&
+      !UserHistory.exists_for_user?(@user, type, topic_id: @details[:topic_id])
+    end
+
     def creating_topic?
       @details[:composer_action] == "createTopic"
     end

spec/components/composer_messages_finder_spec.rb

@@ -15,6 +15,7 @@ describe ComposerMessagesFinder do
       finder.expects(:check_sequential_replies).once
       finder.expects(:check_dominating_topic).once
       finder.expects(:check_reviving_old_topic).once
+      finder.expects(:check_get_a_room).once
       finder.find
     end
 
@@ -197,7 +198,7 @@ describe ComposerMessagesFinder do
         expect(finder.check_sequential_replies).to be_blank
       end
 
-      it "doesn't notify in message" do
+      it "doesn't notify in a message" do
         Topic.any_instance.expects(:private_message?).returns(true)
         expect(finder.check_sequential_replies).to be_blank
       end
@@ -303,6 +304,111 @@ describe ComposerMessagesFinder do
 
   end
 
+  context '.check_get_a_room' do
+    let(:user) { Fabricate(:user) }
+    let(:other_user) { Fabricate(:user) }
+    let(:third_user) { Fabricate(:user) }
+    let(:topic) { Fabricate(:topic, user: other_user) }
+    let(:op) { Fabricate(:post, topic_id: topic.id, user: other_user) }
+
+    let!(:other_user_reply) {
+      Fabricate(:post, topic: topic, user: third_user, reply_to_user_id: op.user_id)
+    }
+
+    let!(:first_reply) {
+      Fabricate(:post, topic: topic, user: user, reply_to_user_id: op.user_id)
+    }
+
+    let!(:second_reply) {
+      Fabricate(:post, topic: topic, user: user, reply_to_user_id: op.user_id)
+    }
+
+    before do
+      SiteSetting.educate_until_posts = 10
+      user.stubs(:post_count).returns(11)
+      SiteSetting.get_a_room_threshold = 2
+    end
+
+    it "does not show the message for new topics" do
+      finder = ComposerMessagesFinder.new(user, composer_action: 'createTopic')
+      expect(finder.check_get_a_room).to be_blank
+    end
+
+    it "does not give a message without a topic id" do
+      expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_get_a_room).to be_blank
+    end
+
+    context "reply" do
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply', topic_id: topic.id, post_id: op.id) }
+
+      it "does not give a message to users who are still in the 'education' phase" do
+        user.stubs(:post_count).returns(9)
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "doesn't notify a user it has already notified about sequential replies" do
+        UserHistory.create!(
+          action: UserHistory.actions[:notified_about_get_a_room],
+          target_user_id: user.id,
+          topic_id: topic.id
+        )
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "will notify you if it hasn't in the current topic" do
+        UserHistory.create!(
+          action: UserHistory.actions[:notified_about_get_a_room],
+          target_user_id: user.id,
+          topic_id: topic.id+1
+        )
+        expect(finder.check_get_a_room).to be_present
+      end
+
+      it "won't notify you if you haven't had enough posts" do
+        SiteSetting.get_a_room_threshold = 10
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "doesn't notify you if the posts aren't all to the same person" do
+        first_reply.update_column(:reply_to_user_id, user.id)
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "doesn't notify you of posts to yourself" do
+        first_reply.update_column(:reply_to_user_id, user.id)
+        second_reply.update_column(:reply_to_user_id, user.id)
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "doesn't notify in a message" do
+        Topic.any_instance.expects(:private_message?).returns(true)
+        expect(finder.check_get_a_room).to be_blank
+      end
+
+      it "doesn't notify when replying to a different user" do
+        other_finder = ComposerMessagesFinder.new(
+          user,
+          composer_action: 'reply',
+          topic_id: topic.id,
+          post_id: other_user_reply.id
+        )
+
+        expect(other_finder.check_get_a_room).to be_blank
+      end
+
+      context "success" do
+        let!(:message) { finder.check_get_a_room }
+
+        it "works as expected" do
+          expect(message).to be_present
+          expect(UserHistory.exists_for_user?(user, :notified_about_get_a_room)).to eq(true)
+        end
+      end
+    end
+
+  end
+
+
   context '.check_reviving_old_topic' do
     let(:user)  { Fabricate(:user) }
     let(:topic) { Fabricate(:topic) }