Merge branch 'ali/refactor-usage-data-finished-at' into 'master'

Refactor how finished_at is merged into usage data

Closes #219472

See merge request gitlab-org/gitlab!34331

ee/lib/ee/gitlab/usage_data.rb

@@ -43,16 +43,11 @@ module EE
 
         override :uncached_data
         def uncached_data
-          super
-            .merge(usage_activity_by_stage)
-            .merge(usage_activity_by_stage(:usage_activity_by_stage_monthly, default_time_period))
-            .merge(recording_ee_finish_data)
-        end
-
-        def recording_ee_finish_data
-          {
-            recording_ee_finished_at: Time.now
-          }
+          with_finished_at(:recording_ee_finished_at) do
+            super
+              .merge(usage_activity_by_stage)
+              .merge(usage_activity_by_stage(:usage_activity_by_stage_monthly, default_time_period))
+          end
         end
 
         override :features_usage_data

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -157,6 +157,10 @@ RSpec.describe Gitlab::UsageData do
       expect(subject[:counts][:user_preferences_group_overview_details]).to eq(User.active.count - 2) # we have exactly 2 active users with security dashboard set
       expect(subject[:counts][:user_preferences_group_overview_security_dashboard]).to eq 2
     end
+
+    it 'includes a recording_ee_finished_at timestamp' do
+      expect(subject[:recording_ee_finished_at]).to be_a(Time)
+    end
   end
 
   describe '.features_usage_data_ee' do
@@ -721,14 +725,6 @@ RSpec.describe Gitlab::UsageData do
     end
   end
 
-  describe '.recording_ee_finished_at' do
-    subject { described_class.recording_ee_finish_data }
-
-    it 'gathers time ee recording finishes at' do
-      expect(subject[:recording_ee_finished_at]).to be_a(Time)
-    end
-  end
-
   def for_defined_days_back(days: [29, 2])
     days.each do |n|
       Timecop.travel(n.days.ago) do

lib/gitlab/usage_data.rb

@@ -29,14 +29,15 @@ module Gitlab
       def uncached_data
         clear_memoized_limits
 
-        license_usage_data
-          .merge(system_usage_data)
-          .merge(features_usage_data)
-          .merge(components_usage_data)
-          .merge(cycle_analytics_usage_data)
-          .merge(object_store_usage_data)
-          .merge(topology_usage_data)
-          .merge(recording_ce_finish_data)
+        with_finished_at(:recording_ce_finished_at) do
+          license_usage_data
+            .merge(system_usage_data)
+            .merge(features_usage_data)
+            .merge(components_usage_data)
+            .merge(cycle_analytics_usage_data)
+            .merge(object_store_usage_data)
+            .merge(topology_usage_data)
+        end
       end
 
       def to_json(force_refresh: false)
@@ -59,12 +60,6 @@ module Gitlab
         Time.now
       end
 
-      def recording_ce_finish_data
-        {
-          recording_ce_finished_at: Time.now
-        }
-      end
-
       # rubocop: disable Metrics/AbcSize
       # rubocop: disable CodeReuse/ActiveRecord
       def system_usage_data

lib/gitlab/utils/usage_data.rb

@@ -92,6 +92,10 @@ module Gitlab
         [result, duration]
       end
 
+      def with_finished_at(key, &block)
+        yield.merge(key => Time.now)
+      end
+
       private
 
       def redis_usage_counter

spec/lib/gitlab/usage_data_spec.rb

@@ -169,6 +169,10 @@ describe Gitlab::UsageData, :aggregate_failures do
       expect { subject }.not_to raise_error
     end
 
+    it 'includes a recording_ce_finished_at timestamp' do
+      expect(subject[:recording_ce_finished_at]).to be_a(Time)
+    end
+
     it 'jira usage works when queries time out' do
       allow_any_instance_of(ActiveRecord::Relation)
         .to receive(:find_in_batches).and_raise(ActiveRecord::StatementInvalid.new(''))
@@ -216,14 +220,6 @@ describe Gitlab::UsageData, :aggregate_failures do
     end
   end
 
-  describe '.recording_ce_finished_at' do
-    subject { described_class.recording_ce_finish_data }
-
-    it 'gathers time ce recording finishes at' do
-      expect(subject[:recording_ce_finished_at]).to be_a(Time)
-    end
-  end
-
   context 'when not relying on database records' do
     describe '#features_usage_data_ce' do
       subject { described_class.features_usage_data_ce }

spec/lib/gitlab/utils/usage_data_spec.rb

@@ -108,4 +108,14 @@ describe Gitlab::Utils::UsageData do
       expect(duration).to eq(2)
     end
   end
+
+  describe '#with_finished_at' do
+    it 'adds a timestamp to the hash yielded by the block' do
+      freeze_time do
+        result = described_class.with_finished_at(:current_time) { { a: 1 } }
+
+        expect(result).to eq(a: 1, current_time: Time.now)
+      end
+    end
+  end
 end