Commit f4364dd3 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Refactor positioning_scope method

Change the code so that this is only used within
handle_move_between_ids.
parent f2b99bfb
...@@ -496,6 +496,8 @@ class IssuableBaseService < ::BaseProjectService ...@@ -496,6 +496,8 @@ class IssuableBaseService < ::BaseProjectService
after_id, before_id = params.delete(:move_between_ids) after_id, before_id = params.delete(:move_between_ids)
positioning_scope = issuable_position.class.relative_positioning_query_base(issuable_position)
issuable_before = issuable_for_positioning(before_id, positioning_scope) issuable_before = issuable_for_positioning(before_id, positioning_scope)
issuable_after = issuable_for_positioning(after_id, positioning_scope) issuable_after = issuable_for_positioning(after_id, positioning_scope)
......
...@@ -2,13 +2,13 @@ ...@@ -2,13 +2,13 @@
module Issues module Issues
class ReorderService < Issues::BaseService class ReorderService < Issues::BaseService
include Gitlab::Utils::StrongMemoize
def execute(issue) def execute(issue)
return false unless can?(current_user, :update_issue, issue) return false unless can?(current_user, :update_issue, issue)
return false unless move_between_ids
attrs = issue_params update(issue, { move_between_ids: move_between_ids })
return false if attrs.empty?
update(issue, attrs)
end end
private private
...@@ -19,22 +19,14 @@ module Issues ...@@ -19,22 +19,14 @@ module Issues
false false
end end
def issue_params
attrs = {}
if move_between_ids
attrs[:move_between_ids] = move_between_ids
end
attrs
end
def move_between_ids def move_between_ids
ids = [params[:move_after_id], params[:move_before_id]] strong_memoize(:move_between_ids) do
.map(&:to_i) ids = [params[:move_after_id], params[:move_before_id]]
.map { |m| m > 0 ? m : nil } .map(&:to_i)
.map { |m| m > 0 ? m : nil }
ids.any? ? ids : nil ids.any? ? ids : nil
end
end end
end end
end end
...@@ -13,8 +13,6 @@ module Issues ...@@ -13,8 +13,6 @@ module Issues
end end
def execute(issue) def execute(issue)
@issue = issue
handle_move_between_ids(issue) handle_move_between_ids(issue)
change_issue_duplicate(issue) change_issue_duplicate(issue)
...@@ -227,10 +225,6 @@ module Issues ...@@ -227,10 +225,6 @@ module Issues
issue if can?(current_user, :update_issue, issue) issue if can?(current_user, :update_issue, issue)
end end
def positioning_scope
Issue.relative_positioning_query_base(@issue)
end
def create_confidentiality_note(issue) def create_confidentiality_note(issue)
SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user) SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user)
end end
......
...@@ -11,6 +11,8 @@ module Epics ...@@ -11,6 +11,8 @@ module Epics
due_date_is_fixed due_date_is_fixed
].freeze ].freeze
attr_reader :epic_board_id
def execute(epic) def execute(epic)
reposition_on_board(epic) reposition_on_board(epic)
...@@ -106,27 +108,25 @@ module Epics ...@@ -106,27 +108,25 @@ module Epics
end end
def reposition_on_board(epic) def reposition_on_board(epic)
@epic_board_id = params.delete(:board_id)
return unless params[:move_between_ids] return unless params[:move_between_ids]
return unless epic_board_id return unless epic_board_id
fill_missing_positions_before fill_missing_positions_before
epic_board_position = issuable_for_positioning(epic.id, positioning_scope, create_missing: true) # we want to create missing only for the epic being moved
# other records are handled by PositionCreateService
epic_board_position = Boards::EpicBoardPosition.find_or_create_by!(epic_board_id: epic_board_id, epic_id: epic.id) # rubocop: disable CodeReuse/ActiveRecord
handle_move_between_ids(epic_board_position) handle_move_between_ids(epic_board_position)
epic_board_position.save! epic_board_position.save!
end end
# we want to create missing only for the epic being moved def issuable_for_positioning(id, positioning_scope)
# other records are handled by PositionCreateService
def issuable_for_positioning(id, positioning_scope, create_missing: false)
return unless id return unless id
position = positioning_scope.find_by_epic_id(id) positioning_scope.find_by_epic_id(id)
return position if position
positioning_scope.create!(epic_id: id) if create_missing
end end
def fill_missing_positions_before def fill_missing_positions_before
...@@ -136,7 +136,7 @@ module Epics ...@@ -136,7 +136,7 @@ module Epics
return unless before_id return unless before_id
# if position for the epic above exists we don't need to create positioning records # if position for the epic above exists we don't need to create positioning records
return if issuable_for_positioning(before_id, positioning_scope) return if Boards::EpicBoardPosition.exists?(epic_board_id: epic_board_id, epic_id: before_id) # rubocop: disable CodeReuse/ActiveRecord
service_params = { service_params = {
board_id: epic_board_id, board_id: epic_board_id,
...@@ -147,16 +147,6 @@ module Epics ...@@ -147,16 +147,6 @@ module Epics
Boards::Epics::PositionCreateService.new(board_group, current_user, service_params).execute Boards::Epics::PositionCreateService.new(board_group, current_user, service_params).execute
end end
def positioning_scope
Boards::EpicBoardPosition.where(epic_board_id: epic_board_id) # rubocop: disable CodeReuse/ActiveRecord
end
def epic_board_id
strong_memoize(:epic_board_id) do
params.delete(:board_id)
end
end
def saved_change_to_epic_dates?(epic) def saved_change_to_epic_dates?(epic)
(epic.saved_changes.keys.map(&:to_sym) & EPIC_DATE_FIELDS).present? (epic.saved_changes.keys.map(&:to_sym) & EPIC_DATE_FIELDS).present?
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment