refactor: replace count and average timeseries builders with unified ReportBuilder
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
class V2::ReportBuilder
|
||||
include DateRangeHelper
|
||||
include ReportHelper
|
||||
|
||||
attr_reader :account, :params
|
||||
|
||||
DEFAULT_GROUP_BY = 'day'.freeze
|
||||
|
||||
@@ -3,23 +3,10 @@ class V2::Reports::Conversations::BaseReportBuilder
|
||||
|
||||
private
|
||||
|
||||
AVG_METRICS = %w[avg_first_response_time avg_resolution_time reply_time].freeze
|
||||
COUNT_METRICS = %w[
|
||||
conversations_count
|
||||
incoming_messages_count
|
||||
outgoing_messages_count
|
||||
resolutions_count
|
||||
bot_resolutions_count
|
||||
bot_handoffs_count
|
||||
].freeze
|
||||
|
||||
def builder_class(metric)
|
||||
case metric
|
||||
when *AVG_METRICS
|
||||
V2::Reports::Timeseries::AverageReportBuilder
|
||||
when *COUNT_METRICS
|
||||
V2::Reports::Timeseries::CountReportBuilder
|
||||
end
|
||||
return unless ReportingEvents::MetricRegistry.supported_metric?(metric)
|
||||
|
||||
V2::Reports::Timeseries::ReportBuilder
|
||||
end
|
||||
|
||||
def log_invalid_metric
|
||||
|
||||
@@ -1,48 +0,0 @@
|
||||
class V2::Reports::Timeseries::AverageReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
def timeseries
|
||||
grouped_average_time = reporting_events.average(average_value_key)
|
||||
grouped_event_count = reporting_events.count
|
||||
grouped_average_time.each_with_object([]) do |element, arr|
|
||||
event_date, average_time = element
|
||||
arr << {
|
||||
value: average_time,
|
||||
timestamp: event_date.in_time_zone(timezone).to_i,
|
||||
count: grouped_event_count[event_date]
|
||||
}
|
||||
end
|
||||
end
|
||||
|
||||
def aggregate_value
|
||||
object_scope.average(average_value_key)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def event_name
|
||||
metric_to_event_name = {
|
||||
avg_first_response_time: :first_response,
|
||||
avg_resolution_time: :conversation_resolved,
|
||||
reply_time: :reply_time
|
||||
}
|
||||
metric_to_event_name[params[:metric].to_sym]
|
||||
end
|
||||
|
||||
def object_scope
|
||||
scope.reporting_events.where(name: event_name, created_at: range, account_id: account.id)
|
||||
end
|
||||
|
||||
def reporting_events
|
||||
@grouped_values = object_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 average_value_key
|
||||
@average_value_key ||= params[:business_hours].present? ? :value_in_business_hours : :value
|
||||
end
|
||||
end
|
||||
@@ -1,12 +1,13 @@
|
||||
class V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
include TimezoneHelper
|
||||
include DateRangeHelper
|
||||
|
||||
DEFAULT_GROUP_BY = 'day'.freeze
|
||||
|
||||
pattr_initialize :account, :params
|
||||
|
||||
def scope
|
||||
case params[:type].to_sym
|
||||
case dimension_type.to_sym
|
||||
when :account
|
||||
account
|
||||
when :inbox
|
||||
@@ -20,6 +21,21 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
end
|
||||
end
|
||||
|
||||
def data_source
|
||||
@data_source ||= Reports::DataSource.for(
|
||||
account: account,
|
||||
metric: params[:metric],
|
||||
dimension_type: dimension_type,
|
||||
dimension_id: params[:id],
|
||||
scope: scope,
|
||||
range: range,
|
||||
group_by: group_by,
|
||||
timezone: timezone,
|
||||
timezone_offset: params[:timezone_offset],
|
||||
business_hours: params[:business_hours]
|
||||
)
|
||||
end
|
||||
|
||||
def inbox
|
||||
@inbox ||= account.inboxes.find(params[:id])
|
||||
end
|
||||
@@ -41,6 +57,12 @@ class V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
end
|
||||
|
||||
def timezone
|
||||
@timezone ||= timezone_name_from_offset(params[:timezone_offset])
|
||||
@timezone ||= timezone_name_from_params(params[:timezone], params[:timezone_offset])
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def dimension_type
|
||||
(params[:type].presence || 'account').to_s
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,78 +0,0 @@
|
||||
class V2::Reports::Timeseries::CountReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
def timeseries
|
||||
grouped_count.each_with_object([]) do |element, arr|
|
||||
event_date, event_count = element
|
||||
|
||||
# The `event_date` is in Date format (without time), such as "Wed, 15 May 2024".
|
||||
# We need a timestamp for the start of the day. However, we can't use `event_date.to_time.to_i`
|
||||
# because it converts the date to 12:00 AM server timezone.
|
||||
# The desired output should be 12:00 AM in the specified timezone.
|
||||
arr << { value: event_count, timestamp: event_date.in_time_zone(timezone).to_i }
|
||||
end
|
||||
end
|
||||
|
||||
def aggregate_value
|
||||
object_scope.count
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def metric
|
||||
@metric ||= params[:metric]
|
||||
end
|
||||
|
||||
def object_scope
|
||||
send("scope_for_#{metric}")
|
||||
end
|
||||
|
||||
def scope_for_conversations_count
|
||||
scope.conversations.where(account_id: account.id, created_at: range)
|
||||
end
|
||||
|
||||
def scope_for_incoming_messages_count
|
||||
scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order)
|
||||
end
|
||||
|
||||
def scope_for_outgoing_messages_count
|
||||
scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order)
|
||||
end
|
||||
|
||||
def scope_for_resolutions_count
|
||||
scope.reporting_events.where(
|
||||
name: :conversation_resolved,
|
||||
account_id: account.id,
|
||||
created_at: range
|
||||
)
|
||||
end
|
||||
|
||||
def scope_for_bot_resolutions_count
|
||||
scope.reporting_events.where(
|
||||
name: :conversation_bot_resolved,
|
||||
account_id: account.id,
|
||||
created_at: range
|
||||
)
|
||||
end
|
||||
|
||||
def scope_for_bot_handoffs_count
|
||||
scope.reporting_events.joins(:conversation).select(:conversation_id).where(
|
||||
name: :conversation_bot_handoff,
|
||||
account_id: account.id,
|
||||
created_at: range
|
||||
).distinct
|
||||
end
|
||||
|
||||
def grouped_count
|
||||
# IMPORTANT: time_zone parameter affects both data grouping AND output timestamps
|
||||
# It converts timestamps to the target timezone before grouping, which means
|
||||
# the same event can fall into different day buckets depending on timezone
|
||||
# Example: 2024-01-15 00:00 UTC becomes 2024-01-14 16:00 PST (falls on different day)
|
||||
@grouped_values = object_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
|
||||
end
|
||||
@@ -0,0 +1,9 @@
|
||||
class V2::Reports::Timeseries::ReportBuilder < V2::Reports::Timeseries::BaseTimeseriesBuilder
|
||||
def timeseries
|
||||
data_source.timeseries
|
||||
end
|
||||
|
||||
def aggregate_value
|
||||
data_source.aggregate
|
||||
end
|
||||
end
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -0,0 +1,470 @@
|
||||
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: timezone,
|
||||
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) { nil }
|
||||
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 rollups are enabled' do
|
||||
let(:timezone_offset) { '5.5' }
|
||||
|
||||
before do
|
||||
account.update!(reporting_timezone: 'Chennai')
|
||||
allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: (current_time - 1.week).to_date,
|
||||
dimension_type: 'account',
|
||||
dimension_id: account.id,
|
||||
metric: 'first_response',
|
||||
count: 1,
|
||||
sum_value: 93.0,
|
||||
sum_value_business_hours: 30.0)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: current_time.to_date,
|
||||
dimension_type: 'account',
|
||||
dimension_id: account.id,
|
||||
metric: 'first_response',
|
||||
count: 2,
|
||||
sum_value: 180.0,
|
||||
sum_value_business_hours: 30.0)
|
||||
end
|
||||
|
||||
it 'preserves empty buckets in the timeseries' do
|
||||
rollup_timezone = ActiveSupport::TimeZone['Chennai']
|
||||
rollup_start_date = DateTime.strptime(params[:since], '%s').in_time_zone(rollup_timezone).to_date
|
||||
rollup_end_date = (DateTime.strptime(params[:until], '%s') - 1.second).in_time_zone(rollup_timezone).to_date
|
||||
rollup_dates = rollup_start_date..rollup_end_date
|
||||
|
||||
expected_timeseries = rollup_dates.map do |date|
|
||||
value = if date == (current_time - 1.week).to_date
|
||||
93.0
|
||||
elsif date == current_time.to_date
|
||||
90.0
|
||||
else
|
||||
0
|
||||
end
|
||||
|
||||
count = if date == (current_time - 1.week).to_date
|
||||
1
|
||||
elsif date == current_time.to_date
|
||||
2
|
||||
else
|
||||
0
|
||||
end
|
||||
|
||||
{ count: count, timestamp: date.in_time_zone('Chennai').to_i, value: value }
|
||||
end
|
||||
|
||||
expect(subject.timeseries).to eq(expected_timeseries)
|
||||
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 timezone is provided' do
|
||||
let(:timezone) { 'Asia/Kolkata' }
|
||||
let(:timezone_offset) { '0' }
|
||||
let(:group_by) { 'week' }
|
||||
|
||||
it 'uses the timezone name instead of the offset for timestamps' do
|
||||
expect(subject.timeseries).to eq(
|
||||
[
|
||||
{ count: 1, timestamp: (current_time - 1.week).in_time_zone('Asia/Kolkata').beginning_of_week(:sunday).to_i, value: 93.0 },
|
||||
{ count: 2, timestamp: current_time.in_time_zone('Asia/Kolkata').beginning_of_week(:sunday).to_i, value: 90.0 }
|
||||
]
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when weekly rollups are enabled' do
|
||||
let(:group_by) { 'week' }
|
||||
let(:timezone_offset) { '5.5' }
|
||||
|
||||
before do
|
||||
account.update!(reporting_timezone: 'Chennai')
|
||||
allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: current_time.to_date - 1.day,
|
||||
dimension_type: 'account',
|
||||
dimension_id: account.id,
|
||||
metric: 'first_response',
|
||||
count: 1,
|
||||
sum_value: 80.0,
|
||||
sum_value_business_hours: 10.0)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: current_time.to_date,
|
||||
dimension_type: 'account',
|
||||
dimension_id: account.id,
|
||||
metric: 'first_response',
|
||||
count: 1,
|
||||
sum_value: 100.0,
|
||||
sum_value_business_hours: 20.0)
|
||||
end
|
||||
|
||||
it 'groups weeks using sunday boundaries' do
|
||||
expect(subject.timeseries).to eq(
|
||||
[
|
||||
{ count: 0, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i, value: 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('reporting_events_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: timezone,
|
||||
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) { nil }
|
||||
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('reporting_events_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
|
||||
|
||||
context 'when rollups are enabled and grouped by week' do
|
||||
let(:group_by) { 'week' }
|
||||
let(:current_time) { Time.zone.parse('2020-10-26 10:00:00 UTC') }
|
||||
let(:since_time) { current_time - 1.week }
|
||||
let(:timezone_offset) { '5.5' }
|
||||
|
||||
before do
|
||||
account.update!(reporting_timezone: 'Chennai')
|
||||
allow(account).to receive(:feature_enabled?).with('reporting_events_rollup').and_return(true)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: current_time.to_date - 1.day,
|
||||
dimension_type: 'agent',
|
||||
dimension_id: user.id,
|
||||
metric: 'resolutions_count',
|
||||
count: 1,
|
||||
sum_value: 0.0,
|
||||
sum_value_business_hours: 0.0)
|
||||
|
||||
create(:reporting_events_rollup,
|
||||
account: account,
|
||||
date: current_time.to_date,
|
||||
dimension_type: 'agent',
|
||||
dimension_id: user.id,
|
||||
metric: 'resolutions_count',
|
||||
count: 1,
|
||||
sum_value: 0.0,
|
||||
sum_value_business_hours: 0.0)
|
||||
end
|
||||
|
||||
it 'groups weeks using sunday boundaries' do
|
||||
expect(subject.timeseries).to eq(
|
||||
[
|
||||
{ value: 0, timestamp: (current_time - 1.week).in_time_zone('Chennai').beginning_of_week(:sunday).to_i },
|
||||
{ value: 2, timestamp: current_time.in_time_zone('Chennai').beginning_of_week(:sunday).to_i }
|
||||
]
|
||||
)
|
||||
end
|
||||
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
|
||||
Reference in New Issue
Block a user