Commit 9895d670 authored by Valery Sizov's avatar Valery Sizov

[Issue Board Sorting] More accurate move through the list

parent 1497db75
...@@ -4,6 +4,10 @@ module RelativePositioning ...@@ -4,6 +4,10 @@ module RelativePositioning
MIN_POSITION = 0 MIN_POSITION = 0
MAX_POSITION = Gitlab::Database::MAX_INT_VALUE MAX_POSITION = Gitlab::Database::MAX_INT_VALUE
included do
after_save :save_positionable_neighbours
end
def min_relative_position def min_relative_position
self.class.in_projects(project.id).minimum(:relative_position) self.class.in_projects(project.id).minimum(:relative_position)
end end
...@@ -12,32 +16,61 @@ module RelativePositioning ...@@ -12,32 +16,61 @@ module RelativePositioning
self.class.in_projects(project.id).maximum(:relative_position) self.class.in_projects(project.id).maximum(:relative_position)
end end
def prev_relative_position
prev_pos = nil
if self.relative_position
prev_pos = self.class.
in_projects(project.id).
where('relative_position < ?', self.relative_position).
maximum(:relative_position)
end
prev_pos || MIN_POSITION
end
def next_relative_position
next_pos = nil
if self.relative_position
next_pos = self.class.
in_projects(project.id).
where('relative_position > ?', self.relative_position).
minimum(:relative_position)
end
next_pos || MAX_POSITION
end
def move_between(before, after) def move_between(before, after)
return move_to_end unless after return move_after(before) unless after
return move_to_top unless before return move_before(after) unless before
pos_before = before.relative_position pos_before = before.relative_position
pos_after = after.relative_position pos_after = after.relative_position
if pos_after && (pos_before == pos_after) if pos_after && (pos_before == pos_after)
self.relative_position = pos_before self.relative_position = pos_before
before.decrement! :relative_position before.move_before(self)
after.increment! :relative_position after.move_after(self)
@positionable_neighbours = [before, after]
else else
self.relative_position = position_between(pos_before, pos_after) self.relative_position = position_between(pos_before, pos_after)
end end
end end
def move_to_top def move_before(after)
self.relative_position = position_between(MIN_POSITION, min_relative_position) self.relative_position = position_between(after.prev_relative_position, after.relative_position)
end end
def move_to_end def move_after(before)
self.relative_position = position_between(max_relative_position, MAX_POSITION) self.relative_position = position_between(before.relative_position, before.next_relative_position)
end end
def move_between!(*args) def move_to_end
move_between(*args) && save! self.relative_position = position_between(max_relative_position, MAX_POSITION)
end end
private private
...@@ -57,4 +90,13 @@ module RelativePositioning ...@@ -57,4 +90,13 @@ module RelativePositioning
rand(pos_before.next..pos_after.pred) rand(pos_before.next..pos_after.pred)
end end
def save_positionable_neighbours
return unless @positionable_neighbours
status = @positionable_neighbours.all?(&:save)
@positionable_neighbours = nil
status
end
end end
...@@ -24,13 +24,44 @@ describe Issue, 'RelativePositioning' do ...@@ -24,13 +24,44 @@ describe Issue, 'RelativePositioning' do
end end
end end
describe '#move_to_top' do describe '#prev_relative_position' do
it 'moves issue to the end' do it 'returns previous position if there is an issue above' do
new_issue = create :issue, project: project expect(issue1.prev_relative_position).to eq issue.relative_position
end
new_issue.move_to_top it 'returns minimum position if there is no issue above' do
expect(issue.prev_relative_position).to eq RelativePositioning::MIN_POSITION
end
end
expect(new_issue.relative_position).to be < issue.relative_position describe '#next_relative_position' do
it 'returns next position if there is an issue below' do
expect(issue.next_relative_position).to eq issue1.relative_position
end
it 'returns next position if there is no issue below' do
expect(issue1.next_relative_position).to eq RelativePositioning::MAX_POSITION
end
end
describe '#move_before' do
it 'moves issue before' do
[issue1, issue].each(&:move_to_end)
issue.move_before(issue1)
expect(issue.relative_position).to be < issue1.relative_position
end
end
describe '#move_after' do
it 'moves issue after' do
[issue, issue1].each(&:move_to_end)
issue.move_after(issue1)
expect(issue.relative_position).to be > issue1.relative_position
end end
end end
......
...@@ -78,10 +78,10 @@ describe Boards::Issues::MoveService, services: true do ...@@ -78,10 +78,10 @@ describe Boards::Issues::MoveService, services: true do
end end
context 'when moving to same list' do context 'when moving to same list' do
let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) }
let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) }
let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) }
let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } }
it 'returns false' do it 'returns false' do
expect(described_class.new(project, user, params).execute(issue)).to eq false expect(described_class.new(project, user, params).execute(issue)).to eq false
...@@ -94,13 +94,13 @@ describe Boards::Issues::MoveService, services: true do ...@@ -94,13 +94,13 @@ describe Boards::Issues::MoveService, services: true do
end end
it 'sorts issues' do it 'sorts issues' do
[issue1, issue2].each(&:move_to_end) [issue, issue1, issue2].each do |issue|
issue.move_to_end && issue.save!
issue.move_between!(issue1, issue2) end
params.merge!(move_after_iid: issue.iid, move_before_iid: issue2.iid) params.merge!(move_after_iid: issue1.iid, move_before_iid: issue2.iid)
described_class.new(project, user, params).execute(issue1) described_class.new(project, user, params).execute(issue)
expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
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