Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Show warning if user won't be mentioned (#15339) 

The new warnings cover more cases and more accurate. Most of the
warnings will be visible only to staff members because otherwise they
would leak information about user's preferences.
This commit is contained in:
Bianca Nenciu 2022-01-11 09:16:20 +02:00 committed by GitHub
parent 6626089034
commit 5a8b8f6f1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 90 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/assets/javascripts/discourse/app/components/composer-editor.js
View File

@ -22,6 +22,7 @@ import {
linkSeenHashtags,
} from "discourse/lib/link-hashtags";
import {
cannotSee,
fetchUnseenMentions,
linkSeenMentions,
} from "discourse/lib/link-mentions";
@ -540,7 +541,7 @@ export default Component.extend(ComposerUploadUppy, {
`.mention.cannot-see[data-name="${name}"]`
)?.length > 0
) {
this.cannotSeeMention([{ name }]);
this.cannotSeeMention([{ name, reason: cannotSee[name] }]);
found.push(name);
}
},

10
app/assets/javascripts/discourse/app/controllers/composer.js
View File

@ -696,16 +696,12 @@ export default Controller.extend({
cannotSeeMention(mentions) {
mentions.forEach((mention) => {
const translation = this.get("model.topic.isPrivateMessage")
? "composer.cannot_see_mention.private"
: "composer.cannot_see_mention.category";
const body = I18n.t(translation, {
username: `@${mention.name}`,
});
this.appEvents.trigger("composer-messages:create", {
extraClass: "custom-body",
templateName: "custom-body",
body,
body: I18n.t(`composer.cannot_see_mention.${mention.reason}`, {
username: mention.name,
}),
});
});
},

6
app/assets/javascripts/discourse/app/lib/link-mentions.js
View File

@ -46,7 +46,7 @@ const found = {};
const foundGroups = {};
const mentionableGroups = {};
const checked = {};
const cannotSee = [];
export const cannotSee = {};
function updateFound(mentions, usernames) {
mentions.forEach((mention, index) => {
@ -101,7 +101,9 @@ export function fetchUnseenMentions(usernames, topic_id) {
r.valid.forEach((v) => (found[v] = true));
r.valid_groups.forEach((vg) => (foundGroups[vg] = true));
r.mentionable_groups.forEach((mg) => (mentionableGroups[mg.name] = mg));
r.cannot_see.forEach((cs) => (cannotSee[cs] = true));
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;

52
app/controllers/users_controller.rb
View File

@ -477,7 +477,7 @@ class UsersController < ApplicationController
usernames = params[:usernames] if params[:usernames].present?
usernames = [params[:username]] if params[:username].present?
raise Discourse::InvalidParameters.new(:usernames) if !usernames.kind_of?(Array)
raise Discourse::InvalidParameters.new(:usernames) if !usernames.kind_of?(Array) || usernames.size > 20
groups = Group.where(name: usernames).pluck(:name)
mentionable_groups =
@ -496,15 +496,53 @@ class UsersController < ApplicationController
usernames -= groups
usernames.each(&:downcase!)
cannot_see = []
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|
if !Guardian.new(User.find_by_username(username)).can_see?(topic)
cannot_see.push(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?
@ -512,12 +550,8 @@ class UsersController < ApplicationController
end
end
result = User.where(staged: false)
.where(username_lower: usernames)
.pluck(:username_lower)
render json: {
valid: result,
valid: users.keys,
valid_groups: groups,
mentionable_groups: mentionable_groups,
cannot_see: cannot_see,

6
config/locales/client.en.yml
View File

@ -2110,8 +2110,10 @@ en:
one: "By mentioning %{group}, you are about to notify <a href='%{group_link}'>%{count} person</a> – are you sure?"
other: "By mentioning %{group}, you are about to notify <a href='%{group_link}'>%{count} people</a> – are you sure?"
cannot_see_mention:
category: "You mentioned %{username} but they won't be notified because they do not have access to this category. You will need to add them to a group that has access to this category."
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 PM."
category: "You mentioned @%{username} but they won't be notified because they do not have access to this category. You will need to add them to a group that has access to this category."
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."
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?"

60
spec/requests/users_controller_spec.rb
View File

@ -3073,34 +3073,30 @@ describe UsersController do
get "/u/is_local_username.json", params: { username: user1.username }
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["valid"][0]).to eq(user1.username)
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)
json = response.parsed_body
expect(json["valid_groups"]).to include(group.name)
expect(json["mentionable_groups"].find { |g| g['name'] == group.name }).to be_present
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)
json = response.parsed_body
expect(json["valid_groups"]).to include(unmentionable.name)
expect(json["mentionable_groups"]).to be_blank
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)
json = response.parsed_body
expect(json["valid"].size).to eq(2)
expect(response.parsed_body["valid"]).to contain_exactly(user1.username, "system")
end
it "never includes staged accounts" do
@ -3109,8 +3105,7 @@ describe UsersController do
get "/u/is_local_username.json", params: { usernames: [staged.username] }
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["valid"].size).to eq(0)
expect(response.parsed_body["valid"]).to be_blank
end
it "returns user who cannot see topic" do
@ -3121,44 +3116,57 @@ describe UsersController do
}
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["cannot_see"].size).to eq(1)
expect(response.parsed_body["cannot_see"][user1.username]).to eq("category")
end
it "never returns a user who can see the topic" do
Guardian.any_instance.expects(:can_see?).with(topic).returns(true)
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: topic.id
}
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["cannot_see"].size).to eq(0)
expect(response.parsed_body["cannot_see"]).to be_blank
end
it "returns user who cannot see a private topic" do
Guardian.any_instance.expects(:can_see?).with(private_topic).returns(false)
get "/u/is_local_username.json", params: {
usernames: [user1.username], topic_id: private_topic.id
}
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["cannot_see"].size).to eq(1)
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
Guardian.any_instance.expects(:can_see?).with(private_topic).returns(true)
get "/u/is_local_username.json", params: {
usernames: [allowed_user.username], topic_id: private_topic.id
}
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["cannot_see"].size).to eq(0)
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