REFACTOR: Move mention warnings logic into a separate service. (#19465)

First follow-up to the feature introduced in #19034. We don't want to pollute main components like `chat-live-pane`, so we'll use a service to track and manage the state needed to display before-send warnings while composing a chat message. We also move from acceptance tests to system specs.
2023-02-03 15:38:30 -03:00 · 2023-02-03 15:38:30 -03:00 · 85e1a4934b
parent 132e802d5d
commit 85e1a4934b
7 changed files with 257 additions and 162 deletions
--- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js
@ -21,15 +21,12 @@ import { Promise } from "rsvp";
 import { translations } from "pretty-text/emoji/data";
 import { channelStatusName } from "discourse/plugins/chat/discourse/models/chat-channel";
 import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete";
-import discourseDebounce from "discourse-common/lib/debounce";
 import {
  chatComposerButtons,
  chatComposerButtonsDependentKeys,
 } from "discourse/plugins/chat/discourse/lib/chat-composer-buttons";
-import { mentionRegex } from "pretty-text/mentions";

 const THROTTLE_MS = 150;
-const MENTION_DEBOUNCE_MS = 1000;

 export default Component.extend(TextareaTextManipulation, {
  chatChannel: null,
@ -44,7 +41,6 @@ export default Component.extend(TextareaTextManipulation, {
  editingMessage: null,
  onValueChange: null,
  timer: null,
-  mentionsTimer: null,
  value: "",
  inProgressUploads: null,
  composerEventPrefix: "chat",
@ -52,6 +48,7 @@ export default Component.extend(TextareaTextManipulation, {
  canAttachUploads: reads("siteSettings.chat_allow_uploads"),
  isNetworkUnreliable: reads("chat.isNetworkUnreliable"),
  typingMention: false,
+  chatComposerWarningsTracker: service(),

  @discourseComputed(...chatComposerButtonsDependentKeys())
  inlineButtons() {
@ -150,7 +147,6 @@ export default Component.extend(TextareaTextManipulation, {
    );

    cancel(this.timer);
-    cancel(this.mentionsTimer);

    this.appEvents.off("chat:focus-composer", this, "_focusTextArea");
    this.appEvents.off("chat:insert-text", this, "insertText");
@ -233,7 +229,7 @@ export default Component.extend(TextareaTextManipulation, {
        replyToMsg: this.draft.replyToMsg,
      });

-      this._debouncedCaptureMentions();
+      this._captureMentions();
      this._syncUploads(this.draft.uploads);
      this.setInReplyToMsg(this.draft.replyToMsg);
    }
@ -298,12 +294,7 @@ export default Component.extend(TextareaTextManipulation, {
    this.set("value", value);
    this.resizeTextarea();

-    this.typingMention = value.slice(-1) === "@";
-
-    if (this.typingMention && value.slice(-1) === " ") {
-      this.typingMention = false;
-      this._debouncedCaptureMentions();
-    }
+    this._captureMentions();

    // throttle, not debounce, because we do eventually want to react during the typing
    this.timer = throttle(this, this._handleTextareaInput, THROTTLE_MS);
@ -315,42 +306,9 @@ export default Component.extend(TextareaTextManipulation, {
    this.onValueChange?.(this.value, this._uploads, this.replyToMsg);
  },

-  @bind
-  _debouncedCaptureMentions() {
-    this.mentionsTimer = discourseDebounce(
-      this,
-      this._captureMentions,
-      MENTION_DEBOUNCE_MS
-    );
-  },
-
  @bind
  _captureMentions() {
-    if (this.siteSettings.enable_mentions) {
-      const mentions = this._extractMentions();
-      this.onMentionUpdates(mentions);
-    }
-  },
-
-  _extractMentions() {
-    let message = this.value;
-    const regex = mentionRegex(this.siteSettings.unicode_usernames);
-    const mentions = [];
-    let mentionsLeft = true;
-
-    while (mentionsLeft) {
-      const matches = message.match(regex);
-
-      if (matches) {
-        const mention = matches[1] || matches[2];
-        mentions.push(mention);
-        message = message.replaceAll(`${mention}`, "");
-      } else {
-        mentionsLeft = false;
-      }
-    }
-
-    return mentions;
+    this.chatComposerWarningsTracker.trackMentions(this.value);
  },

  @bind
@ -412,7 +370,7 @@ export default Component.extend(TextareaTextManipulation, {
        afterComplete: (text) => {
          this.set("value", text);
          this._focusTextArea();
-          this._debouncedCaptureMentions();
+          this._captureMentions();
        },
      });
    }
@ -728,7 +686,7 @@ export default Component.extend(TextareaTextManipulation, {
      value: "",
      inReplyMsg: null,
    });
-    this.onMentionUpdates([]);
+    this._captureMentions();
    this._syncUploads([]);
    this._focusTextArea({ ensureAtEnd: true, resizeTextarea: true });
    this.onValueChange?.(this.value, this._uploads, this.replyToMsg);
--- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs
@ -40,12 +40,7 @@

 <ChatRetentionReminder @chatChannel={{this.chatChannel}} />

-<ChatMentionWarnings
-  @unreachableGroupMentions={{this.unreachableGroupMentions}}
-  @overMembersLimitGroupMentions={{this.overMembersLimitGroupMentions}}
-  @tooManyMentions={{this.tooManyMentions}}
-  @mentionsCount={{this.mentionsCount}}
-/>
+<ChatMentionWarnings />

 <div class="chat-message-actions-mobile-anchor"></div>
 <div
@ -147,7 +142,6 @@
      @onEditLastMessageRequested={{this.editLastMessageRequested}}
      @onValueChange={{action "composerValueChanged"}}
      @chatChannel={{this.chatChannel}}
-      @onMentionUpdates={{this.updateMentions}}
    />
  {{else}}
    <ChatChannelPreviewCard @channel={{this.chatChannel}} />
--- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js
@ -37,12 +37,6 @@ const FETCH_MORE_MESSAGES_THROTTLE_MS = isTesting() ? 0 : 500;
 const PAST = "past";
 const FUTURE = "future";

-const MENTION_RESULT = {
-  invalid: -1,
-  unreachable: 0,
-  over_members_limit: 1,
-};
-
 export default Component.extend({
  classNameBindings: [":chat-live-pane", "sendingLoading", "loading"],
  chatChannel: null,
@ -73,14 +67,6 @@ export default Component.extend({
  targetMessageId: null,
  hasNewMessages: null,

-  // Track mention hints to display warnings
-  unreachableGroupMentions: null, // Array
-  overMembersLimitGroupMentions: null, // Array
-  tooManyMentions: false,
-  mentionsCount: null,
-  // Complimentary structure to avoid repeating mention checks.
-  _mentionWarningsSeen: null, // Hash
-
  chat: service(),
  chatChannelsManager: service(),
  router: service(),
@ -1326,81 +1312,6 @@ export default Component.extend({
    }
  },

-  @action
-  updateMentions(mentions) {
-    const mentionsCount = mentions?.length;
-    this.set("mentionsCount", mentionsCount);
-
-    if (mentionsCount > 0) {
-      if (mentionsCount > this.siteSettings.max_mentions_per_chat_message) {
-        this.set("tooManyMentions", true);
-      } else {
-        this.set("tooManyMentions", false);
-        const newMentions = mentions.filter(
-          (mention) => !(mention in this._mentionWarningsSeen)
-        );
-
-        if (newMentions?.length > 0) {
-          this._recordNewWarnings(newMentions, mentions);
-        } else {
-          this._rebuildWarnings(mentions);
-        }
-      }
-    } else {
-      this.set("tooManyMentions", false);
-      this.set("unreachableGroupMentions", []);
-      this.set("overMembersLimitGroupMentions", []);
-    }
-  },
-
-  _recordNewWarnings(newMentions, mentions) {
-    ajax("/chat/api/mentions/groups.json", {
-      data: { mentions: newMentions },
-    })
-      .then((newWarnings) => {
-        newWarnings.unreachable.forEach((warning) => {
-          this._mentionWarningsSeen[warning] = MENTION_RESULT["unreachable"];
-        });
-
-        newWarnings.over_members_limit.forEach((warning) => {
-          this._mentionWarningsSeen[warning] =
-            MENTION_RESULT["over_members_limit"];
-        });
-
-        newWarnings.invalid.forEach((warning) => {
-          this._mentionWarningsSeen[warning] = MENTION_RESULT["invalid"];
-        });
-
-        this._rebuildWarnings(mentions);
-      })
-      .catch(this._rebuildWarnings(mentions));
-  },
-
-  _rebuildWarnings(mentions) {
-    const newWarnings = mentions.reduce(
-      (memo, mention) => {
-        if (
-          mention in this._mentionWarningsSeen &&
-          !(this._mentionWarningsSeen[mention] === MENTION_RESULT["invalid"])
-        ) {
-          if (
-            this._mentionWarningsSeen[mention] === MENTION_RESULT["unreachable"]
-          ) {
-            memo[0].push(mention);
-          } else {
-            memo[1].push(mention);
-          }
-        }
-
-        return memo;
-      },
-      [[], []]
-    );
-
-    this.set("unreachableGroupMentions", newWarnings[0]);
-    this.set("overMembersLimitGroupMentions", newWarnings[1]);
-  },
-
  @action
  reStickScrollIfNeeded() {
    if (this.stickyScroll) {
--- a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs
@ -8,13 +8,13 @@
        {{this.warningHeaderText}}
      </div>
      <ul class={{this.listStyleClass}}>
-        {{#if @tooManyMentions}}
+        {{#if this.hasTooManyMentions}}
          <li>{{this.tooManyMentionsBody}}</li>
        {{else}}
-          {{#if @unreachableGroupMentions}}
+          {{#if this.hasUnreachableGroupMentions}}
            <li>{{this.unreachableBody}}</li>
          {{/if}}
-          {{#if @overMembersLimitGroupMentions}}
+          {{#if this.hasOverMembersLimitGroupMentions}}
            <li>{{this.overMembersLimitBody}}</li>
          {{/if}}
        {{/if}}
--- a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js
@ -6,17 +6,30 @@ import { inject as service } from "@ember/service";
 export default class ChatMentionWarnings extends Component {
  @service siteSettings;
  @service currentUser;
+  @service chatComposerWarningsTracker;

-  get unreachableGroupMentionsCount() {
-    return this.args?.unreachableGroupMentions.length;
+  get unreachableGroupMentions() {
+    return this.chatComposerWarningsTracker.unreachableGroupMentions;
  }

-  get overMembersLimitMentionsCount() {
-    return this.args?.overMembersLimitGroupMentions.length;
+  get overMembersLimitGroupMentions() {
+    return this.chatComposerWarningsTracker.overMembersLimitGroupMentions;
  }

  get hasTooManyMentions() {
-    return this.args?.tooManyMentions;
+    return this.chatComposerWarningsTracker.tooManyMentions;
+  }
+
+  get mentionsCount() {
+    return this.chatComposerWarningsTracker.mentionsCount;
+  }
+
+  get unreachableGroupMentionsCount() {
+    return this.unreachableGroupMentions.length;
+  }
+
+  get overMembersLimitMentionsCount() {
+    return this.overMembersLimitGroupMentions.length;
  }

  get hasUnreachableGroupMentions() {
@ -54,10 +67,7 @@ export default class ChatMentionWarnings extends Component {
  }

  get warningHeaderText() {
-    if (
-      this.args?.mentionsCount <= this.warningsCount ||
-      this.hasTooManyMentions
-    ) {
+    if (this.mentionsCount <= this.warningsCount || this.hasTooManyMentions) {
      return I18n.t("chat.mention_warning.groups.header.all");
    } else {
      return I18n.t("chat.mention_warning.groups.header.some");
@ -103,13 +113,13 @@ export default class ChatMentionWarnings extends Component {

    if (this.unreachableGroupMentionsCount <= 2) {
      return I18n.t("chat.mention_warning.groups.unreachable", {
-        group: this.args.unreachableGroupMentions[0],
-        group_2: this.args.unreachableGroupMentions[1],
+        group: this.unreachableGroupMentions[0],
+        group_2: this.unreachableGroupMentions[1],
        count: this.unreachableGroupMentionsCount,
      });
    } else {
      return I18n.t("chat.mention_warning.groups.unreachable_multiple", {
-        group: this.args.unreachableGroupMentions[0],
+        group: this.unreachableGroupMentions[0],
        count: this.unreachableGroupMentionsCount - 1, //N others
      });
    }
@ -142,8 +152,8 @@ export default class ChatMentionWarnings extends Component {
    if (this.hasOverMembersLimitGroupMentions <= 2) {
      return htmlSafe(
        I18n.t("chat.mention_warning.groups.too_many_members", {
-          group: this.args.overMembersLimitGroupMentions[0],
-          group_2: this.args.overMembersLimitGroupMentions[1],
+          group: this.overMembersLimitGroupMentions[0],
+          group_2: this.overMembersLimitGroupMentions[1],
          count: this.overMembersLimitMentionsCount,
          notification_limit: notificationLimit,
          limit: settingLimit,
@ -152,7 +162,7 @@ export default class ChatMentionWarnings extends Component {
    } else {
      return htmlSafe(
        I18n.t("chat.mention_warning.groups.too_many_members_multiple", {
-          group: this.args.overMembersLimitGroupMentions[0],
+          group: this.overMembersLimitGroupMentions[0],
          count: this.overMembersLimitMentionsCount - 1, //N others
          notification_limit: notificationLimit,
          limit: settingLimit,
--- a/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js
@ -0,0 +1,146 @@
+import Service, { inject as service } from "@ember/service";
+import { ajax } from "discourse/lib/ajax";
+import discourseDebounce from "discourse-common/lib/debounce";
+import { bind } from "discourse-common/utils/decorators";
+import { mentionRegex } from "pretty-text/mentions";
+import { cancel } from "@ember/runloop";
+import { tracked } from "@glimmer/tracking";
+
+const MENTION_RESULT = {
+  invalid: -1,
+  unreachable: 0,
+  over_members_limit: 1,
+};
+
+const MENTION_DEBOUNCE_MS = 1000;
+
+export default class ChatComposerWarningsTracker extends Service {
+  @service siteSettings;
+
+  // Track mention hints to display warnings
+  @tracked unreachableGroupMentions = [];
+  @tracked overMembersLimitGroupMentions = [];
+  @tracked tooManyMentions = false;
+  @tracked mentionsCount = 0;
+  @tracked mentionsTimer = null;
+
+  // Complimentary structure to avoid repeating mention checks.
+  _mentionWarningsSeen = {};
+
+  willDestroy() {
+    cancel(this.mentionsTimer);
+  }
+
+  @bind
+  trackMentions(message) {
+    this.mentionsTimer = discourseDebounce(
+      this,
+      this._trackMentions,
+      message,
+      MENTION_DEBOUNCE_MS
+    );
+  }
+
+  @bind
+  _trackMentions(message) {
+    if (!this.siteSettings.enable_mentions) {
+      return;
+    }
+
+    const mentions = this._extractMentions(message);
+    this.mentionsCount = mentions?.length;
+
+    if (this.mentionsCount > 0) {
+      this.tooManyMentions =
+        this.mentionsCount > this.siteSettings.max_mentions_per_chat_message;
+
+      if (!this.tooManyMentions) {
+        const newMentions = mentions.filter(
+          (mention) => !(mention in this._mentionWarningsSeen)
+        );
+
+        if (newMentions?.length > 0) {
+          this._recordNewWarnings(newMentions, mentions);
+        } else {
+          this._rebuildWarnings(mentions);
+        }
+      }
+    } else {
+      this.tooManyMentions = false;
+      this.unreachableGroupMentions = [];
+      this.overMembersLimitGroupMentions = [];
+    }
+  }
+
+  _extractMentions(message) {
+    const regex = mentionRegex(this.siteSettings.unicode_usernames);
+    const mentions = [];
+    let mentionsLeft = true;
+
+    while (mentionsLeft) {
+      const matches = message.match(regex);
+
+      if (matches) {
+        const mention = matches[1] || matches[2];
+        mentions.push(mention);
+        message = message.replaceAll(`${mention}`, "");
+
+        if (mentions.length > this.siteSettings.max_mentions_per_chat_message) {
+          mentionsLeft = false;
+        }
+      } else {
+        mentionsLeft = false;
+      }
+    }
+
+    return mentions;
+  }
+
+  _recordNewWarnings(newMentions, mentions) {
+    ajax("/chat/api/mentions/groups.json", {
+      data: { mentions: newMentions },
+    })
+      .then((newWarnings) => {
+        newWarnings.unreachable.forEach((warning) => {
+          this._mentionWarningsSeen[warning] = MENTION_RESULT["unreachable"];
+        });
+
+        newWarnings.over_members_limit.forEach((warning) => {
+          this._mentionWarningsSeen[warning] =
+            MENTION_RESULT["over_members_limit"];
+        });
+
+        newWarnings.invalid.forEach((warning) => {
+          this._mentionWarningsSeen[warning] = MENTION_RESULT["invalid"];
+        });
+
+        this._rebuildWarnings(mentions);
+      })
+      .catch(this._rebuildWarnings(mentions));
+  }
+
+  _rebuildWarnings(mentions) {
+    const newWarnings = mentions.reduce(
+      (memo, mention) => {
+        if (
+          mention in this._mentionWarningsSeen &&
+          !(this._mentionWarningsSeen[mention] === MENTION_RESULT["invalid"])
+        ) {
+          if (
+            this._mentionWarningsSeen[mention] === MENTION_RESULT["unreachable"]
+          ) {
+            memo[0].push(mention);
+          } else {
+            memo[1].push(mention);
+          }
+        }
+
+        return memo;
+      },
+      [[], []]
+    );
+
+    this.unreachableGroupMentions = newWarnings[0];
+    this.overMembersLimitGroupMentions = newWarnings[1];
+  }
+}
--- a/plugins/chat/spec/system/mention_warnings_spec.rb
+++ b/plugins/chat/spec/system/mention_warnings_spec.rb
@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+RSpec.describe "Mentions warnings", type: :system, js: true do
+  fab!(:current_user) { Fabricate(:user) }
+  fab!(:channel_1) { Fabricate(:chat_channel) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new }
+
+  before do
+    chat_system_bootstrap(current_user, [channel_1])
+    sign_in(current_user)
+  end
+
+  describe "composing a message with mentions" do
+    context "when mentioning a group" do
+      context "when the group doesn’t allow mentions" do
+        fab!(:admin_mentionable_group) do
+          Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:only_admins])
+        end
+
+        it "displays a warning" do
+          chat_page.visit_channel(channel_1)
+          expect(chat_channel_page).to have_no_loading_skeleton
+          chat_channel_page.type_in_composer("@#{admin_mentionable_group.name} ")
+
+          expect(page).to have_css(".chat-mention-warnings")
+          expect(page.find(".chat-mention-warnings-list__simple")).to have_content(
+            admin_mentionable_group.name,
+          )
+        end
+      end
+
+      context "when the group allow mentions" do
+        fab!(:publicly_mentionable_group) do
+          Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone])
+        end
+        fab!(:user_2) { Fabricate(:user) }
+
+        context "when the group has too many members" do
+          before do
+            SiteSetting.max_users_notified_per_group_mention = 1
+            publicly_mentionable_group.add(user_2)
+            publicly_mentionable_group.add(Fabricate(:user))
+          end
+
+          it "displays a warning" do
+            chat_page.visit_channel(channel_1)
+            expect(chat_channel_page).to have_no_loading_skeleton
+            chat_channel_page.type_in_composer("@#{publicly_mentionable_group.name} ")
+
+            expect(page).to have_css(".chat-mention-warnings")
+            expect(page.find(".chat-mention-warnings-list__simple")).to have_content(
+              publicly_mentionable_group.name,
+            )
+          end
+        end
+
+        context "when typing too many mentions" do
+          before { SiteSetting.max_mentions_per_chat_message = 1 }
+
+          it "displays a warning" do
+            chat_page.visit_channel(channel_1)
+            expect(chat_channel_page).to have_no_loading_skeleton
+            chat_channel_page.type_in_composer(
+              "@#{user_2.username} @#{publicly_mentionable_group.name} ",
+            )
+
+            expect(page).to have_css(".chat-mention-warnings")
+            expect(page.find(".chat-mention-warnings-list__simple")).to be_present
+          end
+        end
+      end
+    end
+  end
+end