fix: exclude until boundary from rollup date range
DateRangeHelper#range builds an exclusive range (since...until), but rollup_date_range was converting range.last directly to a date, causing an extra day to be included when until falls at midnight.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user