diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 054392acd..5389b578d 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -70,14 +70,19 @@ class Admin::GroupsController < Admin::AdminController def add_members group = Group.find(params.require(:id)) - usernames = params.require(:usernames) return can_not_modify_automatic if group.automatic - usernames.split(",").each do |username| - if user = User.find_by_username(username) - group.add(user) - end + if params[:usernames].present? + users = User.where(username: params[:usernames].split(",")) + elsif params[:user_ids].present? + users = User.find(params[:user_ids].split(",")) + else + raise Discourse::InvalidParameters.new('user_ids or usernames must be present') + end + + users.each do |user| + group.add(user) end if group.save @@ -89,14 +94,20 @@ class Admin::GroupsController < Admin::AdminController def remove_member group = Group.find(params.require(:id)) - user_id = params.require(:user_id).to_i return can_not_modify_automatic if group.automatic - user = User.find(user_id) + if params[:user_id].present? + user = User.find(params[:user_id]) + elsif params[:username].present? + user = User.find_by_username(params[:username]) + else + raise Discourse::InvalidParameters.new('user_id or username must be present') + end + user.primary_group_id = nil if user.primary_group_id == group.id - group.users.delete(user_id) + group.users.delete(user.id) if group.save && user.save render json: success_json diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index f81e492b5..b56f2a4d6 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -110,16 +110,27 @@ describe Admin::GroupsController do expect(response.status).to eq(422) end - it "is able to add several members to a group" do - user1 = Fabricate(:user) - user2 = Fabricate(:user) - group = Fabricate(:group) + context "is able to add several members to a group" do - xhr :put, :add_members, id: group.id, usernames: [user1.username, user2.username].join(",") + let(:user1) { Fabricate(:user) } + let(:user2) { Fabricate(:user) } + let(:group) { Fabricate(:group) } - expect(response).to be_success - group.reload - expect(group.users.count).to eq(2) + it "adds by username" do + xhr :put, :add_members, id: group.id, usernames: [user1.username, user2.username].join(",") + + expect(response).to be_success + group.reload + expect(group.users.count).to eq(2) + end + + it "adds by id" do + xhr :put, :add_members, id: group.id, user_ids: [user1.id, user2.id].join(",") + + expect(response).to be_success + group.reload + expect(group.users.count).to eq(2) + end end end @@ -131,23 +142,41 @@ describe Admin::GroupsController do expect(response.status).to eq(422) end - it "is able to remove a member" do - group = Fabricate(:group) - user = Fabricate(:user) - group.add(user) - group.save + context "is able to remove a member" do - user.primary_group_id = group.id - user.save + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } - xhr :delete, :remove_member, id: group.id, user_id: user.id + before do + group.add(user) + group.save + end - expect(response).to be_success - group.reload - expect(group.users.count).to eq(0) + it "removes by id" do + xhr :delete, :remove_member, id: group.id, user_id: user.id - user.reload - expect(user.primary_group_id).to eq(nil) + expect(response).to be_success + group.reload + expect(group.users.count).to eq(0) + end + + it "removes by username" do + xhr :delete, :remove_member, id: group.id, username: user.username + + expect(response).to be_success + group.reload + expect(group.users.count).to eq(0) + end + + it "removes user.primary_group_id when user is removed from group" do + user.primary_group_id = group.id + user.save + + xhr :delete, :remove_member, id: group.id, username: user.username + + user.reload + expect(user.primary_group_id).to eq(nil) + end end end