Reviewer feedback

- Remove unused classes
- Use `fetch` instead of ternary logic
- Add test for calling the `Users::UpdateService`
- Convert setup_for_company param from true to false in test case
- Adjust radio button styling

app/controllers/groups_controller.rb

@@ -350,7 +350,7 @@ class GroupsController < Groups::ApplicationController
   end
 
   def update_user_role_and_setup_for_company
-    user_params = params.key?(:user) ? params.require(:user).permit(:role) : {}
+    user_params = params.fetch(:user, {}).permit(:role)
     user_params[:setup_for_company] = @group.setup_for_company if !@group.setup_for_company.nil? && current_user.setup_for_company.nil?
     Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute if user_params.present?
   end

app/views/groups/_personalize.html.haml

@@ -14,12 +14,12 @@
   .form-group.col-sm-4
     = f.label :setup_for_company, _('Who will be using this group?')
     .gl-display-flex.gl-flex-direction-column.gl-lg-flex-direction-row
-      .gl-flex-grow-1
-        = f.radio_button :setup_for_company, true, class: 'js-setup-for-company', checked: true
-        = f.label :setup_for_company, _('My company or team'), class: 'gl-font-weight-normal', value: 'true'
-      .gl-flex-grow-1
-        = f.radio_button :setup_for_company, false, class: 'js-setup-for-me'
-        = f.label :setup_for_company, _('Just me'), class: 'gl-font-weight-normal', value: 'false'
+      .gl-flex-grow-1.gl-display-flex.gl-align-items-center
+        = f.radio_button :setup_for_company, true, checked: true
+        = f.label :setup_for_company, _('My company or team'), class: 'gl-font-weight-normal gl-mb-0 gl-ml-2', value: 'true'
+      .gl-flex-grow-1.gl-display-flex.gl-align-items-center
+        = f.radio_button :setup_for_company, false
+        = f.label :setup_for_company, _('Just me'), class: 'gl-font-weight-normal gl-mb-0 gl-ml-2', value: 'false'
 
 .row
   .form-group.col-sm-4

spec/controllers/groups_controller_spec.rb

@@ -387,27 +387,29 @@ RSpec.describe GroupsController, factory_default: :keep do
       end
 
       subject do
-        post :create, params: { group: { name: 'new_group', path: 'new_group', setup_for_company: 'true' } }
+        post :create, params: { group: { name: 'new_group', path: 'new_group', setup_for_company: 'false' } }
       end
 
       it 'sets the groups `setup_for_company` value' do
         subject
-        expect(Group.last.setup_for_company).to be(true)
+        expect(Group.last.setup_for_company).to be(false)
       end
 
       context 'when the user already has a value for `setup_for_company`' do
         before do
-          user.update_attribute(:setup_for_company, false)
+          user.update_attribute(:setup_for_company, true)
         end
 
         it 'does not change the users `setup_for_company` value' do
-          expect { subject }.not_to change { user.reload.setup_for_company }.from(false)
+          expect(Users::UpdateService).not_to receive(:new)
+          expect { subject }.not_to change { user.reload.setup_for_company }.from(true)
         end
       end
 
       context 'when the user has no value for `setup_for_company`' do
         it 'changes the users `setup_for_company` value' do
-          expect { subject }.to change { user.reload.setup_for_company }.to(true)
+          expect(Users::UpdateService).to receive(:new).and_call_original
+          expect { subject }.to change { user.reload.setup_for_company }.to(false)
         end
       end
     end