From d27575176a274ddee7513fb2d43aa9e453f9a68b Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Tue, 28 Feb 2017 16:47:16 -0500 Subject: [PATCH] Enforce a minimum amount of posters in a topic for `get_a_room` --- lib/composer_messages_finder.rb | 3 +- .../composer_messages_finder_spec.rb | 30 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index 04893ebda80..41064a632aa 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -136,7 +136,7 @@ class ComposerMessagesFinder } end - def check_get_a_room + def check_get_a_room(min_users_posted: 5) return unless educate_reply?(:notified_about_get_a_room) return unless @details[:post_id].present? @@ -152,6 +152,7 @@ class ComposerMessagesFinder find_all {|uid| uid != @user.id && uid == reply_to_user_id} return unless last_x_replies.size == SiteSetting.get_a_room_threshold + return unless @topic.posts.count('distinct user_id') >= min_users_posted UserHistory.create!(action: UserHistory.actions[:notified_about_get_a_room], target_user_id: @user.id, diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index bc15511490b..adf6f81db2e 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/spec/components/composer_messages_finder_spec.rb @@ -331,11 +331,11 @@ describe ComposerMessagesFinder do 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 + expect(finder.check_get_a_room(min_users_posted: 2)).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 + expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_get_a_room(min_users_posted: 2)).to be_blank end context "reply" do @@ -343,7 +343,7 @@ describe ComposerMessagesFinder do 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 + expect(finder.check_get_a_room(min_users_posted: 2)).to be_blank end it "doesn't notify a user it has already notified about sequential replies" do @@ -352,7 +352,7 @@ describe ComposerMessagesFinder do target_user_id: user.id, topic_id: topic.id ) - expect(finder.check_get_a_room).to be_blank + expect(finder.check_get_a_room(min_users_posted: 2)).to be_blank end it "will notify you if it hasn't in the current topic" do @@ -361,28 +361,28 @@ describe ComposerMessagesFinder do target_user_id: user.id, topic_id: topic.id+1 ) - expect(finder.check_get_a_room).to be_present + expect(finder.check_get_a_room(min_users_posted: 2)).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 + expect(finder.check_get_a_room(min_users_posted: 2)).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 + expect(finder.check_get_a_room(min_users_posted: 2)).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 + expect(finder.check_get_a_room(min_users_posted: 2)).to be_blank end it "doesn't notify in a message" do topic.update_columns(category_id: nil, archetype: 'private_message') - expect(finder.check_get_a_room).to be_blank + expect(finder.check_get_a_room(min_users_posted: 2)).to be_blank end it "doesn't notify when replying to a different user" do @@ -393,11 +393,19 @@ describe ComposerMessagesFinder do post_id: other_user_reply.id ) - expect(other_finder.check_get_a_room).to be_blank + expect(other_finder.check_get_a_room(min_users_posted: 2)).to be_blank + end + + context "with a default min_users_posted value" do + let!(:message) { finder.check_get_a_room } + + it "works as expected" do + expect(message).to be_blank + end end context "success" do - let!(:message) { finder.check_get_a_room } + let!(:message) { finder.check_get_a_room(min_users_posted: 2) } it "works as expected" do expect(message).to be_present