diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-07-05 12:16:08 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-07-06 15:09:36 +0300 |
commit | de35c044fb5a2def205d9da84e2792c97f40d481 (patch) | |
tree | eb4dfdf1e27b70238915302ae2cb4ad2961aca49 /app | |
parent | 34fe32740f475051dfcbb696f11c270ae327dc4a (diff) |
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.
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/concerns/group_tree.rb | 40 | ||||
-rw-r--r-- | app/models/concerns/group_descendant.rb | 4 |
2 files changed, 28 insertions, 16 deletions
diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 56770a17406..6ec6897e707 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -1,21 +1,16 @@ module GroupTree # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_group_tree(groups) - @groups = if params[:filter].present? - # 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. - group_ids = groups.search(params[:filter]).select(:id) - Gitlab::GroupHierarchy.new(Group.where(id: group_ids)) - .base_and_ancestors - else - # Only show root groups if no parent-id is given - groups.where(parent_id: params[:parent_id]) - end + groups = groups.sort_by_attribute(@sort = params[:sort]) - @groups = @groups.with_selects_for_list(archived: params[:archived]) - .sort_by_attribute(@sort = params[:sort]) - .page(params[:page]) + groups = if params[:filter].present? + filtered_groups_with_ancestors(groups) + else + # If `params[:parent_id]` is `nil`, we will only show root-groups + groups.where(parent_id: params[:parent_id]).page(params[:page]) + end + + @groups = groups.with_selects_for_list(archived: params[:archived]) respond_to do |format| format.html @@ -28,4 +23,21 @@ module GroupTree end # rubocop:enable Gitlab/ModuleWithInstanceVariables 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 diff --git a/app/models/concerns/group_descendant.rb b/app/models/concerns/group_descendant.rb index 261ace57a17..5e9a95c3282 100644 --- a/app/models/concerns/group_descendant.rb +++ b/app/models/concerns/group_descendant.rb @@ -44,8 +44,8 @@ module GroupDescendant This error is not user facing, but causes a +1 query. MSG extras = { - parent: parent, - child: child, + parent: parent.inspect, + child: child.inspect, preloaded: preloaded.map(&:full_path) } issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785' |