From 23371b026de7cce8ea2d05aed36a27bf4eefb8bd Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 28 Oct 2015 12:21:54 -0400 Subject: [PATCH] FIX: Don't raise an error if you try to assign a group that exists --- app/controllers/admin/users_controller.rb | 5 ++++- spec/controllers/admin/users_controller_spec.rb | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index c096cf078f6..ee03ac7058d 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -121,7 +121,10 @@ class Admin::UsersController < Admin::AdminController def add_group group = Group.find(params[:group_id].to_i) return render_json_error group unless group && !group.automatic - group.users << @user + + # We don't care about duplicate group assignment + group.users << @user rescue ActiveRecord::RecordNotUnique + render nothing: true end diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 4f93fe55a55..20d5d198df0 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -37,7 +37,6 @@ describe Admin::UsersController do expect(UserHistory.where(action: UserHistory.actions[:check_email], acting_user_id: @user.id).count).to eq(0) xhr :get, :index, show_emails: "true" - data = ::JSON.parse(response.body) expect(UserHistory.where(action: UserHistory.actions[:check_email], acting_user_id: @user.id).count).to eq(1) end @@ -173,6 +172,22 @@ describe Admin::UsersController do end end + context '.add_group' do + let(:user) { Fabricate(:user) } + let(:group) { Fabricate(:group) } + + it 'adds the user to the group' do + xhr :post, :add_group, group_id: group.id, user_id: user.id + expect(response).to be_success + + expect(GroupUser.where(user_id: user.id, group_id: group.id).exists?).to eq(true) + + # Doing it again doesn't raise an error + xhr :post, :add_group, group_id: group.id, user_id: user.id + expect(response).to be_success + end + end + context '.primary_group' do before do @another_user = Fabricate(:coding_horror)