Commit de35c044 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Preload ancestors after pagination when filtering

We need to preload the ancestors of search results after applying
pagination limits. This way the search results itself are paginated,
but not the ancestors.

If we don't do this, we might not preload a parent group of a search
result as it has been cut off by pagination.
parent 34fe3274
module GroupTree module GroupTree
# rubocop:disable Gitlab/ModuleWithInstanceVariables # rubocop:disable Gitlab/ModuleWithInstanceVariables
def render_group_tree(groups) def render_group_tree(groups)
@groups = if params[:filter].present? groups = groups.sort_by_attribute(@sort = params[:sort])
# We find the ancestors by ID of the search results here.
# Otherwise the ancestors would also have filters applied, groups = if params[:filter].present?
# which would cause them not to be preloaded. filtered_groups_with_ancestors(groups)
group_ids = groups.search(params[:filter]).select(:id)
Gitlab::GroupHierarchy.new(Group.where(id: group_ids))
.base_and_ancestors
else else
# Only show root groups if no parent-id is given # If `params[:parent_id]` is `nil`, we will only show root-groups
groups.where(parent_id: params[:parent_id]) groups.where(parent_id: params[:parent_id]).page(params[:page])
end end
@groups = @groups.with_selects_for_list(archived: params[:archived]) @groups = groups.with_selects_for_list(archived: params[:archived])
.sort_by_attribute(@sort = params[:sort])
.page(params[:page])
respond_to do |format| respond_to do |format|
format.html format.html
...@@ -28,4 +23,21 @@ module GroupTree ...@@ -28,4 +23,21 @@ module GroupTree
end end
# rubocop:enable Gitlab/ModuleWithInstanceVariables # rubocop:enable Gitlab/ModuleWithInstanceVariables
end end
def filtered_groups_with_ancestors(groups)
filtered_groups = groups.search(params[:filter]).page(params[:page])
if Group.supports_nested_groups?
# We find the ancestors by ID of the search results here.
# Otherwise the ancestors would also have filters applied,
# which would cause them not to be preloaded.
#
# Pagination needs to be applied before loading the ancestors to
# make sure ancestors are not cut off by pagination.
Gitlab::GroupHierarchy.new(Group.where(id: filtered_groups.select(:id)))
.base_and_ancestors
else
filtered_groups
end
end
end end
...@@ -44,8 +44,8 @@ module GroupDescendant ...@@ -44,8 +44,8 @@ module GroupDescendant
This error is not user facing, but causes a +1 query. This error is not user facing, but causes a +1 query.
MSG MSG
extras = { extras = {
parent: parent, parent: parent.inspect,
child: child, child: child.inspect,
preloaded: preloaded.map(&:full_path) preloaded: preloaded.map(&:full_path)
} }
issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785' issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785'
......
---
title: Reduce the number of queries when searching for groups
merge_request: 20398
author:
type: performance
...@@ -63,6 +63,17 @@ describe GroupTree do ...@@ -63,6 +63,17 @@ describe GroupTree do
expect(assigns(:groups)).to contain_exactly(parent, subgroup) expect(assigns(:groups)).to contain_exactly(parent, subgroup)
end end
it 'preloads parents regardless of pagination' do
allow(Kaminari.config).to receive(:default_per_page).and_return(1)
group = create(:group, :public)
subgroup = create(:group, :public, parent: group)
search_result = create(:group, :public, name: 'result', parent: subgroup)
get :index, filter: 'resu', format: :json
expect(assigns(:groups)).to contain_exactly(group, subgroup, search_result)
end
end end
context 'json content' do context 'json content' do
......
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