mirror of
https://github.com/lingble/chatwoot.git
synced 2025-11-15 10:35:09 +00:00
ef4e287f0d
There was a fundamental difference in how resolution counts were
calculated between the agent summary and timeseries reports, causing
confusion for users when the numbers didn't match.
The agent summary report counted all `conversation_resolved` events
within a time period by querying the `reporting_events` table directly.
However, the timeseries report had an additional constraint that
required the conversation to currently be in resolved status
(`conversations.status = 1`). This meant that if an agent resolved a
conversation that was later reopened, the resolution action would be
counted in the summary but not in the timeseries.
This fix aligns both reports to count resolution events rather than
conversations in resolved state. When an agent resolves a conversation,
they should receive credit for that action regardless of what happens to
the conversation afterward. The same logic now applies to bot
resolutions as well.
The change removes the `conversations: { status: :resolved }` condition
from both `scope_for_resolutions_count` and
`scope_for_bot_resolutions_count` methods in CountReportBuilder, and
updates the corresponding test expectations to reflect that all
resolution events are counted.
## About timezone
When a timezone is specified via `timezone_offset` parameter, the
reporting system:
1. Converts timestamps to the target timezone before grouping
2. Groups data by local day/week/month boundaries in that timezone, but
the primary boundaries are sent by the frontend and used as-is
3. Returns timestamps representing midnight in the target timezone
This means the same events can appear in different day buckets depending
on the timezone used. For summary reports, it works fine, since the user
only needs the total count between two timestamps and the frontend sends
the timestamps adjusted for timezone.
## Testing Locally
Run the following command, this will erase all data for that account and
put in 1000 conversations over last 3 months, parameters of this can be
tweaked in `Seeders::Reports::ReportDataSeeder`
I'd suggest updating the values to generate data over 30 days, with
10000 conversations, it will take it's sweet time to run but then the
data will be really rich, great for testing.
```
ACCOUNT_ID=2 ENABLE_ACCOUNT_SEEDING=true bundle exec rake db:seed:reports_data
```
Pro Tip: Don't run the app when the seeder is active, we manually create
the reporting events anyway. So once done just use `redis-cli FLUSHALL`
to clear all sidekiq jobs. Will be easier on the system
Use the following scripts to test it
- https://gist.github.com/scmmishra/1263a922f5efd24df8e448a816a06257
- https://gist.github.com/scmmishra/ca0b861fa0139e2cccdb72526ea844b2
- https://gist.github.com/scmmishra/5fe73d1f48f35422fd1fd142ea3498f3
- https://gist.github.com/scmmishra/3b7b1f9e2ff149007170e5c329432f45
- https://gist.github.com/scmmishra/f245fa2f44cd973e5d60aac64f979162
---------
Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
Co-authored-by: Pranav <pranav@chatwoot.com>
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
479 lines
16 KiB
Ruby
479 lines
16 KiB
Ruby
require 'rails_helper'
|
|
|
|
describe V2::ReportBuilder do
|
|
include ActiveJob::TestHelper
|
|
let_it_be(:account) { create(:account) }
|
|
let_it_be(:label_1) { create(:label, title: 'Label_1', account: account) }
|
|
let_it_be(:label_2) { create(:label, title: 'Label_2', account: account) }
|
|
|
|
describe '#timeseries' do
|
|
before do
|
|
travel_to(Time.zone.today) do
|
|
user = create(:user, account: account)
|
|
inbox = create(:inbox, account: account)
|
|
create(:inbox_member, user: user, inbox: inbox)
|
|
|
|
gravatar_url = 'https://www.gravatar.com'
|
|
stub_request(:get, /#{gravatar_url}.*/).to_return(status: 404)
|
|
|
|
perform_enqueued_jobs do
|
|
10.times do
|
|
conversation = create(:conversation, account: account,
|
|
inbox: inbox, assignee: user,
|
|
created_at: Time.zone.today)
|
|
create_list(:message, 5, message_type: 'outgoing',
|
|
account: account, inbox: inbox,
|
|
conversation: conversation, created_at: Time.zone.today + 2.hours)
|
|
create_list(:message, 2, message_type: 'incoming',
|
|
account: account, inbox: inbox,
|
|
conversation: conversation,
|
|
created_at: Time.zone.today + 3.hours)
|
|
conversation.update_labels('label_1')
|
|
conversation.label_list
|
|
conversation.save!
|
|
end
|
|
|
|
5.times do
|
|
conversation = create(:conversation, account: account,
|
|
inbox: inbox, assignee: user,
|
|
created_at: (Time.zone.today - 2.days))
|
|
create_list(:message, 3, message_type: 'outgoing',
|
|
account: account, inbox: inbox,
|
|
conversation: conversation,
|
|
created_at: (Time.zone.today - 2.days))
|
|
create_list(:message, 1, message_type: 'incoming',
|
|
account: account, inbox: inbox,
|
|
conversation: conversation,
|
|
created_at: (Time.zone.today - 2.days))
|
|
conversation.update_labels('label_2')
|
|
conversation.label_list
|
|
conversation.save!
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'when report type is account' do
|
|
it 'return conversations count' do
|
|
params = {
|
|
metric: 'conversations_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today]).to be 10
|
|
expect(metrics[Time.zone.today - 2.days]).to be 5
|
|
end
|
|
|
|
it 'return incoming messages count' do
|
|
params = {
|
|
metric: 'incoming_messages_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today]).to be 20
|
|
expect(metrics[Time.zone.today - 2.days]).to be 5
|
|
end
|
|
|
|
it 'return outgoing messages count' do
|
|
params = {
|
|
metric: 'outgoing_messages_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today]).to be 50
|
|
expect(metrics[Time.zone.today - 2.days]).to be 15
|
|
end
|
|
|
|
it 'return resolutions count' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'resolutions_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
perform_enqueued_jobs do
|
|
# Resolve all 5 conversations
|
|
conversations.each(&:resolved!)
|
|
|
|
# Reopen 1 conversation
|
|
conversations.first.open!
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
# 5 resolution events occurred (even though 1 was later reopened)
|
|
expect(metrics[Time.zone.today]).to be 5
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
end
|
|
|
|
it 'return resolutions count with multiple resolutions of same conversation' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'resolutions_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
perform_enqueued_jobs do
|
|
# Resolve all 5 conversations (first round)
|
|
conversations.each(&:resolved!)
|
|
|
|
# Reopen 2 conversations and resolve them again
|
|
conversations.first(2).each do |conversation|
|
|
conversation.open!
|
|
conversation.resolved!
|
|
end
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
# 7 total resolution events: 5 initial + 2 re-resolutions
|
|
expect(metrics[Time.zone.today]).to be 7
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
end
|
|
|
|
it 'returns bot_resolutions count' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'bot_resolutions_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
create(:agent_bot_inbox, inbox: account.inboxes.first)
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
conversations.each do |conversation|
|
|
conversation.messages.outgoing.all.update(sender: nil)
|
|
end
|
|
|
|
perform_enqueued_jobs do
|
|
# Resolve all 5 conversations
|
|
conversations.each(&:resolved!)
|
|
|
|
# Reopen 1 conversation
|
|
conversations.first.open!
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
summary = builder.bot_summary
|
|
|
|
# 5 bot resolution events occurred (even though 1 was later reopened)
|
|
expect(metrics[Time.zone.today]).to be 5
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
expect(summary[:bot_resolutions_count]).to be 5
|
|
end
|
|
end
|
|
|
|
it 'return bot_handoff count' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'bot_handoffs_count',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
create(:agent_bot_inbox, inbox: account.inboxes.first)
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
conversations.each do |conversation|
|
|
conversation.pending!
|
|
conversation.messages.outgoing.all.update(sender: nil)
|
|
end
|
|
|
|
perform_enqueued_jobs do
|
|
# Resolve all 5 conversations
|
|
conversations.each(&:bot_handoff!)
|
|
|
|
# Reopen 1 conversation
|
|
conversations.first.open!
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
summary = builder.bot_summary
|
|
|
|
# 4 conversations are resolved
|
|
expect(metrics[Time.zone.today]).to be 5
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
expect(summary[:bot_handoffs_count]).to be 5
|
|
end
|
|
end
|
|
|
|
it 'returns average first response time' do
|
|
params = {
|
|
metric: 'avg_first_response_time',
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today].to_f).to be 0.48e4
|
|
end
|
|
|
|
it 'returns summary' do
|
|
params = {
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.summary
|
|
|
|
expect(metrics[:conversations_count]).to be 15
|
|
expect(metrics[:incoming_messages_count]).to be 25
|
|
expect(metrics[:outgoing_messages_count]).to be 65
|
|
expect(metrics[:avg_resolution_time]).to be 0
|
|
expect(metrics[:resolutions_count]).to be 0
|
|
end
|
|
|
|
it 'returns argument error for incorrect group by' do
|
|
params = {
|
|
type: :account,
|
|
metric: 'avg_first_response_time',
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s,
|
|
group_by: 'test'.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
expect { builder.timeseries }.to raise_error(ArgumentError)
|
|
end
|
|
|
|
it 'logs error when metric is nil' do
|
|
params = {
|
|
metric: nil, # Set metric to nil to test this case
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
|
|
expect(Rails.logger).to receive(:error).with('ReportBuilder: Invalid metric - ')
|
|
builder.timeseries
|
|
end
|
|
|
|
it 'calls the appropriate metric method for a valid metric' do
|
|
params = {
|
|
metric: 'not_conversation_count', # Provide a invalid metric
|
|
type: :account,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
expect(Rails.logger).to receive(:error).with('ReportBuilder: Invalid metric - not_conversation_count')
|
|
|
|
builder.timeseries
|
|
end
|
|
end
|
|
|
|
context 'when report type is label' do
|
|
it 'return conversations count' do
|
|
params = {
|
|
metric: 'conversations_count',
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today - 2.days]).to be 5
|
|
end
|
|
|
|
it 'return incoming messages count' do
|
|
params = {
|
|
metric: 'incoming_messages_count',
|
|
type: :label,
|
|
id: label_1.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: (Time.zone.today + 1.day).to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today]).to be 20
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
|
|
it 'return outgoing messages count' do
|
|
params = {
|
|
metric: 'outgoing_messages_count',
|
|
type: :label,
|
|
id: label_1.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: (Time.zone.today + 1.day).to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
expect(metrics[Time.zone.today]).to be 50
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
|
|
it 'return resolutions count' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'resolutions_count',
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: (Time.zone.today + 1.day).to_time.to_i.to_s
|
|
}
|
|
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
|
|
perform_enqueued_jobs do
|
|
# ensure 5 reporting events are created
|
|
conversations.each(&:resolved!)
|
|
|
|
# open one of the conversations to check if it is not counted
|
|
conversations.last.open!
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
# this should count all 5 resolution events (even though 1 was later reopened)
|
|
expect(metrics[Time.zone.today]).to be 5
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
end
|
|
|
|
it 'return resolutions count with multiple resolutions of same conversation' do
|
|
travel_to(Time.zone.today) do
|
|
params = {
|
|
metric: 'resolutions_count',
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: (Time.zone.today + 1.day).to_time.to_i.to_s
|
|
}
|
|
|
|
conversations = account.conversations.where('created_at < ?', 1.day.ago)
|
|
|
|
perform_enqueued_jobs do
|
|
# Resolve all 5 conversations (first round)
|
|
conversations.each(&:resolved!)
|
|
|
|
# Reopen 3 conversations and resolve them again
|
|
conversations.first(3).each do |conversation|
|
|
conversation.open!
|
|
conversation.resolved!
|
|
end
|
|
end
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
|
|
# 8 total resolution events: 5 initial + 3 re-resolutions
|
|
expect(metrics[Time.zone.today]).to be 8
|
|
expect(metrics[Time.zone.today - 2.days]).to be 0
|
|
end
|
|
end
|
|
|
|
it 'returns average first response time' do
|
|
label_2.reporting_events.update(value: 1.5)
|
|
|
|
params = {
|
|
metric: 'avg_first_response_time',
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.timeseries
|
|
expect(metrics[Time.zone.today].to_f).to be 0.15e1
|
|
end
|
|
|
|
it 'returns summary' do
|
|
params = {
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.summary
|
|
|
|
expect(metrics[:conversations_count]).to be 5
|
|
expect(metrics[:incoming_messages_count]).to be 5
|
|
expect(metrics[:outgoing_messages_count]).to be 15
|
|
expect(metrics[:avg_resolution_time]).to be 0
|
|
expect(metrics[:resolutions_count]).to be 0
|
|
end
|
|
|
|
it 'returns summary for correct group by' do
|
|
params = {
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s,
|
|
group_by: 'week'.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
metrics = builder.summary
|
|
|
|
expect(metrics[:conversations_count]).to be 5
|
|
expect(metrics[:incoming_messages_count]).to be 5
|
|
expect(metrics[:outgoing_messages_count]).to be 15
|
|
expect(metrics[:avg_resolution_time]).to be 0
|
|
expect(metrics[:resolutions_count]).to be 0
|
|
end
|
|
|
|
it 'returns argument error for incorrect group by' do
|
|
params = {
|
|
metric: 'avg_first_response_time',
|
|
type: :label,
|
|
id: label_2.id,
|
|
since: (Time.zone.today - 3.days).to_time.to_i.to_s,
|
|
until: Time.zone.today.end_of_day.to_time.to_i.to_s,
|
|
group_by: 'test'.to_s
|
|
}
|
|
|
|
builder = described_class.new(account, params)
|
|
expect { builder.timeseries }.to raise_error(ArgumentError)
|
|
end
|
|
end
|
|
end
|
|
end
|