FEATURE: Improve composer warnings for mentions (#18796)

* FEATURE: Show warning if group cannot be mentioned

A similar warning is displayed when the user cannot be mentioned because
they have not been invited to the topic.

* FEATURE: Resolve mentions for new topic

This commit improves several improvements and refactors
/u/is_local_username route to a better /composer/mentions route that
can handle new topics too.

* FEATURE: Show warning if only some are notified

Sometimes users are still notified even if the group that was mentioned
was not invited to the message. This happens because its members were
invited directly or are members of other groups that were invited.

* DEV: Refactor _warnCannotSeeMention
This commit is contained in:
Bianca Nenciu 2022-12-05 20:22:05 +02:00 committed by GitHub
parent b46a7b51f7
commit 93859037ef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 718 additions and 400 deletions

View File

@ -23,7 +23,6 @@ import {
linkSeenHashtagsInContext,
} from "discourse/lib/hashtag-autocomplete";
import {
cannotSee,
fetchUnseenMentions,
linkSeenMentions,
} from "discourse/lib/link-mentions";
@ -126,6 +125,7 @@ export default Component.extend(ComposerUploadUppy, {
init() {
this._super(...arguments);
this.warnedCannotSeeMentions = [];
this.warnedGroupMentions = [];
},
@discourseComputed("composer.requiredCategoryMissing")
@ -474,13 +474,15 @@ export default Component.extend(ComposerUploadUppy, {
},
_renderUnseenMentions(preview, unseen) {
// 'Create a New Topic' scenario is not supported (per conversation with codinghorror)
// https://meta.discourse.org/t/taking-another-1-7-release-task/51986/7
fetchUnseenMentions(unseen, this.get("composer.topic.id")).then((r) => {
fetchUnseenMentions({
names: unseen,
topicId: this.get("composer.topic.id"),
allowedNames: this.get("composer.targetRecipients")?.split(","),
}).then((response) => {
linkSeenMentions(preview, this.siteSettings);
this._warnMentionedGroups(preview);
this._warnCannotSeeMention(preview);
this._warnHereMention(r.here_count);
this._warnHereMention(response.here_count);
});
},
@ -506,28 +508,27 @@ export default Component.extend(ComposerUploadUppy, {
}
},
@debounce(2000)
_warnMentionedGroups(preview) {
schedule("afterRender", () => {
let found = this.warnedGroupMentions || [];
preview?.querySelectorAll(".mention-group.notify")?.forEach((mention) => {
if (this._isInQuote(mention)) {
return;
}
preview
.querySelectorAll(".mention-group[data-mentionable-user-count]")
.forEach((mention) => {
const { name } = mention.dataset;
if (
this.warnedGroupMentions.includes(name) ||
this._isInQuote(mention)
) {
return;
}
let name = mention.dataset.name;
if (!found.includes(name)) {
this.groupsMentioned([
{
name,
user_count: mention.dataset.mentionableUserCount,
max_mentions: mention.dataset.maxMentions,
},
]);
found.push(name);
}
});
this.set("warnedGroupMentions", found);
this.warnedGroupMentions.push(name);
this.groupsMentioned({
name,
userCount: mention.dataset.mentionableUserCount,
maxMentions: mention.dataset.maxMentions,
});
});
});
},
@ -539,22 +540,35 @@ export default Component.extend(ComposerUploadUppy, {
return;
}
const warnings = [];
preview.querySelectorAll(".mention.cannot-see").forEach((mention) => {
preview.querySelectorAll(".mention[data-reason]").forEach((mention) => {
const { name } = mention.dataset;
if (this.warnedCannotSeeMentions.includes(name)) {
return;
}
this.warnedCannotSeeMentions.push(name);
warnings.push({ name, reason: cannotSee[name] });
this.cannotSeeMention({
name,
reason: mention.dataset.reason,
});
});
if (warnings.length > 0) {
this.cannotSeeMention(warnings);
}
preview
.querySelectorAll(".mention-group[data-reason]")
.forEach((mention) => {
const { name } = mention.dataset;
if (this.warnedCannotSeeMentions.includes(name)) {
return;
}
this.warnedCannotSeeMentions.push(name);
this.cannotSeeMention({
name,
reason: mention.dataset.reason,
notifiedCount: mention.dataset.notifiedUserCount,
isGroup: true,
});
});
},
_warnHereMention(hereCount) {
@ -562,13 +576,7 @@ export default Component.extend(ComposerUploadUppy, {
return;
}
discourseLater(
this,
() => {
this.hereMention(hereCount);
},
2000
);
this.hereMention(hereCount);
},
@bind

View File

@ -782,51 +782,62 @@ export default Controller.extend({
}
},
groupsMentioned(groups) {
groupsMentioned({ name, userCount, maxMentions }) {
if (
!this.get("model.creatingPrivateMessage") &&
!this.get("model.topic.isPrivateMessage")
this.get("model.creatingPrivateMessage") ||
this.get("model.topic.isPrivateMessage")
) {
groups.forEach((group) => {
let body;
const groupLink = getURL(`/g/${group.name}/members`);
const maxMentions = parseInt(group.max_mentions, 10);
const userCount = parseInt(group.user_count, 10);
return;
}
if (maxMentions < userCount) {
body = I18n.t("composer.group_mentioned_limit", {
group: `@${group.name}`,
count: maxMentions,
group_link: groupLink,
});
} else if (group.user_count > 0) {
body = I18n.t("composer.group_mentioned", {
group: `@${group.name}`,
count: userCount,
group_link: groupLink,
});
}
maxMentions = parseInt(maxMentions, 10);
userCount = parseInt(userCount, 10);
if (body) {
this.appEvents.trigger("composer-messages:create", {
extraClass: "custom-body",
templateName: "education",
body,
});
}
let body;
const groupLink = getURL(`/g/${name}/members`);
if (userCount > maxMentions) {
body = I18n.t("composer.group_mentioned_limit", {
group: `@${name}`,
count: maxMentions,
group_link: groupLink,
});
} else if (userCount > 0) {
body = I18n.t("composer.group_mentioned", {
group: `@${name}`,
count: userCount,
group_link: groupLink,
});
}
if (body) {
this.appEvents.trigger("composer-messages:create", {
extraClass: "custom-body",
templateName: "education",
body,
});
}
},
cannotSeeMention(mentions) {
mentions.forEach((mention) => {
this.appEvents.trigger("composer-messages:create", {
extraClass: "custom-body",
templateName: "education",
body: I18n.t(`composer.cannot_see_mention.${mention.reason}`, {
username: mention.name,
}),
cannotSeeMention({ name, reason, notifiedCount, isGroup }) {
notifiedCount = parseInt(notifiedCount, 10);
let body;
if (isGroup) {
body = I18n.t(`composer.cannot_see_group_mention.${reason}`, {
group: name,
count: notifiedCount,
});
} else {
body = I18n.t(`composer.cannot_see_mention.${reason}`, {
username: name,
});
}
this.appEvents.trigger("composer-messages:create", {
extraClass: "custom-body",
templateName: "education",
body,
});
},

View File

@ -1,112 +1,110 @@
import deprecated from "discourse-common/lib/deprecated";
import { ajax } from "discourse/lib/ajax";
import { formatUsername } from "discourse/lib/utilities";
import getURL from "discourse-common/lib/get-url";
import { ajax } from "discourse/lib/ajax";
import { userPath } from "discourse/lib/url";
import jQuery from "jquery";
import { formatUsername } from "discourse/lib/utilities";
let checked = {};
let foundUsers = {};
let userReasons = {};
let foundGroups = {};
let groupReasons = {};
let maxGroupMention;
function replaceSpan(element, username, opts) {
let extra = {};
let extraClass = [];
export function resetMentions() {
checked = {};
foundUsers = {};
userReasons = {};
foundGroups = {};
groupReasons = {};
maxGroupMention = null;
}
function replaceSpan(element, name, opts) {
const a = document.createElement("a");
if (opts && opts.group) {
if (opts.mentionable) {
extra = {
name: username,
mentionableUserCount: opts.mentionable.user_count,
maxMentions: maxGroupMention,
};
extraClass.push("notify");
}
if (opts.group) {
a.href = getURL(`/g/${name}`);
a.innerText = `@${name}`;
a.classList.add("mention-group");
a.setAttribute("href", getURL("/g/") + username);
a.classList.add("mention-group", ...extraClass);
a.innerText = `@${username}`;
if (!opts.reason && opts.details) {
a.dataset.mentionableUserCount = opts.details.user_count;
a.dataset.maxMentions = maxGroupMention;
}
} else {
if (opts && opts.cannot_see) {
extra = { name: username };
extraClass.push("cannot-see");
}
a.href = userPath(username.toLowerCase());
a.classList.add("mention", ...extraClass);
a.innerText = `@${formatUsername(username)}`;
a.href = userPath(name.toLowerCase());
a.innerText = `@${formatUsername(name)}`;
a.classList.add("mention");
}
Object.keys(extra).forEach((key) => {
a.dataset[key] = extra[key];
});
a.dataset.name = name;
if (opts.reason) {
a.dataset.reason = opts.reason;
if (opts.details) {
a.dataset.notifiedUserCount = opts.details.notified_count;
}
}
element.replaceWith(a);
}
const found = {};
const foundGroups = {};
const mentionableGroups = {};
const checked = {};
export const cannotSee = {};
function updateFound(mentions, usernames) {
function updateFound(mentions, names) {
mentions.forEach((mention, index) => {
const username = usernames[index];
if (found[username.toLowerCase()]) {
replaceSpan(mention, username, { cannot_see: cannotSee[username] });
} else if (mentionableGroups[username]) {
replaceSpan(mention, username, {
group: true,
mentionable: mentionableGroups[username],
const name = names[index];
if (foundUsers[name.toLowerCase()]) {
replaceSpan(mention, name, {
reason: userReasons[name],
});
} else if (foundGroups[username]) {
replaceSpan(mention, username, { group: true });
} else if (checked[username]) {
} else if (foundGroups[name]) {
replaceSpan(mention, name, {
group: true,
details: foundGroups[name],
reason: groupReasons[name],
});
} else if (checked[name]) {
mention.classList.add("mention-tested");
}
});
}
export function linkSeenMentions(elem, siteSettings) {
if (elem instanceof jQuery) {
elem = elem[0];
deprecated("linkSeenMentions now expects a DOM node as first parameter", {
since: "2.8.0.beta7",
dropFrom: "2.9.0.beta1",
id: "discourse.link-mentions.dom-node",
});
}
export function linkSeenMentions(element, siteSettings) {
const mentions = [
...elem.querySelectorAll("span.mention:not(.mention-tested)"),
...element.querySelectorAll("span.mention:not(.mention-tested)"),
];
if (mentions.length) {
const usernames = mentions.map((m) => m.innerText.slice(1));
updateFound(mentions, usernames);
return usernames
.uniq()
.filter(
(u) => !checked[u] && u.length >= siteSettings.min_username_length
);
if (mentions.length === 0) {
return [];
}
return [];
const names = mentions.map((mention) => mention.innerText.slice(1));
updateFound(mentions, names);
return names
.uniq()
.filter(
(name) =>
!checked[name] && name.length >= siteSettings.min_username_length
);
}
// 'Create a New Topic' scenario is not supported (per conversation with codinghorror)
// https://meta.discourse.org/t/taking-another-1-7-release-task/51986/7
export function fetchUnseenMentions(usernames, topic_id) {
return ajax(userPath("is_local_username"), {
data: { usernames, topic_id },
}).then((r) => {
r.valid.forEach((v) => (found[v] = true));
r.valid_groups.forEach((vg) => (foundGroups[vg] = true));
r.mentionable_groups.forEach((mg) => (mentionableGroups[mg.name] = mg));
Object.entries(r.cannot_see).forEach(
([username, reason]) => (cannotSee[username] = reason)
);
maxGroupMention = r.max_users_notified_per_group_mention;
usernames.forEach((u) => (checked[u] = true));
return r;
export async function fetchUnseenMentions({ names, topicId, allowedNames }) {
const response = await ajax("/composer/mentions", {
data: { names, topic_id: topicId, allowed_names: allowedNames },
});
names.forEach((name) => (checked[name] = true));
response.users.forEach((username) => (foundUsers[username] = true));
Object.entries(response.user_reasons).forEach(
([username, reason]) => (userReasons[username] = reason)
);
Object.entries(response.groups).forEach(
([name, details]) => (foundGroups[name] = details)
);
Object.entries(response.group_reasons).forEach(
([name, reason]) => (groupReasons[name] = reason)
);
maxGroupMention = response.max_users_notified_per_group_mention;
return response;
}

View File

@ -20,12 +20,12 @@ acceptance("Composer - Image Preview", function (needs) {
server.get("/posts/419", () => {
return helper.response({ id: 419 });
});
server.get("/u/is_local_username", () => {
server.get("/composer/mentions", () => {
return helper.response({
valid: [],
valid_groups: ["staff"],
mentionable_groups: [{ name: "staff", user_count: 30 }],
cannot_see: [],
users: [],
user_reasons: {},
groups: { staff: { user_count: 30 } },
group_reasons: {},
max_users_notified_per_group_mention: 100,
});
});

View File

@ -3,7 +3,7 @@ import {
exists,
query,
} from "discourse/tests/helpers/qunit-helpers";
import { click, triggerKeyEvent, visit } from "@ember/test-helpers";
import { click, fillIn, triggerKeyEvent, visit } from "@ember/test-helpers";
import { test } from "qunit";
import I18n from "I18n";
@ -39,3 +39,60 @@ acceptance("Composer - Messages", function (needs) {
);
});
});
acceptance("Composer - Messages - Cannot see group", function (needs) {
needs.user();
needs.pretender((server, helper) => {
server.get("/composer/mentions", () => {
return helper.response({
users: [],
user_reasons: {},
groups: {
staff: { user_count: 30 },
staff2: { user_count: 30, notified_count: 10 },
},
group_reasons: { staff: "not_allowed", staff2: "some_not_allowed" },
max_users_notified_per_group_mention: 100,
});
});
});
test("Shows warning in composer if group hasn't been invited", async function (assert) {
await visit("/t/130");
await click("button.create");
assert.ok(
!exists(".composer-popup"),
"composer warning is not shown by default"
);
await fillIn(".d-editor-input", "Mention @staff");
assert.ok(exists(".composer-popup"), "shows composer warning message");
assert.ok(
query(".composer-popup").innerHTML.includes(
I18n.t("composer.cannot_see_group_mention.not_allowed", {
group: "staff",
})
),
"warning message has correct body"
);
});
test("Shows warning in composer if group hasn't been invited, but some members have access already", async function (assert) {
await visit("/t/130");
await click("button.create");
assert.ok(
!exists(".composer-popup"),
"composer warning is not shown by default"
);
await fillIn(".d-editor-input", "Mention @staff2");
assert.ok(exists(".composer-popup"), "shows composer warning message");
assert.ok(
query(".composer-popup").innerHTML.includes(
I18n.t("composer.cannot_see_group_mention.some_not_allowed", {
group: "staff2",
count: 10,
})
),
"warning message has correct body"
);
});
});

View File

@ -70,12 +70,12 @@ acceptance("Composer", function (needs) {
server.get("/posts/419", () => {
return helper.response({ id: 419 });
});
server.get("/u/is_local_username", () => {
server.get("/composer/mentions", () => {
return helper.response({
valid: [],
valid_groups: ["staff"],
mentionable_groups: [{ name: "staff", user_count: 30 }],
cannot_see: [],
users: [],
user_reasons: {},
groups: { staff: { user_count: 30 } },
group_reasons: {},
max_users_notified_per_group_mention: 100,
});
});

View File

@ -173,12 +173,13 @@ export function applyDefaultHandlers(pretender) {
return response({ email: "eviltrout@example.com" });
});
pretender.get("/u/is_local_username", () =>
pretender.get("/composer/mentions", () =>
response({
valid: [],
valid_groups: [],
mentionable_groups: [],
cannot_see: [],
users: [],
user_reasons: {},
groups: {},
group_reasons: {},
max_users_notified_per_group_mention: 100,
})
);

View File

@ -75,6 +75,7 @@ import { resetSidebarSection } from "discourse/lib/sidebar/custom-sections";
import { resetNotificationTypeRenderers } from "discourse/lib/notification-types-manager";
import { resetUserMenuTabs } from "discourse/lib/user-menu/tab";
import { reset as resetLinkLookup } from "discourse/lib/link-lookup";
import { resetMentions } from "discourse/lib/link-mentions";
import { resetModelTransformers } from "discourse/lib/model-transformers";
import { cleanupTemporaryModuleRegistrations } from "./temporary-module-helper";
@ -208,6 +209,7 @@ export function testCleanup(container, app) {
resetUserMenuTabs();
resetLinkLookup();
resetModelTransformers();
resetMentions();
cleanupTemporaryModuleRegistrations();
}

View File

@ -8,26 +8,28 @@ module("Integration | Component | ComposerEditor", function (hooks) {
setupRenderingTest(hooks);
test("warns about users that will not see a mention", async function (assert) {
assert.expect(1);
assert.expect(2);
this.set("model", {});
this.set("noop", () => {});
this.set("expectation", (warnings) => {
assert.deepEqual(warnings, [
{ name: "user-no", reason: "a reason" },
{ name: "user-nope", reason: "a reason" },
]);
this.set("expectation", (warning) => {
if (warning.name === "user-no") {
assert.deepEqual(warning, { name: "user-no", reason: "a reason" });
} else if (warning.name === "user-nope") {
assert.deepEqual(warning, { name: "user-nope", reason: "a reason" });
}
});
pretender.get("/u/is_local_username", () => {
pretender.get("/composer/mentions", () => {
return response({
cannot_see: {
users: ["user-ok", "user-no", "user-nope"],
user_reasons: {
"user-no": "a reason",
"user-nope": "a reason",
},
mentionable_groups: [],
valid: ["user-ok", "user-no", "user-nope"],
valid_groups: [],
groups: {},
group_reasons: {},
max_users_notified_per_group_mention: 100,
});
});

View File

@ -8,27 +8,22 @@ import domFromString from "discourse-common/lib/dom-from-string";
module("Unit | Utility | link-mentions", function () {
test("linkSeenMentions replaces users and groups", async function (assert) {
pretender.get("/u/is_local_username", () =>
pretender.get("/composer/mentions", () =>
response({
valid: ["valid_user"],
valid_groups: ["valid_group"],
mentionable_groups: [
{
name: "mentionable_group",
user_count: 1,
},
],
cannot_see: [],
users: ["valid_user"],
user_reasons: {},
groups: {
valid_group: { user_count: 1 },
mentionable_group: { user_count: 1 },
},
group_reasons: { valid_group: "not_mentionable" },
max_users_notified_per_group_mention: 100,
})
);
await fetchUnseenMentions([
"valid_user",
"mentionable_group",
"valid_group",
"invalid",
]);
await fetchUnseenMentions({
names: ["valid_user", "mentionable_group", "valid_group", "invalid"],
});
const root = domFromString(`
<div>
@ -43,7 +38,7 @@ module("Unit | Utility | link-mentions", function () {
assert.strictEqual(root.querySelector("a").innerText, "@valid_user");
assert.strictEqual(root.querySelectorAll("a")[1].innerText, "@valid_group");
assert.strictEqual(
root.querySelector("a.notify").innerText,
root.querySelector("a[data-mentionable-user-count]").innerText,
"@mentionable_group"
);
assert.strictEqual(

View File

@ -0,0 +1,184 @@
# frozen_string_literal: true
class ComposerController < ApplicationController
requires_login
def mentions
@names = params.require(:names)
raise Discourse::InvalidParameters.new(:names) if !@names.kind_of?(Array) || @names.size > 20
if params[:topic_id].present?
@topic = Topic.find_by(id: params[:topic_id])
guardian.ensure_can_see!(@topic)
end
# allowed_names is necessary just for new private messages.
@allowed_names = if params[:allowed_names].present?
raise Discourse::InvalidParameters(:allowed_names) if !params[:allowed_names].is_a?(Array)
params[:allowed_names] << current_user.username
else
[]
end
user_reasons = {}
group_reasons = {}
@names.each do |name|
if user = users[name]
reason = user_reason(user)
user_reasons[name] = reason if reason.present?
elsif group = groups[name]
reason = group_reason(group)
group_reasons[name] = reason if reason.present?
end
end
if @topic && @names.include?(SiteSetting.here_mention) && guardian.can_mention_here?
here_count = PostAlerter.new.expand_here_mention(@topic.first_post, exclude_ids: [current_user.id]).size
end
serialized_groups = groups.values.reduce({}) do |hash, group|
serialized_group = { user_count: group.user_count }
if group_reasons[group.name] == :not_allowed &&
members_visible_group_ids.include?(group.id) &&
(@topic&.private_message? || @allowed_names.present?)
# Find users that are notified already because they have been invited
# directly or via a group
notified_count = GroupUser
# invited directly
.where(user_id: topic_allowed_user_ids)
.or(
# invited via a group
GroupUser.where(
user_id: GroupUser.where(group_id: topic_allowed_group_ids).select(:user_id)
)
)
.where(group_id: group.id)
.select(:user_id).distinct.count
if notified_count > 0
group_reasons[group.name] = :some_not_allowed
serialized_group[:notified_count] = notified_count
end
end
hash[group.name] = serialized_group
hash
end
render json: {
users: users.keys,
user_reasons: user_reasons,
groups: serialized_groups,
group_reasons: group_reasons,
here_count: here_count,
max_users_notified_per_group_mention: SiteSetting.max_users_notified_per_group_mention,
}
end
private
def user_reason(user)
reason = if @topic && !user.guardian.can_see?(@topic)
@topic.private_message? ? :private : :category
elsif @allowed_names.present? && !is_user_allowed?(user, topic_allowed_user_ids, topic_allowed_group_ids)
# This would normally be handled by the previous if, but that does not work for new private messages.
:private
elsif topic_muted_by.include?(user.id)
:muted_topic
elsif @topic&.private_message? && !is_user_allowed?(user, topic_allowed_user_ids, topic_allowed_group_ids)
# Admins can see the topic, but they will not be mentioned if they were not invited.
:not_allowed
end
# Regular users can see only basic information why the users cannot see the topic.
reason = nil if !guardian.is_staff? && reason != :private && reason != :category
reason
end
def group_reason(group)
if !mentionable_group_ids.include?(group.id)
:not_mentionable
elsif (@topic&.private_message? || @allowed_names.present?) && !topic_allowed_group_ids.include?(group.id)
:not_allowed
end
end
def is_user_allowed?(user, user_ids, group_ids)
user_ids.include?(user.id) || user.group_ids.any? { |group_id| group_ids.include?(group_id) }
end
def users
@users ||= User
.not_staged
.where(username_lower: @names.map(&:downcase))
.index_by(&:username_lower)
end
def groups
@groups ||= Group
.visible_groups(current_user)
.where('lower(name) IN (?)', @names.map(&:downcase))
.index_by(&:name)
end
def mentionable_group_ids
@mentionable_group_ids ||= Group
.mentionable(current_user, include_public: false)
.where(name: @names)
.pluck(:id)
.to_set
end
def members_visible_group_ids
@members_visible_group_ids ||= Group
.members_visible_groups(current_user)
.where(name: @names)
.pluck(:id)
.to_set
end
def topic_muted_by
@topic_muted_by ||= if @topic.present?
TopicUser
.where(topic: @topic)
.where(user_id: users.values.map(&:id))
.where(notification_level: TopicUser.notification_levels[:muted])
.pluck(:user_id)
.to_set
else
Set.new
end
end
def topic_allowed_user_ids
@topic_allowed_user_ids ||= if @allowed_names.present?
User
.where(username_lower: @allowed_names.map(&:downcase))
.pluck(:id)
.to_set
elsif @topic&.private_message?
TopicAllowedUser
.where(topic: @topic)
.pluck(:user_id)
.to_set
end
end
def topic_allowed_group_ids
@topic_allowed_group_ids ||= if @allowed_names.present?
Group
.messageable(current_user)
.where(name: @allowed_names)
.pluck(:id)
.to_set
elsif @topic&.private_message?
TopicAllowedGroup
.where(topic: @topic)
.pluck(:group_id)
.to_set
end
end
end

View File

@ -497,93 +497,6 @@ class UsersController < ApplicationController
end
end
def is_local_username
usernames = params[:usernames] if params[:usernames].present?
usernames = [params[:username]] if params[:username].present?
raise Discourse::InvalidParameters.new(:usernames) if !usernames.kind_of?(Array) || usernames.size > 20
groups = Group.where(name: usernames).pluck(:name)
mentionable_groups =
if current_user
Group.mentionable(current_user, include_public: false)
.where(name: usernames)
.pluck(:name, :user_count)
.map do |name, user_count|
{
name: name,
user_count: user_count
}
end
end
usernames -= groups
usernames.each(&:downcase!)
users = User
.where(staged: false, username_lower: usernames)
.index_by(&:username_lower)
cannot_see = {}
here_count = nil
topic_id = params[:topic_id]
if topic_id.present? && topic = Topic.find_by(id: topic_id)
topic_muted_by = TopicUser
.where(topic: topic)
.where(user_id: users.values.map(&:id))
.where(notification_level: TopicUser.notification_levels[:muted])
.pluck(:user_id)
.to_set
if topic.private_message?
topic_allowed_user_ids = TopicAllowedUser
.where(topic: topic)
.where(user_id: users.values.map(&:id))
.pluck(:user_id)
.to_set
topic_allowed_group_ids = TopicAllowedGroup
.where(topic: topic)
.pluck(:group_id)
.to_set
end
usernames.each do |username|
user = users[username]
next if user.blank?
cannot_see_reason = nil
if !user.guardian.can_see?(topic)
cannot_see_reason = topic.private_message? ? :private : :category
elsif topic_muted_by.include?(user.id)
cannot_see_reason = :muted_topic
elsif topic.private_message? && !topic_allowed_user_ids.include?(user.id) && !user.group_ids.any? { |group_id| topic_allowed_group_ids.include?(group_id) }
cannot_see_reason = :not_allowed
end
if !guardian.is_staff? && cannot_see_reason.present? && cannot_see_reason != :private && cannot_see_reason != :category
cannot_see_reason = nil # do not leak private information
end
cannot_see[username] = cannot_see_reason if cannot_see_reason.present?
end
if usernames.include?(SiteSetting.here_mention) && guardian.can_mention_here?
here_count = PostAlerter.new.expand_here_mention(topic.first_post, exclude_ids: [current_user.id]).size
end
end
render json: {
valid: users.keys,
valid_groups: groups,
mentionable_groups: mentionable_groups,
cannot_see: cannot_see,
here_count: here_count,
max_users_notified_per_group_mention: SiteSetting.max_users_notified_per_group_mention
}
end
def render_available_true
render(json: { available: true })
end

View File

@ -2233,6 +2233,12 @@ en:
private: "You mentioned @%{username} but they won't be notified because they are unable to see this personal message. You will need to invite them to this personal message."
muted_topic: "You mentioned @%{username} but they won't be notified because they muted this topic."
not_allowed: "You mentioned @%{username} but they won't be notified because they were not invited to this topic."
cannot_see_group_mention:
not_mentionable: "You cannot mention group @%{group}."
some_not_allowed:
one: "You mentioned @%{group} but only %{count} member will be notified because the other members are unable to see this personal message. You will need to invite them to this personal message."
other: "You mentioned @%{group} but only %{count} members will be notified because the other members are unable to see this personal message. You will need to invite them to this personal message."
not_allowed: "You mentioned @%{group} but none of its members will be notified because they are unable to see this personal message. You will need to invite them to this personal message."
here_mention:
one: "By mentioning <b>@%{here}</b>, you are about to notify %{count} user are you sure?"
other: "By mentioning <b>@%{here}</b>, you are about to notify %{count} users are you sure?"

View File

@ -393,6 +393,7 @@ Discourse::Application.routes.draw do
post "session/2fa/test-action" => "session#test_second_factor_restricted_route"
end
get "session/scopes" => "session#scopes"
get "composer/mentions" => "composer#mentions"
get "composer_messages" => "composer_messages#index"
get "composer_messages/user_not_seen_in_a_while" => "composer_messages#user_not_seen_in_a_while"
@ -425,7 +426,6 @@ Discourse::Application.routes.draw do
collection do
get "check_username"
get "check_email"
get "is_local_username"
end
end

View File

@ -0,0 +1,251 @@
# frozen_string_literal: true
RSpec.describe ComposerController do
describe '#mentions' do
fab!(:current_user) { Fabricate(:user) }
fab!(:user) { Fabricate(:user) }
fab!(:group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone], mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:invisible_group) { Fabricate(:group, visibility_level: Group.visibility_levels[:owners]) }
fab!(:unmessageable_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:nobody], mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
fab!(:unmentionable_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone], mentionable_level: Group::ALIAS_LEVELS[:nobody]) }
before do
sign_in(current_user)
end
context 'without a topic' do
it 'finds mentions' do
get '/composer/mentions.json', params: {
names: [
'invaliduserorgroup',
user.username,
group.name,
invisible_group.name,
unmessageable_group.name,
unmentionable_group.name
],
}
expect(response.status).to eq(200)
expect(response.parsed_body['users']).to contain_exactly(user.username)
expect(response.parsed_body['user_reasons']).to eq({})
expect(response.parsed_body['groups']).to eq({
group.name => { 'user_count' => group.user_count },
unmessageable_group.name => { 'user_count' => unmessageable_group.user_count },
unmentionable_group.name => { 'user_count' => unmentionable_group.user_count },
})
expect(response.parsed_body['group_reasons']).to eq({
unmentionable_group.name => 'not_mentionable',
})
expect(response.parsed_body['max_users_notified_per_group_mention'])
.to eq(SiteSetting.max_users_notified_per_group_mention)
end
end
context 'with a regular topic' do
fab!(:topic) { Fabricate(:topic) }
it 'finds mentions' do
get '/composer/mentions.json', params: {
names: [
'invaliduserorgroup',
user.username,
group.name,
invisible_group.name,
unmessageable_group.name,
unmentionable_group.name
],
topic_id: topic.id,
}
expect(response.status).to eq(200)
expect(response.parsed_body['users']).to contain_exactly(user.username)
expect(response.parsed_body['user_reasons']).to eq({})
expect(response.parsed_body['groups']).to eq(
group.name => { 'user_count' => group.user_count },
unmessageable_group.name => { 'user_count' => unmessageable_group.user_count },
unmentionable_group.name => { 'user_count' => unmentionable_group.user_count },
)
expect(response.parsed_body['group_reasons']).to eq(
unmentionable_group.name => 'not_mentionable',
)
expect(response.parsed_body['max_users_notified_per_group_mention'])
.to eq(SiteSetting.max_users_notified_per_group_mention)
end
end
context 'with a private message' do
fab!(:allowed_user) { Fabricate(:user) }
fab!(:topic) { Fabricate(:private_message_topic, user: allowed_user) }
it 'does not work if topic is not visible' do
get '/composer/mentions.json', params: {
names: [allowed_user.username], topic_id: topic.id
}
expect(response.status).to eq(403)
end
it 'finds mentions' do
sign_in(allowed_user)
topic.invite_group(Discourse.system_user, unmentionable_group)
get '/composer/mentions.json', params: {
names: [
'invaliduserorgroup',
user.username,
allowed_user.username,
group.name,
invisible_group.name,
unmessageable_group.name,
unmentionable_group.name
],
topic_id: topic.id,
}
expect(response.status).to eq(200)
expect(response.parsed_body['users']).to contain_exactly(
user.username, allowed_user.username
)
expect(response.parsed_body['user_reasons']).to eq(
user.username => 'private'
)
expect(response.parsed_body['groups']).to eq(
group.name => { 'user_count' => group.user_count },
unmessageable_group.name => { 'user_count' => unmessageable_group.user_count },
unmentionable_group.name => { 'user_count' => unmentionable_group.user_count },
)
expect(response.parsed_body['group_reasons']).to eq(
group.name => 'not_allowed',
unmessageable_group.name => 'not_allowed',
unmentionable_group.name => 'not_mentionable',
)
expect(response.parsed_body['max_users_notified_per_group_mention'])
.to eq(SiteSetting.max_users_notified_per_group_mention)
end
it 'returns notified_count' do
sign_in(allowed_user)
group.add(user)
topic.invite_group(Discourse.system_user, group)
other_group = Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone])
other_group.add(allowed_user)
other_group.add(user)
other_group.add(Fabricate(:user))
# Trying to mention other_group which has not been invited, but two of
# its members have been (allowed_user directly and user via group).
get '/composer/mentions.json', params: {
names: [other_group.name],
topic_id: topic.id,
}
expect(response.status).to eq(200)
expect(response.parsed_body['groups']).to eq(
other_group.name => { 'user_count' => 3, 'notified_count' => 2 }
)
expect(response.parsed_body['group_reasons']).to eq(
other_group.name => 'some_not_allowed'
)
end
end
context 'with a new private message' do
fab!(:allowed_user) { Fabricate(:user) }
it 'finds mentions' do
get '/composer/mentions.json', params: {
names: [
'invaliduserorgroup',
user.username,
allowed_user.username,
group.name,
invisible_group.name,
unmessageable_group.name,
unmentionable_group.name
],
allowed_names: [allowed_user.username, unmentionable_group.name],
}
expect(response.status).to eq(200)
expect(response.parsed_body['users']).to contain_exactly(
user.username, allowed_user.username
)
expect(response.parsed_body['user_reasons']).to eq(
user.username => 'private',
)
expect(response.parsed_body['groups']).to eq(
group.name => { 'user_count' => group.user_count },
unmessageable_group.name => { 'user_count' => unmessageable_group.user_count },
unmentionable_group.name => { 'user_count' => unmentionable_group.user_count },
)
expect(response.parsed_body['group_reasons']).to eq(
group.name => 'not_allowed',
unmessageable_group.name => 'not_allowed',
unmentionable_group.name => 'not_mentionable',
)
expect(response.parsed_body['max_users_notified_per_group_mention'])
.to eq(SiteSetting.max_users_notified_per_group_mention)
end
it 'returns notified_count' do
sign_in(allowed_user)
group.add(user)
other_group = Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:everyone])
other_group.add(allowed_user)
other_group.add(user)
other_group.add(Fabricate(:user))
# Trying to mention other_group which has not been invited, but two of
# its members have been (allowed_user directly and user via group).
get '/composer/mentions.json', params: {
names: [other_group.name],
allowed_names: [allowed_user.username, group.name],
}
expect(response.status).to eq(200)
expect(response.parsed_body['groups']).to eq(
other_group.name => { 'user_count' => 3, 'notified_count' => 2 }
)
expect(response.parsed_body['group_reasons']).to eq(
other_group.name => 'some_not_allowed'
)
end
end
context 'with an invalid topic' do
it 'returns an error' do
get '/composer/mentions.json', params: {
names: [
'invaliduserorgroup',
user.username,
group.name,
invisible_group.name,
unmessageable_group.name,
unmentionable_group.name
],
topic_id: -1,
}
expect(response.status).to eq(403)
end
end
end
end

View File

@ -3670,116 +3670,6 @@ RSpec.describe UsersController do
end
end
describe '#is_local_username' do
fab!(:group) { Fabricate(:group, name: "Discourse", mentionable_level: Group::ALIAS_LEVELS[:everyone]) }
let(:unmentionable) {
Fabricate(:group, name: "Unmentionable", mentionable_level: Group::ALIAS_LEVELS[:nobody])
}
fab!(:topic) { Fabricate(:topic) }
fab!(:allowed_user) { Fabricate(:user) }
fab!(:private_topic) { Fabricate(:private_message_topic, user: allowed_user) }
it "finds the user" do
get "/u/is_local_username.json", params: { username: user1.username }
expect(response.status).to eq(200)
expect(response.parsed_body["valid"][0]).to eq(user1.username)
end
it "finds the group" do
sign_in(user1)
get "/u/is_local_username.json", params: { username: group.name }
expect(response.status).to eq(200)
expect(response.parsed_body["valid_groups"]).to include(group.name)
expect(response.parsed_body["mentionable_groups"].find { |g| g['name'] == group.name }).to be_present
end
it "finds unmentionable groups" do
sign_in(user1)
get "/u/is_local_username.json", params: { username: unmentionable.name }
expect(response.status).to eq(200)
expect(response.parsed_body["valid_groups"]).to include(unmentionable.name)
expect(response.parsed_body["mentionable_groups"]).to be_blank
end
it "supports multiples usernames" do
get "/u/is_local_username.json", params: { usernames: [user1.username, "system"] }
expect(response.status).to eq(200)
expect(response.parsed_body["valid"]).to contain_exactly(user1.username, "system")
end
it "never includes staged accounts" do
staged = Fabricate(:user, staged: true)
get "/u/is_local_username.json", params: { usernames: [staged.username] }
expect(response.status).to eq(200)
expect(response.parsed_body["valid"]).to be_blank
end
it "returns user who cannot see topic" do
Guardian.any_instance.expects(:can_see?).with(topic).returns(false)
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"][user1.username]).to eq("category")
end
it "never returns a user who can see the topic" do
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"]).to be_blank
end
it "returns user who cannot see a private topic" do
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: private_topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"][user1.username]).to eq("private")
end
it "returns user who was not invited to topic" do
sign_in(Fabricate(:admin))
get "/u/is_local_username.json", params: {
usernames: [admin.username], topic_id: private_topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"][admin.username]).to eq("not_allowed")
end
it "never returns a user who can see the topic" do
get "/u/is_local_username.json", params: {
usernames: [allowed_user.username], topic_id: private_topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"]).to be_blank
end
it "returns the appropriate reason why user cannot see the topic" do
TopicUser.create!(user_id: user1.id, topic_id: topic.id, notification_level: TopicUser.notification_levels[:muted])
sign_in(admin)
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: topic.id
}
expect(response.status).to eq(200)
expect(response.parsed_body["cannot_see"][user1.username]).to eq("muted_topic")
end
end
describe '#topic_tracking_state' do
context 'when anon' do
it "raises an error on anon for topic_tracking_state" do