diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index f28bbdeff5a..fc8d4d02ddf 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -43,9 +43,13 @@ class Admin::GroupsController < Admin::ApplicationController end def members_update - @group.add_users(params[:user_ids].split(','), params[:access_level], current_user: current_user) + status = Members::CreateService.new(@group, current_user, params).execute - redirect_to [:admin, @group], notice: 'Users were successfully added.' + if status + redirect_to [:admin, @group], notice: 'Users were successfully added.' + else + redirect_to [:admin, @group], alert: 'No users specified.' + end end def destroy diff --git a/changelogs/unreleased/dz-refactor-admin-group-members.yml b/changelogs/unreleased/dz-refactor-admin-group-members.yml new file mode 100644 index 00000000000..993a6cac0df --- /dev/null +++ b/changelogs/unreleased/dz-refactor-admin-group-members.yml @@ -0,0 +1,4 @@ +--- +title: Refactor Admin::GroupsController#members_update method and add some specs +merge_request: 10735 +author: diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 84db26a958a..c29b2fe8946 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -22,4 +22,28 @@ describe Admin::GroupsController do expect(response).to redirect_to(admin_groups_path) end end + + describe 'PUT #members_update' do + let(:group_user) { create(:user) } + + it 'adds user to members' do + put :members_update, id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).to include group_user + end + + it 'adds no user to members' do + put :members_update, id: group, + user_ids: '', + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).not_to include group_user + end + end end