Merge branch...

Merge branch '207091-only-include-user-s-highest-level-in-the-particular-namespace-while-deciding-if-using-paid' into 'master'

Check Users ID in namespace while deciding paid seat

Closes #207091

See merge request gitlab-org/gitlab!25437

ee/app/models/ee/group.rb

@@ -244,25 +244,32 @@ module EE
       project
     end
 
+    override :billable_members_count
+    def billable_members_count(requested_hosted_plan = nil)
+      billed_user_ids(requested_hosted_plan).count
+    end
+
     # For now, we are not billing for members with a Guest role for subscriptions
     # with a Gold plan. The other plans will treat Guest members as a regular member
     # for billing purposes.
     #
     # We are plucking the user_ids from the "Members" table in an array and
-    # concatenating the array of user_ids with ruby "|" (pipe) method to generate
-    # one single array of unique user_ids.
-    override :billable_members_count
-    def billable_members_count(requested_hosted_plan = nil)
+    # converting the array of user_ids to a Set which will have unique user_ids.
+    def billed_user_ids(requested_hosted_plan = nil)
       if [actual_plan_name, requested_hosted_plan].include?(Plan::GOLD)
-        (billed_group_members.non_guests.distinct.pluck(:user_id) |
-        billed_project_members.non_guests.distinct.pluck(:user_id) |
-        billed_shared_group_members.non_guests.distinct.pluck(:user_id) |
-        billed_invited_group_members.non_guests.distinct.pluck(:user_id)).count
+        strong_memoize(:gold_billed_user_ids) do
+          (billed_group_members.non_guests.distinct.pluck(:user_id) +
+          billed_project_members.non_guests.distinct.pluck(:user_id) +
+          billed_shared_group_members.non_guests.distinct.pluck(:user_id) +
+          billed_invited_group_members.non_guests.distinct.pluck(:user_id)).to_set
+        end
       else
-        (billed_group_members.distinct.pluck(:user_id) |
-        billed_project_members.distinct.pluck(:user_id) |
-        billed_shared_group_members.distinct.pluck(:user_id) |
-        billed_invited_group_members.distinct.pluck(:user_id)).count
+        strong_memoize(:non_gold_billed_user_ids) do
+          (billed_group_members.distinct.pluck(:user_id) +
+          billed_project_members.distinct.pluck(:user_id) +
+          billed_shared_group_members.distinct.pluck(:user_id) +
+          billed_invited_group_members.distinct.pluck(:user_id)).to_set
+        end
       end
     end
 

ee/app/models/ee/namespace.rb

@@ -326,6 +326,14 @@ module EE
       1
     end
 
+    # When a purchasing a GL.com plan for a User namespace
+    # we only charge for a single user.
+    # This method is overwritten in Group where we made the calculation
+    # for Group namespaces.
+    def billed_user_ids(_requested_hosted_plan = nil)
+      [owner_id]
+    end
+
     def eligible_for_trial?
       ::Gitlab.com? &&
         parent_id.nil? &&

ee/app/models/ee/user.rb

@@ -262,15 +262,11 @@ module EE
     end
 
     def using_gitlab_com_seat?(namespace)
-      return false unless ::Gitlab.com?
-      return false unless namespace.present?
-      return false if namespace.free_plan?
-
-      if namespace.gold_plan?
-        highest_role > ::Gitlab::Access::GUEST
-      else
-        true
-      end
+      ::Gitlab.com? &&
+      namespace.present? &&
+      active? &&
+      !namespace.root_ancestor.free_plan? &&
+      namespace.root_ancestor.billed_user_ids.include?(self.id)
     end
 
     def group_sso?(group)

ee/changelogs/unreleased/207091-only-include-user-s-highest-level-in-the-particular-namespace-w.yml

+---
+title: Check user role for each of namespaces while deciding if it is using a paid seat in it.
+merge_request: 25437
+author:
+type: fixed

ee/spec/models/namespace_spec.rb

@@ -831,6 +831,191 @@ describe Namespace do
     end
   end
 
+  describe '#billed_user_ids' do
+    context 'with a user namespace' do
+      let(:user) { create(:user) }
+
+      it 'returns 1' do
+        expect(user.namespace.billed_user_ids).to eq([user.id])
+      end
+    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
+
+      context 'with a gold plan' do
+        before do
+          create(:gitlab_subscription, namespace: group, hosted_plan: gold_plan)
+        end
+
+        it 'does not include guest users and only active users' do
+          expect(group.billed_user_ids).to match_array([developer.id])
+        end
+
+        context 'when group has a project and users are invited to it' do
+          let(:project) { create(:project, namespace: group) }
+          let(:project_developer) { create(:user) }
+
+          before do
+            project.add_developer(project_developer)
+            project.add_guest(create(:user))
+            project.add_developer(developer)
+            project.add_developer(create(:user, :blocked))
+          end
+
+          it 'includes invited active users except guests to the group' do
+            expect(group.billed_user_ids).to match_array([project_developer.id, developer.id])
+          end
+
+          context 'when group is invited to the project' do
+            let(:invited_group) { create(:group) }
+            let(:invited_group_developer) { create(:user) }
+
+            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)
+              create(:project_group_link, project: project, group: invited_group)
+            end
+
+            it 'includes the only active users except guests of the invited groups' do
+              expect(group.billed_user_ids).to match_array([invited_group_developer.id, project_developer.id, developer.id])
+            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: group,
+              shared_group: shared_group })
+          end
+
+          context 'when feature is not enabled' do
+            before do
+              stub_feature_flags(share_group_with_group: false)
+            end
+
+            it 'does not include users coming from the shared groups' do
+              expect(group.billed_user_ids).to match_array([developer.id])
+            end
+          end
+
+          context 'when feature is enabled' do
+            before do
+              stub_feature_flags(share_group_with_group: true)
+            end
+
+            it 'includes active users from the shared group to the billed members count' do
+              expect(group.billed_user_ids).to match_array([shared_group_developer.id, developer.id])
+            end
+          end
+        end
+      end
+
+      context 'with other plans' do
+        %i[bronze_plan silver_plan].each do |plan|
+          it 'includes active guest users' do
+            create(:gitlab_subscription, namespace: group, hosted_plan: send(plan))
+            expect(group.billed_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
+              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 'includes invited active users to the group' do
+              expect(group.billed_user_ids).to match_array([guest.id, 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) }
+
+              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' do
+                expect(group.billed_user_ids).to match_array([guest.id,
+                                                              developer.id,
+                                                              project_guest.id,
+                                                              project_developer.id,
+                                                              invited_group_developer.id,
+                                                              invited_group_guest.id])
+              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) }
+
+            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: group,
+                shared_group: shared_group })
+            end
+
+            context 'when feature is not enabled' do
+              before do
+                stub_feature_flags(share_group_with_group: false)
+              end
+
+              it 'does not include users coming from the shared groups' do
+                expect(group.billed_user_ids).to match_array([developer.id, guest.id])
+              end
+            end
+
+            context 'when feature is enabled' do
+              before do
+                stub_feature_flags(share_group_with_group: true)
+              end
+
+              it 'includes active users from the shared group including guests' do
+                expect(group.billed_user_ids).to match_array([developer.id, guest.id, shared_group_developer.id, shared_group_guest.id])
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+
   describe '#billable_members_count' do
     context 'with a user namespace' do
       let(:user) { create(:user) }

ee/spec/models/user_spec.rb

@@ -671,69 +671,102 @@ describe User do
 
   describe '#using_gitlab_com_seat?' do
     let(:user) { create(:user) }
+    let(:namespace) { create(:group) }
+
+    subject { user.using_gitlab_com_seat?(namespace) }
 
     context 'when Gitlab.com? is false' do
       before do
         allow(Gitlab).to receive(:com?).and_return(false)
       end
 
-      it 'returns false' do
-        expect(user.using_gitlab_com_seat?(nil)).to eq(false)
+      it { is_expected.to be_falsey }
     end
+
+    context 'when user is not active' do
+      let(:user) { create(:user, :blocked) }
+
+      it { is_expected.to be_falsey }
     end
 
     context 'when Gitlab.com? is true' do
-      let(:namespace) { create(:namespace) }
-
       before do
         allow(Gitlab).to receive(:com?).and_return(true)
-        allow(namespace).to receive(:gold_plan?).and_return(false)
-        allow(namespace).to receive(:free_plan?).and_return(false)
       end
 
       context 'when namespace is nil' do
         let(:namespace) { nil }
 
-        it 'returns false' do
-          expect(user.using_gitlab_com_seat?(namespace)).to eq(false)
-        end
+        it { is_expected.to be_falsey }
       end
 
       context 'when namespace is on a free plan' do
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when namespace is on a gold plan' do
         before do
-          allow(namespace).to receive(:free_plan?).and_return(true)
+          create(:gitlab_subscription, namespace: namespace.root_ancestor, hosted_plan: create(:gold_plan))
         end
 
-        it 'returns false' do
-          expect(user.using_gitlab_com_seat?(namespace)).to eq(false)
+        context 'user is a guest' do
+          before do
+            namespace.add_guest(user)
           end
+
+          it { is_expected.to be_falsey }
         end
 
-      context 'when namespace is on a gold plan' do
+        context 'user is not a guest' do
           before do
-          allow(namespace).to receive(:gold_plan?).and_return(true)
+            namespace.add_developer(user)
           end
 
-        context 'user is not a guest' do
-          let(:user) { create(:project_member, :developer).user }
+          it { is_expected.to be_truthy }
+        end
 
-          it 'returns true' do
-            expect(user.using_gitlab_com_seat?(namespace)).to eq(true)
+        context 'when user is within project' do
+          let(:group) { create(:group) }
+          let(:namespace) { create(:project, namespace: group) }
+
+          before do
+            namespace.add_developer(user)
           end
+
+          it { is_expected.to be_truthy }
         end
 
-        context 'user is guest' do
-          let(:user) { create(:project_member, :guest).user }
+        context 'when user is within subgroup' do
+          let(:group) { create(:group) }
+          let(:namespace) { create(:group, parent: group) }
 
-          it 'returns false' do
-            expect(user.using_gitlab_com_seat?(namespace)).to eq(false)
+          before do
+            namespace.add_developer(user)
           end
+
+          it { is_expected.to be_truthy }
         end
       end
 
       context 'when namespace is on a plan that is not free or gold' do
-        it 'returns true' do
-          expect(user.using_gitlab_com_seat?(namespace)).to eq(true)
+        before do
+          create(:gitlab_subscription, namespace: namespace, hosted_plan: create(:silver_plan))
+        end
+
+        context 'user is a guest' do
+          before do
+            namespace.add_guest(user)
+          end
+
+          it { is_expected.to be_truthy }
+        end
+
+        context 'user is not a guest' do
+          before do
+            namespace.add_developer(user)
+          end
+
+          it { is_expected.to be_truthy }
         end
       end
     end