diff --git a/app/builders/v2/reports/agent_summary_builder.rb b/app/builders/v2/reports/agent_summary_builder.rb index 9b4541aaa..58689aaad 100644 --- a/app/builders/v2/reports/agent_summary_builder.rb +++ b/app/builders/v2/reports/agent_summary_builder.rb @@ -11,10 +11,6 @@ class V2::Reports::AgentSummaryBuilder < V2::Reports::BaseSummaryBuilder attr_reader :conversations_count, :resolved_count, :avg_resolution_time, :avg_first_response_time, :avg_reply_time - def fetch_conversations_count - account.conversations.where(created_at: range).group('assignee_id').count - end - def prepare_report account.account_users.map do |account_user| build_agent_stats(account_user) diff --git a/app/builders/v2/reports/base_summary_builder.rb b/app/builders/v2/reports/base_summary_builder.rb index 04a37e693..ec98a7a63 100644 --- a/app/builders/v2/reports/base_summary_builder.rb +++ b/app/builders/v2/reports/base_summary_builder.rb @@ -10,19 +10,15 @@ class V2::Reports::BaseSummaryBuilder private def load_data - @conversations_count = fetch_conversations_count results = data_source.summary + @conversations_count = results.transform_values { |data| data[:conversations_count] } @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 - # Override this method - end - def group_by_key # Override this method end diff --git a/app/builders/v2/reports/inbox_summary_builder.rb b/app/builders/v2/reports/inbox_summary_builder.rb index 93bc9d3cb..fcfabc599 100644 --- a/app/builders/v2/reports/inbox_summary_builder.rb +++ b/app/builders/v2/reports/inbox_summary_builder.rb @@ -11,10 +11,6 @@ class V2::Reports::InboxSummaryBuilder < V2::Reports::BaseSummaryBuilder attr_reader :conversations_count, :resolved_count, :avg_resolution_time, :avg_first_response_time, :avg_reply_time - def fetch_conversations_count - account.conversations.where(created_at: range).group(group_by_key).count - end - def prepare_report account.inboxes.map do |inbox| build_inbox_stats(inbox) diff --git a/app/builders/v2/reports/team_summary_builder.rb b/app/builders/v2/reports/team_summary_builder.rb index 70c699324..3bcf51816 100644 --- a/app/builders/v2/reports/team_summary_builder.rb +++ b/app/builders/v2/reports/team_summary_builder.rb @@ -6,10 +6,6 @@ class V2::Reports::TeamSummaryBuilder < V2::Reports::BaseSummaryBuilder attr_reader :conversations_count, :resolved_count, :avg_resolution_time, :avg_first_response_time, :avg_reply_time - def fetch_conversations_count - account.conversations.where(created_at: range).group(:team_id).count - end - def prepare_report account.teams.map do |team| build_team_stats(team) diff --git a/app/services/reporting_events/metric_registry.rb b/app/services/reporting_events/metric_registry.rb index 791bdf700..363ebf69b 100644 --- a/app/services/reporting_events/metric_registry.rb +++ b/app/services/reporting_events/metric_registry.rb @@ -1,3 +1,8 @@ +# Raw reporting events and rollup rows do not share a single metric namespace. +# For example, the raw `conversation_resolved` event emits both the +# `resolutions_count` and `resolution_time` rollup metrics. This registry keeps +# that mapping in one place so the write path, rollup reads, and raw reads stay +# aligned. module ReportingEvents::MetricRegistry EVENT_METRICS = { 'conversation_resolved' => lambda do |event| diff --git a/app/services/reports/raw_data_source.rb b/app/services/reports/raw_data_source.rb index 07be03097..23ccf0764 100644 --- a/app/services/reports/raw_data_source.rb +++ b/app/services/reports/raw_data_source.rb @@ -8,19 +8,12 @@ class Reports::RawDataSource < Reports::DataSource end def summary - results = summary_scope - .select(*summary_select_fields) - .group(summary_group_by_key) - .index_by { |record| record.public_send(summary_index_key) } + metric_results = summary_scope + .select(*summary_select_fields) + .group(summary_group_by_key) + .index_by { |record| record.public_send(summary_index_key) } - results.transform_values do |record| - { - resolved_conversations_count: record.resolved_count.to_i, - avg_resolution_time: record.avg_resolution_time, - avg_first_response_time: record.avg_first_response_time, - avg_reply_time: record.avg_reply_time - } - end + merge_summary_results(metric_results, summary_conversation_counts) end private @@ -102,6 +95,27 @@ class Reports::RawDataSource < Reports::DataSource scope end + def summary_conversation_counts + account.conversations + .where(created_at: range) + .group(summary_conversation_group_by_key) + .count + end + + def merge_summary_results(metric_results, conversation_counts) + (metric_results.keys | conversation_counts.keys).each_with_object({}) do |dimension_id, results| + record = metric_results[dimension_id] + + results[dimension_id] = { + conversations_count: conversation_counts[dimension_id].to_i, + resolved_conversations_count: record&.resolved_count.to_i, + avg_resolution_time: record&.avg_resolution_time, + avg_first_response_time: record&.avg_first_response_time, + avg_reply_time: record&.avg_reply_time + } + end + end + def summary_select_fields [ "#{summary_group_by_key} as #{summary_index_key}", @@ -133,6 +147,15 @@ class Reports::RawDataSource < Reports::DataSource }[dimension_type] end + def summary_conversation_group_by_key + { + 'account' => :account_id, + 'agent' => :assignee_id, + 'inbox' => :inbox_id, + 'team' => :team_id + }[dimension_type] + end + def summary_index_key summary_group_by_key.to_s.split('.').last end diff --git a/app/services/reports/rollup_data_source.rb b/app/services/reports/rollup_data_source.rb index 1f2383bfa..4cd75e729 100644 --- a/app/services/reports/rollup_data_source.rb +++ b/app/services/reports/rollup_data_source.rb @@ -8,9 +8,9 @@ class Reports::RollupDataSource < Reports::DataSource end def summary - summary_rows.each_with_object({}) do |row, results| - results[row.dimension_id] = summary_attributes_for(row) - end + metric_results = summary_rows.index_by(&:dimension_id) + + merge_summary_results(metric_results, summary_conversation_counts) end private @@ -76,6 +76,19 @@ class Reports::RollupDataSource < Reports::DataSource ).group(:dimension_id).select(*summary_select_fields) end + def summary_conversation_counts + account.conversations + .where(created_at: range) + .group(summary_conversation_group_by_key) + .count + end + + def merge_summary_results(metric_results, conversation_counts) + (metric_results.keys | conversation_counts.keys).index_with do |dimension_id| + summary_attributes_for(metric_results[dimension_id], conversation_counts[dimension_id]) + end + end + def summary_select_fields [ 'dimension_id', @@ -101,12 +114,13 @@ class Reports::RollupDataSource < Reports::DataSource ReportingEvents::MetricRegistry.rollup_metric_for(metric_name) end - def summary_attributes_for(row) + def summary_attributes_for(row, conversations_count = 0) { - resolved_conversations_count: row.resolved_count.to_i, - avg_resolution_time: average_from(row.resolution_sum_value, row.resolution_count), - avg_first_response_time: average_from(row.first_response_sum_value, row.first_response_count), - avg_reply_time: average_from(row.reply_sum_value, row.reply_count) + conversations_count: conversations_count.to_i, + resolved_conversations_count: row&.resolved_count.to_i, + avg_resolution_time: average_from(row&.resolution_sum_value, row&.resolution_count), + avg_first_response_time: average_from(row&.first_response_sum_value, row&.first_response_count), + avg_reply_time: average_from(row&.reply_sum_value, row&.reply_count) } end @@ -114,6 +128,15 @@ class Reports::RollupDataSource < Reports::DataSource dimension_type == 'account' ? account.id : scope.id end + def summary_conversation_group_by_key + { + 'account' => :account_id, + 'agent' => :assignee_id, + 'inbox' => :inbox_id, + 'team' => :team_id + }[dimension_type] + end + def rollup_value_column use_business_hours? ? :sum_value_business_hours : :sum_value end diff --git a/spec/services/reports/data_source_adapter_contract_spec.rb b/spec/services/reports/data_source_adapter_contract_spec.rb deleted file mode 100644 index 33e1ffcd9..000000000 --- a/spec/services/reports/data_source_adapter_contract_spec.rb +++ /dev/null @@ -1,268 +0,0 @@ -require 'rails_helper' - -RSpec.describe Reports::DataSource do - let(:account) { create(:account, reporting_timezone: 'Etc/UTC') } - let(:user1) { create(:user) } - let(:user2) { create(:user) } - let(:inbox1) { create(:inbox, account: account) } - let(:inbox2) { create(:inbox, account: account) } - let(:timezone) { 'UTC' } - let(:timezone_offset) { '0' } - let(:current_time) { Time.zone.parse('2026-01-15 10:00:00 UTC') } - let(:full_range) { Time.zone.parse('2026-01-05 00:00:00 UTC')...Time.zone.parse('2026-01-16 00:00:00 UTC') } - let(:day_range) { Time.zone.parse('2026-01-14 00:00:00 UTC')...Time.zone.parse('2026-01-16 00:00:00 UTC') } - - before do - travel_to current_time - create(:account_user, account: account, user: user1) - create(:account_user, account: account, user: user2) - - conversation_one = create( - :conversation, - account: account, - inbox: inbox1, - assignee: user1, - created_at: Time.zone.parse('2026-01-06 09:00:00 UTC') - ) - conversation_two = create( - :conversation, - account: account, - inbox: inbox1, - assignee: user1, - created_at: Time.zone.parse('2026-01-14 09:00:00 UTC') - ) - conversation_three = create( - :conversation, - account: account, - inbox: inbox2, - assignee: user2, - created_at: Time.zone.parse('2026-01-15 09:00:00 UTC') - ) - - create(:reporting_event, - name: 'first_response', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_one, - value: 20, - value_in_business_hours: 10, - created_at: Time.zone.parse('2026-01-06 10:00:00 UTC')) - create(:reporting_event, - name: 'reply_time', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_one, - value: 10, - value_in_business_hours: 5, - created_at: Time.zone.parse('2026-01-06 11:00:00 UTC')) - create(:reporting_event, - name: 'conversation_resolved', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_one, - value: 100, - value_in_business_hours: 80, - created_at: Time.zone.parse('2026-01-06 12:00:00 UTC')) - - create(:reporting_event, - name: 'first_response', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_two, - value: 40, - value_in_business_hours: 30, - created_at: Time.zone.parse('2026-01-14 10:00:00 UTC')) - create(:reporting_event, - name: 'reply_time', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_two, - value: 30, - value_in_business_hours: 20, - created_at: Time.zone.parse('2026-01-14 11:00:00 UTC')) - create(:reporting_event, - name: 'conversation_resolved', - account: account, - user: user1, - inbox: inbox1, - conversation: conversation_two, - value: 200, - value_in_business_hours: 150, - created_at: Time.zone.parse('2026-01-14 12:00:00 UTC')) - - create(:reporting_event, - name: 'first_response', - account: account, - user: user2, - inbox: inbox2, - conversation: conversation_three, - value: 60, - value_in_business_hours: 50, - created_at: Time.zone.parse('2026-01-15 10:00:00 UTC')) - create(:reporting_event, - name: 'reply_time', - account: account, - user: user2, - inbox: inbox2, - conversation: conversation_three, - value: 50, - value_in_business_hours: 40, - created_at: Time.zone.parse('2026-01-15 11:00:00 UTC')) - create(:reporting_event, - name: 'conversation_resolved', - account: account, - user: user2, - inbox: inbox2, - conversation: conversation_three, - value: 300, - value_in_business_hours: 250, - created_at: Time.zone.parse('2026-01-15 12:00:00 UTC')) - - [Date.new(2026, 1, 6), Date.new(2026, 1, 14), Date.new(2026, 1, 15)].each do |date| - ReportingEvents::BackfillService.backfill_date(account, date) - end - end - - shared_examples 'report adapter contract' do |adapter_class| - context 'with average metrics' do - subject(:source) do - adapter_class.new( - account: account, - metric: 'avg_first_response_time', - dimension_type: 'account', - dimension_id: nil, - scope: account, - range: day_range, - group_by: 'day', - timezone: timezone, - timezone_offset: timezone_offset, - business_hours: business_hours - ) - end - - let(:business_hours) { false } - - it 'returns the expected aggregate' do - expect(source.aggregate).to eq(50.0) - end - - it 'returns the expected day timeseries' do - expect(source.timeseries).to eq( - [ - { count: 1, timestamp: Date.new(2026, 1, 14).in_time_zone(timezone).to_i, value: 40.0 }, - { count: 1, timestamp: Date.new(2026, 1, 15).in_time_zone(timezone).to_i, value: 60.0 } - ] - ) - end - - context 'when business hours are requested' do - let(:business_hours) { true } - - it 'returns the business-hours aggregate and timeseries' do - expect(source.aggregate).to eq(40.0) - expect(source.timeseries).to eq( - [ - { count: 1, timestamp: Date.new(2026, 1, 14).in_time_zone(timezone).to_i, value: 30.0 }, - { count: 1, timestamp: Date.new(2026, 1, 15).in_time_zone(timezone).to_i, value: 50.0 } - ] - ) - end - end - end - - context 'with count metrics' do - subject(:source) do - adapter_class.new( - account: account, - metric: 'resolutions_count', - dimension_type: 'agent', - dimension_id: user1.id, - scope: user1, - range: full_range, - group_by: 'week', - timezone: timezone, - timezone_offset: timezone_offset, - business_hours: false - ) - end - - it 'returns the expected aggregate' do - expect(source.aggregate).to eq(2) - end - - it 'returns the expected week timeseries' do - expect(source.timeseries).to eq( - [ - { value: 1, timestamp: Date.new(2026, 1, 4).in_time_zone(timezone).to_i }, - { value: 1, timestamp: Date.new(2026, 1, 11).in_time_zone(timezone).to_i } - ] - ) - end - end - - context 'with summary metrics' do - subject(:source) do - adapter_class.new( - account: account, - metric: nil, - dimension_type: 'inbox', - dimension_id: nil, - scope: nil, - range: full_range, - group_by: 'day', - timezone: timezone, - timezone_offset: timezone_offset, - business_hours: business_hours - ) - end - - let(:business_hours) { false } - - it 'returns the expected summary shape and values' do - expect(source.summary).to eq( - inbox1.id => { - resolved_conversations_count: 2, - avg_resolution_time: 150.0, - avg_first_response_time: 30.0, - avg_reply_time: 20.0 - }, - inbox2.id => { - resolved_conversations_count: 1, - avg_resolution_time: 300.0, - avg_first_response_time: 60.0, - avg_reply_time: 50.0 - } - ) - end - - context 'when business hours are requested' do - let(:business_hours) { true } - - it 'returns the business-hours summary values' do - expect(source.summary).to eq( - inbox1.id => { - resolved_conversations_count: 2, - avg_resolution_time: 115.0, - avg_first_response_time: 20.0, - avg_reply_time: 12.5 - }, - inbox2.id => { - resolved_conversations_count: 1, - avg_resolution_time: 250.0, - avg_first_response_time: 50.0, - avg_reply_time: 40.0 - } - ) - end - end - end - end - - it_behaves_like 'report adapter contract', Reports::RawDataSource - it_behaves_like 'report adapter contract', Reports::RollupDataSource -end diff --git a/spec/services/reports/data_source_spec.rb b/spec/services/reports/data_source_spec.rb index fb7bd2e3b..3be3c0629 100644 --- a/spec/services/reports/data_source_spec.rb +++ b/spec/services/reports/data_source_spec.rb @@ -59,4 +59,275 @@ RSpec.describe Reports::DataSource do expect(described_class.for(**params, metric: 'conversations_count')).to be_a(Reports::RawDataSource) end end + + describe 'adapter contract' do + let(:account) { create(:account, reporting_timezone: 'Etc/UTC') } + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:inbox1) { create(:inbox, account: account) } + let(:inbox2) { create(:inbox, account: account) } + let(:timezone) { 'UTC' } + let(:timezone_offset) { '0' } + let(:current_time) { Time.zone.parse('2026-01-15 10:00:00 UTC') } + let(:full_range) { Time.zone.parse('2026-01-05 00:00:00 UTC')...Time.zone.parse('2026-01-16 00:00:00 UTC') } + let(:day_range) { Time.zone.parse('2026-01-14 00:00:00 UTC')...Time.zone.parse('2026-01-16 00:00:00 UTC') } + + before do + travel_to current_time + create(:account_user, account: account, user: user1) + create(:account_user, account: account, user: user2) + + conversation_one = create( + :conversation, + account: account, + inbox: inbox1, + assignee: user1, + created_at: Time.zone.parse('2026-01-06 09:00:00 UTC') + ) + conversation_two = create( + :conversation, + account: account, + inbox: inbox1, + assignee: user1, + created_at: Time.zone.parse('2026-01-14 09:00:00 UTC') + ) + conversation_three = create( + :conversation, + account: account, + inbox: inbox2, + assignee: user2, + created_at: Time.zone.parse('2026-01-15 09:00:00 UTC') + ) + + create(:reporting_event, + name: 'first_response', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_one, + value: 20, + value_in_business_hours: 10, + created_at: Time.zone.parse('2026-01-06 10:00:00 UTC')) + create(:reporting_event, + name: 'reply_time', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_one, + value: 10, + value_in_business_hours: 5, + created_at: Time.zone.parse('2026-01-06 11:00:00 UTC')) + create(:reporting_event, + name: 'conversation_resolved', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_one, + value: 100, + value_in_business_hours: 80, + created_at: Time.zone.parse('2026-01-06 12:00:00 UTC')) + + create(:reporting_event, + name: 'first_response', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_two, + value: 40, + value_in_business_hours: 30, + created_at: Time.zone.parse('2026-01-14 10:00:00 UTC')) + create(:reporting_event, + name: 'reply_time', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_two, + value: 30, + value_in_business_hours: 20, + created_at: Time.zone.parse('2026-01-14 11:00:00 UTC')) + create(:reporting_event, + name: 'conversation_resolved', + account: account, + user: user1, + inbox: inbox1, + conversation: conversation_two, + value: 200, + value_in_business_hours: 150, + created_at: Time.zone.parse('2026-01-14 12:00:00 UTC')) + + create(:reporting_event, + name: 'first_response', + account: account, + user: user2, + inbox: inbox2, + conversation: conversation_three, + value: 60, + value_in_business_hours: 50, + created_at: Time.zone.parse('2026-01-15 10:00:00 UTC')) + create(:reporting_event, + name: 'reply_time', + account: account, + user: user2, + inbox: inbox2, + conversation: conversation_three, + value: 50, + value_in_business_hours: 40, + created_at: Time.zone.parse('2026-01-15 11:00:00 UTC')) + create(:reporting_event, + name: 'conversation_resolved', + account: account, + user: user2, + inbox: inbox2, + conversation: conversation_three, + value: 300, + value_in_business_hours: 250, + created_at: Time.zone.parse('2026-01-15 12:00:00 UTC')) + + [Date.new(2026, 1, 6), Date.new(2026, 1, 14), Date.new(2026, 1, 15)].each do |date| + ReportingEvents::BackfillService.backfill_date(account, date) + end + end + + shared_examples 'report adapter contract' do |adapter_class| + context 'with average metrics' do + subject(:source) do + adapter_class.new( + account: account, + metric: 'avg_first_response_time', + dimension_type: 'account', + dimension_id: nil, + scope: account, + range: day_range, + group_by: 'day', + timezone: timezone, + timezone_offset: timezone_offset, + business_hours: business_hours + ) + end + + let(:business_hours) { false } + + it 'returns the expected aggregate' do + expect(source.aggregate).to eq(50.0) + end + + it 'returns the expected day timeseries' do + expect(source.timeseries).to eq( + [ + { count: 1, timestamp: Date.new(2026, 1, 14).in_time_zone(timezone).to_i, value: 40.0 }, + { count: 1, timestamp: Date.new(2026, 1, 15).in_time_zone(timezone).to_i, value: 60.0 } + ] + ) + end + + context 'when business hours are requested' do + let(:business_hours) { true } + + it 'returns the business-hours aggregate and timeseries' do + expect(source.aggregate).to eq(40.0) + expect(source.timeseries).to eq( + [ + { count: 1, timestamp: Date.new(2026, 1, 14).in_time_zone(timezone).to_i, value: 30.0 }, + { count: 1, timestamp: Date.new(2026, 1, 15).in_time_zone(timezone).to_i, value: 50.0 } + ] + ) + end + end + end + + context 'with count metrics' do + subject(:source) do + adapter_class.new( + account: account, + metric: 'resolutions_count', + dimension_type: 'agent', + dimension_id: user1.id, + scope: user1, + range: full_range, + group_by: 'week', + timezone: timezone, + timezone_offset: timezone_offset, + business_hours: false + ) + end + + it 'returns the expected aggregate' do + expect(source.aggregate).to eq(2) + end + + it 'returns the expected week timeseries' do + expect(source.timeseries).to eq( + [ + { value: 1, timestamp: Date.new(2026, 1, 4).in_time_zone(timezone).to_i }, + { value: 1, timestamp: Date.new(2026, 1, 11).in_time_zone(timezone).to_i } + ] + ) + end + end + + context 'with summary metrics' do + subject(:source) do + adapter_class.new( + account: account, + metric: nil, + dimension_type: 'inbox', + dimension_id: nil, + scope: nil, + range: full_range, + group_by: 'day', + timezone: timezone, + timezone_offset: timezone_offset, + business_hours: business_hours + ) + end + + let(:business_hours) { false } + + it 'returns the expected summary shape and values' do + expect(source.summary).to eq( + inbox1.id => { + conversations_count: 2, + resolved_conversations_count: 2, + avg_resolution_time: 150.0, + avg_first_response_time: 30.0, + avg_reply_time: 20.0 + }, + inbox2.id => { + conversations_count: 1, + resolved_conversations_count: 1, + avg_resolution_time: 300.0, + avg_first_response_time: 60.0, + avg_reply_time: 50.0 + } + ) + end + + context 'when business hours are requested' do + let(:business_hours) { true } + + it 'returns the business-hours summary values' do + expect(source.summary).to eq( + inbox1.id => { + conversations_count: 2, + resolved_conversations_count: 2, + avg_resolution_time: 115.0, + avg_first_response_time: 20.0, + avg_reply_time: 12.5 + }, + inbox2.id => { + conversations_count: 1, + resolved_conversations_count: 1, + avg_resolution_time: 250.0, + avg_first_response_time: 50.0, + avg_reply_time: 40.0 + } + ) + end + end + end + end + + it_behaves_like 'report adapter contract', Reports::RawDataSource + it_behaves_like 'report adapter contract', Reports::RollupDataSource + end end