Merge branch 'change-max-user-calculation' into 'master'

Display Maximum Users in Admin Dashboard for Any License Duration

See merge request gitlab-org/gitlab!49861

ee/app/models/license.rb

@@ -473,7 +473,7 @@ class License < ApplicationRecord
   end
 
   def overage_with_historical_max
-    overage(historical_max_with_default_period)
+    overage(maximum_user_count)
   end
 
   def historical_max(from = nil, to = nil)
@@ -481,12 +481,7 @@ class License < ApplicationRecord
   end
 
   def maximum_user_count
-    [historical_max, daily_billable_users_count].max
-  end
-
-  def historical_max_with_default_period
-    @historical_max_with_default_period ||=
-      historical_max
+    [historical_max(starts_at), daily_billable_users_count].max
   end
 
   def update_trial_setting

ee/changelogs/unreleased/change-max-user-calculation.yml

+---
+title: Display correct maximum users in admin dashboard for license of any duration
+merge_request: 49861
+author:
+type: changed

ee/spec/helpers/license_monitoring_helper_spec.rb

@@ -108,28 +108,45 @@ RSpec.describe LicenseMonitoringHelper do
   end
 
   describe '#users_over_license' do
-    context 'with an user overage' do
-      let(:license) { build(:license) }
+    let(:now) { Date.current }
 
-      before do
-        allow(helper).to receive(:license_is_over_capacity?).and_return true
+    def setup_license(starts_at:, expires_at:, max_users:)
+      license = build(:license,
+                      starts_at: starts_at,
+                      expires_at: expires_at,
+                      restrictions: { active_user_count: max_users })
       allow(License).to receive(:current).and_return(license)
-        allow(license).to receive(:overage_with_historical_max) { 5 }
-      end
 
-      it 'shows overage as a number' do
-        expect(helper.users_over_license).to eq 5
+      license
     end
+
+    it 'shows overage as a number when there is an overage' do
+      license = setup_license(starts_at: now - 3.months, expires_at: now + 9.months, max_users: 10)
+      create(:historical_data, recorded_at: license.starts_at + 1.month, active_user_count: 15)
+
+      expect(helper.users_over_license).to eq(5)
     end
 
-    context 'without an user overage' do
-      before do
-        allow(helper).to receive(:license_is_over_capacity?).and_return false
+    it 'shows overage as a number when there is not an overage' do
+      setup_license(starts_at: now - 3.months, expires_at: now + 9.months, max_users: 10)
+
+      expect(helper.users_over_license).to eq(0)
     end
 
-      it 'shows overage as a number' do
-        expect(helper.users_over_license).to eq 0
+    it 'reports overage for a license of 6 months in duration' do
+      license = setup_license(starts_at: now - 3.months, expires_at: now + 3.months, max_users: 15)
+      create(:historical_data, recorded_at: license.starts_at - 2.months, active_user_count: 45)
+      create(:historical_data, recorded_at: license.starts_at + 1.month, active_user_count: 25)
+
+      expect(helper.users_over_license).to eq(10)
     end
+
+    it 'reports overage when the most recent billable user count is higher than the historical max active users' do
+      license = setup_license(starts_at: now - 3.months, expires_at: now + 9.months, max_users: 40)
+      create(:historical_data, recorded_at: license.expires_at - 2.months, active_user_count: 45)
+      create(:instance_statistics_measurement, recorded_at: now - 1.day, identifier: :billable_users, count: 70)
+
+      expect(helper.users_over_license).to eq(30)
     end
   end
 end

ee/spec/models/license_spec.rb

@@ -855,21 +855,91 @@ RSpec.describe License do
   end
 
   describe '#maximum_user_count' do
-    subject { license.maximum_user_count }
+    let(:now) { Date.current }
 
-    where(:daily_billable_users_count, :historical_max, :expected) do
-      100 | 50  | 100
-      50  | 100 | 100
-      50  | 50  | 50
+    it 'returns zero when there is no data' do
+      expect(license.maximum_user_count).to eq(0)
     end
 
-    with_them do
-      before do
-        allow(license).to receive(:daily_billable_users_count) { daily_billable_users_count }
-        allow(license).to receive(:historical_max) { historical_max }
+    it 'returns historical data' do
+      create(:historical_data, active_user_count: 1)
+
+      expect(license.maximum_user_count).to eq(1)
     end
 
-      it { is_expected.to eq(expected) }
+    it 'returns the billable users count' do
+      create(:instance_statistics_measurement, identifier: :billable_users, count: 2)
+
+      expect(license.maximum_user_count).to eq(2)
+    end
+
+    it 'returns the daily billable users count when it is higher than historical data' do
+      create(:historical_data, active_user_count: 50)
+      create(:instance_statistics_measurement, identifier: :billable_users, count: 100)
+
+      expect(license.maximum_user_count).to eq(100)
+    end
+
+    it 'returns historical data when it is higher than the billable users count' do
+      create(:historical_data, active_user_count: 100)
+      create(:instance_statistics_measurement, identifier: :billable_users, count: 50)
+
+      expect(license.maximum_user_count).to eq(100)
+    end
+
+    it 'returns the correct value when historical data and billable users are equal' do
+      create(:historical_data, active_user_count: 100)
+      create(:instance_statistics_measurement, identifier: :billable_users, count: 100)
+
+      expect(license.maximum_user_count).to eq(100)
+    end
+
+    it 'returns the highest value from historical data' do
+      create(:historical_data, recorded_at: license.expires_at - 4.months, active_user_count: 130)
+      create(:historical_data, recorded_at: license.expires_at - 3.months, active_user_count: 250)
+      create(:historical_data, recorded_at: license.expires_at - 1.month, active_user_count: 215)
+
+      expect(license.maximum_user_count).to eq(250)
+    end
+
+    it 'uses only the most recent billable users entry' do
+      create(:instance_statistics_measurement, recorded_at: license.expires_at - 3.months, identifier: :billable_users, count: 150)
+      create(:historical_data, recorded_at: license.expires_at - 3.months, active_user_count: 140)
+      create(:instance_statistics_measurement, recorded_at: license.expires_at - 2.months, identifier: :billable_users, count: 100)
+
+      expect(license.maximum_user_count).to eq(140)
+    end
+
+    it 'returns the highest historical data since the license started for a 1 year license' do
+      license = build(:license, starts_at: now - 4.months, expires_at: now + 8.months )
+      create(:historical_data, recorded_at: license.starts_at - 1.day, active_user_count: 100)
+      create(:historical_data, recorded_at: now, active_user_count: 40)
+
+      expect(license.maximum_user_count).to eq(40)
+    end
+
+    it 'returns the highest historical data since the license started for a license that lasts 6 months' do
+      license = build(:license, starts_at: now - 4.months, expires_at: now + 2.months )
+      create(:historical_data, recorded_at: license.starts_at - 1.day, active_user_count: 80)
+      create(:historical_data, recorded_at: now, active_user_count: 30)
+
+      expect(license.maximum_user_count).to eq(30)
+    end
+
+    it 'returns the highest historical data since the license started for a license that lasts two years' do
+      license = build(:license, starts_at: now - 6.months, expires_at: now + 18.months )
+      create(:historical_data, recorded_at: license.starts_at - 1.day, active_user_count: 400)
+      create(:historical_data, recorded_at: now, active_user_count: 300)
+
+      expect(license.maximum_user_count).to eq(300)
+    end
+
+    it 'returns the highest historical data during the license period for an expired license' do
+      license = build(:license, starts_at: now - 14.months, expires_at: now - 2.months )
+      create(:historical_data, recorded_at: license.expires_at - 1.month, active_user_count: 400)
+      create(:historical_data, recorded_at: now, active_user_count: 500)
+
+      expect(license.maximum_user_count).to eq(400)
     end
   end