diff --git a/app/builders/v2/reports/conversations/base_report_builder.rb b/app/builders/v2/reports/conversations/base_report_builder.rb index 57b115945..ee0155150 100644 --- a/app/builders/v2/reports/conversations/base_report_builder.rb +++ b/app/builders/v2/reports/conversations/base_report_builder.rb @@ -4,7 +4,7 @@ class V2::Reports::Conversations::BaseReportBuilder private def builder_class(metric) - return unless ReportingEvents::MetricRegistry.supported_metric?(metric) + return unless Reports::ReportMetricRegistry.supported?(metric) V2::Reports::Timeseries::ReportBuilder end diff --git a/app/services/reports/data_source.rb b/app/services/reports/data_source.rb index 7c013295d..0206551d8 100644 --- a/app/services/reports/data_source.rb +++ b/app/services/reports/data_source.rb @@ -49,7 +49,7 @@ class Reports::DataSource end def supported_metric?(metric) - metric.blank? || ReportingEvents::MetricRegistry.rollup_supported_metric?(metric) + metric.blank? || Reports::ReportMetricRegistry.rollup_supported?(metric) end def normalized_timezone_identifier(timezone) @@ -73,11 +73,11 @@ class Reports::DataSource private def report_metric - @report_metric ||= ReportingEvents::MetricRegistry.report_metric(metric) + @report_metric ||= Reports::ReportMetricRegistry.fetch(metric) end def average_metric? - report_metric&.dig(:aggregate) == :average + report_metric&.average? end def count_metric? @@ -85,19 +85,19 @@ class Reports::DataSource end def rollup_metric - report_metric&.dig(:rollup_metric) + report_metric&.rollup_metric end def raw_event_name - report_metric&.dig(:raw_event_name) + report_metric&.raw_event_name end def raw_count_strategy - report_metric&.dig(:raw_count_strategy) + report_metric&.raw_count_strategy end def summary_metrics - @summary_metrics ||= ReportingEvents::MetricRegistry.summary_metrics + @summary_metrics ||= Reports::ReportMetricRegistry.summary_metrics end def use_business_hours? diff --git a/app/services/reports/raw_data_source.rb b/app/services/reports/raw_data_source.rb index 5abcd7176..f37b3c454 100644 --- a/app/services/reports/raw_data_source.rb +++ b/app/services/reports/raw_data_source.rb @@ -114,17 +114,17 @@ class Reports::RawDataSource < Reports::DataSource end def summary_select_field(definition) - if definition[:aggregate] == :count - "COUNT(CASE WHEN name = '#{definition[:raw_event_name]}' THEN 1 END) as #{definition[:summary_key]}" + if definition.count? + "COUNT(CASE WHEN name = '#{definition.raw_event_name}' THEN 1 END) as #{definition.summary_key}" else - "AVG(CASE WHEN name = '#{definition[:raw_event_name]}' THEN #{average_value_key} END) as #{definition[:summary_key]}" + "AVG(CASE WHEN name = '#{definition.raw_event_name}' THEN #{average_value_key} END) as #{definition.summary_key}" end end def summary_attributes_for(record, conversations_count = 0) summary_metrics.each_with_object({ conversations_count: conversations_count.to_i }) do |definition, attributes| - value = record&.public_send(definition[:summary_key]) - attributes[definition[:summary_key]] = definition[:aggregate] == :count ? value.to_i : value + value = record&.public_send(definition.summary_key) + attributes[definition.summary_key] = definition.count? ? value.to_i : value end end diff --git a/app/services/reports/report_metric_registry.rb b/app/services/reports/report_metric_registry.rb new file mode 100644 index 000000000..51af20076 --- /dev/null +++ b/app/services/reports/report_metric_registry.rb @@ -0,0 +1,135 @@ +module Reports::ReportMetricRegistry + # Describes one public report metric. + # name: API-facing metric name requested by reports. + # aggregate: whether the metric is a count or average. + # raw_event_name: source reporting_events name for raw queries. + # rollup_metric: source reporting_events_rollups metric for rollup queries. + # summary_key: key used when this metric appears in grouped summary responses. + # raw_count_strategy: optional raw-query counting rule, such as distinct conversations. + Metric = Data.define( + :name, + :aggregate, + :raw_event_name, + :rollup_metric, + :summary_key, + :raw_count_strategy + ) do + def average? + aggregate == :average + end + + def count? + aggregate == :count + end + + def rollup_supported? + rollup_metric.present? + end + + def summary? + summary_key.present? + end + end + + METRICS = { + conversations_count: Metric.new( + name: :conversations_count, + aggregate: :count, + raw_event_name: nil, + rollup_metric: nil, + summary_key: nil, + raw_count_strategy: nil + ), + incoming_messages_count: Metric.new( + name: :incoming_messages_count, + aggregate: :count, + raw_event_name: nil, + rollup_metric: nil, + summary_key: nil, + raw_count_strategy: nil + ), + outgoing_messages_count: Metric.new( + name: :outgoing_messages_count, + aggregate: :count, + raw_event_name: nil, + rollup_metric: nil, + summary_key: nil, + raw_count_strategy: nil + ), + avg_first_response_time: Metric.new( + name: :avg_first_response_time, + aggregate: :average, + raw_event_name: :first_response, + rollup_metric: :first_response, + summary_key: :avg_first_response_time, + raw_count_strategy: nil + ), + avg_resolution_time: Metric.new( + name: :avg_resolution_time, + aggregate: :average, + raw_event_name: :conversation_resolved, + rollup_metric: :resolution_time, + summary_key: :avg_resolution_time, + raw_count_strategy: nil + ), + reply_time: Metric.new( + name: :reply_time, + aggregate: :average, + raw_event_name: :reply_time, + rollup_metric: :reply_time, + summary_key: :avg_reply_time, + raw_count_strategy: nil + ), + resolutions_count: Metric.new( + name: :resolutions_count, + aggregate: :count, + raw_event_name: :conversation_resolved, + rollup_metric: :resolutions_count, + summary_key: :resolved_conversations_count, + raw_count_strategy: nil + ), + bot_resolutions_count: Metric.new( + name: :bot_resolutions_count, + aggregate: :count, + raw_event_name: :conversation_bot_resolved, + rollup_metric: :bot_resolutions_count, + summary_key: nil, + raw_count_strategy: nil + ), + bot_handoffs_count: Metric.new( + name: :bot_handoffs_count, + aggregate: :count, + raw_event_name: :conversation_bot_handoff, + rollup_metric: :bot_handoffs_count, + summary_key: nil, + raw_count_strategy: :distinct_conversation + ) + }.freeze + + SUMMARY_METRIC_NAMES = %i[ + resolutions_count + avg_resolution_time + avg_first_response_time + reply_time + ].freeze + + module_function + + def fetch(name) + return if name.blank? + + METRICS[name.to_sym] + end + + def supported?(name) + fetch(name).present? + end + + def rollup_supported?(name) + fetch(name)&.rollup_supported? || false + end + + def summary_metrics + SUMMARY_METRIC_NAMES.map { |metric_name| METRICS.fetch(metric_name) } + end +end diff --git a/spec/services/reports/report_metric_registry_spec.rb b/spec/services/reports/report_metric_registry_spec.rb new file mode 100644 index 000000000..33fcb41e5 --- /dev/null +++ b/spec/services/reports/report_metric_registry_spec.rb @@ -0,0 +1,74 @@ +require 'rails_helper' + +RSpec.describe Reports::ReportMetricRegistry do + describe '.fetch' do + it 'returns the definition for raw-only count metrics' do + metric = described_class.fetch(:conversations_count) + + expect(metric.name).to eq(:conversations_count) + expect(metric.count?).to be(true) + expect(metric.rollup_supported?).to be(false) + expect(metric.raw_event_name).to be_nil + end + + it 'returns the definition for avg_resolution_time' do + metric = described_class.fetch(:avg_resolution_time) + + expect(metric.name).to eq(:avg_resolution_time) + expect(metric.average?).to be(true) + expect(metric.raw_event_name).to eq(:conversation_resolved) + expect(metric.rollup_metric).to eq(:resolution_time) + expect(metric.summary_key).to eq(:avg_resolution_time) + end + + it 'locks the distinct conversation strategy for bot_handoffs_count' do + metric = described_class.fetch(:bot_handoffs_count) + + expect(metric.count?).to be(true) + expect(metric.raw_event_name).to eq(:conversation_bot_handoff) + expect(metric.rollup_metric).to eq(:bot_handoffs_count) + expect(metric.raw_count_strategy).to eq(:distinct_conversation) + end + + it 'returns nil for unsupported metrics' do + expect(described_class.fetch(:unknown_metric)).to be_nil + end + end + + describe '.supported?' do + it 'returns true for supported raw-only metrics' do + expect(described_class.supported?(:conversations_count)).to be(true) + end + + it 'returns false for unsupported metrics' do + expect(described_class.supported?(:unknown_metric)).to be(false) + end + end + + describe '.rollup_supported?' do + it 'returns true for rollup-backed metrics' do + expect(described_class.rollup_supported?(:reply_time)).to be(true) + end + + it 'returns false for raw-only metrics' do + expect(described_class.rollup_supported?(:conversations_count)).to be(false) + end + end + + describe '.summary_metrics' do + it 'returns the summary metric definitions in registry order' do + expect( + described_class.summary_metrics.map do |metric| + [metric.name, metric.summary_key, metric.aggregate, metric.raw_event_name, metric.rollup_metric] + end + ).to eq( + [ + [:resolutions_count, :resolved_conversations_count, :count, :conversation_resolved, :resolutions_count], + [:avg_resolution_time, :avg_resolution_time, :average, :conversation_resolved, :resolution_time], + [:avg_first_response_time, :avg_first_response_time, :average, :first_response, :first_response], + [:reply_time, :avg_reply_time, :average, :reply_time, :reply_time] + ] + ) + end + end +end