From 9899e8d4a5c71d62f7cdc5dd1c71abc621b2fdc0 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 2 Dec 2015 15:49:43 +1100
Subject: [PATCH] FEATURE: First class messages to groups, you can select a
 group as a target of a message

---
 .../discourse/components/user-selector.js.es6 | 18 +++++++---
 .../discourse/controllers/composer.js.es6     |  3 +-
 .../discourse/lib/user-search.js.es6          |  6 ++--
 .../components/private-message-map.hbs        |  4 +--
 .../discourse/templates/composer.hbs          |  6 ++--
 app/controllers/posts_controller.rb           |  8 +++++
 app/controllers/users_controller.rb           |  8 +++++
 app/models/group.rb                           |  2 +-
 lib/topic_creator.rb                          | 17 ++++++++--
 spec/components/post_creator_spec.rb          |  2 +-
 spec/controllers/posts_controller_spec.rb     | 34 +++++++++++++++++++
 11 files changed, 93 insertions(+), 15 deletions(-)

diff --git a/app/assets/javascripts/discourse/components/user-selector.js.es6 b/app/assets/javascripts/discourse/components/user-selector.js.es6
index 5b0be280c..aa1c4b7e8 100644
--- a/app/assets/javascripts/discourse/components/user-selector.js.es6
+++ b/app/assets/javascripts/discourse/components/user-selector.js.es6
@@ -6,7 +6,9 @@ export default TextField.extend({
   _initializeAutocomplete: function() {
     var self = this,
         selected = [],
+        groups = [],
         currentUser = this.currentUser,
+        includeMentionableGroups = this.get('includeMentionableGroups') === 'true',
         includeGroups = this.get('includeGroups') === 'true',
         allowedUsers = this.get('allowedUsers') === 'true';
 
@@ -24,18 +26,22 @@ export default TextField.extend({
       allowAny: this.get('allowAny'),
 
       dataSource: function(term) {
-        return userSearch({
+        var results = userSearch({
           term: term.replace(/[^a-zA-Z0-9_\-\.]/, ''),
           topicId: self.get('topicId'),
           exclude: excludedUsernames(),
           includeGroups,
-          allowedUsers
+          allowedUsers,
+          includeMentionableGroups
         });
+
+        return results;
       },
 
       transformComplete: function(v) {
-        if (v.username) {
-          return v.username;
+        if (v.username || v.name) {
+          if (!v.username) { groups.push(v.name); }
+          return v.username || v.name;
         } else {
           var excludes = excludedUsernames();
           return v.usernames.filter(function(item){
@@ -45,10 +51,14 @@ export default TextField.extend({
       },
 
       onChangeItems: function(items) {
+        var hasGroups = false;
         items = items.map(function(i) {
+          if (groups.indexOf(i) > -1) { hasGroups = true; }
           return i.username ? i.username : i;
         });
         self.set('usernames', items.join(","));
+        self.set('hasGroups', hasGroups);
+
         selected = items;
       },
 
diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6
index d77c061c4..d4b15a0af 100644
--- a/app/assets/javascripts/discourse/controllers/composer.js.es6
+++ b/app/assets/javascripts/discourse/controllers/composer.js.es6
@@ -75,9 +75,10 @@ export default Ember.Controller.extend({
     if (!Discourse.User.currentProp('staff')) { return false; }
 
     var usernames = this.get('model.targetUsernames');
+    var hasTargetGroups = this.get('model.hasTargetGroups');
 
     // We need exactly one user to issue a warning
-    if (Ember.isEmpty(usernames) || usernames.split(',').length !== 1) {
+    if (Ember.isEmpty(usernames) || usernames.split(',').length !== 1 || hasTargetGroups) {
       return false;
     }
     return this.get('model.creatingPrivateMessage');
diff --git a/app/assets/javascripts/discourse/lib/user-search.js.es6 b/app/assets/javascripts/discourse/lib/user-search.js.es6
index 3600814bb..08d5c6bd6 100644
--- a/app/assets/javascripts/discourse/lib/user-search.js.es6
+++ b/app/assets/javascripts/discourse/lib/user-search.js.es6
@@ -6,7 +6,7 @@ var cache = {},
     currentTerm,
     oldSearch;
 
-function performSearch(term, topicId, includeGroups, allowedUsers, resultsFn) {
+function performSearch(term, topicId, includeGroups, includeMentionableGroups, allowedUsers, resultsFn) {
   var cached = cache[term];
   if (cached) {
     resultsFn(cached);
@@ -18,6 +18,7 @@ function performSearch(term, topicId, includeGroups, allowedUsers, resultsFn) {
     data: { term: term,
             topic_id: topicId,
             include_groups: includeGroups,
+            include_mentionable_groups: includeMentionableGroups,
             topic_allowed_users: allowedUsers }
   });
 
@@ -76,6 +77,7 @@ function organizeResults(r, options) {
 export default function userSearch(options) {
   var term = options.term || "",
       includeGroups = options.includeGroups,
+      includeMentionableGroups = options.includeMentionableGroups,
       allowedUsers = options.allowedUsers,
       topicId = options.topicId;
 
@@ -103,7 +105,7 @@ export default function userSearch(options) {
       resolve(CANCELLED_STATUS);
     }, 5000);
 
-    debouncedSearch(term, topicId, includeGroups, allowedUsers, function(r) {
+    debouncedSearch(term, topicId, includeGroups, includeMentionableGroups, allowedUsers, function(r) {
       clearTimeout(clearPromise);
       resolve(organizeResults(r, options));
     });
diff --git a/app/assets/javascripts/discourse/templates/components/private-message-map.hbs b/app/assets/javascripts/discourse/templates/components/private-message-map.hbs
index 64677ca63..eb2d2dd5f 100644
--- a/app/assets/javascripts/discourse/templates/components/private-message-map.hbs
+++ b/app/assets/javascripts/discourse/templates/components/private-message-map.hbs
@@ -1,8 +1,8 @@
-<h3>{{fa-icon "envelope"}} {{i18n 'private_message_info.title'}}</h3>
+<h3>{{fa-icon 'envelope'}} {{i18n 'private_message_info.title'}}</h3>
 <div class='participants clearfix'>
   {{#each details.allowed_groups as |ag|}}
     <div class='user group'>
-      #{{unbound ag.name}}
+      {{fa-icon 'users'}} {{#link-to "group.index" ag.name}}{{unbound ag.name}}{{/link-to}}
     </div>
   {{/each}}
   {{#each details.allowed_users as |au|}}
diff --git a/app/assets/javascripts/discourse/templates/composer.hbs b/app/assets/javascripts/discourse/templates/composer.hbs
index aac6c07f1..077793780 100644
--- a/app/assets/javascripts/discourse/templates/composer.hbs
+++ b/app/assets/javascripts/discourse/templates/composer.hbs
@@ -41,11 +41,13 @@
                 {{user-selector topicId=controller.controllers.topic.model.id
                                 excludeCurrentUser="true"
                                 id="private-message-users"
-                                includeGroups="true"
+                                includeMentionableGroups="true"
                                 class="span8"
                                 placeholderKey="composer.users_placeholder"
                                 tabindex="1"
-                                usernames=model.targetUsernames}}
+                                usernames=model.targetUsernames
+                                hasGroups=model.hasTargetGroups
+                                }}
                 {{#if showWarning}}
                   <div class='add-warning'>
                     <label>
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 0c20e433a..2b172ed8c 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -507,6 +507,14 @@ class PostsController < ApplicationController
     result[:user_agent] = request.user_agent
     result[:referrer] = request.env["HTTP_REFERER"]
 
+    if usernames = result[:target_usernames]
+      usernames = usernames.split(",")
+      groups = Group.mentionable(current_user).where('name in (?)', usernames).pluck('name')
+      usernames -= groups
+      result[:target_usernames] = usernames.join(",")
+      result[:target_group_names] = groups.join(",")
+    end
+
     result
   end
 
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 83a5ede59..8c28d3f18 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -548,6 +548,14 @@ class UsersController < ApplicationController
       end
     end
 
+    if params[:include_mentionable_groups] == "true" && current_user
+      to_render[:groups] = Group.mentionable(current_user)
+                                .where("name ILIKE :term_like", term_like: "#{term}%")
+                                .map do |m|
+        {name: m.name, usernames: []}
+      end
+    end
+
     render json: to_render
   end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 82ab38952..858a06d4d 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -201,7 +201,7 @@ class Group < ActiveRecord::Base
   end
 
   def self.search_group(name)
-    Group.where(visible: true).where("name ILIKE :term_like", term_like: "#{name.downcase}%")
+    Group.where(visible: true).where("name ILIKE :term_like", term_like: "#{name}%")
   end
 
   def self.lookup_group(name)
diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb
index ce8760a84..7d6691e99 100644
--- a/lib/topic_creator.rb
+++ b/lib/topic_creator.rb
@@ -150,19 +150,32 @@ class TopicCreator
 
   def add_users(topic, usernames)
     return unless usernames
-    User.where(username: usernames.split(',').flatten).each do |user|
+
+    names = usernames.split(',').flatten
+    len = 0
+
+    User.where(username: names).each do |user|
       check_can_send_permission!(topic, user)
       @added_users << user
       topic.topic_allowed_users.build(user_id: user.id)
+      len += 1
     end
+
+    rollback_with!(topic, :target_user_not_found) unless len == names.length
   end
 
   def add_groups(topic, groups)
     return unless groups
-    Group.where(name: groups.split(',')).each do |group|
+    names = groups.split(',')
+    len = 0
+
+    Group.where(name: names).each do |group|
       check_can_send_permission!(topic, group)
       topic.topic_allowed_groups.build(group_id: group.id)
+      len += 1
     end
+
+    rollback_with!(topic, :target_group_not_found) unless len == names.length
   end
 
   def check_can_send_permission!(topic, obj)
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
index 33bd648bb..5bbada846 100644
--- a/spec/components/post_creator_spec.rb
+++ b/spec/components/post_creator_spec.rb
@@ -544,7 +544,7 @@ describe PostCreator do
                                target_group_names: group.name)
     end
 
-    it 'acts correctly' do
+    it 'can post to a group correctly' do
       expect(post.topic.archetype).to eq(Archetype.private_message)
       expect(post.topic.topic_allowed_users.count).to eq(1)
       expect(post.topic.topic_allowed_groups.count).to eq(1)
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index 885840e76..f7afc4254 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -583,6 +583,40 @@ describe PostsController do
         expect(parsed['cooked']).to be_present
       end
 
+      it "can send a message to a group" do
+
+        group = Group.create(name: 'test_group', alias_level: Group::ALIAS_LEVELS[:nobody])
+        user1 = Fabricate(:user)
+        group.add(user1)
+
+        xhr :post, :create, {
+          raw: 'I can haz a test',
+          title: 'I loves my test',
+          target_usernames: group.name,
+          archetype: Archetype.private_message
+        }
+
+        expect(response).not_to be_success
+
+        # allow pm to this group
+        group.update_columns(alias_level: Group::ALIAS_LEVELS[:everyone])
+
+        xhr :post, :create, {
+          raw: 'I can haz a test',
+          title: 'I loves my test',
+          target_usernames: group.name,
+          archetype: Archetype.private_message
+        }
+
+        expect(response).to be_success
+
+        parsed = ::JSON.parse(response.body)
+        post = Post.find(parsed['id'])
+
+        expect(post.topic.topic_allowed_users.length).to eq(1)
+        expect(post.topic.topic_allowed_groups.length).to eq(1)
+      end
+
       it "returns the nested post with a param" do
         xhr :post, :create, {raw: 'this is the test content',
                              title: 'this is the test title for the topic',