Commit 96fe1856 authored by Valery Sizov's avatar Valery Sizov

Fix relative position calculation

parent b3eda944
......@@ -3,6 +3,7 @@ module RelativePositioning
MIN_POSITION = 0
MAX_POSITION = Gitlab::Database::MAX_INT_VALUE
DISTANCE = 500
included do
after_save :save_positionable_neighbours
......@@ -49,7 +50,9 @@ module RelativePositioning
pos_before = before.relative_position
pos_after = after.relative_position
if pos_after && (pos_before == pos_after)
# 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)
......@@ -75,19 +78,20 @@ module RelativePositioning
private
# This method takes two integer values (positions) and
# calculates some random position between them. The range is huge as
# the maximum integer value is 2147483647. Ideally, the calculated value would be
# exactly between those terminating values, but this will introduce possibility of a race condition
# so two or more issues can get the same value, we want to avoid that and we also want to avoid
# using a lock here. If we have two issues with distance more than one thousand, we are OK.
# Given the huge range of possible values that integer can fit we shoud never face a problem.
# 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
def position_between(pos_before, pos_after)
pos_before ||= MIN_POSITION
pos_after ||= MAX_POSITION
pos_before, pos_after = [pos_before, pos_after].sort
rand(pos_before.next..pos_after.pred)
if pos_after - pos_before > DISTANCE * 2
pos_before + DISTANCE
else
pos_before + (pos_after - pos_before) / 2
end
end
def save_positionable_neighbours
......
......@@ -100,5 +100,32 @@ describe Issue, 'RelativePositioning' do
expect(new_issue.relative_position).to be > issue.relative_position
expect(issue.relative_position).to be < issue1.relative_position
end
it 'positions issues between other two if distance is 1' do
issue1.update relative_position: issue.relative_position + 1
new_issue.move_between(issue, issue1)
expect(new_issue.relative_position).to be > issue.relative_position
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
new_issue.move_between(issue, issue1)
expect(new_issue.relative_position).to eq(100 + 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
new_issue.move_between(issue, issue1)
expect(new_issue.relative_position).to eq(250)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment