From 85e1a4934b1ecdb632a8f081bb2dae55b3c13c6e Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 3 Feb 2023 15:38:30 -0300 Subject: [PATCH] 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. --- .../discourse/components/chat-composer.js | 54 +------ .../discourse/components/chat-live-pane.hbs | 8 +- .../discourse/components/chat-live-pane.js | 89 ----------- .../components/chat-mention-warnings.hbs | 6 +- .../components/chat-mention-warnings.js | 40 +++-- .../chat-composer-warnings-tracker.js | 146 ++++++++++++++++++ .../chat/spec/system/mention_warnings_spec.rb | 76 +++++++++ 7 files changed, 257 insertions(+), 162 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js create mode 100644 plugins/chat/spec/system/mention_warnings_spec.rb diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index 88f02c986af..e71bdc4cab7 100644 --- 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); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs index a6a17db58fb..c979299974d 100644 --- 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 @@ - +
{{else}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js index 42fdf413451..9fb539a0ea6 100644 --- 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) { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs index 104a4bb92dc..c02e048596d 100644 --- 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}}
    - {{#if @tooManyMentions}} + {{#if this.hasTooManyMentions}}
  • {{this.tooManyMentionsBody}}
    • {{else}} - {{#if @unreachableGroupMentions}} + {{#if this.hasUnreachableGroupMentions}}
  • {{this.unreachableBody}}
    • {{/if}} - {{#if @overMembersLimitGroupMentions}} + {{#if this.hasOverMembersLimitGroupMentions}}
  • {{this.overMembersLimitBody}}
    • {{/if}} {{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js index e97bfefbda3..8ea0880fab2 100644 --- 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, diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js b/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js new file mode 100644 index 00000000000..165d327729d --- /dev/null +++ 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]; + } +} diff --git a/plugins/chat/spec/system/mention_warnings_spec.rb b/plugins/chat/spec/system/mention_warnings_spec.rb new file mode 100644 index 00000000000..e217b9bb29b --- /dev/null +++ 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