Merge branch '349811-feature-flag-for-free-user-cap' into 'master'

Add ability to check when free user cap setting reached on SaaS

See merge request gitlab-org/gitlab!80518

ee/app/models/ee/group.rb

@@ -342,12 +342,20 @@ module EE
       billable_ids[:user_ids].count
     end
 
+    override :free_plan_members_count
+    def free_plan_members_count
+      billable_ids = billed_user_ids_including_guests
+
+      billable_ids[:user_ids].count
+    end
+
     # For now, we are not billing for members with a Guest role for subscriptions
     # with a Gold/Ultimate plan. The other plans will treat Guest members as a regular member
     # for billing purposes.
     #
     # For the user_ids key, we are plucking the user_ids from the "Members" table in an array and
     # converting the array of user_ids to a Set which will have unique user_ids.
+    override :billed_user_ids
     def billed_user_ids(requested_hosted_plan = nil)
       exclude_guests = ([actual_plan_name, requested_hosted_plan] & [::Plan::GOLD, ::Plan::ULTIMATE, ::Plan::ULTIMATE_TRIAL]).any?
 
@@ -527,6 +535,11 @@ module EE
       user_cap <= members_count
     end
 
+    override :user_limit_reached?
+    def user_limit_reached?(use_cache: false)
+      super || user_cap_reached?(use_cache: use_cache)
+    end
+
     def shared_externally?
       strong_memoize(:shared_externally) do
         internal_groups = ::Group.groups_including_descendants_by([self])
@@ -687,10 +700,6 @@ module EE
       ::GroupMember.active_without_invites_and_requests.where(source_id: groups.self_and_ancestors)
     end
 
-    def users_without_project_bots(members)
-      ::User.where(id: members.distinct.select(:user_id)).without_project_bot
-    end
-
     override :_safe_read_repository_read_only_column
     def _safe_read_repository_read_only_column
       ::NamespaceSetting.where(namespace: self).pick(:repository_read_only)

ee/app/models/ee/member.rb

@@ -147,10 +147,10 @@ module EE
     end
 
     def set_membership_activation
-      return unless group
-      return if user && ::Member.in_hierarchy(group).with_user(user).with_state(:active).any?
+      return unless source.root_ancestor.apply_user_cap? # this guard is likely cheaper than doing the Member query all the time
+      return if user && ::Member.in_hierarchy(source).with_user(user).with_state(:active).any?
 
-      self.state = ::Member::STATE_AWAITING if group.user_cap_reached?
+      self.state = ::Member::STATE_AWAITING if source.root_ancestor.user_limit_reached?
     end
   end
 end

ee/app/models/ee/namespace.rb

@@ -338,6 +338,10 @@ module EE
       }
     end
 
+    def free_plan_members_count
+      free_plan_user_ids.count
+    end
+
     def eligible_for_trial?
       ::Gitlab.com? &&
         !has_parent? &&
@@ -484,8 +488,39 @@ module EE
       user_cap_available? || apply_free_user_cap?
     end
 
+    def free_user_cap_reached?
+      return false unless apply_free_user_cap?
+
+      members_count = root_ancestor.free_plan_members_count
+      return false unless members_count
+
+      ::Plan::FREE_USER_LIMIT <= members_count
+    end
+
+    def user_limit_reached?(use_cache: false)
+      free_user_cap_reached?
+    end
+
+    def free_plan_user_ids
+      strong_memoize(:free_plan_user_ids) do
+        billed_users.pluck(:id)
+      end
+    end
+
     private
 
+    # Members belonging directly to Projects within user/project namespaces
+    def billed_users
+      # this will include the namespace owner(user namespace) as well
+      members = ::ProjectMember.without_invites_and_requests.where(source_id: ::Project.in_namespace(self))
+
+      users_without_project_bots(members).with_state(:active)
+    end
+
+    def users_without_project_bots(members)
+      ::User.id_in(members.distinct.select(:user_id)).without_project_bot
+    end
+
     def any_project_with_shared_runners_enabled_with_cte?
       projects_query = if user_namespace?
                          projects

ee/app/models/ee/plan.rb

@@ -16,6 +16,8 @@ module EE
       PREMIUM_TRIAL = 'premium_trial'
       OPEN_SOURCE = 'opensource'
 
+      FREE_USER_LIMIT = 5
+
       EE_DEFAULT_PLANS = (const_get(:DEFAULT_PLANS, false) + [FREE]).freeze
       PAID_HOSTED_PLANS = [BRONZE, SILVER, PREMIUM, GOLD, ULTIMATE, ULTIMATE_TRIAL, PREMIUM_TRIAL, OPEN_SOURCE].freeze
       EE_ALL_PLANS = (EE_DEFAULT_PLANS + PAID_HOSTED_PLANS).freeze

ee/spec/models/ee/group_spec.rb

@@ -3,9 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Group do
+  using RSpec::Parameterized::TableSyntax
+
   let(:group) { create(:group) }
 
   it { is_expected.to include_module(EE::Group) }
+  it { is_expected.to be_kind_of(ReactiveCaching) }
 
   describe 'associations' do
     it { is_expected.to have_many(:audit_events).dependent(false) }
@@ -1605,6 +1608,55 @@ RSpec.describe Group do
     end
   end
 
+  describe '#calculate_reactive_cache' do
+    let(:group) { build(:group) }
+
+    subject { group.calculate_reactive_cache }
+
+    it 'returns cache data for the free plan members count' do
+      expect(group).to receive(:billable_members_count).and_return(5)
+
+      is_expected.to eq(5)
+    end
+  end
+
+  describe '#user_limit_reached?' do
+    where(:user_cap_reached, :free_user_cap_reached, :result) do
+      false | false | false
+      false | true  | true
+      true  | false | true
+      true  | true  | true
+    end
+
+    subject { group.user_limit_reached? }
+
+    with_them do
+      before do
+        allow(group).to receive(:user_cap_reached?).and_return(user_cap_reached)
+        allow(group).to receive(:free_user_cap_reached?).and_return(free_user_cap_reached)
+      end
+
+      it { is_expected.to eq(result) }
+    end
+  end
+
+  describe '#free_plan_members_count' do
+    let_it_be(:namespace) { create(:group) }
+    let_it_be(:owner) { create(:user) }
+    let_it_be(:project) { create(:project, group: namespace) }
+    let_it_be(:project_user) { create(:project_member, project: project).user }
+    let_it_be(:project_2) { create(:project, group: namespace) }
+    let_it_be(:project2_user) { create(:project_member, project: project_2).user }
+
+    before do
+      namespace.add_owner(owner)
+    end
+
+    it 'has the correct count' do
+      expect(namespace.free_plan_members_count).to eq 3
+    end
+  end
+
   describe '#shared_externally?' do
     let_it_be(:group, refind: true) { create(:group) }
     let_it_be(:subgroup_1) { create(:group, parent: group) }

ee/spec/models/gitlab_subscription_spec.rb

@@ -103,6 +103,11 @@ RSpec.describe GitlabSubscription, :saas do
         %w[bronze premium].each do |plan|
           it 'excludes these members' do
             gitlab_subscription.update!(plan_code: plan)
+            # plan has already memoized in ee/namespace.rb as `actual_plan`, so this then
+            # is not known at this point since `actual_plan` has already been set when
+            # `group.add_guest` in the before action, and was performed due to member set logic where we
+            # go through that path already
+            group.clear_memoization(:actual_plan)
 
             expect(gitlab_subscription.calculate_seats_in_use).to eq(1)
           end

ee/spec/models/member_spec.rb

@@ -275,6 +275,169 @@ RSpec.describe Member, type: :model do
     end
   end
 
+  context 'check if free user cap has been reached', :saas do
+    let_it_be(:group, refind: true) { create(:group_with_plan, plan: :free_plan) }
+    let_it_be(:subgroup) { create(:group, parent: group) }
+    let_it_be(:project, refind: true) { create(:project, namespace: group)}
+    let_it_be(:user) { create(:user) }
+
+    before_all do
+      group.add_developer(create(:user))
+    end
+
+    context 'when the :free_user_cap feature flag is disabled' do
+      before do
+        stub_feature_flags(free_user_cap: false)
+      end
+
+      it 'sets the group member state to active' do
+        group.add_developer(user)
+
+        expect(user.group_members.last).to be_active
+      end
+
+      it 'sets the project member state to active' do
+        project.add_developer(user)
+
+        expect(user.project_members.last).to be_active
+      end
+    end
+
+    context 'when the :free_user_cap feature flag is enabled' do
+      before do
+        stub_feature_flags(free_user_cap: true)
+      end
+
+      context 'when the free user cap has not been reached' do
+        it 'sets the group member to active' do
+          group.add_developer(user)
+
+          expect(user.group_members.last).to be_active
+        end
+
+        it 'sets the project member to active' do
+          project.add_developer(user)
+
+          expect(user.project_members.last).to be_active
+        end
+
+        context 'when user is added to a group-less project' do
+          let(:project) do
+            project = create(:project)
+            namespace = project.namespace
+            create(:gitlab_subscription, hosted_plan: create(:free_plan), namespace: namespace)
+            project
+          end
+
+          it 'adds project member and leaves the state to active' do
+            project.root_ancestor.clear_memoization(:existing_free_plan)
+            project.add_developer(create(:user))
+            project.add_developer(user)
+
+            expect(user.project_members.last).to be_active
+          end
+        end
+      end
+
+      context 'when the free user cap has been reached' do
+        before do
+          stub_const('::Plan::FREE_USER_LIMIT', 1)
+        end
+
+        it 'sets the group member to awaiting' do
+          group.add_developer(user)
+
+          expect(user.group_members.last).to be_awaiting
+        end
+
+        it 'sets the group member to awaiting when added to a subgroup' do
+          subgroup.add_developer(user)
+
+          expect(user.group_members.last).to be_awaiting
+        end
+
+        it 'sets the project member to awaiting' do
+          project.add_developer(user)
+
+          expect(user.project_members.last).to be_awaiting
+        end
+
+        context 'when multiple members are added' do
+          before do
+            stub_const('::Plan::FREE_USER_LIMIT', 2)
+          end
+
+          it 'sets members to the correct status' do
+            over_limit_user = create(:user)
+            project.root_namespace.clear_memoization(:billed_user_ids_including_guests)
+            project.add_developer(user)
+            project.root_namespace.clear_memoization(:billed_user_ids_including_guests)
+            project.add_developer(over_limit_user)
+
+            expect(user.project_members.last).to be_active
+            expect(over_limit_user.project_members.last).to be_awaiting
+          end
+        end
+
+        context 'when the user is already an active root group member' do
+          it 'sets the group member to active' do
+            create(:group_member, :active, group: group, user: user)
+
+            subgroup.add_owner(user)
+
+            expect(user.group_members.last).to be_active
+          end
+        end
+
+        context 'when the user is already an active subgroup member' do
+          it 'sets the group member to active' do
+            other_subgroup = create(:group, parent: group)
+            create(:group_member, :active, group: other_subgroup, user: user)
+
+            subgroup.add_developer(user)
+
+            expect(user.group_members.last).to be_active
+          end
+        end
+
+        context 'when the user is already an active project member' do
+          it 'sets the group member to active' do
+            create(:project_member, :active, project: project, user: user)
+
+            expect { subgroup.add_owner(user) }.to change { ::Member.with_state(:active).count }.by(1)
+            expect(user.group_members.last).to be_active
+          end
+        end
+
+        context 'when user is added to a group-less project' do
+          let(:project) do
+            project = create(:project)
+            namespace = project.namespace
+            create(:gitlab_subscription, hosted_plan: create(:free_plan), namespace: namespace)
+            project
+          end
+
+          before do
+            stub_const('::Plan::FREE_USER_LIMIT', 2)
+          end
+
+          it 'adds multiple members and correctly shows the state' do
+            project.root_ancestor.clear_memoization(:has_free_or_no_subscription)
+            over_limit_user = create(:user)
+
+            project.root_ancestor.clear_memoization(:free_plan_user_ids)
+            project.add_developer(user)
+            project.root_ancestor.clear_memoization(:free_plan_user_ids)
+            project.add_developer(over_limit_user)
+
+            expect(user.project_members.last).to be_active
+            expect(over_limit_user.project_members.last).to be_awaiting
+          end
+        end
+      end
+    end
+  end
+
   describe '.distinct_awaiting_or_invited_for_group' do
     let_it_be(:other_sub_group) { create(:group, parent: group) }
     let_it_be(:active_group_member) { create(:group_member, group: group) }

ee/spec/services/ci/create_pipeline_service_spec.rb

@@ -193,6 +193,9 @@ RSpec.describe Ci::CreatePipelineService, '#execute', :saas do
         before do
           allow(::Gitlab).to receive(:com?).and_return(true)
           namespace.gitlab_subscription.update!(hosted_plan: create(:free_plan))
+          # namespace gitlab_subscription is cached when source.root_ancestor.user_limit_reached?
+          # is called on member creation callback from add_developer call
+          project.root_ancestor.gitlab_subscription.reset
           user.created_at = ::Users::CreditCardValidation::RELEASE_DAY
         end
 

ee/spec/services/ee/members/create_service_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe Members::CreateService do
     }
   end
 
-  subject { described_class.new(user, params.merge({ source: project })).execute }
+  subject(:execute_service) { described_class.new(user, params.merge({ source: project })).execute }
 
   before_all do
     project.add_maintainer(user)
@@ -41,8 +41,8 @@ RSpec.describe Members::CreateService do
     end
 
     shared_examples 'quota limit exceeded' do |limit|
-      it { expect(subject).to include(status: :error, message: "Invite limit of #{limit} per day exceeded") }
-      it { expect { subject }.not_to change { Member.count } }
+      it { expect(execute_service).to include(status: :error, message: "Invite limit of #{limit} per day exceeded") }
+      it { expect { execute_service }.not_to change { Member.count } }
     end
 
     context 'already exceeded invite quota limit' do
@@ -60,9 +60,11 @@ RSpec.describe Members::CreateService do
     context 'within invite quota limit' do
       let(:daily_invites) { 5 }
 
-      it { expect(subject).to eq({ status: :success }) }
+      it { expect(execute_service).to eq({ status: :success }) }
+
       it do
-        subject
+        execute_service
+
         expect(project.users).to include(*project_users)
       end
     end
@@ -71,8 +73,10 @@ RSpec.describe Members::CreateService do
       let(:daily_invites) { 0 }
 
       it { expect(subject).to eq({ status: :success }) }
+
       it do
-        subject
+        execute_service
+
         expect(project.users).to include(*project_users)
       end
     end
@@ -81,9 +85,11 @@ RSpec.describe Members::CreateService do
   context 'without a plan' do
     let(:plan) { nil }
 
-    it { expect(subject).to eq({ status: :success }) }
+    it { expect(execute_service).to eq({ status: :success }) }
+
     it do
-      subject
+      execute_service
+
       expect(project.users).to include(*project_users)
     end
   end
@@ -107,7 +113,7 @@ RSpec.describe Members::CreateService do
           .once
           .and_call_original
 
-        expect { subject }.to change { project.issues.reload.count }.by(2)
+        expect { execute_service }.to change { project.issues.reload.count }.by(2)
 
         expect(project.issues).to all have_attributes(
           project: project,
@@ -117,4 +123,39 @@ RSpec.describe Members::CreateService do
       end
     end
   end
+
+  context 'when reaching the free user cap limit', :saas do
+    let_it_be(:project_user) { project_users.first }
+    let_it_be(:over_limit_user) { project_users.last }
+
+    before do
+      stub_const('::Plan::FREE_USER_LIMIT', 3)
+    end
+
+    context 'with a group-less project' do
+      let_it_be(:project) { create(:project) }
+
+      before do
+        project.add_maintainer(user)
+      end
+
+      it 'sets members to the correct status' do
+        expect(execute_service[:status]).to eq(:success)
+        expect(project_user.project_members.last).to be_active
+        expect(over_limit_user.project_members.last).to be_awaiting
+      end
+    end
+
+    context 'with a group project' do
+      before do
+        project.add_developer(create(:user))
+      end
+
+      it 'sets members to the correct status' do
+        expect(execute_service[:status]).to eq(:success)
+        expect(project_user.project_members.last).to be_active
+        expect(over_limit_user.project_members.last).to be_awaiting
+      end
+    end
+  end
 end

spec/requests/api/invitations_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe API::Invitations do
   let(:email) { 'email1@example.com' }
   let(:email2) { 'email2@example.com' }
 
-  let_it_be(:project) do
+  let_it_be(:project, reload: true) do
     create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project|
       project.add_developer(developer)
       project.add_maintainer(maintainer)

spec/requests/api/project_import_spec.rb

@@ -47,7 +47,7 @@ RSpec.describe API::ProjectImport, :aggregate_failures do
     it 'executes a limited number of queries' do
       control_count = ActiveRecord::QueryRecorder.new { subject }.count
 
-      expect(control_count).to be <= 104
+      expect(control_count).to be <= 105
     end
 
     it 'schedules an import using a namespace' do

spec/services/notification_service_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe NotificationService, :mailer do
   include ExternalAuthorizationServiceHelpers
   include NotificationHelpers
 
-  let_it_be_with_refind(:project) { create(:project, :public) }
+  let_it_be_with_refind(:project, reload: true) { create(:project, :public) }
   let_it_be_with_refind(:assignee) { create(:user) }
 
   let(:notification) { described_class.new }