DEV: Rearchitect chat tracking state (#21550)

This moves chat tracking state calculation for channels
and threads into a central Chat::TrackingStateManager service, that
serves a similar purpose to the TopicTrackingState model
in core.

This service calls down to these query classes:

* ThreadUnreadsQuery
* ChannelUnreadsQuery

To get the unread_count and mention_count for the appropriate
channels and threads.

As well as this, this commit refactors the client-side chat
tracking state.

Now, there is a central ChatTrackingStateManager Ember Service
so all tracking is accessible and can be counted from one place,
which can also initialize tracking from an initial payload.

The actual tracking counts are now maintained in a ChatTrackingState
class that is initialized on the `.tracking` property of both channel and
thread objects.

This removes the attributes on UserChatChannelMembership and decoration
of said membership from ChannelFetcher, preferring instead to have an additional
object for tracking in the JSON.

plugins/chat/spec/fabricators/chat_fabricator.rb

@@ -155,4 +155,12 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
   after_create { |thread| thread.original_message.update!(thread_id: thread.id) }
 end
 
-Fabricator(:user_chat_thread_membership, class_name: "Chat::UserChatThreadMembership") { user }
+Fabricator(:user_chat_thread_membership, class_name: "Chat::UserChatThreadMembership") do
+  user
+  after_create do |membership|
+    Chat::UserChatChannelMembership.find_or_create_by!(
+      user: membership.user,
+      chat_channel: membership.thread.channel,
+    ).update!(following: true)
+  end
+end

plugins/chat/spec/lib/chat/channel_fetcher_spec.rb

@@ -45,7 +45,7 @@ describe Chat::ChannelFetcher do
     end
   end
 
-  describe ".unread_counts" do
+  describe ".tracking_state" do
     context "when user is member of the channel" do
       before do
         Fabricate(:user_chat_channel_membership, chat_channel: category_channel, user: user1)
@@ -58,19 +58,17 @@ describe Chat::ChannelFetcher do
         end
 
         it "returns the correct count" do
-          unread_counts = described_class.unread_counts([category_channel], user1)
-          expect(
-            unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count,
-          ).to eq(2)
+          tracking_state =
+            described_class.tracking_state([category_channel.id], Guardian.new(user1))
+          expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(2)
         end
       end
 
       context "with no unread messages" do
         it "returns the correct count" do
-          unread_counts = described_class.unread_counts([category_channel], user1)
-          expect(
-            unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count,
-          ).to eq(0)
+          tracking_state =
+            described_class.tracking_state([category_channel.id], Guardian.new(user1))
+          expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0)
         end
       end
 
@@ -82,10 +80,9 @@ describe Chat::ChannelFetcher do
         before { last_unread.update!(deleted_at: Time.zone.now) }
 
         it "returns the correct count" do
-          unread_counts = described_class.unread_counts([category_channel], user1)
-          expect(
-            unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count,
-          ).to eq(0)
+          tracking_state =
+            described_class.tracking_state([category_channel.id], Guardian.new(user1))
+          expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0)
         end
       end
     end
@@ -97,10 +94,9 @@ describe Chat::ChannelFetcher do
         end
 
         it "returns the correct count" do
-          unread_counts = described_class.unread_counts([category_channel], user1)
-          expect(
-            unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count,
-          ).to eq(0)
+          tracking_state =
+            described_class.tracking_state([category_channel.id], Guardian.new(user1))
+          expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0)
         end
       end
     end
@@ -336,32 +332,17 @@ describe Chat::ChannelFetcher do
         Fabricate(:chat_message, user: user2, chat_channel: category_channel)
 
         resolved_memberships = memberships
-        described_class.secured_public_channels(
-          guardian,
-          resolved_memberships,
-          following: following,
-        )
+        result =
+          described_class.tracking_state(resolved_memberships.map(&:chat_channel_id), guardian)
 
-        expect(
-          resolved_memberships
-            .find { |membership| membership.chat_channel_id == category_channel.id }
-            .unread_count,
-        ).to eq(2)
-
-        resolved_memberships.last.update!(muted: true)
+        expect(result.channel_tracking[category_channel.id][:unread_count]).to eq(2)
 
         resolved_memberships = memberships
-        described_class.secured_public_channels(
-          guardian,
-          resolved_memberships,
-          following: following,
-        )
+        resolved_memberships.last.update!(muted: true)
+        result =
+          described_class.tracking_state(resolved_memberships.map(&:chat_channel_id), guardian)
 
-        expect(
-          resolved_memberships
-            .find { |membership| membership.chat_channel_id == category_channel.id }
-            .unread_count,
-        ).to eq(0)
+        expect(result.channel_tracking[category_channel.id][:unread_count]).to eq(0)
       end
     end
   end
@@ -422,17 +403,17 @@ describe Chat::ChannelFetcher do
       Fabricate(:chat_message, user: user2, chat_channel: direct_message_channel1)
       resolved_memberships = memberships
 
-      described_class.secured_direct_message_channels(user1.id, resolved_memberships, guardian)
       target_membership =
         resolved_memberships.find { |mem| mem.chat_channel_id == direct_message_channel1.id }
-      expect(target_membership.unread_count).to eq(2)
+      result = described_class.tracking_state([direct_message_channel1.id], guardian)
+      expect(result.channel_tracking[target_membership.chat_channel_id][:unread_count]).to eq(2)
 
       resolved_memberships = memberships
       target_membership =
         resolved_memberships.find { |mem| mem.chat_channel_id == direct_message_channel1.id }
       target_membership.update!(muted: true)
-      described_class.secured_direct_message_channels(user1.id, resolved_memberships, guardian)
-      expect(target_membership.unread_count).to eq(0)
+      result = described_class.tracking_state([direct_message_channel1.id], guardian)
+      expect(result.channel_tracking[target_membership.chat_channel_id][:unread_count]).to eq(0)
     end
   end
 

plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb

@@ -21,6 +21,21 @@ describe Chat::ChannelUnreadsQuery do
       ).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
     end
 
+    context "when the membership has been muted" do
+      before do
+        channel_1
+          .user_chat_channel_memberships
+          .find_by(user_id: current_user.id)
+          .update!(muted: true)
+      end
+
+      it "returns a zeroed unread count" do
+        expect(
+          described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
+        ).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id })
+      end
+    end
+
     context "for unread messages in a thread" do
       fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1) }
       fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) }
@@ -80,13 +95,13 @@ describe Chat::ChannelUnreadsQuery do
           ).to match_array([{ mention_count: 0, unread_count: 1, channel_id: channel_1.id }])
         end
 
-        context "when include_no_membership_channels is true" do
+        context "when include_missing_memberships is true" do
           it "does return zeroed counts for the channels" do
             expect(
               described_class.call(
                 channel_ids: [channel_1.id, channel_2.id],
                 user_id: current_user.id,
-                include_no_membership_channels: true,
+                include_missing_memberships: true,
               ).map(&:to_h),
             ).to match_array(
               [
@@ -122,6 +137,28 @@ describe Chat::ChannelUnreadsQuery do
       ).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
     end
 
+    context "for unread mentions in a thread" do
+      fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1) }
+      fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) }
+
+      it "does include the original message in the mention count" do
+        create_mention(thread_om, channel_1)
+        expect(
+          described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
+        ).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id })
+      end
+
+      it "does not include other thread messages in the mention count" do
+        thread_message_1 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
+        thread_message_2 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread)
+        create_mention(thread_message_1, channel_1)
+        create_mention(thread_message_2, channel_1)
+        expect(
+          described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h,
+        ).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id })
+      end
+    end
+
     context "for multiple channels" do
       fab!(:channel_2) { Fabricate(:category_channel) }
 

plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb

@@ -0,0 +1,176 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe Chat::ThreadUnreadsQuery do
+  fab!(:channel_1) { Fabricate(:category_channel, threading_enabled: true) }
+  fab!(:channel_2) { Fabricate(:category_channel, threading_enabled: true) }
+  fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) }
+  fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_1) }
+  fab!(:thread_3) { Fabricate(:chat_thread, channel: channel_2) }
+  fab!(:thread_4) { Fabricate(:chat_thread, channel: channel_2) }
+  fab!(:current_user) { Fabricate(:user) }
+
+  let(:params) { { user_id: current_user.id, channel_ids: channel_ids, thread_ids: thread_ids } }
+  let(:include_missing_memberships) { false }
+  let(:channel_ids) { [] }
+  let(:thread_ids) { [] }
+  let(:subject) do
+    described_class.call(
+      channel_ids: channel_ids,
+      thread_ids: thread_ids,
+      user_id: current_user.id,
+      include_missing_memberships: include_missing_memberships,
+    )
+  end
+
+  before do
+    SiteSetting.chat_enabled = true
+    SiteSetting.enable_experimental_chat_threaded_discussions = true
+    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+    channel_1.add(current_user)
+    channel_2.add(current_user)
+    thread_1.add(current_user)
+    thread_2.add(current_user)
+    thread_3.add(current_user)
+    thread_4.add(current_user)
+  end
+
+  context "with unread messages across multiple threads" do
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, thread: thread_1) }
+    fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_2, thread: thread_3) }
+    fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_2, thread: thread_4) }
+
+    context "when only the channel ids are provided" do
+      let(:channel_ids) { [channel_1.id, channel_2.id] }
+
+      it "gets a count of all the thread unreads across the channels" do
+        expect(subject.map(&:to_h)).to match_array(
+          [
+            { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 },
+            { channel_id: channel_1.id, mention_count: 0, thread_id: thread_2.id, unread_count: 0 },
+            { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 },
+            { channel_id: channel_2.id, mention_count: 0, thread_id: thread_4.id, unread_count: 1 },
+          ],
+        )
+      end
+
+      it "does not count deleted messages" do
+        message_1.trash!
+        expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+        )
+      end
+
+      it "does not messages in threads where threading_enabled is false on the channel" do
+        channel_1.update!(threading_enabled: false)
+        expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+        )
+        expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_2.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_2.id, unread_count: 0 },
+        )
+      end
+
+      it "does not count as unread if the last_read_message_id is greater than or equal to the message id" do
+        thread_1
+          .user_chat_thread_memberships
+          .find_by(user: current_user)
+          .update!(last_read_message_id: message_1.id)
+        expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+        )
+      end
+
+      it "does not count the original message ID as unread" do
+        thread_1.original_message.destroy
+        thread_1.update!(original_message: message_1)
+        expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+        )
+      end
+    end
+
+    context "when only the thread_ids are provided" do
+      let(:thread_ids) { [thread_1.id, thread_3.id] }
+
+      it "gets a count of all the thread unreads for the specified threads" do
+        expect(subject.map(&:to_h)).to match_array(
+          [
+            { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 },
+            { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 },
+          ],
+        )
+      end
+
+      context "when the notification_level for the thread is muted" do
+        before do
+          thread_1
+            .user_chat_thread_memberships
+            .find_by(user: current_user)
+            .update!(notification_level: :muted)
+        end
+
+        it "gets a zeroed out count for the thread" do
+          expect(subject.map(&:to_h)).to include(
+            { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+          )
+        end
+      end
+
+      context "when the user is not a member of a thread" do
+        before { thread_1.user_chat_thread_memberships.find_by(user: current_user).destroy! }
+
+        it "does not get that thread unread count by default" do
+          expect(subject.map(&:to_h)).to match_array(
+            [
+              {
+                channel_id: channel_2.id,
+                mention_count: 0,
+                thread_id: thread_3.id,
+                unread_count: 1,
+              },
+            ],
+          )
+        end
+
+        context "when include_missing_memberships is true" do
+          let(:include_missing_memberships) { true }
+
+          it "includes the thread that the user is not a member of with zeroed out counts" do
+            expect(subject.map(&:to_h)).to match_array(
+              [
+                {
+                  channel_id: channel_1.id,
+                  mention_count: 0,
+                  thread_id: thread_1.id,
+                  unread_count: 0,
+                },
+                {
+                  channel_id: channel_2.id,
+                  mention_count: 0,
+                  thread_id: thread_3.id,
+                  unread_count: 1,
+                },
+              ],
+            )
+          end
+        end
+      end
+    end
+
+    context "when channel_ids and thread_ids are provided" do
+      let(:channel_ids) { [channel_1.id, channel_2.id] }
+      let(:thread_ids) { [thread_1.id, thread_3.id] }
+
+      it "gets a count of all the thread unreads across the channels filtered by thread id" do
+        expect(subject.map(&:to_h)).to match_array(
+          [
+            { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 },
+            { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 },
+          ],
+        )
+      end
+    end
+  end
+end

plugins/chat/spec/services/chat/tracking_state_spec.rb

@@ -0,0 +1,184 @@
+# frozen_string_literal: true
+
+RSpec.describe ::Chat::TrackingState do
+  describe ".call" do
+    subject(:result) { described_class.call(params) }
+
+    fab!(:current_user) { Fabricate(:user) }
+    fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
+    fab!(:channel_2) { Fabricate(:chat_channel, threading_enabled: true) }
+    fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) }
+    fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_1) }
+    fab!(:thread_3) { Fabricate(:chat_thread, channel: channel_2) }
+    fab!(:thread_4) { Fabricate(:chat_thread, channel: channel_2) }
+
+    let(:guardian) { Guardian.new(current_user) }
+    let(:id_params) { { channel_ids: [channel_1.id], thread_ids: [thread_1.id] } }
+    let(:include_threads) { nil }
+    let(:include_missing_memberships) { nil }
+
+    let(:params) do
+      id_params.merge(guardian: guardian).merge(
+        include_threads: include_threads,
+        include_missing_memberships: include_missing_memberships,
+      )
+    end
+
+    context "when enable_experimental_chat_threaded_discussions is disabled" do
+      before { SiteSetting.enable_experimental_chat_threaded_discussions = false }
+
+      context "when include_threads is true" do
+        let(:include_threads) { true }
+        it { is_expected.to fail_a_policy(:threaded_discussions_settings_ok) }
+      end
+
+      context "when include_threads is false" do
+        let(:include_threads) { false }
+        it { is_expected.not_to fail_a_policy(:threaded_discussions_settings_ok) }
+      end
+    end
+
+    context "when enable_experimental_chat_threaded_discussions is enabled" do
+      before { SiteSetting.enable_experimental_chat_threaded_discussions = true }
+
+      let(:include_threads) { true }
+      fab!(:channel_1_membership) do
+        Fabricate(:user_chat_channel_membership, chat_channel: channel_1, user: current_user)
+      end
+      fab!(:thread_1_membership) do
+        Fabricate(:user_chat_thread_membership, thread: thread_1, user: current_user)
+      end
+      fab!(:thread_2_membership) do
+        Fabricate(:user_chat_thread_membership, thread: thread_2, user: current_user)
+      end
+
+      context "when not including channels and threads where the user is not a member" do
+        context "when only channel_ids are provided" do
+          let(:id_params) { { channel_ids: [channel_1.id, channel_2.id] } }
+
+          it "gets the tracking state of the channels" do
+            generate_tracking_state
+            expect(result.report.channel_tracking).to eq(
+              channel_1.id => {
+                unread_count: 4, # 2 messages + 2 thread original messages
+                mention_count: 0,
+              },
+            )
+          end
+
+          it "gets the tracking state of the threads in the channels" do
+            generate_tracking_state
+            expect(result.report.thread_tracking).to eq(
+              thread_1.id => {
+                channel_id: channel_1.id,
+                unread_count: 1,
+                mention_count: 0,
+              },
+              thread_2.id => {
+                channel_id: channel_1.id,
+                unread_count: 2,
+                mention_count: 0,
+              },
+            )
+          end
+
+          context "when include_threads is false" do
+            let(:include_threads) { false }
+
+            it "only gets channel tracking state and no thread tracking state" do
+              generate_tracking_state
+              expect(result.report.thread_tracking).to eq({})
+              expect(result.report.channel_tracking).to eq(
+                channel_1.id => {
+                  unread_count: 4, # 2 messages + 2 thread original messages
+                  mention_count: 0,
+                },
+              )
+            end
+          end
+        end
+
+        context "when thread_ids and channel_ids are provided" do
+          let(:id_params) do
+            { channel_ids: [channel_1.id, channel_2.id], thread_ids: [thread_2.id] }
+          end
+
+          it "gets the tracking state of the channels" do
+            generate_tracking_state
+            expect(result.report.channel_tracking).to eq(
+              channel_1.id => {
+                unread_count: 4, # 2 messages + 2 thread original messages
+                mention_count: 0,
+              },
+            )
+          end
+
+          it "only gets the tracking state of the specified threads in the channels" do
+            generate_tracking_state
+            expect(result.report.thread_tracking).to eq(
+              thread_2.id => {
+                channel_id: channel_1.id,
+                unread_count: 2,
+                mention_count: 0,
+              },
+            )
+          end
+        end
+      end
+
+      context "when including channels and threads where the user is not a member" do
+        let(:id_params) { { channel_ids: [channel_1.id, channel_2.id] } }
+        let(:include_missing_memberships) { true }
+        let(:include_threads) { true }
+
+        it "gets the tracking state of all channels including the ones where the user is not a member" do
+          generate_tracking_state
+          expect(result.report.channel_tracking).to eq(
+            channel_1.id => {
+              unread_count: 4, # 2 messages + 2 thread original messages
+              mention_count: 0,
+            },
+            channel_2.id => {
+              unread_count: 0,
+              mention_count: 0,
+            },
+          )
+        end
+
+        it "gets the tracking state of all the threads in the channels including the ones where the user is not a member" do
+          generate_tracking_state
+          expect(result.report.thread_tracking).to eq(
+            thread_1.id => {
+              channel_id: channel_1.id,
+              unread_count: 1,
+              mention_count: 0,
+            },
+            thread_2.id => {
+              channel_id: channel_1.id,
+              unread_count: 2,
+              mention_count: 0,
+            },
+            thread_3.id => {
+              channel_id: channel_2.id,
+              unread_count: 0,
+              mention_count: 0,
+            },
+            thread_4.id => {
+              channel_id: channel_2.id,
+              unread_count: 0,
+              mention_count: 0,
+            },
+          )
+        end
+      end
+    end
+  end
+
+  def generate_tracking_state
+    Fabricate(:chat_message, chat_channel: channel_1)
+    Fabricate(:chat_message, chat_channel: channel_1)
+    Fabricate(:chat_message, chat_channel: channel_1, thread: thread_1)
+    Fabricate(:chat_message, chat_channel: channel_1, thread: thread_2)
+    Fabricate(:chat_message, chat_channel: channel_1, thread: thread_2)
+  end
+end