Merge branch '223695-exclude-bots-from-licensed-users' into 'master'

Exclude bots from licensed user count

See merge request gitlab-org/gitlab!42034

app/models/concerns/has_user_type.rb

@@ -21,6 +21,7 @@ module HasUserType
   included do
     scope :humans, -> { where(user_type: :human) }
     scope :bots, -> { where(user_type: BOT_USER_TYPES) }
+    scope :without_bots, -> { humans.or(where.not(user_type: BOT_USER_TYPES)) }
     scope :bots_without_project_bot, -> { where(user_type: BOT_USER_TYPES - ['project_bot']) }
     scope :non_internal, -> { humans.or(where(user_type: NON_INTERNAL_USER_TYPES)) }
     scope :without_ghosts, -> { humans.or(where.not(user_type: :ghost)) }

doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md

@@ -411,6 +411,9 @@ user.skip_reconfirmation!
 # Active users on the instance, now
 User.active.count
 
+# Users taking a seat on the instance
+License.current.current_active_users_count
+
 # The historical max on the instance as of the past year
 ::HistoricalData.max_historical_user_count
 ```

doc/subscriptions/gitlab_com/index.md

@@ -39,8 +39,7 @@ subscription according to the maximum number of users enabled at once. You can
 add and remove users during the subscription period, as long as the total users
 at any given time is within your subscription count.
 
-Every occupied seat, whether by person, job, or bot is counted in the subscription,
-with the following exception:
+Every occupied seat is counted in the subscription, with the following exception:
 
 - Members with Guest permissions on a Gold subscription.
 

doc/subscriptions/self_managed/index.md

@@ -23,15 +23,14 @@ For instances that aren't offline or on a closed network, the maximum number of
 simultaneous users in the self-managed installation is checked each quarter,
 using [Seat Link](#seat-link).
 
-Every occupied seat, whether by person, job, or bot is counted in the subscription,
-with the following exceptions:
+Every occupied seat is counted in the subscription, with the following exceptions:
 
 - [Deactivated](../../user/admin_area/activating_deactivating_users.md#deactivating-a-user) and
   [blocked](../../user/admin_area/blocking_unblocking_users.md) users who are restricted prior to the
   renewal of a subscription won't be counted as active users for the renewal subscription. They may
   count as active users in the subscription period in which they were originally added.
 - Members with Guest permissions on an Ultimate subscription.
-- GitLab-created service accounts: `Ghost User`, `Support Bot` and [`Project bot users`](../../user/project/settings/project_access_tokens.md#project-bot-users).
+- GitLab-created service accounts: `Ghost User` and bots (`Support Bot`, [`Project bot users`](../../user/project/settings/project_access_tokens.md#project-bot-users), etc.).
 
 ### Users statistics
 

doc/user/project/settings/project_access_tokens.md

@@ -53,8 +53,7 @@ Furthermore, the bot user can not be added to any other project.
 
 When the project access token is [revoked](#revoking-a-project-access-token) the bot user is then deleted and all records are moved to a system-wide user with the username "Ghost User". For more information, see [Associated Records](../../profile/account/delete_account.md#associated-records).
 
-Project bot users are a [GitLab-created service account](../../../subscriptions/self_managed/index.md#choose-the-number-of-users), but count as a licensed seat.
-These users will not count against your licensed seat in the future when [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/223695) is resolved.
+Project bot users are [GitLab-created service accounts](../../../subscriptions/self_managed/index.md#choose-the-number-of-users) and do not count as licensed seats.
 
 ## Revoking a project access token
 

ee/app/helpers/license_helper.rb

@@ -54,6 +54,6 @@ module LicenseHelper
   private
 
   def active_user_count
-    User.active.count
+    License.current_active_users.count
   end
 end

ee/app/models/license.rb

@@ -302,6 +302,10 @@ class License < ApplicationRecord
       yield(current_license) if block_given?
     end
 
+    def current_active_users
+      User.active.without_bots
+    end
+
     private
 
     def load_future_dated
@@ -414,11 +418,9 @@ class License < ApplicationRecord
 
   def current_active_users_count
     @current_active_users_count ||= begin
-      if exclude_guests_from_active_count?
-        User.active.excluding_guests.count
-      else
-        User.active.count
-      end
+      scope = self.class.current_active_users
+      scope = scope.excluding_guests if exclude_guests_from_active_count?
+      scope.count
     end
   end
 

ee/changelogs/unreleased/223695-exclude-bots-from-licensed-users.yml

+---
+title: Exclude bots from licensed user count
+merge_request: 42034
+author:
+type: fixed

ee/lib/ee/api/entities/gitlab_license_with_active_users.rb

@@ -5,7 +5,7 @@ module EE
     module Entities
       class GitlabLicenseWithActiveUsers < GitlabLicense
         expose :active_users do |license, options|
-          ::User.active.count
+          ::License.current_active_users.count
         end
       end
     end

ee/spec/helpers/license_helper_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe LicenseHelper do
       it 'returns the number of active users' do
         allow(License).to receive(:current).and_return(nil)
 
-        expect(current_active_user_count).to eq(User.active.count)
+        expect(current_active_user_count).to eq(License.current_active_users.count)
       end
     end
   end

ee/spec/models/historical_data_spec.rb

@@ -29,7 +29,7 @@ RSpec.describe HistoricalData do
 
   describe ".track!" do
     before do
-      allow(User).to receive(:active).and_return([1, 2, 3, 4, 5])
+      allow(License).to receive(:current_active_users).and_return([1, 2, 3, 4, 5])
     end
 
     it "creates a new historical data record" do

ee/spec/models/license_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe License do
     end
 
     describe "Historical active user count" do
-      let(:active_user_count) { User.active.count + 10 }
+      let(:active_user_count) { described_class.current_active_users.count + 10 }
       let(:date)              { described_class.current.starts_at }
       let!(:historical_data)  { HistoricalData.create!(date: date, active_user_count: active_user_count) }
 
@@ -82,7 +82,7 @@ RSpec.describe License do
 
           gl_license.restrictions = {
             previous_user_count: 1,
-            active_user_count: User.active.count - 1
+            active_user_count: described_class.current_active_users.count - 1
           }
 
           HistoricalData.delete_all
@@ -583,6 +583,21 @@ RSpec.describe License do
         end
       end
     end
+
+    describe '.current_active_users' do
+      before do
+        create(:group_member)
+        create(:group_member, user: create(:admin))
+        create(:group_member, user: create(:user, :bot))
+        create(:group_member, user: create(:user, :project_bot))
+        create(:group_member, user: create(:user, :ghost))
+        create(:group_member).user.deactivate!
+      end
+
+      it 'includes guests in the count' do
+        expect(license.current_active_users_count).to eq(2)
+      end
+    end
   end
 
   describe "#md5" do
@@ -747,6 +762,34 @@ RSpec.describe License do
     end
   end
 
+  describe '#current_active_users_count' do
+    before_all do
+      create(:group_member)
+      create(:group_member, user: create(:admin))
+      create(:group_member, :guest)
+      create(:group_member, user: create(:user, :bot))
+      create(:group_member, user: create(:user, :project_bot))
+      create(:group_member, user: create(:user, :ghost))
+      create(:group_member).user.deactivate!
+    end
+
+    context 'when license is not for Ultimate plan' do
+      it 'includes guests in the count' do
+        expect(license.current_active_users_count).to eq(3)
+      end
+    end
+
+    context 'when license is for Ultimate plan' do
+      before do
+        allow(license).to receive(:plan).and_return(License::ULTIMATE_PLAN)
+      end
+
+      it 'excludes guests in the count' do
+        expect(license.current_active_users_count).to eq(2)
+      end
+    end
+  end
+
   describe '#overage' do
     it 'returns 0 if restricted_user_count is nil' do
       allow(license).to receive(:restricted_user_count) { nil }

spec/models/concerns/has_user_type_spec.rb

@@ -31,6 +31,12 @@ RSpec.describe User do
       end
     end
 
+    describe '.without_bots' do
+      it 'includes everyone except bots' do
+        expect(described_class.without_bots).to match_array(everyone - bots)
+      end
+    end
+
     describe '.bots_without_project_bot' do
       it 'includes all bots except project_bot' do
         expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot])