From 2312caccdc27f7947254a1a31baa61d3d5c94eb0 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Fri, 8 Mar 2019 10:49:34 +0200 Subject: [PATCH] FEATURE: Skip small actions when counting replies in PMs. (#7108) --- app/models/topic.rb | 62 ++++++++++++++++++++++++---- lib/post_creator.rb | 6 ++- spec/components/post_creator_spec.rb | 22 ++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/app/models/topic.rb b/app/models/topic.rb index 419a06357ee..028b6fc7a55 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -527,10 +527,10 @@ class Topic < ActiveRecord::Base end # Atomically creates the next post number - def self.next_post_number(topic_id, reply = false, whisper = false) + def self.next_post_number(topic_id, opts = {}) highest = DB.query_single("SELECT coalesce(max(post_number),0) AS max FROM posts WHERE topic_id = ?", topic_id).first.to_i - if whisper + if opts[:whisper] result = DB.query_single(<<~SQL, highest, topic_id) UPDATE topics @@ -543,13 +543,15 @@ class Topic < ActiveRecord::Base else - reply_sql = reply ? ", reply_count = reply_count + 1" : "" + reply_sql = opts[:reply] ? ", reply_count = reply_count + 1" : "" + posts_sql = opts[:post] ? ", posts_count = posts_count + 1" : "" result = DB.query_single(<<~SQL, highest: highest, topic_id: topic_id) UPDATE topics SET highest_staff_post_number = :highest + 1, - highest_post_number = :highest + 1#{reply_sql}, - posts_count = posts_count + 1 + highest_post_number = :highest + 1 + #{reply_sql} + #{posts_sql} WHERE id = :topic_id RETURNING highest_post_number SQL @@ -585,6 +587,43 @@ class Topic < ActiveRecord::Base posts_count = Y.posts_count FROM X, Y WHERE + topics.archetype <> 'private_message' AND + X.topic_id = topics.id AND + Y.topic_id = topics.id AND ( + topics.highest_staff_post_number <> X.highest_post_number OR + topics.highest_post_number <> Y.highest_post_number OR + topics.last_posted_at <> Y.last_posted_at OR + topics.posts_count <> Y.posts_count + ) + SQL + + DB.exec <<~SQL + WITH + X as ( + SELECT topic_id, + COALESCE(MAX(post_number), 0) highest_post_number + FROM posts + WHERE deleted_at IS NULL + GROUP BY topic_id + ), + Y as ( + SELECT topic_id, + coalesce(MAX(post_number), 0) highest_post_number, + count(*) posts_count, + max(created_at) last_posted_at + FROM posts + WHERE deleted_at IS NULL AND post_type <> 3 AND post_type <> 4 + GROUP BY topic_id + ) + UPDATE topics + SET + highest_staff_post_number = X.highest_post_number, + highest_post_number = Y.highest_post_number, + last_posted_at = Y.last_posted_at, + posts_count = Y.posts_count + FROM X, Y + WHERE + topics.archetype = 'private_message' AND X.topic_id = topics.id AND Y.topic_id = topics.id AND ( topics.highest_staff_post_number <> X.highest_post_number OR @@ -597,32 +636,39 @@ class Topic < ActiveRecord::Base # If a post is deleted we have to update our highest post counters def self.reset_highest(topic_id) + archetype = Topic.where(id: topic_id).pluck(:archetype).first + + # ignore small_action replies for private messages + post_type = archetype == Archetype.private_message ? " AND post_type <> #{Post.types[:small_action]}" : '' + result = DB.query_single(<<~SQL, topic_id: topic_id) UPDATE topics SET - highest_staff_post_number = ( + highest_staff_post_number = ( SELECT COALESCE(MAX(post_number), 0) FROM posts WHERE topic_id = :topic_id AND deleted_at IS NULL ), - highest_post_number = ( + highest_post_number = ( SELECT COALESCE(MAX(post_number), 0) FROM posts WHERE topic_id = :topic_id AND deleted_at IS NULL AND post_type <> 4 + #{post_type} ), posts_count = ( SELECT count(*) FROM posts WHERE deleted_at IS NULL AND topic_id = :topic_id AND post_type <> 4 + #{post_type} ), - last_posted_at = ( SELECT MAX(created_at) FROM posts WHERE topic_id = :topic_id AND deleted_at IS NULL AND post_type <> 4 + #{post_type} ) WHERE id = :topic_id RETURNING highest_post_number diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 6f773199bbf..a74f43f030d 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -248,7 +248,11 @@ class PostCreator post.word_count = post.raw.scan(/[[:word:]]+/).size whisper = post.post_type == Post.types[:whisper] - post.post_number ||= Topic.next_post_number(post.topic_id, post.reply_to_post_number.present?, whisper) + increase_posts_count = !post.topic&.private_message? || post.post_type != Post.types[:small_action] + post.post_number ||= Topic.next_post_number(post.topic_id, + reply: post.reply_to_post_number.present?, + whisper: whisper, + post: increase_posts_count) cooking_options = post.cooking_options || {} cooking_options[:topic_id] = post.topic_id diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 97a4960f319..2fe93452b72 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -776,6 +776,28 @@ describe PostCreator do expect(post.topic.topic_allowed_users.where(user_id: admin2.id).count).to eq(0) end + + it 'does not increase posts count for small actions' do + topic = Fabricate(:private_message_topic, user: Fabricate(:user)) + + Fabricate(:post, topic: topic) + + 1.upto(3) do |i| + user = Fabricate(:user) + topic.invite(topic.user, user.username) + topic.reload + expect(topic.posts_count).to eq(1) + expect(topic.posts.where(post_type: Post.types[:small_action]).count).to eq(i) + end + + Fabricate(:post, topic: topic) + Topic.reset_highest(topic.id) + expect(topic.reload.posts_count).to eq(2) + + Fabricate(:post, topic: topic) + Topic.reset_all_highest! + expect(topic.reload.posts_count).to eq(3) + end end context "warnings" do