From 824c2357608dd4b227fab861a3306cf56c539f07 Mon Sep 17 00:00:00 2001
From: cpradio <forums@cpradio.com>
Date: Mon, 14 Nov 2016 22:03:16 -0500
Subject: [PATCH] FEATURE: Notify user when mention can't see the reply they
were mentioned in FIX: Group Mention Notifications
---
.../components/composer-editor.js.es6 | 30 ++++++++++++----
.../discourse/controllers/composer.js.es6 | 16 +++++++++
.../discourse/lib/link-mentions.js.es6 | 21 +++++++----
.../discourse/templates/composer.hbs | 1 +
app/controllers/users_controller.rb | 9 ++++-
config/locales/client.en.yml | 3 ++
spec/controllers/users_controller_spec.rb | 35 +++++++++++++++++++
7 files changed, 100 insertions(+), 15 deletions(-)
diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6
index 4ba0c200574..06e04cf75c1 100644
--- a/app/assets/javascripts/discourse/components/composer-editor.js.es6
+++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6
@@ -137,9 +137,10 @@ export default Ember.Component.extend({
},
_renderUnseenMentions($preview, unseen) {
- fetchUnseenMentions(unseen).then(() => {
+ fetchUnseenMentions(unseen, this.get('topic.id')).then(() => {
linkSeenMentions($preview, this.siteSettings);
this._warnMentionedGroups($preview);
+ this._warnCannotSeeMention($preview);
});
},
@@ -170,19 +171,33 @@ export default Ember.Component.extend({
_warnMentionedGroups($preview) {
Ember.run.scheduleOnce('afterRender', () => {
- this._warnedMentions = this._warnedMentions || [];
- var found = [];
+ var found = this.get('warnedGroupMentions') || [];
$preview.find('.mention-group.notify').each((idx,e) => {
const $e = $(e);
var name = $e.data('name');
- found.push(name);
- if (this._warnedMentions.indexOf(name) === -1){
- this._warnedMentions.push(name);
+ if (found.indexOf(name) === -1){
this.sendAction('groupsMentioned', [{name: name, user_count: $e.data('mentionable-user-count')}]);
+ found.push(name);
}
});
- this._warnedMentions = found;
+ this.set('warnedGroupMentions', found);
+ });
+ },
+
+ _warnCannotSeeMention($preview) {
+ Ember.run.scheduleOnce('afterRender', () => {
+ var found = this.get('warnedCannotSeeMentions') || [];
+ $preview.find('.mention.cannot-see').each((idx,e) => {
+ const $e = $(e);
+ var name = $e.data('name');
+ if (found.indexOf(name) === -1) {
+ this.sendAction('cannotSeeMention', [{name: name}]);
+ found.push(name);
+ }
+ });
+
+ this.set('warnedCannotSeeMentions', found);
});
},
@@ -490,6 +505,7 @@ export default Ember.Component.extend({
}
this._warnMentionedGroups($preview);
+ this._warnCannotSeeMention($preview);
// Paint category hashtags
const unseenCategoryHashtags = linkSeenCategoryHashtags($preview);
diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6
index da85c4a66da..b25d7d76f65 100644
--- a/app/assets/javascripts/discourse/controllers/composer.js.es6
+++ b/app/assets/javascripts/discourse/controllers/composer.js.es6
@@ -344,6 +344,22 @@ export default Ember.Controller.extend({
});
});
}
+ },
+
+ cannotSeeMention(mentions) {
+ mentions.forEach(mention => {
+ const translation = (this.get('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
+ });
+ });
}
},
diff --git a/app/assets/javascripts/discourse/lib/link-mentions.js.es6 b/app/assets/javascripts/discourse/lib/link-mentions.js.es6
index d41bd7e5b9a..bed1ba9b515 100644
--- a/app/assets/javascripts/discourse/lib/link-mentions.js.es6
+++ b/app/assets/javascripts/discourse/lib/link-mentions.js.es6
@@ -1,16 +1,21 @@
import { ajax } from 'discourse/lib/ajax';
function replaceSpan($e, username, opts) {
+ let extra = "";
+ let extraClass = "";
+
if (opts && opts.group) {
- let extra = "";
- let extraClass = "";
if (opts.mentionable) {
extra = `data-name='${username}' data-mentionable-user-count='${opts.mentionable.user_count}'`;
extraClass = "notify";
}
$e.replaceWith(`<a href='${Discourse.getURL("/groups/") + username}' class='mention-group ${extraClass}' ${extra}>@${username}</a>`);
} else {
- $e.replaceWith(`<a href='${Discourse.getURL("/users/") + username.toLowerCase()}' class='mention'>@${username}</a>`);
+ if (opts && opts.cannot_see) {
+ extra = `data-name='${username}'`;
+ extraClass = "cannot-see";
+ }
+ $e.replaceWith(`<a href='${Discourse.getURL("/users/") + username.toLowerCase()}' class='mention ${extraClass}' ${extra}>@${username}</a>`);
}
}
@@ -18,6 +23,7 @@ const found = {};
const foundGroups = {};
const mentionableGroups = {};
const checked = {};
+const cannotSee = [];
function updateFound($mentions, usernames) {
Ember.run.scheduleOnce('afterRender', function() {
@@ -25,7 +31,7 @@ function updateFound($mentions, usernames) {
const $e = $(e);
const username = usernames[i];
if (found[username.toLowerCase()]) {
- replaceSpan($e, username);
+ replaceSpan($e, username, { cannot_see: cannotSee[username] });
} else if (foundGroups[username]) {
replaceSpan($e, username, { group: true, mentionable: mentionableGroups[username] });
} else if (checked[username]) {
@@ -45,11 +51,12 @@ export function linkSeenMentions($elem, siteSettings) {
return [];
}
-export function fetchUnseenMentions(usernames) {
- return ajax("/users/is_local_username", { data: { usernames } }).then(r => {
+export function fetchUnseenMentions(usernames, topic_id) {
+ return ajax("/users/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] = true);
+ r.mentionable_groups.forEach(mg => mentionableGroups[mg.name] = mg);
+ r.cannot_see.forEach(cs => cannotSee[cs] = true);
usernames.forEach(u => checked[u] = true);
return r;
});
diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs
index 2c47fc8fcd2..144427d9ef9 100644
--- a/app/assets/javascripts/discourse/templates/composer.hbs
+++ b/app/assets/javascripts/discourse/templates/composer.hbs
@@ -88,6 +88,7 @@
draftStatus=model.draftStatus
isUploading=isUploading
groupsMentioned="groupsMentioned"
+ cannotSeeMention="cannotSeeMention"
importQuote="importQuote"
showOptions="showOptions"
hideOptions="hideOptions"
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 4f85726b510..ea8e2c12a86 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -242,11 +242,18 @@ class UsersController < ApplicationController
usernames -= groups
usernames.each(&:downcase!)
+ cannot_see = []
+ topic_id = params[:topic_id]
+ unless topic_id.blank?
+ topic = Topic.find_by(id: topic_id)
+ usernames.each{ |username| cannot_see.push(username) unless Guardian.new(User.find_by_username(username)).can_see?(topic) }
+ end
+
result = User.where(staged: false)
.where(username_lower: usernames)
.pluck(:username_lower)
- render json: {valid: result, valid_groups: groups, mentionable_groups: mentionable_groups}
+ render json: {valid: result, valid_groups: groups, mentionable_groups: mentionable_groups, cannot_see: cannot_see}
end
def render_available_true
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 99415e82d0b..9fe08a651d0 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1045,6 +1045,9 @@ en:
group_mentioned:
one: "By mentioning {{group}}, you are about to notify <a href='{{group_link}}'>1 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. Would you like 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. Would you like to add them to this PM?"
duplicate_link: "It looks like your link to <b>{{domain}}</b> was already posted in the topic by <b>@{{username}}</b> in <a href='{{post_url}}'>a reply {{ago}}</a> – are you sure you want to post it again?"
error:
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 3f6331f0bc6..95a8f2e5737 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -1602,6 +1602,9 @@ describe UsersController do
let(:user) { Fabricate(:user) }
let(:group) { Fabricate(:group, name: "Discourse") }
+ let(:topic) { Fabricate(:topic) }
+ let(:allowed_user) { Fabricate(:user) }
+ let(:private_topic) { Fabricate(:private_message_topic, user: allowed_user) }
it "finds the user" do
xhr :get, :is_local_username, username: user.username
@@ -1632,6 +1635,38 @@ describe UsersController do
expect(json["valid"].size).to eq(0)
end
+ it "returns user who cannot see topic" do
+ Guardian.any_instance.expects(:can_see?).with(topic).returns(false)
+ xhr :get, :is_local_username, usernames: [user.username], topic_id: topic.id
+ expect(response).to be_success
+ json = JSON.parse(response.body)
+ expect(json["cannot_see"].size).to eq(1)
+ end
+
+ it "never returns a user who can see the topic" do
+ Guardian.any_instance.expects(:can_see?).with(topic).returns(true)
+ xhr :get, :is_local_username, usernames: [user.username], topic_id: topic.id
+ expect(response).to be_success
+ json = JSON.parse(response.body)
+ expect(json["cannot_see"].size).to eq(0)
+ end
+
+ it "returns user who cannot see a private topic" do
+ Guardian.any_instance.expects(:can_see?).with(private_topic).returns(false)
+ xhr :get, :is_local_username, usernames: [user.username], topic_id: private_topic.id
+ expect(response).to be_success
+ json = JSON.parse(response.body)
+ expect(json["cannot_see"].size).to eq(1)
+ end
+
+ it "never returns a user who can see the topic" do
+ Guardian.any_instance.expects(:can_see?).with(private_topic).returns(true)
+ xhr :get, :is_local_username, usernames: [allowed_user.username], topic_id: private_topic.id
+ expect(response).to be_success
+ json = JSON.parse(response.body)
+ expect(json["cannot_see"].size).to eq(0)
+ end
+
end
describe '.topic_tracking_state' do