From 91588cf93870057e52689dcc7b80b228ef6f1edb Mon Sep 17 00:00:00 2001
From: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Date: Tue, 11 Jul 2023 10:49:27 +0800
Subject: [PATCH] DEV: Improve composer-messages implementation for PMs
 (#22529)

What does this commit do??

This commit introduces two changes:

1. As a follow up review comment to
   cc463c3e9bed468defe5399ee4fa16216ed96264, we remove the top level
recipientNames cache in composer message to be a property of the
`ComposerMessage` component instead. Across components, we're more
likely to get a cache miss than a hit since we're caching the entire
recipient array so we can just drop it. If we really need this
optimisation, we should probably use a map and cache the information for
each user instead. However, the request is fairly cheap so we avoid that
optimisation for now.

2. This commit adds a debounce to `_typeReply` as well since we were not
   debouncing and the method was being called each time we received the
event.
---
 .../app/components/composer-messages.js       | 17 ++++++-------
 .../acceptance/composer-messages-test.js      | 25 +++++++++++++++++++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/components/composer-messages.js b/app/assets/javascripts/discourse/app/components/composer-messages.js
index 282fade88d7..28d7c1b5d57 100644
--- a/app/assets/javascripts/discourse/app/components/composer-messages.js
+++ b/app/assets/javascripts/discourse/app/components/composer-messages.js
@@ -7,13 +7,10 @@ import { not } from "@ember/object/computed";
 import { ajax } from "discourse/lib/ajax";
 import { inject as service } from "@ember/service";
 import { tracked } from "@glimmer/tracking";
+import { INPUT_DELAY } from "discourse-common/config/environment";
+import { debounce } from "discourse-common/utils/decorators";
 
 let _messagesCache = {};
-let _recipientNames = [];
-
-const clearRecipientNamesCache = function () {
-  _recipientNames.clear();
-};
 
 @classNameBindings(":composer-popup-container", "hidden")
 export default class ComposerMessages extends Component {
@@ -26,6 +23,7 @@ export default class ComposerMessages extends Component {
   queuedForTyping = null;
   similarTopics = null;
   usersNotSeen = null;
+  recipientNames = [];
 
   @not("composer.viewOpenOrFullscreen") hidden;
 
@@ -51,8 +49,6 @@ export default class ComposerMessages extends Component {
     this.appEvents.off("composer:find-similar", this, this._findSimilar);
     this.appEvents.off("composer-messages:close", this, this._closeTop);
     this.appEvents.off("composer-messages:create", this, this._create);
-
-    clearRecipientNamesCache();
   }
 
   _closeTop() {
@@ -92,6 +88,7 @@ export default class ComposerMessages extends Component {
 
   // Called after the user has typed a reply.
   // Some messages only get shown after being typed.
+  @debounce(INPUT_DELAY)
   async _typedReply() {
     if (this.isDestroying || this.isDestroyed) {
       return;
@@ -128,10 +125,10 @@ export default class ComposerMessages extends Component {
 
       if (
         recipient_names.length > 0 &&
-        recipient_names.length !== _recipientNames.length &&
-        !recipient_names.every((v, i) => v === _recipientNames[i])
+        recipient_names.length !== this.recipientNames.length &&
+        !recipient_names.every((v, i) => v === this.recipientNames[i])
       ) {
-        _recipientNames = recipient_names;
+        this.recipientNames = recipient_names;
 
         const response = await ajax(
           `/composer_messages/user_not_seen_in_a_while`,
diff --git a/app/assets/javascripts/discourse/tests/acceptance/composer-messages-test.js b/app/assets/javascripts/discourse/tests/acceptance/composer-messages-test.js
index b864e19ecca..bbb54a45e5b 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/composer-messages-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/composer-messages-test.js
@@ -18,8 +18,16 @@ import pretender, { response } from "../helpers/create-pretender";
 acceptance("Composer - Messages", function (needs) {
   needs.user();
 
+  let userNotSeenRequestCount = 0;
+
+  needs.hooks.afterEach(() => {
+    userNotSeenRequestCount = 0;
+  });
+
   needs.pretender((server, helper) => {
     server.get("/composer_messages/user_not_seen_in_a_while", () => {
+      userNotSeenRequestCount += 1;
+
       return helper.response({
         user_count: 1,
         usernames: ["charlie"],
@@ -31,13 +39,16 @@ acceptance("Composer - Messages", function (needs) {
   test("Shows warning in composer if user hasn't been seen in a long time.", async function (assert) {
     await visit("/u/charlie");
     await click("button.compose-pm");
+
     assert.false(
       exists(".composer-popup"),
       "composer warning is not shown by default"
     );
 
     await triggerKeyEvent(".d-editor-input", "keyup", "Space");
+
     assert.true(exists(".composer-popup"), "shows composer warning message");
+
     assert.true(
       query(".composer-popup").innerHTML.includes(
         I18n.t("composer.user_not_seen_in_a_while.single", {
@@ -47,6 +58,20 @@ acceptance("Composer - Messages", function (needs) {
       ),
       "warning message has correct body"
     );
+
+    assert.strictEqual(
+      userNotSeenRequestCount,
+      1,
+      "ne user not seen request is made to the server"
+    );
+
+    await triggerKeyEvent(".d-editor-input", "keyup", "Space");
+
+    assert.strictEqual(
+      userNotSeenRequestCount,
+      1,
+      "does not make additional user not seen request to the server if the recipient names are the same"
+    );
   });
 });