diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index b362117cc..19591ff06 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -258,6 +258,8 @@ class SiteSetting < ActiveRecord::Base setting(:detect_custom_avatars, false) setting(:max_daily_gravatar_crawls, 500) + setting(:sequential_replies_threshold, 2) + def self.generate_api_key! self.api_key = SecureRandom.hex(32) end diff --git a/app/models/user_history.rb b/app/models/user_history.rb index d8b9a245a..81b41b0b2 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -16,7 +16,8 @@ class UserHistory < ActiveRecord::Base :change_site_customization, :delete_site_customization, :checked_for_custom_avatar, - :notified_about_avatar) + :notified_about_avatar, + :notified_about_sequential_replies) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 67cbae13d..b05adac77 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -113,6 +113,13 @@ en: It's easier to follow community discussions and find interesting people in conversations when everyone has a unique avatar! + sequential_replies: | + ### Consider replying to several posts at once + + Rather than many sequential replies to a topic, please consider a single reply that includes quotes or @name references to previous posts. + + It's easier for everyone to read topics that have fewer in-depth replies versus lots of small, individual replies. + activerecord: attributes: @@ -686,6 +693,7 @@ en: detect_custom_avatars: "Whether or not to check that users have uploaded custom avatars" max_daily_gravatar_crawls: "The maximum amount of times Discourse will check gravatar for custom avatars in a day" + sequential_replies_threshold: "The amount of posts a user has to make in a row in a topic before being notified" notification_types: mentioned: "%{display_username} mentioned you in %{link}" diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index c3d6c1398..f05fa8626 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -7,7 +7,8 @@ class ComposerMessagesFinder def find check_education_message || - check_avatar_notification + check_avatar_notification || + check_sequential_replies end # Determines whether to show the user education text @@ -49,6 +50,40 @@ class ComposerMessagesFinder {templateName: 'composer/education', body: PrettyText.cook(I18n.t('education.avatar', profile_path: "/users/#{@user.username_lower}")) } end + # Is a user replying too much in succession? + def check_sequential_replies + + # We only care about replies to topics + return unless replying? && @details[:topic_id] && + + # For users who are not new + @user.has_trust_level?(:basic) && + + # And who have posted enough topics + (@user.topic_reply_count > SiteSetting.educate_until_posts) && + + # And who haven't been notified about sequential replies already + (!UserHistory.exists_for_user?(@user, :notified_about_sequential_replies)) + + # Count the topics made by this user in the last day + recent_posts_user_ids = Post.where(topic_id: @details[:topic_id]) + .where("created_at > ?", 1.day.ago) + .order('created_at desc') + .limit(SiteSetting.sequential_replies_threshold) + .pluck(:user_id) + + # Did we get back as many posts as we asked for, and are they all by the current user? + return if recent_posts_user_ids.size != SiteSetting.sequential_replies_threshold || + recent_posts_user_ids.detect {|u| u != @user.id } + + # If we got this far, log that we've nagged them about the sequential replies + UserHistory.create!(action: UserHistory.actions[:notified_about_sequential_replies], target_user_id: @user.id ) + + {templateName: 'composer/education', + wait_for_typing: true, + body: PrettyText.cook(I18n.t('education.sequential_replies')) } + end + private def creating_topic? diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index 1f0214f7c..35e16b318 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/spec/components/composer_messages_finder_spec.rb @@ -11,8 +11,8 @@ describe ComposerMessagesFinder do it "calls all the message finders" do finder.expects(:check_education_message).once finder.expects(:check_avatar_notification).once + finder.expects(:check_sequential_replies).once finder.find - end end @@ -86,7 +86,7 @@ describe ComposerMessagesFinder do end it "doesn't return notifications for new users" do - user.trust_level = 0 + user.trust_level = TrustLevel.levels[:newuser] finder.check_avatar_notification.should be_blank end @@ -101,6 +101,72 @@ describe ComposerMessagesFinder do end end + end + + context '.check_sequential_replies' do + let(:user) { Fabricate(:user) } + let(:topic) { Fabricate(:topic) } + + before do + SiteSetting.stubs(:educate_until_posts).returns(10) + user.topic_reply_count = 11 + + Fabricate(:post, topic: topic, user: user) + Fabricate(:post, topic: topic, user: user) + + SiteSetting.stubs(:sequential_replies_threshold).returns(2) + end + + it "does not give a message for new topics" do + finder = ComposerMessagesFinder.new(user, composerAction: 'createTopic') + finder.check_sequential_replies.should be_blank + end + + it "does not give a message without a topic id" do + ComposerMessagesFinder.new(user, composerAction: 'reply').check_sequential_replies.should be_blank + end + + context "reply" do + let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.id) } + + it "does not give a message to new users" do + user.trust_level = TrustLevel.levels[:newuser] + finder.check_sequential_replies.should be_blank + end + + it "does not give a message to users who are still in the 'education' phase" do + user.topic_reply_count = 10 + finder.check_sequential_replies.should be_blank + end + + it "doesn't notify a user it has already notified about sequential replies" do + UserHistory.create!(action: UserHistory.actions[:notified_about_sequential_replies], target_user_id: user.id ) + finder.check_sequential_replies.should be_blank + end + + it "doesn't notify a user who has less than the `sequential_replies_threshold` threshold posts" do + SiteSetting.stubs(:sequential_replies_threshold).returns(5) + finder.check_sequential_replies.should be_blank + end + + it "doesn't notify a user if another user posted" do + Fabricate(:post, topic: topic, user: Fabricate(:user)) + finder.check_sequential_replies.should be_blank + end + + context "success" do + let!(:message) { finder.check_sequential_replies } + + it "returns a message" do + message.should be_present + end + + it "creates a notified_about_sequential_replies log" do + UserHistory.exists_for_user?(user, :notified_about_sequential_replies).should be_true + end + + end + end end