From f64a639bcfa1fc2bc89ca7db268f594306edfd7c Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 16 Mar 2021 18:18:33 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-10-stable-ee --- app/services/boards/base_item_move_service.rb | 72 +++++++++++++++++++++++++ app/services/boards/base_items_list_service.rb | 16 ++++++ app/services/boards/issues/list_service.rb | 16 ------ app/services/boards/issues/move_service.rb | 75 +++----------------------- app/services/boards/list_service.rb | 32 ----------- app/services/boards/lists/list_service.rb | 21 +++++++- app/services/boards/lists/update_service.rb | 4 +- app/services/boards/update_service.rb | 11 ++++ 8 files changed, 128 insertions(+), 119 deletions(-) create mode 100644 app/services/boards/base_item_move_service.rb delete mode 100644 app/services/boards/list_service.rb (limited to 'app/services/boards') diff --git a/app/services/boards/base_item_move_service.rb b/app/services/boards/base_item_move_service.rb new file mode 100644 index 00000000000..bf3e29df54b --- /dev/null +++ b/app/services/boards/base_item_move_service.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Boards + class BaseItemMoveService < Boards::BaseService + def execute(issuable) + issuable_modification_params = issuable_params(issuable) + return false if issuable_modification_params.empty? + + move_single_issuable(issuable, issuable_modification_params) + end + + private + + def issuable_params(issuable) + attrs = {} + + if move_between_lists? + attrs.merge!( + add_label_ids: add_label_ids, + remove_label_ids: remove_label_ids, + state_event: issuable_state + ) + end + + attrs + end + + def move_single_issuable(issuable, issuable_modification_params) + ability_name = :"admin_#{issuable.to_ability_name}" + return unless can?(current_user, ability_name, issuable) + + update(issuable, issuable_modification_params) + end + + def move_between_lists? + moving_from_list.present? && moving_to_list.present? && + moving_from_list != moving_to_list + end + + def moving_from_list + return unless params[:from_list_id].present? + + @moving_from_list ||= board.lists.id_in(params[:from_list_id]).first + end + + def moving_to_list + return unless params[:to_list_id].present? + + @moving_to_list ||= board.lists.id_in(params[:to_list_id]).first + end + + def issuable_state + return 'reopen' if moving_from_list.closed? + return 'close' if moving_to_list.closed? + end + + def add_label_ids + [moving_to_list.label_id].compact + end + + def remove_label_ids + label_ids = + if moving_to_list.movable? + moving_from_list.label_id + else + ::Label.ids_on_board(board.id) + end + + Array(label_ids).compact + end + end +end diff --git a/app/services/boards/base_items_list_service.rb b/app/services/boards/base_items_list_service.rb index 851120ef597..5aebf216460 100644 --- a/app/services/boards/base_items_list_service.rb +++ b/app/services/boards/base_items_list_service.rb @@ -11,8 +11,24 @@ module Boards ordered_items end + # rubocop: disable CodeReuse/ActiveRecord + def metadata + issuables = item_model.arel_table + keys = metadata_fields.keys + # TODO: eliminate need for SQL literal fragment + columns = Arel.sql(metadata_fields.values_at(*keys).join(', ')) + results = item_model.where(id: items.select(issuables[:id])).pluck(columns) + + Hash[keys.zip(results.flatten)] + end + # rubocop: enable CodeReuse/ActiveRecord + private + def metadata_fields + { size: 'COUNT(*)' } + end + def ordered_items raise NotImplementedError end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 27d59e052c7..c6855f29af0 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -9,18 +9,6 @@ module Boards IssuesFinder.valid_params end - # rubocop: disable CodeReuse/ActiveRecord - def metadata - issues = Issue.arel_table - keys = metadata_fields.keys - # TODO: eliminate need for SQL literal fragment - columns = Arel.sql(metadata_fields.values_at(*keys).join(', ')) - results = Issue.where(id: items.select(issues[:id])).pluck(columns) - - Hash[keys.zip(results.flatten)] - end - # rubocop: enable CodeReuse/ActiveRecord - private def ordered_items @@ -35,10 +23,6 @@ module Boards @board ||= parent.boards.find(params[:board_id]) end - def metadata_fields - { size: 'COUNT(*)' } - end - def filter_params set_scope set_non_archived diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 56a7e228b10..99374fa01ae 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -2,13 +2,8 @@ module Boards module Issues - class MoveService < Boards::BaseService - def execute(issue) - issue_modification_params = issue_params(issue) - return false if issue_modification_params.empty? - - move_single_issue(issue, issue_modification_params) - end + class MoveService < Boards::BaseItemMoveService + extend ::Gitlab::Utils::Override def execute_multiple(issues) return execute_multiple_empty_result if issues.empty? @@ -16,7 +11,7 @@ module Boards handled_issues = [] last_inserted_issue_id = nil count = issues.each.inject(0) do |moved_count, issue| - issue_modification_params = issue_params(issue) + issue_modification_params = issuable_params(issue) next moved_count if issue_modification_params.empty? if last_inserted_issue_id @@ -24,7 +19,7 @@ module Boards end last_inserted_issue_id = issue.id - handled_issue = move_single_issue(issue, issue_modification_params) + handled_issue = move_single_issuable(issue, issue_modification_params) handled_issues << present_issue_entity(handled_issue) if handled_issue handled_issue && handled_issue.valid? ? moved_count + 1 : moved_count end @@ -54,51 +49,17 @@ module Boards move_between_ids({ move_after_id: nil, move_before_id: id }) end - def move_single_issue(issue, issue_modification_params) - return unless can?(current_user, :update_issue, issue) - - update(issue, issue_modification_params) - end - def board @board ||= parent.boards.find(params[:board_id]) end - def move_between_lists? - moving_from_list.present? && moving_to_list.present? && - moving_from_list != moving_to_list - end - - # rubocop: disable CodeReuse/ActiveRecord - def moving_from_list - return unless params[:from_list_id].present? - - @moving_from_list ||= board.lists.find_by(id: params[:from_list_id]) - end - # rubocop: enable CodeReuse/ActiveRecord - - # rubocop: disable CodeReuse/ActiveRecord - def moving_to_list - return unless params[:to_list_id].present? - - @moving_to_list ||= board.lists.find_by(id: params[:to_list_id]) - end - # rubocop: enable CodeReuse/ActiveRecord - def update(issue, issue_modification_params) ::Issues::UpdateService.new(issue.project, current_user, issue_modification_params).execute(issue) end - def issue_params(issue) - attrs = {} - - if move_between_lists? - attrs.merge!( - add_label_ids: add_label_ids, - remove_label_ids: remove_label_ids, - state_event: issue_state - ) - end + override :issuable_params + def issuable_params(issuable) + attrs = super move_between_ids = move_between_ids(params) if move_between_ids @@ -109,28 +70,6 @@ module Boards attrs end - def issue_state - return 'reopen' if moving_from_list.closed? - return 'close' if moving_to_list.closed? - end - - def add_label_ids - [moving_to_list.label_id].compact - end - - # rubocop: disable CodeReuse/ActiveRecord - def remove_label_ids - label_ids = - if moving_to_list.movable? - moving_from_list.label_id - else - ::Label.on_board(board.id).pluck(:label_id) - end - - Array(label_ids).compact - end - # rubocop: enable CodeReuse/ActiveRecord - def move_between_ids(move_params) ids = [move_params[:move_after_id], move_params[:move_before_id]] .map(&:to_i) diff --git a/app/services/boards/list_service.rb b/app/services/boards/list_service.rb deleted file mode 100644 index 80ceb91f56d..00000000000 --- a/app/services/boards/list_service.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Boards - class ListService < Boards::BaseService - def execute - find_boards - end - - private - - def boards - parent.boards.order_by_name_asc - end - - def first_board - parent.boards.first_board - end - - def find_boards - found = - if parent.multiple_issue_boards_available? - boards - else - # When multiple issue boards are not available - # a user is only allowed to view the default shown board - first_board - end - - params[:board_id].present? ? [found.find(params[:board_id])] : found - end - end -end diff --git a/app/services/boards/lists/list_service.rb b/app/services/boards/lists/list_service.rb index e4c789c4597..3c296cde51e 100644 --- a/app/services/boards/lists/list_service.rb +++ b/app/services/boards/lists/list_service.rb @@ -9,7 +9,26 @@ module Boards end lists = board.lists.preload_associated_models - params[:list_id].present? ? lists.where(id: params[:list_id]) : lists # rubocop: disable CodeReuse/ActiveRecord + + return lists.id_in(params[:list_id]) if params[:list_id].present? + + list_types = unavailable_list_types_for(board) + lists.without_types(list_types) + end + + private + + def unavailable_list_types_for(board) + hidden_lists_for(board) + end + + def hidden_lists_for(board) + hidden = [] + + hidden << ::List.list_types[:backlog] if board.hide_backlog_list + hidden << ::List.list_types[:closed] if board.hide_closed_list + + hidden end end end diff --git a/app/services/boards/lists/update_service.rb b/app/services/boards/lists/update_service.rb index 4a463372c82..e2d9c371ca2 100644 --- a/app/services/boards/lists/update_service.rb +++ b/app/services/boards/lists/update_service.rb @@ -47,11 +47,11 @@ module Boards end def can_read?(list) - Ability.allowed?(current_user, :read_list, parent) + Ability.allowed?(current_user, :read_issue_board_list, parent) end def can_admin?(list) - Ability.allowed?(current_user, :admin_list, parent) + Ability.allowed?(current_user, :admin_issue_board_list, parent) end end end diff --git a/app/services/boards/update_service.rb b/app/services/boards/update_service.rb index 0340836fd78..48c6e44d55e 100644 --- a/app/services/boards/update_service.rb +++ b/app/services/boards/update_service.rb @@ -2,9 +2,20 @@ module Boards class UpdateService < Boards::BaseService + PERMITTED_PARAMS = %i(name hide_backlog_list hide_closed_list).freeze + def execute(board) + filter_params board.update(params) end + + def filter_params + params.slice!(*permitted_params) + end + + def permitted_params + PERMITTED_PARAMS + end end end -- cgit v1.2.3