DEV: Chat thread reply counter cache (#21050)

Similar to 22a55ef0ce,
this commit adds a replies_count to the Chat::Thread
table, which is updated every 15 minutes via PeriodicalUpdates.
This is done so the new thread indicator for the UI can
show the count without intense serializer queries, but
in future we likely want this to update more frequently.

plugins/chat/app/jobs/scheduled/chat/periodical_updates.rb

@@ -9,6 +9,7 @@ module Jobs
         # TODO: Add rebaking of old messages (baked_version <
         # Chat::Message::BAKED_VERSION or baked_version IS NULL)
         ::Chat::Channel.ensure_consistency!
+        ::Chat::Thread.ensure_consistency!
         nil
       end
     end

plugins/chat/app/models/chat/channel.rb

@@ -165,32 +165,33 @@ end
 #
 # Table name: chat_channels
 #
-#  id                           :bigint           not null, primary key
-#  chatable_id                  :integer          not null
-#  deleted_at                   :datetime
-#  deleted_by_id                :integer
-#  featured_in_category_id      :integer
-#  delete_after_seconds         :integer
-#  chatable_type                :string           not null
-#  created_at                   :datetime         not null
-#  updated_at                   :datetime         not null
-#  name                         :string
-#  description                  :text
-#  status                       :integer          default("open"), not null
-#  user_count                   :integer          default(0), not null
-#  last_message_sent_at         :datetime         not null
-#  auto_join_users              :boolean          default(FALSE), not null
-#  allow_channel_wide_mentions  :boolean          default(TRUE), not null
-#  user_count_stale             :boolean          default(FALSE), not null
-#  slug                         :string
-#  type                         :string
-#  threading_enabled            :boolean          default(FALSE), not null
+#  id                          :bigint           not null, primary key
+#  chatable_id                 :integer          not null
+#  deleted_at                  :datetime
+#  deleted_by_id               :integer
+#  featured_in_category_id     :integer
+#  delete_after_seconds        :integer
+#  chatable_type               :string           not null
+#  created_at                  :datetime         not null
+#  updated_at                  :datetime         not null
+#  name                        :string
+#  description                 :text
+#  status                      :integer          default("open"), not null
+#  user_count                  :integer          default(0), not null
+#  last_message_sent_at        :datetime         not null
+#  auto_join_users             :boolean          default(FALSE), not null
+#  user_count_stale            :boolean          default(FALSE), not null
+#  slug                        :string
+#  type                        :string
+#  allow_channel_wide_mentions :boolean          default(TRUE), not null
+#  messages_count              :integer          default(0), not null
+#  threading_enabled           :boolean          default(FALSE), not null
 #
 # Indexes
 #
-#  index_chat_channels_on_messages_count             (messages_count)
 #  index_chat_channels_on_chatable_id                    (chatable_id)
 #  index_chat_channels_on_chatable_id_and_chatable_type  (chatable_id,chatable_type)
+#  index_chat_channels_on_messages_count                 (messages_count)
 #  index_chat_channels_on_slug                           (slug) UNIQUE
 #  index_chat_channels_on_status                         (status)
 #

plugins/chat/app/models/chat/message.rb

@@ -348,6 +348,7 @@ end
 # Indexes
 #
 #  idx_chat_messages_by_created_at_not_deleted            (created_at) WHERE (deleted_at IS NULL)
+#  idx_chat_messages_by_thread_id_not_deleted             (thread_id) WHERE (deleted_at IS NULL)
 #  index_chat_messages_on_chat_channel_id_and_created_at  (chat_channel_id,created_at)
 #  index_chat_messages_on_chat_channel_id_and_id          (chat_channel_id,id) WHERE (deleted_at IS NULL)
 #  index_chat_messages_on_last_editor_id                  (last_editor_id)

plugins/chat/app/models/chat/thread.rb

@@ -18,6 +18,10 @@ module Chat
 
     enum :status, { open: 0, read_only: 1, closed: 2, archived: 3 }, scopes: false
 
+    def replies
+      self.chat_messages.where.not(id: self.original_message_id)
+    end
+
     def url
       "#{channel.url}/t/#{self.id}"
     end
@@ -29,6 +33,32 @@ module Chat
     def excerpt
       original_message.excerpt(max_length: EXCERPT_LENGTH)
     end
+
+    def self.ensure_consistency!
+      update_counts
+    end
+
+    def self.update_counts
+      # NOTE: Chat::Thread#replies_count is not updated every time
+      # a message is created or deleted in a channel, the UI will lag
+      # behind unless it is kept in sync with MessageBus. The count
+      # has 1 subtracted from it to account for the original message.
+      #
+      # It is updated eventually via Jobs::Chat::PeriodicalUpdates. In
+      # future we may want to update this more frequently.
+      DB.exec <<~SQL
+        UPDATE chat_threads threads
+        SET replies_count = subquery.replies_count
+        FROM (
+          SELECT COUNT(*) - 1 AS replies_count, thread_id
+          FROM chat_messages
+          WHERE chat_messages.deleted_at IS NULL AND thread_id IS NOT NULL
+          GROUP BY thread_id
+        ) subquery
+        WHERE threads.id = subquery.thread_id
+        AND subquery.replies_count != threads.replies_count
+      SQL
+    end
   end
 end
 
@@ -44,6 +74,7 @@ end
 #  title                    :string
 #  created_at               :datetime         not null
 #  updated_at               :datetime         not null
+#  replies_count            :integer          default(0), not null
 #
 # Indexes
 #
@@ -51,5 +82,6 @@ end
 #  index_chat_threads_on_channel_id_and_status     (channel_id,status)
 #  index_chat_threads_on_original_message_id       (original_message_id)
 #  index_chat_threads_on_original_message_user_id  (original_message_user_id)
+#  index_chat_threads_on_replies_count             (replies_count)
 #  index_chat_threads_on_status                    (status)
 #

plugins/chat/db/migrate/20230411012630_add_thread_not_deleted_index_chat_messages.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+class AddThreadNotDeletedIndexChatMessages < ActiveRecord::Migration[7.0]
+  disable_ddl_transaction!
+
+  def change
+    execute <<~SQL
+      DROP INDEX IF EXISTS idx_chat_messages_by_thread_id_not_deleted
+    SQL
+
+    execute <<~SQL
+      CREATE INDEX CONCURRENTLY IF NOT EXISTS
+      idx_chat_messages_by_thread_id_not_deleted
+      ON chat_messages (thread_id)
+      WHERE deleted_at IS NULL
+    SQL
+  end
+
+  def down
+    execute <<~SQL
+      DROP INDEX IF EXISTS idx_chat_messages_by_thread_id_not_deleted
+    SQL
+  end
+end

plugins/chat/db/migrate/20230411023246_add_chat_message_replies_count_to_chat_threads.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddChatMessageRepliesCountToChatThreads < ActiveRecord::Migration[7.0]
+  def change
+    add_column :chat_threads, :replies_count, :integer, null: false, default: 0
+    add_index :chat_threads, :replies_count
+  end
+end

plugins/chat/db/post_migrate/20230411023340_update_thread_reply_counts.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+class UpdateThreadReplyCounts < ActiveRecord::Migration[7.0]
+  def up
+    DB.exec <<~SQL
+      UPDATE chat_threads threads
+      SET replies_count = subquery.replies_count
+      FROM (
+        SELECT COUNT(*) - 1 AS replies_count, thread_id
+        FROM chat_messages
+        WHERE chat_messages.deleted_at IS NULL AND thread_id IS NOT NULL
+        GROUP BY thread_id
+      ) subquery
+      WHERE threads.id = subquery.thread_id
+      AND subquery.replies_count != threads.replies_count
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

plugins/chat/spec/fabricators/chat_fabricator.rb

@@ -147,4 +147,6 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
   original_message do |attrs|
     Fabricate(:chat_message, chat_channel: attrs[:channel] || Fabricate(:chat_channel))
   end
+
+  after_create { |thread| thread.original_message.update!(thread_id: thread.id) }
 end

plugins/chat/spec/models/chat/thread_spec.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::Thread do
+  describe ".ensure_consistency!" do
+    fab!(:channel) { Fabricate(:category_channel) }
+    fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) }
+    fab!(:thread_2) { Fabricate(:chat_thread, channel: channel) }
+    fab!(:thread_3) { Fabricate(:chat_thread, channel: channel) }
+
+    before do
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
+
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_2)
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_2)
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_2)
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_2)
+
+      Fabricate(:chat_message, chat_channel: channel, thread: thread_3)
+    end
+
+    describe "updating replies_count for all threads" do
+      it "counts correctly and does not include the original message" do
+        described_class.ensure_consistency!
+        expect(thread_1.reload.replies_count).to eq(3)
+        expect(thread_2.reload.replies_count).to eq(4)
+        expect(thread_3.reload.replies_count).to eq(1)
+      end
+
+      it "does not count deleted messages" do
+        thread_1.chat_messages.last.trash!
+        described_class.ensure_consistency!
+        expect(thread_1.reload.replies_count).to eq(2)
+      end
+
+      it "sets the replies count to 0 if all the messages but the original message are deleted" do
+        thread_1.replies.delete_all
+
+        described_class.ensure_consistency!
+        expect(thread_1.reload.replies_count).to eq(0)
+      end
+    end
+  end
+end