From 9a17ae6a76bcf3514e517d86df31970f8f86816b Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 11 Feb 2026 22:42:19 +0530 Subject: [PATCH] refactor: Add use_rollup parameter to base summary builder Add explicit use_rollup parameter to BaseSummaryBuilder that can override the automatic rollup detection logic. This allows the comparison rake task to explicitly control whether to use rollup or raw data without toggling feature flags. Cleaner approach that avoids database writes during comparison. --- .../v2/reports/base_summary_builder.rb | 10 ++++++++ .../reporting_events_rollup_compare.rake | 25 ++++++------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/app/builders/v2/reports/base_summary_builder.rb b/app/builders/v2/reports/base_summary_builder.rb index 7883b26ed..968c9bc02 100644 --- a/app/builders/v2/reports/base_summary_builder.rb +++ b/app/builders/v2/reports/base_summary_builder.rb @@ -9,6 +9,14 @@ class V2::Reports::BaseSummaryBuilder private + def use_rollup? + # If use_rollup is explicitly set in params, respect that + return params[:use_rollup] if params.key?(:use_rollup) + + # Otherwise, use the default rollup conditions + super + end + def load_data @conversations_count = fetch_conversations_count use_rollup? ? load_rollup_data : load_reporting_events_data @@ -35,6 +43,7 @@ class V2::Reports::BaseSummaryBuilder @avg_reply_time = results.transform_values(&:avg_reply_time) end + # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity def load_rollup_data group_by_key.to_s.split('.').last dimension_type = dimension_type_to_rollup @@ -72,6 +81,7 @@ class V2::Reports::BaseSummaryBuilder r.reply_count.to_i.zero? ? nil : (r.reply_sum_value.to_f / r.reply_count.to_i) end end + # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity def reporting_events @reporting_events ||= account.reporting_events.where(created_at: range) diff --git a/lib/tasks/reporting_events_rollup_compare.rake b/lib/tasks/reporting_events_rollup_compare.rake index deca42fa8..2c3045646 100644 --- a/lib/tasks/reporting_events_rollup_compare.rake +++ b/lib/tasks/reporting_events_rollup_compare.rake @@ -101,8 +101,8 @@ namespace :reporting_events_rollup do puts "Comparing #{dimension[:name]} summaries..." weeks.each_with_index do |week, week_index| - # Build params for this week - params = { + # Build base params for this week + base_params = { since: week[:start].to_s, until: week[:end].to_s, type: dimension[:name], @@ -110,13 +110,13 @@ namespace :reporting_events_rollup do business_hours: false } - # Get raw data (disable rollup temporarily) - raw_results = with_rollup_disabled(account) do - dimension[:builder].new(account, params).build - end + # Get raw data (explicitly disable rollup) + raw_params = base_params.merge(use_rollup: false) + raw_results = dimension[:builder].new(account: account, params: raw_params).build - # Get rollup data (with rollup enabled) - rollup_results = dimension[:builder].new(account, params).build + # Get rollup data (explicitly enable rollup) + rollup_params = base_params.merge(use_rollup: true) + rollup_results = dimension[:builder].new(account: account, params: rollup_params).build # Compare results dimension[:entities].each do |entity_id| @@ -215,15 +215,6 @@ namespace :reporting_events_rollup do end end - # Helper: Temporarily disable rollup for an account - def with_rollup_disabled(account) - original_flag = account.feature_enabled?('reporting_events_rollup') - account.disable_features!('reporting_events_rollup') if original_flag - result = yield - account.enable_features!('reporting_events_rollup') if original_flag - result - end - # Helper: Compare two metric hashes def compare_metrics(raw, rollup) metrics_to_compare = %i[