diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index f59200d3b1f..dbc1c8bcc28 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/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 diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb new file mode 100644 index 00000000000..35624577024 --- /dev/null +++ b/app/services/projects/group_links/create_service.rb @@ -0,0 +1,15 @@ +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 diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb new file mode 100644 index 00000000000..fbf31214c28 --- /dev/null +++ b/app/services/projects/group_links/destroy_service.rb @@ -0,0 +1,10 @@ +module Projects + module GroupLinks + class DestroyService < BaseService + def execute(group_link) + return false unless group_link + group_link.destroy + end + end + end +end diff --git a/changelogs/unreleased/refactor-group_links_controller.yml b/changelogs/unreleased/refactor-group_links_controller.yml new file mode 100644 index 00000000000..af3d22c34cb --- /dev/null +++ b/changelogs/unreleased/refactor-group_links_controller.yml @@ -0,0 +1,5 @@ +--- +title: Refactor GroupLinksController +merge_request: +author: 15121 +type: other diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb new file mode 100644 index 00000000000..ffb270d277e --- /dev/null +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -0,0 +1,22 @@ +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 diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb new file mode 100644 index 00000000000..336ee01ae50 --- /dev/null +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -0,0 +1,16 @@ +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