Merge branch 'track-ci-minutes-notification-level-independently' into 'master'

Separate CI minutes notification levels between new and legacy tracking

See merge request gitlab-org/gitlab!75512

ee/app/models/ci/minutes/context.rb

@@ -8,10 +8,11 @@ module Ci
 
       attr_reader :level
 
-      def initialize(project, namespace)
+      def initialize(project, namespace, tracking_strategy: nil)
         @project = project
         @namespace = project&.shared_runners_limit_namespace || namespace
         @level = project || namespace
+        @tracking_strategy = tracking_strategy
       end
 
       def percent_total_minutes_remaining
@@ -23,7 +24,7 @@ module Ci
       attr_reader :project, :namespace
 
       def quota
-        @quota ||= ::Ci::Minutes::Quota.new(namespace)
+        @quota ||= ::Ci::Minutes::Quota.new(namespace, tracking_strategy: @tracking_strategy)
       end
     end
   end

ee/app/models/ci/minutes/notification.rb

@@ -10,8 +10,8 @@ module Ci
         exceeded: 0
       }.freeze
 
-      def initialize(project, namespace)
-        @context = Ci::Minutes::Context.new(project, namespace)
+      def initialize(project, namespace, tracking_strategy: nil)
+        @context = Ci::Minutes::Context.new(project, namespace, tracking_strategy: tracking_strategy)
         @stage = calculate_notification_stage if eligible_for_notifications?
       end
 

ee/app/models/ci/minutes/quota.rb

@@ -11,9 +11,12 @@ module Ci
 
       attr_reader :namespace, :limit
 
-      def initialize(namespace)
+      def initialize(namespace, tracking_strategy: nil)
         @namespace = namespace
         @limit = ::Ci::Minutes::Limit.new(namespace)
+        # TODO: remove `tracking_strategy` after `ci_use_new_monthly_minutes` feature flag
+        # https://gitlab.com/gitlab-org/gitlab/-/issues/341730
+        @tracking_strategy = tracking_strategy
       end
 
       def enabled?
@@ -36,21 +39,19 @@ module Ci
 
       def total_minutes_used
         strong_memoize(:total_minutes_used) do
-          if namespace.new_monthly_ci_minutes_enabled?
-            current_usage.amount_used.to_i
-          else
-            namespace.namespace_statistics&.shared_runners_seconds.to_i / 60
-          end
+          conditional_value(
+            when_new_strategy:    -> { current_usage.amount_used.to_i },
+            when_legacy_strategy: -> { namespace.namespace_statistics&.shared_runners_seconds.to_i / 60 }
+          )
         end
       end
 
       def reset_date
         strong_memoize(:reset_date) do
-          if namespace.new_monthly_ci_minutes_enabled?
-            current_usage.date
-          else
-            namespace.namespace_statistics&.shared_runners_seconds_last_reset
-          end
+          conditional_value(
+            when_new_strategy:    -> { current_usage.date },
+            when_legacy_strategy: -> { namespace.namespace_statistics&.shared_runners_seconds_last_reset }
+          )
         end
       end
 
@@ -90,6 +91,18 @@ module Ci
       def total_minutes_remaining
         [current_balance, 0].max
       end
+
+      def conditional_value(when_new_strategy:, when_legacy_strategy:)
+        if @tracking_strategy == :new
+          when_new_strategy.call
+        elsif @tracking_strategy == :legacy
+          when_legacy_strategy.call
+        elsif namespace.new_monthly_ci_minutes_enabled?
+          when_new_strategy.call
+        else
+          when_legacy_strategy.call
+        end
+      end
     end
   end
 end

ee/app/services/ci/minutes/email_notification_service.rb

@@ -3,58 +3,69 @@
 module Ci
   module Minutes
     class EmailNotificationService < ::BaseService
+      include Gitlab::Utils::StrongMemoize
+
       def execute
         return unless notification.eligible_for_notifications?
 
+        legacy_notify
         notify
       end
 
       private
 
+      # We use 2 notification objects for new and legacy tracking side-by-side.
+      # We read and write data to each tracking using the respective data but we alert
+      # only based on the currently active tracking.
       def notification
-        @notification ||= ::Ci::Minutes::Notification.new(project, nil)
+        @notification ||= ::Ci::Minutes::Notification.new(project, nil, tracking_strategy: :new)
+      end
+
+      def legacy_notification
+        @legacy_notification ||= ::Ci::Minutes::Notification.new(project, nil, tracking_strategy: :legacy)
       end
 
       def notify
         if notification.no_remaining_minutes?
-          notify_total_usage
+          return if namespace_usage.total_usage_notified?
+
+          namespace_usage.update!(notification_level: current_alert_percentage)
+
+          if ci_minutes_use_notification_level?
+            CiMinutesUsageMailer.notify(namespace, recipients).deliver_later
+          end
         elsif notification.running_out?
-          notify_partial_usage
+          return if namespace_usage.usage_notified?(current_alert_percentage)
+
+          namespace_usage.update!(notification_level: current_alert_percentage)
+
+          if ci_minutes_use_notification_level?
+            CiMinutesUsageMailer.notify_limit(namespace, recipients, current_alert_percentage).deliver_later
+          end
         end
       end
 
-      def notify_total_usage
-        # TODO: Enable the FF on the month after this is released.
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/339324
-        if Feature.enabled?(:ci_minutes_use_notification_level, namespace, default_enabled: :yaml)
-          return if namespace_usage.total_usage_notified?
-        else
+      def legacy_notify
+        if legacy_notification.no_remaining_minutes?
           return if namespace.last_ci_minutes_notification_at
-        end
 
-        legacy_track_total_usage
-        namespace_usage.update!(notification_level: current_alert_percentage)
+          namespace.update_columns(last_ci_minutes_notification_at: Time.current)
 
+          unless ci_minutes_use_notification_level?
             CiMinutesUsageMailer.notify(namespace, recipients).deliver_later
           end
+        elsif legacy_notification.running_out?
+          current_alert_percentage = legacy_notification.stage_percentage
 
-      def notify_partial_usage
-        # TODO: Enable the FF on the month after this is released.
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/339324
-        if Feature.enabled?(:ci_minutes_use_notification_level, namespace, default_enabled: :yaml)
-          return if namespace_usage.usage_notified?(current_alert_percentage)
-        else
-          return if already_notified_running_out
-        end
+          # exit if we have already sent a notification for the same level
+          return if namespace.last_ci_minutes_usage_notification_level == current_alert_percentage
 
-        legacy_track_partial_usage
-        namespace_usage.update!(notification_level: current_alert_percentage)
+          namespace.update_columns(last_ci_minutes_usage_notification_level: current_alert_percentage)
 
+          unless ci_minutes_use_notification_level?
             CiMinutesUsageMailer.notify_limit(namespace, recipients, current_alert_percentage).deliver_later
           end
-
-      def already_notified_running_out
-        namespace.last_ci_minutes_usage_notification_level == current_alert_percentage
+        end
       end
 
       def recipients
@@ -74,14 +85,10 @@ module Ci
         notification.stage_percentage
       end
 
-      # TODO: delete this method after full rollout of ci_minutes_use_notification_level Feature Flag
-      def legacy_track_total_usage
-        namespace.update_columns(last_ci_minutes_notification_at: Time.current)
+      def ci_minutes_use_notification_level?
+        strong_memoize(:ci_minutes_use_notification_level) do
+          Feature.enabled?(:ci_minutes_use_notification_level, namespace, default_enabled: :yaml)
         end
-
-      # TODO: delete this method after full rollout of ci_minutes_use_notification_level Feature Flag
-      def legacy_track_partial_usage
-        namespace.update_columns(last_ci_minutes_usage_notification_level: current_alert_percentage)
       end
     end
   end

ee/spec/models/ci/minutes/quota_spec.rb

@@ -92,6 +92,8 @@ RSpec.describe Ci::Minutes::Quota do
   end
 
   describe '#total_minutes_used' do
+    let(:namespace) { create(:namespace, :with_ci_minutes, ci_minutes_used: minutes_used) }
+
     subject { quota.total_minutes_used }
 
     where(:minutes_used, :expected_minutes) do
@@ -103,11 +105,31 @@ RSpec.describe Ci::Minutes::Quota do
     end
 
     with_them do
-      let(:namespace) { create(:namespace, :with_ci_minutes, ci_minutes_used: minutes_used) }
+      it { is_expected.to eq(expected_minutes) }
+    end
+
+    context 'with tracking_strategy' do
+      where(:minutes_used, :legacy_minutes_used, :tracking_strategy, :ff_enabled, :expected_minutes) do
+        0   | 100 | nil     | true  | 0
+        0   | 100 | nil     | false | 100
+        0   | 100 | :new    | true  | 0
+        0   | 100 | :new    | false | 0
+        0   | 100 | :legacy | true  | 100
+        0   | 100 | :legacy | false | 100
+      end
+
+      with_them do
+        let(:quota) { described_class.new(namespace, tracking_strategy: tracking_strategy) }
+
+        before do
+          stub_feature_flags(ci_use_new_monthly_minutes: ff_enabled)
+          namespace.namespace_statistics.update!(shared_runners_seconds: legacy_minutes_used.minutes)
+        end
 
         it { is_expected.to eq(expected_minutes) }
       end
     end
+  end
 
   describe '#percent_total_minutes_remaining' do
     subject { quota.percent_total_minutes_remaining }
@@ -214,6 +236,9 @@ RSpec.describe Ci::Minutes::Quota do
   end
 
   describe '#reset_date' do
+    let(:quota) { described_class.new(namespace, tracking_strategy: tracking_strategy) }
+    let(:tracking_strategy) { nil }
+
     subject(:reset_date) { quota.reset_date }
 
     around do |example|
@@ -228,6 +253,22 @@ RSpec.describe Ci::Minutes::Quota do
       expect(reset_date).to eq(Date.new(2021, 07, 1))
     end
 
+    context 'when tracking_strategy: :new' do
+      let(:tracking_strategy) { :new }
+
+      it 'corresponds to the beginning of the current month' do
+        expect(reset_date).to eq(Date.new(2021, 07, 1))
+      end
+    end
+
+    context 'when tracking_strategy: :legacy' do
+      let(:tracking_strategy) { :legacy }
+
+      it 'corresponds to the current time' do
+        expect(reset_date).to eq(Date.new(2021, 07, 14))
+      end
+    end
+
     context 'when feature flag ci_use_new_monthly_minutes is disabled' do
       before do
         stub_feature_flags(ci_use_new_monthly_minutes: false)

ee/spec/services/ci/minutes/email_notification_service_spec.rb

@@ -10,6 +10,141 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
 
     subject { described_class.new(project).execute }
 
+    where(:ff_enabled, :monthly_minutes_limit, :minutes_used, :current_notification_level, :new_notification_level, :legacy_minutes_used, :legacy_current_notification_level, :legacy_new_notification_level, :result) do
+      # when legacy and new tracking usage matches
+      true  | 1000 | 500  | 100 | 100 | 500  | 100 | 100 | [false]
+      false | 1000 | 500  | 100 | 100 | 500  | 100 | 100 | [false]
+      true  | 1000 | 800  | 100 | 30  | 800  | 100 | 30  | [true, 30]
+      false | 1000 | 800  | 100 | 30  | 800  | 100 | 30  | [true, 30]
+      true  | 1000 | 800  | 30  | 30  | 800  | 30  | 30  | [false]
+      false | 1000 | 800  | 30  | 30  | 800  | 30  | 30  | [false]
+      true  | 1000 | 950  | 100 | 5   | 950  | 100 | 5   | [true, 5]
+      false | 1000 | 950  | 100 | 5   | 950  | 100 | 5   | [true, 5]
+      true  | 1000 | 950  | 30  | 5   | 950  | 30  | 5   | [true, 5]
+      false | 1000 | 950  | 30  | 5   | 950  | 30  | 5   | [true, 5]
+      true  | 1000 | 950  | 5   | 5   | 950  | 5   | 5   | [false]
+      false | 1000 | 950  | 5   | 5   | 950  | 5   | 5   | [false]
+      true  | 1000 | 1000 | 100 | 0   | 1000 | 100 | 0   | [true, 0]
+      false | 1000 | 1000 | 100 | 0   | 1000 | 100 | 0   | [true, 0]
+      true  | 1000 | 1000 | 30  | 0   | 1000 | 30  | 0   | [true, 0]
+      false | 1000 | 1000 | 30  | 0   | 1000 | 30  | 0   | [true, 0]
+      true  | 1000 | 1000 | 5   | 0   | 1000 | 5   | 0   | [true, 0]
+      false | 1000 | 1000 | 5   | 0   | 1000 | 5   | 0   | [true, 0]
+      true  | 1000 | 1001 | 5   | 0   | 1001 | 5   | 0   | [true, 0]
+      false | 1000 | 1001 | 5   | 0   | 1001 | 5   | 0   | [true, 0]
+      true  | 1000 | 1000 | 0   | 0   | 1000 | 0   | 0   | [false]
+      false | 1000 | 1000 | 0   | 0   | 1000 | 0   | 0   | [false]
+      true  | 0    | 1000 | 100 | 100 | 1000 | 100 | 100 | [false]
+      false | 0    | 1000 | 100 | 100 | 1000 | 100 | 100 | [false]
+
+      # when legacy and new tracking usage doesn't match we send notifications
+      # based on the feature flag.
+      true  | 1000 | 500  | 100 | 100 | 800  | 100 | 30  | [false]
+      false | 1000 | 500  | 100 | 100 | 800  | 100 | 30  | [true, 30]
+      true  | 1000 | 800  | 100 | 30  | 500  | 100 | 100 | [true, 30]
+      false | 1000 | 800  | 100 | 30  | 500  | 100 | 100 | [false]
+      true  | 1000 | 950  | 100 | 5   | 800  | 100 | 30  | [true, 5]
+      false | 1000 | 950  | 100 | 5   | 800  | 100 | 30  | [true, 30]
+      true  | 1000 | 950  | 100 | 5   | 1001 | 30  | 0   | [true, 5]
+      false | 1000 | 950  | 100 | 5   | 1001 | 30  | 0   | [true, 0]
+    end
+
+    with_them do
+      shared_examples 'matches the expectations' do
+        it 'matches the expectation on the email sent' do
+          email_sent, level_notified = result
+
+          if email_sent
+            if level_notified > 0
+              expect(CiMinutesUsageMailer)
+                .to receive(:notify_limit)
+                .with(namespace, match_array(recipients), level_notified)
+                .and_call_original
+            else
+              expect(CiMinutesUsageMailer)
+                .to receive(:notify)
+                .with(namespace, match_array(recipients))
+                .and_call_original
+            end
+          else
+            expect(CiMinutesUsageMailer).not_to receive(:notify_limit)
+            expect(CiMinutesUsageMailer).not_to receive(:notify)
+          end
+
+          subject
+        end
+
+        it 'matches the updated notification level' do
+          subject
+
+          expect(namespace_usage.reload.notification_level).to eq(new_notification_level)
+        end
+
+        it 'matches the updated legacy notification level' do
+          subject
+
+          if legacy_new_notification_level == 0
+            expect(namespace.reload.last_ci_minutes_notification_at).to be_present
+          elsif legacy_new_notification_level == 100
+            expect(namespace.reload.last_ci_minutes_notification_at).to be_nil
+            expect(namespace.reload.last_ci_minutes_usage_notification_level).to be_nil
+          else
+            expect(namespace.reload.last_ci_minutes_usage_notification_level).to eq(legacy_new_notification_level)
+          end
+        end
+      end
+
+      let_it_be(:user) { create(:user) }
+      let_it_be(:user_2) { create(:user) }
+
+      let(:project) { create(:project, namespace: namespace) }
+
+      let(:namespace_usage) do
+        Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace_id: namespace.id)
+      end
+
+      before do
+        stub_feature_flags(ci_minutes_use_notification_level: ff_enabled)
+
+        if namespace.namespace_statistics
+          namespace.namespace_statistics.update!(shared_runners_seconds: legacy_minutes_used.minutes)
+        else
+          namespace.create_namespace_statistics(shared_runners_seconds: legacy_minutes_used.minutes)
+        end
+
+        namespace_usage.update!(amount_used: minutes_used)
+
+        namespace_usage.update_column(:notification_level, current_notification_level)
+        namespace.update_column(:shared_runners_minutes_limit, monthly_minutes_limit)
+
+        if legacy_current_notification_level == 0
+          namespace.update_column(:last_ci_minutes_notification_at, Time.current)
+        elsif current_notification_level != 100
+          namespace.update_column(:last_ci_minutes_usage_notification_level, legacy_current_notification_level)
+        end
+      end
+
+      context 'when on personal namespace' do
+        let(:namespace) { create(:namespace, owner: user) }
+        let(:recipients) { [user.email] }
+
+        it_behaves_like 'matches the expectations'
+      end
+
+      context 'when on group' do
+        let(:namespace) { create(:group) }
+        let(:recipients) { [user.email, user_2.email] }
+
+        before do
+          namespace.add_owner(user)
+          namespace.add_owner(user_2)
+        end
+
+        it_behaves_like 'matches the expectations'
+      end
+    end
+
+    context 'legacy path - to remove' do
       def expect_warning_usage_notification(new_notification_level)
         expect(CiMinutesUsageMailer)
           .to receive(:notify_limit)
@@ -46,19 +181,19 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
         expect(namespace_usage.reload.notification_level).to eq(current_notification_level)
       end
 
-    where(:monthly_minutes_limit, :minutes_used, :current_notification_level, :result) do
-      1000 | 500  | 100 | [:not_notified]
-      1000 | 800  | 100 | [:notified, 30]
-      1000 | 800  | 30  | [:not_notified]
-      1000 | 950  | 100 | [:notified, 5]
-      1000 | 950  | 30  | [:notified, 5]
-      1000 | 950  | 5   | [:not_notified]
-      1000 | 1000 | 100 | [:notified, 0]
-      1000 | 1000 | 30  | [:notified, 0]
-      1000 | 1000 | 5   | [:notified, 0]
-      1000 | 1001 | 5   | [:notified, 0]
-      1000 | 1000 | 0   | [:not_notified]
-      0    | 1000 | 100 | [:not_notified]
+      where(:monthly_minutes_limit, :minutes_used, :legacy_minutes_used, :current_notification_level, :legacy_notification_level, :result, :legacy_result) do
+        1000 | 500  | 500  | 100 | 100 | [:not_notified] | [:not_notified]
+        1000 | 800  | 800  | 100 | 100 | [:notified, 30] | [:notified, 30]
+        1000 | 800  | 800  | 30  | 30  | [:not_notified] | [:not_notified]
+        1000 | 950  | 950  | 100 | 100 | [:notified, 5]  | [:notified, 5]
+        1000 | 950  | 950  | 30  | 30  | [:notified, 5]  | [:notified, 5]
+        1000 | 950  | 950  | 5   | 5   | [:not_notified] | [:not_notified]
+        1000 | 1000 | 1000 | 100 | 100 | [:notified, 0]  | [:notified, 0]
+        1000 | 1000 | 1000 | 30  | 30  | [:notified, 0]  | [:notified, 0]
+        1000 | 1000 | 1000 | 5   | 5   | [:notified, 0]  | [:notified, 0]
+        1000 | 1001 | 1001 | 5   | 5   | [:notified, 0]  | [:notified, 0]
+        1000 | 1000 | 1000 | 0   | 0   | [:not_notified] | [:not_notified]
+        0    | 1000 | 1000 | 100 | 100 | [:not_notified] | [:not_notified]
       end
 
       with_them do
@@ -91,7 +226,13 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
         end
 
         before do
-        set_ci_minutes_used(namespace, minutes_used)
+          if namespace.namespace_statistics
+            namespace.namespace_statistics.update!(shared_runners_seconds: legacy_minutes_used.minutes)
+          else
+            namespace.create_namespace_statistics(shared_runners_seconds: legacy_minutes_used.minutes)
+          end
+
+          namespace_usage.update!(amount_used: minutes_used)
 
           namespace_usage.update_column(:notification_level, current_notification_level)
           namespace.update_column(:shared_runners_minutes_limit, monthly_minutes_limit)
@@ -139,4 +280,5 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
         end
       end
     end
+  end
 end