diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index 68edfb76cca..fcac8d528b9 100644 --- a/lib/composer_messages_finder.rb +++ b/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 diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index 2a48b11a54f..763c0397719 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/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 }