diff --git a/app/builders/v2/reports/concerns/rollup_conditions.rb b/app/builders/v2/reports/concerns/rollup_conditions.rb index 5513ef20d..a908b028f 100644 --- a/app/builders/v2/reports/concerns/rollup_conditions.rb +++ b/app/builders/v2/reports/concerns/rollup_conditions.rb @@ -73,7 +73,9 @@ module V2::Reports::Concerns::RollupConditions def rollup_date_range tz = ActiveSupport::TimeZone[account.reporting_timezone] start_date = range.first.in_time_zone(tz).to_date - end_date = range.last.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 diff --git a/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb b/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb index 916c04d0e..dcc6cd20b 100644 --- a/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb +++ b/spec/builders/v2/reports/concerns/rollup_conditions_spec.rb @@ -20,7 +20,7 @@ describe V2::Reports::Concerns::RollupConditions do before do builder.account = account - account.enable_features!('reporting_events_rollup') + allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true) end describe '#use_rollup?' do @@ -52,7 +52,7 @@ describe V2::Reports::Concerns::RollupConditions do context 'Condition 1: feature flag is disabled' do it 'returns false' do - account.disable_features!('reporting_events_rollup') + 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 @@ -206,6 +206,28 @@ describe V2::Reports::Concerns::RollupConditions do expect(builder.use_rollup?).to be true 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: -5 } + builder._group_by = 'day' + # 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: -5 } + summary_builder._group_by = 'day' + expect(summary_builder.use_rollup?).to be true + end + end end describe '#metric_to_rollup_metric' do