don't notify users about sequential replies in messages

lib/composer_messages_finder.rb

@@ -3,14 +3,15 @@ class ComposerMessagesFinder
   def initialize(user, details)
     @user = user
     @details = details
+    @topic = Topic.find_by(id: details[:topic_id]) if details[:topic_id]
   end
 
   def find
-    check_reviving_old_topic ||
-    check_education_message ||
+    check_reviving_old_topic    ||
+    check_education_message     ||
     check_new_user_many_replies ||
-    check_avatar_notification ||
-    check_sequential_replies ||
+    check_avatar_notification   ||
+    check_sequential_replies    ||
     check_dominating_topic
   end
 
@@ -74,6 +75,9 @@ class ComposerMessagesFinder
                   # 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])
 
@@ -109,16 +113,14 @@ class ComposerMessagesFinder
                   (@user.post_count >= SiteSetting.educate_until_posts) &&
                   !UserHistory.exists_for_user?(@user, :notified_about_dominating_topic, topic_id: @details[:topic_id])
 
-    topic = Topic.find_by(id: @details[:topic_id])
+    return if @topic.blank? ||
+              @topic.user_id == @user.id ||
+              @topic.posts_count < SiteSetting.summary_posts_required ||
+              @topic.private_message?
 
-    return if topic.blank? ||
-              topic.user_id == @user.id ||
-              topic.posts_count < SiteSetting.summary_posts_required ||
-              topic.archetype == Archetype.private_message
+    posts_by_user = @user.posts.where(topic_id: @topic.id).count
 
-    posts_by_user = @user.posts.where(topic_id: topic.id).count
-
-    ratio = (posts_by_user.to_f / topic.posts_count.to_f)
+    ratio = (posts_by_user.to_f / @topic.posts_count.to_f)
     return if ratio < (SiteSetting.dominating_topic_minimum_percent.to_f / 100.0)
 
     # Log the topic notification
@@ -135,22 +137,17 @@ class ComposerMessagesFinder
   end
 
   def check_reviving_old_topic
-    return unless @details[:topic_id]
-
-    topic = Topic.find_by(id: @details[:topic_id])
-
     return unless replying?
-
-    return if topic.nil? ||
+    return if @topic.nil? ||
               SiteSetting.warn_reviving_old_topic_age < 1 ||
-              topic.last_posted_at.nil? ||
-              topic.last_posted_at > SiteSetting.warn_reviving_old_topic_age.days.ago
+              @topic.last_posted_at.nil? ||
+              @topic.last_posted_at > SiteSetting.warn_reviving_old_topic_age.days.ago
 
     {
       templateName: 'composer/education',
       wait_for_typing: false,
       extraClass: 'old-topic',
-      body: PrettyText.cook(I18n.t('education.reviving_old_topic', days: (Time.zone.now - topic.last_posted_at).round / 1.day))
+      body: PrettyText.cook(I18n.t('education.reviving_old_topic', days: (Time.zone.now - @topic.last_posted_at).round / 1.day))
     }
   end
 

spec/components/composer_messages_finder_spec.rb

@@ -137,7 +137,6 @@ describe ComposerMessagesFinder do
     context "reply" do
       let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.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_sequential_replies).to be_blank
@@ -148,7 +147,6 @@ describe ComposerMessagesFinder do
         expect(finder.check_sequential_replies).to be_blank
       end
 
-
       it "will notify you if it hasn't in the current topic" do
         UserHistory.create!(action: UserHistory.actions[:notified_about_sequential_replies], target_user_id: user.id, topic_id: topic.id+1 )
         expect(finder.check_sequential_replies).to be_present
@@ -164,6 +162,11 @@ describe ComposerMessagesFinder do
         expect(finder.check_sequential_replies).to be_blank
       end
 
+      it "doesn't notify in message" do
+        Topic.any_instance.expects(:private_message?).returns(true)
+        expect(finder.check_sequential_replies).to be_blank
+      end
+
       context "success" do
         let!(:message) { finder.check_sequential_replies }