diff --git a/app/assets/javascripts/admin/models/admin-user.js.es6 b/app/assets/javascripts/admin/models/admin-user.js.es6 index b1e1202a906..0720976e69b 100644 --- a/app/assets/javascripts/admin/models/admin-user.js.es6 +++ b/app/assets/javascripts/admin/models/admin-user.js.es6 @@ -71,7 +71,12 @@ const AdminUser = Discourse.User.extend({ groupRemoved(groupId) { return ajax("/admin/users/" + this.get('id') + "/groups/" + groupId, { type: 'DELETE' - }).then(() => this.set('groups.[]', this.get('groups').rejectBy("id", groupId))); + }).then(() => { + this.set('groups.[]', this.get('groups').rejectBy("id", groupId)); + if (this.get('primary_group_id') === groupId) { + this.set('primary_group_id', null); + } + }); }, revokeApiKey() { diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 3f007d48c51..5db8236745c 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -140,11 +140,13 @@ class Admin::UsersController < Admin::AdminController render nothing: true end - def primary_group + group = Group.find(params[:primary_group_id].to_i) guardian.ensure_can_change_primary_group!(@user) - @user.primary_group_id = params[:primary_group_id] - @user.save! + if group.users.include?(@user) + @user.primary_group_id = params[:primary_group_id] + @user.save! + end render nothing: true end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 23bb1ebe8c7..f0613510857 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -195,6 +195,8 @@ describe Admin::UsersController do end context '.primary_group' do + let(:group) { Fabricate(:group) } + before do @another_user = Fabricate(:coding_horror) end @@ -211,9 +213,16 @@ describe Admin::UsersController do end it "changes the user's primary group" do - xhr :put, :primary_group, user_id: @another_user.id, primary_group_id: 2 + group.add(@another_user) + xhr :put, :primary_group, user_id: @another_user.id, primary_group_id: group.id @another_user.reload - expect(@another_user.primary_group_id).to eq(2) + expect(@another_user.primary_group_id).to eq(group.id) + end + + it "doesn't change primary group if they aren't a member of the group" do + xhr :put, :primary_group, user_id: @another_user.id, primary_group_id: group.id + @another_user.reload + expect(@another_user.primary_group_id).to be_nil end end @@ -519,6 +528,15 @@ describe Admin::UsersController do end end + context 'remove_group' do + it "also clears the user's primary group" do + g = Fabricate(:group) + u = Fabricate(:user, primary_group: g) + xhr :delete, :remove_group, group_id: g.id, user_id: u.id + expect(u.reload.primary_group).to be_nil + end + end + end