Changes to support the needs of epic trees

Hetrogenous sets of items need to override a couple of methods

app/models/concerns/relative_positioning.rb

@@ -38,6 +38,8 @@ module RelativePositioning
     end
   end
 
+  # This class is API private - it should not be explicitly instantiated
+  # outside of tests
   class ItemContext
     include Gitlab::Utils::StrongMemoize
 
@@ -72,7 +74,7 @@ module RelativePositioning
     end
 
     def relative_siblings(relation = scoped_items)
-      relation.id_not_in(object.id)
+      object.relative_siblings(relation)
     end
 
     # Handles the possibility that the position is already occupied by a sibling
@@ -237,9 +239,7 @@ module RelativePositioning
     def move_sequence(start_pos, end_pos, delta, include_self = false)
       relation = include_self ? scoped_items : relative_siblings
 
-      relation
-        .where('relative_position BETWEEN ? AND ?', start_pos, end_pos)
-        .update_all("relative_position = relative_position + #{delta}")
+      object.update_relative_siblings(relation, (start_pos..end_pos), delta)
     end
 
     def find_next_gap_before
@@ -559,38 +559,47 @@ module RelativePositioning
     end
   end
 
+  def self.mover
+    Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
+  end
+
   def move_between(before, after)
-    mover = Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
     before, after = [before, after].sort_by(&:relative_position) if before && after
 
-    mover.move(self, before, after)
+    RelativePositioning.mover.move(self, before, after)
   end
 
   def move_after(before = self)
-    mover = Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
-
-    mover.move(self, before, nil)
+    RelativePositioning.mover.move(self, before, nil)
   end
 
   def move_before(after = self)
-    mover = Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
-
-    mover.move(self, nil, after)
+    RelativePositioning.mover.move(self, nil, after)
   end
 
   def move_to_end
-    mover = Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
-
-    mover.move_to_end(self)
+    RelativePositioning.mover.move_to_end(self)
   rescue NoSpaceLeft
     self.relative_position = MAX_POSITION
   end
 
   def move_to_start
-    mover = Mover.new(START_POSITION, (MIN_POSITION..MAX_POSITION))
-
-    mover.move_to_start(self)
+    RelativePositioning.mover.move_to_start(self)
   rescue NoSpaceLeft
     self.relative_position = MIN_POSITION
   end
+
+  # This method is used during rebalancing - override it to customise the update
+  # logic:
+  def update_relative_siblings(relation, range, delta)
+    relation
+      .where(relative_position: range)
+      .update_all("relative_position = relative_position + #{delta}")
+  end
+
+  # This method is used to exclude the current self from a relation. Customize
+  # this if `id <> :id` is not sufficient
+  def relative_siblings(relation)
+    relation.id_not_in(id)
+  end
 end

ee/app/models/concerns/epic_tree_sorting.rb

@@ -21,15 +21,11 @@ module EpicTreeSorting
   included do
     extend ::Gitlab::Utils::Override
 
-    override :move_sequence
-    def move_sequence(start_pos, end_pos, delta, include_self = false)
-      items_to_update = scoped_items
+    override :update_relative_siblings
+    def update_relative_siblings(relation, range, delta)
+      items_to_update = relation
         .select(:id, :object_type)
-        .where('relative_position BETWEEN ? AND ?', start_pos, end_pos)
-
-      unless include_self
-        items_to_update = relative_siblings(items_to_update)
-      end
+        .where(relative_position: range)
 
       items_to_update.group_by { |item| item.object_type }.each do |type, group_items|
         ids = group_items.map(&:id)
@@ -38,10 +34,8 @@ module EpicTreeSorting
       end
     end
 
-    private
-
     override :relative_siblings
-    def relative_siblings(relation = scoped_items)
+    def relative_siblings(relation)
       relation.where.not('object_type = ? AND id = ?', self.class.table_name.singularize, self.id)
     end
   end

ee/spec/models/concerns/epic_tree_sorting_spec.rb

@@ -106,9 +106,16 @@ RSpec.describe EpicTreeSorting do
     let!(:epic2) { create(:epic, parent: base_epic, group: group, relative_position: 1003) }
     let!(:epic3) { create(:epic, parent: base_epic, group: group, relative_position: 1005) }
 
+    def move_sequence(range)
+      dx = 500
+      RelativePositioning.mover.context(item).move_sequence(range.first, range.last, dx)
+    end
+
     context 'when self is an epic' do
+      let(:item) { epic1 }
+
       it 'moves all objects correctly' do
-        epic1.move_sequence(1003, 1005, 500)
+        move_sequence(1003..1005)
 
         expect(epic_issue1.reload.relative_position).to eq(1000)
         expect(epic_issue2.reload.relative_position).to eq(1001)
@@ -121,8 +128,10 @@ RSpec.describe EpicTreeSorting do
     end
 
     context 'when self is an epic_issue' do
+      let(:item) { epic_issue1 }
+
       it 'moves all objects correctly' do
-        epic_issue1.move_sequence(1001, 1005, 500)
+        move_sequence(1001..1005)
 
         expect(epic_issue1.reload.relative_position).to eq(1000)
         expect(epic_issue2.reload.relative_position).to eq(1501)