Merge branch 'rename-relative-position-move-to-end' into 'master'

Fix bug when moving batches of items to the end

See merge request gitlab-org/gitlab-ce!31351

3 files changed, 13 insertions, 10 deletions

diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 90528f75ffd..1d1a72d21f1 100644
--- a/app/controllers/boards/issues_controller.rb
+++ b/app/controllers/boards/issues_controller.rb
@@ -26,7 +26,7 @@ module Boards
       list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params)
       issues = list_service.execute
       issues = issues.page(params[:page]).per(params[:per] || 20).without_count
-      Issue.move_to_end(issues) if Gitlab::Database.read_write?
+      Issue.move_nulls_to_end(issues) if Gitlab::Database.read_write?
       issues = issues.preload(:milestone,
                               :assignees,
                               project: [
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index 9cd7b8d6258..4a1441805fc 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -34,7 +34,7 @@ module RelativePositioning
   end
 
   class_methods do
-    def move_to_end(objects)
+    def move_nulls_to_end(objects)
       objects = objects.reject(&:relative_position)
 
       return if objects.empty?
@@ -43,7 +43,7 @@ module RelativePositioning
 
       self.transaction do
         objects.each do |object|
-          relative_position = position_between(max_relative_position, MAX_POSITION)
+          relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
           object.relative_position = relative_position
           max_relative_position = relative_position
           object.save(touch: false)
diff --git a/spec/support/shared_examples/relative_positioning_shared_examples.rb b/spec/support/shared_examples/relative_positioning_shared_examples.rb
index 5ee62644c54..45eacf3721e 100644
--- a/spec/support/shared_examples/relative_positioning_shared_examples.rb
+++ b/spec/support/shared_examples/relative_positioning_shared_examples.rb
@@ -9,24 +9,27 @@ RSpec.shared_examples "a class that supports relative positioning" do
     create(factory, params.merge(default_params))
   end
 
-  describe '.move_to_end' do
-    it 'moves the object to the end' do
-      item1.update(relative_position: 5)
-      item2.update(relative_position: 15)
-
-      described_class.move_to_end([item1, item2])
+  describe '.move_nulls_to_end' do
+    it 'moves items with null relative_position to the end' do
+      described_class.move_nulls_to_end([item1, item2])
 
       expect(item2.prev_relative_position).to eq item1.relative_position
       expect(item1.prev_relative_position).to eq nil
       expect(item2.next_relative_position).to eq nil
     end
 
+    it 'moves the item near the start position when there are no existing positions' do
+      described_class.move_nulls_to_end([item1])
+
+      expect(item1.relative_position).to eq(described_class::START_POSITION + described_class::IDEAL_DISTANCE)
+    end
+
     it 'does not perform any moves if all items have their relative_position set' do
       item1.update!(relative_position: 1)
 
       expect(item1).not_to receive(:save)
 
-      described_class.move_to_end([item1])
+      described_class.move_nulls_to_end([item1])
     end
   end