From 3e3f15729bf7119eabddb17970a53c2b0046b8dc Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 17 Apr 2017 16:06:11 +0200 Subject: [PATCH 1/2] add specs --- spec/lib/gitlab/usage_data_spec.rb | 2 +- spec/models/license_spec.rb | 18 +++++++++--------- spec/models/user_spec.rb | 12 ++++++++++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 3ab18f4dda3c99..e1a84fa62c3667 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 ba8c0845e00891..8b00d1af65a2bd 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 c0f5c44708601d..d2ba2dc223c1a3 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 -- GitLab From 31ed483d5a49ae93c4ef131cd6864afa0d8652c1 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Mon, 17 Apr 2017 16:13:34 +0200 Subject: [PATCH 2/2] fix user count --- app/helpers/license_helper.rb | 2 +- app/models/historical_data.rb | 2 +- app/models/license.rb | 2 +- app/models/user.rb | 1 + app/views/admin/dashboard/index.html.haml | 2 +- app/views/admin/users/index.html.haml | 2 +- changelogs/unreleased-ee/fix-user-count.yml | 4 ++++ lib/api/entities.rb | 2 +- lib/gitlab/usage_data.rb | 2 +- 9 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased-ee/fix-user-count.yml diff --git a/app/helpers/license_helper.rb b/app/helpers/license_helper.rb index e94d8c0ca32a70..d1cf1bf3a618ec 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 7dc2e127bbd398..a106abc1b3922c 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 f20a8df41b2870..9c47e7f1711600 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 2df03f1c5ef3d7..fcfa477af2cb39 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 c422bff1c1dcac..632a31dde3dc54 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 9aa8eeaf256831..b910e379aaa801 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 00000000000000..bfac4a9b269402 --- /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 ff02f5588d17fc..cb427b8961b85a 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 51bc4c4285e69e..db1b3d65c7dc56 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 -- GitLab