Merge branch '339440-fj-use-linear-group-descendants-finder-scopes' into 'master'

Enable linear versions in GroupDescendantsFinder

See merge request gitlab-org/gitlab!75877

app/finders/group_descendants_finder.rb

@@ -87,13 +87,7 @@ class GroupDescendantsFinder
       visible_to_user = visible_to_user.or(authorized_to_user)
     end
 
-    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)
+    parent_group.descendants.where(visible_to_user)
     # rubocop: enable CodeReuse/Finder
   end
   # rubocop: enable CodeReuse/ActiveRecord
@@ -159,13 +153,7 @@ class GroupDescendantsFinder
   # rubocop: disable CodeReuse/ActiveRecord
   def projects_matching_filter
     # rubocop: disable CodeReuse/Finder
-    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)
+    projects_nested_in_group = Project.where(namespace_id: parent_group.self_and_descendants.as_ids)
     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

@@ -17,262 +17,250 @@ RSpec.describe GroupDescendantsFinder do
     described_class.new(current_user: user, parent_group: group, params: params)
   end
 
-  shared_examples 'group descentants finder examples' do
-    describe '#has_children?' do
+  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
       it 'is true when there are projects' do
-        create(:project, namespace: group)
+        create(:group, parent: group)
 
         expect(finder.has_children?).to be_truthy
       end
+    end
+  end
 
-      context 'when there are subgroups' do
-        it 'is true when there are projects' do
-          create(:group, parent: group)
+  describe '#execute' do
+    it 'includes projects' do
+      project = create(:project, namespace: group)
 
-          expect(finder.has_children?).to be_truthy
-        end
-      end
+      expect(finder.execute).to contain_exactly(project)
     end
 
-    describe '#execute' do
-      it 'includes projects' do
+    context 'when archived is `true`' do
+      let(:params) { { archived: 'true' } }
+
+      it 'includes archived projects' do
+        archived_project = create(:project, namespace: group, archived: true)
         project = create(:project, namespace: group)
 
-        expect(finder.execute).to contain_exactly(project)
+        expect(finder.execute).to contain_exactly(archived_project, project)
       end
+    end
 
-      context 'when archived is `true`' do
-        let(:params) { { archived: 'true' } }
+    context 'when archived is `only`' do
+      let(:params) { { archived: 'only' } }
 
-        it 'includes archived projects' do
-          archived_project = create(:project, namespace: group, archived: true)
-          project = create(:project, namespace: group)
+      it 'includes only archived projects' do
+        archived_project = create(:project, namespace: group, archived: true)
+        _project = create(:project, namespace: group)
 
-          expect(finder.execute).to contain_exactly(archived_project, project)
-        end
+        expect(finder.execute).to contain_exactly(archived_project)
       end
+    end
 
-      context 'when archived is `only`' do
-        let(:params) { { archived: 'only' } }
+    it 'does not include archived projects' do
+      _archived_project = create(:project, :archived, namespace: group)
 
-        it 'includes only archived projects' do
-          archived_project = create(:project, namespace: group, archived: true)
-          _project = create(:project, namespace: group)
+      expect(finder.execute).to be_empty
+    end
 
-          expect(finder.execute).to contain_exactly(archived_project)
-        end
-      end
+    context 'with a filter' do
+      let(:params) { { filter: 'test' } }
 
-      it 'does not include archived projects' do
-        _archived_project = create(:project, :archived, namespace: group)
+      it 'includes only projects matching the filter' do
+        _other_project = create(:project, namespace: group)
+        matching_project = create(:project, namespace: group, name: 'testproject')
 
-        expect(finder.execute).to be_empty
+        expect(finder.execute).to contain_exactly(matching_project)
       end
+    end
 
-      context 'with a filter' do
-        let(:params) { { filter: 'test' } }
+    it 'sorts elements by name as default' do
+      project1 = create(:project, namespace: group, name: 'z')
+      project2 = create(:project, namespace: group, name: 'a')
 
-        it 'includes only projects matching the filter' do
-          _other_project = create(:project, namespace: group)
-          matching_project = create(:project, namespace: group, name: 'testproject')
+      expect(subject.execute).to match_array([project2, project1])
+    end
 
-          expect(finder.execute).to contain_exactly(matching_project)
-        end
+    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 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])
+      it 'sorts elements by name' do
+        expect(subject.execute).to eq(
+          [
+            project1,
+            project2
+          ]
+        )
       end
 
-      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
+      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
-
-        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
+    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)
+    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
+      expect(finder.execute).to contain_exactly(project)
     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
+  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 'without common ancestor' do
+    context 'without common ancestor' do
+      it { expect(finder.execute).to be_empty }
+    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 'querying under the common ancestor' do
         it { expect(finder.execute).to be_empty }
       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 'querying under the common ancestor' do
-          it { expect(finder.execute).to be_empty }
+      context 'querying the common ancestor' do
+        subject(:finder) do
+          described_class.new(current_user: user, parent_group: common_ancestor, params: params)
         end
 
-        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
+        it 'contains shared subgroups' do
+          expect(finder.execute).to contain_exactly(group, other_group)
         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)
-          end
+          expect(finder.execute.first.preloaded_project_count).to eq(1)
         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)
-              end
+              expect(finder.execute).to contain_exactly(subgroup, matching)
             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)
-            end
+            expect(finder.execute).not_to include(group)
           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