Merge branch 'bvl-preload-parents-after-pagination' into 'master'

Preload ancestors after pagination when filtering

Closes #40785

See merge request gitlab-org/gitlab-ce!20398

app/controllers/concerns/group_tree.rb

 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
+    groups = groups.sort_by_attribute(@sort = params[:sort])
+
+    groups = if params[:filter].present?
+               filtered_groups_with_ancestors(groups)
              else
-                # Only show root groups if no parent-id is given
-                groups.where(parent_id: params[:parent_id])
+               # 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])
-                .sort_by_attribute(@sort = params[:sort])
-                .page(params[:page])
+    @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

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'

changelogs/unreleased/bvl-preload-parents-after-pagination.yml

+---
+title: Reduce the number of queries when searching for groups
+merge_request: 20398
+author:
+type: performance

spec/controllers/concerns/group_tree_spec.rb

@@ -63,6 +63,17 @@ describe GroupTree do
 
         expect(assigns(:groups)).to contain_exactly(parent, subgroup)
       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
 
     context 'json content' do