Merge branch 'vs-user-count-threshold-banner-v2' into 'master'

Use active user count instead of historical max

Closes #224681

See merge request gitlab-org/gitlab!37916

ee/app/helpers/license_monitoring_helper.rb

@@ -13,26 +13,31 @@ module LicenseMonitoringHelper
 
   def show_active_user_count_threshold_banner?
     return if ::Gitlab.com?
-    return if current_license.nil? || current_license.trial?
+    return unless admin_section?
     return if user_dismissed?(UserCalloutsHelper::ACTIVE_USER_COUNT_THRESHOLD)
-    return if license_is_over_capacity?
+    return if license_not_available_or_trial?
+    return if current_active_users_count > total_user_count
 
     current_user&.admin? && active_user_count_threshold_reached?
   end
 
   private
 
+  def license_not_available_or_trial?
+    current_license.nil? || current_license.trial?
+  end
+
   def license_is_over_capacity?
-    return if current_license.nil? || current_license.trial?
+    return if license_not_available_or_trial?
 
     current_license_overage > 0
   end
 
   def active_user_count_threshold_reached?
-    return if total_user_count.nil? || total_user_count == 1
+    return if current_active_users_count <= 1
 
     active_user_count_threshold[:value] >= if active_user_count_threshold[:percentage]
-                                             remaining_user_count.fdiv(total_user_count) * 100
+                                             remaining_user_count.fdiv(current_active_users_count) * 100
                                            else
                                              remaining_user_count
                                            end
@@ -46,12 +51,16 @@ 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 }
+  end
+
   def total_user_count
-    strong_memoize(:total_user_count) { current_license.restricted_user_count }
+    strong_memoize(:total_user_count) { current_license.restricted_user_count || 0 }
   end
 
   def remaining_user_count
-    strong_memoize(:remaining_user_count) { total_user_count - current_license.maximum_user_count }
+    strong_memoize(:remaining_user_count) { total_user_count - current_active_users_count }
   end
 
   def active_user_count_threshold

ee/changelogs/unreleased/vs-user-count-threshold-banner-v2.yml

+---
+title: Use active user count instead historial max
+merge_request: 37916
+author:
+type: fixed

ee/spec/features/admin/licenses/show_user_count_threshold_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe 'Display approaching user count limit banner', :js do
   let_it_be(:admin) { create(:admin) }
   let_it_be(:user) { create(:user) }
   let_it_be(:license_seats_limit) { 10 }
+  let_it_be(:visit_path) { root_dashboard_path }
 
   let_it_be(:license) do
     create(:license, data: build(:gitlab_license, restrictions: { active_user_count: license_seats_limit }).export)
@@ -13,7 +14,7 @@ RSpec.describe 'Display approaching user count limit banner', :js do
 
   shared_examples_for 'a visible banner' do
     it 'shows the banner' do
-      visit root_dashboard_path
+      visit visit_path
 
       expect(page).to have_content('Your instance is approaching its licensed user count')
       expect(page).to have_link('View users statistics', href: admin_users_path)
@@ -23,7 +24,7 @@ RSpec.describe 'Display approaching user count limit banner', :js do
 
   shared_examples_for 'a hidden banner' do
     it 'does not show the banner' do
-      visit root_dashboard_path
+      visit visit_path
 
       expect(page).not_to have_content('Your instance is approaching its licensed user count')
       expect(page).not_to have_link('View users statistics', href: admin_users_path)
@@ -32,24 +33,35 @@ RSpec.describe 'Display approaching user count limit banner', :js do
   end
 
   before do
-    create(:historical_data, date: license.created_at + 1.month, active_user_count: active_user_count)
+    create_list(:user, active_user_count)
   end
 
   context 'with reached user count threshold' do
-    let(:active_user_count) { license_seats_limit - 1 }
+    let(:active_user_count) { license_seats_limit - 3 }
 
     context 'when admin is logged in' do
       before do
         gitlab_sign_in(admin)
       end
 
-      it_behaves_like 'a visible banner'
+      context 'in admin area' do
+        let(:visit_path) { admin_root_path }
 
-      context 'when banner was dismissed' do
+        it_behaves_like 'a visible banner'
+
+        context 'when banner was dismissed' do
+          before do
+            visit admin_root_path
+            find('.gl-alert-dismiss').click
+          end
+
+          it_behaves_like 'a hidden banner'
+        end
+      end
+
+      context 'in regular area' do
         before do
           visit root_dashboard_path
-
-          find('.gl-alert-dismiss').click
         end
 
         it_behaves_like 'a hidden banner'

ee/spec/helpers/license_monitoring_helper_spec.rb

@@ -11,66 +11,93 @@ RSpec.describe LicenseMonitoringHelper do
     create(:license, data: build(:gitlab_license, restrictions: { active_user_count: license_seats_limit }).export)
   end
 
-  before do
-    create(:historical_data, date: license.created_at + 1.month, active_user_count: active_user_count)
-  end
-
   describe '#show_active_user_count_threshold_banner?' do
-    let_it_be(:active_user_count) { 1 }
-
     subject { helper.show_active_user_count_threshold_banner? }
 
-    context 'when admin user is logged in' do
+    shared_examples 'banner hidden when below the threshold' do
+      let(:active_user_count) { 1 }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'on GitLab.com' do
       before do
-        allow(helper).to receive(:current_user).and_return(admin)
+        allow(Gitlab).to receive(:com?).and_return(true)
       end
 
-      context 'when active users count is above the threshold' do
-        let(:active_user_count) { license_seats_limit - 1 }
+      it { is_expected.to be_falsey }
+    end
 
-        it { is_expected.to be_truthy }
+    context 'on self-managed instance' do
+      before do
+        allow(Gitlab).to receive(:com?).and_return(false)
       end
 
-      context 'when active users count is below the threshold' do
-        let(:active_user_count) { 1 }
+      context 'when callout dismissed' do
+        before do
+          allow(helper).to receive(:user_dismissed?).with(UserCalloutsHelper::ACTIVE_USER_COUNT_THRESHOLD).and_return(true)
+        end
 
         it { is_expected.to be_falsey }
       end
-    end
 
-    context 'when regular user is logged in' do
-      before do
-        allow(helper).to receive(:current_user).and_return(user)
-      end
+      context 'when license' do
+        context 'is not available' do
+          before do
+            allow(License).to receive(:current).and_return(nil)
+          end
 
-      context 'when active users count is above the threshold' do
-        let(:active_user_count) { license_seats_limit - 1 }
+          it { is_expected.to be_falsey }
+        end
 
-        it { is_expected.to be_falsey }
+        context 'is trial' do
+          before do
+            allow(License.current).to receive(:trial?).and_return(true)
+          end
+
+          it { is_expected.to be_falsey }
+        end
       end
 
-      context 'when active users count is below the threshold' do
-        let(:active_user_count) { 1 }
+      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)
+        end
 
         it { is_expected.to be_falsey }
       end
-    end
 
-    context 'with anonymous user' do
-      before do
-        allow(helper).to receive(:current_user).and_return(nil)
+      context 'when logged in as an admin user' 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 }
+
+          it { is_expected.to be_truthy }
+        end
+
+        it_behaves_like 'banner hidden when below the threshold'
       end
 
-      context 'when active users count is above the threshold' do
-        let(:active_user_count) { license_seats_limit - 1 }
+      context 'when logged in as a regular user' do
+        before do
+          allow(helper).to receive(:current_user).and_return(user)
+        end
 
-        it { is_expected.to be_falsey }
+        it_behaves_like 'banner hidden when below the threshold'
       end
 
-      context 'when active users count is below the threshold' do
-        let(:active_user_count) { 1 }
+      context 'when not logged in' do
+        before do
+          allow(helper).to receive(:current_user).and_return(nil)
+        end
 
-        it { is_expected.to be_falsey }
+        it_behaves_like 'banner hidden when below the threshold'
       end
     end
   end