-
+
+
+
+
+ {{
+ `${$t('EDIT_CONTACT.TITLE')} - ${contact.name || contact.email}`
+ }}
+
+
+ {{ $t('EDIT_CONTACT.DESC') }}
+
+
+
+
-
+
diff --git a/app/services/reports/data_source.rb b/app/services/reports/data_source.rb
new file mode 100644
index 000000000..dff392c12
--- /dev/null
+++ b/app/services/reports/data_source.rb
@@ -0,0 +1,64 @@
+class Reports::DataSource
+ include TimezoneHelper
+
+ attr_reader :account, :metric, :dimension_type, :dimension_id,
+ :scope, :range, :group_by, :timezone_offset,
+ :business_hours
+
+ class << self
+ def for(**context)
+ # TODO: Route to Reports::RollupDataSource when rollup reads are implemented
+ Reports::RawDataSource.new(**context)
+ end
+ end
+
+ def initialize(**context)
+ @account = context[:account]
+ @metric = context[:metric]
+ @dimension_type = (context[:dimension_type].presence || 'account').to_s
+ @dimension_id = context[:dimension_id]
+ @scope = context[:scope]
+ @range = context[:range]
+ @group_by = context[:group_by].to_s.presence || 'day'
+ @timezone_offset = context[:timezone_offset]
+ @business_hours = context[:business_hours]
+ end
+
+ private
+
+ def report_metric
+ @report_metric ||= Reports::ReportMetricRegistry.fetch(metric)
+ end
+
+ def average_metric?
+ report_metric&.average?
+ end
+
+ def count_metric?
+ !average_metric?
+ end
+
+ def rollup_metric
+ report_metric&.rollup_metric
+ end
+
+ def raw_event_name
+ report_metric&.raw_event_name
+ end
+
+ def raw_count_strategy
+ report_metric&.raw_count_strategy
+ end
+
+ def summary_metrics
+ @summary_metrics ||= Reports::ReportMetricRegistry.summary_metrics
+ end
+
+ def timezone
+ @timezone ||= timezone_name_from_offset(timezone_offset)
+ end
+
+ def use_business_hours?
+ ActiveModel::Type::Boolean.new.cast(business_hours)
+ end
+end
diff --git a/app/services/reports/raw_data_source.rb b/app/services/reports/raw_data_source.rb
new file mode 100644
index 000000000..f37b3c454
--- /dev/null
+++ b/app/services/reports/raw_data_source.rb
@@ -0,0 +1,156 @@
+class Reports::RawDataSource < Reports::DataSource
+ def timeseries
+ average_metric? ? average_timeseries : count_timeseries
+ end
+
+ def aggregate
+ average_metric? ? average_scope.average(average_value_key) : count_scope.count
+ end
+
+ def summary
+ metric_results = summary_scope
+ .select(*summary_select_fields)
+ .group(summary_group_by_key)
+ .index_by { |record| record.public_send(summary_index_key) }
+
+ merge_summary_results(metric_results, summary_conversation_counts)
+ end
+
+ private
+
+ def count_timeseries
+ grouped_count.map do |event_date, event_count|
+ { value: event_count, timestamp: event_date.in_time_zone(timezone).to_i }
+ end
+ end
+
+ def average_timeseries
+ grouped_average_time = grouped_average_scope.average(average_value_key)
+ grouped_event_count = grouped_average_scope.count
+
+ grouped_average_time.each_with_object([]) do |(event_date, average_time), results|
+ results << {
+ value: average_time,
+ timestamp: event_date.in_time_zone(timezone).to_i,
+ count: grouped_event_count[event_date]
+ }
+ end
+ end
+
+ def grouped_average_scope
+ average_scope.group_by_period(
+ group_by,
+ :created_at,
+ default_value: 0,
+ range: range,
+ permit: %w[day week month year hour],
+ time_zone: timezone
+ )
+ end
+
+ def grouped_count
+ count_scope.group_by_period(
+ group_by,
+ :created_at,
+ default_value: 0,
+ range: range,
+ permit: %w[day week month year hour],
+ time_zone: timezone
+ ).count
+ end
+
+ def average_scope
+ scope.reporting_events.where(name: raw_event_name, created_at: range, account_id: account.id)
+ end
+
+ def count_scope
+ case metric.to_s
+ when 'conversations_count'
+ scope.conversations.where(account_id: account.id, created_at: range)
+ when 'incoming_messages_count'
+ scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order)
+ when 'outgoing_messages_count'
+ scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order)
+ else
+ reporting_event_count_scope
+ end
+ end
+
+ def reporting_event_count_scope
+ events = scope.reporting_events.where(
+ name: raw_event_name,
+ account_id: account.id,
+ created_at: range
+ )
+
+ return events unless raw_count_strategy == :distinct_conversation
+
+ events.joins(:conversation).select(:conversation_id).distinct
+ end
+
+ def summary_scope
+ scope = account.reporting_events.where(created_at: range)
+ return scope.joins(:conversation) if dimension_type == 'team'
+
+ 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] = summary_attributes_for(record, conversation_counts[dimension_id])
+ end
+ end
+
+ def summary_select_fields
+ ["#{summary_group_by_key} as #{summary_index_key}"] + summary_metrics.map { |definition| summary_select_field(definition) }
+ end
+
+ def summary_select_field(definition)
+ 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}"
+ 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.count? ? value.to_i : value
+ end
+ end
+
+ def summary_group_by_key
+ {
+ 'account' => :account_id,
+ 'agent' => :user_id,
+ 'inbox' => :inbox_id,
+ 'team' => 'conversations.team_id'
+ }[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
+
+ def average_value_key
+ use_business_hours? ? :value_in_business_hours : :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..8df2e091d
--- /dev/null
+++ b/app/services/reports/report_metric_registry.rb
@@ -0,0 +1,120 @@
+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 initialize(name:, aggregate:, raw_event_name: nil, rollup_metric: nil, summary_key: nil, raw_count_strategy: nil) # rubocop:disable Metrics/ParameterLists
+ super
+ end
+
+ 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
+ ),
+ incoming_messages_count: Metric.new(
+ name: :incoming_messages_count,
+ aggregate: :count
+ ),
+ outgoing_messages_count: Metric.new(
+ name: :outgoing_messages_count,
+ aggregate: :count
+ ),
+ 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
+ ),
+ 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
+ ),
+ reply_time: Metric.new(
+ name: :reply_time,
+ aggregate: :average,
+ raw_event_name: :reply_time,
+ rollup_metric: :reply_time,
+ summary_key: :avg_reply_time
+ ),
+ resolutions_count: Metric.new(
+ name: :resolutions_count,
+ aggregate: :count,
+ raw_event_name: :conversation_resolved,
+ rollup_metric: :resolutions_count,
+ summary_key: :resolved_conversations_count
+ ),
+ bot_resolutions_count: Metric.new(
+ name: :bot_resolutions_count,
+ aggregate: :count,
+ raw_event_name: :conversation_bot_resolved,
+ rollup_metric: :bot_resolutions_count
+ ),
+ bot_handoffs_count: Metric.new(
+ name: :bot_handoffs_count,
+ aggregate: :count,
+ raw_event_name: :conversation_bot_handoff,
+ rollup_metric: :bot_handoffs_count,
+ 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/builders/v2/reports/conversations/metric_builder_spec.rb b/spec/builders/v2/reports/conversations/metric_builder_spec.rb
index 1b0ed7a38..018bd567b 100644
--- a/spec/builders/v2/reports/conversations/metric_builder_spec.rb
+++ b/spec/builders/v2/reports/conversations/metric_builder_spec.rb
@@ -5,12 +5,10 @@ RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do
let(:account) { create(:account) }
let(:params) { { since: '2023-01-01', until: '2024-01-01' } }
- let(:count_builder_instance) { instance_double(V2::Reports::Timeseries::CountReportBuilder, aggregate_value: 42) }
- let(:avg_builder_instance) { instance_double(V2::Reports::Timeseries::AverageReportBuilder, aggregate_value: 42) }
+ let(:builder_instance) { instance_double(V2::Reports::Timeseries::ReportBuilder, aggregate_value: 42) }
before do
- allow(V2::Reports::Timeseries::CountReportBuilder).to receive(:new).and_return(count_builder_instance)
- allow(V2::Reports::Timeseries::AverageReportBuilder).to receive(:new).and_return(avg_builder_instance)
+ allow(V2::Reports::Timeseries::ReportBuilder).to receive(:new).and_return(builder_instance)
end
describe '#summary' do
@@ -31,8 +29,8 @@ RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do
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::AverageReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
+ expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count'))
+ expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
end
end
diff --git a/spec/builders/v2/reports/conversations/report_builder_spec.rb b/spec/builders/v2/reports/conversations/report_builder_spec.rb
index db7a0ac45..a2b293461 100644
--- a/spec/builders/v2/reports/conversations/report_builder_spec.rb
+++ b/spec/builders/v2/reports/conversations/report_builder_spec.rb
@@ -4,19 +4,19 @@ describe V2::Reports::Conversations::ReportBuilder do
subject { described_class.new(account, params) }
let(:account) { create(:account) }
- let(:average_builder) { V2::Reports::Timeseries::AverageReportBuilder }
- let(:count_builder) { V2::Reports::Timeseries::CountReportBuilder }
+ let(:builder) { V2::Reports::Timeseries::ReportBuilder }
- shared_examples 'valid metric handler' do |metric, method, builder|
+ shared_examples 'valid metric handler' do |metric, method|
context 'when a valid metric is given' do
let(:params) { { metric: metric } }
- it "calls the correct #{method} builder for #{metric}" do
+ it "calls the shared #{method} builder for #{metric}" do
builder_instance = instance_double(builder)
allow(builder).to receive(:new).and_return(builder_instance)
- allow(builder_instance).to receive(method)
+ allow(builder_instance).to receive(method).and_return(:result)
- builder_instance.public_send(method)
+ expect(subject.public_send(method)).to eq(:result)
+ expect(builder).to have_received(:new).with(account, params)
expect(builder_instance).to have_received(method)
end
end
@@ -33,12 +33,12 @@ describe V2::Reports::Conversations::ReportBuilder do
end
describe '#timeseries' do
- it_behaves_like 'valid metric handler', 'avg_first_response_time', :timeseries, V2::Reports::Timeseries::AverageReportBuilder
- it_behaves_like 'valid metric handler', 'conversations_count', :timeseries, V2::Reports::Timeseries::CountReportBuilder
+ it_behaves_like 'valid metric handler', 'avg_first_response_time', :timeseries
+ it_behaves_like 'valid metric handler', 'conversations_count', :timeseries
end
describe '#aggregate_value' do
- it_behaves_like 'valid metric handler', 'avg_first_response_time', :aggregate_value, V2::Reports::Timeseries::AverageReportBuilder
- it_behaves_like 'valid metric handler', 'conversations_count', :aggregate_value, V2::Reports::Timeseries::CountReportBuilder
+ it_behaves_like 'valid metric handler', 'avg_first_response_time', :aggregate_value
+ it_behaves_like 'valid metric handler', 'conversations_count', :aggregate_value
end
end
diff --git a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb
deleted file mode 100644
index 4f6036f07..000000000
--- a/spec/builders/v2/reports/timeseries/average_report_builder_spec.rb
+++ /dev/null
@@ -1,174 +0,0 @@
-require 'rails_helper'
-
-describe V2::Reports::Timeseries::AverageReportBuilder do
- subject { described_class.new(account, params) }
-
- let(:account) { create(:account) }
- let(:team) { create(:team, account: account) }
- let(:inbox) { create(:inbox, account: account) }
- let(:label) { create(:label, title: 'spec-billing', account: account) }
- let!(:conversation) { create(:conversation, account: account, inbox: inbox, team: team) }
- let(:current_time) { '26.10.2020 10:00'.to_datetime }
-
- let(:params) do
- {
- type: filter_type,
- business_hours: business_hours,
- timezone_offset: timezone_offset,
- group_by: group_by,
- metric: metric,
- since: (current_time - 1.week).beginning_of_day.to_i.to_s,
- until: current_time.end_of_day.to_i.to_s,
- id: filter_id
- }
- end
- let(:timezone_offset) { nil }
- let(:group_by) { 'day' }
- let(:metric) { 'avg_first_response_time' }
- let(:business_hours) { false }
- let(:filter_type) { :account }
- let(:filter_id) { '' }
-
- before do
- travel_to current_time
- conversation.label_list.add(label.title)
- conversation.save!
- create(:reporting_event, name: 'first_response', value: 80, value_in_business_hours: 10, account: account, created_at: Time.zone.now,
- conversation: conversation, inbox: inbox)
- create(:reporting_event, name: 'first_response', value: 100, value_in_business_hours: 20, account: account, created_at: 1.hour.ago)
- create(:reporting_event, name: 'first_response', value: 93, value_in_business_hours: 30, account: account, created_at: 1.week.ago)
- end
-
- describe '#timeseries' do
- context 'when there is no filter applied' do
- it 'returns the correct values' do
- timeseries_values = subject.timeseries
-
- expect(timeseries_values).to eq(
- [
- { count: 1, timestamp: 1_603_065_600, value: 93.0 },
- { count: 0, timestamp: 1_603_152_000, value: 0 },
- { count: 0, timestamp: 1_603_238_400, value: 0 },
- { count: 0, timestamp: 1_603_324_800, value: 0 },
- { count: 0, timestamp: 1_603_411_200, value: 0 },
- { count: 0, timestamp: 1_603_497_600, value: 0 },
- { count: 0, timestamp: 1_603_584_000, value: 0 },
- { count: 2, timestamp: 1_603_670_400, value: 90.0 }
- ]
- )
- end
-
- context 'when business hours is provided' do
- let(:business_hours) { true }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
-
- expect(timeseries_values).to eq(
- [
- { count: 1, timestamp: 1_603_065_600, value: 30.0 },
- { count: 0, timestamp: 1_603_152_000, value: 0 },
- { count: 0, timestamp: 1_603_238_400, value: 0 },
- { count: 0, timestamp: 1_603_324_800, value: 0 },
- { count: 0, timestamp: 1_603_411_200, value: 0 },
- { count: 0, timestamp: 1_603_497_600, value: 0 },
- { count: 0, timestamp: 1_603_584_000, value: 0 },
- { count: 2, timestamp: 1_603_670_400, value: 15.0 }
- ]
- )
- end
- end
-
- context 'when group_by is provided' do
- let(:group_by) { 'week' }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
- expect(timeseries_values).to eq(
- [
- { count: 1, timestamp: (current_time - 1.week).beginning_of_week(:sunday).to_i, value: 93.0 },
- { count: 2, timestamp: current_time.beginning_of_week(:sunday).to_i, value: 90.0 }
- ]
- )
- end
- end
-
- context 'when timezone offset is provided' do
- let(:timezone_offset) { '5.5' }
- let(:group_by) { 'week' }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
- expect(timeseries_values).to eq(
- [
- { count: 1, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 93.0 },
- { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 }
- ]
- )
- end
- end
- end
-
- context 'when the label filter is applied' do
- let(:group_by) { 'week' }
- let(:filter_type) { 'label' }
- let(:filter_id) { label.id }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
- start_of_the_week = current_time.beginning_of_week(:sunday).to_i
- last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
- expect(timeseries_values).to eq(
- [
- { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
- { count: 1, timestamp: start_of_the_week, value: 80.0 }
- ]
- )
- end
- end
-
- context 'when the inbox filter is applied' do
- let(:group_by) { 'week' }
- let(:filter_type) { 'inbox' }
- let(:filter_id) { inbox.id }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
- start_of_the_week = current_time.beginning_of_week(:sunday).to_i
- last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
- expect(timeseries_values).to eq(
- [
- { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
- { count: 1, timestamp: start_of_the_week, value: 80.0 }
- ]
- )
- end
- end
-
- context 'when the team filter is applied' do
- let(:group_by) { 'week' }
- let(:filter_type) { 'team' }
- let(:filter_id) { team.id }
-
- it 'returns correct timeseries' do
- timeseries_values = subject.timeseries
- start_of_the_week = current_time.beginning_of_week(:sunday).to_i
- last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
- expect(timeseries_values).to eq(
- [
- { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
- { count: 1, timestamp: start_of_the_week, value: 80.0 }
- ]
- )
- end
- end
- end
-
- describe '#aggregate_value' do
- context 'when there is no filter applied' do
- it 'returns the correct average value' do
- expect(subject.aggregate_value).to eq 91.0
- end
- end
- end
-end
diff --git a/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb b/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb
deleted file mode 100644
index 038bd61c2..000000000
--- a/spec/builders/v2/reports/timeseries/count_report_builder_spec.rb
+++ /dev/null
@@ -1,113 +0,0 @@
-require 'rails_helper'
-
-describe V2::Reports::Timeseries::CountReportBuilder do
- subject { described_class.new(account, params) }
-
- let(:account) { create(:account) }
- let(:account2) { create(:account) }
- let(:user) { create(:user, email: 'agent1@example.com') }
- let(:inbox) { create(:inbox, account: account) }
- let(:inbox2) { create(:inbox, account: account2) }
- let(:current_time) { Time.current }
-
- let(:params) do
- {
- type: 'agent',
- metric: 'resolutions_count',
- since: (current_time - 1.day).beginning_of_day.to_i.to_s,
- until: current_time.end_of_day.to_i.to_s,
- id: user.id.to_s
- }
- end
-
- before do
- travel_to current_time
-
- # Add the same user to both accounts
- create(:account_user, account: account, user: user)
- create(:account_user, account: account2, user: user)
-
- # Create conversations in account1
- conversation1 = create(:conversation, account: account, inbox: inbox, assignee: user)
- conversation2 = create(:conversation, account: account, inbox: inbox, assignee: user)
-
- # Create conversations in account2
- conversation3 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
- conversation4 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
-
- # User resolves 2 conversations in account1
- create(:reporting_event,
- name: 'conversation_resolved',
- account: account,
- user: user,
- conversation: conversation1,
- created_at: current_time - 12.hours)
-
- create(:reporting_event,
- name: 'conversation_resolved',
- account: account,
- user: user,
- conversation: conversation2,
- created_at: current_time - 6.hours)
-
- # Same user resolves 3 conversations in account2 - these should NOT be counted for account1
- create(:reporting_event,
- name: 'conversation_resolved',
- account: account2,
- user: user,
- conversation: conversation3,
- created_at: current_time - 8.hours)
-
- create(:reporting_event,
- name: 'conversation_resolved',
- account: account2,
- user: user,
- conversation: conversation4,
- created_at: current_time - 4.hours)
-
- # Create another conversation in account2 for testing
- conversation5 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
- create(:reporting_event,
- name: 'conversation_resolved',
- account: account2,
- user: user,
- conversation: conversation5,
- created_at: current_time - 2.hours)
- end
-
- describe '#aggregate_value' do
- it 'returns only resolutions performed by the user in the specified account' do
- # User should have 2 resolutions in account1, not 5 (total across both accounts)
- expect(subject.aggregate_value).to eq(2)
- end
-
- context 'when querying account2' do
- subject { described_class.new(account2, params) }
-
- it 'returns only resolutions for account2' do
- # User should have 3 resolutions in account2
- expect(subject.aggregate_value).to eq(3)
- end
- end
- end
-
- describe '#timeseries' do
- it 'filters resolutions by account' do
- result = subject.timeseries
- # Should only count the 2 resolutions from account1
- total_count = result.sum { |r| r[:value] }
- expect(total_count).to eq(2)
- end
- end
-
- describe 'account isolation' do
- it 'does not leak data between accounts' do
- # If account isolation works correctly, the counts should be different
- account1_count = described_class.new(account, params).aggregate_value
- account2_count = described_class.new(account2, params).aggregate_value
-
- expect(account1_count).to eq(2)
- expect(account2_count).to eq(3)
- end
- end
-end
diff --git a/spec/builders/v2/reports/timeseries/report_builder_spec.rb b/spec/builders/v2/reports/timeseries/report_builder_spec.rb
new file mode 100644
index 000000000..eec3e99da
--- /dev/null
+++ b/spec/builders/v2/reports/timeseries/report_builder_spec.rb
@@ -0,0 +1,313 @@
+require 'rails_helper'
+
+describe V2::Reports::Timeseries::ReportBuilder do
+ describe 'average metrics' do
+ subject { described_class.new(account, params) }
+
+ let(:account) { create(:account) }
+ let(:team) { create(:team, account: account) }
+ let(:inbox) { create(:inbox, account: account) }
+ let(:label) { create(:label, title: 'spec-billing', account: account) }
+ let!(:conversation) { create(:conversation, account: account, inbox: inbox, team: team) }
+ let(:current_time) { '26.10.2020 10:00'.to_datetime }
+
+ let(:params) do
+ {
+ type: filter_type,
+ business_hours: business_hours,
+ timezone_offset: timezone_offset,
+ group_by: group_by,
+ metric: metric,
+ since: (current_time - 1.week).beginning_of_day.to_i.to_s,
+ until: current_time.end_of_day.to_i.to_s,
+ id: filter_id
+ }
+ end
+ let(:timezone_offset) { nil }
+ let(:group_by) { 'day' }
+ let(:metric) { 'avg_first_response_time' }
+ let(:business_hours) { false }
+ let(:filter_type) { :account }
+ let(:filter_id) { '' }
+
+ before do
+ travel_to current_time
+ conversation.label_list.add(label.title)
+ conversation.save!
+ create(:reporting_event, name: 'first_response', value: 80, value_in_business_hours: 10, account: account, created_at: Time.zone.now,
+ conversation: conversation, inbox: inbox)
+ create(:reporting_event, name: 'first_response', value: 100, value_in_business_hours: 20, account: account, created_at: 1.hour.ago)
+ create(:reporting_event, name: 'first_response', value: 93, value_in_business_hours: 30, account: account, created_at: 1.week.ago)
+ end
+
+ describe '#timeseries' do
+ it 'returns the correct values' do
+ timeseries_values = subject.timeseries
+
+ expect(timeseries_values).to eq(
+ [
+ { count: 1, timestamp: 1_603_065_600, value: 93.0 },
+ { count: 0, timestamp: 1_603_152_000, value: 0 },
+ { count: 0, timestamp: 1_603_238_400, value: 0 },
+ { count: 0, timestamp: 1_603_324_800, value: 0 },
+ { count: 0, timestamp: 1_603_411_200, value: 0 },
+ { count: 0, timestamp: 1_603_497_600, value: 0 },
+ { count: 0, timestamp: 1_603_584_000, value: 0 },
+ { count: 2, timestamp: 1_603_670_400, value: 90.0 }
+ ]
+ )
+ end
+
+ context 'when business hours is provided' do
+ let(:business_hours) { true }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+
+ expect(timeseries_values).to eq(
+ [
+ { count: 1, timestamp: 1_603_065_600, value: 30.0 },
+ { count: 0, timestamp: 1_603_152_000, value: 0 },
+ { count: 0, timestamp: 1_603_238_400, value: 0 },
+ { count: 0, timestamp: 1_603_324_800, value: 0 },
+ { count: 0, timestamp: 1_603_411_200, value: 0 },
+ { count: 0, timestamp: 1_603_497_600, value: 0 },
+ { count: 0, timestamp: 1_603_584_000, value: 0 },
+ { count: 2, timestamp: 1_603_670_400, value: 15.0 }
+ ]
+ )
+ end
+ end
+
+ context 'when group_by is provided' do
+ let(:group_by) { 'week' }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+ expect(timeseries_values).to eq(
+ [
+ { count: 1, timestamp: (current_time - 1.week).beginning_of_week(:sunday).to_i, value: 93.0 },
+ { count: 2, timestamp: current_time.beginning_of_week(:sunday).to_i, value: 90.0 }
+ ]
+ )
+ end
+ end
+
+ context 'when timezone offset is provided' do
+ let(:timezone_offset) { '5.5' }
+ let(:group_by) { 'week' }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+ expect(timeseries_values).to eq(
+ [
+ { count: 1, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 93.0 },
+ { count: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 90.0 }
+ ]
+ )
+ end
+ end
+
+ context 'when the label filter is applied' do
+ let(:group_by) { 'week' }
+ let(:filter_type) { 'label' }
+ let(:filter_id) { label.id }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+ start_of_the_week = current_time.beginning_of_week(:sunday).to_i
+ last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
+ expect(timeseries_values).to eq(
+ [
+ { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
+ { count: 1, timestamp: start_of_the_week, value: 80.0 }
+ ]
+ )
+ end
+ end
+
+ context 'when the inbox filter is applied' do
+ let(:group_by) { 'week' }
+ let(:filter_type) { 'inbox' }
+ let(:filter_id) { inbox.id }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+ start_of_the_week = current_time.beginning_of_week(:sunday).to_i
+ last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
+ expect(timeseries_values).to eq(
+ [
+ { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
+ { count: 1, timestamp: start_of_the_week, value: 80.0 }
+ ]
+ )
+ end
+ end
+
+ context 'when the team filter is applied' do
+ let(:group_by) { 'week' }
+ let(:filter_type) { 'team' }
+ let(:filter_id) { team.id }
+
+ it 'returns correct timeseries' do
+ timeseries_values = subject.timeseries
+ start_of_the_week = current_time.beginning_of_week(:sunday).to_i
+ last_week_start_of_the_week = (current_time - 1.week).beginning_of_week(:sunday).to_i
+ expect(timeseries_values).to eq(
+ [
+ { count: 0, timestamp: last_week_start_of_the_week, value: 0 },
+ { count: 1, timestamp: start_of_the_week, value: 80.0 }
+ ]
+ )
+ end
+ end
+ end
+
+ describe '#aggregate_value' do
+ context 'when there is no filter applied' do
+ it 'returns the correct average value' do
+ expect(subject.aggregate_value).to eq 91.0
+ end
+ end
+
+ context 'when rollups are enabled and the agent does not exist' do
+ let(:filter_type) { :agent }
+ let(:filter_id) { '999999' }
+ let(:timezone_offset) { '0' }
+
+ before do
+ account.update!(reporting_timezone: 'Etc/UTC')
+ allow(account).to receive(:feature_enabled?).with(:report_rollup).and_return(true)
+ end
+
+ it 'raises record not found to preserve raw path behavior' do
+ expect { subject.aggregate_value }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+ end
+ end
+
+ describe 'count metrics' do
+ subject { described_class.new(account, params) }
+
+ let(:account) { create(:account) }
+ let(:account2) { create(:account) }
+ let(:user) { create(:user, email: 'agent1@example.com') }
+ let(:inbox) { create(:inbox, account: account) }
+ let(:inbox2) { create(:inbox, account: account2) }
+ let(:current_time) { Time.current }
+
+ let(:params) do
+ {
+ type: 'agent',
+ metric: 'resolutions_count',
+ since: since_time.beginning_of_day.to_i.to_s,
+ until: current_time.end_of_day.to_i.to_s,
+ timezone_offset: timezone_offset,
+ group_by: group_by,
+ id: user.id.to_s
+ }
+ end
+ let(:group_by) { 'day' }
+ let(:since_time) { current_time - 1.day }
+ let(:timezone_offset) { nil }
+
+ before do
+ travel_to current_time
+
+ create(:account_user, account: account, user: user)
+ create(:account_user, account: account2, user: user)
+
+ conversation1 = create(:conversation, account: account, inbox: inbox, assignee: user)
+ conversation2 = create(:conversation, account: account, inbox: inbox, assignee: user)
+
+ conversation3 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
+ conversation4 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
+
+ create(:reporting_event,
+ name: 'conversation_resolved',
+ account: account,
+ user: user,
+ conversation: conversation1,
+ created_at: current_time - 12.hours)
+
+ create(:reporting_event,
+ name: 'conversation_resolved',
+ account: account,
+ user: user,
+ conversation: conversation2,
+ created_at: current_time - 6.hours)
+
+ create(:reporting_event,
+ name: 'conversation_resolved',
+ account: account2,
+ user: user,
+ conversation: conversation3,
+ created_at: current_time - 8.hours)
+
+ create(:reporting_event,
+ name: 'conversation_resolved',
+ account: account2,
+ user: user,
+ conversation: conversation4,
+ created_at: current_time - 4.hours)
+
+ conversation5 = create(:conversation, account: account2, inbox: inbox2, assignee: user)
+ create(:reporting_event,
+ name: 'conversation_resolved',
+ account: account2,
+ user: user,
+ conversation: conversation5,
+ created_at: current_time - 2.hours)
+ end
+
+ describe '#aggregate_value' do
+ it 'returns only resolutions performed by the user in the specified account' do
+ expect(subject.aggregate_value).to eq(2)
+ end
+
+ context 'when rollups are enabled and the agent does not exist' do
+ let(:timezone_offset) { '0' }
+
+ let(:params) do
+ super().merge(id: '999999')
+ end
+
+ before do
+ account.update!(reporting_timezone: 'Etc/UTC')
+ allow(account).to receive(:feature_enabled?).with(:report_rollup).and_return(true)
+ end
+
+ it 'raises record not found to preserve raw path behavior' do
+ expect { subject.aggregate_value }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+
+ context 'when querying account2' do
+ subject { described_class.new(account2, params) }
+
+ it 'returns only resolutions for account2' do
+ expect(subject.aggregate_value).to eq(3)
+ end
+ end
+ end
+
+ describe '#timeseries' do
+ it 'filters resolutions by account' do
+ result = subject.timeseries
+ total_count = result.sum { |row| row[:value] }
+ expect(total_count).to eq(2)
+ end
+ end
+
+ describe 'account isolation' do
+ it 'does not leak data between accounts' do
+ account1_count = described_class.new(account, params).aggregate_value
+ account2_count = described_class.new(account2, params).aggregate_value
+
+ expect(account1_count).to eq(2)
+ expect(account2_count).to eq(3)
+ end
+ end
+ end
+end
diff --git a/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb b/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb
index 38c66eeb0..696e1f53e 100644
--- a/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb
+++ b/spec/controllers/api/v2/accounts/summary_reports_controller_spec.rb
@@ -45,7 +45,10 @@ RSpec.describe 'Summary Reports API', type: :request do
headers: admin.create_new_auth_token,
as: :json
- expect(V2::Reports::AgentSummaryBuilder).to have_received(:new).with(account: account, params: params)
+ expect(V2::Reports::AgentSummaryBuilder).to have_received(:new).with(
+ account: account,
+ params: params.merge(type: :agent)
+ )
expect(agent_summary_builder).to have_received(:build)
expect(response).to have_http_status(:success)
@@ -96,7 +99,10 @@ RSpec.describe 'Summary Reports API', type: :request do
headers: admin.create_new_auth_token,
as: :json
- expect(V2::Reports::InboxSummaryBuilder).to have_received(:new).with(account: account, params: params)
+ expect(V2::Reports::InboxSummaryBuilder).to have_received(:new).with(
+ account: account,
+ params: params.merge(type: :inbox)
+ )
expect(inbox_summary_builder).to have_received(:build)
expect(response).to have_http_status(:success)
@@ -147,7 +153,10 @@ RSpec.describe 'Summary Reports API', type: :request do
headers: admin.create_new_auth_token,
as: :json
- expect(V2::Reports::TeamSummaryBuilder).to have_received(:new).with(account: account, params: params)
+ expect(V2::Reports::TeamSummaryBuilder).to have_received(:new).with(
+ account: account,
+ params: params.merge(type: :team)
+ )
expect(team_summary_builder).to have_received(:build)
expect(response).to have_http_status(:success)
diff --git a/spec/finders/email_channel_finder_spec.rb b/spec/finders/email_channel_finder_spec.rb
index d56d97008..06fe5c968 100644
--- a/spec/finders/email_channel_finder_spec.rb
+++ b/spec/finders/email_channel_finder_spec.rb
@@ -83,7 +83,7 @@ describe EmailChannelFinder do
reply_mail.mail['bcc'] = 'test@example.com'
# Configure other account IDs but not this one
- other_account_ids = [123, 456, 789]
+ other_account_ids = [channel_email.account_id + 1, channel_email.account_id + 2, channel_email.account_id + 3]
allow(GlobalConfigService).to receive(:load)
.with('SKIP_INCOMING_BCC_PROCESSING', '')
.and_return(other_account_ids.join(','))
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