From f56b0c9f2e790d0a220355d3c749cbb37fd47ee3 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 10 Mar 2026 13:20:05 +0530 Subject: [PATCH] fix: coerce nil rollup metric values to zero --- .../reporting_events/backfill_service.rb | 4 +- .../reporting_events/rollup_service.rb | 4 +- .../reporting_events/backfill_service_spec.rb | 45 +++++++++++++++++++ .../reporting_events/rollup_service_spec.rb | 35 +++++++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 spec/services/reporting_events/backfill_service_spec.rb diff --git a/app/services/reporting_events/backfill_service.rb b/app/services/reporting_events/backfill_service.rb index 428549cad..9798191e9 100644 --- a/app/services/reporting_events/backfill_service.rb +++ b/app/services/reporting_events/backfill_service.rb @@ -82,8 +82,8 @@ class ReportingEvents::BackfillService key = [dimension_type, dimension_id, metric] aggregates[key][:count] += metric_data[:count] - aggregates[key][:sum_value] += metric_data[:sum_value] - aggregates[key][:sum_value_business_hours] += metric_data[:sum_value_business_hours] + aggregates[key][:sum_value] += metric_data[:sum_value].to_f + aggregates[key][:sum_value_business_hours] += metric_data[:sum_value_business_hours].to_f end end end diff --git a/app/services/reporting_events/rollup_service.rb b/app/services/reporting_events/rollup_service.rb index 95dbee76b..c673a3ae2 100644 --- a/app/services/reporting_events/rollup_service.rb +++ b/app/services/reporting_events/rollup_service.rb @@ -131,8 +131,8 @@ module ReportingEvents::RollupService { account_id: @account.id, date: event_date, dimension_type: dimension_type, dimension_id: dimension_id, metric: metric, - count: metric_data[:count], sum_value: metric_data[:sum_value], - sum_value_business_hours: metric_data[:sum_value_business_hours], + count: metric_data[:count], sum_value: metric_data[:sum_value].to_f, + sum_value_business_hours: metric_data[:sum_value_business_hours].to_f, created_at: Time.current, updated_at: Time.current } end diff --git a/spec/services/reporting_events/backfill_service_spec.rb b/spec/services/reporting_events/backfill_service_spec.rb new file mode 100644 index 000000000..228f59c08 --- /dev/null +++ b/spec/services/reporting_events/backfill_service_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +describe ReportingEvents::BackfillService do + describe '.backfill_date' do + let(:account) { create(:account, reporting_timezone: 'America/New_York') } + let(:date) { Date.new(2026, 2, 11) } + let(:user) { create(:user, account: account) } + let(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox, assignee: user) } + + it 'treats nil metric values as zero during backfill' do + reporting_event = create( + :reporting_event, + account: account, + name: 'first_response', + value: 100, + value_in_business_hours: 50, + user: user, + inbox: inbox, + conversation: conversation, + created_at: Time.utc(2026, 2, 11, 15) + ) + # Simulate a legacy row that already exists in the database with nil metrics. + # rubocop:disable Rails/SkipsModelValidations + reporting_event.update_columns(value: nil, value_in_business_hours: nil) + # rubocop:enable Rails/SkipsModelValidations + + expect do + described_class.backfill_date(account, date) + end.not_to raise_error + + rollup = ReportingEventsRollup.find_by!( + account_id: account.id, + date: date, + dimension_type: 'account', + dimension_id: account.id, + metric: 'first_response' + ) + + expect(rollup.count).to eq(1) + expect(rollup.sum_value).to eq(0) + expect(rollup.sum_value_business_hours).to eq(0) + end + end +end diff --git a/spec/services/reporting_events/rollup_service_spec.rb b/spec/services/reporting_events/rollup_service_spec.rb index 1fc49634c..255a9dd5c 100644 --- a/spec/services/reporting_events/rollup_service_spec.rb +++ b/spec/services/reporting_events/rollup_service_spec.rb @@ -186,6 +186,41 @@ describe ReportingEvents::RollupService do end end + describe 'when metric values are nil' do + [ + %w[conversation_resolved resolution_time], + %w[first_response first_response], + %w[reply_time reply_time] + ].each do |event_name, metric| + it "treats nil values as zero for #{event_name}" do + reporting_event = create(:reporting_event, + account: account, + name: event_name, + value: 100, + value_in_business_hours: 50, + user: user, + inbox: inbox, + conversation: conversation) + reporting_event.assign_attributes(value: nil, value_in_business_hours: nil) + + expect do + described_class.perform(reporting_event) + end.not_to raise_error + + rollup = ReportingEventsRollup.find_by( + account_id: account.id, + dimension_type: 'account', + metric: metric + ) + + expect(rollup).to be_present + expect(rollup.count).to eq(1) + expect(rollup.sum_value).to eq(0) + expect(rollup.sum_value_business_hours).to eq(0) + end + end + end + describe 'conversation_bot_resolved event' do let(:reporting_event) do create(:reporting_event,