Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: System messages are counted as agents' first responses in livechat rooms #32846

Merged
merged 37 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
0e69a66
fix: System messages are counted as first responses to the first resp…
matheusbsilva137 Jul 17, 2024
c864c7b
tests: add end-to-end tests
matheusbsilva137 Jul 19, 2024
1bc7c50
tests: add end-to-end tests for the best response time metric
matheusbsilva137 Jul 19, 2024
d5a7981
tests: improve average first response time tests
matheusbsilva137 Jul 19, 2024
89e9184
Create changeset
matheusbsilva137 Jul 19, 2024
bddee63
tests: fix total number of rooms
matheusbsilva137 Jul 19, 2024
6905cf1
tests: fix end-to-end tests
matheusbsilva137 Jul 23, 2024
92b8198
Merge branch 'develop' into fix/first-response-time-sys-messages
matheusbsilva137 Jul 23, 2024
a8de432
tests: use value expected by incorrect tests
matheusbsilva137 Jul 23, 2024
f3a2de1
Merge branch 'fix/first-response-time-sys-messages' of https://github…
matheusbsilva137 Jul 23, 2024
cbdcb10
tests: apply changes requested
matheusbsilva137 Jul 24, 2024
182644a
fix lint
matheusbsilva137 Jul 24, 2024
af9bd4c
fix end-to-end tests
matheusbsilva137 Jul 24, 2024
eb678ab
Validate message type in isSystemMessage
matheusbsilva137 Jul 25, 2024
4859b8e
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jul 25, 2024
e4248ab
improve omnichannel system message types
matheusbsilva137 Jul 25, 2024
e9ba575
improve tests legibility
matheusbsilva137 Aug 7, 2024
637ff8e
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Aug 7, 2024
a5bb27d
solve remaining conflict
matheusbsilva137 Aug 7, 2024
458cded
check for system messages in all omnichannel callbacks that handle re…
matheusbsilva137 Aug 7, 2024
ece1f89
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Aug 7, 2024
554eb10
do not handle system message in markRoomResponded
matheusbsilva137 Aug 7, 2024
de1ff87
do not consider visitor messages as responses in livechat metrics
matheusbsilva137 Aug 8, 2024
a56436a
import fix from 33022
matheusbsilva137 Aug 9, 2024
327f893
Fix typecheck
matheusbsilva137 Aug 9, 2024
05355e1
Merge branch 'develop' into fix/first-response-time-sys-messages
matheusbsilva137 Aug 9, 2024
4549843
Merge branch 'develop' into fix/first-response-time-sys-messages
KevLehman Aug 13, 2024
424ff03
Revert "Fix typecheck"
matheusbsilva137 Aug 19, 2024
c2955b7
Revert "import fix from 33022"
matheusbsilva137 Aug 19, 2024
e1f0a04
Revert "do not consider visitor messages as responses in livechat met…
matheusbsilva137 Aug 19, 2024
ca9a5ea
Merge branch 'develop' into fix/first-response-time-sys-messages
matheusbsilva137 Aug 20, 2024
f274aad
Merge branch 'develop' into fix/first-response-time-sys-messages
KevLehman Aug 20, 2024
f91b1ab
Use isSystemMessage instead of message.t check
matheusbsilva137 Aug 21, 2024
abed53c
Update apps/meteor/app/livechat/server/hooks/markRoomResponded.ts
matheusbsilva137 Aug 21, 2024
488f185
Assure data array shouldnt be empty in end-to-end tests
matheusbsilva137 Aug 21, 2024
661a82d
Update delayInS variable name
matheusbsilva137 Aug 21, 2024
f7151c7
Merge branch 'develop' into fix/first-response-time-sys-messages
matheusbsilva137 Aug 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/rotten-camels-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/core-typings": patch
---

Fixed issue with system messages being counted as agents' first responses in livechat rooms (which caused the "best first response time" and "average first response time" metrics to be unreliable for all agents)
6 changes: 3 additions & 3 deletions apps/meteor/app/livechat/server/hooks/saveAnalyticsData.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isEditedMessage, isOmnichannelRoom } from '@rocket.chat/core-typings';
import { isEditedMessage, isOmnichannelRoom, isSystemMessage } from '@rocket.chat/core-typings';
import { LivechatRooms } from '@rocket.chat/models';

import { callbacks } from '../../../../lib/callbacks';
Expand All @@ -12,8 +12,8 @@ callbacks.add(
return message;
}

// skips this callback if the message was edited
if (!message || isEditedMessage(message)) {
// skips this callback if the message was edited or if it is a system message
if (!message || isEditedMessage(message) || isSystemMessage(message)) {
return message;
}

Expand Down
5 changes: 4 additions & 1 deletion apps/meteor/app/livechat/server/lib/AnalyticsTyped.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { OmnichannelAnalytics } from '@rocket.chat/core-services';
import mem from 'mem';

export const getAgentOverviewDataCached = mem(OmnichannelAnalytics.getAgentOverviewData, { maxAge: 60000, cacheKey: JSON.stringify });
export const getAgentOverviewDataCached = mem(OmnichannelAnalytics.getAgentOverviewData, {
maxAge: process.env.TEST_MODE === 'true' ? 1 : 60000,
cacheKey: JSON.stringify,
});
// Agent overview data on realtime is cached for 5 seconds
// while the data on the overview page is cached for 1 minute
export const getAnalyticsOverviewDataCached = mem(OmnichannelAnalytics.getAnalyticsOverviewData, {
Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/tests/data/livechat/rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,11 @@ export const uploadFile = (roomId: string, visitorToken: string): Promise<IMessa
};

// Sends a message using sendMessage method from agent
export const sendAgentMessage = (roomId: string, msg?: string): Promise<IMessage> => {
export const sendAgentMessage = (roomId: string, userCredentials: Credentials = credentials, msg?: string): Promise<IMessage> => {
return new Promise((resolve, reject) => {
void request
.post(methodCall('sendMessage'))
.set(credentials)
.set(userCredentials)
.send({
message: JSON.stringify({
method: 'sendMessage',
Expand Down
214 changes: 211 additions & 3 deletions apps/meteor/tests/end-to-end/api/livechat/04-dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import type { Credentials } from '@rocket.chat/api-client';
import type { ILivechatDepartment, IUser } from '@rocket.chat/core-typings';
import { Random } from '@rocket.chat/random';
import { expect } from 'chai';
import { before, describe, it } from 'mocha';
import { before, after, afterEach, describe, it } from 'mocha';
import moment from 'moment';
import type { Response } from 'supertest';

import { getCredentials, api, request, credentials } from '../../../data/api-data';
import { addOrRemoveAgentFromDepartment, createDepartmentWithAnOnlineAgent } from '../../../data/livechat/department';
import {
closeOmnichannelRoom,
deleteVisitor,
placeRoomOnHold,
sendAgentMessage,
sendMessage,
Expand All @@ -19,6 +20,7 @@ import {
import { createAnOnlineAgent } from '../../../data/livechat/users';
import { sleep } from '../../../data/livechat/utils';
import { removePermissionFromAllRoles, restorePermissionToRoles, updateSetting } from '../../../data/permissions.helper';
import { deleteUser } from '../../../data/users.helper';
import { IS_EE } from '../../../e2e/config/constants';

describe('LIVECHAT - dashboards', function () {
Expand Down Expand Up @@ -777,6 +779,212 @@ describe('LIVECHAT - dashboards', function () {
});
});

(IS_EE ? describe : describe.skip)('[livechat/analytics/agent-overview] - Average first response time', () => {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
let agent: { credentials: Credentials; user: IUser & { username: string } };
let originalFirstResponseTimeInSeconds: number;
let roomId: string;
let visitorToken: string;
const firstDelayInS = 4;
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
const secondDelayInS = 8;
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

before(async () => {
agent = await createAnOnlineAgent();
});

after(async () => {
await deleteUser(agent.user);
});

afterEach(() => Promise.all([deleteVisitor(visitorToken), closeOmnichannelRoom(roomId)]));
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

it('should return no average response time for an agent if no response has been sent in the period', async () => {
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

const today = moment().startOf('day').format('YYYY-MM-DD');

const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Avg_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');
expect(result.body.data).to.not.deep.include({ name: agent.user.username });
KevLehman marked this conversation as resolved.
Show resolved Hide resolved
});

it('should only consider a first response has been sent when an agent sends a text message', async () => {
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

await sleep(firstDelayInS * 1000);
await sendAgentMessage(roomId, agent.credentials);

const today = moment().startOf('day').format('YYYY-MM-DD');
const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Avg_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');

const agentData = result.body.data.find(
(agentOverviewData: { name: string; value: string }) => agentOverviewData.name === agent.user.username,
);
expect(agentData).to.not.be.undefined;
expect(agentData).to.have.property('name', agent.user.username);
expect(agentData).to.have.property('value');
originalFirstResponseTimeInSeconds = moment.duration(agentData.value).asSeconds();
expect(originalFirstResponseTimeInSeconds).to.be.greaterThanOrEqual(firstDelayInS);
});

it('should correctly calculate the average time of first responses for an agent', async () => {
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

await sleep(secondDelayInS * 1000);
await sendAgentMessage(roomId, agent.credentials);

const today = moment().startOf('day').format('YYYY-MM-DD');
const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Avg_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');

const agentData = result.body.data.find(
(agentOverviewData: { name: string; value: string }) => agentOverviewData.name === agent.user.username,
);
expect(agentData).to.not.be.undefined;
expect(agentData).to.have.property('name', agent.user.username);
expect(agentData).to.have.property('value');
const averageFirstResponseTimeInSeconds = moment.duration(agentData.value).asSeconds();
expect(averageFirstResponseTimeInSeconds).to.be.greaterThan(originalFirstResponseTimeInSeconds);
expect(averageFirstResponseTimeInSeconds).to.be.greaterThanOrEqual((firstDelayInS + secondDelayInS) / 2);
expect(averageFirstResponseTimeInSeconds).to.be.lessThan(secondDelayInS);
});
});

(IS_EE ? describe : describe.skip)('[livechat/analytics/agent-overview] - Best first response time', () => {
let agent: { credentials: Credentials; user: IUser & { username: string } };
let originalBestFirstResponseTimeInSeconds: number;
let roomId: string;
let visitorToken: string;

before(async () => {
agent = await createAnOnlineAgent();
});

after(() => deleteUser(agent.user));

afterEach(() => Promise.all([deleteVisitor(visitorToken), closeOmnichannelRoom(roomId)]));
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

it('should return no best response time for an agent if no response has been sent in the period', async () => {
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

const today = moment().startOf('day').format('YYYY-MM-DD');

const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Best_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');
expect(result.body.data).to.not.deep.include({ name: agent.user.username });
KevLehman marked this conversation as resolved.
Show resolved Hide resolved
});

it('should only consider a first response has been sent when an agent sends a text message', async () => {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

const delayInS = 4;
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
await sleep(delayInS * 1000);

await sendAgentMessage(roomId, agent.credentials);

const today = moment().startOf('day').format('YYYY-MM-DD');
const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Best_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');

const agentData = result.body.data.find(
(agentOverviewData: { name: string; value: string }) => agentOverviewData.name === agent.user.username,
);
expect(agentData).to.not.be.undefined;
expect(agentData).to.have.property('name', agent.user.username);
expect(agentData).to.have.property('value');
originalBestFirstResponseTimeInSeconds = moment.duration(agentData.value).asSeconds();
expect(originalBestFirstResponseTimeInSeconds).to.be.greaterThanOrEqual(delayInS);
});

it('should correctly calculate the best first response time for an agent and there are multiple first responses in the period', async () => {
const response = await startANewLivechatRoomAndTakeIt({ agent: agent.credentials });
roomId = response.room._id;
visitorToken = response.visitor.token;

const delayInS = 6;
await sleep(delayInS * 1000);

await sendAgentMessage(roomId, agent.credentials);

const today = moment().startOf('day').format('YYYY-MM-DD');
const result = await request
.get(api('livechat/analytics/agent-overview'))
.query({ from: today, to: today, name: 'Best_first_response_time' })
.set(credentials)
.expect('Content-Type', 'application/json')
.expect(200);

expect(result.body).to.have.property('success', true);
expect(result.body).to.have.property('head');
expect(result.body).to.have.property('data');
expect(result.body.data).to.be.an('array');

const agentData = result.body.data.find(
(agentOverviewData: { name: string; value: string }) => agentOverviewData.name === agent.user.username,
);
expect(agentData).to.not.be.undefined;
expect(agentData).to.have.property('name', agent.user.username);
expect(agentData).to.have.property('value');
const bestFirstResponseTimeInSeconds = moment.duration(agentData.value).asSeconds();
expect(bestFirstResponseTimeInSeconds).to.be.equal(originalBestFirstResponseTimeInSeconds);
});
});

describe('livechat/analytics/overview', () => {
it('should return an "unauthorized error" when the user does not have the necessary permission', async () => {
await removePermissionFromAllRoles('view-livechat-manager');
Expand Down Expand Up @@ -835,12 +1043,12 @@ describe('LIVECHAT - dashboards', function () {
expect(result.body).to.be.an('array');

const expectedResult = [
{ title: 'Total_conversations', value: 7 },
{ title: 'Total_conversations', value: 13 },
{ title: 'Open_conversations', value: 4 },
{ title: 'On_Hold_conversations', value: 1 },
// { title: 'Total_messages', value: 6 },
// { title: 'Busiest_day', value: moment().format('dddd') },
{ title: 'Conversations_per_day', value: '3.50' },
{ title: 'Conversations_per_day', value: '6.50' },
// { title: 'Busiest_time', value: '' },
];

Expand Down
4 changes: 2 additions & 2 deletions apps/meteor/tests/end-to-end/api/livechat/20-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ILivechatAgent, ILivechatVisitor, IOmnichannelRoom, IRoom } from '
import { expect } from 'chai';
import { before, describe, it, after } from 'mocha';

import { api, getCredentials, request } from '../../../data/api-data';
import { api, getCredentials, credentials, request } from '../../../data/api-data';
import { sendSimpleMessage } from '../../../data/chat.helper';
import {
sendMessage,
Expand Down Expand Up @@ -39,7 +39,7 @@ describe('LIVECHAT - messages', () => {

await sendMessage(roomId, 'Hello from visitor', token);
const agentMsgSentence = faker.lorem.sentence();
const agentMsg = await sendAgentMessage(roomId, agentMsgSentence);
const agentMsg = await sendAgentMessage(roomId, credentials, agentMsgSentence);
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

const siteUrl = process.env.SITE_URL || process.env.TEST_API_URL || 'http://localhost:3000';

Expand Down
8 changes: 5 additions & 3 deletions packages/core-typings/src/IMessage/IMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ export interface IMessage extends IRocketChatRecord {
};
}

export type MessageSystem = {
t: 'system';
};
export interface ISystemMessage extends IMessage {
t: MessageTypesValues;
}

export interface IEditedMessage extends IMessage {
editedAt: Date;
Expand All @@ -249,6 +249,8 @@ export const isEditedMessage = (message: IMessage): message is IEditedMessage =>
'_id' in (message as IEditedMessage).editedBy &&
typeof (message as IEditedMessage).editedBy._id === 'string';

export const isSystemMessage = (message: IMessage): message is ISystemMessage => 't' in message;
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

export const isDeletedMessage = (message: IMessage): message is IEditedMessage => isEditedMessage(message) && message.t === 'rm';
export const isMessageFromMatrixFederation = (message: IMessage): boolean =>
'federation' in message && Boolean(message.federation?.eventId);
Expand Down
Loading