FEATURE: chat-replying indicator for threads (#21485)

This feature adds the replying indicator in threads, it uses the same `/chat-reply/CHANNEL_ID` prefix than the channel composer replying indicator as we don't have specific right on threads ATM (if you can access channel, you can access thread). Thread will however use a presence channel name of the following format: `/chat-reply/CHANNEL_ID/thread/THREAD_ID`

This commit also simplifies the computation of `users` to eventually avoid a race-condition leading to a leak of the indicator in another channel/thread.

<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->

plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs

@@ -93,7 +93,9 @@
 
   {{#if this.shouldRenderReplyingIndicator}}
     <div class="chat-replying-indicator-container">
-      <ChatReplyingIndicator @channel={{@channel}} />
+      <ChatReplyingIndicator
+        @presenceChannelName={{this.presenceChannelName}}
+      />
     </div>
   {{/if}}
 

plugins/chat/assets/javascripts/discourse/components/chat-composer.js

@@ -35,9 +35,10 @@ export default class ChatComposer extends Component {
 
   @tracked isFocused = false;
   @tracked inProgressUploadsCount = 0;
+  @tracked presenceChannelName;
 
   get shouldRenderReplyingIndicator() {
-    return this.context === "channel" && !this.args.channel?.isDraft;
+    return !this.args.channel?.isDraft;
   }
 
   get shouldRenderMessageDetails() {
@@ -248,7 +249,7 @@ export default class ChatComposer extends Component {
     }
 
     this.chatComposerPresenceManager.notifyState(
-      this.args.channel.id,
+      this.presenceChannelName,
       !this.currentMessage.editing && this.hasContent
     );
   }

plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.hbs

@@ -1,11 +1,11 @@
-{{#if @channel}}
+{{#if @presenceChannelName}}
   <div
     class={{concat-class
       "chat-replying-indicator"
       (if this.presenceChannel.subscribed "is-subscribed")
     }}
     {{did-insert this.subscribe}}
-    {{did-update this.updateSubscription @channel.id}}
+    {{did-update this.updateSubscription @presenceChannelName}}
     {{will-destroy this.unsubscribe}}
   >
     {{#if this.shouldRender}}

plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.js

@@ -10,7 +10,6 @@ export default class ChatReplyingIndicator extends Component {
   @service presence;
 
   @tracked presenceChannel = null;
-  @tracked users = [];
 
   @action
   async updateSubscription() {
@@ -20,31 +19,29 @@ export default class ChatReplyingIndicator extends Component {
 
   @action
   async subscribe() {
-    this.presenceChannel = this.presence.getChannel(this.channelName);
+    this.presenceChannel = this.presence.getChannel(
+      this.args.presenceChannelName
+    );
     await this.presenceChannel.subscribe();
-    this.users = this.presenceChannel.users || [];
-    this.presenceChannel.on("change", this.handlePresenceChange);
   }
 
   @action
   async unsubscribe() {
-    this.users = [];
-
-    if (this.presenceChannel.subscribed) {
-      this.presenceChannel.off("change", this.handlePresenceChange);
+    if (this.presenceChannel?.subscribed) {
       await this.presenceChannel.unsubscribe();
     }
   }
 
-  @action
-  handlePresenceChange(presenceChannel) {
-    this.users = presenceChannel.users || [];
+  get users() {
+    return (
+      this.presenceChannel
+        ?.get("users")
+        ?.filter((u) => u.id !== this.currentUser.id) || []
+    );
   }
 
   get usernames() {
-    return this.users
-      .filter((u) => u.id !== this.currentUser.id)
-      .mapBy("username");
+    return this.users.mapBy("username");
   }
 
   get text() {
@@ -77,8 +74,4 @@ export default class ChatReplyingIndicator extends Component {
   get shouldRender() {
     return isPresent(this.usernames);
   }
-
-  get channelName() {
-    return `/chat-reply/${this.args.channel.id}`;
-  }
 }

plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs

@@ -60,6 +60,7 @@
   {{else}}
     <Chat::Composer::Thread
       @channel={{this.channel}}
+      @thread={{this.thread}}
       @onSendMessage={{this.onSendMessage}}
       @uploadDropZone={{this.uploadDropZone}}
     />

plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js

@@ -12,6 +12,11 @@ export default class ChatComposerChannel extends ChatComposer {
 
   composerId = "channel-composer";
 
+  get presenceChannelName() {
+    const channel = this.args.channel;
+    return `/chat-reply/${channel.id}`;
+  }
+
   @action
   persistDraft() {
     if (this.args.channel?.isDraft) {

plugins/chat/assets/javascripts/discourse/components/chat/composer/thread.js

@@ -13,6 +13,10 @@ export default class ChatComposerThread extends ChatComposer {
 
   composerId = "thread-composer";
 
+  get presenceChannelName() {
+    return `/chat-reply/${this.args.channel.id}/thread/${this.args.thread.id}`;
+  }
+
   get placeholder() {
     return I18n.t("chat.placeholder_thread");
   }

plugins/chat/assets/javascripts/discourse/services/chat-composer-presence-manager.js

@@ -2,7 +2,6 @@ import Service, { inject as service } from "@ember/service";
 import { cancel, debounce } from "@ember/runloop";
 import { isTesting } from "discourse-common/config/environment";
 
-const CHAT_PRESENCE_CHANNEL_PREFIX = "/chat-reply";
 const KEEP_ALIVE_DURATION_SECONDS = 10;
 
 // This service is loosely based on discourse-presence's ComposerPresenceManager service
@@ -16,15 +15,15 @@ export default class ChatComposerPresenceManager extends Service {
     this.leave();
   }
 
-  notifyState(chatChannelId, replying) {
+  notifyState(channelName, replying) {
     if (!replying) {
       this.leave();
       return;
     }
 
-    if (this._chatChannelId !== chatChannelId) {
-      this._enter(chatChannelId);
-      this._chatChannelId = chatChannelId;
+    if (this._channelName !== channelName) {
+      this._enter(channelName);
+      this._channelName = channelName;
     }
 
     if (!isTesting()) {
@@ -39,17 +38,16 @@ export default class ChatComposerPresenceManager extends Service {
   leave() {
     this._presentChannel?.leave();
     this._presentChannel = null;
-    this._chatChannelId = null;
+    this._channelName = null;
     if (this._autoLeaveTimer) {
       cancel(this._autoLeaveTimer);
       this._autoLeaveTimer = null;
     }
   }
 
-  _enter(chatChannelId) {
+  _enter(channelName) {
     this.leave();
 
-    let channelName = `${CHAT_PRESENCE_CHANNEL_PREFIX}/${chatChannelId}`;
     this._presentChannel = this.presence.getChannel(channelName);
     this._presentChannel.enter();
   }

plugins/chat/assets/stylesheets/common/chat-thread.scss

@@ -27,8 +27,4 @@
     display: flex;
     flex-direction: column;
   }
-
-  .chat-composer__wrapper {
-    padding-bottom: 27px;
-  }
 }

plugins/chat/test/javascripts/components/chat-replying-indicator-test.js

@@ -4,35 +4,81 @@ import hbs from "htmlbars-inline-precompile";
 import fabricators from "../helpers/fabricators";
 import { module, test } from "qunit";
 import { render } from "@ember/test-helpers";
-import { joinChannel } from "discourse/tests/helpers/presence-pretender";
+import {
+  joinChannel,
+  leaveChannel,
+} from "discourse/tests/helpers/presence-pretender";
 
-async function addUserToChannel(channelId, id, username) {
-  await joinChannel(`/chat-reply/${channelId}`, {
+async function addUser(id, username, channelName = "/chat-reply/1") {
+  await joinChannel(channelName, {
     id,
     avatar_template: "/images/avatar.png",
     username,
   });
 }
 
+async function removeUser(id, channelName = "/chat-reply/1") {
+  await leaveChannel(channelName, {
+    id,
+  });
+}
+
 module(
   "Discourse Chat | Component | chat-replying-indicator",
   function (hooks) {
     setupRenderingTest(hooks);
 
     test("not displayed when no one is replying", async function (assert) {
-      this.channel = fabricators.chatChannel();
-
-      await render(hbs`<ChatReplyingIndicator @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName="/chat-reply/1" />`
+      );
 
       assert.dom(".chat-replying-indicator__text").doesNotExist();
     });
 
+    test("working for thread", async function (assert) {
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName="/chat-reply/1/thread/1" />`
+      );
+
+      await addUser(1, "sam", "/chat-reply/1/thread/1");
+
+      assert.strictEqual(
+        query(".chat-replying-indicator__text").innerText,
+        "sam is typing"
+      );
+    });
+
+    test("doesn’t leak in other indicators", async function (assert) {
+      await render(
+        hbs`
+          <div class="channel"><ChatReplyingIndicator @presenceChannelName="/chat-reply/1" /></div>
+          <div class="thread"><ChatReplyingIndicator @presenceChannelName="/chat-reply/1/thread/1" /></div>
+        `
+      );
+
+      await addUser(1, "sam");
+
+      assert
+        .dom(".channel .chat-replying-indicator__text")
+        .hasText("sam is typing");
+      assert.dom(".thread .chat-replying-indicator__text").doesNotExist();
+
+      await addUser(2, "mark", "/chat-reply/1/thread/1");
+      await removeUser(1);
+
+      assert.dom(".channel .chat-replying-indicator__text").doesNotExist();
+      assert
+        .dom(".thread .chat-replying-indicator__text")
+        .hasText("mark is typing");
+    });
+
     test("displays indicator when user is replying", async function (assert) {
-      this.channel = fabricators.chatChannel();
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName="/chat-reply/1" />`
+      );
 
-      await render(hbs`<ChatReplyingIndicator @channel={{this.channel}} />`);
-
-      await addUserToChannel(1, 1, "sam");
+      await addUser(1, "sam");
 
       assert.strictEqual(
         query(".chat-replying-indicator__text").innerText,
@@ -43,10 +89,12 @@ module(
     test("displays indicator when 2 or 3 users are replying", async function (assert) {
       this.channel = fabricators.chatChannel();
 
-      await render(hbs`<ChatReplyingIndicator @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName="/chat-reply/1" />`
+      );
 
-      await addUserToChannel(1, 1, "sam");
-      await addUserToChannel(1, 2, "mark");
+      await addUser(1, "sam");
+      await addUser(2, "mark");
 
       assert
         .dom(".chat-replying-indicator__text")
@@ -56,11 +104,13 @@ module(
     test("displays indicator when 3 users are replying", async function (assert) {
       this.channel = fabricators.chatChannel();
 
-      await render(hbs`<ChatReplyingIndicator @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName="/chat-reply/1" />`
+      );
 
-      await addUserToChannel(1, 1, "sam");
-      await addUserToChannel(1, 2, "mark");
-      await addUserToChannel(1, 3, "joffrey");
+      await addUser(1, "sam");
+      await addUser(2, "mark");
+      await addUser(3, "joffrey");
 
       assert
         .dom(".chat-replying-indicator__text")
@@ -70,12 +120,14 @@ module(
     test("displays indicator when more than 3 users are replying", async function (assert) {
       this.channel = fabricators.chatChannel();
 
-      await render(hbs`<ChatReplyingIndicator  @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator  @presenceChannelName="/chat-reply/1" />`
+      );
 
-      await addUserToChannel(1, 1, "sam");
-      await addUserToChannel(1, 2, "mark");
-      await addUserToChannel(1, 3, "joffrey");
-      await addUserToChannel(1, 4, "taylor");
+      await addUser(1, "sam");
+      await addUser(2, "mark");
+      await addUser(3, "joffrey");
+      await addUser(4, "taylor");
 
       assert
         .dom(".chat-replying-indicator__text")
@@ -85,24 +137,28 @@ module(
     test("filters current user from list of repliers", async function (assert) {
       this.channel = fabricators.chatChannel();
 
-      await render(hbs`<ChatReplyingIndicator  @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator  @presenceChannelName="/chat-reply/1" />`
+      );
 
-      await addUserToChannel(1, 1, "sam");
-      await addUserToChannel(1, this.currentUser.id, this.currentUser.username);
+      await addUser(1, "sam");
+      await addUser(this.currentUser.id, this.currentUser.username);
 
       assert.dom(".chat-replying-indicator__text").hasText("sam is typing");
     });
 
     test("resets presence when channel changes", async function (assert) {
-      this.set("channel", fabricators.chatChannel());
+      this.set("presenceChannelName", "/chat-reply/1");
 
-      await addUserToChannel(1, 1, "sam");
+      await addUser(1, "sam");
 
-      await render(hbs`<ChatReplyingIndicator @channel={{this.channel}} />`);
+      await render(
+        hbs`<ChatReplyingIndicator @presenceChannelName={{this.presenceChannelName}} />`
+      );
 
       assert.dom(".chat-replying-indicator__text").hasText("sam is typing");
 
-      this.set("channel", fabricators.chatChannel({ id: 2 }));
+      this.set("presenceChannelName", "/chat-reply/2");
 
       assert.dom(".chat-replying-indicator__text").doesNotExist();
     });