Refactor/group links controller

app/controllers/projects/group_links_controller.rb

@@ -12,12 +12,7 @@ class Projects::GroupLinksController < Projects::ApplicationController
 
     if group
       return render_404 unless can?(current_user, :read_group, group)
-
-      project.project_group_links.create(
-        group: group,
-        group_access: params[:link_group_access],
-        expires_at: params[:expires_at]
-      )
+      Projects::GroupLinks::CreateService.new(project, current_user, group_link_create_params).execute(group)
     else
       flash[:alert] = 'Please select a group.'
     end
@@ -32,7 +27,9 @@ class Projects::GroupLinksController < Projects::ApplicationController
   end
 
   def destroy
-    project.project_group_links.find(params[:id]).destroy
+    group_link = project.project_group_links.find(params[:id])
+
+    ::Projects::GroupLinks::DestroyService.new(project, current_user).execute(group_link)
 
     respond_to do |format|
       format.html do
@@ -47,4 +44,8 @@ class Projects::GroupLinksController < Projects::ApplicationController
   def group_link_params
     params.require(:group_link).permit(:group_access, :expires_at)
   end
+
+  def group_link_create_params
+    params.permit(:link_group_access, :expires_at)
+  end
 end

app/services/projects/group_links/create_service.rb

+module Projects
+  module GroupLinks
+    class CreateService < BaseService
+      def execute(group)
+        return false unless group
+
+        project.project_group_links.create(
+          group: group,
+          group_access: params[:link_group_access],
+          expires_at: params[:expires_at]
+        )
+      end
+    end
+  end
+end

app/services/projects/group_links/destroy_service.rb

+module Projects
+  module GroupLinks
+    class DestroyService < BaseService
+      def execute(group_link)
+        return false unless group_link
+        group_link.destroy
+      end
+    end
+  end
+end

changelogs/unreleased/refactor-group_links_controller.yml

+---
+title: Refactor GroupLinksController
+merge_request:
+author: 15121
+type: other

spec/services/projects/group_links/create_service_spec.rb

+require 'spec_helper'
+
+describe Projects::GroupLinks::CreateService, '#execute' do
+  let(:user) { create :user }
+  let(:group) { create :group }
+  let(:project) { create :project }
+  let(:opts) do
+    {
+      link_group_access: '30',
+      expires_at: nil
+    }
+  end
+  let(:subject) { described_class.new(project, user, opts) }
+
+  it 'adds group to project' do
+    expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1)
+  end
+
+  it 'returns false if group is blank' do
+    expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
+  end
+end

spec/services/projects/group_links/destroy_service_spec.rb

+require 'spec_helper'
+
+describe Projects::GroupLinks::DestroyService, '#execute' do
+  let(:group_link) { create :project_group_link }
+  let(:project) { group_link.project }
+  let(:user) { create :user }
+  let(:subject) { described_class.new(project, user) }
+
+  it 'removes group from project' do
+    expect { subject.execute(group_link) }.to change { project.project_group_links.count }.from(1).to(0)
+  end
+
+  it 'returns false if group_link is blank' do
+    expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
+  end
+end