FIX: be consistent with how first posts in topics are counted. do like DirectoryItem.refresh_period :all

This commit is contained in:
Neil Lalonde 2017-11-09 18:05:53 -05:00
parent 0ccefb0365
commit 9dc9ca4ac0
7 changed files with 68 additions and 21 deletions

View File

@ -17,7 +17,7 @@ class AnonymousShadowCreator
if (shadow_id = user.custom_fields["shadow_id"].to_i) > 0 if (shadow_id = user.custom_fields["shadow_id"].to_i) > 0
shadow = User.find_by(id: shadow_id) shadow = User.find_by(id: shadow_id)
if shadow && shadow.post_count > 0 && if shadow && (shadow.post_count + shadow.topic_count) > 0 &&
shadow.last_posted_at < SiteSetting.anonymous_account_duration_minutes.minutes.ago shadow.last_posted_at < SiteSetting.anonymous_account_duration_minutes.minutes.ago
shadow = nil shadow = nil
end end

View File

@ -9,7 +9,7 @@ module UserGuardian
return false if (SiteSetting.sso_overrides_username? && SiteSetting.enable_sso?) return false if (SiteSetting.sso_overrides_username? && SiteSetting.enable_sso?)
return true if is_staff? return true if is_staff?
return false if SiteSetting.username_change_period <= 0 return false if SiteSetting.username_change_period <= 0
is_me?(user) && (user.post_count == 0 || user.created_at > SiteSetting.username_change_period.days.ago) is_me?(user) && ((user.post_count + user.topic_count) == 0 || user.created_at > SiteSetting.username_change_period.days.ago)
end end
def can_edit_email?(user) def can_edit_email?(user)

View File

@ -473,7 +473,7 @@ class PostCreator
end end
unless @post.topic.private_message? unless @post.topic.private_message?
@user.user_stat.post_count += 1 if @post.post_type == Post.types[:regular] @user.user_stat.post_count += 1 if @post.post_type == Post.types[:regular] && !@post.is_first_post?
@user.user_stat.topic_count += 1 if @post.is_first_post? @user.user_stat.topic_count += 1 if @post.is_first_post?
end end

View File

@ -76,12 +76,26 @@ class PostDestroyer
@post.recover! @post.recover!
if author = @post.user if author = @post.user
if @post.is_first_post?
author.user_stat.topic_count += 1
else
author.user_stat.post_count += 1 author.user_stat.post_count += 1
end
author.user_stat.save! author.user_stat.save!
end 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)
end
end
end
@post.publish_change_to_clients! :recovered @post.publish_change_to_clients! :recovered
TopicTrackingState.publish_recover(@post.topic) if @post.topic && @post.post_number == 1 TopicTrackingState.publish_recover(@post.topic) if @post.topic && @post.is_first_post?
end end
# When a post is properly deleted. Well, it's still soft deleted, but it will no longer # When a post is properly deleted. Well, it's still soft deleted, but it will no longer
@ -231,12 +245,12 @@ class PostDestroyer
author.user_stat.first_post_created_at = author.posts.order('created_at ASC').first.try(:created_at) author.user_stat.first_post_created_at = author.posts.order('created_at ASC').first.try(:created_at)
end end
if @post.post_type == Post.types[:regular] && !(@topic.nil? && !@post.is_first_post?) if @post.post_type == Post.types[:regular] && !@post.is_first_post? && !@topic.nil?
author.user_stat.post_count -= 1 author.user_stat.post_count -= 1
end end
author.user_stat.topic_count -= 1 if @post.is_first_post? author.user_stat.topic_count -= 1 if @post.is_first_post?
# We don't count replies to your own topics # We don't count replies to your own topics in topic_reply_count
if @topic && author.id != @topic.user_id if @topic && author.id != @topic.user_id
author.user_stat.update_topic_reply_count author.user_stat.update_topic_reply_count
end end
@ -250,7 +264,7 @@ class PostDestroyer
if @post.is_first_post? && @post.topic && !@post.topic.private_message? if @post.is_first_post? && @post.topic && !@post.topic.private_message?
# Update stats of all people who replied # Update stats of all people who replied
counts = Post.where(post_type: Post.types[:regular]).where(topic_id: @post.topic_id).group(:user_id).count 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| counts.each do |user_id, count|
if user_stat = UserStat.where(user_id: user_id).first if user_stat = UserStat.where(user_id: user_id).first
user_stat.update_attributes(post_count: user_stat.post_count - count) user_stat.update_attributes(post_count: user_stat.post_count - count)

View File

@ -399,7 +399,7 @@ describe PostCreator do
raw: 'this is a whispered reply').create raw: 'this is a whispered reply').create
# don't count whispers in user stats # don't count whispers in user stats
expect(user_stat.reload.post_count).to eq(1) expect(user_stat.reload.post_count).to eq(0)
expect(whisper).to be_present expect(whisper).to be_present
expect(whisper.post_type).to eq(Post.types[:whisper]) expect(whisper.post_type).to eq(Post.types[:whisper])
@ -413,7 +413,7 @@ describe PostCreator do
expect(whisper_reply).to be_present expect(whisper_reply).to be_present
expect(whisper_reply.post_type).to eq(Post.types[:whisper]) expect(whisper_reply.post_type).to eq(Post.types[:whisper])
expect(user_stat.reload.post_count).to eq(1) expect(user_stat.reload.post_count).to eq(0)
# date is not precise enough in db # date is not precise enough in db
whisper_reply.reload whisper_reply.reload

View File

@ -167,26 +167,45 @@ describe PostDestroyer do
before do before do
post post
@user = post.user @user = post.user
@reply = create_post(topic: post.topic, user: @user)
expect(@user.user_stat.post_count).to eq(1) expect(@user.user_stat.post_count).to eq(1)
end end
context "recovered by user" do context "recovered by user" do
it "should increment the user's post count" do it "should increment the user's post count" do
PostDestroyer.new(@user, post).destroy PostDestroyer.new(@user, @reply).destroy
expect(@user.user_stat.topic_count).to eq(1)
expect(@user.user_stat.post_count).to eq(1) expect(@user.user_stat.post_count).to eq(1)
PostDestroyer.new(@user, post.reload).recover PostDestroyer.new(@user, @reply.reload).recover
expect(@user.user_stat.topic_count).to eq(1)
expect(@user.reload.user_stat.post_count).to eq(1) expect(@user.reload.user_stat.post_count).to eq(1)
expect(UserAction.where(target_topic_id: post.topic_id, action_type: UserAction::NEW_TOPIC).count).to eq(1)
expect(UserAction.where(target_topic_id: post.topic_id, action_type: UserAction::REPLY).count).to eq(1)
end end
end end
context "recovered by admin" do context "recovered by admin" do
it "should increment the user's post count" do it "should increment the user's post count" do
PostDestroyer.new(moderator, @reply).destroy
expect(@user.reload.user_stat.topic_count).to eq(1)
expect(@user.user_stat.post_count).to eq(0)
PostDestroyer.new(admin, @reply).recover
expect(@user.reload.user_stat.topic_count).to eq(1)
expect(@user.user_stat.post_count).to eq(1)
PostDestroyer.new(moderator, post).destroy PostDestroyer.new(moderator, post).destroy
expect(@user.reload.user_stat.topic_count).to eq(0)
expect(@user.user_stat.post_count).to eq(0) expect(@user.user_stat.post_count).to eq(0)
PostDestroyer.new(admin, post).recover PostDestroyer.new(admin, post).recover
expect(@user.reload.user_stat.post_count).to eq(1) expect(@user.reload.user_stat.topic_count).to eq(1)
expect(@user.user_stat.post_count).to eq(1)
expect(UserAction.where(target_topic_id: post.topic_id, action_type: UserAction::NEW_TOPIC).count).to eq(1)
expect(UserAction.where(target_topic_id: post.topic_id, action_type: UserAction::REPLY).count).to eq(1)
end end
end end
end end
@ -218,7 +237,8 @@ describe PostDestroyer do
expect(post2.raw).to eq(I18n.t('js.post.deleted_by_author', count: 24)) expect(post2.raw).to eq(I18n.t('js.post.deleted_by_author', count: 24))
expect(post2.version).to eq(2) expect(post2.version).to eq(2)
expect(called).to eq(1) expect(called).to eq(1)
expect(user_stat.reload.post_count).to eq(1) expect(user_stat.reload.post_count).to eq(0)
expect(user_stat.reload.topic_count).to eq(1)
called = 0 called = 0
topic_recovered = -> (topic, user) do topic_recovered = -> (topic, user) do
@ -236,7 +256,8 @@ describe PostDestroyer do
expect(post2.user_deleted).to eq(false) expect(post2.user_deleted).to eq(false)
expect(post2.cooked).to eq(@orig) expect(post2.cooked).to eq(@orig)
expect(called).to eq(1) expect(called).to eq(1)
expect(user_stat.reload.post_count).to eq(1) expect(user_stat.reload.post_count).to eq(0)
expect(user_stat.reload.topic_count).to eq(1)
ensure ensure
DiscourseEvent.off(:topic_destroyed, &topic_destroyed) DiscourseEvent.off(:topic_destroyed, &topic_destroyed)
DiscourseEvent.off(:topic_recovered, &topic_recovered) DiscourseEvent.off(:topic_recovered, &topic_recovered)
@ -251,7 +272,7 @@ describe PostDestroyer do
reply = create_post(topic_id: post.topic_id, user: user2) reply = create_post(topic_id: post.topic_id, user: user2)
reply2 = create_post(topic_id: post.topic_id, user: user1) reply2 = create_post(topic_id: post.topic_id, user: user1)
expect(user1.user_stat.topic_count).to eq(1) expect(user1.user_stat.topic_count).to eq(1)
expect(user1.user_stat.post_count).to eq(2) expect(user1.user_stat.post_count).to eq(1)
expect(user2.user_stat.topic_count).to eq(0) expect(user2.user_stat.topic_count).to eq(0)
expect(user2.user_stat.post_count).to eq(1) expect(user2.user_stat.post_count).to eq(1)
PostDestroyer.new(Fabricate(:admin), post).destroy PostDestroyer.new(Fabricate(:admin), post).destroy
@ -279,14 +300,15 @@ describe PostDestroyer do
context "as a moderator" do context "as a moderator" do
it "deletes the post" do it "deletes the post" do
author = post.user author = post.user
reply = create_post(topic_id: post.topic_id, user: author)
post_count = author.post_count post_count = author.post_count
history_count = UserHistory.count history_count = UserHistory.count
PostDestroyer.new(moderator, post).destroy PostDestroyer.new(moderator, reply).destroy
expect(post.deleted_at).to be_present expect(reply.deleted_at).to be_present
expect(post.deleted_by).to eq(moderator) expect(reply.deleted_by).to eq(moderator)
author.reload author.reload
expect(author.post_count).to eq(post_count - 1) expect(author.post_count).to eq(post_count - 1)
@ -301,12 +323,23 @@ describe PostDestroyer do
expect(post.deleted_by).to eq(admin) expect(post.deleted_by).to eq(admin)
end end
it "updates the user's post_count" do it "updates the user's topic_count for first post" do
author = post.user author = post.user
expect { expect {
PostDestroyer.new(admin, post).destroy PostDestroyer.new(admin, post).destroy
author.reload author.reload
}.to change { author.topic_count }.by(-1)
expect(author.user_stat.post_count).to eq(0)
end
it "updates the user's post_count for reply" do
author = post.user
reply = create_post(topic: post.topic, user: author)
expect {
PostDestroyer.new(admin, reply).destroy
author.reload
}.to change { author.post_count }.by(-1) }.to change { author.post_count }.by(-1)
expect(author.user_stat.topic_count).to eq(1)
end end
it "doesn't count whispers" do it "doesn't count whispers" do

View File

@ -2148,7 +2148,7 @@ describe UsersController do
json = JSON.parse(response.body) json = JSON.parse(response.body)
expect(json["user_summary"]["topic_count"]).to eq(1) expect(json["user_summary"]["topic_count"]).to eq(1)
expect(json["user_summary"]["post_count"]).to eq(1) expect(json["user_summary"]["post_count"]).to eq(0)
end end
end end