diff options
| -rw-r--r-- | app/models/concerns/relative_positioning.rb | 20 | ||||
| -rw-r--r-- | spec/models/concerns/relative_positioning_spec.rb | 27 |
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 |