Use linear versions of descendants in GroupDescendantsFinder

app/finders/group_descendants_finder.rb

@@ -87,9 +87,13 @@ class GroupDescendantsFinder
       visible_to_user = visible_to_user.or(authorized_to_user)
     end
 
-    hierarchy_for_parent
-      .descendants
-      .where(visible_to_user)
+    group_to_query = if Feature.enabled?(:linear_group_descendants_finder, current_user, default_enabled: :yaml)
+                       parent_group
+                     else
+                       hierarchy_for_parent
+                     end
+
+    group_to_query.descendants.where(visible_to_user)
     # rubocop: enable CodeReuse/Finder
   end
   # rubocop: enable CodeReuse/ActiveRecord
@@ -155,7 +159,13 @@ class GroupDescendantsFinder
   # rubocop: disable CodeReuse/ActiveRecord
   def projects_matching_filter
     # rubocop: disable CodeReuse/Finder
-    projects_nested_in_group = Project.where(namespace_id: hierarchy_for_parent.base_and_descendants.select(:id))
+    objects_in_hierarchy = if Feature.enabled?(:linear_group_descendants_finder, current_user, default_enabled: :yaml)
+                             parent_group.self_and_descendants.as_ids
+                           else
+                             hierarchy_for_parent.base_and_descendants.select(:id)
+                           end
+
+    projects_nested_in_group = Project.where(namespace_id: objects_in_hierarchy)
     params_with_search = params.merge(search: params[:filter])
 
     ProjectsFinder.new(params: params_with_search,

config/feature_flags/development/linear_group_descendants_finder.yml

+---
+name: linear_group_descendants_finder
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68954
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339440
+milestone: '14.6'
+type: development
+group: group::access
+default_enabled: false

spec/finders/group_descendants_finder_spec.rb

@@ -4,7 +4,12 @@ require 'spec_helper'
 
 RSpec.describe GroupDescendantsFinder do
   let_it_be(:user) { create(:user) }
-  let_it_be(:group) { create(:group) }
+
+  let_it_be_with_reload(:group) do
+    create(:group).tap do |g|
+      g.add_owner(user)
+    end
+  end
 
   let(:params) { {} }
 
@@ -12,254 +17,262 @@ RSpec.describe GroupDescendantsFinder do
     described_class.new(current_user: user, parent_group: group, params: params)
   end
 
-  before do
-    group.add_owner(user)
-  end
-
-  describe '#has_children?' do
-    it 'is true when there are projects' do
-      create(:project, namespace: group)
-
-      expect(finder.has_children?).to be_truthy
-    end
-
-    context 'when there are subgroups' do
+  shared_examples 'group descentants finder examples' do
+    describe '#has_children?' do
       it 'is true when there are projects' do
-        create(:group, parent: group)
+        create(:project, namespace: group)
 
         expect(finder.has_children?).to be_truthy
       end
-    end
-  end
 
-  describe '#execute' do
-    it 'includes projects' do
-      project = create(:project, namespace: group)
+      context 'when there are subgroups' do
+        it 'is true when there are projects' do
+          create(:group, parent: group)
 
-      expect(finder.execute).to contain_exactly(project)
+          expect(finder.has_children?).to be_truthy
+        end
+      end
     end
 
-    context 'when archived is `true`' do
-      let(:params) { { archived: 'true' } }
-
-      it 'includes archived projects' do
-        archived_project = create(:project, namespace: group, archived: true)
+    describe '#execute' do
+      it 'includes projects' do
         project = create(:project, namespace: group)
 
-        expect(finder.execute).to contain_exactly(archived_project, project)
+        expect(finder.execute).to contain_exactly(project)
       end
-    end
 
-    context 'when archived is `only`' do
-      let(:params) { { archived: 'only' } }
+      context 'when archived is `true`' do
+        let(:params) { { archived: 'true' } }
 
-      it 'includes only archived projects' do
-        archived_project = create(:project, namespace: group, archived: true)
-        _project = create(:project, namespace: group)
+        it 'includes archived projects' do
+          archived_project = create(:project, namespace: group, archived: true)
+          project = create(:project, namespace: group)
 
-        expect(finder.execute).to contain_exactly(archived_project)
+          expect(finder.execute).to contain_exactly(archived_project, project)
+        end
       end
-    end
 
-    it 'does not include archived projects' do
-      _archived_project = create(:project, :archived, namespace: group)
+      context 'when archived is `only`' do
+        let(:params) { { archived: 'only' } }
 
-      expect(finder.execute).to be_empty
-    end
+        it 'includes only archived projects' do
+          archived_project = create(:project, namespace: group, archived: true)
+          _project = create(:project, namespace: group)
 
-    context 'with a filter' do
-      let(:params) { { filter: 'test' } }
+          expect(finder.execute).to contain_exactly(archived_project)
+        end
+      end
 
-      it 'includes only projects matching the filter' do
-        _other_project = create(:project, namespace: group)
-        matching_project = create(:project, namespace: group, name: 'testproject')
+      it 'does not include archived projects' do
+        _archived_project = create(:project, :archived, namespace: group)
 
-        expect(finder.execute).to contain_exactly(matching_project)
+        expect(finder.execute).to be_empty
       end
-    end
 
-    it 'sorts elements by name as default' do
-      project1 = create(:project, namespace: group, name: 'z')
-      project2 = create(:project, namespace: group, name: 'a')
+      context 'with a filter' do
+        let(:params) { { filter: 'test' } }
 
-      expect(subject.execute).to eq([project2, project1])
-    end
+        it 'includes only projects matching the filter' do
+          _other_project = create(:project, namespace: group)
+          matching_project = create(:project, namespace: group, name: 'testproject')
 
-    context 'sorting by name' do
-      let!(:project1) { create(:project, namespace: group, name: 'a', path: 'project-a') }
-      let!(:project2) { create(:project, namespace: group, name: 'z', path: 'project-z') }
-      let(:params) do
-        {
-          sort: 'name_asc'
-        }
+          expect(finder.execute).to contain_exactly(matching_project)
+        end
       end
 
-      it 'sorts elements by name' do
-        expect(subject.execute).to eq(
-          [
-            project1,
-            project2
-          ]
-        )
+      it 'sorts elements by name as default' do
+        project1 = create(:project, namespace: group, name: 'z')
+        project2 = create(:project, namespace: group, name: 'a')
+
+        expect(subject.execute).to match_array([project2, project1])
       end
 
-      context 'with nested groups' do
-        let!(:subgroup1) { create(:group, parent: group, name: 'a', path: 'sub-a') }
-        let!(:subgroup2) { create(:group, parent: group, name: 'z', path: 'sub-z') }
+      context 'sorting by name' do
+        let!(:project1) { create(:project, namespace: group, name: 'a', path: 'project-a') }
+        let!(:project2) { create(:project, namespace: group, name: 'z', path: 'project-z') }
+        let(:params) do
+          {
+            sort: 'name_asc'
+          }
+        end
 
         it 'sorts elements by name' do
           expect(subject.execute).to eq(
             [
-              subgroup1,
-              subgroup2,
               project1,
               project2
             ]
           )
         end
-      end
-    end
-
-    it 'does not include projects shared with the group' do
-      project = create(:project, namespace: group)
-      other_project = create(:project)
-      other_project.project_group_links.create!(group: group,
-                                               group_access: Gitlab::Access::MAINTAINER)
 
-      expect(finder.execute).to contain_exactly(project)
-    end
-  end
+        context 'with nested groups' do
+          let!(:subgroup1) { create(:group, parent: group, name: 'a', path: 'sub-a') }
+          let!(:subgroup2) { create(:group, parent: group, name: 'z', path: 'sub-z') }
+
+          it 'sorts elements by name' do
+            expect(subject.execute).to eq(
+              [
+                subgroup1,
+                subgroup2,
+                project1,
+                project2
+              ]
+            )
+          end
+        end
+      end
 
-  context 'with shared groups' do
-    let_it_be(:other_group) { create(:group) }
-    let_it_be(:shared_group_link) do
-      create(:group_group_link,
-             shared_group: group,
-             shared_with_group: other_group)
-    end
+      it 'does not include projects shared with the group' do
+        project = create(:project, namespace: group)
+        other_project = create(:project)
+        other_project.project_group_links.create!(group: group,
+                                                group_access: Gitlab::Access::MAINTAINER)
 
-    context 'without common ancestor' do
-      it { expect(finder.execute).to be_empty }
+        expect(finder.execute).to contain_exactly(project)
+      end
     end
 
-    context 'with common ancestor' do
-      let_it_be(:common_ancestor) { create(:group) }
-      let_it_be(:other_group) { create(:group, parent: common_ancestor) }
-      let_it_be(:group) { create(:group, parent: common_ancestor) }
+    context 'with shared groups' do
+      let_it_be(:other_group) { create(:group) }
+      let_it_be(:shared_group_link) do
+        create(:group_group_link,
+              shared_group: group,
+              shared_with_group: other_group)
+      end
 
-      context 'querying under the common ancestor' do
+      context 'without common ancestor' do
         it { expect(finder.execute).to be_empty }
       end
 
-      context 'querying the common ancestor' do
-        subject(:finder) do
-          described_class.new(current_user: user, parent_group: common_ancestor, params: params)
+      context 'with common ancestor' do
+        let_it_be(:common_ancestor) { create(:group) }
+        let_it_be(:other_group) { create(:group, parent: common_ancestor) }
+        let_it_be(:group) { create(:group, parent: common_ancestor) }
+
+        context 'querying under the common ancestor' do
+          it { expect(finder.execute).to be_empty }
         end
 
-        it 'contains shared subgroups' do
-          expect(finder.execute).to contain_exactly(group, other_group)
+        context 'querying the common ancestor' do
+          subject(:finder) do
+            described_class.new(current_user: user, parent_group: common_ancestor, params: params)
+          end
+
+          it 'contains shared subgroups' do
+            expect(finder.execute).to contain_exactly(group, other_group)
+          end
         end
       end
     end
-  end
 
-  context 'with nested groups' do
-    let!(:project) { create(:project, namespace: group) }
-    let!(:subgroup) { create(:group, :private, parent: group) }
+    context 'with nested groups' do
+      let!(:project) { create(:project, namespace: group) }
+      let!(:subgroup) { create(:group, :private, parent: group) }
 
-    describe '#execute' do
-      it 'contains projects and subgroups' do
-        expect(finder.execute).to contain_exactly(subgroup, project)
-      end
+      describe '#execute' do
+        it 'contains projects and subgroups' do
+          expect(finder.execute).to contain_exactly(subgroup, project)
+        end
 
-      it 'does not include subgroups the user does not have access to' do
-        subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+        it 'does not include subgroups the user does not have access to' do
+          subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
 
-        public_subgroup = create(:group, :public, parent: group, path: 'public-group')
-        other_subgroup = create(:group, :private, parent: group, path: 'visible-private-group')
-        other_user = create(:user)
-        other_subgroup.add_developer(other_user)
+          public_subgroup = create(:group, :public, parent: group, path: 'public-group')
+          other_subgroup = create(:group, :private, parent: group, path: 'visible-private-group')
+          other_user = create(:user)
+          other_subgroup.add_developer(other_user)
 
-        finder = described_class.new(current_user: other_user, parent_group: group)
+          finder = described_class.new(current_user: other_user, parent_group: group)
 
-        expect(finder.execute).to contain_exactly(public_subgroup, other_subgroup)
-      end
+          expect(finder.execute).to contain_exactly(public_subgroup, other_subgroup)
+        end
 
-      it 'only includes public groups when no user is given' do
-        public_subgroup = create(:group, :public, parent: group)
-        _private_subgroup = create(:group, :private, parent: group)
+        it 'only includes public groups when no user is given' do
+          public_subgroup = create(:group, :public, parent: group)
+          _private_subgroup = create(:group, :private, parent: group)
 
-        finder = described_class.new(current_user: nil, parent_group: group)
+          finder = described_class.new(current_user: nil, parent_group: group)
 
-        expect(finder.execute).to contain_exactly(public_subgroup)
-      end
+          expect(finder.execute).to contain_exactly(public_subgroup)
+        end
 
-      context 'when archived is `true`' do
-        let(:params) { { archived: 'true' } }
+        context 'when archived is `true`' do
+          let(:params) { { archived: 'true' } }
 
-        it 'includes archived projects in the count of subgroups' do
-          create(:project, namespace: subgroup, archived: true)
+          it 'includes archived projects in the count of subgroups' do
+            create(:project, namespace: subgroup, archived: true)
 
-          expect(finder.execute.first.preloaded_project_count).to eq(1)
+            expect(finder.execute.first.preloaded_project_count).to eq(1)
+          end
         end
-      end
 
-      context 'with a filter' do
-        let(:params) { { filter: 'test' } }
+        context 'with a filter' do
+          let(:params) { { filter: 'test' } }
 
-        it 'contains only matching projects and subgroups' do
-          matching_project = create(:project, namespace: group, name: 'Testproject')
-          matching_subgroup = create(:group, name: 'testgroup', parent: group)
+          it 'contains only matching projects and subgroups' do
+            matching_project = create(:project, namespace: group, name: 'Testproject')
+            matching_subgroup = create(:group, name: 'testgroup', parent: group)
 
-          expect(finder.execute).to contain_exactly(matching_subgroup, matching_project)
-        end
+            expect(finder.execute).to contain_exactly(matching_subgroup, matching_project)
+          end
 
-        it 'does not include subgroups the user does not have access to' do
-          _invisible_subgroup = create(:group, :private, parent: group, name: 'test1')
-          other_subgroup = create(:group, :private, parent: group, name: 'test2')
-          public_subgroup = create(:group, :public, parent: group, name: 'test3')
-          other_subsubgroup = create(:group, :private, parent: other_subgroup, name: 'test4')
-          other_user = create(:user)
-          other_subgroup.add_developer(other_user)
+          it 'does not include subgroups the user does not have access to' do
+            _invisible_subgroup = create(:group, :private, parent: group, name: 'test1')
+            other_subgroup = create(:group, :private, parent: group, name: 'test2')
+            public_subgroup = create(:group, :public, parent: group, name: 'test3')
+            other_subsubgroup = create(:group, :private, parent: other_subgroup, name: 'test4')
+            other_user = create(:user)
+            other_subgroup.add_developer(other_user)
 
-          finder = described_class.new(current_user: other_user,
-                                       parent_group: group,
-                                       params: params)
+            finder = described_class.new(current_user: other_user,
+                                        parent_group: group,
+                                        params: params)
 
-          expect(finder.execute).to contain_exactly(other_subgroup, public_subgroup, other_subsubgroup)
-        end
+            expect(finder.execute).to contain_exactly(other_subgroup, public_subgroup, other_subsubgroup)
+          end
 
-        context 'with matching children' do
-          it 'includes a group that has a subgroup matching the query and its parent' do
-            matching_subgroup = create(:group, :private, name: 'testgroup', parent: subgroup)
+          context 'with matching children' do
+            it 'includes a group that has a subgroup matching the query and its parent' do
+              matching_subgroup = create(:group, :private, name: 'testgroup', parent: subgroup)
 
-            expect(finder.execute).to contain_exactly(subgroup, matching_subgroup)
-          end
+              expect(finder.execute).to contain_exactly(subgroup, matching_subgroup)
+            end
 
-          it 'includes the parent of a matching project' do
-            matching_project = create(:project, namespace: subgroup, name: 'Testproject')
+            it 'includes the parent of a matching project' do
+              matching_project = create(:project, namespace: subgroup, name: 'Testproject')
 
-            expect(finder.execute).to contain_exactly(subgroup, matching_project)
-          end
+              expect(finder.execute).to contain_exactly(subgroup, matching_project)
+            end
 
-          context 'with a small page size' do
-            let(:params) { { filter: 'test', per_page: 1 } }
+            context 'with a small page size' do
+              let(:params) { { filter: 'test', per_page: 1 } }
 
-            it 'contains all the ancestors of a matching subgroup regardless the page size' do
-              subgroup = create(:group, :private, parent: group)
-              matching = create(:group, :private, name: 'testgroup', parent: subgroup)
+              it 'contains all the ancestors of a matching subgroup regardless the page size' do
+                subgroup = create(:group, :private, parent: group)
+                matching = create(:group, :private, name: 'testgroup', parent: subgroup)
 
-              expect(finder.execute).to contain_exactly(subgroup, matching)
+                expect(finder.execute).to contain_exactly(subgroup, matching)
+              end
             end
-          end
 
-          it 'does not include the parent itself' do
-            group.update!(name: 'test')
+            it 'does not include the parent itself' do
+              group.update!(name: 'test')
 
-            expect(finder.execute).not_to include(group)
+              expect(finder.execute).not_to include(group)
+            end
           end
         end
       end
     end
   end
+
+  it_behaves_like 'group descentants finder examples'
+
+  context 'when feature flag :linear_group_descendants_finder is disabled' do
+    before do
+      stub_feature_flags(linear_group_descendants_finder: false)
+    end
+
+    it_behaves_like 'group descentants finder examples'
+  end
 end