From efb4a2feca86a340564f962054654beb22afd927 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 11 Mar 2026 18:59:19 +0530 Subject: [PATCH] refactor: route reports through data sources --- .../v2/reports/base_summary_builder.rb | 102 ++---- .../v2/reports/concerns/rollup_conditions.rb | 75 ---- .../v2/reports/inbox_summary_builder.rb | 4 - .../v2/reports/team_summary_builder.rb | 4 - .../timeseries/average_report_builder.rb | 140 +------- .../timeseries/base_timeseries_builder.rb | 24 +- .../timeseries/count_report_builder.rb | 164 +-------- .../concerns/rollup_conditions_spec.rb | 327 ------------------ .../timeseries/average_report_builder_spec.rb | 7 +- 9 files changed, 60 insertions(+), 787 deletions(-) delete mode 100644 app/builders/v2/reports/concerns/rollup_conditions.rb delete mode 100644 spec/builders/v2/reports/concerns/rollup_conditions_spec.rb diff --git a/app/builders/v2/reports/base_summary_builder.rb b/app/builders/v2/reports/base_summary_builder.rb index 0ac3d32be..04a37e693 100644 --- a/app/builders/v2/reports/base_summary_builder.rb +++ b/app/builders/v2/reports/base_summary_builder.rb @@ -1,6 +1,6 @@ class V2::Reports::BaseSummaryBuilder include DateRangeHelper - include V2::Reports::Concerns::RollupConditions + include TimezoneHelper def build load_data @@ -9,80 +9,14 @@ class V2::Reports::BaseSummaryBuilder private - # Summary builders fetch all metrics in a single query, so the per-metric - # check from RollupConditions does not apply. - def metric_covered? - true - end - def load_data @conversations_count = fetch_conversations_count - use_rollup? ? load_rollup_data : load_reporting_events_data - end + results = data_source.summary - def load_reporting_events_data - # Extract the column name for indexing (e.g., 'conversations.team_id' -> 'team_id') - index_key = group_by_key.to_s.split('.').last - - results = reporting_events - .select( - "#{group_by_key} as #{index_key}", - "COUNT(CASE WHEN name = 'conversation_resolved' THEN 1 END) as resolved_count", - "AVG(CASE WHEN name = 'conversation_resolved' THEN #{average_value_key} END) as avg_resolution_time", - "AVG(CASE WHEN name = 'first_response' THEN #{average_value_key} END) as avg_first_response_time", - "AVG(CASE WHEN name = 'reply_time' THEN #{average_value_key} END) as avg_reply_time" - ) - .group(group_by_key) - .index_by { |record| record.public_send(index_key) } - - @resolved_count = results.transform_values(&:resolved_count) - @avg_resolution_time = results.transform_values(&:avg_resolution_time) - @avg_first_response_time = results.transform_values(&:avg_first_response_time) - @avg_reply_time = results.transform_values(&:avg_reply_time) - end - - # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity - def load_rollup_data - dimension_type = dimension_type_to_rollup - value_col = rollup_value_column - - # Fetch rollup data grouped by dimension_id - rollup_rows = ReportingEventsRollup.where( - account_id: account.id, - dimension_type: dimension_type, - date: rollup_date_range - ).group(:dimension_id).select( - 'dimension_id', - 'SUM(CASE WHEN metric = \'resolutions_count\' THEN count ELSE 0 END) as resolved_count', - "SUM(CASE WHEN metric = 'resolution_time' THEN count ELSE 0 END) as resolution_count", - "SUM(CASE WHEN metric = 'resolution_time' THEN #{value_col} ELSE 0 END) as resolution_sum_value", - "SUM(CASE WHEN metric = 'first_response' THEN count ELSE 0 END) as first_response_count", - "SUM(CASE WHEN metric = 'first_response' THEN #{value_col} ELSE 0 END) as first_response_sum_value", - "SUM(CASE WHEN metric = 'reply_time' THEN count ELSE 0 END) as reply_count", - "SUM(CASE WHEN metric = 'reply_time' THEN #{value_col} ELSE 0 END) as reply_sum_value" - ) - - # Convert to the expected format - results = {} - rollup_rows.each do |row| - results[row.dimension_id] = row - end - - @resolved_count = results.transform_values { |r| r.resolved_count.to_i } - @avg_resolution_time = results.transform_values do |r| - r.resolution_count.to_i.zero? ? nil : (r.resolution_sum_value.to_f / r.resolution_count.to_i) - end - @avg_first_response_time = results.transform_values do |r| - r.first_response_count.to_i.zero? ? nil : (r.first_response_sum_value.to_f / r.first_response_count.to_i) - end - @avg_reply_time = results.transform_values do |r| - 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) + @resolved_count = results.transform_values { |data| data[:resolved_conversations_count] } + @avg_resolution_time = results.transform_values { |data| data[:avg_resolution_time] } + @avg_first_response_time = results.transform_values { |data| data[:avg_first_response_time] } + @avg_reply_time = results.transform_values { |data| data[:avg_reply_time] } end def fetch_conversations_count @@ -97,7 +31,27 @@ class V2::Reports::BaseSummaryBuilder # Override this method end - def average_value_key - ActiveModel::Type::Boolean.new.cast(params[:business_hours]).present? ? :value_in_business_hours : :value + def data_source + @data_source ||= Reports::DataSource.for( + account: account, + metric: nil, + dimension_type: summary_dimension_type, + dimension_id: nil, + scope: nil, + range: range, + group_by: 'day', + timezone: timezone_name_from_offset(params[:timezone_offset]), + timezone_offset: params[:timezone_offset], + business_hours: params[:business_hours] + ) + end + + def summary_dimension_type + { + 'account_id' => 'account', + 'user_id' => 'agent', + 'inbox_id' => 'inbox', + 'conversations.team_id' => 'team' + }.fetch(group_by_key.to_s) end end diff --git a/app/builders/v2/reports/concerns/rollup_conditions.rb b/app/builders/v2/reports/concerns/rollup_conditions.rb deleted file mode 100644 index 8fa364322..000000000 --- a/app/builders/v2/reports/concerns/rollup_conditions.rb +++ /dev/null @@ -1,75 +0,0 @@ -module V2::Reports::Concerns::RollupConditions - COVERED_METRICS = { - avg_first_response_time: :first_response, - avg_resolution_time: :resolution_time, - reply_time: :reply_time, - resolutions_count: :resolutions_count, - bot_resolutions_count: :bot_resolutions_count, - bot_handoffs_count: :bot_handoffs_count - }.freeze - - SUPPORTED_DIMENSIONS = %w[account agent inbox].freeze - - def use_rollup? - # Condition 0: reporting_timezone must be configured on account - return false if account.reporting_timezone.blank? - - # Condition 1: Feature flag must be enabled - return false unless account.feature_enabled?('reporting_events_rollup') - - # Condition 2: Metric must be covered by rollup - return false unless metric_covered? - - # Condition 3: Not hourly granularity - return false if params[:group_by] == 'hour' - - # Condition 4: Dimension must be supported - return false unless dimension_supported? - - # Condition 5: Timezone must match - timezone_matches_account? - end - - private - - def metric_covered? - return false if params[:metric].blank? - - COVERED_METRICS.key?(params[:metric].to_sym) - end - - def dimension_supported? - return true if params[:type].blank? # account dimension (no specific ID) - - SUPPORTED_DIMENSIONS.include?(params[:type].to_s) - end - - def timezone_matches_account? - return false if params[:timezone_offset].blank? - - offset_in_seconds = params[:timezone_offset].to_f * 3600 - account_zone = ActiveSupport::TimeZone[account.reporting_timezone] - account_zone&.now&.utc_offset == offset_in_seconds - end - - def metric_to_rollup_metric(metric) - COVERED_METRICS[metric.to_sym] - end - - def rollup_value_column - ActiveModel::Type::Boolean.new.cast(params[:business_hours]).present? ? :sum_value_business_hours : :sum_value - end - - def rollup_date_range - tz = ActiveSupport::TimeZone[account.reporting_timezone] - start_date = range.first.in_time_zone(tz).to_date - # range is exclusive (since...until), so subtract a second to avoid - # including the until boundary day when it falls exactly at midnight. - end_date = (range.last - 1.second).in_time_zone(tz).to_date - start_date..end_date - end - - def dimension_type_to_rollup - { 'account' => :account, 'agent' => :agent, 'inbox' => :inbox }[params[:type].to_s] - end -end diff --git a/app/builders/v2/reports/inbox_summary_builder.rb b/app/builders/v2/reports/inbox_summary_builder.rb index 8897b2e30..93bc9d3cb 100644 --- a/app/builders/v2/reports/inbox_summary_builder.rb +++ b/app/builders/v2/reports/inbox_summary_builder.rb @@ -35,8 +35,4 @@ class V2::Reports::InboxSummaryBuilder < V2::Reports::BaseSummaryBuilder def group_by_key :inbox_id end - - def average_value_key - ActiveModel::Type::Boolean.new.cast(params[:business_hours]) ? :value_in_business_hours : :value - end end diff --git a/app/builders/v2/reports/team_summary_builder.rb b/app/builders/v2/reports/team_summary_builder.rb index b98151bc6..70c699324 100644 --- a/app/builders/v2/reports/team_summary_builder.rb +++ b/app/builders/v2/reports/team_summary_builder.rb @@ -10,10 +10,6 @@ class V2::Reports::TeamSummaryBuilder < V2::Reports::BaseSummaryBuilder account.conversations.where(created_at: range).group(:team_id).count end - def reporting_events - @reporting_events ||= account.reporting_events.where(created_at: range).joins(:conversation) - end - def prepare_report account.teams.map do |team| build_team_stats(team) diff --git a/app/builders/v2/reports/timeseries/average_report_builder.rb b/app/builders/v2/reports/timeseries/average_report_builder.rb index 4103807fe..945beee78 100644 --- a/app/builders/v2/reports/timeseries/average_report_builder.rb +++ b/app/builders/v2/reports/timeseries/average_report_builder.rb @@ -1,145 +1,9 @@ class V2::Reports::Timeseries::AverageReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder def timeseries - use_rollup? ? rollup_timeseries : raw_timeseries + data_source.timeseries end def aggregate_value - use_rollup? ? rollup_aggregate_value : raw_aggregate_value - end - - private - - def raw_timeseries - grouped_average_time = reporting_events.average(average_value_key) - grouped_event_count = reporting_events.count - grouped_average_time.each_with_object([]) do |element, arr| - event_date, average_time = element - arr << { - value: average_time, - timestamp: event_date.in_time_zone(timezone).to_i, - count: grouped_event_count[event_date] - } - end - end - - def rollup_timeseries - dimension_type = dimension_type_to_rollup - dimension_id = dimension_id_for_rollup - metric = metric_to_rollup_metric(params[:metric]) - - rollup_rows = ReportingEventsRollup.where( - account_id: account.id, - metric: metric, - dimension_type: dimension_type, - dimension_id: dimension_id, - date: rollup_date_range - ) - - group_and_aggregate_rollup(rollup_rows) - end - - def raw_aggregate_value - object_scope.average(average_value_key) - end - - def rollup_aggregate_value - dimension_type = dimension_type_to_rollup - dimension_id = dimension_id_for_rollup - metric = metric_to_rollup_metric(params[:metric]) - - value_col = rollup_value_column - - result = ReportingEventsRollup.where( - account_id: account.id, - metric: metric, - dimension_type: dimension_type, - dimension_id: dimension_id, - date: rollup_date_range - ).pick(Arel.sql("SUM(count), SUM(#{value_col})")) - - return nil if result.blank? || result[0].to_i.zero? - - result[1].to_f / result[0].to_i - end - - def dimension_id_for_rollup - params[:type].to_s == 'account' ? account.id : scope.id - end - - def group_and_aggregate_rollup(rollup_rows) - grouped_data = aggregate_rollup_rows(rollup_rows, all_periods_in_range.index_with { { count: 0, sum_value: 0.0 } }) - - results = grouped_data.map do |date_key, data| - value = data[:count].zero? ? 0 : data[:sum_value] / data[:count] - { value: value, timestamp: date_key.in_time_zone(timezone).to_i, count: data[:count] } - end - results.sort_by { |h| h[:timestamp] } - end - - def aggregate_rollup_rows(rollup_rows, grouped_data = {}) - value_col = rollup_value_column - rollup_rows.each_with_object(grouped_data) do |row, all_grouped_data| - date_key = date_key_for_group(row.date) - all_grouped_data[date_key] ||= { count: 0, sum_value: 0.0 } - all_grouped_data[date_key][:count] += row.count - all_grouped_data[date_key][:sum_value] += row.public_send(value_col) - end - end - - def date_key_for_group(date) - case group_by - when 'week' then date.beginning_of_week(:sunday) - when 'month' then date.beginning_of_month - when 'year' then date.beginning_of_year - else date - end - end - - def all_periods_in_range - date_range = rollup_date_range - dates = [] - current = date_key_for_group(date_range.first) - while current <= date_range.last - dates << current - current = advance_period(current) - end - dates - end - - def advance_period(date) - case group_by - when 'week' then date + 1.week - when 'month' then date + 1.month - when 'year' then date + 1.year - else date + 1.day - end - end - - def event_name - metric_to_event_name = { - avg_first_response_time: :first_response, - avg_resolution_time: :conversation_resolved, - reply_time: :reply_time - } - metric_to_event_name[params[:metric].to_sym] - end - - def object_scope - scope.reporting_events.where(name: event_name, created_at: range, account_id: account.id) - end - - def reporting_events - @grouped_values = object_scope.group_by_period( - group_by, - :created_at, - default_value: 0, - range: range, - permit: %w[day week month year hour], - time_zone: timezone - ) - end - - def average_value_key - @average_value_key ||= params[:business_hours].present? ? :value_in_business_hours : :value + data_source.aggregate end end diff --git a/app/builders/v2/reports/timeseries/base_timeseries_builder.rb b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb index e8e5245ab..e1c668e44 100644 --- a/app/builders/v2/reports/timeseries/base_timeseries_builder.rb +++ b/app/builders/v2/reports/timeseries/base_timeseries_builder.rb @@ -1,14 +1,13 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder include TimezoneHelper include DateRangeHelper - include V2::Reports::Concerns::RollupConditions DEFAULT_GROUP_BY = 'day'.freeze pattr_initialize :account, :params def scope - case params[:type].to_sym + case dimension_type.to_sym when :account account when :inbox @@ -22,6 +21,21 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder end end + def data_source + @data_source ||= Reports::DataSource.for( + account: account, + metric: params[:metric], + dimension_type: dimension_type, + dimension_id: params[:id], + scope: scope, + range: range, + group_by: group_by, + timezone: timezone, + timezone_offset: params[:timezone_offset], + business_hours: params[:business_hours] + ) + end + def inbox @inbox ||= account.inboxes.find(params[:id]) end @@ -45,4 +59,10 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder def timezone @timezone ||= timezone_name_from_offset(params[:timezone_offset]) end + + private + + def dimension_type + (params[:type].presence || 'account').to_s + end end diff --git a/app/builders/v2/reports/timeseries/count_report_builder.rb b/app/builders/v2/reports/timeseries/count_report_builder.rb index 0192c242c..07e651718 100644 --- a/app/builders/v2/reports/timeseries/count_report_builder.rb +++ b/app/builders/v2/reports/timeseries/count_report_builder.rb @@ -1,169 +1,9 @@ class V2::Reports::Timeseries::CountReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder def timeseries - use_rollup? ? rollup_timeseries : raw_timeseries + data_source.timeseries end def aggregate_value - use_rollup? ? rollup_aggregate_value : raw_aggregate_value - end - - private - - def raw_timeseries - grouped_count.each_with_object([]) do |element, arr| - event_date, event_count = element - - # The `event_date` is in Date format (without time), such as "Wed, 15 May 2024". - # We need a timestamp for the start of the day. However, we can't use `event_date.to_time.to_i` - # because it converts the date to 12:00 AM server timezone. - # The desired output should be 12:00 AM in the specified timezone. - arr << { value: event_count, timestamp: event_date.in_time_zone(timezone).to_i } - end - end - - def rollup_timeseries - metric = metric_to_rollup_metric(params[:metric]) - return [] if metric.blank? - - dimension_type = dimension_type_to_rollup - dimension_id = dimension_id_for_rollup - - rollup_rows = ReportingEventsRollup.where( - account_id: account.id, - metric: metric, - dimension_type: dimension_type, - dimension_id: dimension_id, - date: rollup_date_range - ) - - group_and_aggregate_rollup_counts(rollup_rows) - end - - def raw_aggregate_value - object_scope.count - end - - def rollup_aggregate_value - metric = metric_to_rollup_metric(params[:metric]) - return 0 if metric.blank? - - dimension_type = dimension_type_to_rollup - dimension_id = dimension_id_for_rollup - - ReportingEventsRollup.where( - account_id: account.id, - metric: metric, - dimension_type: dimension_type, - dimension_id: dimension_id, - date: rollup_date_range - ).sum(:count).to_i - end - - def dimension_id_for_rollup - params[:type].to_s == 'account' ? account.id : scope.id - end - - def group_and_aggregate_rollup_counts(rollup_rows) - # Pre-fill all periods with zero to match raw path's default_value: 0 behavior - grouped_data = all_periods_in_range.index_with { 0 } - - rollup_rows.each do |row| - date_key = date_key_for_period(row.date) - grouped_data[date_key] ||= 0 - grouped_data[date_key] += row.count - end - - results = grouped_data.map do |date_key, count| - { value: count, timestamp: date_key.in_time_zone(timezone).to_i } - end - results.sort_by { |h| h[:timestamp] } - end - - def date_key_for_period(date) - case group_by - when 'week' then date.beginning_of_week(:sunday) - when 'month' then date.beginning_of_month - when 'year' then date.beginning_of_year - else date - end - end - - def all_periods_in_range - date_range = rollup_date_range - dates = [] - current = date_key_for_period(date_range.first) - while current <= date_range.last - dates << current - current = advance_period(current) - end - dates - end - - def advance_period(date) - case group_by - when 'week' then date + 1.week - when 'month' then date + 1.month - when 'year' then date + 1.year - else date + 1.day - end - end - - def metric - @metric ||= params[:metric] - end - - def object_scope - send("scope_for_#{metric}") - end - - def scope_for_conversations_count - scope.conversations.where(account_id: account.id, created_at: range) - end - - def scope_for_incoming_messages_count - scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order) - end - - def scope_for_outgoing_messages_count - scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order) - end - - def scope_for_resolutions_count - scope.reporting_events.where( - name: :conversation_resolved, - account_id: account.id, - created_at: range - ) - end - - def scope_for_bot_resolutions_count - scope.reporting_events.where( - name: :conversation_bot_resolved, - account_id: account.id, - created_at: range - ) - end - - def scope_for_bot_handoffs_count - scope.reporting_events.joins(:conversation).select(:conversation_id).where( - name: :conversation_bot_handoff, - account_id: account.id, - created_at: range - ).distinct - end - - def grouped_count - # IMPORTANT: time_zone parameter affects both data grouping AND output timestamps - # It converts timestamps to the target timezone before grouping, which means - # the same event can fall into different day buckets depending on timezone - # Example: 2024-01-15 00:00 UTC becomes 2024-01-14 16:00 PST (falls on different day) - @grouped_values = object_scope.group_by_period( - group_by, - :created_at, - default_value: 0, - range: range, - permit: %w[day week month year hour], - time_zone: timezone - ).count + data_source.aggregate end end diff --git a/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb b/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb deleted file mode 100644 index 8f35bd0a6..000000000 --- a/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb +++ /dev/null @@ -1,327 +0,0 @@ -require 'rails_helper' - -describe V2::Reports::Concerns::RollupConditions do - let(:dummy_class) do - Class.new do - include V2::Reports::Concerns::RollupConditions - - attr_accessor :account, :params - - # Make private methods public for testing - public :metric_to_rollup_metric, :dimension_type_to_rollup, :timezone_matches_account? - end - end - - let(:account) { create(:account, reporting_timezone: 'America/New_York') } - let(:builder) { dummy_class.new } - # Compute the current offset dynamically so tests pass in both EST and EDT - let(:current_ny_offset) { ActiveSupport::TimeZone['America/New_York'].now.utc_offset / 3600.0 } - - before do - builder.account = account - allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) - end - - describe '#use_rollup?' do - let(:valid_params) do - { - metric: 'avg_resolution_time', - type: 'account', - group_by: 'day', - timezone_offset: current_ny_offset - } - end - - context 'when all conditions pass' do - it 'returns true' do - builder.params = valid_params - expect(builder.use_rollup?).to be true - end - end - - context 'when reporting_timezone is blank' do - it 'returns false' do - account.update!(reporting_timezone: nil) - builder.params = valid_params - expect(builder.use_rollup?).to be false - end - end - - context 'when feature flag is disabled' do - it 'returns false' do - allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(false) - builder.params = valid_params - expect(builder.use_rollup?).to be false - end - end - - context 'when metric is not covered' do - it 'returns false for conversations_count' do - builder.params = valid_params.merge(metric: 'conversations_count') - - expect(builder.use_rollup?).to be false - end - - it 'returns false for incoming_messages_count' do - builder.params = valid_params.merge(metric: 'incoming_messages_count') - - expect(builder.use_rollup?).to be false - end - - it 'returns false for outgoing_messages_count' do - builder.params = valid_params.merge(metric: 'outgoing_messages_count') - - expect(builder.use_rollup?).to be false - end - end - - context 'when metric is covered' do - it 'returns true for avg_first_response_time' do - builder.params = valid_params.merge(metric: 'avg_first_response_time') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for avg_resolution_time' do - builder.params = valid_params - expect(builder.use_rollup?).to be true - end - - it 'returns true for reply_time' do - builder.params = valid_params.merge(metric: 'reply_time') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for resolutions_count' do - builder.params = valid_params.merge(metric: 'resolutions_count') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for bot_resolutions_count' do - builder.params = valid_params.merge(metric: 'bot_resolutions_count') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for bot_handoffs_count' do - builder.params = valid_params.merge(metric: 'bot_handoffs_count') - - expect(builder.use_rollup?).to be true - end - end - - context 'when group_by is hourly' do - it 'returns false when group_by is hour' do - builder.params = valid_params.merge(group_by: 'hour') - expect(builder.use_rollup?).to be false - end - - it 'returns true for day granularity' do - builder.params = valid_params - expect(builder.use_rollup?).to be true - end - - it 'returns true for week granularity' do - builder.params = valid_params.merge(group_by: 'week') - expect(builder.use_rollup?).to be true - end - - it 'returns true for month granularity' do - builder.params = valid_params.merge(group_by: 'month') - expect(builder.use_rollup?).to be true - end - - it 'returns true for year granularity' do - builder.params = valid_params.merge(group_by: 'year') - expect(builder.use_rollup?).to be true - end - end - - context 'when dimension is not supported' do - it 'returns false for label dimension' do - builder.params = valid_params.merge(type: 'label') - - expect(builder.use_rollup?).to be false - end - - it 'returns true for account dimension' do - builder.params = valid_params.merge(type: 'account') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for agent dimension' do - builder.params = valid_params.merge(type: 'agent') - - expect(builder.use_rollup?).to be true - end - - it 'returns true for inbox dimension' do - builder.params = valid_params.merge(type: 'inbox') - - expect(builder.use_rollup?).to be true - end - - it 'returns false for team dimension' do - builder.params = valid_params.merge(type: 'team') - - expect(builder.use_rollup?).to be false - end - end - - context 'when timezone_offset does not match' do - it 'returns false when timezone_offset is UTC' do - builder.params = valid_params.merge(timezone_offset: 0) - - expect(builder.use_rollup?).to be false - end - - it 'returns false when timezone_offset is UTC+5:30' do - builder.params = valid_params.merge(timezone_offset: 5.5) - - expect(builder.use_rollup?).to be false - end - - it 'returns true when timezone_offset matches account timezone' do - builder.params = valid_params.merge(timezone_offset: current_ny_offset) - - expect(builder.use_rollup?).to be true - end - - it 'returns false when timezone_offset is blank' do - builder.params = valid_params.merge(timezone_offset: nil) - - expect(builder.use_rollup?).to be false - end - end - - context 'when used from BaseSummaryBuilder (no params[:metric])' do - it 'returns true because summary builders override metric_covered?' do - builder.params = { type: 'agent', timezone_offset: current_ny_offset } - - # Without the override, this would return false since params[:metric] is blank - expect(builder.use_rollup?).to be false - - # BaseSummaryBuilder overrides metric_covered? to always return true - summary_builder_class = Class.new(dummy_class) do - def metric_covered? - true - end - end - - summary_builder = summary_builder_class.new - summary_builder.account = account - summary_builder.params = { type: 'agent', timezone_offset: current_ny_offset, group_by: 'day' } - expect(summary_builder.use_rollup?).to be true - end - end - end - - describe '#metric_to_rollup_metric' do - it 'maps avg_resolution_time to resolution_time' do - builder.params = { metric: 'avg_resolution_time' } - expect(builder.metric_to_rollup_metric('avg_resolution_time')).to eq(:resolution_time) - end - - it 'maps avg_first_response_time to first_response' do - builder.params = { metric: 'avg_first_response_time' } - expect(builder.metric_to_rollup_metric('avg_first_response_time')).to eq(:first_response) - end - - it 'maps reply_time to reply_time' do - builder.params = { metric: 'reply_time' } - expect(builder.metric_to_rollup_metric('reply_time')).to eq(:reply_time) - end - - it 'maps resolutions_count to resolutions_count' do - builder.params = { metric: 'resolutions_count' } - expect(builder.metric_to_rollup_metric('resolutions_count')).to eq(:resolutions_count) - end - - it 'maps bot_resolutions_count to bot_resolutions_count' do - builder.params = { metric: 'bot_resolutions_count' } - expect(builder.metric_to_rollup_metric('bot_resolutions_count')).to eq(:bot_resolutions_count) - end - - it 'maps bot_handoffs_count to bot_handoffs_count' do - builder.params = { metric: 'bot_handoffs_count' } - expect(builder.metric_to_rollup_metric('bot_handoffs_count')).to eq(:bot_handoffs_count) - end - end - - describe '#dimension_type_to_rollup' do - it 'maps account type to account' do - builder.params = { type: 'account' } - expect(builder.dimension_type_to_rollup).to eq(:account) - end - - it 'maps agent type to agent' do - builder.params = { type: 'agent' } - expect(builder.dimension_type_to_rollup).to eq(:agent) - end - - it 'maps inbox type to inbox' do - builder.params = { type: 'inbox' } - expect(builder.dimension_type_to_rollup).to eq(:inbox) - end - - it 'returns nil for team type' do - builder.params = { type: 'team' } - expect(builder.dimension_type_to_rollup).to be_nil - end - - it 'returns nil for unsupported type' do - builder.params = { type: 'label' } - expect(builder.dimension_type_to_rollup).to be_nil - end - - it 'returns nil when type is blank' do - builder.params = { type: nil } - expect(builder.dimension_type_to_rollup).to be_nil - end - end - - describe '#timezone_matches_account?' do - it 'returns false when timezone_offset is blank' do - builder.params = { timezone_offset: nil } - expect(builder.timezone_matches_account?).to be false - end - - it 'returns true when offset matches account timezone current offset' do - builder.params = { timezone_offset: current_ny_offset } - expect(builder.timezone_matches_account?).to be true - end - - it 'returns false when offset does not match account timezone' do - # UTC+5:30 (IST) will never match America/New_York - builder.params = { timezone_offset: 5.5 } - expect(builder.timezone_matches_account?).to be false - end - - it 'returns false when account timezone is not recognized' do - allow(account).to receive(:reporting_timezone).and_return('Invalid/Zone') - builder.params = { timezone_offset: current_ny_offset } - expect(builder.timezone_matches_account?).to be false - end - - it 'works with fractional offsets like UTC+5:45' do - account.update!(reporting_timezone: 'Asia/Kathmandu') - kathmandu_offset = ActiveSupport::TimeZone['Asia/Kathmandu'].now.utc_offset / 3600.0 - builder.params = { timezone_offset: kathmandu_offset } - expect(builder.timezone_matches_account?).to be true - end - - it 'handles DST-aware comparison correctly' do - # Use a timezone with no DST (UTC) to have a stable control - account.update!(reporting_timezone: 'Etc/UTC') - builder.params = { timezone_offset: 0 } - expect(builder.timezone_matches_account?).to be true - - # A non-zero offset should not match UTC - builder.params = { timezone_offset: -5 } - expect(builder.timezone_matches_account?).to be false - end - end -end diff --git a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb index 347fc9944..bf174089c 100644 --- a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb +++ b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb @@ -108,7 +108,12 @@ describe V2::Reports::Timeseries::AverageReportBuilder do end it 'preserves empty buckets in the timeseries' do - expected_timeseries = subject.send(:rollup_date_range).map do |date| + rollup_timezone = ActiveSupport::TimeZone['Chennai'] + rollup_start_date = DateTime.strptime(params[:since], '%s').in_time_zone(rollup_timezone).to_date + rollup_end_date = (DateTime.strptime(params[:until], '%s') - 1.second).in_time_zone(rollup_timezone).to_date + rollup_dates = rollup_start_date..rollup_end_date + + expected_timeseries = rollup_dates.map do |date| value = if date == (current_time - 1.week).to_date 93.0 elsif date == current_time.to_date