fix: emit zero-value periods in rollup count timeseries

The rollup path only emitted periods with data, while the raw path
uses group_by_period with default_value: 0 to include inactive periods.
This also reads group_by from params instead of calling a method that
summary builders don't implement.

app/builders/v2/report_builder.rb

@@ -1,6 +1,7 @@
 class V2::ReportBuilder
   include DateRangeHelper
   include ReportHelper
+
   attr_reader :account, :params
 
   DEFAULT_GROUP_BY = 'day'.freeze

app/builders/v2/reports/concerns/rollup_conditions.rb

@@ -21,7 +21,7 @@ module V2::Reports::Concerns::RollupConditions
     return false unless metric_covered?
 
     # Condition 3: Not hourly granularity
-    return false if group_by == 'hour'
+    return false if params[:group_by] == 'hour'
 
     # Condition 4: Dimension must be supported
     return false unless dimension_supported?

app/builders/v2/reports/timeseries/base_timeseries_builder.rb

@@ -2,6 +2,7 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder
   include TimezoneHelper
   include DateRangeHelper
   include V2::Reports::Concerns::RollupConditions
+
   DEFAULT_GROUP_BY = 'day'.freeze
 
   pattr_initialize :account, :params

app/builders/v2/reports/timeseries/count_report_builder.rb

@@ -73,34 +73,51 @@ class V2::Reports::Timeseries::CountReportBuilder < V2::Reports::Timeseries::Bas
   end
 
   def group_and_aggregate_rollup_counts(rollup_rows)
-    grouped_data = {}
+    # 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 = case group_by
-                 when 'day'
-                   row.date
-                 when 'week'
-                   row.date.beginning_of_week(:monday)
-                 when 'month'
-                   row.date.beginning_of_month
-                 when 'year'
-                   row.date.beginning_of_year
-                 else
-                   row.date
-                 end
-
+      date_key = date_key_for_period(row.date)
       grouped_data[date_key] ||= 0
       grouped_data[date_key] += row.count
     end
 
-    grouped_data.each_with_object([]) do |(date_key, count), arr|
-      arr << {
-        value: count,
-        timestamp: date_key.in_time_zone(timezone).to_i
-      }
+    grouped_data.map do |date_key, count|
+      { value: count, timestamp: date_key.in_time_zone(timezone).to_i }
     end.sort_by { |h| h[:timestamp] }
   end
 
+  def date_key_for_period(date)
+    case group_by
+    when 'day' then date
+    when 'week' then date.beginning_of_week(:monday)
+    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 'day' then date + 1.day
+    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

spec/builders/v2/reports/concerns/rollup_conditions_spec.rb

@@ -4,11 +4,8 @@ describe V2::Reports::Concerns::RollupConditions do
   let(:dummy_class) do
     Class.new do
       include V2::Reports::Concerns::RollupConditions
-      attr_accessor :account, :params, :_group_by
 
-      def group_by
-        @_group_by || 'day'
-      end
+      attr_accessor :account, :params
 
       # Make private methods public for testing
       public :metric_to_rollup_metric, :dimension_type_to_rollup
@@ -36,7 +33,6 @@ describe V2::Reports::Concerns::RollupConditions do
     context 'when all conditions pass' do
       it 'returns true' do
         builder.params = valid_params
-        builder._group_by = 'day'
         expect(builder.use_rollup?).to be true
       end
     end
@@ -45,7 +41,6 @@ describe V2::Reports::Concerns::RollupConditions do
       it 'returns false' do
         account.update!(reporting_timezone: nil)
         builder.params = valid_params
-        builder._group_by = 'day'
         expect(builder.use_rollup?).to be false
       end
     end
@@ -54,7 +49,6 @@ describe V2::Reports::Concerns::RollupConditions do
       it 'returns false' do
         allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(false)
         builder.params = valid_params
-        builder._group_by = 'day'
         expect(builder.use_rollup?).to be false
       end
     end
@@ -62,19 +56,19 @@ describe V2::Reports::Concerns::RollupConditions do
     context 'Condition 2: metric is not covered' do
       it 'returns false for conversations_count' do
         builder.params = valid_params.merge(metric: 'conversations_count')
-        builder._group_by = 'day'
+
         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')
-        builder._group_by = 'day'
+
         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')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be false
       end
     end
@@ -82,69 +76,63 @@ describe V2::Reports::Concerns::RollupConditions do
     context 'Condition 2: metric is covered' do
       it 'returns true for avg_first_response_time' do
         builder.params = valid_params.merge(metric: 'avg_first_response_time')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for avg_resolution_time' do
         builder.params = valid_params
-        builder._group_by = 'day'
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for reply_time' do
         builder.params = valid_params.merge(metric: 'reply_time')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for resolutions_count' do
         builder.params = valid_params.merge(metric: 'resolutions_count')
-        builder._group_by = 'day'
+
         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')
-        builder._group_by = 'day'
+
         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')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
     end
 
     context 'Condition 3: group_by is hourly' do
       it 'returns false when group_by is hour' do
-        builder.params = valid_params
-        builder._group_by = 'hour'
+        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
-        builder._group_by = 'day'
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for week granularity' do
-        builder.params = valid_params
-        builder._group_by = 'week'
+        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
-        builder._group_by = 'month'
+        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
-        builder._group_by = 'year'
+        builder.params = valid_params.merge(group_by: 'year')
         expect(builder.use_rollup?).to be true
       end
     end
@@ -152,31 +140,31 @@ describe V2::Reports::Concerns::RollupConditions do
     context 'Condition 4: dimension is not supported' do
       it 'returns false for label dimension' do
         builder.params = valid_params.merge(type: 'label')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be false
       end
 
       it 'returns true for account dimension' do
         builder.params = valid_params.merge(type: 'account')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for agent dimension' do
         builder.params = valid_params.merge(type: 'agent')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for inbox dimension' do
         builder.params = valid_params.merge(type: 'inbox')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true for team dimension' do
         builder.params = valid_params.merge(type: 'team')
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
     end
@@ -184,25 +172,25 @@ describe V2::Reports::Concerns::RollupConditions do
     context 'Condition 5: timezone_offset does not match' do
       it 'returns false when timezone_offset is UTC instead of EST' do
         builder.params = valid_params.merge(timezone_offset: 0)
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be false
       end
 
       it 'returns false when timezone_offset is UTC+5:30 (different from EST)' do
         builder.params = valid_params.merge(timezone_offset: 5.5)
-        builder._group_by = 'day'
+
         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: -5)
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
 
       it 'returns true when timezone_offset is blank (defaults to account timezone)' do
         builder.params = valid_params.merge(timezone_offset: nil)
-        builder._group_by = 'day'
+
         expect(builder.use_rollup?).to be true
       end
     end
@@ -210,7 +198,7 @@ describe V2::Reports::Concerns::RollupConditions do
     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: -5 }
-        builder._group_by = 'day'
+
         # Without the override, this would return false since params[:metric] is blank
         expect(builder.use_rollup?).to be false
 
@@ -223,8 +211,7 @@ describe V2::Reports::Concerns::RollupConditions do
 
         summary_builder = summary_builder_class.new
         summary_builder.account = account
-        summary_builder.params = { type: 'agent', timezone_offset: -5 }
-        summary_builder._group_by = 'day'
+        summary_builder.params = { type: 'agent', timezone_offset: -5, group_by: 'day' }
         expect(summary_builder.use_rollup?).to be true
       end
     end