Merge pull request #4550 from cpradio/cannot-see-mention

FEATURE: Notify user when mention can't see the reply they were mentioned in

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);

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
+        });
+      });
     }
 
   },

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;
   });

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"

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

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. 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."
       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:

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