diff --git a/app/helpers/license_helper.rb b/app/helpers/license_helper.rb index e94d8c0ca32a7094dfe8578edbc6a75190590b5c..d1cf1bf3a618ec4d3434d268e4a6a18393068408 100644 --- a/app/helpers/license_helper.rb +++ b/app/helpers/license_helper.rb @@ -5,7 +5,7 @@ module LicenseHelper delegate :new_admin_license_path, to: 'Gitlab::Routing.url_helpers' def current_active_user_count - User.active.count + User.real.count end def max_historical_user_count diff --git a/app/models/historical_data.rb b/app/models/historical_data.rb index 7dc2e127bbd39895c98e11a1d3926788b16738bd..a106abc1b3922cecbcd0735d67adf55702f753c6 100644 --- a/app/models/historical_data.rb +++ b/app/models/historical_data.rb @@ -10,7 +10,7 @@ class << self def track! create!( date: Date.today, - active_user_count: User.active.count + active_user_count: User.real.count ) end diff --git a/app/models/license.rb b/app/models/license.rb index f20a8df41b287072800e9e1f59b1e945337ef59e..9c47e7f171160006ae50afdffcee8680b8d313d5 100644 --- a/app/models/license.rb +++ b/app/models/license.rb @@ -100,7 +100,7 @@ def previous_user_count end def current_active_users_count - @current_active_users_count ||= User.active.count + @current_active_users_count ||= User.real.count end def validate_with_trueup? diff --git a/app/models/user.rb b/app/models/user.rb index 2df03f1c5ef3d721583c106be5ee9868ceefa56e..fcfa477af2cb3958fa78a917ed107bcc830fbb8f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -210,6 +210,7 @@ def inactive_message scope :blocked, -> { with_states(:blocked, :ldap_blocked) } scope :external, -> { where(external: true) } scope :active, -> { with_state(:active) } + scope :real, -> { active.non_internal } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members WHERE user_id IS NOT NULL AND requested_at IS NULL)') } scope :subscribed_for_admin_email, -> { where(admin_email_unsubscribed_at: nil) } diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index c422bff1c1dcacff2a34e5b5012df04d9ea126bf..632a31dde3dc547e6e6876f585334f58dad84127 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -41,7 +41,7 @@ %p Active Users %span.light.pull-right - = number_with_delimiter(User.active.count) + = number_with_delimiter(User.real.count) .col-md-4 %h4 Features diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index 9aa8eeaf2568310b0731b5c3c44165ac57019a89..b910e379aaa801e3ae21c2cfc3a347de6cb0a233 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -42,7 +42,7 @@ = nav_link(html_options: { class: active_when(params[:filter].nil?) }) do = link_to admin_users_path do Active - %small.badge= number_with_delimiter(User.active.count) + %small.badge= number_with_delimiter(User.real.count) = nav_link(html_options: { class: active_when(params[:filter] == 'admins') }) do = link_to admin_users_path(filter: "admins") do Admins diff --git a/changelogs/unreleased-ee/fix-user-count.yml b/changelogs/unreleased-ee/fix-user-count.yml new file mode 100644 index 0000000000000000000000000000000000000000..bfac4a9b26940275253ca56c3a89d7ec2297a026 --- /dev/null +++ b/changelogs/unreleased-ee/fix-user-count.yml @@ -0,0 +1,4 @@ +--- +title: Fix user count to ignore non internal users +merge_request: +author: diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ff02f5588d17fc62ab30974f6955032bf7d59088..cb427b8961b85af386b01f45216da7d7a1c9cc86 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -662,7 +662,7 @@ class License < Grape::Entity end expose :active_users do |license, options| - ::User.active.count + ::User.real.count end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 51bc4c4285e69ebc392ba3c288b577ee3b01dd73..db1b3d65c7dc5695e4020dfc39f1faf521db7c55 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -69,7 +69,7 @@ def service_desk_counts def license_usage_data usage_data = { uuid: current_application_settings.uuid, version: Gitlab::VERSION, - active_user_count: User.active.count, + active_user_count: User.real.count, mattermost_enabled: Gitlab.config.mattermost.enabled } license = ::License.current diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 3ab18f4dda3c99c8607d75bbeab4541098e7007c..e1a84fa62c3667ebdde5e868a1faff7da4fd1b3b 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -78,7 +78,7 @@ expect(subject[:license_md5]).to eq(Digest::MD5.hexdigest(license.data)) expect(subject[:version]).to eq(Gitlab::VERSION) expect(subject[:licensee]).to eq(license.licensee) - expect(subject[:active_user_count]).to eq(User.active.count) + expect(subject[:active_user_count]).to eq(User.real.count) expect(subject[:licensee]).to eq(license.licensee) expect(subject[:license_user_count]).to eq(license.restricted_user_count) expect(subject[:license_starts_at]).to eq(license.starts_at) diff --git a/spec/models/license_spec.rb b/spec/models/license_spec.rb index ba8c0845e0089137cee638a49d6af851c7b68455..8b00d1af65a2bd351e9869b4bd81b5b7c10778cf 100644 --- a/spec/models/license_spec.rb +++ b/spec/models/license_spec.rb @@ -24,9 +24,9 @@ end describe "Historical active user count" do - let(:active_user_count) { User.active.count + 10 } + let(:real_user_count) { User.real.count + 10 } let(:date) { License.current.starts_at } - let!(:historical_data) { HistoricalData.create!(date: date, active_user_count: active_user_count) } + let!(:historical_data) { HistoricalData.create!(date: date, real_user_count: real_user_count) } context "when there is no active user count restriction" do it "is valid" do @@ -36,7 +36,7 @@ context "when the active user count restriction is exceeded" do before do - gl_license.restrictions = { active_user_count: active_user_count - 1 } + gl_license.restrictions = { real_user_count: real_user_count - 1 } end context "when the license started" do @@ -72,7 +72,7 @@ context "when the active user count restriction is not exceeded" do before do - gl_license.restrictions = { active_user_count: active_user_count + 1 } + gl_license.restrictions = { real_user_count: real_user_count + 1 } end it "is valid" do @@ -82,8 +82,8 @@ context "when the active user count is met exactly" do it "is valid" do - active_user_count = 100 - gl_license.restrictions = { active_user_count: active_user_count } + real_user_count = 100 + gl_license.restrictions = { real_user_count: real_user_count } expect(license).to be_valid end @@ -179,7 +179,7 @@ describe 'downgrade' do context 'when more users were added in previous period' do before do - HistoricalData.create!(date: 6.months.ago, active_user_count: 15) + HistoricalData.create!(date: 6.months.ago, real_user_count: 15) set_restrictions(restricted_user_count: 5, previous_user_count: 10) end @@ -191,7 +191,7 @@ context 'when no users were added in the previous period' do before do - HistoricalData.create!(date: 6.months.ago, active_user_count: 15) + HistoricalData.create!(date: 6.months.ago, real_user_count: 15) set_restrictions(restricted_user_count: 10, previous_user_count: 15) end @@ -347,7 +347,7 @@ def build_license_with_add_ons(add_ons) def set_restrictions(opts) gl_license.restrictions = { - active_user_count: opts[:restricted_user_count], + real_user_count: opts[:restricted_user_count], previous_user_count: opts[:previous_user_count], trueup_quantity: opts[:trueup_quantity], trueup_from: (Date.today - 1.year).to_s, diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c0f5c44708601de26f6116198ca3e7f105b20776..d2ba2dc223c1a3f48c5e6fa6953f1df0b22fcd97 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1846,4 +1846,16 @@ def add_user(access) end end end + + context '.real' do + before do + User.ghost + create(:user, name: 'user', state: 'active') + create(:user, name: 'user', state: 'blocked') + end + + it 'only counts active and non internal users' do + expect(User.real.count).to eq(1) + end + end end