refactor: better data access

app/builders/v2/reports/base_summary_builder.rb

@@ -10,10 +10,28 @@ class V2::Reports::BaseSummaryBuilder
 
   def load_data
     @conversations_count = fetch_conversations_count
-    @resolved_count = fetch_resolved_count
+    load_reporting_events_data
-    @avg_resolution_time = fetch_average_time('conversation_resolved')
+  end
-    @avg_first_response_time = fetch_average_time('first_response')
+
-    @avg_reply_time = fetch_average_time('reply_time')
+  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
@@ -24,14 +42,6 @@ class V2::Reports::BaseSummaryBuilder
     # Override this method
   end
 
-  def fetch_average_time(event_name)
-    get_grouped_average(reporting_events.where(name: event_name))
-  end
-
-  def fetch_resolved_count
-    reporting_events.where(name: 'conversation_resolved').group(group_by_key).count
-  end
-
   def group_by_key
     # Override this method
   end
@@ -40,10 +50,6 @@ class V2::Reports::BaseSummaryBuilder
     # Override this method
   end
 
-  def get_grouped_average(events)
-    events.group(group_by_key).average(average_value_key)
-  end
-
   def average_value_key
     ActiveModel::Type::Boolean.new.cast(params[:business_hours]).present? ? :value_in_business_hours : :value
   end

app/builders/v2/reports/conversations/base_aggregator.rb

@@ -0,0 +1,13 @@
+class V2::Reports::Conversations::BaseAggregator < V2::Reports::Timeseries::BaseTimeseriesBuilder
+  def metrics
+    return {} if range.blank?
+
+    compute_metrics
+  end
+
+  private
+
+  def compute_metrics
+    raise NotImplementedError, 'Subclasses must implement #compute_metrics'
+  end
+end

app/builders/v2/reports/conversations/better_metric_builder.rb

@@ -0,0 +1,67 @@
+class V2::Reports::Conversations::BetterMetricBuilder < V2::Reports::Conversations::BaseReportBuilder
+  COUNT_METRICS = %i[
+    conversations_count
+    incoming_messages_count
+    outgoing_messages_count
+    resolutions_count
+    bot_resolutions_count
+    bot_handoffs_count
+  ].freeze
+
+  AVERAGE_METRICS = %i[
+    avg_first_response_time
+    avg_resolution_time
+    reply_time
+  ].freeze
+
+  SUMMARY_METRICS = %i[
+    conversations_count
+    outgoing_messages_count
+    avg_first_response_time
+    avg_resolution_time
+    resolutions_count
+    reply_time
+  ].freeze
+
+  BOT_SUMMARY_METRICS = %i[
+    bot_resolutions_count
+    bot_handoffs_count
+  ].freeze
+
+  AGGREGATOR_CLASSES = [
+    V2::Reports::Conversations::ConversationsAggregator,
+    V2::Reports::Conversations::MessagesAggregator,
+    V2::Reports::Conversations::ReportingEventsAggregator
+  ].freeze
+
+  def summary
+    result = SUMMARY_METRICS.index_with { |metric| fetch_metric(metric) }
+    result[:incoming_messages_count] = fetch_metric(:incoming_messages_count) unless params[:type] == :agent
+    result
+  end
+
+  def bot_summary
+    BOT_SUMMARY_METRICS.index_with { |metric| fetch_metric(metric) }
+  end
+
+  private
+
+  def fetch_metric(key)
+    aggregated_metrics.fetch(key, default_for(key))
+  end
+
+  def aggregated_metrics
+    @aggregated_metrics ||= AGGREGATOR_CLASSES.each_with_object({}) do |aggregator_class, memo|
+      memo.merge!(aggregator_class.new(account, params).metrics) do |_metric, existing_value, new_value|
+        new_value.nil? ? existing_value : new_value
+      end
+    end
+  end
+
+  def default_for(metric)
+    return 0 if COUNT_METRICS.include?(metric)
+    return nil if AVERAGE_METRICS.include?(metric)
+
+    nil
+  end
+end

app/builders/v2/reports/conversations/conversations_aggregator.rb

@@ -0,0 +1,16 @@
+class V2::Reports::Conversations::ConversationsAggregator < V2::Reports::Conversations::BaseAggregator
+  private
+
+  def compute_metrics
+    {
+      conversations_count: base_relation.count
+    }
+  end
+
+  def base_relation
+    scope.conversations
+         .where(account_id: account.id, created_at: range)
+         .unscope(:order)
+         .reorder(nil)
+  end
+end

app/builders/v2/reports/conversations/messages_aggregator.rb

@@ -0,0 +1,26 @@
+class V2::Reports::Conversations::MessagesAggregator < V2::Reports::Conversations::BaseAggregator
+  private
+
+  def compute_metrics
+    incoming_count, outgoing_count = base_relation.pluck(
+      Arel.sql(count_sql(Message.message_types[:incoming])),
+      Arel.sql(count_sql(Message.message_types[:outgoing]))
+    ).first || [0, 0]
+
+    {
+      incoming_messages_count: incoming_count || 0,
+      outgoing_messages_count: outgoing_count || 0
+    }
+  end
+
+  def base_relation
+    scope.messages
+         .where(account_id: account.id, created_at: range)
+         .unscope(:order)
+         .reorder(nil)
+  end
+
+  def count_sql(message_type)
+    "COUNT(*) FILTER (WHERE message_type = #{message_type})"
+  end
+end

app/builders/v2/reports/conversations/metric_builder.rb

@@ -1,14 +1,19 @@
 class V2::Reports::Conversations::MetricBuilder < V2::Reports::Conversations::BaseReportBuilder
   def summary
-    {
+    result = {
       conversations_count: count('conversations_count'),
-      incoming_messages_count: count('incoming_messages_count'),
       outgoing_messages_count: count('outgoing_messages_count'),
       avg_first_response_time: count('avg_first_response_time'),
       avg_resolution_time: count('avg_resolution_time'),
+      incoming_messages_count: 0,
       resolutions_count: count('resolutions_count'),
       reply_time: count('reply_time')
     }
+
+    # Only include incoming_messages_count for non-agent summaries
+    result[:incoming_messages_count] = count('incoming_messages_count') unless params[:type] == :agent
+
+    result
   end
 
   def bot_summary

app/builders/v2/reports/conversations/reporting_events_aggregator.rb

@@ -0,0 +1,56 @@
+class V2::Reports::Conversations::ReportingEventsAggregator < V2::Reports::Conversations::BaseAggregator
+  COUNT_MAPPINGS = {
+    resolutions_count: :conversation_resolved,
+    bot_resolutions_count: :conversation_bot_resolved,
+    bot_handoffs_count: :conversation_bot_handoff
+  }.freeze
+
+  AVERAGE_MAPPINGS = {
+    avg_first_response_time: :first_response,
+    avg_resolution_time: :conversation_resolved,
+    reply_time: :reply_time
+  }.freeze
+
+  private
+
+  def compute_metrics
+    result = base_relation.select(*select_fragments).first
+    return default_metrics unless result
+
+    COUNT_MAPPINGS.keys.index_with { |key| result.public_send(key) || 0 }
+                  .merge(AVERAGE_MAPPINGS.keys.index_with { |key| result.public_send(key) })
+  end
+
+  def select_fragments
+    COUNT_MAPPINGS.map { |metric, event_name| "#{count_fragment(event_name)} AS #{metric}" } +
+      AVERAGE_MAPPINGS.map { |metric, event_name| "#{average_fragment(event_name)} AS #{metric}" }
+  end
+
+  def count_fragment(event_name)
+    if event_name == :conversation_bot_handoff
+      Arel.sql("COUNT(DISTINCT CASE WHEN name = '#{event_name}' THEN conversation_id END)")
+    else
+      Arel.sql("COUNT(*) FILTER (WHERE name = '#{event_name}')")
+    end
+  end
+
+  def average_fragment(event_name)
+    Arel.sql("AVG(CASE WHEN name = '#{event_name}' THEN #{value_column} END)")
+  end
+
+  def base_relation
+    scope.reporting_events
+         .where(account_id: account.id, created_at: range)
+         .unscope(:order)
+         .reorder(nil)
+  end
+
+  def value_column
+    params[:business_hours].present? ? 'value_in_business_hours' : 'value'
+  end
+
+  def default_metrics
+    COUNT_MAPPINGS.keys.index_with { 0 }
+                  .merge(AVERAGE_MAPPINGS.keys.index_with { nil })
+  end
+end

app/builders/v2/reports/inbox_summary_builder.rb

@@ -13,10 +13,7 @@ class V2::Reports::InboxSummaryBuilder < V2::Reports::BaseSummaryBuilder
 
   def load_data
     @conversations_count = fetch_conversations_count
-    @resolved_count = fetch_resolved_count
+    load_reporting_events_data
-    @avg_resolution_time = fetch_average_time('conversation_resolved')
-    @avg_first_response_time = fetch_average_time('first_response')
-    @avg_reply_time = fetch_average_time('reply_time')
   end
 
   def fetch_conversations_count

app/controllers/api/v2/accounts/reports_controller.rb

@@ -125,7 +125,7 @@ class Api::V2::Accounts::ReportsController < Api::V1::Accounts::BaseController
   end
 
   def build_summary(method)
-    builder = V2::Reports::Conversations::MetricBuilder
+    builder = V2::Reports::Conversations::BetterMetricBuilder
     current_summary = builder.new(Current.account, current_summary_params).send(method)
     previous_summary = builder.new(Current.account, previous_summary_params).send(method)
     current_summary.merge(previous: previous_summary)

spec/builders/v2/reports/conversations/metric_builder_spec.rb

@@ -14,25 +14,55 @@ RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do
   end
 
   describe '#summary' do
-    it 'returns the correct summary values' do
+    context 'when type is not agent' do
-      summary = subject.summary
+      let(:params) { { since: '2023-01-01', until: '2024-01-01', type: :inbox } }
-      expect(summary).to eq(
+
-        {
+      it 'returns the correct summary values including incoming_messages_count' do
-          conversations_count: 42,
+        summary = subject.summary
-          incoming_messages_count: 42,
+        expect(summary).to eq(
-          outgoing_messages_count: 42,
+          {
-          avg_first_response_time: 42,
+            conversations_count: 42,
-          avg_resolution_time: 42,
+            outgoing_messages_count: 42,
-          resolutions_count: 42,
+            avg_first_response_time: 42,
-          reply_time: 42
+            avg_resolution_time: 42,
-        }
+            resolutions_count: 42,
-      )
+            reply_time: 42,
+            incoming_messages_count: 42
+          }
+        )
+      end
+
+      it 'creates builders with proper params' do
+        subject.summary
+        expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count'))
+        expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'incoming_messages_count'))
+        expect(V2::Reports::Timeseries::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
+      end
     end
 
-    it 'creates builders with proper params' do
+    context 'when type is agent' do
-      subject.summary
+      let(:params) { { since: '2023-01-01', until: '2024-01-01', type: :agent } }
-      expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count'))
+
-      expect(V2::Reports::Timeseries::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
+      it 'returns the correct summary values without incoming_messages_count' do
+        summary = subject.summary
+        expect(summary).to eq(
+          {
+            conversations_count: 42,
+            outgoing_messages_count: 42,
+            avg_first_response_time: 42,
+            avg_resolution_time: 42,
+            resolutions_count: 42,
+            reply_time: 42
+          }
+        )
+      end
+
+      it 'does not create builder for incoming_messages_count' do
+        subject.summary
+        expect(V2::Reports::Timeseries::CountReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count'))
+        expect(V2::Reports::Timeseries::CountReportBuilder).not_to have_received(:new).with(account, params.merge(metric: 'incoming_messages_count'))
+        expect(V2::Reports::Timeseries::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
+      end
     end
   end