Merge pull request #4983 from tgxworld/group_owners_can_invite_users_to_groups

Group owners can invite users to groups

app/assets/javascripts/admin/models/web-hook.js.es6

@@ -37,7 +37,7 @@ export default RestModel.extend({
   },
 
   groupFinder(term) {
-    return Group.findAll({search: term, ignore_automatic: false});
+    return Group.findAll({ term: term, ignore_automatic: false });
   },
 
   @computed('wildcard_web_hook', 'web_hook_event_types.[]')
@@ -82,4 +82,3 @@ export default RestModel.extend({
     return this.createProperties();
   }
 });
-

app/assets/javascripts/discourse/components/group-selector.js.es6

@@ -15,31 +15,27 @@ export default Ember.Component.extend({
 
   @on('didInsertElement')
   _initializeAutocomplete(opts) {
-    var self = this;
-    var selectedGroups;
-    var groupNames = this.get('groupNames');
+    let selectedGroups;
+    let groupNames = this.get('groupNames');
 
-    self.$('input').autocomplete({
+    this.$('input').autocomplete({
       allowAny: false,
       items: _.isArray(groupNames) ? groupNames : (Ember.isEmpty(groupNames)) ? [] : [groupNames],
       single: this.get('single'),
       updateData: (opts && opts.updateData) ? opts.updateData : false,
-      onChangeItems: function(items){
+      onChangeItems: items => {
         selectedGroups = items;
-        self.set("groupNames", items.join(","));
+        this.set("groupNames", items.join(","));
       },
-      transformComplete: function(g) {
+      transformComplete: g => {
         return g.name;
       },
-      dataSource: function(term) {
-        return self.get("groupFinder")(term).then(function(groups){
+      dataSource: term => {
+        return this.get("groupFinder")(term).then(groups => {
+          if(!selectedGroups) return groups;
 
-          if(!selectedGroups){
-            return groups;
-          }
-
-          return groups.filter(function(group){
-            return !selectedGroups.any(function(s){return s === group.name;});
+          return groups.filter(group => {
+            return !selectedGroups.any(s => s === group.name);
           });
         });
       },

app/assets/javascripts/discourse/components/search-advanced-options.js.es6

@@ -527,7 +527,7 @@ export default Em.Component.extend({
   },
 
   groupFinder(term) {
-    return Group.findAll({search: term, ignore_automatic: false});
+    return Group.findAll({ term: term, ignore_automatic: false });
   },
 
   badgeFinder(term) {

app/assets/javascripts/discourse/controllers/invite.js.es6

@@ -26,12 +26,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
     }
   },
 
-  @computed
-  isAdmin() {
-    return Discourse.User.currentProp("admin");
-  },
-
-  @computed('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to')
+  @computed('model.admin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to')
   disabled(isAdmin, emailOrUsername, invitingToTopic, isPrivateTopic, groupNames, saving, can_invite_to) {
     if (saving) return true;
     if (Ember.isEmpty(emailOrUsername)) return true;
@@ -48,7 +43,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
     return false;
   },
 
-  @computed('isAdmin', 'emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage')
+  @computed('model.admin', 'emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage')
   disabledCopyLink(isAdmin, emailOrUsername, saving, isPrivateTopic, groupNames, hasCustomMessage) {
     if (hasCustomMessage) return true;
     if (saving) return true;
@@ -98,10 +93,15 @@ export default Ember.Controller.extend(ModalFunctionality, {
   // Allow Existing Members? (username autocomplete)
   allowExistingMembers: Ember.computed.alias('invitingToTopic'),
 
+  @computed("model.admin", "model.group_users")
+  isGroupOwnerOrAdmin(isAdmin, groupUsers) {
+    return isAdmin || groupUsers.some(groupUser => groupUser.owner);
+  },
+
   // Show Groups? (add invited user to private group)
-  @computed('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'canInviteViaEmail')
-  showGroups(isAdmin, emailOrUsername, isPrivateTopic, isMessage, invitingToTopic, canInviteViaEmail) {
-    return isAdmin &&
+  @computed('isGroupOwnerOrAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'canInviteViaEmail')
+  showGroups(isGroupOwnerOrAdmin, emailOrUsername, isPrivateTopic, isMessage, invitingToTopic, canInviteViaEmail) {
+    return isGroupOwnerOrAdmin &&
            canInviteViaEmail &&
            !isMessage &&
            (emailValid(emailOrUsername) || isPrivateTopic || !invitingToTopic);
@@ -113,7 +113,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
   },
 
   // Instructional text for the modal.
-  @computed('isMessage', 'invitingToTopic', 'emailOrUsername', 'isPrivateTopic', 'isAdmin', 'canInviteViaEmail')
+  @computed('isMessage', 'invitingToTopic', 'emailOrUsername', 'isPrivateTopic', 'model.admin', 'canInviteViaEmail')
   inviteInstructions(isMessage, invitingToTopic, emailOrUsername, isPrivateTopic, isAdmin, canInviteViaEmail) {
     if (!canInviteViaEmail) {
       // can't invite via email, only existing users
@@ -150,7 +150,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
   },
 
   groupFinder(term) {
-    return Group.findAll({search: term, ignore_automatic: true});
+    return Group.findAll({ term: term, ignore_automatic: true });
   },
 
   @computed('isMessage', 'emailOrUsername', 'invitingExistingUserToTopic')

app/assets/javascripts/discourse/models/group.js.es6

@@ -216,7 +216,7 @@ const Group = RestModel.extend({
 
 Group.reopenClass({
   findAll(opts) {
-    return ajax("/admin/groups.json", { data: opts }).then(function (groups){
+    return ajax("/groups/search.json", { data: opts }).then(groups => {
       return groups.map(g => Group.create(g));
     });
   },

app/controllers/admin/groups_controller.rb

@@ -1,19 +1,4 @@
 class Admin::GroupsController < Admin::AdminController
-
-  def index
-    groups = Group.order(:name).where("groups.id <> ?", Group::AUTO_GROUPS[:everyone])
-
-    if search = params[:search].to_s
-      groups = groups.where("name ILIKE ?", "%#{search}%")
-    end
-
-    if params[:ignore_automatic].to_s == "true"
-      groups = groups.where(automatic: false)
-    end
-
-    render_serialized(groups, BasicGroupSerializer)
-  end
-
   def show
     render nothing: true
   end

app/controllers/groups_controller.rb

@@ -6,7 +6,8 @@ class GroupsController < ApplicationController
     :update,
     :messages,
     :histories,
-    :request_membership
+    :request_membership,
+    :search
   ]
 
   skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed]
@@ -296,6 +297,22 @@ class GroupsController < ApplicationController
     )
   end
 
+  def search
+    groups = Group.visible_groups(current_user)
+      .where("groups.id <> ?", Group::AUTO_GROUPS[:everyone])
+      .order(:name)
+
+    if term = params[:term].to_s
+      groups = groups.where("name ILIKE :term OR full_name ILIKE :term", term: "%#{term}%")
+    end
+
+    if params[:ignore_automatic].to_s == "true"
+      groups = groups.where(automatic: false)
+    end
+
+    render_serialized(groups, BasicGroupSerializer)
+  end
+
   private
 
   def group_params

app/controllers/invites_controller.rb

@@ -61,9 +61,13 @@ class InvitesController < ApplicationController
   def create
     params.require(:email)
 
-    group_ids = Group.lookup_group_ids(params)
+    groups = Group.lookup_groups(
+      group_ids: params[:group_ids],
+      group_names: params[:group_names]
+    )
 
-    guardian.ensure_can_invite_to_forum!(group_ids)
+    guardian.ensure_can_invite_to_forum!(groups)
+    group_ids = groups.map(&:id)
 
     invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first
     if invite_exists && !guardian.can_send_multiple_invites?(current_user)
@@ -71,7 +75,7 @@ class InvitesController < ApplicationController
     end
 
     begin
-      if Invite.invite_by_email(params[:email], current_user, _topic=nil,  group_ids, params[:custom_message])
+      if Invite.invite_by_email(params[:email], current_user, nil, group_ids, params[:custom_message])
         render json: success_json
       else
         render json: failed_json, status: 422
@@ -83,9 +87,15 @@ class InvitesController < ApplicationController
 
   def create_invite_link
     params.require(:email)
-    group_ids = Group.lookup_group_ids(params)
+
+    groups = Group.lookup_groups(
+      group_ids: params[:group_ids],
+      group_names: params[:group_names]
+    )
+
+    guardian.ensure_can_invite_to_forum!(groups)
     topic = Topic.find_by(id: params[:topic_id])
-    guardian.ensure_can_invite_to_forum!(group_ids)
+    group_ids = groups.map(&:id)
 
     invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first
     if invite_exists && !guardian.can_send_multiple_invites?(current_user)

app/controllers/topics_controller.rb

@@ -484,8 +484,14 @@ class TopicsController < ApplicationController
 
     topic = Topic.find_by(id: params[:topic_id])
 
-    group_ids = Group.lookup_group_ids(params)
-    guardian.ensure_can_invite_to!(topic,group_ids)
+
+    groups = Group.lookup_groups(
+      group_ids: params[:group_ids],
+      group_names: params[:group_names]
+    )
+
+    guardian.ensure_can_invite_to!(topic, groups)
+    group_ids = groups.map(&:id)
 
     begin
       if topic.invite(current_user, username_or_email, group_ids, params[:custom_message])

app/models/group.rb

@@ -338,22 +338,19 @@ class Group < ActiveRecord::Base
     end
   end
 
-  def self.lookup_group_ids(opts)
-    if group_ids = opts[:group_ids]
-      group_ids = group_ids.split(",").map(&:to_i)
-      group_ids = Group.where(id: group_ids).pluck(:id)
+  def self.lookup_groups(group_ids: [], group_names: [])
+    if group_ids.present?
+      group_ids = group_ids.split(",")
+      group_ids.map!(&:to_i)
+      groups = Group.where(id: group_ids) if group_ids.present?
     end
 
-    group_ids ||= []
-
-    if group_names = opts[:group_names]
+    if group_names.present?
       group_names = group_names.split(",")
-      if group_names.present?
-        group_ids += Group.where(name: group_names).pluck(:id)
-      end
+      groups = (groups || Group).where(name: group_names) if group_names.present?
     end
 
-    group_ids
+    groups || []
   end
 
   def self.desired_trust_level_groups(trust_level)

app/serializers/basic_group_user_serializer.rb

@@ -1,3 +1,7 @@
 class BasicGroupUserSerializer < ApplicationSerializer
-  attributes :group_id, :user_id, :notification_level
+  attributes :group_id, :user_id, :notification_level, :owner
+
+  def include_owner?
+    object.user_id == scope&.user&.id
+  end
 end

config/routes.rb

@@ -440,6 +440,10 @@ Discourse::Application.routes.draw do
     get 'mentionable'
     get 'logs' => 'groups#histories'
 
+    collection do
+      get "search" => "groups#search"
+    end
+
     member do
       put "members" => "groups#add_members"
       delete "members" => "groups#remove_member"

lib/guardian.rb

@@ -242,16 +242,16 @@ class Guardian
       (!SiteSetting.must_approve_users? && @user.has_trust_level?(TrustLevel[2])) ||
       is_staff?
     ) &&
-    (groups.blank? || is_admin?)
+    (groups.blank? || is_admin? || groups.all? { |g| can_edit_group?(g) })
   end
 
-  def can_invite_to?(object, group_ids=nil)
+  def can_invite_to?(object, groups=nil)
     return false unless authenticated?
     return true if is_admin?
     return false unless SiteSetting.enable_private_messages?
     return false if (SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?)
     return false unless can_see?(object)
-    return false if group_ids.present?
+    return false if groups.present?
 
     if object.is_a?(Topic) && object.category
       if object.category.groups.any?

spec/components/guardian_spec.rb

@@ -304,7 +304,7 @@ describe Guardian do
     end
 
     it 'returns true when the site requires approving users and is mod' do
-      SiteSetting.expects(:must_approve_users?).returns(true)
+      SiteSetting.must_approve_users = true
       expect(Guardian.new(moderator).can_invite_to_forum?).to be_truthy
     end
 
@@ -328,6 +328,29 @@ describe Guardian do
       expect(Guardian.new(moderator).can_invite_to_forum?).to be_falsey
     end
 
+    context 'with groups' do
+      let(:group) { Fabricate(:group) }
+      let(:another_group) { Fabricate(:group) }
+      let(:groups) { [group, another_group] }
+
+      before do
+        user.update!(trust_level: TrustLevel[2])
+        group.add_owner(user)
+      end
+
+      it 'returns false when user is not allowed to edit a group' do
+        expect(Guardian.new(user).can_invite_to_forum?(groups)).to eq(false)
+
+        expect(Guardian.new(Fabricate(:admin)).can_invite_to_forum?(groups))
+          .to eq(true)
+      end
+
+      it 'returns true when user is allowed to edit groups' do
+        another_group.add_owner(user)
+
+        expect(Guardian.new(user).can_invite_to_forum?(groups)).to eq(true)
+      end
+    end
   end
 
   describe 'can_invite_to?' do

spec/controllers/admin/groups_controller_spec.rb

@@ -12,46 +12,6 @@ describe Admin::GroupsController do
     expect(Admin::GroupsController < Admin::AdminController).to eq(true)
   end
 
-  context ".index" do
-
-    it "produces valid json for groups" do
-      group = Fabricate.build(:group, name: "test")
-      group.add(@admin)
-      group.save
-
-      xhr :get, :index
-      expect(response.status).to eq(200)
-      json = ::JSON.parse(response.body)
-      expect(json.select { |r| r["id"] == Group::AUTO_GROUPS[:everyone] }).to be_empty
-      expect(json.select { |r| r["id"] == group.id }).to eq([{
-        "id"=>group.id,
-        "name"=>group.name,
-        "user_count"=>1,
-        "automatic"=>false,
-        "alias_level"=>0,
-        "visibility_level"=>0,
-        "automatic_membership_email_domains"=>nil,
-        "automatic_membership_retroactive"=>false,
-        "title"=>nil,
-        "primary_group"=>false,
-        "grant_trust_level"=>nil,
-        "incoming_email"=>nil,
-        "has_messages"=>false,
-        "flair_url"=>nil,
-        "flair_bg_color"=>nil,
-        "flair_color"=>nil,
-        "bio_raw"=>nil,
-        "bio_cooked"=>nil,
-        "public"=>false,
-        "allow_membership_requests"=>false,
-        "full_name"=>group.full_name,
-        "default_notification_level"=>3
-      }])
-
-    end
-
-  end
-
   context ".bulk" do
     it "can assign users to a group by email or username" do
       group = Fabricate(:group, name: "test", primary_group: true, title: 'WAT', grant_trust_level: 3)

spec/controllers/invites_controller_spec.rb

@@ -52,11 +52,11 @@ describe InvitesController do
 
   end
 
-  context '.create' do
+  context '#create' do
     it 'requires you to be logged in' do
-      expect {
+      expect do
         post :create, email: 'jake@adventuretime.ooo'
-      }.to raise_error(Discourse::NotLoggedIn)
+      end.to raise_error(Discourse::NotLoggedIn)
     end
 
     context 'while logged in' do
@@ -86,6 +86,18 @@ describe InvitesController do
         expect(Invite.find_by(email: email).invited_groups.count).to eq(1)
       end
 
+      it 'allows group owners to invite to groups' do
+        group = Fabricate(:group)
+        user = log_in
+        user.update!(trust_level: TrustLevel[2])
+        group.add_owner(user)
+
+        post :create, email: email, group_names: group.name
+
+        expect(response).to be_success
+        expect(Invite.find_by(email: email).invited_groups.count).to eq(1)
+      end
+
       it "allows admin to send multiple invites to same email" do
         user = log_in(:admin)
         invite = Invite.invite_by_email("invite@example.com", user)

spec/integration/groups_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 describe "Groups" do
   let(:user) { Fabricate(:user) }
-  let!(:group) { Fabricate(:group, users: [user]) }
+  let(:group) { Fabricate(:group, users: [user]) }
 
   describe 'viewing groups' do
     let!(:staff_group) do
@@ -19,6 +19,7 @@ describe "Groups" do
     end
 
     it 'should return the right response' do
+      group
       get "/groups.json"
 
       expect(response).to be_success
@@ -578,4 +579,96 @@ describe "Groups" do
       expect(topic.allowed_groups).to eq([])
     end
   end
+
+  describe 'search for groups' do
+    let(:hidden_group) do
+      Fabricate(:group,
+        visibility_level: Group.visibility_levels[:owners],
+        name: 'KingOfTheNorth'
+      )
+    end
+
+    before do
+      group.update!(
+        name: 'GOT',
+        full_name: 'Daenerys Targaryen'
+      )
+
+      hidden_group
+    end
+
+    context 'as an anon user' do
+      it "returns the right response" do
+        expect { xhr :get, '/groups/search' }.to raise_error(Discourse::NotLoggedIn)
+      end
+    end
+
+    context 'as a normal user' do
+      it "returns the right response" do
+        sign_in(user)
+
+        xhr :get, '/groups/search'
+
+        expect(response).to be_success
+        groups = JSON.parse(response.body)
+
+        expected_ids = Group::AUTO_GROUPS.map { |name, id| id }
+        expected_ids.delete(Group::AUTO_GROUPS[:everyone])
+        expected_ids << group.id
+
+        expect(groups.map { |group| group["id"] }).to contain_exactly(*expected_ids)
+
+        ['GO', 'nerys'].each do |term|
+          xhr :get, "/groups/search?term=#{term}"
+
+          expect(response).to be_success
+          groups = JSON.parse(response.body)
+
+          expect(groups.length).to eq(1)
+          expect(groups.first['id']).to eq(group.id)
+        end
+
+        xhr :get, "/groups/search?term=KingOfTheNorth"
+
+        expect(response).to be_success
+        groups = JSON.parse(response.body)
+
+        expect(groups).to eq([])
+      end
+    end
+
+    context 'as a group owner' do
+      before do
+        hidden_group.add_owner(user)
+      end
+
+      it "returns the right response" do
+        sign_in(user)
+
+        xhr :get, "/groups/search?term=north"
+
+        expect(response).to be_success
+        groups = JSON.parse(response.body)
+
+        expect(groups.length).to eq(1)
+        expect(groups.first['id']).to eq(hidden_group.id)
+      end
+    end
+
+    context 'as an admin' do
+      it "returns the right response" do
+        sign_in(Fabricate(:admin))
+
+        xhr :get, '/groups/search?ignore_automatic=true'
+
+        expect(response).to be_success
+        groups = JSON.parse(response.body)
+
+        expect(groups.length).to eq(2)
+
+        expect(groups.map { |group| group['id'] })
+          .to contain_exactly(group.id, hidden_group.id)
+      end
+    end
+  end
 end

spec/serializers/basic_group_user_serializer_spec.rb

@@ -0,0 +1,36 @@
+require 'rails_helper'
+
+describe BasicGroupUserSerializer do
+  let(:group) { Fabricate(:group) }
+  let(:user) { Fabricate(:user) }
+
+  before do
+    group.add(user)
+  end
+
+  describe '#owner' do
+    describe 'when scoped to the user' do
+      it 'should be false' do
+        json = described_class.new(
+          GroupUser.last,
+          scope: Guardian.new(user),
+          root: false
+        ).as_json
+
+        expect(json[:owner]).to eq(false)
+      end
+    end
+
+    describe 'when not scoped to the user' do
+      it 'should be nil' do
+        json = described_class.new(
+          GroupUser.last,
+          scope: Guardian.new,
+          root: false
+        ).as_json
+
+        expect(json[:owner]).to eq(nil)
+      end
+    end
+  end
+end