summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/concerns/relative_positioning.rb20
-rw-r--r--spec/models/concerns/relative_positioning_spec.rb27
2 files changed, 39 insertions, 8 deletions
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index 603f2dd7e5d..ec23c8a08fb 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -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
diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb
index 69906382545..12f44bfe0a5 100644
--- a/spec/models/concerns/relative_positioning_spec.rb
+++ b/spec/models/concerns/relative_positioning_spec.rb
@@ -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