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.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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[
|
||||
|
||||
Reference in New Issue
Block a user