Merge branch 'vs-move-threshold-to-a-model' into 'master'

Move active user count logic to License model

See merge request gitlab-org/gitlab!41678

ee/app/helpers/license_monitoring_helper.rb

@@ -3,22 +3,13 @@
 module LicenseMonitoringHelper
   include Gitlab::Utils::StrongMemoize
 
-  ACTIVE_USER_COUNT_THRESHOLD_LEVELS = [
-    { range: (2..15), percentage: false, value: 1 },
-    { range: (16..25), percentage: false, value: 2 },
-    { range: (26..99), percentage: true, value: 10 },
-    { range: (100..999), percentage: true, value: 8 },
-    { range: (1000..nil), percentage: true, value: 5 }
-  ].freeze
-
   def show_active_user_count_threshold_banner?
     return if ::Gitlab.com?
     return unless admin_section?
     return if user_dismissed?(UserCalloutsHelper::ACTIVE_USER_COUNT_THRESHOLD)
     return if license_not_available_or_trial?
-    return if current_active_users_count > total_user_count
 
-    current_user&.admin? && active_user_count_threshold_reached?
+    current_user&.admin? && current_license.active_user_count_threshold_reached?
   end
 
   def license_is_over_capacity?
@@ -34,16 +25,6 @@ module LicenseMonitoringHelper
     current_license.nil? || current_license.trial?
   end
 
-  def active_user_count_threshold_reached?
-    return if current_active_users_count <= 1
-
-    active_user_count_threshold[:value] >= if active_user_count_threshold[:percentage]
-                                             remaining_user_count.fdiv(current_active_users_count) * 100
-                                           else
-                                             remaining_user_count
-                                           end
-  end
-
   def current_license
     strong_memoize(:current_license) { License.current }
   end
@@ -52,23 +33,15 @@ module LicenseMonitoringHelper
     strong_memoize(:current_license_overage) { current_license.overage_with_historical_max }
   end
 
-  def current_active_users_count
-    strong_memoize(:current_active_users_count) { current_license.current_active_users_count }
+  def active_user_count_threshold
+    strong_memoize(:active_user_count_threshold) { current_license.active_user_count_threshold }
   end
 
   def total_user_count
-    strong_memoize(:total_user_count) { current_license.restricted_user_count || 0 }
+    strong_memoize(:total_user_count) { current_license.restricted_user_count }
   end
 
   def remaining_user_count
-    strong_memoize(:remaining_user_count) { total_user_count - current_active_users_count }
-  end
-
-  def active_user_count_threshold
-    strong_memoize(:active_user_count_threshold) do
-      ACTIVE_USER_COUNT_THRESHOLD_LEVELS.find do |threshold|
-        threshold[:range].include?(total_user_count)
-      end
-    end
+    strong_memoize(:remaining_user_count) { current_license.restricted_user_count }
   end
 end

ee/app/models/license.rb

@@ -197,6 +197,14 @@ class License < ApplicationRecord
     usage_quotas
   ].freeze
 
+  ACTIVE_USER_COUNT_THRESHOLD_LEVELS = [
+    { range: (2..15), percentage: false, value: 1 },
+    { range: (16..25), percentage: false, value: 2 },
+    { range: (26..99), percentage: true, value: 10 },
+    { range: (100..999), percentage: true, value: 8 },
+    { range: (1000..nil), percentage: true, value: 5 }
+  ].freeze
+
   validate :valid_license
   validate :check_users_limit, if: :new_record?, unless: :validate_with_trueup?
   validate :check_trueup, unless: :persisted?, if: :validate_with_trueup?
@@ -472,6 +480,28 @@ class License < ApplicationRecord
     false
   end
 
+  def active_user_count_threshold
+    ACTIVE_USER_COUNT_THRESHOLD_LEVELS.find do |threshold|
+      threshold[:range].include?(restricted_user_count)
+    end
+  end
+
+  def active_user_count_threshold_reached?
+    return false if restricted_user_count.nil?
+    return false if current_active_users_count <= 1
+    return false if current_active_users_count > restricted_user_count
+
+    active_user_count_threshold[:value] >= if active_user_count_threshold[:percentage]
+                                             remaining_user_count.fdiv(current_active_users_count) * 100
+                                           else
+                                             remaining_user_count
+                                           end
+  end
+
+  def remaining_user_count
+    restricted_user_count - current_active_users_count
+  end
+
   private
 
   def restricted_attr(name, default = nil)

ee/spec/helpers/license_monitoring_helper_spec.rb

@@ -15,7 +15,9 @@ RSpec.describe LicenseMonitoringHelper do
     subject { helper.show_active_user_count_threshold_banner? }
 
     shared_examples 'banner hidden when below the threshold' do
-      let(:active_user_count) { 1 }
+      before do
+        allow(license).to receive(:active_user_count_threshold_reached?).and_return(false)
+      end
 
       it { is_expected.to be_falsey }
     end
@@ -52,7 +54,8 @@ RSpec.describe LicenseMonitoringHelper do
 
         context 'is trial' do
           before do
-            allow(License.current).to receive(:trial?).and_return(true)
+            allow(license).to receive(:trial?).and_return(true)
+            allow(License).to receive(:current).and_return(license)
           end
 
           it { is_expected.to be_falsey }
@@ -61,8 +64,9 @@ RSpec.describe LicenseMonitoringHelper do
 
       context 'when current active user count greater than total user count' do
         before do
-          allow(helper).to receive(:total_user_count).and_return(license_seats_limit)
-          allow(helper).to receive(:current_active_users_count).and_return(license_seats_limit + 1)
+          allow(license).to receive(:restricted_user_count).and_return(license_seats_limit)
+          allow(license).to receive(:current_active_users_count).and_return(license_seats_limit + 1)
+          allow(License).to receive(:current).and_return(license)
         end
 
         it { is_expected.to be_falsey }
@@ -72,11 +76,13 @@ RSpec.describe LicenseMonitoringHelper do
         before do
           allow(helper).to receive(:current_user).and_return(admin)
           allow(helper).to receive(:admin_section?).and_return(true)
-          allow(helper).to receive(:current_active_users_count).and_return(active_user_count)
         end
 
         context 'when above the threshold' do
-          let(:active_user_count) { license_seats_limit - 1 }
+          before do
+            allow(license).to receive(:active_user_count_threshold_reached?).and_return(license_seats_limit + 1)
+            allow(License).to receive(:current).and_return(license)
+          end
 
           it { is_expected.to be_truthy }
         end

ee/spec/models/license_spec.rb

@@ -963,4 +963,58 @@ RSpec.describe License do
       expect(license.auto_renew).to be false
     end
   end
+
+  describe '#active_user_count_threshold' do
+    subject { license.active_user_count_threshold }
+
+    it 'returns nil for license with unlimited user count' do
+      allow(license).to receive(:restricted_user_count).and_return(nil)
+
+      expect(subject).to be_nil
+    end
+
+    context 'for license with users' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:restricted_user_count, :active_user_count, :percentage, :threshold_value) do
+        3    | 2    | false | 1
+        20   | 18   | false | 2
+        90   | 80   | true  | 10
+        300  | 275  | true  | 8
+        1200 | 1100 | true  | 5
+      end
+
+      with_them do
+        before do
+          allow(license).to receive(:restricted_user_count).and_return(restricted_user_count)
+          allow(license).to receive(:current_active_users_count).and_return(active_user_count)
+        end
+
+        it { is_expected.not_to be_nil }
+        it { is_expected.to include(value: threshold_value, percentage: percentage) }
+      end
+    end
+  end
+
+  describe '#active_user_count_threshold_reached?' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { license.active_user_count_threshold_reached? }
+
+    where(:restricted_user_count, :current_active_users_count, :result) do
+      10   | 9   | true
+      nil  | 9   | false
+      10   | 15  | false
+      100  | 95  | true
+    end
+
+    with_them do
+      before do
+        allow(license).to receive(:current_active_users_count).and_return(current_active_users_count)
+        allow(license).to receive(:restricted_user_count).and_return(restricted_user_count)
+      end
+
+      it { is_expected.to eq(result) }
+    end
+  end
 end