diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 849890ad7..e257ee2f2 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -263,6 +263,8 @@ class SiteSetting < ActiveRecord::Base client_setting(:enable_mobile_theme, true) + setting(:dominating_topic_minimum_percent, 20) + 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 81b41b0b2..5cb133bb4 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -17,7 +17,8 @@ class UserHistory < ActiveRecord::Base :delete_site_customization, :checked_for_custom_avatar, :notified_about_avatar, - :notified_about_sequential_replies) + :notified_about_sequential_replies, + :notitied_about_dominating_topic) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. @@ -47,8 +48,11 @@ class UserHistory < ActiveRecord::Base query end - def self.exists_for_user?(user, action_type) - self.where(target_user_id: user.id, action: UserHistory.actions[action_type]).exists? + def self.exists_for_user?(user, action_type, opts=nil) + opts = opts || {} + result = self.where(target_user_id: user.id, action: UserHistory.actions[action_type]) + result = result.where(topic_id: opts[:topic_id]) if opts[:topic_id] + result.exists? end def new_value_is_json? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 22f8a53cb..40bb36300 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -120,6 +120,12 @@ en: It's easier for everyone to read topics that have fewer in-depth replies versus lots of small, individual replies. + dominating_topic: | + ### Let others join the conversation + + This topic is clearly important to you – you've posted more than %{percent}% of the replies here. + + Are you sure you're providing adequate time for other people to share their points of view, too? activerecord: attributes: @@ -698,6 +704,8 @@ en: 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 we consider it dominating." + notification_types: mentioned: "%{display_username} mentioned you in %{link}" liked: "%{display_username} liked your post in %{link}" diff --git a/db/migrate/20130917174738_add_topic_id_to_user_histories.rb b/db/migrate/20130917174738_add_topic_id_to_user_histories.rb new file mode 100644 index 000000000..2b0c17946 --- /dev/null +++ b/db/migrate/20130917174738_add_topic_id_to_user_histories.rb @@ -0,0 +1,5 @@ +class AddTopicIdToUserHistories < ActiveRecord::Migration + def change + add_column :user_histories, :topic_id, :integer + end +end diff --git a/lib/composer_messages_finder.rb b/lib/composer_messages_finder.rb index cc40873e3..d475cc62f 100644 --- a/lib/composer_messages_finder.rb +++ b/lib/composer_messages_finder.rb @@ -8,7 +8,8 @@ class ComposerMessagesFinder def find check_education_message || check_avatar_notification || - check_sequential_replies + check_sequential_replies || + check_dominating_topic end # Determines whether to show the user education text @@ -60,7 +61,7 @@ class ComposerMessagesFinder (@user.post_count >= SiteSetting.educate_until_posts) && # And who haven't been notified about sequential replies already - (!UserHistory.exists_for_user?(@user, :notified_about_sequential_replies)) + !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]) @@ -74,13 +75,44 @@ class ComposerMessagesFinder 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 ) + UserHistory.create!(action: UserHistory.actions[:notified_about_sequential_replies], + target_user_id: @user.id, + topic_id: @details[:topic_id] ) {templateName: 'composer/education', wait_for_typing: true, body: PrettyText.cook(I18n.t('education.sequential_replies')) } end + def check_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, :notitied_about_dominating_topic, topic_id: @details[:topic_id]) + + topic = Topic.where(id: @details[:topic_id]).first + return if topic.blank? || + topic.user_id == @user.id || + topic.posts_count < SiteSetting.best_of_posts_required + + posts_by_user = @user.posts.where(topic_id: topic.id).count + + 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 + UserHistory.create!(action: UserHistory.actions[:notitied_about_dominating_topic], + target_user_id: @user.id, + topic_id: @details[:topic_id]) + + + {templateName: 'composer/education', + wait_for_typing: true, + body: PrettyText.cook(I18n.t('education.dominating_topic', percent: (ratio * 100).round)) } + end + private def creating_topic? diff --git a/spec/components/composer_messages_finder_spec.rb b/spec/components/composer_messages_finder_spec.rb index cf6f62478..1451aeaa1 100644 --- a/spec/components/composer_messages_finder_spec.rb +++ b/spec/components/composer_messages_finder_spec.rb @@ -12,6 +12,7 @@ describe ComposerMessagesFinder do finder.expects(:check_education_message).once finder.expects(:check_avatar_notification).once finder.expects(:check_sequential_replies).once + finder.expects(:check_dominating_topic).once finder.find end @@ -136,10 +137,16 @@ describe ComposerMessagesFinder do 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 ) + UserHistory.create!(action: UserHistory.actions[:notified_about_sequential_replies], target_user_id: user.id, topic_id: topic.id ) finder.check_sequential_replies.should 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 ) + finder.check_sequential_replies.should be_present + 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 @@ -166,5 +173,85 @@ describe ComposerMessagesFinder do end + context '.check_dominating_topic' do + let(:user) { Fabricate(:user) } + let(:topic) { Fabricate(:topic) } + + before do + SiteSetting.stubs(:educate_until_posts).returns(10) + user.stubs(:post_count).returns(11) + + SiteSetting.stubs(:best_of_posts_required).returns(1) + + Fabricate(:post, topic: topic, user: user) + Fabricate(:post, topic: topic, user: user) + Fabricate(:post, topic: topic, user: Fabricate(: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_dominating_topic.should be_blank + end + + it "does not give a message without a topic id" do + ComposerMessagesFinder.new(user, composerAction: 'reply').check_dominating_topic.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 users who are still in the 'education' phase" do + user.stubs(:post_count).returns(9) + finder.check_dominating_topic.should be_blank + end + + it "does not notify if the `best_of_posts_required` has not been reached" do + SiteSetting.stubs(:best_of_posts_required).returns(100) + finder.check_dominating_topic.should be_blank + end + + it "doesn't notify a user it has already notified in this topic" do + UserHistory.create!(action: UserHistory.actions[:notitied_about_dominating_topic], topic_id: topic.id, target_user_id: user.id ) + finder.check_dominating_topic.should be_blank + end + + it "notifies a user if the topic is different" do + UserHistory.create!(action: UserHistory.actions[:notitied_about_dominating_topic], topic_id: topic.id+1, target_user_id: user.id ) + finder.check_dominating_topic.should be_present + end + + it "doesn't notify a user if the topic has less than `best_of_posts_required` posts" do + SiteSetting.stubs(:best_of_posts_required).returns(5) + finder.check_dominating_topic.should be_blank + end + + it "doesn't notify a user if they've posted less than the percentage" do + SiteSetting.stubs(:dominating_topic_minimum_percent).returns(100) + finder.check_dominating_topic.should be_blank + end + + it "doesn't notify you if it's your own topic" do + topic.update_column(:user_id, user.id) + finder.check_dominating_topic.should be_blank + end + + context "success" do + let!(:message) { finder.check_dominating_topic } + + it "returns a message" do + message.should be_present + end + + it "creates a notitied_about_dominating_topic log" do + UserHistory.exists_for_user?(user, :notitied_about_dominating_topic).should be_true + end + + end + end + + end + end