From 4fbf0172724de0836b9f7561a511722aedf184f1 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 5 Apr 2013 15:29:46 +1100 Subject: [PATCH] get regular trust level going, self heal inconsistent topic timings --- app/models/site_setting.rb | 8 +++ app/models/topic_user.rb | 20 ++++++ app/models/user_action.rb | 17 +++++ config/clock.rb | 2 +- config/locales/server.en.yml | 9 +++ db/migrate/20130404232558_add_user_extras.rb | 70 ++++++++++++++++++++ lib/guardian.rb | 2 +- lib/jobs/ensure_db_consistency.rb | 8 +++ lib/post_creator.rb | 11 ++- lib/promotion.rb | 13 ++++ spec/components/post_creator_spec.rb | 13 +++- spec/components/promotion_spec.rb | 41 ++++++++++++ spec/models/topic_user_spec.rb | 19 ++++++ spec/models/user_action_spec.rb | 12 ++-- 14 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20130404232558_add_user_extras.rb create mode 100644 lib/jobs/ensure_db_consistency.rb diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 7215c1d4a13..ecc82ad5424 100755 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -152,6 +152,14 @@ class SiteSetting < ActiveRecord::Base setting(:basic_requires_read_posts, 50) setting(:basic_requires_time_spent_mins, 15) + setting(:regular_requires_topics_entered, 3) + setting(:regular_requires_read_posts, 100) + setting(:regular_requires_time_spent_mins, 60) + setting(:regular_requires_days_visited, 15) + setting(:regular_requires_likes_received, 1) + setting(:regular_requires_likes_given, 1) + setting(:regular_requires_topic_reply_count, 3) + # Entropy checks setting(:title_min_entropy, 10) setting(:body_min_entropy, 7) diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb index e3d29d0a819..5052999b7b9 100644 --- a/app/models/topic_user.rb +++ b/app/models/topic_user.rb @@ -171,4 +171,24 @@ class TopicUser < ActiveRecord::Base end + def self.ensure_consistency! + exec_sql < last_read OR + seen_post_count <> post_count + ) +SQL + end + end diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 283e22bed2c..a7278a48a92 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -201,6 +201,15 @@ JOIN users pu on pu.id = COALESCE(p.user_id, t.user_id) action.created_at = hash[:created_at] end action.save! + + action_type = hash[:action_type] + user_id = hash[:user_id] + if action_type == LIKE + User.update_all('likes_given = likes_given + 1', id: user_id) + elsif action_type == WAS_LIKED + User.update_all('likes_received = likes_received + 1', id: user_id) + end + rescue ActiveRecord::RecordNotUnique # can happen, don't care already logged raise ActiveRecord::Rollback @@ -214,6 +223,14 @@ JOIN users pu on pu.id = COALESCE(p.user_id, t.user_id) action.destroy MessageBus.publish("/user/#{hash[:user_id]}", {user_action_id: action.id, remove: true}) end + + action_type = hash[:action_type] + user_id = hash[:user_id] + if action_type == LIKE + User.update_all('likes_given = likes_given - 1', id: user_id) + elsif action_type == WAS_LIKED + User.update_all('likes_received = likes_received - 1', id: user_id) + end end protected diff --git a/config/clock.rb b/config/clock.rb index dde27d2dc31..bc0c4962e75 100644 --- a/config/clock.rb +++ b/config/clock.rb @@ -12,8 +12,8 @@ module Clockwork every(1.day, 'enqueue_digest_emails', at: '06:00') every(1.day, 'category_stats', at: '04:00') + every(1.day, 'ensure_db_consistency', at: '02:00') every(10.minutes, 'periodical_updates') every(1.day, 'version_check') every(1.minute, 'clockwork_heartbeat') - end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4705b80f315..27d58b8451a 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -455,6 +455,15 @@ en: basic_requires_topics_entered: "How many a topics a new user must enter before promotion to basic (1) trust level" basic_requires_read_posts: "How many posts a new user must read before promotion to basic (1) trust level" basic_requires_time_spent_mins: "How many minutes a new user must read posts before promotion to basic (1) trust level" + + regular_requires_topics_entered: "How many a topics a basic user must enter before promotion to regular (2) trust level" + regular_requires_read_posts: "How many posts a basic user must read before promotion to regular (2) trust level" + regular_requires_time_spent_mins: "How many minutes a basic user must read posts before promotion to regular (2) trust level" + regular_requires_days_visited: "How many days a basic user must visit the site before promotion to regular (2) trust level" + regular_requires_likes_received: "How many likes a basic user must receive before promotion to regular (2) trust level" + 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" + visitor_max_links: "How many links a visitor can add to a post" visitor_max_images: "How many images a visitor can add to a post" visitor_max_mentions_per_post: "Maximum number of @name notifications a visitor can use in a post" diff --git a/db/migrate/20130404232558_add_user_extras.rb b/db/migrate/20130404232558_add_user_extras.rb new file mode 100644 index 00000000000..73d1add55dd --- /dev/null +++ b/db/migrate/20130404232558_add_user_extras.rb @@ -0,0 +1,70 @@ +class AddUserExtras < ActiveRecord::Migration + def up + + # NOTE: our user table is getting bloated, we probably want to split it for performance + # put lesser used columns into a user_extras table and frequently used ones here. + add_column :users, :likes_given, :integer, null: false, default: 0 + add_column :users, :likes_received, :integer, null: false, default: 0 + add_column :users, :topic_reply_count, :integer, null: false, default: 0 + + + # NOTE: to keep migrations working through refactorings we avoid externalizing this stuff. + # even though a helper method may make sense + execute < p.user_id AND + p.deleted_at IS NULL AND t.deleted_at IS NULL + GROUP BY p.user_id +) Z +WHERE + Z.user_id = u.id +SQL + + end + + def down + remove_column :users, :likes_given + remove_column :users, :likes_received + remove_column :users, :topic_reply_count + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index 0158c670daa..c2f19994de0 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -186,7 +186,7 @@ class Guardian return false if @user.blank? return false unless can_see?(object) return false if SiteSetting.must_approve_users? - @user.moderator? + @user.has_trust_level?(:regular) || @user.moderator? end diff --git a/lib/jobs/ensure_db_consistency.rb b/lib/jobs/ensure_db_consistency.rb new file mode 100644 index 00000000000..024787c324b --- /dev/null +++ b/lib/jobs/ensure_db_consistency.rb @@ -0,0 +1,8 @@ +module Jobs + # checks to see if any users need to be promoted + class EnsureDbConsistency < Jobs::Base + def execute(args) + TopicUser.ensure_consistency! + end + end +end diff --git a/lib/post_creator.rb b/lib/post_creator.rb index e352f433da2..83e22966d8d 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -23,6 +23,8 @@ class PostCreator # target_usernames - comma delimited list of usernames for membership (private message) # meta_data - Topic meta data hash def initialize(user, opts) + # TODO: we should reload user in case it is tainted, should take in a user_id as opposed to user + # If we don't do this we introduce a rather risky dependency @user = user @opts = opts raise Discourse::InvalidParameters.new(:raw) if @opts[:raw].blank? @@ -113,8 +115,13 @@ class PostCreator # Track the topic TopicUser.auto_track(@user.id, topic.id, TopicUser.notification_reasons[:created_post]) - # Update `last_posted_at` to match the post's created_at - @user.update_column(:last_posted_at, post.created_at) + + if @user.id != topic.user_id + @user.topic_reply_count += 1 + end + + @user.last_posted_at = post.created_at + @user.save! # Publish the post in the message bus MessageBus.publish("/topic/#{post.topic_id}", diff --git a/lib/promotion.rb b/lib/promotion.rb index 465ec71b50c..897d5a3edaf 100644 --- a/lib/promotion.rb +++ b/lib/promotion.rb @@ -32,4 +32,17 @@ class Promotion true end + def review_basic + return false if @user.topics_entered < SiteSetting.regular_requires_topics_entered + return false if @user.posts_read_count < SiteSetting.regular_requires_read_posts + return false if (@user.time_read / 60) < SiteSetting.regular_requires_time_spent_mins + return false if @user.days_visited < SiteSetting.regular_requires_days_visited + return false if @user.likes_received < SiteSetting.regular_requires_likes_received + return false if @user.likes_given < SiteSetting.regular_requires_likes_given + return false if @user.topic_reply_count < SiteSetting.regular_requires_topic_reply_count + + @user.trust_level = TrustLevel.levels[:regular] + @user.save + end + end diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index d158e5e3a0b..2a0c184c533 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -71,7 +71,6 @@ describe PostCreator do creator.create end - it 'assigns a category when supplied' do creator_with_category.create.topic.category.should == category end @@ -84,6 +83,18 @@ describe PostCreator do Post.any_instance.expects(:image_sizes=).with(image_sizes) creator_with_image_sizes.create end + + it 'increases topic response counts' do + first_post = creator.create + user2 = Fabricate(:coding_horror) + + user2.topic_reply_count.should == 0 + first_post.user.reload.topic_reply_count.should == 0 + + PostCreator.new(user2, topic_id: first_post.topic_id, raw: "this is my test post 123").create + user2.reload.topic_reply_count.should == 1 + first_post.user.reload.topic_reply_count.should == 0 + end end end diff --git a/spec/components/promotion_spec.rb b/spec/components/promotion_spec.rb index 13074236138..112eba0c558 100644 --- a/spec/components/promotion_spec.rb +++ b/spec/components/promotion_spec.rb @@ -44,5 +44,46 @@ describe Promotion do end + context "basic" do + + let(:user) { Fabricate(:user, trust_level: TrustLevel.levels[:basic])} + let(:promotion) { Promotion.new(user) } + + context 'that has done nothing' do + let!(:result) { promotion.review } + + it "returns false" do + result.should be_false + end + + it "has not changed the user's trust level" do + user.trust_level.should == TrustLevel.levels[:basic] + end + end + + context "that has done the requisite things" do + + before do + user.topics_entered = SiteSetting.regular_requires_topics_entered + user.posts_read_count = SiteSetting.regular_requires_read_posts + user.time_read = SiteSetting.regular_requires_time_spent_mins * 60 + user.days_visited = SiteSetting.regular_requires_days_visited * 60 + user.likes_received = SiteSetting.regular_requires_likes_received + user.likes_given = SiteSetting.regular_requires_likes_given + user.topic_reply_count = SiteSetting.regular_requires_topic_reply_count + + @result = promotion.review + end + + it "returns true" do + @result.should be_true + end + + it "has upgraded the user to regular" do + user.trust_level.should == TrustLevel.levels[:regular] + end + end + + end end diff --git a/spec/models/topic_user_spec.rb b/spec/models/topic_user_spec.rb index 029af8c92f8..b50ad407a83 100644 --- a/spec/models/topic_user_spec.rb +++ b/spec/models/topic_user_spec.rb @@ -207,4 +207,23 @@ describe TopicUser do end + + it "is able to self heal" do + p1 = Fabricate(:post) + p2 = Fabricate(:post, user: p1.user, topic: p1.topic, post_number: 2) + + TopicUser.exec_sql("UPDATE topic_users set seen_post_count=0, last_read_post_number=0 + WHERE topic_id = :topic_id AND user_id = :user_id", topic_id: p1.topic_id, user_id: p1.user_id) + + [p1,p2].each do |p| + PostTiming.create(topic_id: p.topic_id, post_number: p.post_number, user_id: p.user_id, msecs: 100) + end + + TopicUser.ensure_consistency! + + tu = TopicUser.where(user_id: p1.user_id, topic_id: p1.topic_id).first + tu.last_read_post_number.should == p2.post_number + tu.seen_post_count.should == 2 + end + end diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 8c2db847ffd..e79f8a6df9c 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -108,13 +108,17 @@ describe UserAction do @likee_action = likee.user_actions.where(action_type: UserAction::WAS_LIKED).first end - it 'should create a like action on the liker' do + it 'should result in correct data assignment' do @liker_action.should_not be_nil + @likee_action.should_not be_nil + likee.reload.likes_received.should == 1 + liker.reload.likes_given.should == 1 + + PostAction.remove_act(liker, post, PostActionType.types[:like]) + likee.reload.likes_received.should == 0 + liker.reload.likes_given.should == 0 end - it 'should create a like action on the likee' do - @likee_action.should_not be_nil - end end context "liking a private message" do