summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeinrich Lee Yu <heinrich@gitlab.com>2019-08-01 01:57:04 +0800
committerHeinrich Lee Yu <heinrich@gitlab.com>2019-08-01 02:04:10 +0800
commit8992013689e358328d9ae74623b9b29d80f7b17b (patch)
tree2242db40a6b13180192ea29544aac3fc85bb6036
parent2e7f4bbb66b4bae61c9dd09234e8435c91e7e986 (diff)
downloadgitlab-ce-rename-relative-position-move-to-end.tar.gz
Fix bug when moving batches of items to the endrename-relative-position-move-to-end
Starts from START_POSITION when there are no existing positions. Also improves the test to actually test the behavior
-rw-r--r--app/controllers/boards/issues_controller.rb2
-rw-r--r--app/models/concerns/relative_positioning.rb4
-rw-r--r--spec/support/shared_examples/relative_positioning_shared_examples.rb17
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