Merge branch '348293-fj-introduce-group-descendants-finder-upto' into 'master'

Introduce linear ancestors upto in GroupDescendantsFinder

See merge request gitlab-org/gitlab!78991

app/finders/group_descendants_finder.rb

@@ -110,8 +110,13 @@ class GroupDescendantsFinder
   # rubocop: disable CodeReuse/ActiveRecord
   def ancestors_of_groups(base_for_ancestors)
     group_ids = base_for_ancestors.except(:select, :sort).select(:id)
-    Gitlab::ObjectHierarchy.new(Group.where(id: group_ids))
-      .base_and_ancestors(upto: parent_group.id)
+    groups = Group.where(id: group_ids)
+
+    if Feature.enabled?(:linear_group_descendants_finder_upto, current_user, default_enabled: :yaml)
+      groups.self_and_ancestors(upto: parent_group.id)
+    else
+      Gitlab::ObjectHierarchy.new(groups).base_and_ancestors(upto: parent_group.id)
+    end
   end
   # rubocop: enable CodeReuse/ActiveRecord
 

config/feature_flags/development/linear_group_descendants_finder_upto.yml

+---
+name: linear_group_descendants_finder_upto
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78991
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350972
+milestone: '14.8'
+type: development
+group: group::authentication and authorization
+default_enabled: false

spec/finders/group_descendants_finder_spec.rb

@@ -165,8 +165,8 @@ RSpec.describe GroupDescendantsFinder do
   end
 
   context 'with nested groups' do
-    let!(:project) { create(:project, namespace: group) }
-    let!(:subgroup) { create(:group, :private, parent: group) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be_with_reload(:subgroup) { create(:group, :private, parent: group) }
 
     describe '#execute' do
       it 'contains projects and subgroups' do
@@ -208,57 +208,69 @@ RSpec.describe GroupDescendantsFinder do
       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)
+        shared_examples 'filter examples' do
+          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 } }
+
+              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)
 
-          context 'with a small page size' do
-            let(:params) { { filter: 'test', per_page: 1 } }
+                expect(finder.execute).to contain_exactly(subgroup, matching)
+              end
+            end
 
-            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 'does not include the parent itself' do
+              group.update!(name: 'test')
 
-              expect(finder.execute).to contain_exactly(subgroup, matching)
+              expect(finder.execute).not_to include(group)
             end
           end
+        end
 
-          it 'does not include the parent itself' do
-            group.update!(name: 'test')
+        it_behaves_like 'filter examples'
 
-            expect(finder.execute).not_to include(group)
+        context 'when feature flag :linear_group_descendants_finder_upto is disabled' do
+          before do
+            stub_feature_flags(linear_group_descendants_finder_upto: false)
           end
+
+          it_behaves_like 'filter examples'
         end
       end
     end