New composer message for people dominating a topic

This commit is contained in:
Robin Ward 2013-09-17 14:38:39 -04:00
parent 16dc0a7001
commit 99b6a62fcb
6 changed files with 145 additions and 7 deletions

View file

@ -263,6 +263,8 @@ class SiteSetting < ActiveRecord::Base
client_setting(:enable_mobile_theme, true) client_setting(:enable_mobile_theme, true)
setting(:dominating_topic_minimum_percent, 20)
def self.generate_api_key! def self.generate_api_key!
self.api_key = SecureRandom.hex(32) self.api_key = SecureRandom.hex(32)
end end

View file

@ -17,7 +17,8 @@ class UserHistory < ActiveRecord::Base
:delete_site_customization, :delete_site_customization,
:checked_for_custom_avatar, :checked_for_custom_avatar,
:notified_about_avatar, :notified_about_avatar,
:notified_about_sequential_replies) :notified_about_sequential_replies,
:notitied_about_dominating_topic)
end end
# Staff actions is a subset of all actions, used to audit actions taken by staff users. # 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 query
end end
def self.exists_for_user?(user, action_type) def self.exists_for_user?(user, action_type, opts=nil)
self.where(target_user_id: user.id, action: UserHistory.actions[action_type]).exists? 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 end
def new_value_is_json? def new_value_is_json?

View file

@ -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. 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 &ndash; 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: activerecord:
attributes: 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." 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: notification_types:
mentioned: "%{display_username} mentioned you in %{link}" mentioned: "%{display_username} mentioned you in %{link}"
liked: "%{display_username} liked your post in %{link}" liked: "%{display_username} liked your post in %{link}"

View file

@ -0,0 +1,5 @@
class AddTopicIdToUserHistories < ActiveRecord::Migration
def change
add_column :user_histories, :topic_id, :integer
end
end

View file

@ -8,7 +8,8 @@ class ComposerMessagesFinder
def find def find
check_education_message || check_education_message ||
check_avatar_notification || check_avatar_notification ||
check_sequential_replies check_sequential_replies ||
check_dominating_topic
end end
# Determines whether to show the user education text # Determines whether to show the user education text
@ -60,7 +61,7 @@ class ComposerMessagesFinder
(@user.post_count >= SiteSetting.educate_until_posts) && (@user.post_count >= SiteSetting.educate_until_posts) &&
# And who haven't been notified about sequential replies already # 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 # Count the topics made by this user in the last day
recent_posts_user_ids = Post.where(topic_id: @details[:topic_id]) 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 } recent_posts_user_ids.detect {|u| u != @user.id }
# If we got this far, log that we've nagged them about the sequential replies # 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', {templateName: 'composer/education',
wait_for_typing: true, wait_for_typing: true,
body: PrettyText.cook(I18n.t('education.sequential_replies')) } body: PrettyText.cook(I18n.t('education.sequential_replies')) }
end 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 private
def creating_topic? def creating_topic?

View file

@ -12,6 +12,7 @@ describe ComposerMessagesFinder do
finder.expects(:check_education_message).once finder.expects(:check_education_message).once
finder.expects(:check_avatar_notification).once finder.expects(:check_avatar_notification).once
finder.expects(:check_sequential_replies).once finder.expects(:check_sequential_replies).once
finder.expects(:check_dominating_topic).once
finder.find finder.find
end end
@ -136,10 +137,16 @@ describe ComposerMessagesFinder do
end end
it "doesn't notify a user it has already notified about sequential replies" do 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 finder.check_sequential_replies.should be_blank
end 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 it "doesn't notify a user who has less than the `sequential_replies_threshold` threshold posts" do
SiteSetting.stubs(:sequential_replies_threshold).returns(5) SiteSetting.stubs(:sequential_replies_threshold).returns(5)
finder.check_sequential_replies.should be_blank finder.check_sequential_replies.should be_blank
@ -166,5 +173,85 @@ describe ComposerMessagesFinder do
end 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 end