diff --git a/app/models/post_action.rb b/app/models/post_action.rb index d8a6dc570..b8a1746d2 100644 --- a/app/models/post_action.rb +++ b/app/models/post_action.rb @@ -136,7 +136,7 @@ class PostAction < ActiveRecord::Base # Voting also changes the sort_order if post_action_type == :vote - Post.update_all ["vote_count = vote_count + :delta, sort_order = :max - (vote_count + :delta)", delta: delta, max: Topic::MAX_SORT_ORDER], id: post_id + Post.update_all ["vote_count = vote_count + :delta, sort_order = :max - (vote_count + :delta)", delta: delta, max: Topic.max_sort_order], id: post_id else Post.update_all ["#{column} = #{column} + ?", delta], id: post_id end diff --git a/app/models/topic.rb b/app/models/topic.rb index d7b012c0b..6bc15d769 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -8,8 +8,13 @@ class Topic < ActiveRecord::Base include ActionView::Helpers include RateLimiter::OnCreateRecord - MAX_SORT_ORDER = 2**31 - 1 - FEATURED_USERS = 4 + def self.max_sort_order + 2**31 - 1 + end + + def self.featured_users_count + 4 + end versioned if: :new_version_required? acts_as_paranoid @@ -123,7 +128,7 @@ class Topic < ActiveRecord::Base return title unless SiteSetting.title_fancy_entities? # We don't always have to require this, if fancy is disabled - # see: http://meta.discourse.org/t/pattern-for-defer-loading-gems-and-profiling-with-perftools-rb/4629 + # see: http://meta.discourse.org/t/pattern-for-defer-loading-gems-and-profiling-with-perftools-rb/4629 require 'redcarpet' unless defined? Redcarpet Redcarpet::Render::SmartyPants.render(title) @@ -416,8 +421,6 @@ class Topic < ActiveRecord::Base end # Update denormalized values since we've manually moved stuff - Topic.reset_highest(topic.id) - Topic.reset_highest(id) end # Add a moderator post explaining that the post was moved @@ -427,15 +430,61 @@ class Topic < ActiveRecord::Base add_moderator_post(moved_by, I18n.t("move_posts.moderator_post", count: post_ids.size, topic_link: topic_link), post_number: first_post_number) Jobs.enqueue(:notify_moved_posts, post_ids: post_ids, moved_by_id: moved_by.id) + + topic.update_statistics + update_statistics end + topic end + # Updates the denormalized statistics of a topic including featured posters. They shouldn't + # go out of sync unless you do something drastic live move posts from one topic to another. + # this recalculates everything. + def update_statistics + feature_topic_users + update_action_counts + Topic.reset_highest(id) + end + def update_flagged_posts_count PostAction.update_flagged_posts_count end + def update_action_counts + PostActionType.types.keys.each do |type| + count_field = "#{type}_count" + update_column(count_field, Post.where(topic_id: id).sum(count_field)) + end + end + + # Chooses which topic users to feature + def feature_topic_users(args={}) + reload + + to_feature = posts + + # Don't include the OP or the last poster + to_feature = to_feature.where('user_id NOT IN (?, ?)', user_id, last_post_user_id) + + # Exclude a given post if supplied (in the case of deletes) + to_feature = to_feature.where("id <> ?", args[:except_post_id]) if args[:except_post_id].present? + + # Clear the featured users by default + Topic.featured_users_count.times do |i| + send("featured_user#{i+1}_id=", nil) + end + + # Assign the featured_user{x} columns + to_feature = to_feature.group(:user_id).order('count_all desc').limit(Topic.featured_users_count) + + to_feature.count.keys.each_with_index do |user_id, i| + send("featured_user#{i+1}_id=", user_id) + end + + save + end # Create the summary of the interesting posters in a topic. Cheats to avoid # many queries. diff --git a/lib/jobs/feature_topic_users.rb b/lib/jobs/feature_topic_users.rb index aea450434..a13e5f79b 100644 --- a/lib/jobs/feature_topic_users.rb +++ b/lib/jobs/feature_topic_users.rb @@ -6,28 +6,7 @@ module Jobs topic = Topic.where(id: args[:topic_id]).first raise Discourse::InvalidParameters.new(:topic_id) unless topic.present? - to_feature = topic.posts - - # Don't include the OP or the last poster - to_feature = to_feature.where('user_id <> ?', topic.user_id) - to_feature = to_feature.where('user_id <> ?', topic.last_post_user_id) - - # Exclude a given post if supplied (in the case of deletes) - to_feature = to_feature.where("id <> ?", args[:except_post_id]) if args[:except_post_id].present? - - - # Clear the featured users by default - Topic::FEATURED_USERS.times do |i| - topic.send("featured_user#{i+1}_id=", nil) - end - - # Assign the featured_user{x} columns - to_feature = to_feature.group(:user_id).order('count_all desc').limit(Topic::FEATURED_USERS) - to_feature.count.keys.each_with_index do |user_id, i| - topic.send("featured_user#{i+1}_id=", user_id) - end - - topic.save + topic.feature_topic_users(args) end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 32a8afdd7..d6a139724 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -181,13 +181,19 @@ describe Topic do context 'move_posts' do let(:user) { Fabricate(:user) } + let(:another_user) { Fabricate(:evil_trout) } let(:category) { Fabricate(:category, user: user) } let!(:topic) { Fabricate(:topic, user: user, category: category) } let!(:p1) { Fabricate(:post, topic: topic, user: user) } - let!(:p2) { Fabricate(:post, topic: topic, user: user)} + let!(:p2) { Fabricate(:post, topic: topic, user: another_user)} let!(:p3) { Fabricate(:post, topic: topic, user: user)} let!(:p4) { Fabricate(:post, topic: topic, user: user)} + before do + # add a like to a post + PostAction.act(another_user, p4, PostActionType.types[:like]) + end + context 'success' do it "enqueues a job to notify users" do @@ -233,10 +239,22 @@ describe Topic do new_topic.should be_present end + it "has the featured user" do + new_topic.featured_user1_id.should == another_user.id + end + + it "has the correct like_count" do + new_topic.like_count.should == 1 + end + it "has the correct category" do new_topic.category.should == category end + it "has removed the second poster from the featured users, since they moved" do + topic.featured_user1_id.should be_blank + end + it "has two posts" do new_topic.reload new_topic.posts_count.should == 2 @@ -277,6 +295,14 @@ describe Topic do topic.reload end + it "has removed the second poster from the featured users, since they moved" do + topic.featured_user1_id.should be_blank + end + + it "has the correct like_count" do + topic.like_count.should == 0 + end + it "has 2 posts now" do topic.posts_count.should == 2 end