summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--.flayignore1
-rw-r--r--app/models/concerns/relative_positioning.rb77
-rw-r--r--spec/models/concerns/relative_positioning_spec.rb66
3 files changed, 119 insertions, 25 deletions
diff --git a/.flayignore b/.flayignore
index fc64b0b5892..47597025115 100644
--- a/.flayignore
+++ b/.flayignore
@@ -2,3 +2,4 @@
lib/gitlab/sanitizers/svg/whitelist.rb
lib/gitlab/diff/position_tracer.rb
app/policies/project_policy.rb
+app/models/concerns/relative_positioning.rb
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index f8ab16a9f4c..f1d8532a6d6 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -4,7 +4,7 @@ module RelativePositioning
MIN_POSITION = 0
START_POSITION = Gitlab::Database::MAX_INT_VALUE / 2
MAX_POSITION = Gitlab::Database::MAX_INT_VALUE
- DISTANCE = 500
+ IDEAL_DISTANCE = 500
included do
after_save :save_positionable_neighbours
@@ -44,55 +44,86 @@ module RelativePositioning
return move_after(before) unless after
return move_before(after) unless before
+ # If there is no place to insert an issue we need to create one by moving the before issue closer
+ # to its predecessor. This process will recursively move all the predecessors until we have a place
+ if (after.relative_position - before.relative_position) < 2
+ before.move_before
+ @positionable_neighbours = [before]
+ end
+
+ self.relative_position = position_between(before.relative_position, after.relative_position)
+ end
+
+ def move_after(before = self)
pos_before = before.relative_position
- pos_after = after.relative_position
+ pos_after = before.next_relative_position
- # We can't insert an issue between two other if the distance is 1 or 0
- # so we need to handle this collision properly
- if pos_after && (pos_after - pos_before).abs <= 1
- self.relative_position = pos_before
- before.move_before(self)
- after.move_after(self)
+ if before.shift_after?
+ issue_to_move = self.class.in_projects(project.id).find_by!(relative_position: pos_after)
+ issue_to_move.move_after
+ @positionable_neighbours = [issue_to_move]
- @positionable_neighbours = [before, after]
- else
- self.relative_position = position_between(pos_before, pos_after)
+ pos_after = issue_to_move.relative_position
end
- end
- def move_before(after)
- self.relative_position = position_between(after.prev_relative_position, after.relative_position)
+ self.relative_position = position_between(pos_before, pos_after)
end
- def move_after(before)
- self.relative_position = position_between(before.relative_position, before.next_relative_position)
+ def move_before(after = self)
+ pos_after = after.relative_position
+ pos_before = after.prev_relative_position
+
+ if after.shift_before?
+ issue_to_move = self.class.in_projects(project.id).find_by!(relative_position: pos_before)
+ issue_to_move.move_before
+ @positionable_neighbours = [issue_to_move]
+
+ pos_before = issue_to_move.relative_position
+ end
+
+ self.relative_position = position_between(pos_before, pos_after)
end
def move_to_end
self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
end
+ # Indicates if there is an issue that should be shifted to free the place
+ def shift_after?
+ next_pos = next_relative_position
+ next_pos && (next_pos - relative_position) == 1
+ end
+
+ # Indicates if there is an issue that should be shifted to free the place
+ def shift_before?
+ prev_pos = prev_relative_position
+ prev_pos && (relative_position - prev_pos) == 1
+ end
+
private
# This method takes two integer values (positions) and
# calculates the position between them. The range is huge as
- # the maximum integer value is 2147483647. We are incrementing position by 1000 every time
- # when we have enough space. If distance is less then 500 we are calculating an average number
+ # the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time
+ # when we have enough space. If distance is less then IDEAL_DISTANCE we are calculating an average number
def position_between(pos_before, pos_after)
pos_before ||= MIN_POSITION
pos_after ||= MAX_POSITION
pos_before, pos_after = [pos_before, pos_after].sort
- if pos_after - pos_before < DISTANCE * 2
- (pos_after + pos_before) / 2
+ halfway = (pos_after + pos_before) / 2
+ distance_to_halfway = pos_after - halfway
+
+ if distance_to_halfway < IDEAL_DISTANCE
+ halfway
else
if pos_before == MIN_POSITION
- pos_after - DISTANCE
+ pos_after - IDEAL_DISTANCE
elsif pos_after == MAX_POSITION
- pos_before + DISTANCE
+ pos_before + IDEAL_DISTANCE
else
- (pos_after + pos_before) / 2
+ halfway
end
end
end
diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb
index ebbf14fb5ba..255b584a85e 100644
--- a/spec/models/concerns/relative_positioning_spec.rb
+++ b/spec/models/concerns/relative_positioning_spec.rb
@@ -66,6 +66,34 @@ describe Issue, 'RelativePositioning' do
end
end
+ describe '#shift_after?' do
+ it 'returns true' do
+ issue.update(relative_position: issue1.relative_position - 1)
+
+ expect(issue.shift_after?).to be_truthy
+ end
+
+ it 'returns false' do
+ issue.update(relative_position: issue1.relative_position - 2)
+
+ expect(issue.shift_after?).to be_falsey
+ end
+ end
+
+ describe '#shift_before?' do
+ it 'returns true' do
+ issue.update(relative_position: issue1.relative_position + 1)
+
+ expect(issue.shift_before?).to be_truthy
+ end
+
+ it 'returns false' do
+ issue.update(relative_position: issue1.relative_position + 2)
+
+ expect(issue.shift_before?).to be_falsey
+ end
+ end
+
describe '#move_between' do
it 'positions issue between two other' do
new_issue.move_between(issue, issue1)
@@ -118,7 +146,7 @@ describe Issue, 'RelativePositioning' do
new_issue.move_between(nil, issue1)
- expect(new_issue.relative_position).to eq(6000 - RelativePositioning::DISTANCE)
+ expect(new_issue.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE)
end
it 'positions issue closer to the middle if we are at the very bottom' do
@@ -127,7 +155,7 @@ describe Issue, 'RelativePositioning' do
new_issue.move_between(issue, nil)
- expect(new_issue.relative_position).to eq(6000 + RelativePositioning::DISTANCE)
+ expect(new_issue.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE)
end
it 'positions issue in the middle of other two if distance is not big enough' do
@@ -138,5 +166,39 @@ describe Issue, 'RelativePositioning' do
expect(new_issue.relative_position).to eq(250)
end
+
+ it 'positions issue in the middle of other two is there is no place' do
+ issue.update relative_position: 100
+ issue1.update relative_position: 101
+
+ new_issue.move_between(issue, issue1)
+
+ expect(new_issue.relative_position).to be_between(issue.relative_position, issue1.relative_position)
+ end
+
+ it 'uses rebalancing if there is no place' do
+ issue.update relative_position: 100
+ issue1.update relative_position: 101
+ issue2 = create(:issue, relative_position: 102, project: project)
+ new_issue.update relative_position: 103
+
+ new_issue.move_between(issue1, issue2)
+ new_issue.save!
+
+ expect(new_issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+ expect(issue.reload.relative_position).not_to eq(100)
+ end
+
+ it 'positions issue right if we pass none-sequential parameters' do
+ issue.update relative_position: 99
+ issue1.update relative_position: 101
+ issue2 = create(:issue, relative_position: 102, project: project)
+ new_issue.update relative_position: 103
+
+ new_issue.move_between(issue, issue2)
+ new_issue.save!
+
+ expect(new_issue.relative_position).to be(100)
+ end
end
end
View Lorry Controller Status