From aff7a2ef55b317b3e5c8d83f4e9ae9fc64f4fa8f Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 2 Aug 2016 21:49:26 -0300 Subject: [PATCH] Extract base service class for boards related services --- .../projects/board_lists_controller.rb | 6 +++--- app/controllers/projects/boards_controller.rb | 2 +- app/services/boards/base_service.rb | 14 +++++++++++++ app/services/boards/create_service.rb | 8 +------ app/services/boards/issues/list_service.rb | 12 +---------- app/services/boards/issues/move_service.rb | 12 +---------- app/services/boards/lists/create_service.rb | 9 +------- app/services/boards/lists/destroy_service.rb | 9 +------- app/services/boards/lists/move_service.rb | 9 +------- spec/services/boards/create_service_spec.rb | 2 +- .../boards/lists/create_service_spec.rb | 7 ++++--- .../boards/lists/destroy_service_spec.rb | 9 ++++---- .../boards/lists/move_service_spec.rb | 21 ++++++++++--------- 13 files changed, 45 insertions(+), 75 deletions(-) create mode 100644 app/services/boards/base_service.rb diff --git a/app/controllers/projects/board_lists_controller.rb b/app/controllers/projects/board_lists_controller.rb index 0969fef6999..738c06c57ef 100644 --- a/app/controllers/projects/board_lists_controller.rb +++ b/app/controllers/projects/board_lists_controller.rb @@ -4,7 +4,7 @@ class Projects::BoardListsController < Projects::ApplicationController rescue_from ActiveRecord::RecordNotFound, with: :record_not_found def create - list = Boards::Lists::CreateService.new(project, list_params).execute + list = Boards::Lists::CreateService.new(project, current_user, list_params).execute if list.valid? render json: list.as_json(only: [:id, :list_type, :position], methods: [:title], include: { label: { only: [:id, :title, :color] } }) @@ -14,7 +14,7 @@ class Projects::BoardListsController < Projects::ApplicationController end def update - service = Boards::Lists::MoveService.new(project, move_params) + service = Boards::Lists::MoveService.new(project, current_user, move_params) if service.execute head :ok @@ -24,7 +24,7 @@ class Projects::BoardListsController < Projects::ApplicationController end def destroy - service = Boards::Lists::DestroyService.new(project, params) + service = Boards::Lists::DestroyService.new(project, current_user, params) if service.execute head :ok diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb index b04a0efbe73..50311acec32 100644 --- a/app/controllers/projects/boards_controller.rb +++ b/app/controllers/projects/boards_controller.rb @@ -1,6 +1,6 @@ class Projects::BoardsController < Projects::ApplicationController def show - board = Boards::CreateService.new(project).execute + board = Boards::CreateService.new(project, current_user).execute respond_to do |format| format.html diff --git a/app/services/boards/base_service.rb b/app/services/boards/base_service.rb new file mode 100644 index 00000000000..e2d2bdfe3d7 --- /dev/null +++ b/app/services/boards/base_service.rb @@ -0,0 +1,14 @@ +module Boards + class BaseService + def initialize(project, user, params = {}) + @project = project + @board = project.board + @user = user + @params = params.dup + end + + private + + attr_reader :project, :board, :user, :params + end +end diff --git a/app/services/boards/create_service.rb b/app/services/boards/create_service.rb index f1eef870925..5881752dabd 100644 --- a/app/services/boards/create_service.rb +++ b/app/services/boards/create_service.rb @@ -1,9 +1,5 @@ module Boards - class CreateService - def initialize(project) - @project = project - end - + class CreateService < BaseService def execute create_board! unless project.board.present? project.board @@ -11,8 +7,6 @@ module Boards private - attr_reader :project - def create_board! project.create_board project.board.lists.create(list_type: :backlog) diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 4948ab3a140..221fea716f7 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -1,12 +1,6 @@ module Boards module Issues - class ListService - def initialize(project, user, params = {}) - @project = project - @user = user - @params = params.dup - end - + class ListService < Boards::BaseService def execute issues = IssuesFinder.new(user, filter_params).execute issues = without_board_labels(issues) if list.backlog? @@ -15,10 +9,6 @@ module Boards private - attr_reader :project, :user, :params - - delegate :board, to: :project - def list @list ||= board.lists.find(params[:list_id]) end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 7daa4d31efd..7d08ef5b6c7 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -1,12 +1,6 @@ module Boards module Issues - class MoveService - def initialize(project, user, params = {}) - @project = project - @user = user - @params = params.dup - end - + class MoveService < Boards::BaseService def execute return false unless issue.present? return false unless user.can?(:update_issue, issue) @@ -20,10 +14,6 @@ module Boards private - attr_reader :project, :user, :params - - delegate :board, to: :project - def issue @issue ||= project.issues.visible_to_user(user).find_by!(iid: params[:id]) end diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index 151bb67bea1..59e8b51e37f 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -1,11 +1,6 @@ module Boards module Lists - class CreateService - def initialize(project, params = {}) - @board = project.board - @params = params.dup - end - + class CreateService < Boards::BaseService def execute List.transaction do position = find_next_position @@ -16,8 +11,6 @@ module Boards private - attr_reader :board, :params - def find_next_position board.lists.label.maximum(:position).to_i + 1 end diff --git a/app/services/boards/lists/destroy_service.rb b/app/services/boards/lists/destroy_service.rb index 5402d009c08..8d244fa3966 100644 --- a/app/services/boards/lists/destroy_service.rb +++ b/app/services/boards/lists/destroy_service.rb @@ -1,11 +1,6 @@ module Boards module Lists - class DestroyService - def initialize(project, params = {}) - @board = project.board - @params = params.dup - end - + class DestroyService < Boards::BaseService def execute return false unless list.label? @@ -17,8 +12,6 @@ module Boards private - attr_reader :board, :params - def list @list ||= board.lists.find(params[:id]) end diff --git a/app/services/boards/lists/move_service.rb b/app/services/boards/lists/move_service.rb index f5735b6bab1..9bd07f43a36 100644 --- a/app/services/boards/lists/move_service.rb +++ b/app/services/boards/lists/move_service.rb @@ -1,11 +1,6 @@ module Boards module Lists - class MoveService - def initialize(project, params = {}) - @board = project.board - @params = params.dup - end - + class MoveService < Boards::BaseService def execute return false unless list.label? return false unless valid_move? @@ -18,8 +13,6 @@ module Boards private - attr_reader :board, :params - def list @list ||= board.lists.find(params[:id]) end diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index e0ce580aa45..a1a4dd4c57c 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Boards::CreateService, services: true do describe '#execute' do - subject(:service) { described_class.new(project) } + subject(:service) { described_class.new(project, double) } context 'when project does not have a board' do let(:project) { create(:empty_project, board: nil) } diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index 25c6d614229..27dd4185adb 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -4,9 +4,10 @@ describe Boards::Lists::CreateService, services: true do describe '#execute' do let(:project) { create(:project_with_board) } let(:board) { project.board } + let(:user) { create(:user) } let(:label) { create(:label, name: 'in-progress') } - subject(:service) { described_class.new(project, label_id: label.id) } + subject(:service) { described_class.new(project, user, label_id: label.id) } context 'when board lists is empty' do it 'creates a new list at beginning of the list' do @@ -31,7 +32,7 @@ describe Boards::Lists::CreateService, services: true do create(:list, board: board, position: 1) create(:list, board: board, position: 2) - list = described_class.new(project, label_id: label.id).execute + list = service.execute expect(list.position).to eq 3 end @@ -43,7 +44,7 @@ describe Boards::Lists::CreateService, services: true do create(:done_list, board: board) list1 = create(:list, board: board, position: 1) - list2 = described_class.new(project, label_id: label.id).execute + list2 = service.execute expect(list1.reload.position).to eq 1 expect(list2.reload.position).to eq 2 diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index 620f9df739f..4e382017c00 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -4,11 +4,12 @@ describe Boards::Lists::DestroyService, services: true do describe '#execute' do let(:project) { create(:project_with_board) } let(:board) { project.board } + let(:user) { create(:user) } context 'when list type is label' do it 'removes list from board' do list = create(:list, board: board) - service = described_class.new(project, id: list.id) + service = described_class.new(project, user, id: list.id) expect { service.execute }.to change(board.lists, :count).by(-1) end @@ -20,7 +21,7 @@ describe Boards::Lists::DestroyService, services: true do staging = create(:list, board: board, position: 3) done = create(:done_list, board: board) - described_class.new(project, id: development.id).execute + described_class.new(project, user, id: development.id).execute expect(backlog.reload.position).to be_nil expect(review.reload.position).to eq 1 @@ -31,14 +32,14 @@ describe Boards::Lists::DestroyService, services: true do it 'does not remove list from board when list type is backlog' do list = create(:backlog_list, board: board) - service = described_class.new(project, id: list.id) + service = described_class.new(project, user, id: list.id) expect { service.execute }.not_to change(board.lists, :count) end it 'does not remove list from board when list type is done' do list = create(:done_list, board: board) - service = described_class.new(project, id: list.id) + service = described_class.new(project, user, id: list.id) expect { service.execute }.not_to change(board.lists, :count) end diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index 54a0c33c5b8..aa93cbad7f4 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -4,6 +4,7 @@ describe Boards::Lists::MoveService, services: true do describe '#execute' do let(:project) { create(:project_with_board) } let(:board) { project.board } + let(:user) { create(:user) } let!(:backlog) { create(:backlog_list, board: board) } let!(:planning) { create(:list, board: board, position: 1) } @@ -14,7 +15,7 @@ describe Boards::Lists::MoveService, services: true do context 'when list type is set to label' do it 'keeps position of lists when new position is nil' do - service = described_class.new(project, { id: planning.id, position: nil }) + service = described_class.new(project, user, id: planning.id, position: nil) service.execute @@ -22,7 +23,7 @@ describe Boards::Lists::MoveService, services: true do end it 'keeps position of lists when new positon is equal to old position' do - service = described_class.new(project, { id: planning.id, position: 1 }) + service = described_class.new(project, user, id: planning.id, position: 1) service.execute @@ -30,7 +31,7 @@ describe Boards::Lists::MoveService, services: true do end it 'keeps position of lists when new positon is negative' do - service = described_class.new(project, { id: planning.id, position: -1 }) + service = described_class.new(project, user, id: planning.id, position: -1) service.execute @@ -38,7 +39,7 @@ describe Boards::Lists::MoveService, services: true do end it 'keeps position of lists when new positon is greater than number of labels lists' do - service = described_class.new(project, { id: planning.id, position: 6 }) + service = described_class.new(project, user, id: planning.id, position: 6) service.execute @@ -46,7 +47,7 @@ describe Boards::Lists::MoveService, services: true do end it 'increments position of intermediate lists when new positon is equal to first position' do - service = described_class.new(project, { id: staging.id, position: 1 }) + service = described_class.new(project, user, id: staging.id, position: 1) service.execute @@ -54,7 +55,7 @@ describe Boards::Lists::MoveService, services: true do end it 'decrements position of intermediate lists when new positon is equal to last position' do - service = described_class.new(project, { id: planning.id, position: 4 }) + service = described_class.new(project, user, id: planning.id, position: 4) service.execute @@ -62,7 +63,7 @@ describe Boards::Lists::MoveService, services: true do end it 'decrements position of intermediate lists when new position is greater than old position' do - service = described_class.new(project, { id: planning.id, position: 3 }) + service = described_class.new(project, user, id: planning.id, position: 3) service.execute @@ -70,7 +71,7 @@ describe Boards::Lists::MoveService, services: true do end it 'increments position of intermediate lists when new position is lower than old position' do - service = described_class.new(project, { id: staging.id, position: 2 }) + service = described_class.new(project, user, id: staging.id, position: 2) service.execute @@ -79,7 +80,7 @@ describe Boards::Lists::MoveService, services: true do end it 'keeps position of lists when list type is backlog' do - service = described_class.new(project, { id: backlog.id, position: 2 }) + service = described_class.new(project, user, id: backlog.id, position: 2) service.execute @@ -87,7 +88,7 @@ describe Boards::Lists::MoveService, services: true do end it 'keeps position of lists when list type is done' do - service = described_class.new(project, { id: done.id, position: 2 }) + service = described_class.new(project, user, id: done.id, position: 2) service.execute