Commit dd49c16a authored by Tyler Amos's avatar Tyler Amos

Exclude bots from current active users count query

- Excluding bots from the current active users count logic in License
model.
- Introduces a new scope `without_bots`.
- Adds the method `current_active_users` to License for easy reuse.
- Update references across the app to use `current_active_users`
instead of `User.active.count`.
- Updates documentation accordingly.
parent 2aa36c4f
...@@ -21,6 +21,7 @@ module HasUserType ...@@ -21,6 +21,7 @@ module HasUserType
included do included do
scope :humans, -> { where(user_type: :human) } scope :humans, -> { where(user_type: :human) }
scope :bots, -> { where(user_type: BOT_USER_TYPES) } 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 :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 :non_internal, -> { humans.or(where(user_type: NON_INTERNAL_USER_TYPES)) }
scope :without_ghosts, -> { humans.or(where.not(user_type: :ghost)) } scope :without_ghosts, -> { humans.or(where.not(user_type: :ghost)) }
......
...@@ -411,6 +411,9 @@ user.skip_reconfirmation! ...@@ -411,6 +411,9 @@ user.skip_reconfirmation!
# Active users on the instance, now # Active users on the instance, now
User.active.count 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 # The historical max on the instance as of the past year
::HistoricalData.max_historical_user_count ::HistoricalData.max_historical_user_count
``` ```
......
...@@ -39,8 +39,7 @@ subscription according to the maximum number of users enabled at once. You can ...@@ -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 add and remove users during the subscription period, as long as the total users
at any given time is within your subscription count. at any given time is within your subscription count.
Every occupied seat, whether by person, job, or bot is counted in the subscription, Every occupied seat is counted in the subscription, with the following exception:
with the following exception:
- Members with Guest permissions on a Gold subscription. - Members with Guest permissions on a Gold subscription.
......
...@@ -23,15 +23,14 @@ For instances that aren't offline or on a closed network, the maximum number of ...@@ -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, simultaneous users in the self-managed installation is checked each quarter,
using [Seat Link](#seat-link). using [Seat Link](#seat-link).
Every occupied seat, whether by person, job, or bot is counted in the subscription, Every occupied seat is counted in the subscription, with the following exceptions:
with the following exceptions:
- [Deactivated](../../user/admin_area/activating_deactivating_users.md#deactivating-a-user) and - [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 [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 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. count as active users in the subscription period in which they were originally added.
- Members with Guest permissions on an Ultimate subscription. - 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 ### Users statistics
......
...@@ -53,8 +53,7 @@ Furthermore, the bot user can not be added to any other project. ...@@ -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). 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. 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.
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.
## Revoking a project access token ## Revoking a project access token
......
...@@ -54,6 +54,6 @@ module LicenseHelper ...@@ -54,6 +54,6 @@ module LicenseHelper
private private
def active_user_count def active_user_count
User.active.count License.current_active_users.count
end end
end end
...@@ -302,6 +302,10 @@ class License < ApplicationRecord ...@@ -302,6 +302,10 @@ class License < ApplicationRecord
yield(current_license) if block_given? yield(current_license) if block_given?
end end
def current_active_users
User.active.without_bots
end
private private
def load_future_dated def load_future_dated
...@@ -414,11 +418,9 @@ class License < ApplicationRecord ...@@ -414,11 +418,9 @@ class License < ApplicationRecord
def current_active_users_count def current_active_users_count
@current_active_users_count ||= begin @current_active_users_count ||= begin
if exclude_guests_from_active_count? scope = self.class.current_active_users
User.active.excluding_guests.count scope = scope.excluding_guests if exclude_guests_from_active_count?
else scope.count
User.active.count
end
end end
end end
......
---
title: Exclude bots from licensed user count
merge_request: 42034
author:
type: fixed
...@@ -5,7 +5,7 @@ module EE ...@@ -5,7 +5,7 @@ module EE
module Entities module Entities
class GitlabLicenseWithActiveUsers < GitlabLicense class GitlabLicenseWithActiveUsers < GitlabLicense
expose :active_users do |license, options| expose :active_users do |license, options|
::User.active.count ::License.current_active_users.count
end end
end end
end end
......
...@@ -24,7 +24,7 @@ RSpec.describe LicenseHelper do ...@@ -24,7 +24,7 @@ RSpec.describe LicenseHelper do
it 'returns the number of active users' do it 'returns the number of active users' do
allow(License).to receive(:current).and_return(nil) 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 end
end end
......
...@@ -29,7 +29,7 @@ RSpec.describe HistoricalData do ...@@ -29,7 +29,7 @@ RSpec.describe HistoricalData do
describe ".track!" do describe ".track!" do
before 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 end
it "creates a new historical data record" do it "creates a new historical data record" do
......
...@@ -66,7 +66,7 @@ RSpec.describe License do ...@@ -66,7 +66,7 @@ RSpec.describe License do
end end
describe "Historical active user count" do 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(:date) { described_class.current.starts_at }
let!(:historical_data) { HistoricalData.create!(date: date, active_user_count: active_user_count) } let!(:historical_data) { HistoricalData.create!(date: date, active_user_count: active_user_count) }
...@@ -82,7 +82,7 @@ RSpec.describe License do ...@@ -82,7 +82,7 @@ RSpec.describe License do
gl_license.restrictions = { gl_license.restrictions = {
previous_user_count: 1, previous_user_count: 1,
active_user_count: User.active.count - 1 active_user_count: described_class.current_active_users.count - 1
} }
HistoricalData.delete_all HistoricalData.delete_all
...@@ -583,6 +583,21 @@ RSpec.describe License do ...@@ -583,6 +583,21 @@ RSpec.describe License do
end end
end 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 end
describe "#md5" do describe "#md5" do
...@@ -747,6 +762,34 @@ RSpec.describe License do ...@@ -747,6 +762,34 @@ RSpec.describe License do
end end
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 describe '#overage' do
it 'returns 0 if restricted_user_count is nil' do it 'returns 0 if restricted_user_count is nil' do
allow(license).to receive(:restricted_user_count) { nil } allow(license).to receive(:restricted_user_count) { nil }
......
...@@ -31,6 +31,12 @@ RSpec.describe User do ...@@ -31,6 +31,12 @@ RSpec.describe User do
end end
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 describe '.bots_without_project_bot' do
it 'includes all bots except project_bot' do it 'includes all bots except project_bot' do
expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot]) expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot])
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment