From 57d40c373d5050fa14a84318a2f74a7f62fd3fda Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Fri, 13 Mar 2026 10:59:25 +0530 Subject: [PATCH] refactor: route summary builders through DataSource --- .../v2/reports/agent_summary_builder.rb | 4 -- .../v2/reports/base_summary_builder.rb | 61 +++++++++---------- .../v2/reports/inbox_summary_builder.rb | 13 ---- .../v2/reports/label_summary_builder.rb | 3 +- .../v2/reports/team_summary_builder.rb | 8 --- .../v2/reports/label_summary_builder_spec.rb | 8 ++- 6 files changed, 37 insertions(+), 60 deletions(-) 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 d4a9e7c0b..603263ee9 100644 --- a/app/builders/v2/reports/base_summary_builder.rb +++ b/app/builders/v2/reports/base_summary_builder.rb @@ -1,5 +1,6 @@ class V2::Reports::BaseSummaryBuilder include DateRangeHelper + include TimezoneHelper def build load_data @@ -9,37 +10,13 @@ class V2::Reports::BaseSummaryBuilder private def load_data - @conversations_count = fetch_conversations_count - load_reporting_events_data - end + results = data_source.summary - def load_reporting_events_data - # Extract the column name for indexing (e.g., 'conversations.team_id' -> 'team_id') - index_key = group_by_key.to_s.split('.').last - - results = reporting_events - .select( - "#{group_by_key} as #{index_key}", - "COUNT(CASE WHEN name = 'conversation_resolved' THEN 1 END) as resolved_count", - "AVG(CASE WHEN name = 'conversation_resolved' THEN #{average_value_key} END) as avg_resolution_time", - "AVG(CASE WHEN name = 'first_response' THEN #{average_value_key} END) as avg_first_response_time", - "AVG(CASE WHEN name = 'reply_time' THEN #{average_value_key} END) as avg_reply_time" - ) - .group(group_by_key) - .index_by { |record| record.public_send(index_key) } - - @resolved_count = results.transform_values(&:resolved_count) - @avg_resolution_time = results.transform_values(&:avg_resolution_time) - @avg_first_response_time = results.transform_values(&:avg_first_response_time) - @avg_reply_time = results.transform_values(&:avg_reply_time) - end - - def reporting_events - @reporting_events ||= account.reporting_events.where(created_at: range) - end - - def fetch_conversations_count - # Override this method + @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 group_by_key @@ -50,7 +27,27 @@ class V2::Reports::BaseSummaryBuilder # Override this method end - def average_value_key - ActiveModel::Type::Boolean.new.cast(params[:business_hours]).present? ? :value_in_business_hours : :value + def data_source + @data_source ||= Reports::DataSource.for( + account: account, + metric: nil, + dimension_type: summary_dimension_type, + dimension_id: nil, + scope: nil, + range: range, + group_by: 'day', + timezone: timezone_name_from_params(params[:timezone], params[:timezone_offset]), + timezone_offset: params[:timezone_offset], + business_hours: params[:business_hours] + ) + end + + def summary_dimension_type + { + 'account_id' => 'account', + 'user_id' => 'agent', + 'inbox_id' => 'inbox', + 'conversations.team_id' => 'team' + }.fetch(group_by_key.to_s) end end diff --git a/app/builders/v2/reports/inbox_summary_builder.rb b/app/builders/v2/reports/inbox_summary_builder.rb index 935afeb82..fcfabc599 100644 --- a/app/builders/v2/reports/inbox_summary_builder.rb +++ b/app/builders/v2/reports/inbox_summary_builder.rb @@ -11,15 +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 load_data - @conversations_count = fetch_conversations_count - load_reporting_events_data - end - - 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) @@ -40,8 +31,4 @@ class V2::Reports::InboxSummaryBuilder < V2::Reports::BaseSummaryBuilder def group_by_key :inbox_id end - - def average_value_key - ActiveModel::Type::Boolean.new.cast(params[:business_hours]) ? :value_in_business_hours : :value - end end diff --git a/app/builders/v2/reports/label_summary_builder.rb b/app/builders/v2/reports/label_summary_builder.rb index 8b7c21e8e..013cc3a0d 100644 --- a/app/builders/v2/reports/label_summary_builder.rb +++ b/app/builders/v2/reports/label_summary_builder.rb @@ -7,8 +7,7 @@ class V2::Reports::LabelSummaryBuilder < V2::Reports::BaseSummaryBuilder @account = account @params = params - timezone_offset = (params[:timezone_offset] || 0).to_f - @timezone = ActiveSupport::TimeZone[timezone_offset]&.name + @timezone = timezone_name_from_params(params[:timezone], params[:timezone_offset]) end # rubocop:enable Lint/MissingSuper diff --git a/app/builders/v2/reports/team_summary_builder.rb b/app/builders/v2/reports/team_summary_builder.rb index b98151bc6..3bcf51816 100644 --- a/app/builders/v2/reports/team_summary_builder.rb +++ b/app/builders/v2/reports/team_summary_builder.rb @@ -6,14 +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 reporting_events - @reporting_events ||= account.reporting_events.where(created_at: range).joins(:conversation) - end - def prepare_report account.teams.map do |team| build_team_stats(team) diff --git a/spec/builders/v2/reports/label_summary_builder_spec.rb b/spec/builders/v2/reports/label_summary_builder_spec.rb index 750f82241..645cc9761 100644 --- a/spec/builders/v2/reports/label_summary_builder_spec.rb +++ b/spec/builders/v2/reports/label_summary_builder_spec.rb @@ -33,7 +33,13 @@ RSpec.describe V2::Reports::LabelSummaryBuilder do it 'sets timezone from timezone_offset' do builder_with_offset = described_class.new(account: account, params: { timezone_offset: -8 }) - expect(builder_with_offset.instance_variable_get(:@timezone)).to eq('Pacific Time (US & Canada)') + expected_timezone = ActiveSupport::TimeZone.all.find { |zone| zone.now.utc_offset == -8.hours }.name + expect(builder_with_offset.instance_variable_get(:@timezone)).to eq(expected_timezone) + end + + it 'prefers timezone when it is provided' do + builder_with_timezone = described_class.new(account: account, params: { timezone: 'Asia/Kolkata', timezone_offset: -8 }) + expect(builder_with_timezone.instance_variable_get(:@timezone)).to eq('Asia/Kolkata') end it 'defaults timezone when timezone_offset is not provided' do