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.
This commit is contained in:
Mark VanLandingham 2023-08-22 15:54:35 -05:00 committed by GitHub
parent c9de84c63d
commit 68eba53e09
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 137 additions and 10 deletions

View File

@ -393,7 +393,8 @@ module Chat
cannot_chat_users, cannot_chat_users,
without_membership, without_membership,
too_many_members, too_many_members,
mentions_disabled mentions_disabled,
global_mentions_disabled
) )
MessageBus.publish( MessageBus.publish(
"/chat/#{chat_message.chat_channel_id}", "/chat/#{chat_message.chat_channel_id}",
@ -405,6 +406,7 @@ module Chat
without_membership.map { |u| { username: u.username, id: u.id } }.as_json, without_membership.map { |u| { username: u.username, id: u.id } }.as_json,
groups_with_too_many_members: too_many_members.map(&:name).as_json, groups_with_too_many_members: too_many_members.map(&:name).as_json,
group_mentions_disabled: mentions_disabled.map(&:name).as_json, group_mentions_disabled: mentions_disabled.map(&:name).as_json,
global_mentions_disabled: global_mentions_disabled,
}, },
user_ids: [user_id], user_ids: [user_id],
) )

View File

@ -98,6 +98,7 @@ export default class ChatComposer extends Component {
this.cancelPersistDraft(); this.cancelPersistDraft();
this.composer.textarea.value = this.currentMessage.message; this.composer.textarea.value = this.currentMessage.message;
this.persistDraft(); this.persistDraft();
this.captureMentions({ skipDebounce: true });
} }
@action @action
@ -372,11 +373,14 @@ export default class ChatComposer extends Component {
} }
@action @action
captureMentions() { captureMentions(opts = { skipDebounce: false }) {
if (this.hasContent) { if (this.hasContent) {
this.chatComposerWarningsTracker.trackMentions( this.chatComposerWarningsTracker.trackMentions(
this.currentMessage.message this.currentMessage,
opts.skipDebounce
); );
} else {
this.chatComposerWarningsTracker.reset();
} }
} }

View File

@ -11,6 +11,11 @@
{{#if this.hasTooManyMentions}} {{#if this.hasTooManyMentions}}
<li>{{this.tooManyMentionsBody}}</li> <li>{{this.tooManyMentionsBody}}</li>
{{else}} {{else}}
{{#if this.channelWideMentionDisallowed}}
<li>{{i18n
"chat.mention_warning.channel_wide_mentions_disallowed"
}}</li>
{{/if}}
{{#if this.hasUnreachableGroupMentions}} {{#if this.hasUnreachableGroupMentions}}
<li>{{this.unreachableBody}}</li> <li>{{this.unreachableBody}}</li>
{{/if}} {{/if}}

View File

@ -21,6 +21,10 @@ export default class ChatMentionWarnings extends Component {
return this.chatComposerWarningsTracker.tooManyMentions; return this.chatComposerWarningsTracker.tooManyMentions;
} }
get channelWideMentionDisallowed() {
return this.chatComposerWarningsTracker.channelWideMentionDisallowed;
}
get mentionsCount() { get mentionsCount() {
return this.chatComposerWarningsTracker.mentionsCount; return this.chatComposerWarningsTracker.mentionsCount;
} }
@ -50,6 +54,7 @@ export default class ChatMentionWarnings extends Component {
get show() { get show() {
return ( return (
this.hasTooManyMentions || this.hasTooManyMentions ||
this.channelWideMentionDisallowed ||
this.hasUnreachableGroupMentions || this.hasUnreachableGroupMentions ||
this.hasOverMembersLimitGroupMentions this.hasOverMembersLimitGroupMentions
); );

View File

@ -51,6 +51,12 @@
{{this.groupsWithTooManyMembers}} {{this.groupsWithTooManyMembers}}
</p> </p>
{{/if}} {{/if}}
{{#if this.mentionWarning.globalMentionsDisabled}}
<p class="chat-message-mention-warning__text -global-mentions-disabled">
{{i18n "chat.mention_warning.channel_wide_mentions_disallowed"}}
</p>
{{/if}}
{{/if}} {{/if}}
</div> </div>
{{/if}} {{/if}}

View File

@ -38,7 +38,8 @@ export default class ChatMessageMentionWarning extends Component {
(this.mentionWarning.groupWithMentionsDisabled?.length || (this.mentionWarning.groupWithMentionsDisabled?.length ||
this.mentionWarning.cannotSee?.length || this.mentionWarning.cannotSee?.length ||
this.mentionWarning.withoutMembership?.length || this.mentionWarning.withoutMembership?.length ||
this.mentionWarning.groupsWithTooManyMembers?.length) this.mentionWarning.groupsWithTooManyMembers?.length ||
this.mentionWarning.globalMentionsDisabled)
); );
} }

View File

@ -10,6 +10,7 @@ export default class ChatMessageMentionWarning {
@tracked withoutMembership; @tracked withoutMembership;
@tracked groupsWithTooManyMembers; @tracked groupsWithTooManyMembers;
@tracked groupWithMentionsDisabled; @tracked groupWithMentionsDisabled;
@tracked globalMentionsDisabled;
constructor(message, args = {}) { constructor(message, args = {}) {
this.message = args.message; this.message = args.message;
@ -17,5 +18,6 @@ export default class ChatMessageMentionWarning {
this.withoutMembership = args.without_membership; this.withoutMembership = args.without_membership;
this.groupsWithTooManyMembers = args.groups_with_too_many_members; this.groupsWithTooManyMembers = args.groups_with_too_many_members;
this.groupWithMentionsDisabled = args.group_mentions_disabled; this.groupWithMentionsDisabled = args.group_mentions_disabled;
this.globalMentionsDisabled = args.global_mentions_disabled;
} }
} }

View File

@ -21,6 +21,7 @@ export default class ChatComposerWarningsTracker extends Service {
@tracked unreachableGroupMentions = []; @tracked unreachableGroupMentions = [];
@tracked overMembersLimitGroupMentions = []; @tracked overMembersLimitGroupMentions = [];
@tracked tooManyMentions = false; @tracked tooManyMentions = false;
@tracked channelWideMentionDisallowed = false;
@tracked mentionsCount = 0; @tracked mentionsCount = 0;
@tracked mentionsTimer = null; @tracked mentionsTimer = null;
@ -32,22 +33,37 @@ export default class ChatComposerWarningsTracker extends Service {
} }
@bind @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.mentionsTimer = discourseDebounce(
this, this,
this._trackMentions, this._trackMentions,
message, currentMessage,
MENTION_DEBOUNCE_MS MENTION_DEBOUNCE_MS
); );
} }
@bind @bind
_trackMentions(message) { _trackMentions(currentMessage) {
if (!this.siteSettings.enable_mentions) { if (!this.siteSettings.enable_mentions) {
return; return;
} }
const mentions = this._extractMentions(message); const mentions = this._extractMentions(currentMessage.message);
this.mentionsCount = mentions?.length; this.mentionsCount = mentions?.length;
if (this.mentionsCount > 0) { if (this.mentionsCount > 0) {
@ -59,6 +75,10 @@ export default class ChatComposerWarningsTracker extends Service {
(mention) => !(mention in this._mentionWarningsSeen) (mention) => !(mention in this._mentionWarningsSeen)
); );
this.channelWideMentionDisallowed =
!currentMessage.channel.allowChannelWideMentions &&
(mentions.includes("here") || mentions.includes("all"));
if (newMentions?.length > 0) { if (newMentions?.length > 0) {
this._recordNewWarnings(newMentions, mentions); this._recordNewWarnings(newMentions, mentions);
} else { } else {
@ -67,6 +87,7 @@ export default class ChatComposerWarningsTracker extends Service {
} }
} else { } else {
this.tooManyMentions = false; this.tooManyMentions = false;
this.channelWideMentionDisallowed = false;
this.unreachableGroupMentions = []; this.unreachableGroupMentions = [];
this.overMembersLimitGroupMentions = []; this.overMembersLimitGroupMentions = [];
} }

View File

@ -146,6 +146,7 @@ en:
group_mentions_disabled_multiple: group_mentions_disabled_multiple:
one: "%{group_name} and %{count} other group don't allow mentions." one: "%{group_name} and %{count} other group don't allow mentions."
other: "%{group_name} and %{count} other groups 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: "%{group_name} has too many members. No one was notified."
too_many_members_multiple: too_many_members_multiple:
one: "%{group_name} and %{count} other group have too many members. No one was notified." one: "%{group_name} and %{count} other group have too many members. No one was notified."

View File

@ -228,7 +228,7 @@ module Chat
group_mentions_disabled = @parsed_mentions.groups_with_disabled_mentions.to_a group_mentions_disabled = @parsed_mentions.groups_with_disabled_mentions.to_a
too_many_members = @parsed_mentions.groups_with_too_many_members.to_a too_many_members = @parsed_mentions.groups_with_too_many_members.to_a
if inaccessible.values.all?(&:blank?) && group_mentions_disabled.empty? && if inaccessible.values.all?(&:blank?) && group_mentions_disabled.empty? &&
too_many_members.empty? too_many_members.empty? && !global_mentions_disabled
return return
end end
@ -239,9 +239,19 @@ module Chat
inaccessible[:welcome_to_join].to_a, inaccessible[:welcome_to_join].to_a,
too_many_members, too_many_members,
group_mentions_disabled, group_mentions_disabled,
global_mentions_disabled,
) )
end 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 # 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 # 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 # a notification via the Jobs::Chat::NotifyMentioned job and are not prompted for

View File

@ -58,6 +58,22 @@ describe Chat::Notifier do
expect(to_notify[list_key]).to be_empty expect(to_notify[list_key]).to be_empty
end 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 it "includes all members of a channel except the sender" do
msg = build_cooked_msg(mention, user_1) msg = build_cooked_msg(mention, user_1)

View File

@ -3,12 +3,13 @@
RSpec.describe "Mentions warnings", type: :system do RSpec.describe "Mentions warnings", type: :system do
fab!(:current_user) { Fabricate(:user) } fab!(:current_user) { Fabricate(:user) }
fab!(:channel_1) { Fabricate(:chat_channel) } fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:channel_2) { Fabricate(:chat_channel) }
let(:chat_page) { PageObjects::Pages::Chat.new } let(:chat_page) { PageObjects::Pages::Chat.new }
let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new } let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new }
before do before do
chat_system_bootstrap(current_user, [channel_1]) chat_system_bootstrap(current_user, [channel_1, channel_2])
sign_in(current_user) sign_in(current_user)
end end
@ -69,5 +70,42 @@ RSpec.describe "Mentions warnings", type: :system do
end end
end 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
end end

View File

@ -98,5 +98,21 @@ module(
) )
.exists(); .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();
});
} }
); );