From a0f03936ff6d8908995489c5676c63ef4478971b Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Wed, 17 May 2017 12:42:04 -0400
Subject: [PATCH] FIX: saving invisible primary group field that you don't
 belong to

---
 .../admin/models/admin-user.js.es6            |  7 +++++-
 app/controllers/admin/users_controller.rb     |  8 ++++---
 .../admin/users_controller_spec.rb            | 22 +++++++++++++++++--
 3 files changed, 31 insertions(+), 6 deletions(-)

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