From d50050b3281955690d241a724e9d8c549d71b602 Mon Sep 17 00:00:00 2001
From: Shivam Mishra <scm.mymail@gmail.com>
Date: Wed, 11 Mar 2026 18:24:41 +0530
Subject: [PATCH] refactor: use upsert_all

---
 .../reporting_events/rollup_service.rb        | 27 +++++----
 .../reporting_events/rollup_service_spec.rb   | 60 +++++++++++++++++++
 2 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/app/services/reporting_events/rollup_service.rb b/app/services/reporting_events/rollup_service.rb
index 4236942da..6bccdd709 100644
--- a/app/services/reporting_events/rollup_service.rb
+++ b/app/services/reporting_events/rollup_service.rb
@@ -16,15 +16,10 @@ module ReportingEvents::RollupService
     def perform
       return unless rollup_enabled?
 
-      # NOTE: Each event produces individual upserts per dimension x metric (up to 6 calls).
-      # If this becomes a bottleneck, batch into a single upsert_all call.
-      dimensions.each do |dimension_type, dimension_id|
-        next if dimension_id.nil?
+      rows = build_rollup_rows
+      return if rows.empty?
 
-        metrics_for_event.each do |metric, metric_data|
-          upsert_rollup(dimension_type, dimension_id, metric, metric_data)
-        end
-      end
+      upsert_rollups(rows)
     end
 
     private
@@ -65,6 +60,16 @@ module ReportingEvents::RollupService
       end
     end
 
+    def build_rollup_rows
+      dimensions.each_with_object([]) do |(dimension_type, dimension_id), rows|
+        next if dimension_id.nil?
+
+        metrics_for_event.each do |metric, metric_data|
+          rows << rollup_attributes(dimension_type, dimension_id, metric, metric_data)
+        end
+      end
+    end
+
     def conversation_resolved_metrics
       {
         resolutions_count: { count: 1, sum_value: 0, sum_value_business_hours: 0 },
@@ -108,10 +113,10 @@ module ReportingEvents::RollupService
       }
     end
 
-    def upsert_rollup(dimension_type, dimension_id, metric, metric_data)
+    def upsert_rollups(rows)
       # rubocop:disable Rails/SkipsModelValidations
-      ReportingEventsRollup.upsert(
-        rollup_attributes(dimension_type, dimension_id, metric, metric_data),
+      ReportingEventsRollup.upsert_all(
+        rows,
         unique_by: [:account_id, :date, :dimension_type, :dimension_id, :metric],
         on_duplicate: upsert_on_duplicate_sql
       )
diff --git a/spec/services/reporting_events/rollup_service_spec.rb b/spec/services/reporting_events/rollup_service_spec.rb
index b54278ea3..2a42f8e54 100644
--- a/spec/services/reporting_events/rollup_service_spec.rb
+++ b/spec/services/reporting_events/rollup_service_spec.rb
@@ -102,6 +102,66 @@ describe ReportingEvents::RollupService do
           expect(resolution_time.sum_value_business_hours).to eq(500)
         end
 
+        it 'batches all rollup rows in a single upsert_all call' do
+          reporting_event.update!(created_at: Time.utc(2026, 2, 12, 4, 0, 0))
+
+          travel_to(Time.utc(2026, 2, 12, 10, 0, 0)) do
+            captured_rows = nil
+            captured_options = nil
+
+            allow(ReportingEventsRollup).to receive(:upsert_all) do |rows, **options|
+              captured_rows = rows
+              captured_options = options
+            end
+
+            described_class.perform(reporting_event)
+
+            expect(ReportingEventsRollup).to have_received(:upsert_all).once
+            expect(captured_options[:unique_by]).to eq(%i[account_id date dimension_type dimension_id metric])
+            expect(captured_options[:on_duplicate].to_s.squish).to eq(
+              'count = reporting_events_rollups.count + EXCLUDED.count, ' \
+              'sum_value = reporting_events_rollups.sum_value + EXCLUDED.sum_value, ' \
+              'sum_value_business_hours = reporting_events_rollups.sum_value_business_hours + EXCLUDED.sum_value_business_hours, ' \
+              'updated_at = EXCLUDED.updated_at'
+            )
+
+            expected_rows = {
+              account: account.id,
+              agent: user.id,
+              inbox: inbox.id
+            }.flat_map do |dimension_type, dimension_id|
+              [
+                {
+                  account_id: account.id,
+                  date: Date.new(2026, 2, 11),
+                  dimension_type: dimension_type,
+                  dimension_id: dimension_id,
+                  metric: :resolutions_count,
+                  count: 1,
+                  sum_value: 0.0,
+                  sum_value_business_hours: 0.0,
+                  created_at: Time.utc(2026, 2, 12, 10, 0, 0),
+                  updated_at: Time.utc(2026, 2, 12, 10, 0, 0)
+                },
+                {
+                  account_id: account.id,
+                  date: Date.new(2026, 2, 11),
+                  dimension_type: dimension_type,
+                  dimension_id: dimension_id,
+                  metric: :resolution_time,
+                  count: 1,
+                  sum_value: 1000.0,
+                  sum_value_business_hours: 500.0,
+                  created_at: Time.utc(2026, 2, 12, 10, 0, 0),
+                  updated_at: Time.utc(2026, 2, 12, 10, 0, 0)
+                }
+              ]
+            end
+
+            expect(captured_rows).to match_array(expected_rows)
+          end
+        end
+
         it 'respects account timezone for date bucketing' do
           # Event created at 2026-02-11 22:00 UTC
           # In EST (UTC-5) that's 2026-02-11 17:00 (same day)