From 68eba53e09285c5496d60f08ce327f29ce0c97e1 Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Tue, 22 Aug 2023 15:54:35 -0500 Subject: [PATCH] FEATURE: Chat global mention warnings (pre-send & post-send) (#22764) This is also fixes the issue of chat composer warnings persisting across channels. Currently if you try to mention more groups than is allowed for example, a mention warning pops up. When you change channels the mention warning will not disappear even if there is no text in the composer. This adds a reset function to the chat-composer-warnings-tracker.js, which is called when the channel is changed and the message is empty. In the event that the message is not empty we call captureMentions to check the loaded drafts' mentions. This PR would be nicer if the post-send notice used the new chat notices API to publish the mention warnings but we would have to change the existing ones and I thought that would be too much change for this PR. It'd be a good followup though. --- plugins/chat/app/services/chat/publisher.rb | 4 +- .../discourse/components/chat-composer.js | 8 +++- .../components/chat-mention-warnings.hbs | 5 +++ .../components/chat-mention-warnings.js | 5 +++ .../chat/message/mention-warning.hbs | 6 +++ .../chat/message/mention-warning.js | 3 +- .../models/chat-message-mention-warning.js | 2 + .../chat-composer-warnings-tracker.js | 29 ++++++++++++-- plugins/chat/config/locales/client.en.yml | 1 + plugins/chat/lib/chat/notifier.rb | 12 +++++- plugins/chat/spec/lib/chat/notifier_spec.rb | 16 ++++++++ .../chat/spec/system/mention_warnings_spec.rb | 40 ++++++++++++++++++- .../chat-message-mention-warning-test.js | 16 ++++++++ 13 files changed, 137 insertions(+), 10 deletions(-) diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 5d5a39bdadd..b463dd3b8b8 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -393,7 +393,8 @@ module Chat cannot_chat_users, without_membership, too_many_members, - mentions_disabled + mentions_disabled, + global_mentions_disabled ) MessageBus.publish( "/chat/#{chat_message.chat_channel_id}", @@ -405,6 +406,7 @@ module Chat without_membership.map { |u| { username: u.username, id: u.id } }.as_json, groups_with_too_many_members: too_many_members.map(&:name).as_json, group_mentions_disabled: mentions_disabled.map(&:name).as_json, + global_mentions_disabled: global_mentions_disabled, }, user_ids: [user_id], ) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index 246736584e0..0e54b5f6dcf 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js @@ -98,6 +98,7 @@ export default class ChatComposer extends Component { this.cancelPersistDraft(); this.composer.textarea.value = this.currentMessage.message; this.persistDraft(); + this.captureMentions({ skipDebounce: true }); } @action @@ -372,11 +373,14 @@ export default class ChatComposer extends Component { } @action - captureMentions() { + captureMentions(opts = { skipDebounce: false }) { if (this.hasContent) { this.chatComposerWarningsTracker.trackMentions( - this.currentMessage.message + this.currentMessage, + opts.skipDebounce ); + } else { + this.chatComposerWarningsTracker.reset(); } } 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 c02e048596d..52a17d19235 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.hbs @@ -11,6 +11,11 @@ {{#if this.hasTooManyMentions}}
  • {{this.tooManyMentionsBody}}
    • {{else}} + {{#if this.channelWideMentionDisallowed}} +
  • {{i18n + "chat.mention_warning.channel_wide_mentions_disallowed" + }}
    • + {{/if}} {{#if this.hasUnreachableGroupMentions}}
  • {{this.unreachableBody}}
    • {{/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 24592fec121..d7811739ca9 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-mention-warnings.js @@ -21,6 +21,10 @@ export default class ChatMentionWarnings extends Component { return this.chatComposerWarningsTracker.tooManyMentions; } + get channelWideMentionDisallowed() { + return this.chatComposerWarningsTracker.channelWideMentionDisallowed; + } + get mentionsCount() { return this.chatComposerWarningsTracker.mentionsCount; } @@ -50,6 +54,7 @@ export default class ChatMentionWarnings extends Component { get show() { return ( this.hasTooManyMentions || + this.channelWideMentionDisallowed || this.hasUnreachableGroupMentions || this.hasOverMembersLimitGroupMentions ); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.hbs index 705e809fb29..eb4b33e4bee 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.hbs @@ -51,6 +51,12 @@ {{this.groupsWithTooManyMembers}}

    {{/if}} + + {{#if this.mentionWarning.globalMentionsDisabled}} +

    + {{i18n "chat.mention_warning.channel_wide_mentions_disallowed"}} +

    + {{/if}} {{/if}} {{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.js b/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.js index ae2849da2c4..5f787646328 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/message/mention-warning.js @@ -38,7 +38,8 @@ export default class ChatMessageMentionWarning extends Component { (this.mentionWarning.groupWithMentionsDisabled?.length || this.mentionWarning.cannotSee?.length || this.mentionWarning.withoutMembership?.length || - this.mentionWarning.groupsWithTooManyMembers?.length) + this.mentionWarning.groupsWithTooManyMembers?.length || + this.mentionWarning.globalMentionsDisabled) ); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message-mention-warning.js b/plugins/chat/assets/javascripts/discourse/models/chat-message-mention-warning.js index 0aec8f7e16e..e837edd4a9b 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message-mention-warning.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-message-mention-warning.js @@ -10,6 +10,7 @@ export default class ChatMessageMentionWarning { @tracked withoutMembership; @tracked groupsWithTooManyMembers; @tracked groupWithMentionsDisabled; + @tracked globalMentionsDisabled; constructor(message, args = {}) { this.message = args.message; @@ -17,5 +18,6 @@ export default class ChatMessageMentionWarning { this.withoutMembership = args.without_membership; this.groupsWithTooManyMembers = args.groups_with_too_many_members; this.groupWithMentionsDisabled = args.group_mentions_disabled; + this.globalMentionsDisabled = args.global_mentions_disabled; } } 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 index 165d327729d..43a8d4adaad 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-composer-warnings-tracker.js @@ -21,6 +21,7 @@ export default class ChatComposerWarningsTracker extends Service { @tracked unreachableGroupMentions = []; @tracked overMembersLimitGroupMentions = []; @tracked tooManyMentions = false; + @tracked channelWideMentionDisallowed = false; @tracked mentionsCount = 0; @tracked mentionsTimer = null; @@ -32,22 +33,37 @@ export default class ChatComposerWarningsTracker extends Service { } @bind - trackMentions(message) { + reset() { + this.unreachableGroupMentions = []; + this.unreachableGroupMentions = []; + this.overMembersLimitGroupMentions = []; + this.tooManyMentions = false; + this.channelWideMentionDisallowed = false; + this.mentionsCount = 0; + cancel(this.mentionsTimer); + } + + @bind + trackMentions(currentMessage, skipDebounce) { + if (skipDebounce) { + return this._trackMentions(currentMessage); + } + this.mentionsTimer = discourseDebounce( this, this._trackMentions, - message, + currentMessage, MENTION_DEBOUNCE_MS ); } @bind - _trackMentions(message) { + _trackMentions(currentMessage) { if (!this.siteSettings.enable_mentions) { return; } - const mentions = this._extractMentions(message); + const mentions = this._extractMentions(currentMessage.message); this.mentionsCount = mentions?.length; if (this.mentionsCount > 0) { @@ -59,6 +75,10 @@ export default class ChatComposerWarningsTracker extends Service { (mention) => !(mention in this._mentionWarningsSeen) ); + this.channelWideMentionDisallowed = + !currentMessage.channel.allowChannelWideMentions && + (mentions.includes("here") || mentions.includes("all")); + if (newMentions?.length > 0) { this._recordNewWarnings(newMentions, mentions); } else { @@ -67,6 +87,7 @@ export default class ChatComposerWarningsTracker extends Service { } } else { this.tooManyMentions = false; + this.channelWideMentionDisallowed = false; this.unreachableGroupMentions = []; this.overMembersLimitGroupMentions = []; } diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index e47267add2a..7407013ceb8 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -146,6 +146,7 @@ en: group_mentions_disabled_multiple: one: "%{group_name} and %{count} other group don't allow mentions." other: "%{group_name} and %{count} other groups don't allow mentions." + channel_wide_mentions_disallowed: "@here and @all mentions are disabled in this channel." too_many_members: "%{group_name} has too many members. No one was notified." too_many_members_multiple: one: "%{group_name} and %{count} other group have too many members. No one was notified." diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index 052d222b4da..32c6c746f8b 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -228,7 +228,7 @@ module Chat group_mentions_disabled = @parsed_mentions.groups_with_disabled_mentions.to_a too_many_members = @parsed_mentions.groups_with_too_many_members.to_a if inaccessible.values.all?(&:blank?) && group_mentions_disabled.empty? && - too_many_members.empty? + too_many_members.empty? && !global_mentions_disabled return end @@ -239,9 +239,19 @@ module Chat inaccessible[:welcome_to_join].to_a, too_many_members, group_mentions_disabled, + global_mentions_disabled, ) end + def global_mentions_disabled + return @global_mentions_disabled if defined?(@global_mentions_disabled) + + @global_mentions_disabled = + (@parsed_mentions.has_global_mention || @parsed_mentions.has_here_mention) && + !@chat_channel.allow_channel_wide_mentions + @global_mentions_disabled + end + # Filters out users from global, here, group, and direct mentions that are # ignoring or muting the creator of the message, so they will not receive # a notification via the Jobs::Chat::NotifyMentioned job and are not prompted for diff --git a/plugins/chat/spec/lib/chat/notifier_spec.rb b/plugins/chat/spec/lib/chat/notifier_spec.rb index f892bcca6a4..e0af4a70e7d 100644 --- a/plugins/chat/spec/lib/chat/notifier_spec.rb +++ b/plugins/chat/spec/lib/chat/notifier_spec.rb @@ -58,6 +58,22 @@ describe Chat::Notifier do expect(to_notify[list_key]).to be_empty end + it "will publish a mention warning" do + channel.update!(allow_channel_wide_mentions: false) + msg = build_cooked_msg(mention, user_1) + + messages = + MessageBus.track_publish("/chat/#{channel.id}") do + to_notify = described_class.new(msg, msg.created_at).notify_new + end + + global_mentions_disabled_message = messages.first + + expect(global_mentions_disabled_message).to be_present + expect(global_mentions_disabled_message.data[:type].to_sym).to eq(:mention_warning) + expect(global_mentions_disabled_message.data[:global_mentions_disabled]).to eq(true) + end + it "includes all members of a channel except the sender" do msg = build_cooked_msg(mention, user_1) diff --git a/plugins/chat/spec/system/mention_warnings_spec.rb b/plugins/chat/spec/system/mention_warnings_spec.rb index 530c68c2909..20ce8fd79a8 100644 --- a/plugins/chat/spec/system/mention_warnings_spec.rb +++ b/plugins/chat/spec/system/mention_warnings_spec.rb @@ -3,12 +3,13 @@ RSpec.describe "Mentions warnings", type: :system do fab!(:current_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:channel_2) { 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]) + chat_system_bootstrap(current_user, [channel_1, channel_2]) sign_in(current_user) end @@ -69,5 +70,42 @@ RSpec.describe "Mentions warnings", type: :system do end end end + + context "when channel has allow_channel_wide_mentions disabled" do + before { channel_1.update(allow_channel_wide_mentions: false) } + + %w[@here @all].each do |mention_text| + it "displays a warning" do + chat_page.visit_channel(channel_1) + chat_channel_page.type_in_composer(mention_text) + + expect(page).to have_css(".chat-mention-warnings") + expect(page.find(".chat-mention-warnings-list__simple")).to be_present + end + end + + it "retains warnings when loading drafts or changing channels with no draft" do + Chat::Draft.create!( + chat_channel: channel_1, + user: current_user, + data: { message: "@all" }.to_json, + ) + chat_page.visit_channel(channel_1) + + # Channel 1 has a draft that causes a mention warning. Should appear on load + expect(page).to have_css(".chat-mention-warnings") + expect(page.find(".chat-mention-warnings-list__simple")).to be_present + + # Channel 2 doesn't have a draft so it should disappear + chat_page.visit_channel(channel_2) + expect(page).to have_no_css(".chat-mention-warnings") + + # Navigating back to channel 1 will make the mention warnings appear b/c the draft + # will trigger the @all mention warning again + chat_page.visit_channel(channel_1) + expect(page).to have_css(".chat-mention-warnings") + expect(page.find(".chat-mention-warnings-list__simple")).to be_present + end + end end end diff --git a/plugins/chat/test/javascripts/components/chat-message-mention-warning-test.js b/plugins/chat/test/javascripts/components/chat-message-mention-warning-test.js index 35af421d57f..9159b29410d 100644 --- a/plugins/chat/test/javascripts/components/chat-message-mention-warning-test.js +++ b/plugins/chat/test/javascripts/components/chat-message-mention-warning-test.js @@ -98,5 +98,21 @@ module( ) .exists(); }); + + test("displays a warning when global mentions are disabled", async function (assert) { + this.message = fabricators.message(); + this.message.mentionWarning = fabricators.messageMentionWarning( + this.message, + { + global_mentions_disabled: true, + } + ); + + await render(template); + + assert + .dom(".chat-message-mention-warning__text.-global-mentions-disabled") + .exists(); + }); } );