FEATURE: Enforce mention limits for chat messages (#19034)

* FEATURE: Enforce mention limits for chat messages

The first part of these changes adds a new setting called `max_mentions_per_chat_message`, which skips notifications when the message contains too many mentions. It also respects the `max_users_notified_per_group_mention` setting
and skips notifications if expanding a group mention would exceed it.

We also include a new component to display JIT warning for these limits to the user while composing a message.

* Simplify ignoring/muting filter in chat_notifier

* Post-send warnings for unsent warnings

* Improve pluralization

* Address review feedback

* Fix test

* Address second feedback round

* Third round of feedback

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
This commit is contained in:
Roman Rizzi 2022-12-06 14:54:04 -03:00 committed by GitHub
parent 4e92a6e804
commit 9c8043a4d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 851 additions and 136 deletions

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -908,7 +908,10 @@ posting:
default: true default: true
client: true client: true
max_mentions_per_post: 10 max_mentions_per_post: 10
max_users_notified_per_group_mention: 100 max_users_notified_per_group_mention:
default: 100
max: 250
client: true
newuser_max_replies_per_topic: 3 newuser_max_replies_per_topic: 3
newuser_max_mentions_per_post: 2 newuser_max_mentions_per_post: 2
here_mention: here_mention:

View File

@ -0,0 +1,39 @@
# frozen_string_literal: true
class Chat::Api::HintsController < ApplicationController
before_action :ensure_logged_in
def check_group_mentions
RateLimiter.new(current_user, "group_mention_hints", 5, 10.seconds).performed!
group_names = params[:mentions]
raise Discourse::InvalidParameters.new(:mentions) if group_names.blank?
visible_groups = Group
.where("LOWER(name) IN (?)", group_names)
.visible_groups(current_user)
.pluck(:name)
mentionable_groups = filter_mentionable_groups(visible_groups)
result = {
unreachable: visible_groups - mentionable_groups.map(&:name),
over_members_limit: mentionable_groups.select { |g| g.user_count > SiteSetting.max_users_notified_per_group_mention }.map(&:name),
}
result[:invalid] = (group_names - result[:unreachable]) - result[:over_members_limit]
render json: result
end
private
def filter_mentionable_groups(group_names)
return [] if group_names.empty?
Group
.select(:name, :user_count)
.where(name: group_names)
.mentionable(current_user, include_public: false)
end
end

View File

@ -153,23 +153,19 @@ module ChatPublisher
user_id, user_id,
chat_message, chat_message,
cannot_chat_users, cannot_chat_users,
without_membership without_membership,
too_many_members,
mentions_disabled
) )
MessageBus.publish( MessageBus.publish(
"/chat/#{chat_message.chat_channel_id}", "/chat/#{chat_message.chat_channel_id}",
{ {
type: :mention_warning, type: :mention_warning,
chat_message_id: chat_message.id, chat_message_id: chat_message.id,
cannot_see: cannot_see: cannot_chat_users.map { |u| { username: u.username, id: u.id } }.as_json,
ActiveModel::ArraySerializer.new( without_membership: without_membership.map { |u| { username: u.username, id: u.id } }.as_json,
cannot_chat_users, groups_with_too_many_members: too_many_members.map(&:name).as_json,
each_serializer: BasicUserSerializer, group_mentions_disabled: mentions_disabled.map(&:name).as_json
).as_json,
without_membership:
ActiveModel::ArraySerializer.new(
without_membership,
each_serializer: BasicUserSerializer,
).as_json,
}, },
user_ids: [user_id], user_ids: [user_id],
) )

View File

@ -21,12 +21,15 @@ import { Promise } from "rsvp";
import { translations } from "pretty-text/emoji/data"; import { translations } from "pretty-text/emoji/data";
import { channelStatusName } from "discourse/plugins/chat/discourse/models/chat-channel"; import { channelStatusName } from "discourse/plugins/chat/discourse/models/chat-channel";
import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete"; import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete";
import discourseDebounce from "discourse-common/lib/debounce";
import { import {
chatComposerButtons, chatComposerButtons,
chatComposerButtonsDependentKeys, chatComposerButtonsDependentKeys,
} from "discourse/plugins/chat/discourse/lib/chat-composer-buttons"; } from "discourse/plugins/chat/discourse/lib/chat-composer-buttons";
import { mentionRegex } from "pretty-text/mentions";
const THROTTLE_MS = 150; const THROTTLE_MS = 150;
const MENTION_DEBOUNCE_MS = 1000;
export default Component.extend(TextareaTextManipulation, { export default Component.extend(TextareaTextManipulation, {
chatChannel: null, chatChannel: null,
@ -41,12 +44,14 @@ export default Component.extend(TextareaTextManipulation, {
editingMessage: null, editingMessage: null,
onValueChange: null, onValueChange: null,
timer: null, timer: null,
mentionsTimer: null,
value: "", value: "",
inProgressUploads: null, inProgressUploads: null,
composerEventPrefix: "chat", composerEventPrefix: "chat",
composerFocusSelector: ".chat-composer-input", composerFocusSelector: ".chat-composer-input",
canAttachUploads: reads("siteSettings.chat_allow_uploads"), canAttachUploads: reads("siteSettings.chat_allow_uploads"),
isNetworkUnreliable: reads("chat.isNetworkUnreliable"), isNetworkUnreliable: reads("chat.isNetworkUnreliable"),
typingMention: false,
@discourseComputed(...chatComposerButtonsDependentKeys()) @discourseComputed(...chatComposerButtonsDependentKeys())
inlineButtons() { inlineButtons() {
@ -144,10 +149,8 @@ export default Component.extend(TextareaTextManipulation, {
"_inProgressUploadsChanged" "_inProgressUploadsChanged"
); );
if (this.timer) { cancel(this.timer);
cancel(this.timer); cancel(this.mentionsTimer);
this.timer = null;
}
this.appEvents.off("chat:focus-composer", this, "_focusTextArea"); this.appEvents.off("chat:focus-composer", this, "_focusTextArea");
this.appEvents.off("chat:insert-text", this, "insertText"); this.appEvents.off("chat:insert-text", this, "insertText");
@ -230,6 +233,7 @@ export default Component.extend(TextareaTextManipulation, {
replyToMsg: this.draft.replyToMsg, replyToMsg: this.draft.replyToMsg,
}); });
this._debouncedCaptureMentions();
this._syncUploads(this.draft.uploads); this._syncUploads(this.draft.uploads);
this.setInReplyToMsg(this.draft.replyToMsg); this.setInReplyToMsg(this.draft.replyToMsg);
} }
@ -294,6 +298,13 @@ export default Component.extend(TextareaTextManipulation, {
this.set("value", value); this.set("value", value);
this.resizeTextarea(); this.resizeTextarea();
this.typingMention = value.slice(-1) === "@";
if (this.typingMention && value.slice(-1) === " ") {
this.typingMention = false;
this._debouncedCaptureMentions();
}
// throttle, not debounce, because we do eventually want to react during the typing // throttle, not debounce, because we do eventually want to react during the typing
this.timer = throttle(this, this._handleTextareaInput, THROTTLE_MS); this.timer = throttle(this, this._handleTextareaInput, THROTTLE_MS);
}, },
@ -304,6 +315,44 @@ export default Component.extend(TextareaTextManipulation, {
this.onValueChange?.(this.value, this._uploads, this.replyToMsg); 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;
},
@bind @bind
_blurInput() { _blurInput() {
document.activeElement?.blur(); document.activeElement?.blur();
@ -350,6 +399,7 @@ export default Component.extend(TextareaTextManipulation, {
afterComplete: (text) => { afterComplete: (text) => {
this.set("value", text); this.set("value", text);
this._focusTextArea(); this._focusTextArea();
this._debouncedCaptureMentions();
}, },
}); });
} }
@ -660,6 +710,7 @@ export default Component.extend(TextareaTextManipulation, {
value: "", value: "",
inReplyMsg: null, inReplyMsg: null,
}); });
this.onMentionUpdates([]);
this._syncUploads([]); this._syncUploads([]);
this._focusTextArea({ ensureAtEnd: true, resizeTextarea: true }); this._focusTextArea({ ensureAtEnd: true, resizeTextarea: true });
this.onValueChange?.(this.value, this._uploads, this.replyToMsg); this.onValueChange?.(this.value, this._uploads, this.replyToMsg);

View File

@ -27,6 +27,13 @@
<ChatRetentionReminder @chatChannel={{this.chatChannel}} /> <ChatRetentionReminder @chatChannel={{this.chatChannel}} />
<ChatMentionWarnings
@unreachableGroupMentions={{this.unreachableGroupMentions}}
@overMembersLimitGroupMentions={{this.overMembersLimitGroupMentions}}
@tooManyMentions={{this.tooManyMentions}}
@mentionsCount={{this.mentionsCount}}
/>
<div class="chat-message-actions-mobile-anchor"></div> <div class="chat-message-actions-mobile-anchor"></div>
<div <div
class={{concat-class class={{concat-class
@ -90,7 +97,22 @@
<ChatSelectionManager @selectedMessageIds={{this.selectedMessageIds}} @chatChannel={{this.chatChannel}} @canModerate={{this.details.can_moderate}} @cancelSelecting={{action "cancelSelecting"}} /> <ChatSelectionManager @selectedMessageIds={{this.selectedMessageIds}} @chatChannel={{this.chatChannel}} @canModerate={{this.details.can_moderate}} @cancelSelecting={{action "cancelSelecting"}} />
{{else}} {{else}}
{{#if (or this.chatChannel.isDraft this.chatChannel.isFollowing)}} {{#if (or this.chatChannel.isDraft this.chatChannel.isFollowing)}}
<ChatComposer @draft={{this.draft}} @details={{this.details}} @canInteractWithChat={{this.canInteractWithChat}} @sendMessage={{action "sendMessage"}} @editMessage={{action "editMessage"}} @setReplyTo={{action "setReplyTo"}} @loading={{this.sendingLoading}} @editingMessage={{readonly this.editingMessage}} @onCancelEditing={{this.cancelEditing}} @setInReplyToMsg={{this.setInReplyToMsg}} @onEditLastMessageRequested={{this.editLastMessageRequested}} @onValueChange={{action "composerValueChanged"}} @chatChannel={{this.chatChannel}} /> <ChatComposer
@draft={{this.draft}}
@details={{this.details}}
@canInteractWithChat={{this.canInteractWithChat}}
@sendMessage={{action "sendMessage"}}
@editMessage={{action "editMessage"}}
@setReplyTo={{action "setReplyTo"}}
@loading={{this.sendingLoading}}
@editingMessage={{readonly this.editingMessage}}
@onCancelEditing={{this.cancelEditing}}
@setInReplyToMsg={{this.setInReplyToMsg}}
@onEditLastMessageRequested={{this.editLastMessageRequested}}
@onValueChange={{action "composerValueChanged"}}
@chatChannel={{this.chatChannel}}
@onMentionUpdates={{this.updateMentions}}
/>
{{else}} {{else}}
<ChatChannelPreviewCard @channel={{this.chatChannel}} /> <ChatChannelPreviewCard @channel={{this.chatChannel}} />
{{/if}} {{/if}}

View File

@ -38,6 +38,12 @@ const FETCH_MORE_MESSAGES_THROTTLE_MS = isTesting() ? 0 : 500;
const PAST = "past"; const PAST = "past";
const FUTURE = "future"; const FUTURE = "future";
const MENTION_RESULT = {
invalid: -1,
unreachable: 0,
over_members_limit: 1,
};
export default Component.extend({ export default Component.extend({
classNameBindings: [":chat-live-pane", "sendingLoading", "loading"], classNameBindings: [":chat-live-pane", "sendingLoading", "loading"],
chatChannel: null, chatChannel: null,
@ -68,6 +74,14 @@ export default Component.extend({
targetMessageId: null, targetMessageId: null,
hasNewMessages: 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(), chat: service(),
router: service(), router: service(),
chatEmojiPickerManager: service(), chatEmojiPickerManager: service(),
@ -82,6 +96,9 @@ export default Component.extend({
this._super(...arguments); this._super(...arguments);
this.set("messages", []); this.set("messages", []);
this.set("_mentionWarningsSeen", {});
this.set("unreachableGroupMentions", []);
this.set("overMembersLimitGroupMentions", []);
}, },
didInsertElement() { didInsertElement() {
@ -1313,6 +1330,81 @@ 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 @action
reStickScrollIfNeeded() { reStickScrollIfNeeded() {
if (this.stickyScroll) { if (this.stickyScroll) {

View File

@ -0,0 +1,24 @@
{{#if this.show}}
<div class="chat-mention-warnings">
<div class="chat-mention-warning__icon">
{{d-icon "exclamation-triangle"}}
</div>
<div class="chat-mention-warning__text">
<div class="chat-mention-warning__header">
{{this.warningHeaderText}}
</div>
<ul class={{this.listStyleClass}}>
{{#if @tooManyMentions}}
<li>{{this.tooManyMentionsBody}}</li>
{{else}}
{{#if @unreachableGroupMentions}}
<li>{{this.unreachableBody}}</li>
{{/if}}
{{#if @overMembersLimitGroupMentions}}
<li>{{this.overMembersLimitBody}}</li>
{{/if}}
{{/if}}
</ul>
</div>
</div>
{{/if}}

View File

@ -0,0 +1,163 @@
import Component from "@glimmer/component";
import I18n from "I18n";
import { htmlSafe } from "@ember/template";
import { inject as service } from "@ember/service";
export default class ChatMentionWarnings extends Component {
@service siteSettings;
@service currentUser;
get unreachableGroupMentionsCount() {
return this.args?.unreachableGroupMentions.length;
}
get overMembersLimitMentionsCount() {
return this.args?.overMembersLimitGroupMentions.length;
}
get hasTooManyMentions() {
return this.args?.tooManyMentions;
}
get hasUnreachableGroupMentions() {
return this.unreachableGroupMentionsCount > 0;
}
get hasOverMembersLimitGroupMentions() {
return this.overMembersLimitMentionsCount > 0;
}
get warningsCount() {
return (
this.unreachableGroupMentionsCount + this.overMembersLimitMentionsCount
);
}
get show() {
return (
this.hasTooManyMentions ||
this.hasUnreachableGroupMentions ||
this.hasOverMembersLimitGroupMentions
);
}
get listStyleClass() {
if (this.hasTooManyMentions) {
return "chat-mention-warnings-list__simple";
}
if (this.warningsCount > 1) {
return "chat-mention-warnings-list__multiple";
} else {
return "chat-mention-warnings-list__simple";
}
}
get warningHeaderText() {
if (
this.args?.mentionsCount <= this.warningsCount ||
this.hasTooManyMentions
) {
return I18n.t("chat.mention_warning.groups.header.all");
} else {
return I18n.t("chat.mention_warning.groups.header.some");
}
}
get tooManyMentionsBody() {
if (!this.hasTooManyMentions) {
return;
}
let notificationLimit = I18n.t(
"chat.mention_warning.groups.notification_limit"
);
if (this.currentUser.staff) {
notificationLimit = htmlSafe(
`<a
target="_blank"
href="/admin/site_settings/category/plugins?filter=max_mentions_per_chat_message"
>
${notificationLimit}
</a>`
);
}
const settingLimit = I18n.t("chat.mention_warning.mentions_limit", {
count: this.siteSettings.max_mentions_per_chat_message,
});
return htmlSafe(
I18n.t("chat.mention_warning.too_many_mentions", {
notification_limit: notificationLimit,
limit: settingLimit,
})
);
}
get unreachableBody() {
if (!this.hasUnreachableGroupMentions) {
return;
}
if (this.unreachableGroupMentionsCount <= 2) {
return I18n.t("chat.mention_warning.groups.unreachable", {
group: this.args.unreachableGroupMentions[0],
group_2: this.args.unreachableGroupMentions[1],
count: this.unreachableGroupMentionsCount,
});
} else {
return I18n.t("chat.mention_warning.groups.unreachable_multiple", {
group: this.args.unreachableGroupMentions[0],
count: this.unreachableGroupMentionsCount - 1, //N others
});
}
}
get overMembersLimitBody() {
if (!this.hasOverMembersLimitGroupMentions) {
return;
}
let notificationLimit = I18n.t(
"chat.mention_warning.groups.notification_limit"
);
if (this.currentUser.staff) {
notificationLimit = htmlSafe(
`<a
target="_blank"
href="/admin/site_settings/category/plugins?filter=max_users_notified_per_group_mention"
>
${notificationLimit}
</a>`
);
}
const settingLimit = I18n.t("chat.mention_warning.groups.users_limit", {
count: this.siteSettings.max_users_notified_per_group_mention,
});
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],
count: this.overMembersLimitMentionsCount,
notification_limit: notificationLimit,
limit: settingLimit,
})
);
} else {
return htmlSafe(
I18n.t("chat.mention_warning.groups.too_many_members_multiple", {
group: this.args.overMembersLimitGroupMentions[0],
count: this.overMembersLimitMentionsCount - 1, //N others
notification_limit: notificationLimit,
limit: settingLimit,
})
);
}
}
}

View File

@ -122,25 +122,25 @@
</div> </div>
{{/if}} {{/if}}
{{#if this.message.mentionWarning}} {{#if this.mentionWarning}}
<div class="alert alert-info chat-message-mention-warning"> <div class="alert alert-info chat-message-mention-warning">
{{#if this.message.mentionWarning.invitationSent}} {{#if this.mentionWarning.invitationSent}}
{{d-icon "check"}} {{d-icon "check"}}
<span> <span>
{{i18n {{i18n
"chat.mention_warning.invitations_sent" "chat.mention_warning.invitations_sent"
count=this.message.mentionWarning.without_membership.length count=this.mentionWarning.without_membership.length
}} }}
</span> </span>
{{else}} {{else}}
<FlatButton @class="dismiss-mention-warning" @title="chat.mention_warning.dismiss" @action={{action "dismissMentionWarning"}} @icon="times" /> <FlatButton @class="dismiss-mention-warning" @title="chat.mention_warning.dismiss" @action={{action "dismissMentionWarning"}} @icon="times" />
{{#if this.message.mentionWarning.cannot_see}} {{#if this.mentionWarning.cannot_see}}
<p class="cannot-see">{{this.mentionedCannotSeeText}}</p> <p class="warning-item cannot-see">{{this.mentionedCannotSeeText}}</p>
{{/if}} {{/if}}
{{#if this.message.mentionWarning.without_membership}} {{#if this.mentionWarning.without_membership}}
<p class="without-membership"> <p class="warning-item without-membership">
<span>{{this.mentionedWithoutMembershipText}}</span> <span>{{this.mentionedWithoutMembershipText}}</span>
<a <a
class="invite-link" class="invite-link"
@ -151,6 +151,13 @@
</a> </a>
</p> </p>
{{/if}} {{/if}}
{{#if this.mentionWarning.group_mentions_disabled}}
<p class="warning-item">{{this.groupsWithDisabledMentions}}</p>
{{/if}}
{{#if this.mentionWarning.groups_with_too_many_members}}
<p class="warning-item">{{this.groupsWithTooManyMembers}}</p>
{{/if}}
{{/if}} {{/if}}
</div> </div>
{{/if}} {{/if}}

View File

@ -41,7 +41,6 @@ export default Component.extend({
canInteractWithChat: false, canInteractWithChat: false,
isHovered: false, isHovered: false,
onHoverMessage: null, onHoverMessage: null,
mentionWarning: null,
chatEmojiReactionStore: service("chat-emoji-reaction-store"), chatEmojiReactionStore: service("chat-emoji-reaction-store"),
chatEmojiPickerManager: service("chat-emoji-picker-manager"), chatEmojiPickerManager: service("chat-emoji-picker-manager"),
adminTools: optionalService(), adminTools: optionalService(),
@ -445,25 +444,56 @@ export default Component.extend({
return Object.values(reactions).some((r) => r.count > 0); return Object.values(reactions).some((r) => r.count > 0);
}, },
@discourseComputed("message.mentionWarning.cannot_see") @discourseComputed("message.mentionWarning")
mentionWarning() {
return this.message.mentionWarning;
},
@discourseComputed("mentionWarning.cannot_see")
mentionedCannotSeeText(users) { mentionedCannotSeeText(users) {
return I18n.t("chat.mention_warning.cannot_see", { return I18n.t("chat.mention_warning.cannot_see", {
usernames: users.mapBy("username").join(", "), username: users[0].username,
count: users.length, count: users.length,
others: this._othersTranslation(users.length - 1),
}); });
}, },
@discourseComputed("message.mentionWarning.without_membership") @discourseComputed("mentionWarning.without_membership")
mentionedWithoutMembershipText(users) { mentionedWithoutMembershipText(users) {
return I18n.t("chat.mention_warning.without_membership", { return I18n.t("chat.mention_warning.without_membership", {
usernames: users.mapBy("username").join(", "), username: users[0].username,
count: users.length, count: users.length,
others: this._othersTranslation(users.length - 1),
});
},
@discourseComputed("mentionWarning.group_mentions_disabled")
groupsWithDisabledMentions(groups) {
return I18n.t("chat.mention_warning.group_mentions_disabled", {
group_name: groups[0],
count: groups.length,
others: this._othersTranslation(groups.length - 1),
});
},
@discourseComputed("mentionWarning.groups_with_too_many_members")
groupsWithTooManyMembers(groups) {
return I18n.t("chat.mention_warning.too_many_members", {
group_name: groups[0],
count: groups.length,
others: this._othersTranslation(groups.length - 1),
});
},
_othersTranslation(othersCount) {
return I18n.t("chat.mention_warning.warning_multiple", {
count: othersCount,
}); });
}, },
@action @action
inviteMentioned() { inviteMentioned() {
const user_ids = this.message.mentionWarning.without_membership.mapBy("id"); const user_ids = this.mentionWarning.without_membership.mapBy("id");
ajax(`/chat/${this.details.chat_channel_id}/invite`, { ajax(`/chat/${this.details.chat_channel_id}/invite`, {
method: "PUT", method: "PUT",

View File

@ -0,0 +1,30 @@
.chat-mention-warnings {
display: flex;
background: var(--tertiary-low);
padding: 0.5em 0 0.5em 1em;
color: var(--primary);
margin: 0.5em;
.chat-mention-warning__icon,
.chat-mention-warning__text {
margin: 0.5em;
}
.chat-mention-warnings-list__simple {
margin: 0.5em 0 0 0;
list-style: none;
}
.chat-mention-warnings-list__multiple {
margin: 0.5em 0 0 1em;
}
.chat-mention-warning__header,
.chat-mention-warning__icon {
font-size: var(--font-up-2);
}
}
.full-page-chat .chat-mention-warnings {
top: 4rem;
}

View File

@ -213,13 +213,12 @@
.dismiss-mention-warning { .dismiss-mention-warning {
position: absolute; position: absolute;
top: 5px; top: 15px;
right: 5px; right: 5px;
cursor: pointer; cursor: pointer;
} }
.cannot-see, .warning-item {
.without-membership {
margin: 0.25em 0; margin: 0.25em 0;
} }

View File

@ -105,17 +105,47 @@ en:
join: "Join" join: "Join"
new_messages: "new messages" new_messages: "new messages"
mention_warning: mention_warning:
cannot_see:
one: "%{usernames} cannot access this channel and was not notified."
other: "%{usernames} cannot access this channel and were not notified."
dismiss: "dismiss" dismiss: "dismiss"
cannot_see:
one: "%{username} cannot access this channel and was not notified."
other: "%{username} and %{others} cannot access this channel and were not notified."
invitations_sent: invitations_sent:
one: "Invitation sent" one: "Invitation sent"
other: "Invitations sent" other: "Invitations sent"
invite: "Invite to channel" invite: "Invite to channel"
without_membership: without_membership:
one: "%{usernames} has not joined this channel." one: "%{username} has not joined this channel."
other: "%{usernames} have not joined this channel." other: "%{username} and %{others} have not joined this channel."
group_mentions_disabled:
one: "%{group_name} doesn't allow mentions"
other: "%{group_name} and %{others} doesn't allow mentions"
too_many_members:
one: "%{group_name} has too many members. No one was notified"
other: "%{group_name} and %{others} have too many members. No one was notified"
warning_multiple:
one: "%{count} other"
other: "%{count} others"
groups:
header:
some: "Some users won't be notified"
all: "Nobody will be notified"
unreachable:
one: "@%{group} doesn't allow mentions"
other: "@%{group} and @%{group_2} doesn't allow mentions"
unreachable_multiple: "@%{group} and %{count} others doesn't allow mentions"
too_many_members:
one: "Mentioning @%{group} exceeds the %{notification_limit} of %{limit}"
other: "Mentioning both @%{group} or @%{group_2} exceeds the %{notification_limit} of %{limit}"
too_many_members_multiple: "These %{count} groups exceed the %{notification_limit} of %{limit}"
users_limit:
one: "%{count} user"
other: "%{count} users"
notification_limit: "notification limit"
too_many_mentions: "This message exceeds the %{notification_limit} of %{limit}"
mentions_limit:
one: "%{count} mention"
other: "%{count} mentions"
aria_roles: aria_roles:
header: "Chat header" header: "Chat header"
composer: "Chat composer" composer: "Chat composer"

View File

@ -17,6 +17,7 @@ en:
default_emoji_reactions: "Default emoji reactions for chat messages. Add up to 5 emojis for quick reaction." default_emoji_reactions: "Default emoji reactions for chat messages. Add up to 5 emojis for quick reaction."
direct_message_enabled_groups: "Allow users within these groups to create user-to-user Personal Chats. Note: staff can always create Personal Chats, and users will be able to reply to Personal Chats initiated by users who have permission to create them." direct_message_enabled_groups: "Allow users within these groups to create user-to-user Personal Chats. Note: staff can always create Personal Chats, and users will be able to reply to Personal Chats initiated by users who have permission to create them."
chat_message_flag_allowed_groups: "Users in these groups are allowed to flag chat messages." chat_message_flag_allowed_groups: "Users in these groups are allowed to flag chat messages."
max_mentions_per_chat_message: "Maximum number of @name notifications a user can use in a chat message."
chat_max_direct_message_users: "Users cannot add more than this number of other users when creating a new direct message. Set to 0 to only allow messages to oneself. Staff are exempt from this setting." chat_max_direct_message_users: "Users cannot add more than this number of other users when creating a new direct message. Set to 0 to only allow messages to oneself. Staff are exempt from this setting."
chat_allow_archiving_channels: "Allow staff to archive messages to a topic when closing a channel." chat_allow_archiving_channels: "Allow staff to archive messages to a topic when closing a channel."
errors: errors:

View File

@ -104,3 +104,9 @@ chat:
client: true client: true
allow_any: false allow_any: false
refresh: true refresh: true
max_mentions_per_chat_message:
client: true
type: integer
default: 5
max: 10
min: 0

View File

@ -56,17 +56,13 @@ class Chat::ChatNotifier
def notify_new def notify_new
to_notify = list_users_to_notify to_notify = list_users_to_notify
inaccessible = to_notify.extract!(:unreachable, :welcome_to_join)
mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids] mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids]
mentioned_user_ids.each do |member_id| mentioned_user_ids.each do |member_id|
ChatPublisher.publish_new_mention(member_id, @chat_channel.id, @chat_message.id) ChatPublisher.publish_new_mention(member_id, @chat_channel.id, @chat_message.id)
end end
notify_creator_of_inaccessible_mentions( notify_creator_of_inaccessible_mentions(to_notify)
inaccessible[:unreachable],
inaccessible[:welcome_to_join],
)
notify_mentioned_users(to_notify) notify_mentioned_users(to_notify)
notify_watching_users(except: mentioned_user_ids << @user.id) notify_watching_users(except: mentioned_user_ids << @user.id)
@ -80,7 +76,6 @@ class Chat::ChatNotifier
already_notified_user_ids = existing_notifications.map(&:user_id) already_notified_user_ids = existing_notifications.map(&:user_id)
to_notify = list_users_to_notify to_notify = list_users_to_notify
inaccessible = to_notify.extract!(:unreachable, :welcome_to_join)
mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids] mentioned_user_ids = to_notify.extract!(:all_mentioned_user_ids)[:all_mentioned_user_ids]
needs_deletion = already_notified_user_ids - mentioned_user_ids needs_deletion = already_notified_user_ids - mentioned_user_ids
@ -93,10 +88,7 @@ class Chat::ChatNotifier
needs_notification_ids = mentioned_user_ids - already_notified_user_ids needs_notification_ids = mentioned_user_ids - already_notified_user_ids
return if needs_notification_ids.blank? return if needs_notification_ids.blank?
notify_creator_of_inaccessible_mentions( notify_creator_of_inaccessible_mentions(to_notify)
inaccessible[:unreachable],
inaccessible[:welcome_to_join],
)
notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids) notify_mentioned_users(to_notify, already_notified_user_ids: already_notified_user_ids)
@ -106,16 +98,23 @@ class Chat::ChatNotifier
private private
def list_users_to_notify def list_users_to_notify
direct_mentions_count = direct_mentions_from_cooked.length
group_mentions_count = group_name_mentions.length
skip_notifications =
(direct_mentions_count + group_mentions_count) >
SiteSetting.max_mentions_per_chat_message
{}.tap do |to_notify| {}.tap do |to_notify|
# The order of these methods is the precedence # The order of these methods is the precedence
# between different mention types. # between different mention types.
already_covered_ids = [] already_covered_ids = []
expand_direct_mentions(to_notify, already_covered_ids) expand_direct_mentions(to_notify, already_covered_ids, skip_notifications)
expand_group_mentions(to_notify, already_covered_ids) expand_group_mentions(to_notify, already_covered_ids, skip_notifications)
expand_here_mention(to_notify, already_covered_ids) expand_here_mention(to_notify, already_covered_ids, skip_notifications)
expand_global_mention(to_notify, already_covered_ids) expand_global_mention(to_notify, already_covered_ids, skip_notifications)
filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids) filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids)
@ -161,10 +160,10 @@ class Chat::ChatNotifier
end end
end end
def expand_global_mention(to_notify, already_covered_ids) def expand_global_mention(to_notify, already_covered_ids, skip)
typed_global_mention = direct_mentions_from_cooked.include?("@all") typed_global_mention = direct_mentions_from_cooked.include?("@all")
if typed_global_mention && @chat_channel.allow_channel_wide_mentions if typed_global_mention && @chat_channel.allow_channel_wide_mentions && !skip
to_notify[:global_mentions] = members_accepting_channel_wide_notifications to_notify[:global_mentions] = members_accepting_channel_wide_notifications
.where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked))
.where.not(id: already_covered_ids) .where.not(id: already_covered_ids)
@ -176,10 +175,10 @@ class Chat::ChatNotifier
end end
end end
def expand_here_mention(to_notify, already_covered_ids) def expand_here_mention(to_notify, already_covered_ids, skip)
typed_here_mention = direct_mentions_from_cooked.include?("@here") typed_here_mention = direct_mentions_from_cooked.include?("@here")
if typed_here_mention && @chat_channel.allow_channel_wide_mentions if typed_here_mention && @chat_channel.allow_channel_wide_mentions && !skip
to_notify[:here_mentions] = members_accepting_channel_wide_notifications to_notify[:here_mentions] = members_accepting_channel_wide_notifications
.where("last_seen_at > ?", 5.minutes.ago) .where("last_seen_at > ?", 5.minutes.ago)
.where.not(username_lower: normalized_mentions(direct_mentions_from_cooked)) .where.not(username_lower: normalized_mentions(direct_mentions_from_cooked))
@ -215,11 +214,15 @@ class Chat::ChatNotifier
} }
end end
def expand_direct_mentions(to_notify, already_covered_ids) def expand_direct_mentions(to_notify, already_covered_ids, skip)
direct_mentions = if skip
chat_users direct_mentions = []
.where(username_lower: normalized_mentions(direct_mentions_from_cooked)) else
.where.not(id: already_covered_ids) direct_mentions =
chat_users
.where(username_lower: normalized_mentions(direct_mentions_from_cooked))
.where.not(id: already_covered_ids)
end
grouped = group_users_to_notify(direct_mentions) grouped = group_users_to_notify(direct_mentions)
@ -236,47 +239,62 @@ class Chat::ChatNotifier
) )
end end
def mentionable_groups def visible_groups
@mentionable_groups ||= @visible_groups ||=
Group.mentionable(@user, include_public: false).where( Group
"LOWER(name) IN (?)", .where("LOWER(name) IN (?)", group_name_mentions)
group_name_mentions, .visible_groups(@user)
)
end end
def expand_group_mentions(to_notify, already_covered_ids) def expand_group_mentions(to_notify, already_covered_ids, skip)
return [] if mentionable_groups.empty? return [] if skip || visible_groups.empty?
mentionable_groups.each { |g| to_notify[g.name.downcase] = [] } mentionable_groups = Group
.mentionable(@user, include_public: false)
.where(id: visible_groups.map(&:id))
mentions_disabled = visible_groups - mentionable_groups
too_many_members, mentionable = mentionable_groups.partition do |group|
group.user_count > SiteSetting.max_users_notified_per_group_mention
end
to_notify[:group_mentions_disabled] = mentions_disabled
to_notify[:too_many_members] = too_many_members
mentionable.each { |g| to_notify[g.name.downcase] = [] }
reached_by_group = reached_by_group =
chat_users.joins(:groups).where(groups: mentionable_groups).where.not(id: already_covered_ids) chat_users.joins(:groups).where(groups: mentionable).where.not(id: already_covered_ids)
grouped = group_users_to_notify(reached_by_group) grouped = group_users_to_notify(reached_by_group)
grouped[:already_participating].each do |user| grouped[:already_participating].each do |user|
# When a user is a member of multiple mentioned groups, # When a user is a member of multiple mentioned groups,
# the most far to the left should take precedence. # the most far to the left should take precedence.
ordered_group_names = group_name_mentions & mentionable_groups.map { |mg| mg.name.downcase } ordered_group_names = group_name_mentions & mentionable.map { |mg| mg.name.downcase }
user_group_names = user.groups.map { |ug| ug.name.downcase } user_group_names = user.groups.map { |ug| ug.name.downcase }
group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) } group_name = ordered_group_names.detect { |gn| user_group_names.include?(gn) }
to_notify[group_name] << user.id to_notify[group_name] << user.id
already_covered_ids << user.id
end end
already_covered_ids.concat(grouped[:already_participating])
to_notify[:welcome_to_join] = to_notify[:welcome_to_join].concat(grouped[:welcome_to_join]) to_notify[:welcome_to_join] = to_notify[:welcome_to_join].concat(grouped[:welcome_to_join])
to_notify[:unreachable] = to_notify[:unreachable].concat(grouped[:unreachable]) to_notify[:unreachable] = to_notify[:unreachable].concat(grouped[:unreachable])
end end
def notify_creator_of_inaccessible_mentions(unreachable, welcome_to_join) def notify_creator_of_inaccessible_mentions(to_notify)
return if unreachable.empty? && welcome_to_join.empty? inaccessible = to_notify.extract!(:unreachable, :welcome_to_join, :too_many_members, :group_mentions_disabled)
return if inaccessible.values.all?(&:blank?)
ChatPublisher.publish_inaccessible_mentions( ChatPublisher.publish_inaccessible_mentions(
@user.id, @user.id,
@chat_message, @chat_message,
unreachable, inaccessible[:unreachable].to_a,
welcome_to_join, inaccessible[:welcome_to_join].to_a,
inaccessible[:too_many_members].to_a,
inaccessible[:group_mentions_disabled].to_a
) )
end end
@ -284,30 +302,28 @@ class Chat::ChatNotifier
# 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 ChatNotifyMentioned job and are not prompted for # a notification via the ChatNotifyMentioned job and are not prompted for
# invitation by the creator. # invitation by the creator.
#
# already_covered_ids and to_notify sometimes contain IDs and sometimes contain
# Users, hence the gymnastics to resolve the user_id
def filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids) def filter_users_ignoring_or_muting_creator(to_notify, already_covered_ids)
user_ids_to_screen = screen_targets = already_covered_ids.concat(to_notify[:welcome_to_join].map(&:id))
already_covered_ids
.map { |ac| user_id_resolver(ac) } return if screen_targets.blank?
.concat(to_notify.values.flatten.map { |tn| user_id_resolver(tn) })
.uniq screener = UserCommScreener.new(acting_user: @user, target_user_ids: screen_targets)
screener = UserCommScreener.new(acting_user: @user, target_user_ids: user_ids_to_screen)
to_notify to_notify
.except(:unreachable) .except(:unreachable, :welcome_to_join)
.each do |key, users_or_ids| .each do |key, user_ids|
to_notify[key] = users_or_ids.reject do |user_or_id| to_notify[key] = user_ids.reject do |user_id|
screener.ignoring_or_muting_actor?(user_id_resolver(user_or_id)) screener.ignoring_or_muting_actor?(user_id)
end end
end end
already_covered_ids.reject! do |already_covered|
screener.ignoring_or_muting_actor?(user_id_resolver(already_covered))
end
end
def user_id_resolver(obj) # :welcome_to_join contains users because it's serialized by MB.
obj.is_a?(User) ? obj.id : obj to_notify[:welcome_to_join] = to_notify[:welcome_to_join].reject do |user|
screener.ignoring_or_muting_actor?(user.id)
end
already_covered_ids.reject! do |already_covered|
screener.ignoring_or_muting_actor?(already_covered)
end
end end
def notify_mentioned_users(to_notify, already_notified_user_ids: []) def notify_mentioned_users(to_notify, already_notified_user_ids: [])

View File

@ -66,6 +66,7 @@ register_asset "stylesheets/common/chat-onebox.scss"
register_asset "stylesheets/common/chat-skeleton.scss" register_asset "stylesheets/common/chat-skeleton.scss"
register_asset "stylesheets/colors.scss", :color_definitions register_asset "stylesheets/colors.scss", :color_definitions
register_asset "stylesheets/common/reviewable-chat-message.scss" register_asset "stylesheets/common/reviewable-chat-message.scss"
register_asset "stylesheets/common/chat-mention-warnings.scss"
register_asset "stylesheets/common/chat-channel-settings-saved-indicator.scss" register_asset "stylesheets/common/chat-channel-settings-saved-indicator.scss"
register_svg_icon "comments" register_svg_icon "comments"
@ -212,6 +213,7 @@ after_initialize do
__FILE__, __FILE__,
) )
load File.expand_path("../app/controllers/api/category_chatables_controller.rb", __FILE__) load File.expand_path("../app/controllers/api/category_chatables_controller.rb", __FILE__)
load File.expand_path("../app/controllers/api/hints_controller.rb", __FILE__)
load File.expand_path("../app/queries/chat_channel_memberships_query.rb", __FILE__) load File.expand_path("../app/queries/chat_channel_memberships_query.rb", __FILE__)
if Discourse.allow_dev_populate? if Discourse.allow_dev_populate?
@ -585,10 +587,13 @@ after_initialize do
put "/chat_channels/:chat_channel_id/notifications_settings" => put "/chat_channels/:chat_channel_id/notifications_settings" =>
"chat_channel_notifications_settings#update" "chat_channel_notifications_settings#update"
# hints controller. Only used by staff members, we don't want to leak category permissions. # Category chatables controller hints. Only used by staff members, we don't want to leak category permissions.
get "/category-chatables/:id/permissions" => "category_chatables#permissions", get "/category-chatables/:id/permissions" => "category_chatables#permissions",
:format => :json, :format => :json,
:constraints => StaffConstraint.new :constraints => StaffConstraint.new
# Hints for JIT warnings.
get "/mentions/groups" => "hints#check_group_mentions", :format => :json
end end
# direct_messages_controller routes # direct_messages_controller routes

View File

@ -297,6 +297,38 @@ describe Chat::ChatNotifier do
expect(to_notify_2[@chat_group.name]).to be_empty expect(to_notify_2[@chat_group.name]).to be_empty
end end
it "skips groups with too many members" do
SiteSetting.max_users_notified_per_group_mention = (group.user_count - 1)
msg = build_cooked_msg("Hello @#{group.name}", user_1)
to_notify = described_class.new(msg, msg.created_at).notify_new
expect(to_notify[group.name]).to be_nil
end
it "respects the 'max_mentions_per_chat_message' setting and skips notifications" do
SiteSetting.max_mentions_per_chat_message = 1
msg = build_cooked_msg("Hello @#{user_2.username} and @#{user_3.username}", user_1)
to_notify = described_class.new(msg, msg.created_at).notify_new
expect(to_notify[:direct_mentions]).to be_empty
expect(to_notify[group.name]).to be_nil
end
it "respects the max mentions setting and skips notifications when mixing users and groups" do
SiteSetting.max_mentions_per_chat_message = 1
msg = build_cooked_msg("Hello @#{user_2.username} and @#{group.name}", user_1)
to_notify = described_class.new(msg, msg.created_at).notify_new
expect(to_notify[:direct_mentions]).to be_empty
expect(to_notify[group.name]).to be_nil
end
describe "users ignoring or muting the user creating the message" do describe "users ignoring or muting the user creating the message" do
it "does not send notifications to the user inside the group who is muting the acting user" do it "does not send notifications to the user inside the group who is muting the acting user" do
group.add(user_3) group.add(user_3)
@ -341,7 +373,7 @@ describe Chat::ChatNotifier do
expect(unreachable_msg).to be_present expect(unreachable_msg).to be_present
expect(unreachable_msg.data[:without_membership]).to be_empty expect(unreachable_msg.data[:without_membership]).to be_empty
unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u[:id] } unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u["id"] }
expect(unreachable_users).to contain_exactly(user_3.id) expect(unreachable_users).to contain_exactly(user_3.id)
end end
@ -375,7 +407,7 @@ describe Chat::ChatNotifier do
expect(unreachable_msg).to be_present expect(unreachable_msg).to be_present
expect(unreachable_msg.data[:without_membership]).to be_empty expect(unreachable_msg.data[:without_membership]).to be_empty
unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u[:id] } unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u["id"] }
expect(unreachable_users).to contain_exactly(user_3.id) expect(unreachable_users).to contain_exactly(user_3.id)
end end
@ -400,7 +432,7 @@ describe Chat::ChatNotifier do
expect(unreachable_msg).to be_present expect(unreachable_msg).to be_present
expect(unreachable_msg.data[:without_membership]).to be_empty expect(unreachable_msg.data[:without_membership]).to be_empty
unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u[:id] } unreachable_users = unreachable_msg.data[:cannot_see].map { |u| u["id"] }
expect(unreachable_users).to contain_exactly(user_3.id) expect(unreachable_users).to contain_exactly(user_3.id)
end end
end end
@ -425,7 +457,7 @@ describe Chat::ChatNotifier do
expect(not_participating_msg).to be_present expect(not_participating_msg).to be_present
expect(not_participating_msg.data[:cannot_see]).to be_empty expect(not_participating_msg.data[:cannot_see]).to be_empty
not_participating_users = not_participating_msg.data[:without_membership].map { |u| u[:id] } not_participating_users = not_participating_msg.data[:without_membership].map { |u| u["id"] }
expect(not_participating_users).to contain_exactly(user_3.id) expect(not_participating_users).to contain_exactly(user_3.id)
end end
@ -477,7 +509,7 @@ describe Chat::ChatNotifier do
expect(not_participating_msg).to be_present expect(not_participating_msg).to be_present
expect(not_participating_msg.data[:cannot_see]).to be_empty expect(not_participating_msg.data[:cannot_see]).to be_empty
not_participating_users = not_participating_msg.data[:without_membership].map { |u| u[:id] } not_participating_users = not_participating_msg.data[:without_membership].map { |u| u["id"] }
expect(not_participating_users).to contain_exactly(user_3.id) expect(not_participating_users).to contain_exactly(user_3.id)
end end
@ -501,7 +533,7 @@ describe Chat::ChatNotifier do
expect(not_participating_msg).to be_present expect(not_participating_msg).to be_present
expect(not_participating_msg.data[:cannot_see]).to be_empty expect(not_participating_msg.data[:cannot_see]).to be_empty
not_participating_users = not_participating_msg.data[:without_membership].map { |u| u[:id] } not_participating_users = not_participating_msg.data[:without_membership].map { |u| u["id"] }
expect(not_participating_users).to contain_exactly(user_3.id) expect(not_participating_users).to contain_exactly(user_3.id)
end end
@ -545,5 +577,48 @@ describe Chat::ChatNotifier do
expect(messages).to be_empty expect(messages).to be_empty
end end
end end
describe "enforcing limits when mentioning groups" do
fab!(:user_3) { Fabricate(:user) }
fab!(:group) do
Fabricate(
:public_group,
users: [user_2, user_3],
mentionable_level: Group::ALIAS_LEVELS[:everyone],
)
end
it "sends a message to the client signaling the group has too many members" do
SiteSetting.max_users_notified_per_group_mention = (group.user_count - 1)
msg = build_cooked_msg("Hello @#{group.name}", user_1)
messages = MessageBus.track_publish("/chat/#{channel.id}") do
to_notify = described_class.new(msg, msg.created_at).notify_new
expect(to_notify[group.name]).to be_nil
end
too_many_members_msg = messages.first
expect(too_many_members_msg).to be_present
too_many_members = too_many_members_msg.data[:groups_with_too_many_members]
expect(too_many_members).to contain_exactly(group.name)
end
it "sends a message to the client signaling the group doesn't allow mentions" do
group.update!(mentionable_level: Group::ALIAS_LEVELS[:only_admins])
msg = build_cooked_msg("Hello @#{group.name}", user_1)
messages = MessageBus.track_publish("/chat/#{channel.id}") do
to_notify = described_class.new(msg, msg.created_at).notify_new
expect(to_notify[group.name]).to be_nil
end
mentions_disabled_msg = messages.first
expect(mentions_disabled_msg).to be_present
mentions_disabled = mentions_disabled_msg.data[:group_mentions_disabled]
expect(mentions_disabled).to contain_exactly(group.name)
end
end
end end
end end

View File

@ -0,0 +1,103 @@
# frozen_string_literal: true
RSpec.describe Chat::Api::HintsController do
describe "#check_group_mentions" do
context "for anons" do
it "returns a 404" do
get "/chat/api/mentions/groups.json", params: { mentions: %w[group1] }
expect(response.status).to eq(403)
end
end
context "for logged in users" do
fab!(:user) { Fabricate(:user) }
fab!(:mentionable_group) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:admin_mentionable_group) { Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:only_admins]) }
before { sign_in(user) }
it "returns a 400 when no mentions are given" do
get "/chat/api/mentions/groups.json"
expect(response.status).to eq(400)
end
it "returns a warning when a group is not mentionable" do
get "/chat/api/mentions/groups.json", params: {
mentions: [mentionable_group.name, admin_mentionable_group.name]
}
expect(response.status).to eq(200)
expect(response.parsed_body["unreachable"]).to contain_exactly(admin_mentionable_group.name)
end
it "returns no warning if the user is allowed to mention" do
user.update!(admin: true)
get "/chat/api/mentions/groups.json", params: {
mentions: [mentionable_group.name, admin_mentionable_group.name]
}
expect(response.status).to eq(200)
expect(response.parsed_body["unreachable"]).to be_empty
end
it "returns a warning if the group has too many users" do
user_1 = Fabricate(:user)
user_2 = Fabricate(:user)
mentionable_group.add(user_1)
mentionable_group.add(user_2)
SiteSetting.max_users_notified_per_group_mention = 1
get "/chat/api/mentions/groups.json", params: {
mentions: [mentionable_group.name, admin_mentionable_group.name]
}
expect(response.status).to eq(200)
expect(response.parsed_body["over_members_limit"]).to contain_exactly(mentionable_group.name)
end
it "returns no warnings when the group doesn't exist" do
get "/chat/api/mentions/groups.json", params: {
mentions: ["a_fake_group"]
}
expect(response.status).to eq(200)
expect(response.parsed_body["unreachable"]).to be_empty
expect(response.parsed_body["over_members_limit"]).to be_empty
end
it "doesn't leak groups that are not visible" do
invisible_group = Fabricate(:group,
visibility_level: Group.visibility_levels[:staff],
mentionable_level: Group::ALIAS_LEVELS[:only_admins]
)
get "/chat/api/mentions/groups.json", params: {
mentions: [invisible_group.name]
}
expect(response.status).to eq(200)
expect(response.parsed_body["unreachable"]).to be_empty
expect(response.parsed_body["over_members_limit"]).to be_empty
expect(response.parsed_body["invalid"]).to contain_exactly(invisible_group.name)
end
it "triggers a rate-limit on too many requests" do
RateLimiter.enable
5.times do
get "/chat/api/mentions/groups.json", params: {
mentions: [mentionable_group.name]
}
end
get "/chat/api/mentions/groups.json", params: {
mentions: [mentionable_group.name]
}
expect(response.status).to eq(429)
end
end
end
end

View File

@ -17,6 +17,8 @@ import {
chatChannelPretender, chatChannelPretender,
} from "../helpers/chat-pretenders"; } from "../helpers/chat-pretenders";
const GROUP_NAME = "group1";
acceptance("Discourse Chat - Composer", function (needs) { acceptance("Discourse Chat - Composer", function (needs) {
needs.user({ has_chat_enabled: true }); needs.user({ has_chat_enabled: true });
needs.settings({ chat_enabled: true, enable_rich_text_paste: true }); needs.settings({ chat_enabled: true, enable_rich_text_paste: true });
@ -32,6 +34,14 @@ acceptance("Discourse Chat - Composer", function (needs) {
server.post("/chat/drafts", () => { server.post("/chat/drafts", () => {
return helper.response([]); return helper.response([]);
}); });
server.get("/chat/api/mentions/groups.json", () => {
return helper.response({
unreachable: [GROUP_NAME],
over_members_limit: [],
invalid: [],
});
});
}); });
needs.hooks.beforeEach(function () { needs.hooks.beforeEach(function () {
@ -105,6 +115,18 @@ acceptance("Discourse Chat - Composer", function (needs) {
"it tracks the emoji" "it tracks the emoji"
); );
}); });
test("JIT warnings for group mentions", async function (assert) {
await visit("/chat/channel/11/-");
await fillIn(".chat-composer-input", `@${GROUP_NAME}`);
assert.equal(
query(".chat-mention-warnings .chat-mention-warnings-list__simple li")
.innerText,
`@${GROUP_NAME} doesn't allow mentions`,
"displays a warning when the group is unreachable"
);
});
}); });
let sendAttempt = 0; let sendAttempt = 0;

View File

@ -864,7 +864,7 @@ Widget.triangulate(arg: "test")
".chat-message-container[data-id='176'] .chat-message-mention-warning .without-membership" ".chat-message-container[data-id='176'] .chat-message-mention-warning .without-membership"
).innerText; ).innerText;
assert.ok(withoutMembershipText.includes("eviltrout")); assert.ok(withoutMembershipText.includes("eviltrout"));
assert.ok(withoutMembershipText.includes("sam")); assert.ok(withoutMembershipText.includes("1 other"));
await click( await click(
".chat-message-container[data-id='176'] .chat-message-mention-warning .invite-link" ".chat-message-container[data-id='176'] .chat-message-mention-warning .invite-link"