From f134701c7b4f4d93d545c447b2e6f76b00d79feb Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Tue, 3 Jul 2018 22:50:51 -0400 Subject: [PATCH] FIX: user topic and post counts can become negative when staff deletes posts in personal messages --- lib/post_destroyer.rb | 36 ++++++++++++++------------ spec/components/post_destroyer_spec.rb | 26 +++++++++++++++++++ spec/models/topic_converter_spec.rb | 2 ++ 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/lib/post_destroyer.rb b/lib/post_destroyer.rb index 11a05b60779..790d8132769 100644 --- a/lib/post_destroyer.rb +++ b/lib/post_destroyer.rb @@ -78,21 +78,23 @@ class PostDestroyer def staff_recovered @post.recover! - if author = @post.user - if @post.is_first_post? - author.user_stat.topic_count += 1 - else - author.user_stat.post_count += 1 + if @post.topic && !@post.topic.private_message? + if author = @post.user + if @post.is_first_post? + author.user_stat.topic_count += 1 + else + author.user_stat.post_count += 1 + end + author.user_stat.save! end - author.user_stat.save! - end - if @post.is_first_post? && @post.topic && !@post.topic.private_message? - # Update stats of all people who replied - counts = Post.where(post_type: Post.types[:regular], topic_id: @post.topic_id).where('post_number > 1').group(:user_id).count - counts.each do |user_id, count| - if user_stat = UserStat.where(user_id: user_id).first - user_stat.update_attributes(post_count: user_stat.post_count + count) + if @post.is_first_post? + # Update stats of all people who replied + counts = Post.where(post_type: Post.types[:regular], topic_id: @post.topic_id).where('post_number > 1').group(:user_id).count + counts.each do |user_id, count| + if user_stat = UserStat.where(user_id: user_id).first + user_stat.update_attributes(post_count: user_stat.post_count + count) + end end end end @@ -248,10 +250,12 @@ class PostDestroyer author.user_stat.first_post_created_at = author.posts.order('created_at ASC').first.try(:created_at) end - if @post.post_type == Post.types[:regular] && !@post.is_first_post? && !@topic.nil? - author.user_stat.post_count -= 1 + if @post.topic && !@post.topic.private_message? + if @post.post_type == Post.types[:regular] && !@post.is_first_post? && !@topic.nil? + author.user_stat.post_count -= 1 + end + author.user_stat.topic_count -= 1 if @post.is_first_post? end - author.user_stat.topic_count -= 1 if @post.is_first_post? # We don't count replies to your own topics in topic_reply_count if @topic && author.id != @topic.user_id diff --git a/spec/components/post_destroyer_spec.rb b/spec/components/post_destroyer_spec.rb index 416686c0635..338d4961265 100644 --- a/spec/components/post_destroyer_spec.rb +++ b/spec/components/post_destroyer_spec.rb @@ -360,6 +360,32 @@ describe PostDestroyer do end + context 'private message' do + let(:author) { Fabricate(:user) } + let(:private_message) { Fabricate(:private_message_topic, user: author) } + let!(:first_post) { Fabricate(:post, topic: private_message, user: author) } + let!(:second_post) { Fabricate(:post, topic: private_message, user: author, post_number: 2)} + + it "doesn't update post_count for a reply" do + expect { + PostDestroyer.new(admin, second_post).destroy + author.reload + }.to_not change { author.post_count } + + expect { + PostDestroyer.new(admin, second_post).recover + }.to_not change { author.post_count } + end + + it "doesn't update topic_count for first post" do + expect { + PostDestroyer.new(admin, first_post).destroy + author.reload + }.to_not change { author.topic_count } + expect(author.post_count).to eq(0) # also unchanged + end + end + context 'deleting the second post in a topic' do let(:user) { Fabricate(:user) } diff --git a/spec/models/topic_converter_spec.rb b/spec/models/topic_converter_spec.rb index ad18f421476..d8f9d6ebf44 100644 --- a/spec/models/topic_converter_spec.rb +++ b/spec/models/topic_converter_spec.rb @@ -56,8 +56,10 @@ describe TopicConverter do first_post topic_user = TopicUser.create!(user_id: author.id, topic_id: private_message.id, posted: true) expect(private_message.user.user_stat.topic_count).to eq(0) + expect(private_message.user.user_stat.post_count).to eq(0) private_message.convert_to_public_topic(admin) expect(private_message.reload.user.user_stat.topic_count).to eq(1) + expect(private_message.user.user_stat.post_count).to eq(1) expect(topic_user.reload.notification_level).to eq(TopicUser.notification_levels[:watching]) end