Merge branch '34947-erroneous-ci_usage_warning_message' into 'master'

Check minutes before displaying CI Minutes warning

Closes #34947

See merge request gitlab-org/gitlab!19751

ee/app/helpers/ee/runners_helper.rb

@@ -26,7 +26,7 @@ module EE
     def ci_usage_base_message(namespace)
       if namespace.shared_runners_minutes_used?
         s_("Pipelines|%{namespace_name} has exceeded its pipeline minutes quota.") % { namespace_name: namespace.name }
-      elsif namespace.last_ci_minutes_usage_notification_level
+      elsif namespace.shared_runners_remaining_minutes_below_threshold?
         s_("Pipelines|%{namespace_name} has less than %{notification_level}%% of CI minutes available.") % { namespace_name: namespace.name, notification_level: namespace.last_ci_minutes_usage_notification_level }
       end
     end

ee/app/models/ee/namespace.rb

@@ -197,6 +197,17 @@ module EE
         shared_runners_minutes.to_i >= actual_shared_runners_minutes_limit
     end
 
+    def shared_runners_remaining_minutes_percent
+      return 0 if shared_runners_remaining_minutes.to_f <= 0
+      return 0 if actual_shared_runners_minutes_limit.to_f == 0
+
+      (shared_runners_remaining_minutes.to_f * 100) / actual_shared_runners_minutes_limit.to_f
+    end
+
+    def shared_runners_remaining_minutes_below_threshold?
+      shared_runners_remaining_minutes_percent.to_i <= last_ci_minutes_usage_notification_level.to_i
+    end
+
     def extra_shared_runners_minutes_used?
       shared_runners_minutes_limit_enabled? &&
         extra_shared_runners_minutes_limit &&
@@ -352,5 +363,9 @@ module EE
         seats: 0
       )
     end
+
+    def shared_runners_remaining_minutes
+      [actual_shared_runners_minutes_limit.to_f - shared_runners_minutes.to_f, 0].max
+    end
   end
 end

ee/app/services/ci_minutes_usage_notify_service.rb

@@ -26,7 +26,7 @@ class CiMinutesUsageNotifyService < BaseService
   def notify_on_partial_usage
     return if namespace.shared_runners_minutes_used?
     return if namespace.last_ci_minutes_usage_notification_level == current_alert_level
-    return if alert_levels.max < ci_minutes_percent_left
+    return if alert_levels.max < namespace.shared_runners_remaining_minutes_percent
 
     namespace.update_columns(last_ci_minutes_usage_notification_level: current_alert_level)
 
@@ -37,21 +37,13 @@ class CiMinutesUsageNotifyService < BaseService
     @namespace ||= project.shared_runners_limit_namespace
   end
 
-  def ci_minutes_percent_left
-    quota = namespace.actual_shared_runners_minutes_limit
-    used = namespace.shared_runners_minutes.to_i
-    minutes_left = quota - used
-
-    return 0 if minutes_left <= 0
-
-    (minutes_left.to_f * 100) / quota.to_f
-  end
-
   def alert_levels
     @alert_levels ||= EE::Namespace::CI_USAGE_ALERT_LEVELS.sort
   end
 
   def current_alert_level
-    @current_alert_level ||= alert_levels.find { |level| level >= ci_minutes_percent_left }
+    remaining_percent = namespace.shared_runners_remaining_minutes_percent
+
+    @current_alert_level ||= alert_levels.find { |level| level >= remaining_percent }
   end
 end

ee/changelogs/unreleased/34947-erroneous-ci_usage_warning_message.yml

+---
+title: Display CI Minutes warning only if minutes left is still below last level
+merge_request: 19751
+author:
+type: fixed

ee/spec/features/ci_shared_runner_warnings_spec.rb

@@ -34,7 +34,10 @@ describe 'CI shared runner limits' do
       end
 
       context 'when usage has reached a notification level' do
-        let(:group) { create(:group, :with_build_minutes_limit, last_ci_minutes_usage_notification_level: 30) }
+        before do
+          group.update(last_ci_minutes_usage_notification_level: 30, shared_runners_minutes_limit: 10)
+          allow_any_instance_of(EE::Namespace).to receive(:shared_runners_remaining_minutes).and_return(2)
+        end
 
         it 'displays a warning message on pipelines page' do
           visit_project_pipelines

ee/spec/helpers/ee/runners_helper_spec.rb

@@ -34,6 +34,38 @@ describe EE::RunnersHelper do
       end
     end
 
+    context 'when the last_ci_minutes_usage_notification_level field is set' do
+      before do
+        namespace.update_attribute(:last_ci_minutes_usage_notification_level, 50)
+      end
+
+      context 'when there are minutes used but remaining minutes percent is still below the notification threshold' do
+        let(:minutes_used) { 51 }
+
+        it 'returns the partial usage notification message' do
+          expect(subject).to match("#{namespace.name} has less than 50% of CI minutes available.")
+        end
+      end
+
+      context 'when limit is increased so there are now more remaining minutes percentage than the notification threshold' do
+        before do
+          namespace.update(shared_runners_minutes_limit: 200)
+        end
+
+        it 'returns nil' do
+          expect(subject).to be_nil
+        end
+      end
+
+      context 'when there are no more remaining minutes' do
+        let(:minutes_used) { 100 }
+
+        it 'returns the exceeded usage message' do
+          expect(subject).to match("#{namespace.name} has exceeded its pipeline minutes quota.")
+        end
+      end
+    end
+
     context 'when current user is an owner' do
       before do
         allow(helper).to receive(:can?).with(user, :admin_project, project) { true }
@@ -50,6 +82,8 @@ describe EE::RunnersHelper do
       end
 
       context 'when usage has reached first level of notification' do
+        let(:minutes_used) { 50 }
+
         before do
           namespace.update_attribute(:last_ci_minutes_usage_notification_level, 50)
         end
@@ -82,6 +116,8 @@ describe EE::RunnersHelper do
       end
 
       context 'when usage has reached first level of notification' do
+        let(:minutes_used) { 50 }
+
         before do
           namespace.update_attribute(:last_ci_minutes_usage_notification_level, 50)
         end

ee/spec/models/namespace_spec.rb

@@ -618,6 +618,69 @@ describe Namespace do
     end
   end
 
+  describe '#shared_runners_remaining_minutes_percent' do
+    let(:namespace) { build(:namespace) }
+    subject { namespace.shared_runners_remaining_minutes_percent }
+
+    it 'returns the minutes left as a percent of the limit' do
+      stub_minutes_used_and_limit(8, 10)
+
+      expect(subject).to eq(20)
+    end
+
+    it 'returns 100 when minutes used are 0' do
+      stub_minutes_used_and_limit(0, 10)
+
+      expect(subject).to eq(100)
+    end
+
+    it 'returns 0 when the limit is 0' do
+      stub_minutes_used_and_limit(0, 0)
+
+      expect(subject).to eq(0)
+    end
+
+    it 'returns 0 when the limit is nil' do
+      stub_minutes_used_and_limit(nil, nil)
+
+      expect(subject).to eq(0)
+    end
+
+    it 'returns 0 when minutes used are over the limit' do
+      stub_minutes_used_and_limit(11, 10)
+
+      expect(subject).to eq(0)
+    end
+
+    it 'returns 0 when minutes used are equal to the limit' do
+      stub_minutes_used_and_limit(10, 10)
+
+      expect(subject).to eq(0)
+    end
+
+    def stub_minutes_used_and_limit(minutes_used, limit)
+      allow(namespace).to receive(:shared_runners_minutes).and_return(minutes_used)
+      allow(namespace).to receive(:actual_shared_runners_minutes_limit).and_return(limit)
+    end
+  end
+
+  describe '#shared_runners_remaining_minutes_below_threshold?' do
+    let(:namespace) { build(:namespace, last_ci_minutes_usage_notification_level: 30) }
+    subject { namespace.shared_runners_remaining_minutes_below_threshold? }
+
+    it 'is true when minutes left is below the notification level' do
+      allow(namespace).to receive(:shared_runners_remaining_minutes_percent).and_return(10)
+
+      expect(subject).to be_truthy
+    end
+
+    it 'is false when minutes left is not below the notification level' do
+      allow(namespace).to receive(:shared_runners_remaining_minutes_percent).and_return(80)
+
+      expect(subject).to be_falsey
+    end
+  end
+
   describe '#actual_plan' do
     context 'when namespace has a plan associated' do
       before do