Skip to content

G
gitlab-ce
Commit f36da457 authored by Adam Hegyi's avatar Adam Hegyi
Browse files
Options

Make RelativePositioning reusable 

RelativePositioning module was heavily dependent on the Issue model.
This changes makes it easier to reuse the functionality provided by
RelativePositioning in other models.

Needed by: https://gitlab.com/gitlab-org/gitlab-ee/issues/12196
parent 32aa7bd6
Hide whitespace changes
Inline Side-by-side
Showing with 300 additions and 261 deletions
app/models/concerns/relative_positioning.rb
View file @ f36da457
# frozen_string_literal: true
# This module makes it possible to handle items as a list, where the order of items can be easily altered
# Requirements:
#
# - Only works for ActiveRecord models
# - relative_position integer field must present on the model
# - This module uses GROUP BY: the model should have a parent relation, example: project -> issues, project is the parent relation (issues table has a parent_id column)
#
# Setup like this in the body of your class:
#
# include RelativePositioning
#
# # base query used for the position calculation
# def self.relative_positioning_query_base(issue)
# where(deleted: false)
# end
#
# # column that should be used in GROUP BY
# def self.relative_positioning_parent_column
# :project_id
# end
#
module RelativePositioning
extend ActiveSupport::Concern
......@@ -93,7 +114,7 @@ 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
# If there is no place to insert an item we need to create one by moving the before item 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
......@@ -108,11 +129,11 @@ module RelativePositioning
pos_after = before.next_relative_position
if before.shift_after?
issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_after)
issue_to_move.move_after
@positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
item_to_move = self.class.relative_positioning_query_base(self).find_by!(relative_position: pos_after)
item_to_move.move_after
@positionable_neighbours = [item_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
pos_after = issue_to_move.relative_position
pos_after = item_to_move.relative_position
end
self.relative_position = self.class.position_between(pos_before, pos_after)
......@@ -123,11 +144,11 @@ module RelativePositioning
pos_before = after.prev_relative_position
if after.shift_before?
issue_to_move = self.class.in_parents(parent_ids).find_by!(relative_position: pos_before)
issue_to_move.move_before
@positionable_neighbours = [issue_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
item_to_move = self.class.relative_positioning_query_base(self).find_by!(relative_position: pos_before)
item_to_move.move_before
@positionable_neighbours = [item_to_move] # rubocop:disable Gitlab/ModuleWithInstanceVariables
pos_before = issue_to_move.relative_position
pos_before = item_to_move.relative_position
end
self.relative_position = self.class.position_between(pos_before, pos_after)
......@@ -141,13 +162,13 @@ module RelativePositioning
self.relative_position = self.class.position_between(min_relative_position || START_POSITION, MIN_POSITION)
end
# Indicates if there is an issue that should be shifted to free the place
# Indicates if there is an item 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
# Indicates if there is an item that should be shifted to free the place
def shift_before?
prev_pos = prev_relative_position
prev_pos && (relative_position - prev_pos) == 1
......@@ -159,7 +180,7 @@ module RelativePositioning
def save_positionable_neighbours
return unless @positionable_neighbours
status = @positionable_neighbours.all? { |issue| issue.save(touch: false) }
status = @positionable_neighbours.all? { |item| item.save(touch: false) }
@positionable_neighbours = nil
status
......@@ -170,16 +191,15 @@ module RelativePositioning
# When calculating across projects, this is much more efficient than
# MAX(relative_position) without the GROUP BY, due to index usage:
# https://gitlab.com/gitlab-org/gitlab-ce/issues/54276#note_119340977
relation = self.class
.in_parents(parent_ids)
relation = self.class.relative_positioning_query_base(self)
.order(Gitlab::Database.nulls_last_order('position', 'DESC'))
.group(self.class.relative_positioning_parent_column)
.limit(1)
.group(self.class.parent_column)
relation = yield relation if block_given?
relation
.pluck(self.class.parent_column, Arel.sql("#{calculation}(relative_position) AS position"))
.pluck(self.class.relative_positioning_parent_column, Arel.sql("#{calculation}(relative_position) AS position"))
.first&.
last
end
......
app/models/issue.rb
View file @ f36da457
......@@ -91,11 +91,11 @@ class Issue < ApplicationRecord
end
end
class << self
alias_method :in_parents, :in_projects
def self.relative_positioning_query_base(issue)
in_projects(issue.parent_ids)
end
def self.parent_column
def self.relative_positioning_parent_column
:project_id
end
......
spec/models/concerns/relative_positioning_spec.rb deleted 100644 → 0
View file @ 32aa7bd6
# frozen_string_literal: true
require 'spec_helper'
describe RelativePositioning do
let(:project) { create(:project) }
let(:issue) { create(:issue, project: project) }
let(:issue1) { create(:issue, project: project) }
let(:new_issue) { create(:issue, project: project) }
describe '.move_to_end' do
it 'moves the object to the end' do
Issue.move_to_end([issue, issue1])
expect(issue1.prev_relative_position).to eq issue.relative_position
expect(issue.prev_relative_position).to eq nil
expect(issue1.next_relative_position).to eq nil
end
it 'does not perform any moves if all issues have their relative_position set' do
issue.update!(relative_position: 1)
expect(issue).not_to receive(:save)
Issue.move_to_end([issue])
end
end
describe '#max_relative_position' do
it 'returns maximum position' do
expect(issue.max_relative_position).to eq issue1.relative_position
end
end
describe '#prev_relative_position' do
it 'returns previous position if there is an issue above' do
expect(issue1.prev_relative_position).to eq issue.relative_position
end
it 'returns nil if there is no issue above' do
expect(issue.prev_relative_position).to eq nil
end
end
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 nil if there is no issue below' do
expect(issue1.next_relative_position).to eq nil
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
describe '#move_to_end' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'moves issue to the end' do
new_issue.move_to_end
expect(new_issue.relative_position).to be > issue1.relative_position
end
end
describe '#shift_after?' do
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
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
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
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
before do
[issue, issue1].each do |issue|
issue.move_to_end && issue.save
end
end
it 'positions issue between two other' do
new_issue.move_between(issue, issue1)
expect(new_issue.relative_position).to be > issue.relative_position
expect(new_issue.relative_position).to be < issue1.relative_position
end
it 'positions issue between on top' do
new_issue.move_between(nil, issue)
expect(new_issue.relative_position).to be < issue.relative_position
end
it 'positions issue between to end' do
new_issue.move_between(issue1, nil)
expect(new_issue.relative_position).to be > issue1.relative_position
end
it 'positions issues even when after and before positions are the same' do
issue1.update relative_position: issue.relative_position
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 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 in the middle of other two if distance is big enough' do
issue.update relative_position: 6000
issue1.update relative_position: 10000
new_issue.move_between(issue, issue1)
expect(new_issue.relative_position).to eq(8000)
end
it 'positions issue closer to the middle if we are at the very top' do
issue1.update relative_position: 6000
new_issue.move_between(nil, issue1)
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
issue.update relative_position: 6000
issue1.update relative_position: nil
new_issue.move_between(issue, nil)
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
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
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
spec/models/issue_spec.rb
View file @ f36da457
......@@ -871,4 +871,12 @@ describe Issue do
expect(issue.labels_hook_attrs).to eq([label.hook_attrs])
end
end
context "relative positioning" do
it_behaves_like "a class that supports relative positioning" do
let(:project) { create(:project) }
let(:factory) { :issue }
let(:default_params) { { project: project } }
end
end
end
spec/support/shared_examples/relative_positioning_shared_examples.rb 0 → 100644
View file @ f36da457
# frozen_string_literal: true
RSpec.shared_examples "a class that supports relative positioning" do
let(:item1) { create(factory, default_params) }
let(:item2) { create(factory, default_params) }
let(:new_item) { create(factory, default_params) }
def create_item(params)
create(factory, params.merge(default_params))
end
describe '.move_to_end' do
it 'moves the object to the end' do
item1.update(relative_position: 5)
item2.update(relative_position: 15)
described_class.move_to_end([item1, item2])
expect(item2.prev_relative_position).to eq item1.relative_position
expect(item1.prev_relative_position).to eq nil
expect(item2.next_relative_position).to eq nil
end
it 'does not perform any moves if all items have their relative_position set' do
item1.update!(relative_position: 1)
expect(item1).not_to receive(:save)
described_class.move_to_end([item1])
end
end
describe '#max_relative_position' do
it 'returns maximum position' do
expect(item1.max_relative_position).to eq item2.relative_position
end
end
describe '#prev_relative_position' do
it 'returns previous position if there is an item above' do
item1.update(relative_position: 5)
item2.update(relative_position: 15)
expect(item2.prev_relative_position).to eq item1.relative_position
end
it 'returns nil if there is no item above' do
expect(item1.prev_relative_position).to eq nil
end
end
describe '#next_relative_position' do
it 'returns next position if there is an item below' do
item1.update(relative_position: 5)
item2.update(relative_position: 15)
expect(item1.next_relative_position).to eq item2.relative_position
end
it 'returns nil if there is no item below' do
expect(item2.next_relative_position).to eq nil
end
end
describe '#move_before' do
it 'moves item before' do
[item2, item1].each(&:move_to_end)
item1.move_before(item2)
expect(item1.relative_position).to be < item2.relative_position
end
end
describe '#move_after' do
it 'moves item after' do
[item1, item2].each(&:move_to_end)
item1.move_after(item2)
expect(item1.relative_position).to be > item2.relative_position
end
end
describe '#move_to_end' do
before do
[item1, item2].each do |item1|
item1.move_to_end && item1.save
end
end
it 'moves item to the end' do
new_item.move_to_end
expect(new_item.relative_position).to be > item2.relative_position
end
end
describe '#shift_after?' do
before do
[item1, item2].each do |item1|
item1.move_to_end && item1.save
end
end
it 'returns true' do
item1.update(relative_position: item2.relative_position - 1)
expect(item1.shift_after?).to be_truthy
end
it 'returns false' do
item1.update(relative_position: item2.relative_position - 2)
expect(item1.shift_after?).to be_falsey
end
end
describe '#shift_before?' do
before do
[item1, item2].each do |item1|
item1.move_to_end && item1.save
end
end
it 'returns true' do
item1.update(relative_position: item2.relative_position + 1)
expect(item1.shift_before?).to be_truthy
end
it 'returns false' do
item1.update(relative_position: item2.relative_position + 2)
expect(item1.shift_before?).to be_falsey
end
end
describe '#move_between' do
before do
[item1, item2].each do |item1|
item1.move_to_end && item1.save
end
end
it 'positions item between two other' do
new_item.move_between(item1, item2)
expect(new_item.relative_position).to be > item1.relative_position
expect(new_item.relative_position).to be < item2.relative_position
end
it 'positions item between on top' do
new_item.move_between(nil, item1)
expect(new_item.relative_position).to be < item1.relative_position
end
it 'positions item between to end' do
new_item.move_between(item2, nil)
expect(new_item.relative_position).to be > item2.relative_position
end
it 'positions items even when after and before positions are the same' do
item2.update relative_position: item1.relative_position
new_item.move_between(item1, item2)
expect(new_item.relative_position).to be > item1.relative_position
expect(item1.relative_position).to be < item2.relative_position
end
it 'positions items between other two if distance is 1' do
item2.update relative_position: item1.relative_position + 1
new_item.move_between(item1, item2)
expect(new_item.relative_position).to be > item1.relative_position
expect(item1.relative_position).to be < item2.relative_position
end
it 'positions item in the middle of other two if distance is big enough' do
item1.update relative_position: 6000
item2.update relative_position: 10000
new_item.move_between(item1, item2)
expect(new_item.relative_position).to eq(8000)
end
it 'positions item closer to the middle if we are at the very top' do
item2.update relative_position: 6000
new_item.move_between(nil, item2)
expect(new_item.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE)
end
it 'positions item closer to the middle if we are at the very bottom' do
new_item.update relative_position: 1
item1.update relative_position: 6000
item2.destroy
new_item.move_between(item1, nil)
expect(new_item.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE)
end
it 'positions item in the middle of other two if distance is not big enough' do
item1.update relative_position: 100
item2.update relative_position: 400
new_item.move_between(item1, item2)
expect(new_item.relative_position).to eq(250)
end
it 'positions item in the middle of other two is there is no place' do
item1.update relative_position: 100
item2.update relative_position: 101
new_item.move_between(item1, item2)
expect(new_item.relative_position).to be_between(item1.relative_position, item2.relative_position)
end
it 'uses rebalancing if there is no place' do
item1.update relative_position: 100
item2.update relative_position: 101
item3 = create_item(relative_position: 102)
new_item.update relative_position: 103
new_item.move_between(item2, item3)
new_item.save!
expect(new_item.relative_position).to be_between(item2.relative_position, item3.relative_position)
expect(item1.reload.relative_position).not_to eq(100)
end
it 'positions item right if we pass none-sequential parameters' do
item1.update relative_position: 99
item2.update relative_position: 101
item3 = create_item(relative_position: 102)
new_item.update relative_position: 103
new_item.move_between(item1, item3)
new_item.save!
expect(new_item.relative_position).to be(100)
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7