diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 045bf392ac8..46d2c345758 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -14,10 +14,12 @@ module RelativePositioning class_methods do def move_to_end(objects) - parent_ids = objects.map(&:parent_ids).flatten.uniq - max_relative_position = in_parents(parent_ids).maximum(:relative_position) || START_POSITION objects = objects.reject(&:relative_position) + return if objects.empty? + + max_relative_position = objects.first.max_relative_position + self.transaction do objects.each do |object| relative_position = position_between(max_relative_position, MAX_POSITION) @@ -55,22 +57,21 @@ module RelativePositioning end end - def min_relative_position - self.class.in_parents(parent_ids).minimum(:relative_position) + def min_relative_position(&block) + calculate_relative_position('MIN', &block) end - def max_relative_position - self.class.in_parents(parent_ids).maximum(:relative_position) + def max_relative_position(&block) + calculate_relative_position('MAX', &block) end def prev_relative_position prev_pos = nil if self.relative_position - prev_pos = self.class - .in_parents(parent_ids) - .where('relative_position < ?', self.relative_position) - .maximum(:relative_position) + prev_pos = max_relative_position do |relation| + relation.where('relative_position < ?', self.relative_position) + end end prev_pos @@ -80,10 +81,9 @@ module RelativePositioning next_pos = nil if self.relative_position - next_pos = self.class - .in_parents(parent_ids) - .where('relative_position > ?', self.relative_position) - .minimum(:relative_position) + next_pos = min_relative_position do |relation| + relation.where('relative_position > ?', self.relative_position) + end end next_pos @@ -165,4 +165,22 @@ module RelativePositioning status end # rubocop:enable Gitlab/ModuleWithInstanceVariables + + def calculate_relative_position(calculation) + # 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) + .order(Gitlab::Database.nulls_last_order('position', 'DESC')) + .limit(1) + .group(self.class.parent_column) + + relation = yield relation if block_given? + + relation + .pluck(self.class.parent_column, "#{calculation}(relative_position) AS position") + .first&. + last + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 0de5e434b02..780035c77e2 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -99,6 +99,10 @@ class Issue < ActiveRecord::Base alias_method :in_parents, :in_projects end + def self.parent_column + :project_id + end + def self.reference_prefix '#' end diff --git a/changelogs/unreleased/speed-up-relative-positioning.yml b/changelogs/unreleased/speed-up-relative-positioning.yml new file mode 100644 index 00000000000..3bd865fb5de --- /dev/null +++ b/changelogs/unreleased/speed-up-relative-positioning.yml @@ -0,0 +1,5 @@ +--- +title: Speed up issue board lists in groups with many projects +merge_request: +author: +type: performance diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb index 66c1f47d12b..ac8da30b6c9 100644 --- a/spec/models/concerns/relative_positioning_spec.rb +++ b/spec/models/concerns/relative_positioning_spec.rb @@ -14,6 +14,14 @@ describe RelativePositioning do 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