Merge branch 'remove-ci-minutes-tracking-strategy' into 'master'

Remove tracking strategy in Ci::Minutes::Quota

See merge request gitlab-org/gitlab!83277

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

@@ -8,9 +8,8 @@ module Ci
 
       attr_reader :namespace
 
-      def initialize(project, namespace, tracking_strategy: nil)
+      def initialize(project, namespace)
         @namespace = project&.shared_runners_limit_namespace || namespace
-        @tracking_strategy = tracking_strategy
       end
 
       def percent_total_minutes_remaining
@@ -20,7 +19,7 @@ module Ci
       private
 
       def quota
-        @quota ||= ::Ci::Minutes::Quota.new(namespace, tracking_strategy: @tracking_strategy)
+        @quota ||= ::Ci::Minutes::Quota.new(namespace)
       end
     end
   end

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

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

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

@@ -11,12 +11,9 @@ module Ci
 
       attr_reader :namespace, :limit
 
-      def initialize(namespace, tracking_strategy: nil)
+      def initialize(namespace)
         @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?
@@ -38,21 +35,11 @@ module Ci
       end
 
       def total_minutes_used
-        strong_memoize(:total_minutes_used) do
-          conditional_value(
-            when_new_strategy:    -> { current_usage.amount_used.to_i },
-            when_legacy_strategy: -> { namespace.namespace_statistics&.shared_runners_seconds.to_i / 60 }
-          )
-        end
+        current_usage.amount_used.to_i
       end
 
       def reset_date
-        strong_memoize(:reset_date) do
-          conditional_value(
-            when_new_strategy:    -> { current_usage.date },
-            when_legacy_strategy: -> { namespace.namespace_statistics&.shared_runners_seconds_last_reset }
-          )
-        end
+        current_usage.date
       end
 
       # === private to view ===
@@ -91,16 +78,6 @@ 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
-        else
-          when_new_strategy.call
-        end
-      end
     end
   end
 end

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

 # frozen_string_literal: true
 
+# This sends an email notification to the namespace owners when a new notification
+# threshold is hit. We do nothing if we have already sent the notification for the
+# current threshold.
 module Ci
   module Minutes
     class EmailNotificationService < ::BaseService
-      include Gitlab::Utils::StrongMemoize
-
       def execute
         return unless notification.eligible_for_notifications?
 
-        legacy_notify
-        notify
+        # We support 2 different messaging based on whether the
+        # namespace is running out of minutes or has already used
+        # up all the minutes.
+        if notification.no_remaining_minutes?
+          notify_minutes_used_up
+        elsif notification.running_out?
+          notify_minutes_running_out
+        end
       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, tracking_strategy: :new)
+        @notification ||= ::Ci::Minutes::Notification.new(project, nil)
       end
 
-      def legacy_notification
-        @legacy_notification ||= ::Ci::Minutes::Notification.new(project, nil, tracking_strategy: :legacy)
-      end
-
-      def notify
-        if notification.no_remaining_minutes?
-          return if namespace_usage.total_usage_notified?
+      def notify_minutes_used_up
+        return if namespace_usage.total_usage_notified?
 
-          namespace_usage.update!(notification_level: current_alert_percentage)
+        namespace_usage.update!(notification_level: current_alert_percentage)
 
-          CiMinutesUsageMailer.notify(namespace, recipients).deliver_later
-        elsif notification.running_out?
-          return if namespace_usage.usage_notified?(current_alert_percentage)
-
-          namespace_usage.update!(notification_level: current_alert_percentage)
-
-          CiMinutesUsageMailer.notify_limit(namespace, recipients, current_alert_percentage).deliver_later
-        end
+        CiMinutesUsageMailer.notify(namespace, recipients).deliver_later
       end
 
-      def legacy_notify
-        if legacy_notification.no_remaining_minutes?
-          return if namespace.last_ci_minutes_notification_at
-
-          namespace.update_columns(last_ci_minutes_notification_at: Time.current)
-        elsif legacy_notification.running_out?
-          current_alert_percentage = legacy_notification.stage_percentage
+      def notify_minutes_running_out
+        return if namespace_usage.usage_notified?(current_alert_percentage)
 
-          # exit if we have already sent a notification for the same level
-          return if namespace.last_ci_minutes_usage_notification_level == current_alert_percentage
+        namespace_usage.update!(notification_level: current_alert_percentage)
 
-          namespace.update_columns(last_ci_minutes_usage_notification_level: current_alert_percentage)
-        end
+        CiMinutesUsageMailer.notify_limit(namespace, recipients, current_alert_percentage).deliver_later
       end
 
       def recipients

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

@@ -107,24 +107,6 @@ RSpec.describe Ci::Minutes::Quota do
     with_them do
       it { is_expected.to eq(expected_minutes) }
     end
-
-    context 'with tracking_strategy' do
-      where(:minutes_used, :legacy_minutes_used, :tracking_strategy, :expected_minutes) do
-        0   | 100 | nil     | 0
-        0   | 100 | :new    | 0
-        0   | 100 | :legacy | 100
-      end
-
-      with_them do
-        let(:quota) { described_class.new(namespace, tracking_strategy: tracking_strategy) }
-
-        before do
-          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
@@ -232,8 +214,7 @@ 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 }
+    let(:quota) { described_class.new(namespace) }
 
     subject(:reset_date) { quota.reset_date }
 
@@ -248,21 +229,5 @@ RSpec.describe Ci::Minutes::Quota do
     it 'corresponds to the beginning of the current month' 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
   end
 end

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

@@ -10,27 +10,19 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
 
     subject { described_class.new(project).execute }
 
-    where(: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
-      1000 | 500  | 100 | 100 | 500  | 100 | 100 | [false]
-      1000 | 800  | 100 | 30  | 800  | 100 | 30  | [true, 30]
-      1000 | 800  | 30  | 30  | 800  | 30  | 30  | [false]
-      1000 | 950  | 100 | 5   | 950  | 100 | 5   | [true, 5]
-      1000 | 950  | 30  | 5   | 950  | 30  | 5   | [true, 5]
-      1000 | 950  | 5   | 5   | 950  | 5   | 5   | [false]
-      1000 | 1000 | 100 | 0   | 1000 | 100 | 0   | [true, 0]
-      1000 | 1000 | 30  | 0   | 1000 | 30  | 0   | [true, 0]
-      1000 | 1000 | 5   | 0   | 1000 | 5   | 0   | [true, 0]
-      1000 | 1001 | 5   | 0   | 1001 | 5   | 0   | [true, 0]
-      1000 | 1000 | 0   | 0   | 1000 | 0   | 0   | [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.
-      1000 | 500  | 100 | 100 | 800  | 100 | 30  | [false]
-      1000 | 800  | 100 | 30  | 500  | 100 | 100 | [true, 30]
-      1000 | 950  | 100 | 5   | 800  | 100 | 30  | [true, 5]
-      1000 | 950  | 100 | 5   | 1001 | 30  | 0   | [true, 5]
+    where(:monthly_minutes_limit, :minutes_used, :current_notification_level, :new_notification_level, :result) do
+      1000 | 500  | 100 | 100 | [false]
+      1000 | 800  | 100 | 30  | [true, 30]
+      1000 | 800  | 30  | 30  | [false]
+      1000 | 950  | 100 | 5   | [true, 5]
+      1000 | 950  | 30  | 5   | [true, 5]
+      1000 | 950  | 5   | 5   | [false]
+      1000 | 1000 | 100 | 0   | [true, 0]
+      1000 | 1000 | 30  | 0   | [true, 0]
+      1000 | 1000 | 5   | 0   | [true, 0]
+      1000 | 1001 | 5   | 0   | [true, 0]
+      1000 | 1000 | 0   | 0   | [false]
+      0    | 1000 | 100 | 100 | [false]
     end
 
     with_them do
@@ -63,19 +55,6 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
 
           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) }
@@ -88,22 +67,8 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
       end
 
       before do
-        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_usage.update!(amount_used: minutes_used, 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
@@ -125,126 +90,5 @@ RSpec.describe Ci::Minutes::EmailNotificationService do
         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)
-          .with(namespace, match_array(recipients), new_notification_level)
-          .and_call_original
-
-        subject
-
-        expect(namespace_usage.reload.notification_level).to eq(new_notification_level)
-        expect(namespace.reload.last_ci_minutes_usage_notification_level).to eq(new_notification_level)
-      end
-
-      def expect_quota_exceeded_notification
-        expect(CiMinutesUsageMailer)
-          .to receive(:notify)
-          .with(namespace, match_array(recipients))
-          .and_call_original
-
-        subject
-
-        expect(namespace_usage.reload.notification_level).to eq(0)
-        expect(namespace.reload.last_ci_minutes_notification_at).to be_present
-      end
-
-      def expect_no_notification(current_notification_level)
-        expect(CiMinutesUsageMailer)
-          .not_to receive(:notify_limit)
-        expect(CiMinutesUsageMailer)
-          .not_to receive(:notify)
-
-        subject
-
-        # notification level remains the same
-        expect(namespace_usage.reload.notification_level).to eq(current_notification_level)
-      end
-
-      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
-        shared_examples 'matches the expectation' do
-          it 'matches the expectation' do
-            expectation, new_notification_level = result
-
-            if expectation == :notified && new_notification_level > 0
-              expect_warning_usage_notification(new_notification_level)
-
-            elsif expectation == :notified && new_notification_level == 0
-              expect_quota_exceeded_notification
-
-            elsif expectation == :not_notified
-              expect_no_notification(current_notification_level)
-
-            else
-              raise 'unexpected test scenario'
-            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
-          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 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, 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 expectation'
-        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 expectation'
-        end
-      end
-    end
   end
 end