Merge branch 'exclude-negative-values-from-vsa-aggregation' into 'master'

Exclude negative values from VSA aggregation

See merge request gitlab-org/gitlab!81409

ee/app/services/analytics/cycle_analytics/data_loader_service.rb

@@ -106,7 +106,12 @@ module Analytics
 
         records.each_value do |record|
           stages.each do |stage|
-            next if record[event_column_name(stage.start_event)].nil?
+            start_event = record[event_column_name(stage.start_event)]
+            end_event = record[event_column_name(stage.end_event)]
+
+            next if start_event.nil?
+            # Avoid negative duration values
+            next if end_event && start_event > end_event
 
             data << {
               stage_event_hash_id: stage.stage_event_hash_id,
@@ -116,8 +121,8 @@ module Analytics
               author_id: record['author_id'],
               milestone_id: record['milestone_id'],
               state_id: record['state_id'],
-              start_event_timestamp: record[event_column_name(stage.start_event)],
-              end_event_timestamp: record[event_column_name(stage.end_event)]
+              start_event_timestamp: start_event,
+              end_event_timestamp: end_event
             }
 
             if data.size == UPSERT_LIMIT

ee/spec/services/analytics/cycle_analytics/consistency_check_service_spec.rb

@@ -60,9 +60,9 @@ RSpec.describe Analytics::CycleAnalytics::ConsistencyCheckService, :aggregate_fa
 
   context 'for issue based stage' do
     let(:event_model) { Analytics::CycleAnalytics::IssueStageEvent }
-    let!(:record1) { create(:issue, :closed, project: project1) }
-    let!(:record2) { create(:issue, :closed, project: project2) }
-    let!(:record3) { create(:issue, :closed, project: project2) }
+    let!(:record1) { create(:issue, :closed, project: project1, created_at: 1.month.ago) }
+    let!(:record2) { create(:issue, :closed, project: project2, created_at: 1.month.ago) }
+    let!(:record3) { create(:issue, :closed, project: project2, created_at: 1.month.ago) }
 
     let!(:stage) { create(:cycle_analytics_group_stage, group: group, start_event_identifier: :issue_created, end_event_identifier: :issue_closed) }
 
@@ -71,9 +71,9 @@ RSpec.describe Analytics::CycleAnalytics::ConsistencyCheckService, :aggregate_fa
 
   context 'for merge request based stage' do
     let(:event_model) { Analytics::CycleAnalytics::MergeRequestStageEvent }
-    let!(:record1) { create(:merge_request, :closed_last_month, project: project1) }
-    let!(:record2) { create(:merge_request, :closed_last_month, project: project2) }
-    let!(:record3) { create(:merge_request, :closed_last_month, project: project2) }
+    let!(:record1) { create(:merge_request, :closed_last_month, project: project1, created_at: 2.months.ago) }
+    let!(:record2) { create(:merge_request, :closed_last_month, project: project2, created_at: 2.months.ago) }
+    let!(:record3) { create(:merge_request, :closed_last_month, project: project2, created_at: 2.months.ago) }
 
     let!(:stage) { create(:cycle_analytics_group_stage, group: group, start_event_identifier: :merge_request_created, end_event_identifier: :merge_request_closed) }
 

ee/spec/services/analytics/cycle_analytics/data_loader_service_spec.rb

@@ -177,9 +177,11 @@ RSpec.describe Analytics::CycleAnalytics::DataLoaderService do
     end
 
     context 'when Issue data is present' do
-      let_it_be(:issue1) { create(:issue, project: project1, closed_at: Time.current) }
-      let_it_be(:issue2) { create(:issue, project: project1, closed_at: Time.current) }
-      let_it_be(:issue3) { create(:issue, project: project2, closed_at: Time.current) }
+      let_it_be(:issue1) { create(:issue, project: project1, closed_at: 5.minutes.from_now) }
+      let_it_be(:issue2) { create(:issue, project: project1, closed_at: 5.minutes.from_now) }
+      let_it_be(:issue3) { create(:issue, project: project2, closed_at: 5.minutes.from_now) }
+      # invalid the creation time would be later than closed_at, this should not be aggregated
+      let_it_be(:issue4) { create(:issue, project: project2, closed_at: 5.minutes.ago) }
 
       it 'inserts stage records' do
         expected_data = [issue1, issue2, issue3].map do |issue|