Implement CreateMembers service to make controller thin

app/controllers/projects/project_members_controller.rb

@@ -25,18 +25,15 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def create
-    if params[:user_ids].blank?
-      return redirect_to(namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.')
-    end
+    status = Members::CreateService.new(@project, current_user, params).execute
 
-    @project.team.add_users(
-      params[:user_ids].split(','),
-      params[:access_level],
-      expires_at: params[:expires_at],
-      current_user: current_user
-    )
+    redirect_url = namespace_project_project_members_path(@project.namespace, @project)
 
-    redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.'
+    if status
+      redirect_to redirect_url, notice: 'Users were successfully added.'
+    else
+      redirect_to redirect_url, alert: 'No users or groups specified.'
+    end
   end
 
   def update

app/services/members/create_service.rb

+module Members
+  class CreateService < BaseService
+    def execute
+      return false if params[:user_ids].blank?
+
+      project.team.add_users(
+        params[:user_ids].split(','),
+        params[:access_level],
+        expires_at: params[:expires_at],
+        current_user: current_user
+      )
+
+      true
+    end
+  end
+end

spec/controllers/projects/project_members_controller_spec.rb

@@ -14,49 +14,49 @@ describe Projects::ProjectMembersController do
   end
 
   describe 'POST create' do
-    context 'when users are added' do
-      let(:project_user) { create(:user) }
+    let(:project_user) { create(:user) }
 
-      before { sign_in(user) }
+    before { sign_in(user) }
 
-      context 'when user does not have enough rights' do
-        before { project.team << [user, :developer] }
+    context 'when user does not have enough rights' do
+      before { project.team << [user, :developer] }
 
-        it 'returns 404' do
-          post :create, namespace_id: project.namespace,
-                        project_id: project,
-                        user_ids: project_user.id,
-                        access_level: Gitlab::Access::GUEST
+      it 'returns 404' do
+        post :create, namespace_id: project.namespace,
+                      project_id: project,
+                      user_ids: project_user.id,
+                      access_level: Gitlab::Access::GUEST
 
-          expect(response).to have_http_status(404)
-          expect(project.users).not_to include project_user
-        end
+        expect(response).to have_http_status(404)
+        expect(project.users).not_to include project_user
       end
+    end
 
-      context 'when user has enough rights' do
-        before { project.team << [user, :master] }
+    context 'when user has enough rights' do
+      before { project.team << [user, :master] }
 
-        it 'adds user to members' do
-          post :create, namespace_id: project.namespace,
-                        project_id: project,
-                        user_ids: project_user.id,
-                        access_level: Gitlab::Access::GUEST
-
-          expect(response).to set_flash.to 'Users were successfully added.'
-          expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
-          expect(project.users).to include project_user
-        end
+      it 'adds user to members' do
+        expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(true)
 
-        it 'adds no user to members' do
-          post :create, namespace_id: project.namespace,
-                        project_id: project,
-                        user_ids: '',
-                        access_level: Gitlab::Access::GUEST
+        post :create, namespace_id: project.namespace,
+                      project_id: project,
+                      user_ids: project_user.id,
+                      access_level: Gitlab::Access::GUEST
 
-          expect(response).to set_flash.to 'No users or groups specified.'
-          expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
-          expect(project.users).not_to include project_user
-        end
+        expect(response).to set_flash.to 'Users were successfully added.'
+        expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
+      end
+
+      it 'adds no user to members' do
+        expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(false)
+
+        post :create, namespace_id: project.namespace,
+                      project_id: project,
+                      user_ids: '',
+                      access_level: Gitlab::Access::GUEST
+
+        expect(response).to set_flash.to 'No users or groups specified.'
+        expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project))
       end
     end
   end

spec/services/members/create_service_spec.rb

+require 'spec_helper'
+
+describe Members::CreateService, services: true do
+  let(:project) { create(:empty_project) }
+  let(:user) { create(:user) }
+  let(:project_user) { create(:user) }
+
+  before { project.team << [user, :master] }
+
+  it 'adds user to members' do
+    params = { user_ids: project_user.id.to_s, access_level: Gitlab::Access::GUEST }
+    result = described_class.new(project, user, params).execute
+
+    expect(result).to be_truthy
+    expect(project.users).to include project_user
+  end
+
+  it 'adds no user to members' do
+    params = { user_ids: '', access_level: Gitlab::Access::GUEST }
+    result = described_class.new(project, user, params).execute
+
+    expect(result).to be_falsey
+    expect(project.users).not_to include project_user
+  end
+end