Skip to content
Snippets Groups Projects
Commit 39c75305 authored by Chantal Rollison's avatar Chantal Rollison
Browse files

Add preload in issues controller

parent a88004c8
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -18,10 +18,15 @@ module Boards
list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params)
issues = list_service.execute
issues = issues.page(params[:page]).per(params[:per] || 20).without_count
make_sure_position_is_set(issues) if Gitlab::Database.read_write?
issues = issues.preload(:project,
:milestone,
Issue.move_to_end(issues) if Gitlab::Database.read_write?
issues = issues.preload(:milestone,
:assignees,
project: [
:route,
{
namespace: [:route]
}
],
labels: [:priorities],
notes: [:award_emoji, :author]
)
Loading
Loading
@@ -60,12 +65,6 @@ module Boards
render json: data
end
 
def make_sure_position_is_set(issues)
issues.each do |issue|
issue.move_to_end && issue.save unless issue.relative_position
end
end
def issue
@issue ||= issues_finder.find(params[:id])
end
Loading
Loading
Loading
Loading
@@ -12,6 +12,49 @@ module RelativePositioning
after_save :save_positionable_neighbours
end
 
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)
self.transaction do
objects.each do |object|
relative_position = position_between(max_relative_position, MAX_POSITION)
object.relative_position = relative_position
max_relative_position = relative_position
object.save
end
end
end
# This method takes two integer values (positions) and
# calculates the position between them. The range is huge as
# the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time
# when we have enough space. If distance is less then IDEAL_DISTANCE 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
halfway = (pos_after + pos_before) / 2
distance_to_halfway = pos_after - halfway
if distance_to_halfway < IDEAL_DISTANCE
halfway
else
if pos_before == MIN_POSITION
pos_after - IDEAL_DISTANCE
elsif pos_after == MAX_POSITION
pos_before + IDEAL_DISTANCE
else
halfway
end
end
end
end
def min_relative_position
self.class.in_parents(parent_ids).minimum(:relative_position)
end
Loading
Loading
@@ -57,7 +100,7 @@ module RelativePositioning
@positionable_neighbours = [before] # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
 
self.relative_position = position_between(before.relative_position, after.relative_position)
self.relative_position = self.class.position_between(before.relative_position, after.relative_position)
end
 
def move_after(before = self)
Loading
Loading
@@ -72,7 +115,7 @@ module RelativePositioning
pos_after = issue_to_move.relative_position
end
 
self.relative_position = position_between(pos_before, pos_after)
self.relative_position = self.class.position_between(pos_before, pos_after)
end
 
def move_before(after = self)
Loading
Loading
@@ -87,15 +130,15 @@ module RelativePositioning
pos_before = issue_to_move.relative_position
end
 
self.relative_position = position_between(pos_before, pos_after)
self.relative_position = self.class.position_between(pos_before, pos_after)
end
 
def move_to_end
self.relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION)
self.relative_position = self.class.position_between(max_relative_position || START_POSITION, MAX_POSITION)
end
 
def move_to_start
self.relative_position = position_between(min_relative_position || START_POSITION, MIN_POSITION)
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
Loading
Loading
@@ -112,32 +155,6 @@ module RelativePositioning
 
private
 
# This method takes two integer values (positions) and
# calculates the position between them. The range is huge as
# the maximum integer value is 2147483647. We are incrementing position by IDEAL_DISTANCE * 2 every time
# when we have enough space. If distance is less then IDEAL_DISTANCE 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
halfway = (pos_after + pos_before) / 2
distance_to_halfway = pos_after - halfway
if distance_to_halfway < IDEAL_DISTANCE
halfway
else
if pos_before == MIN_POSITION
pos_after - IDEAL_DISTANCE
elsif pos_after == MAX_POSITION
pos_before + IDEAL_DISTANCE
else
halfway
end
end
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def save_positionable_neighbours
return unless @positionable_neighbours
Loading
Loading
Loading
Loading
@@ -15,6 +15,7 @@ class List < ActiveRecord::Base
 
scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
scope :preload_associations, -> { preload(:board, :label) }
 
class << self
def destroyable_types
Loading
Loading
Loading
Loading
@@ -6,7 +6,7 @@ module Boards
def execute(board)
board.lists.create(list_type: :backlog) unless board.lists.backlog.exists?
 
board.lists
board.lists.preload_associations
end
end
end
Loading
Loading
---
title: Add preload for routes and namespaces for issues controller.
merge_request: 21651
author:
type: performance
Loading
Loading
@@ -30,6 +30,15 @@ describe Boards::IssuesController do
 
context 'when list id is present' do
context 'with valid list id' do
let(:group) { create(:group, :private, projects: [project]) }
let(:group_board) { create(:board, group: group) }
let!(:list3) { create(:list, board: group_board, label: development, position: 2) }
let(:sub_group_1) { create(:group, :private, parent: group) }
before do
group.add_maintainer(user)
end
it 'returns issues that have the list label applied' do
issue = create(:labeled_issue, project: project, labels: [planning])
create(:labeled_issue, project: project, labels: [planning])
Loading
Loading
@@ -56,6 +65,39 @@ describe Boards::IssuesController do
 
expect { list_issues(user: user, board: board, list: list2) }.not_to exceed_query_limit(control_count)
end
it 'avoids N+1 database queries when adding a project', :request_store do
create(:labeled_issue, project: project, labels: [development])
control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: group_board, list: list3) }.count
2.times do
p = create(:project, group: group)
create(:labeled_issue, project: p, labels: [development])
end
project_2 = create(:project, group: group)
create(:labeled_issue, project: project_2, labels: [development], assignees: [johndoe])
# because each issue without relative_position must be updated with
# a different value, we have 8 extra queries per issue
expect { list_issues(user: user, board: group_board, list: list3) }.not_to exceed_query_limit(control_count + (2 * 8 - 1))
end
it 'avoids N+1 database queries when adding a subgroup, project, and issue', :nested_groups do
create(:project, group: sub_group_1)
create(:labeled_issue, project: project, labels: [development])
control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: group_board, list: list3) }.count
project_2 = create(:project, group: group)
2.times do
p = create(:project, group: sub_group_1)
create(:labeled_issue, project: p, labels: [development])
end
create(:labeled_issue, project: project_2, labels: [development], assignees: [johndoe])
expect { list_issues(user: user, board: group_board, list: list3) }.not_to exceed_query_limit(control_count + (2 * 8 - 1))
end
end
 
context 'with invalid list id' do
Loading
Loading
@@ -102,12 +144,15 @@ describe Boards::IssuesController do
sign_in(user)
 
params = {
namespace_id: project.namespace.to_param,
project_id: project,
board_id: board.to_param,
list_id: list.try(:to_param)
}
 
unless board.try(:parent)&.is_a?(Group)
params[:namespace_id] = project.namespace.to_param
params[:project_id] = project
end
get :index, params.compact
end
end
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment