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

feat: Improve Engagement Dashboard's "Channels" tab performance #32493

Merged
merged 33 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
d6bf987
feat: Improve engagement-dashboard/channels/list endpoint performance
matheusbsilva137 May 24, 2024
af24241
Create changeset
matheusbsilva137 May 24, 2024
7c2fc0b
Apply suggestions from code review
matheusbsilva137 May 24, 2024
a50744a
Apply suggestions from code review
KevLehman May 24, 2024
6a633fb
test: Add end-to-end tests
matheusbsilva137 May 27, 2024
0d05e78
fix typecheck
matheusbsilva137 May 27, 2024
01a5144
test: fix default voip direction
matheusbsilva137 May 27, 2024
309ac03
improve: Add room type check to aggregation pipeline
matheusbsilva137 May 29, 2024
96effb6
Merge branch 'develop' into feat/improve-engagement-dashboard-chanels
debdutdeb Jun 5, 2024
015775c
improve: Added hideRoomsWithNoActivity param
matheusbsilva137 Jun 6, 2024
2786a7f
Merge branch 'feat/improve-engagement-dashboard-chanels' of https://g…
matheusbsilva137 Jun 6, 2024
45653ba
Update changeset
matheusbsilva137 Jun 6, 2024
1142b9d
Create changeset
matheusbsilva137 Jun 6, 2024
3e13306
fix end-to-end test
matheusbsilva137 Jun 6, 2024
e1fbce2
Merge branch 'feat/improve-engagement-dashboard-chanels' of https://g…
matheusbsilva137 Jun 6, 2024
c74186a
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jun 6, 2024
3ba5355
deprecate new hideRoomsWithNoActivity param
matheusbsilva137 Jun 10, 2024
450ec1a
Update apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts
matheusbsilva137 Jun 11, 2024
787d0ff
Improve indexes
matheusbsilva137 Jun 11, 2024
d832107
Remove index fix (to be added in another PR)
matheusbsilva137 Jun 12, 2024
68d6774
improve deprecation message
matheusbsilva137 Jun 17, 2024
60b8c4e
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jun 17, 2024
dfd1c57
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jun 18, 2024
28ff4d9
only display the deprecation message when param is sent
matheusbsilva137 Jun 21, 2024
c405e02
Always display deprecation message
matheusbsilva137 Jul 16, 2024
752265a
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jul 16, 2024
29f8118
update imports (converted to TS)
matheusbsilva137 Jul 16, 2024
5b5e9dd
fix lint
matheusbsilva137 Jul 16, 2024
7aea864
fix: handle case for when there are no messages analytics in the prov…
matheusbsilva137 Jul 18, 2024
ea33446
replace count by findOne
matheusbsilva137 Jul 18, 2024
5cebc20
fix: do not extract data if aggregation returns an empty result
matheusbsilva137 Jul 18, 2024
fc7fbfb
Merge remote-tracking branch 'origin/develop' into feat/improve-engag…
sampaiodiego Jul 19, 2024
f3ca6f0
chore: show deprecation only without param
sampaiodiego Jul 19, 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/proud-waves-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": minor
"@rocket.chat/model-typings": minor
---

Fixed Engagement Dashboard's "Channels" tab not loading when there are many rooms in the workspace
10 changes: 1 addition & 9 deletions apps/meteor/ee/server/lib/engagementDashboard/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,7 @@ export const findAllChannelsWithNumberOfMessages = async ({
options,
}).toArray();

const total =
(
await Rooms.countChannelsWithNumberOfMessagesBetweenDate({
start: convertDateToInt(start),
end: convertDateToInt(end),
startOfLastWeek: convertDateToInt(startOfLastWeek),
endOfLastWeek: convertDateToInt(endOfLastWeek),
}).toArray()
)[0]?.total ?? 0;
const total = await Rooms.countDocuments({});
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

return {
channels,
Expand Down
41 changes: 13 additions & 28 deletions apps/meteor/server/models/raw/Rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,24 +494,25 @@ export class RoomsRaw extends BaseRaw<IRoom> implements IRoomsModel {
diffFromLastWeek: { $subtract: ['$messages', '$lastWeekMessages'] },
},
};
const firstParams = [
lookup,
messagesProject,
messagesUnwind,
messagesGroup,
lastWeekMessagesUnwind,
lastWeekMessagesGroup,
presentationProject,
];
const firstParams = [lookup, messagesProject, messagesUnwind, messagesGroup];
const lastParams = [lastWeekMessagesUnwind, lastWeekMessagesGroup, presentationProject];

const sort = { $sort: options?.sort || { messages: -1 } };
const params: Exclude<Parameters<Collection<IRoom>['aggregate']>[0], undefined> = [...firstParams, sort];
const sortAndPaginationParams: Exclude<Parameters<Collection<IRoom>['aggregate']>[0], undefined> = [sort];

if (options?.offset) {
params.push({ $skip: options.offset });
sortAndPaginationParams.push({ $skip: options.offset });
}

if (options?.count) {
params.push({ $limit: options.count });
sortAndPaginationParams.push({ $limit: options.count });
}
const params: Exclude<Parameters<Collection<IRoom>['aggregate']>[0], undefined> = [...firstParams];

if (options?.sort) {
params.push(...lastParams, ...sortAndPaginationParams);
} else {
params.push(...sortAndPaginationParams, ...lastParams, sort);
}

return params;
Expand All @@ -531,22 +532,6 @@ export class RoomsRaw extends BaseRaw<IRoom> implements IRoomsModel {
});
}

countChannelsWithNumberOfMessagesBetweenDate(params: {
start: number;
end: number;
startOfLastWeek: number;
endOfLastWeek: number;
options?: any;
}): AggregationCursor<{ total: number }> {
const aggregationParams = this.getChannelsWithNumberOfMessagesBetweenDateQuery(params);
aggregationParams.push({ $count: 'total' });

return this.col.aggregate<{ total: number }>(aggregationParams, {
allowDiskUse: true,
readPreference: readSecondaryPreferred(),
});
}

matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
findOneByNameOrFname(name: NonNullable<IRoom['name'] | IRoom['fname']>, options: FindOptions<IRoom> = {}): Promise<IRoom | null> {
const query = {
$or: [
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/tests/data/chat.helper.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { api, credentials, request } from './api-data';

export const sendSimpleMessage = ({ roomId, text = 'test message', tmid }) => {
export const sendSimpleMessage = ({ roomId, text = 'test message', tmid = undefined }) => {
if (!roomId) {
throw new Error('"roomId" is required in "sendSimpleMessage" test helper');
}
Expand Down
16 changes: 8 additions & 8 deletions apps/meteor/tests/data/rooms.helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import { resolve } from 'path';
import { api, credentials, request } from './api-data';

export const createRoom = ({
name,
name = '',
type,
username,
token,
agentId,
members,
credentials: customCredentials,
extraData,
username = '',
token = '',
agentId = '',
members = [],
credentials: customCredentials = undefined,
extraData = undefined,
voipCallDirection = 'inbound',
}) => {
if (!type) {
Expand Down Expand Up @@ -41,7 +41,7 @@ export const createRoom = ({
.set(customCredentials || credentials)
.send({
...params,
...(members && { members }),
...(members && members.length && { members }),
...(extraData && { extraData }),
});
};
Expand Down
4 changes: 0 additions & 4 deletions apps/meteor/tests/data/uploads.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ export async function testFileUploads(filesEndpoint: 'channels.files' | 'groups.
type: 'v',
agentId: testUser._id,
credentials: testUserCredentials,
name: null,
username: null,
members: null,
extraData: null,
});

return roomResponse.body.room;
Expand Down
255 changes: 255 additions & 0 deletions apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
import type { IRoom } from '@rocket.chat/core-typings';
import { expect } from 'chai';
import { after, before, describe, it } from 'mocha';
import type { Response } from 'supertest';

import { getCredentials, api, request, credentials } from '../../data/api-data.js';
import { sendSimpleMessage } from '../../data/chat.helper.js';
import { updatePermission } from '../../data/permissions.helper';
import { createRoom, deleteRoom } from '../../data/rooms.helper';

describe('[Engagement Dashboard]', function () {
this.retries(0);

const isEnterprise = Boolean(process.env.IS_EE);

before((done) => getCredentials(done));

before(() => updatePermission('view-engagement-dashboard', ['admin']));

before(() => updatePermission('view-engagement-dashboard', ['admin']));
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved

(isEnterprise ? describe : describe.skip)('[/engagement-dashboard/channels/list]', () => {
let testRoom: IRoom;

before(async () => {
testRoom = (await createRoom({ type: 'c', name: `channel.test.engagement.${Date.now()}-${Math.random()}` })).body.channel;
});

after(async () => {
await deleteRoom({ type: 'c', roomId: testRoom._id });
});

it('should fail if user does not have the view-engagement-dashboard permission', async () => {
await updatePermission('view-engagement-dashboard', []);
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date().toISOString(),
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
offset: 0,
count: 25,
})
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res: Response) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal('User does not have the permissions required for this action [error-unauthorized]');
});
});

it('should fail if start param is not a valid date', async () => {
await updatePermission('view-engagement-dashboard', ['admin']);
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
start: 'invalid-date',
end: new Date().toISOString(),
offset: 0,
count: 25,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res: Response) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal('Match error: Failed Match.Where validation in field start');
});
});

it('should fail if end param is not a valid date', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
end: 'invalid-date',
offset: 0,
count: 25,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res: Response) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal('Match error: Failed Match.Where validation in field end');
});
});

it('should fail if start param is not provided', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date(),
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res: Response) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal("Match error: Missing key 'start'");
});
});

it('should fail if end param is not provided', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res: Response) => {
expect(res.body).to.have.property('success', false);
expect(res.body.error).to.be.equal("Match error: Missing key 'end'");
});
});

it('should succesfuly return results', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date().toISOString(),
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res: Response) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('offset', 0);
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
expect(res.body).to.have.property('channels');
expect(res.body.channels).to.be.an('array').that.is.not.empty;

expect(res.body.channels[0]).to.be.an('object').that.is.not.empty;
expect(res.body.channels[0]).to.have.property('messages').that.is.a('number');
expect(res.body.channels[0]).to.have.property('lastWeekMessages').that.is.a('number');
expect(res.body.channels[0]).to.have.property('diffFromLastWeek').that.is.a('number');
expect(res.body.channels[0].room).to.be.an('object').that.is.not.empty;

expect(res.body.channels[0].room).to.have.property('_id').that.is.a('string');
expect(res.body.channels[0].room).to.have.property('name').that.is.a('string');
expect(res.body.channels[0].room).to.have.property('ts').that.is.a('string');
expect(res.body.channels[0].room).to.have.property('t').that.is.a('string');
expect(res.body.channels[0].room).to.have.property('_updatedAt').that.is.a('string');
});
});

it('should correctly count messages in an empty room', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date().toISOString(),
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res: Response) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('offset', 0);
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
expect(res.body).to.have.property('channels');
expect(res.body.channels).to.be.an('array').that.is.not.empty;

const channelRecord = res.body.channels.find(({ room }: { room: { _id: string } }) => room._id === testRoom._id);
expect(channelRecord).not.to.be.undefined;

expect(channelRecord).to.be.an('object').that.is.not.empty;
expect(channelRecord).to.have.property('messages', 0);
expect(channelRecord).to.have.property('lastWeekMessages', 0);
expect(channelRecord).to.have.property('diffFromLastWeek', 0);
expect(channelRecord.room).to.be.an('object').that.is.not.empty;

expect(channelRecord.room).to.have.property('_id', testRoom._id);
expect(channelRecord.room).to.have.property('name', testRoom.name);
expect(channelRecord.room).to.have.property('ts', testRoom.ts);
expect(channelRecord.room).to.have.property('t', testRoom.t);
expect(channelRecord.room).to.have.property('_updatedAt', testRoom._updatedAt);
});
});

it('should correctly count messages diff compared to last week when there are messages in a room', async () => {
await sendSimpleMessage({ roomId: testRoom._id });
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date().toISOString(),
start: new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString(),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res: Response) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('offset', 0);
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
expect(res.body).to.have.property('channels');
expect(res.body.channels).to.be.an('array').that.is.not.empty;

const channelRecord = res.body.channels.find(({ room }: { room: { _id: string } }) => room._id === testRoom._id);
expect(channelRecord).not.to.be.undefined;

expect(channelRecord).to.be.an('object').that.is.not.empty;
expect(channelRecord).to.have.property('messages', 1);
expect(channelRecord).to.have.property('lastWeekMessages', 0);
expect(channelRecord).to.have.property('diffFromLastWeek', 1);
expect(channelRecord.room).to.be.an('object').that.is.not.empty;

expect(channelRecord.room).to.have.property('_id', testRoom._id);
expect(channelRecord.room).to.have.property('name', testRoom.name);
expect(channelRecord.room).to.have.property('ts', testRoom.ts);
expect(channelRecord.room).to.have.property('t', testRoom.t);
});
});

it('should correctly count messages from last week and diff when moving to the next week', async () => {
await request
.get(api('engagement-dashboard/channels/list'))
.set(credentials)
.query({
end: new Date(Date.now() + 8 * 24 * 60 * 60 * 1000).toISOString(),
start: new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(),
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res: Response) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('offset', 0);
expect(res.body).to.have.property('count');
expect(res.body).to.have.property('total');
expect(res.body).to.have.property('channels');
expect(res.body.channels).to.be.an('array').that.is.not.empty;

const channelRecord = res.body.channels.find(({ room }: { room: { _id: string } }) => room._id === testRoom._id);
expect(channelRecord).not.to.be.undefined;

expect(channelRecord).to.be.an('object').that.is.not.empty;
expect(channelRecord).to.have.property('messages', 0);
expect(channelRecord).to.have.property('lastWeekMessages', 1);
expect(channelRecord).to.have.property('diffFromLastWeek', -1);
expect(channelRecord.room).to.be.an('object').that.is.not.empty;

expect(channelRecord.room).to.have.property('_id', testRoom._id);
expect(channelRecord.room).to.have.property('name', testRoom.name);
expect(channelRecord.room).to.have.property('ts', testRoom.ts);
expect(channelRecord.room).to.have.property('t', testRoom.t);
});
});
});
});
Loading
Loading