From 8992013689e358328d9ae74623b9b29d80f7b17b Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 1 Aug 2019 01:57:04 +0800 Subject: Fix bug when moving batches of items to the end Starts from START_POSITION when there are no existing positions. Also improves the test to actually test the behavior --- app/controllers/boards/issues_controller.rb | 2 +- app/models/concerns/relative_positioning.rb | 4 ++-- .../relative_positioning_shared_examples.rb | 17 ++++++++++------- 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 -- cgit v1.2.1