diff --git a/app/assets/javascripts/admin/models/leader_requirements.js b/app/assets/javascripts/admin/models/leader_requirements.js index 2076386b384..6c7fd45e526 100644 --- a/app/assets/javascripts/admin/models/leader_requirements.js +++ b/app/assets/javascripts/admin/models/leader_requirements.js @@ -13,11 +13,13 @@ Discourse.LeaderRequirements = Discourse.Model.extend({ topics_replied_to: this.get('num_topics_replied_to') >= this.get('min_topics_replied_to'), topics_viewed: this.get('topics_viewed') >= this.get('min_topics_viewed'), posts_read: this.get('posts_read') >= this.get('min_posts_read'), - flagged_posts: this.get('num_flagged_posts') < this.get('max_flagged_posts') + flagged_posts: this.get('num_flagged_posts') <= this.get('max_flagged_posts'), + flagged_by_users: this.get('num_flagged_by_users') <= this.get('max_flagged_by_users') }; }.property('days_visited', 'min_days_visited', 'num_topics_replied_to', 'min_topics_replied_to', 'topics_viewed', 'min_topics_viewed', 'posts_read', 'min_posts_read', - 'num_flagged_posts', 'max_flagged_posts') + 'num_flagged_posts', 'max_flagged_posts', + 'num_flagged_by_users', 'max_flagged_by_users') }); diff --git a/app/assets/javascripts/admin/templates/user_leader_requirements.js.handlebars b/app/assets/javascripts/admin/templates/user_leader_requirements.js.handlebars index 21ea6926e82..cf5d348eacc 100644 --- a/app/assets/javascripts/admin/templates/user_leader_requirements.js.handlebars +++ b/app/assets/javascripts/admin/templates/user_leader_requirements.js.handlebars @@ -55,18 +55,30 @@ {{num_flagged_posts}} {{max_flagged_posts}} {{i18n max}} + + {{i18n admin.user.tl3_requirements.flagged_by_users}} + + {{num_flagged_by_users}} + {{max_flagged_by_users}} {{i18n max}} + - -
-

- {{#if requirements_met}} - Qualifies for trust level 3 - {{else}} - Doesn't qualify for trust level 3 - {{/if}} -

{{/with}} + +
+

+ {{#if leaderRequirements.requirements_met}} + {{i18n admin.user.tl3_requirements.qualifies}} + {{#unless isLeader}} + {{i18n admin.user.tl3_requirements.will_be_promoted}} + {{/unless}} + {{else}} + {{i18n admin.user.tl3_requirements.does_not_qualify}} + {{#if isLeader}} + {{i18n admin.user.tl3_requirements.will_be_demoted}} + {{/if}} + {{/if}} +

diff --git a/app/assets/javascripts/discourse/models/user.js b/app/assets/javascripts/discourse/models/user.js index cb4398519b9..d25c44a2e13 100644 --- a/app/assets/javascripts/discourse/models/user.js +++ b/app/assets/javascripts/discourse/models/user.js @@ -129,6 +129,7 @@ Discourse.User = Discourse.Model.extend({ return Discourse.Site.currentProp('trustLevels').findProperty('id', parseInt(this.get('trust_level'), 10)); }.property('trust_level'), + isLeader: Em.computed.equal('trust_level', 3), isElder: Em.computed.equal('trust_level', 4), canManageTopic: Em.computed.or('staff', 'isElder'), diff --git a/app/jobs/scheduled/leader_promotions.rb b/app/jobs/scheduled/leader_promotions.rb new file mode 100644 index 00000000000..12d90b692e5 --- /dev/null +++ b/app/jobs/scheduled/leader_promotions.rb @@ -0,0 +1,25 @@ +module Jobs + + class LeaderPromotions < Jobs::Scheduled + daily at: 4.hours + + def execute(args) + return unless Rails.env.test? # do nothing for now + + # Demotions + demoted_user_ids = [] + User.real.where(trust_level: TrustLevel.levels[:leader]).find_each do |u| + unless Promotion.leader_met?(u) + demoted_user_ids << u.id + Promotion.new(u).change_trust_level!(:regular) + end + end + + # Promotions + User.real.where(trust_level: TrustLevel.levels[:regular]).where.not(id: demoted_user_ids).find_each do |u| + Promotion.new(u).review_regular + end + end + end + +end diff --git a/app/models/leader_requirements.rb b/app/models/leader_requirements.rb index 2e5d2997f97..aaa0790cb03 100644 --- a/app/models/leader_requirements.rb +++ b/app/models/leader_requirements.rb @@ -4,8 +4,9 @@ class LeaderRequirements include ActiveModel::Serialization - attr_accessor :time_period, - :days_visited, :min_days_visited, + TIME_PERIOD = 100 # days + + attr_accessor :days_visited, :min_days_visited, :num_topics_replied_to, :min_topics_replied_to, :topics_viewed, :min_topics_viewed, :posts_read, :min_posts_read, @@ -20,54 +21,86 @@ class LeaderRequirements num_topics_replied_to >= min_topics_replied_to && topics_viewed >= min_topics_viewed && posts_read >= min_posts_read && - num_flagged_posts <= max_flagged_posts - end - - def time_period - 100 # days + num_flagged_posts <= max_flagged_posts && + num_flagged_by_users <= max_flagged_by_users end def days_visited - @user.user_visits.where("visited_at > ? and posts_read > 0", time_period.days.ago).count + @user.user_visits.where("visited_at > ? and posts_read > 0", TIME_PERIOD.days.ago).count end def min_days_visited - time_period * 0.5 + (TIME_PERIOD * (SiteSetting.leader_requires_days_visited.to_f / 100.0)).to_i end def num_topics_replied_to - @user.posts.select('distinct topic_id').where('created_at > ? AND post_number > 1', time_period.days.ago).count + @user.posts.select('distinct topic_id').where('created_at > ? AND post_number > 1', TIME_PERIOD.days.ago).count end def min_topics_replied_to - 10 + SiteSetting.leader_requires_topics_replied_to end def topics_viewed - View.where('viewed_at > ?', time_period.days.ago).where(user_id: @user.id, parent_type: 'Topic').select('distinct(parent_id)').count + View.where('viewed_at > ?', TIME_PERIOD.days.ago).where(user_id: @user.id, parent_type: 'Topic').select('distinct(parent_id)').count end def min_topics_viewed - (Topic.listable_topics.visible.created_since(time_period.days.ago).count * 0.25).round + (LeaderRequirements.num_topics_in_time_period.to_i * (SiteSetting.leader_requires_topics_viewed.to_f / 100.0)).round end def posts_read - @user.user_visits.where('visited_at > ?', time_period.days.ago).pluck(:posts_read).sum + @user.user_visits.where('visited_at > ?', TIME_PERIOD.days.ago).pluck(:posts_read).sum end def min_posts_read - (Post.public_posts.visible.created_since(time_period.days.ago).count * 0.25).round + (LeaderRequirements.num_posts_in_time_period.to_i * (SiteSetting.leader_requires_posts_read.to_f / 100.0)).round end def num_flagged_posts - # Count the number of posts that were flagged, and moderators explicitly agreed with the flags - # by clicking the "Agree (hide post + send PM)" or "Defer" (on an automatically hidden post) buttons. - # In both cases, the defer flag is set to true. - post_ids = @user.posts.with_deleted.where('created_at > ? AND (spam_count > 0 OR inappropriate_count > 0)', time_period.days.ago).pluck(:id) - PostAction.with_deleted.where(post_id: post_ids).where(defer: true).pluck(:post_id).uniq.count + PostAction.with_deleted.where(post_id: flagged_post_ids).where.not(user_id: @user.id).pluck(:post_id).uniq.count end def max_flagged_posts - 5 + SiteSetting.leader_requires_max_flagged + end + + def num_flagged_by_users + PostAction.with_deleted.where(post_id: flagged_post_ids).where.not(user_id: @user.id).pluck(:user_id).uniq.count + end + + def max_flagged_by_users + SiteSetting.leader_requires_max_flagged + end + + def self.clear_cache + $redis.del NUM_TOPICS_KEY + $redis.del NUM_POSTS_KEY + end + + + CACHE_DURATION = 1.day.seconds - 60 + NUM_TOPICS_KEY = "tl3_num_topics" + NUM_POSTS_KEY = "tl3_num_posts" + + def self.num_topics_in_time_period + $redis.get(NUM_TOPICS_KEY) || begin + count = Topic.listable_topics.visible.created_since(TIME_PERIOD.days.ago).count + $redis.setex NUM_TOPICS_KEY, CACHE_DURATION, count + count + end + end + + def self.num_posts_in_time_period + $redis.get(NUM_POSTS_KEY) || begin + count = Post.public_posts.visible.created_since(TIME_PERIOD.days.ago).count + $redis.setex NUM_POSTS_KEY, CACHE_DURATION, count + count + end + end + + def flagged_post_ids + # (TODO? and moderators explicitly agreed with the flags) + @user.posts.with_deleted.where('created_at > ? AND (spam_count > 0 OR inappropriate_count > 0)', TIME_PERIOD.days.ago).pluck(:id) end end diff --git a/app/serializers/leader_requirements_serializer.rb b/app/serializers/leader_requirements_serializer.rb index aa32159f975..54008ed3625 100644 --- a/app/serializers/leader_requirements_serializer.rb +++ b/app/serializers/leader_requirements_serializer.rb @@ -5,7 +5,12 @@ class LeaderRequirementsSerializer < ApplicationSerializer :num_topics_replied_to, :min_topics_replied_to, :topics_viewed, :min_topics_viewed, :posts_read, :min_posts_read, - :num_flagged_posts, :max_flagged_posts + :num_flagged_posts, :max_flagged_posts, + :num_flagged_by_users, :max_flagged_by_users + + def time_period + LeaderRequirements::TIME_PERIOD + end def requirements_met object.requirements_met? diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ef7be8491b4..85e585bb3b5 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1800,6 +1800,11 @@ en: topics_viewed: "Topics Viewed" posts_read: "Posts Read" flagged_posts: "Flagged Posts" + flagged_by_users: "Users Who Flagged" + qualifies: "Qualifies for trust level 3." + will_be_promoted: "Will be promoted within 24 hours." + does_not_qualify: "Doesn't qualify for trust level 3." + will_be_demoted: "Will be demoted within 24 hours." site_content: none: "Choose a type of content to begin editing." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 69655ddad09..78eb65ffcf2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -802,6 +802,12 @@ en: regular_requires_likes_given: "How many likes a basic user must cast before promotion to regular (2) trust level" regular_requires_topic_reply_count: "How many topics a basic user must reply to before promotion to regular (2) trust level" + leader_requires_days_visited: "Minimum number of days that a user needs to have visited the site in the last 100 days to qualify for promotion to leader (3) trust level. (0 to 100)" + leader_requires_topics_replied_to: "Minimum number of topics a user needs to have replied to in the last 100 days to qualify for promotion to leader (3) trust level. (0 or higher)" + leader_requires_topics_viewed: "The percentage of topics created in the last 100 days that a user needs to have viewed to qualify for promotion to leader (3) trust level. (0 to 100)" + leader_requires_posts_read: "The percentage of posts created in the last 100 days that a user needs to have viewed to qualify for promotion to leader (3) trust level. (0 to 100)" + leader_requires_max_flagged: "User must not have had more than x posts flagged by x different users in the last 100 days to qualify for promotion to leader (3) trust level, where x is this setting's value. (0 or higher)" + min_trust_to_create_topic: "The minimum trust level required to create a new topic." min_trust_to_edit_wiki_post: "The minimum trust level required to edit post marked as wiki." diff --git a/config/site_settings.yml b/config/site_settings.yml index 1c2d06faca2..5bba7b20201 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -452,6 +452,24 @@ trust: regular_requires_likes_received: 1 regular_requires_likes_given: 1 regular_requires_topic_reply_count: 3 + leader_requires_days_visited: + default: 50 + min: 1 + max: 100 + leader_requires_topics_replied_to: + default: 10 + min: 0 + leader_requires_topics_viewed: + default: 25 + min: 0 + max: 100 + leader_requires_posts_read: + default: 25 + min: 0 + max: 100 + leader_requires_max_flagged: + default: 5 + min: 0 security: use_https: false diff --git a/lib/promotion.rb b/lib/promotion.rb index 94fa3913d66..5ec5f802bf3 100644 --- a/lib/promotion.rb +++ b/lib/promotion.rb @@ -14,6 +14,9 @@ class Promotion # nil users are never promoted return false if @user.blank? + # Promotion beyond basic requires some expensive queries, so don't do that here. + return false if @user.trust_level >= TrustLevel.levels[:regular] + trust_key = TrustLevel.levels[@user.trust_level] review_method = :"review_#{trust_key.to_s}" @@ -30,6 +33,10 @@ class Promotion Promotion.regular_met?(@user) && change_trust_level!(:regular) end + def review_regular + Promotion.leader_met?(@user) && change_trust_level!(:leader) + end + def change_trust_level!(level, opts = {}) raise "Invalid trust level #{level}" unless TrustLevel.valid_level?(level) @@ -88,4 +95,8 @@ class Promotion return true end + def self.leader_met?(user) + LeaderRequirements.new(user).requirements_met? + end + end diff --git a/spec/components/promotion_spec.rb b/spec/components/promotion_spec.rb index 74c1a0e86a3..001270fb759 100644 --- a/spec/components/promotion_spec.rb +++ b/spec/components/promotion_spec.rb @@ -3,6 +3,16 @@ require 'promotion' describe Promotion do + describe "review" do + it "skips regular users" do + # Reviewing users at higher trust levels is expensive, so trigger those reviews in a background job. + regular = Fabricate.build(:user, trust_level: TrustLevel.levels[:regular]) + promotion = described_class.new(regular) + promotion.expects(:review_regular).never + promotion.review + end + end + context "newuser" do let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[:newuser])} @@ -88,4 +98,42 @@ describe Promotion do end + context "regular" do + let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[:regular])} + let(:promotion) { Promotion.new(user) } + + context "doesn't qualify for promotion" do + before do + LeaderRequirements.any_instance.expects(:requirements_met?).at_least_once.returns(false) + end + + it "review_regular returns false" do + expect { + promotion.review_regular.should == false + }.to_not change { user.reload.trust_level } + end + + it "doesn't promote" do + expect { + promotion.review_regular + }.to_not change { user.reload.trust_level } + end + end + + context "qualifies for promotion" do + before do + LeaderRequirements.any_instance.expects(:requirements_met?).at_least_once.returns(true) + end + + it "review_regular returns true" do + promotion.review_regular.should == true + end + + it "promotes to leader" do + promotion.review_regular.should == true + user.reload.trust_level.should == TrustLevel.levels[:leader] + end + end + end + end diff --git a/spec/jobs/leader_promotions_spec.rb b/spec/jobs/leader_promotions_spec.rb new file mode 100644 index 00000000000..f0dae384051 --- /dev/null +++ b/spec/jobs/leader_promotions_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Jobs::LeaderPromotions do + + subject(:run_job) { described_class.new.execute({}) } + + it "promotes tl2 user who qualifies for tl3" do + tl2_user = Fabricate(:user, trust_level: TrustLevel.levels[:regular]) + LeaderRequirements.any_instance.stubs(:requirements_met?).returns(true) + Promotion.any_instance.expects(:change_trust_level!).with(:leader, anything).once + run_job + end + + it "doesn't promote tl1 and tl0 users who have met tl3 requirements" do + tl1_user = Fabricate(:user, trust_level: TrustLevel.levels[:basic]) + tl0_user = Fabricate(:user, trust_level: TrustLevel.levels[:newuser]) + LeaderRequirements.any_instance.expects(:requirements_met?).never + Promotion.any_instance.expects(:change_trust_level!).never + run_job + end + + it "demotes tl3 user who doesn't qualify for tl3 anymore" do + tl3_user = Fabricate(:user, trust_level: TrustLevel.levels[:leader]) + LeaderRequirements.any_instance.stubs(:requirements_met?).returns(false) + Promotion.any_instance.expects(:change_trust_level!).with(:regular, anything).once + run_job + end +end diff --git a/spec/models/leader_requirements_spec.rb b/spec/models/leader_requirements_spec.rb new file mode 100644 index 00000000000..ad5e22d9083 --- /dev/null +++ b/spec/models/leader_requirements_spec.rb @@ -0,0 +1,118 @@ +require 'spec_helper' + +describe LeaderRequirements do + + let(:user) { Fabricate.build(:user) } + subject(:leader_requirements) { described_class.new(user) } + + before do + described_class.clear_cache + end + + describe "requirements" do + it "min_days_visited uses site setting" do + SiteSetting.stubs(:leader_requires_days_visited).returns(66) + leader_requirements.min_days_visited.should == 66 + end + + it "min_topics_replied_to uses site setting" do + SiteSetting.stubs(:leader_requires_topics_replied_to).returns(12) + leader_requirements.min_topics_replied_to.should == 12 + end + + it "min_topics_viewed depends on site setting and number of topics created" do + SiteSetting.stubs(:leader_requires_topics_viewed).returns(75) + described_class.stubs(:num_topics_in_time_period).returns(31) + leader_requirements.min_topics_viewed.should == 23 + end + + it "min_posts_read depends on site setting and number of posts created" do + SiteSetting.stubs(:leader_requires_posts_read).returns(66) + described_class.stubs(:num_posts_in_time_period).returns(1234) + leader_requirements.min_posts_read.should == 814 + end + + it "max_flagged_posts depends on site setting" do + SiteSetting.stubs(:leader_requires_max_flagged).returns(3) + leader_requirements.max_flagged_posts.should == 3 + end + end + + describe "days_visited" do + it "counts visits when posts were read no further back than 100 days ago" do + user.save + user.update_posts_read!(1, 2.days.ago) + user.update_posts_read!(1, 3.days.ago) + user.update_posts_read!(0, 4.days.ago) + user.update_posts_read!(3, 101.days.ago) + leader_requirements.days_visited.should == 2 + end + end + + describe "num_topics_replied_to" do + it "counts topics in which user replied in last 100 days" do + user.save + + not_a_reply = create_post(user: user) # user created the topic, so it doesn't count + + topic1 = create_post.topic + reply1 = create_post(topic: topic1, user: user) + reply_again = create_post(topic: topic1, user: user) # two replies in one topic + + topic2 = create_post(created_at: 101.days.ago).topic + reply2 = create_post(topic: topic2, user: user, created_at: 101.days.ago) # topic is over 100 days old + + leader_requirements.num_topics_replied_to.should == 1 + end + end + + describe "topics_viewed" do + def make_view(id, at, user_id) + View.create!(parent_id: id, parent_type: 'Topic', ip_address: '11.22.33.44', viewed_at: at, user_id: user_id) + end + + it "counts topics views within last 100 days, not counting a topic more than once" do + user.save + make_view(9, 1.day.ago, user.id) + make_view(9, 3.days.ago, user.id) # same topic, different day + make_view(3, 4.days.ago, user.id) + make_view(2, 101.days.ago, user.id) # too long ago + leader_requirements.topics_viewed.should == 2 + end + end + + describe "posts_read" do + it "counts posts read within the last 100 days" do + user.save + user.update_posts_read!(3, 2.days.ago) + user.update_posts_read!(1, 3.days.ago) + user.update_posts_read!(0, 4.days.ago) + user.update_posts_read!(5, 101.days.ago) + leader_requirements.posts_read.should == 4 + end + end + + context "with flagged posts" do + before do + user.save + flags = [:off_topic, :inappropriate, :notify_user, :notify_moderators, :spam].map do |t| + Fabricate(:flag, post: Fabricate(:post, user: user), post_action_type_id: PostActionType.types[t]) + end + + # Same post, different user: + Fabricate(:flag, post: flags[1].post, post_action_type_id: PostActionType.types[:spam]) + + # Flagged their own post: + Fabricate(:flag, user: user, post: Fabricate(:post, user: user), post_action_type_id: PostActionType.types[:spam]) + + # More than 100 days ago: + Fabricate(:flag, post: Fabricate(:post, user: user, created_at: 101.days.ago), post_action_type_id: PostActionType.types[:spam], created_at: 101.days.ago) + end + + it "num_flagged_posts and num_flagged_by_users count spam and inappropriate flags in the last 100 days" do + leader_requirements.num_flagged_posts.should == 2 + leader_requirements.num_flagged_by_users.should == 3 + end + end + +end