[Issue sorting] Filling positions preferable in the middle

2 files changed, 39 insertions, 24 deletions

diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index ec23c8a08fb..f8ab16a9f4c 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -2,6 +2,7 @@ module RelativePositioning
   extend ActiveSupport::Concern
 
   MIN_POSITION = 0
+  START_POSITION = Gitlab::Database::MAX_INT_VALUE / 2
   MAX_POSITION = Gitlab::Database::MAX_INT_VALUE
   DISTANCE = 500
 
@@ -9,10 +10,6 @@ module RelativePositioning
     after_save :save_positionable_neighbours
   end
 
-  def min_relative_position
-    self.class.in_projects(project.id).minimum(:relative_position)
-  end
-
   def max_relative_position
     self.class.in_projects(project.id).maximum(:relative_position)
   end
@@ -27,7 +24,7 @@ module RelativePositioning
         maximum(:relative_position)
     end
 
-    prev_pos || MIN_POSITION
+    prev_pos
   end
 
   def next_relative_position
@@ -40,7 +37,7 @@ module RelativePositioning
         minimum(:relative_position)
     end
 
-    next_pos || MAX_POSITION
+    next_pos
   end
 
   def move_between(before, after)
@@ -72,7 +69,7 @@ module RelativePositioning
   end
 
   def move_to_end
-    self.relative_position = position_between(max_relative_position, MAX_POSITION)
+    self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
   end
 
   private
@@ -87,10 +84,16 @@ module RelativePositioning
 
     pos_before, pos_after = [pos_before, pos_after].sort
 
-    if pos_after - pos_before > DISTANCE * 2
-      pos_before + DISTANCE
+    if pos_after - pos_before < DISTANCE * 2
+      (pos_after + pos_before) / 2
     else
-      pos_before + (pos_after - pos_before) / 2
+      if pos_before == MIN_POSITION
+        pos_after - DISTANCE
+      elsif pos_after == MAX_POSITION
+        pos_before + DISTANCE
+      else
+        (pos_after + pos_before) / 2
+      end
     end
   end
 
diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb
index 12f44bfe0a5..fbae9efcd98 100644
--- a/spec/models/concerns/relative_positioning_spec.rb
+++ b/spec/models/concerns/relative_positioning_spec.rb
@@ -12,12 +12,6 @@ describe Issue, 'RelativePositioning' do
     end
   end
 
-  describe '#min_relative_position' do
-    it 'returns maximum position' do
-      expect(issue.min_relative_position).to eq issue.relative_position
-    end
-  end
-
   describe '#max_relative_position' do
     it 'returns maximum position' do
       expect(issue.max_relative_position).to eq issue1.relative_position
@@ -29,8 +23,8 @@ describe Issue, 'RelativePositioning' do
       expect(issue1.prev_relative_position).to eq issue.relative_position
     end
 
-    it 'returns minimum position if there is no issue above' do
-      expect(issue.prev_relative_position).to eq RelativePositioning::MIN_POSITION
+    it 'returns nil if there is no issue above' do
+      expect(issue.prev_relative_position).to eq nil
     end
   end
 
@@ -39,8 +33,8 @@ describe Issue, 'RelativePositioning' do
       expect(issue.next_relative_position).to eq issue1.relative_position
     end
 
-    it 'returns next position if there is no issue below' do
-      expect(issue1.next_relative_position).to eq RelativePositioning::MAX_POSITION
+    it 'returns nil if there is no issue below' do
+      expect(issue1.next_relative_position).to eq nil
     end
   end
 
@@ -110,15 +104,33 @@ describe Issue, 'RelativePositioning' do
       expect(issue.relative_position).to be < issue1.relative_position
     end
 
-    it 'positions issue closer to before-issue if distance is big enough' do
-      issue.update relative_position: 100
-      issue1.update relative_position: 6000
+    it 'positions issue in the middle of other two if distance is big enough' do
+      issue.update relative_position: 6000
+      issue1.update relative_position: 10000
 
       new_issue.move_between(issue, issue1)
 
-      expect(new_issue.relative_position).to eq(100 + RelativePositioning::DISTANCE)
+      expect(new_issue.relative_position).to eq(8000)
+    end
+
+    it 'positions issue closer to the middle if we are at the very top' do
+      issue1.update relative_position: 6000
+
+      new_issue.move_between(nil, issue1)
+
+      expect(new_issue.relative_position).to eq(6000 - RelativePositioning::DISTANCE)
+    end
+
+    it 'positions issue closer to the middle if we are at the very bottom' do
+      issue.update relative_position: 6000
+      issue1.update relative_position: nil
+
+      new_issue.move_between(issue, nil)
+
+      expect(new_issue.relative_position).to eq(6000 + RelativePositioning::DISTANCE)
     end
 
+
     it 'positions issue in the middle of other two if distance is not big enough' do
       issue.update relative_position: 100
       issue1.update relative_position: 400