Merge branch '339235-fj-use-group-linear-ancestor-scopes' into 'master'

Use Group linear ancestor scopes

See merge request gitlab-org/gitlab!70708

config/feature_flags/development/linear_ee_group_ancestor_scopes.yml

+---
+name: linear_ee_group_ancestor_scopes
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70708
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/341350
+milestone: '14.4'
+type: development
+group: group::access
+default_enabled: false

ee/app/models/ee/group.rb

@@ -614,7 +614,13 @@ module EE
     end
 
     def invited_or_shared_group_members(groups)
-      ::GroupMember.active_without_invites_and_requests.where(source_id: ::Gitlab::ObjectHierarchy.new(groups).base_and_ancestors)
+      groups_and_ancestors = if ::Feature.enabled?(:linear_ee_group_ancestor_scopes, self, default_enabled: :yaml)
+                               groups.self_and_ancestors
+                             else
+                               ::Gitlab::ObjectHierarchy.new(groups).base_and_ancestors
+                             end
+
+      ::GroupMember.active_without_invites_and_requests.where(source_id: groups_and_ancestors)
     end
 
     override :_safe_read_repository_read_only_column

ee/spec/models/ee/namespace_spec.rb

@@ -936,251 +936,263 @@ RSpec.describe Namespace do
     end
 
     context 'with a group namespace' do
-      let(:group) { create(:group) }
-      let(:developer) { create(:user) }
-      let(:guest) { create(:user) }
-
-      before do
-        group.add_developer(developer)
-        group.add_developer(create(:user, :blocked))
-        group.add_guest(guest)
-      end
-
-      subject(:billed_user_ids) { group.billed_user_ids }
-
-      it 'returns a breakdown of billable user ids' do
-        expect(billed_user_ids.keys).to eq([
-          :user_ids,
-          :group_member_user_ids,
-          :project_member_user_ids,
-          :shared_group_user_ids,
-          :shared_project_user_ids
-        ])
-      end
+      shared_examples 'billable group plan retrieval' do
+        let(:group) { create(:group) }
+        let(:developer) { create(:user) }
+        let(:guest) { create(:user) }
 
-      context 'with a ultimate plan' do
         before do
-          create(:gitlab_subscription, namespace: group, hosted_plan: ultimate_plan)
+          group.add_developer(developer)
+          group.add_developer(create(:user, :blocked))
+          group.add_guest(guest)
         end
 
-        it 'does not include guest users and only active users' do
-          expect(billed_user_ids[:user_ids]).to match_array([developer.id])
-        end
+        subject(:billed_user_ids) { group.billed_user_ids }
 
-        context 'when group has a project and users are invited to it' do
-          let(:project) { create(:project, namespace: group) }
-          let(:project_developer) { create(:user) }
+        it 'returns a breakdown of billable user ids' do
+          expect(billed_user_ids.keys).to eq([
+            :user_ids,
+            :group_member_user_ids,
+            :project_member_user_ids,
+            :shared_group_user_ids,
+            :shared_project_user_ids
+          ])
+        end
 
+        context 'with a ultimate plan' do
           before do
-            project.add_developer(project_developer)
-            project.add_guest(create(:user))
-            project.add_developer(developer)
-            project.add_developer(create(:user, :blocked))
+            create(:gitlab_subscription, namespace: group, hosted_plan: ultimate_plan)
           end
 
-          it 'includes invited active users except guests to the group', :aggregate_failures do
-            expect(billed_user_ids[:user_ids]).to match_array([project_developer.id, developer.id])
-            expect(billed_user_ids[:project_member_user_ids]).to match_array([project_developer.id, developer.id])
-            expect(billed_user_ids[:group_member_user_ids]).to match_array([developer.id])
-            expect(billed_user_ids[:shared_group_user_ids]).to match_array([])
-            expect(billed_user_ids[:shared_project_user_ids]).to match_array([])
+          it 'does not include guest users and only active users' do
+            expect(billed_user_ids[:user_ids]).to match_array([developer.id])
           end
 
-          context 'with project bot users' do
-            include_context 'project bot users'
+          context 'when group has a project and users are invited to it' do
+            let(:project) { create(:project, namespace: group) }
+            let(:project_developer) { create(:user) }
 
-            it { expect(billed_user_ids[:user_ids]).not_to include(project_bot.id) }
-            it { expect(billed_user_ids[:project_member_user_ids]).not_to include(project_bot.id) }
-          end
+            before do
+              project.add_developer(project_developer)
+              project.add_guest(create(:user))
+              project.add_developer(developer)
+              project.add_developer(create(:user, :blocked))
+            end
 
-          context 'when group is invited to the project' do
-            let(:invited_group) { create(:group) }
-            let(:invited_group_developer) { create(:user) }
+            it 'includes invited active users except guests to the group', :aggregate_failures do
+              expect(billed_user_ids[:user_ids]).to match_array([project_developer.id, developer.id])
+              expect(billed_user_ids[:project_member_user_ids]).to match_array([project_developer.id, developer.id])
+              expect(billed_user_ids[:group_member_user_ids]).to match_array([developer.id])
+              expect(billed_user_ids[:shared_group_user_ids]).to match_array([])
+              expect(billed_user_ids[:shared_project_user_ids]).to match_array([])
+            end
 
-            before do
-              invited_group.add_developer(invited_group_developer)
-              invited_group.add_guest(create(:user))
-              invited_group.add_developer(create(:user, :blocked))
-              invited_group.add_developer(developer)
+            context 'with project bot users' do
+              include_context 'project bot users'
+
+              it { expect(billed_user_ids[:user_ids]).not_to include(project_bot.id) }
+              it { expect(billed_user_ids[:project_member_user_ids]).not_to include(project_bot.id) }
             end
 
-            context 'when group is invited as non guest' do
+            context 'when group is invited to the project' do
+              let(:invited_group) { create(:group) }
+              let(:invited_group_developer) { create(:user) }
+
               before do
-                create(:project_group_link, project: project, group: invited_group)
+                invited_group.add_developer(invited_group_developer)
+                invited_group.add_guest(create(:user))
+                invited_group.add_developer(create(:user, :blocked))
+                invited_group.add_developer(developer)
               end
 
-              it 'includes only active users except guests of the invited groups', :aggregate_failures do
-                expect(billed_user_ids[:user_ids]).to match_array([invited_group_developer.id, project_developer.id, developer.id])
-                expect(billed_user_ids[:shared_group_user_ids]).to match_array([])
-                expect(billed_user_ids[:shared_project_user_ids]).to match_array([invited_group_developer.id, developer.id])
-                expect(billed_user_ids[:group_member_user_ids]).to match_array([developer.id])
-                expect(billed_user_ids[:project_member_user_ids]).to match_array([developer.id, project_developer.id])
+              context 'when group is invited as non guest' do
+                before do
+                  create(:project_group_link, project: project, group: invited_group)
+                end
+
+                it 'includes only active users except guests of the invited groups', :aggregate_failures do
+                  expect(billed_user_ids[:user_ids]).to match_array([invited_group_developer.id, project_developer.id, developer.id])
+                  expect(billed_user_ids[:shared_group_user_ids]).to match_array([])
+                  expect(billed_user_ids[:shared_project_user_ids]).to match_array([invited_group_developer.id, developer.id])
+                  expect(billed_user_ids[:group_member_user_ids]).to match_array([developer.id])
+                  expect(billed_user_ids[:project_member_user_ids]).to match_array([developer.id, project_developer.id])
+                end
               end
-            end
 
-            context 'when group is invited as a guest to the project' do
-              before do
-                create(:project_group_link, :guest, project: project, group: invited_group)
-              end
+              context 'when group is invited as a guest to the project' do
+                before do
+                  create(:project_group_link, :guest, project: project, group: invited_group)
+                end
 
-              it 'does not include any members from the invited group', :aggregate_failures do
-                expect(billed_user_ids[:user_ids]).to match_array([project_developer.id, developer.id])
-                expect(billed_user_ids[:shared_project_user_ids]).to be_empty
+                it 'does not include any members from the invited group', :aggregate_failures do
+                  expect(billed_user_ids[:user_ids]).to match_array([project_developer.id, developer.id])
+                  expect(billed_user_ids[:shared_project_user_ids]).to be_empty
+                end
               end
             end
           end
-        end
 
-        context 'when group has been shared with another group' do
-          let(:shared_group) { create(:group) }
-          let(:shared_group_developer) { create(:user) }
-
-          before do
-            shared_group.add_developer(shared_group_developer)
-            shared_group.add_guest(create(:user))
-            shared_group.add_developer(create(:user, :blocked))
-
-            create(:group_group_link, { shared_with_group: shared_group,
-                                        shared_group: group })
-          end
+          context 'when group has been shared with another group' do
+            let(:shared_group) { create(:group) }
+            let(:shared_group_developer) { create(:user) }
 
-          it 'includes active users from the shared group to the billed members', :aggregate_failures do
-            expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id])
-            expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id])
-            expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
-          end
+            before do
+              shared_group.add_developer(shared_group_developer)
+              shared_group.add_guest(create(:user))
+              shared_group.add_developer(create(:user, :blocked))
 
-          context 'when subgroup invited another group to collaborate' do
-            let(:another_shared_group) { create(:group) }
-            let(:another_shared_group_developer) { create(:user) }
+              create(:group_group_link, { shared_with_group: shared_group,
+                                          shared_group: group })
+            end
 
-            before do
-              another_shared_group.add_developer(another_shared_group_developer)
-              another_shared_group.add_guest(create(:user))
-              another_shared_group.add_developer(create(:user, :blocked))
+            it 'includes active users from the shared group to the billed members', :aggregate_failures do
+              expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id])
+              expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id])
+              expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
             end
 
-            context 'when subgroup invites another group as non guest' do
-              before do
-                subgroup = create(:group, parent: group)
-                create(:group_group_link, { shared_with_group: another_shared_group,
-                                            shared_group: subgroup })
-              end
+            context 'when subgroup invited another group to collaborate' do
+              let(:another_shared_group) { create(:group) }
+              let(:another_shared_group_developer) { create(:user) }
 
-              it 'includes all the active and non guest users from the shared group', :aggregate_failures do
-                expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id, another_shared_group_developer.id])
-                expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id, another_shared_group_developer.id])
-                expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
-                expect(another_shared_group.billed_user_ids[:user_ids]).not_to include([developer.id, shared_group_developer.id])
+              before do
+                another_shared_group.add_developer(another_shared_group_developer)
+                another_shared_group.add_guest(create(:user))
+                another_shared_group.add_developer(create(:user, :blocked))
               end
-            end
 
-            context 'when subgroup invites another group as guest' do
-              before do
-                subgroup = create(:group, parent: group)
-                create(:group_group_link, :guest, { shared_with_group: another_shared_group,
-                                                    shared_group: subgroup })
+              context 'when subgroup invites another group as non guest' do
+                before do
+                  subgroup = create(:group, parent: group)
+                  create(:group_group_link, { shared_with_group: another_shared_group,
+                                              shared_group: subgroup })
+                end
+
+                it 'includes all the active and non guest users from the shared group', :aggregate_failures do
+                  expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id, another_shared_group_developer.id])
+                  expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id, another_shared_group_developer.id])
+                  expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
+                  expect(another_shared_group.billed_user_ids[:user_ids]).not_to include([developer.id, shared_group_developer.id])
+                end
               end
 
-              it 'does not includes any user from the shared group from the subgroup', :aggregate_failures do
-                expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id])
-                expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id])
+              context 'when subgroup invites another group as guest' do
+                before do
+                  subgroup = create(:group, parent: group)
+                  create(:group_group_link, :guest, { shared_with_group: another_shared_group,
+                                                      shared_group: subgroup })
+                end
+
+                it 'does not includes any user from the shared group from the subgroup', :aggregate_failures do
+                  expect(billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, developer.id])
+                  expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id])
+                end
               end
             end
           end
         end
-      end
 
-      context 'with other plans' do
-        %i[bronze_plan premium_plan].each do |plan|
-          subject(:billed_user_ids) { group.billed_user_ids }
+        context 'with other plans' do
+          %i[bronze_plan premium_plan].each do |plan|
+            subject(:billed_user_ids) { group.billed_user_ids }
 
-          it 'includes active guest users', :aggregate_failures do
-            create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
-            expect(billed_user_ids[:user_ids]).to match_array([guest.id, developer.id])
-            expect(billed_user_ids[:group_member_user_ids]).to match_array([guest.id, developer.id])
-          end
-
-          context 'when group has a project and users invited to it' do
-            let(:project) { create(:project, namespace: group) }
-            let(:project_developer) { create(:user) }
-            let(:project_guest) { create(:user) }
-
-            before do
+            it 'includes active guest users', :aggregate_failures do
               create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
-              project.add_developer(project_developer)
-              project.add_guest(project_guest)
-              project.add_developer(create(:user, :blocked))
-              project.add_developer(developer)
+              expect(billed_user_ids[:user_ids]).to match_array([guest.id, developer.id])
+              expect(billed_user_ids[:group_member_user_ids]).to match_array([guest.id, developer.id])
             end
 
-            it 'includes invited active users to the group', :aggregate_failures do
-              expect(billed_user_ids[:user_ids]).to match_array([guest.id, developer.id, project_guest.id, project_developer.id])
-              expect(billed_user_ids[:project_member_user_ids]).to match_array([developer.id, project_guest.id, project_developer.id])
-            end
+            context 'when group has a project and users invited to it' do
+              let(:project) { create(:project, namespace: group) }
+              let(:project_developer) { create(:user) }
+              let(:project_guest) { create(:user) }
 
-            context 'with project bot users' do
-              include_context 'project bot users'
+              before do
+                create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
+                project.add_developer(project_developer)
+                project.add_guest(project_guest)
+                project.add_developer(create(:user, :blocked))
+                project.add_developer(developer)
+              end
 
-              it { expect(billed_user_ids[:user_ids]).not_to include(project_bot.id) }
-              it { expect(billed_user_ids[:project_member_user_ids]).not_to include(project_bot.id) }
-            end
+              it 'includes invited active users to the group', :aggregate_failures do
+                expect(billed_user_ids[:user_ids]).to match_array([guest.id, developer.id, project_guest.id, project_developer.id])
+                expect(billed_user_ids[:project_member_user_ids]).to match_array([developer.id, project_guest.id, project_developer.id])
+              end
 
-            context 'when group is invited to the project' do
-              let(:invited_group) { create(:group) }
-              let(:invited_group_developer) { create(:user) }
-              let(:invited_group_guest) { create(:user) }
+              context 'with project bot users' do
+                include_context 'project bot users'
 
-              before do
-                invited_group.add_developer(invited_group_developer)
-                invited_group.add_developer(developer)
-                invited_group.add_guest(invited_group_guest)
-                invited_group.add_developer(create(:user, :blocked))
-                create(:project_group_link, project: project, group: invited_group)
+                it { expect(billed_user_ids[:user_ids]).not_to include(project_bot.id) }
+                it { expect(billed_user_ids[:project_member_user_ids]).not_to include(project_bot.id) }
               end
 
-              it 'includes the unique active users and guests of the invited groups', :aggregate_failures do
-                expect(billed_user_ids[:user_ids]).to match_array([
-                  guest.id,
-                  developer.id,
-                  project_guest.id,
-                  project_developer.id,
-                  invited_group_developer.id,
-                  invited_group_guest.id
-                ])
-
-                expect(billed_user_ids[:shared_project_user_ids]).to match_array([
-                  developer.id,
-                  invited_group_developer.id,
-                  invited_group_guest.id
-                ])
+              context 'when group is invited to the project' do
+                let(:invited_group) { create(:group) }
+                let(:invited_group_developer) { create(:user) }
+                let(:invited_group_guest) { create(:user) }
+
+                before do
+                  invited_group.add_developer(invited_group_developer)
+                  invited_group.add_developer(developer)
+                  invited_group.add_guest(invited_group_guest)
+                  invited_group.add_developer(create(:user, :blocked))
+                  create(:project_group_link, project: project, group: invited_group)
+                end
+
+                it 'includes the unique active users and guests of the invited groups', :aggregate_failures do
+                  expect(billed_user_ids[:user_ids]).to match_array([
+                    guest.id,
+                    developer.id,
+                    project_guest.id,
+                    project_developer.id,
+                    invited_group_developer.id,
+                    invited_group_guest.id
+                  ])
+
+                  expect(billed_user_ids[:shared_project_user_ids]).to match_array([
+                    developer.id,
+                    invited_group_developer.id,
+                    invited_group_guest.id
+                  ])
+                end
               end
             end
-          end
 
-          context 'when group has been shared with another group' do
-            let(:shared_group) { create(:group) }
-            let(:shared_group_developer) { create(:user) }
-            let(:shared_group_guest) { create(:user) }
+            context 'when group has been shared with another group' do
+              let(:shared_group) { create(:group) }
+              let(:shared_group_developer) { create(:user) }
+              let(:shared_group_guest) { create(:user) }
 
-            before do
-              create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
-              shared_group.add_developer(shared_group_developer)
-              shared_group.add_guest(shared_group_guest)
-              shared_group.add_developer(create(:user, :blocked))
+              before do
+                create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
+                shared_group.add_developer(shared_group_developer)
+                shared_group.add_guest(shared_group_guest)
+                shared_group.add_developer(create(:user, :blocked))
 
-              create(:group_group_link, { shared_with_group: shared_group,
-                                          shared_group: group })
-            end
+                create(:group_group_link, { shared_with_group: shared_group,
+                                            shared_group: group })
+              end
 
-            it 'includes active users from the shared group including guests', :aggregate_failures do
-              expect(billed_user_ids[:user_ids]).to match_array([developer.id, guest.id, shared_group_developer.id, shared_group_guest.id])
-              expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
-              expect(shared_group.billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
+              it 'includes active users from the shared group including guests', :aggregate_failures do
+                expect(billed_user_ids[:user_ids]).to match_array([developer.id, guest.id, shared_group_developer.id, shared_group_guest.id])
+                expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
+                expect(shared_group.billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
+              end
             end
           end
         end
       end
+
+      it_behaves_like 'billable group plan retrieval'
+
+      context 'when feature flag :linear_ee_group_ancestor_scopes is disabled' do
+        before do
+          stub_feature_flags(linear_ee_group_ancestor_scopes: false)
+        end
+
+        it_behaves_like 'billable group plan retrieval'
+      end
     end
   end