From d6bf98742f545957dc64e50d6a3924324b972ccd Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 24 May 2024 12:54:25 -0300 Subject: [PATCH 01/25] feat: Improve engagement-dashboard/channels/list endpoint performance --- .../lib/engagementDashboard/channels.ts | 10 +---- apps/meteor/server/models/raw/Rooms.ts | 45 +++++++------------ .../model-typings/src/models/IRoomsModel.ts | 9 +--- 3 files changed, 19 insertions(+), 45 deletions(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 834284ebb9b7..2f3b94b13d61 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -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.countTotal(); return { channels, diff --git a/apps/meteor/server/models/raw/Rooms.ts b/apps/meteor/server/models/raw/Rooms.ts index 64bde1c32924..8515ce758044 100644 --- a/apps/meteor/server/models/raw/Rooms.ts +++ b/apps/meteor/server/models/raw/Rooms.ts @@ -494,24 +494,25 @@ export class RoomsRaw extends BaseRaw 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['aggregate']>[0], undefined> = [...firstParams, sort]; + const sortAndPaginationParams: Exclude['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['aggregate']>[0], undefined> = [...firstParams]; + + if (options?.sort) { + params.push(...lastParams, ...sortAndPaginationParams); + } else { + params.push(...sortAndPaginationParams, ...lastParams, sort); } return params; @@ -531,22 +532,6 @@ export class RoomsRaw extends BaseRaw 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(), - }); - } - findOneByNameOrFname(name: NonNullable, options: FindOptions = {}): Promise { const query = { $or: [ @@ -1978,4 +1963,8 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.updateOne(query, update); } + + countTotal(): Promise { + return this.col.countDocuments(); + } } diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index 9bc4fc35a9dc..33966f96355e 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -94,14 +94,6 @@ export interface IRoomsModel extends IBaseModel { options?: any; }): AggregationCursor; - countChannelsWithNumberOfMessagesBetweenDate(params: { - start: number; - end: number; - startOfLastWeek: number; - endOfLastWeek: number; - options?: any; - }): AggregationCursor<{ total: number }>; - findOneByName(name: NonNullable, options?: FindOptions): Promise; findDefaultRoomsForTeam(teamId: any): FindCursor; @@ -278,4 +270,5 @@ export interface IRoomsModel extends IBaseModel { removeDirectRoomContainingUsername(username: string): Promise; countDiscussions(): Promise; setOTRForDMByRoomID(rid: string): Promise; + countTotal(): Promise; } From af24241537c3a83797e9647fdace5038f7b0da93 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 24 May 2024 13:15:17 -0300 Subject: [PATCH 02/25] Create changeset --- .changeset/proud-waves-bathe.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/proud-waves-bathe.md diff --git a/.changeset/proud-waves-bathe.md b/.changeset/proud-waves-bathe.md new file mode 100644 index 000000000000..c77887a3ad1f --- /dev/null +++ b/.changeset/proud-waves-bathe.md @@ -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 From 7c2fc0bfea90beb7066d02dc7fe392596a3f0c80 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Fri, 24 May 2024 16:58:33 -0300 Subject: [PATCH 03/25] Apply suggestions from code review Co-authored-by: Kevin Aleman --- apps/meteor/ee/server/lib/engagementDashboard/channels.ts | 2 +- apps/meteor/server/models/raw/Rooms.ts | 4 ---- packages/model-typings/src/models/IRoomsModel.ts | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 2f3b94b13d61..312dc0f34a39 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -43,7 +43,7 @@ export const findAllChannelsWithNumberOfMessages = async ({ options, }).toArray(); - const total = await Rooms.countTotal(); + const total = await Rooms.countDocuments(); return { channels, diff --git a/apps/meteor/server/models/raw/Rooms.ts b/apps/meteor/server/models/raw/Rooms.ts index 8515ce758044..eaad105f2993 100644 --- a/apps/meteor/server/models/raw/Rooms.ts +++ b/apps/meteor/server/models/raw/Rooms.ts @@ -1963,8 +1963,4 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.updateOne(query, update); } - - countTotal(): Promise { - return this.col.countDocuments(); - } } diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index 33966f96355e..8d7ccf4918e1 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -270,5 +270,4 @@ export interface IRoomsModel extends IBaseModel { removeDirectRoomContainingUsername(username: string): Promise; countDiscussions(): Promise; setOTRForDMByRoomID(rid: string): Promise; - countTotal(): Promise; } From a50744a77829e6b53b1e973c641d59b3f5e965f7 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Fri, 24 May 2024 14:59:02 -0600 Subject: [PATCH 04/25] Apply suggestions from code review oops --- apps/meteor/ee/server/lib/engagementDashboard/channels.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 312dc0f34a39..e32df099ebb4 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -43,7 +43,7 @@ export const findAllChannelsWithNumberOfMessages = async ({ options, }).toArray(); - const total = await Rooms.countDocuments(); + const total = await Rooms.countDocuments({}); return { channels, From 6a633fb02e418bf938d3950d3ab29e2b7a6c3a1f Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 27 May 2024 15:49:53 -0300 Subject: [PATCH 05/25] test: Add end-to-end tests --- apps/meteor/tests/data/chat.helper.js | 2 +- apps/meteor/tests/data/rooms.helper.js | 12 +- .../end-to-end/api/34-engagement-dashboard.ts | 255 ++++++++++++++++++ 3 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts diff --git a/apps/meteor/tests/data/chat.helper.js b/apps/meteor/tests/data/chat.helper.js index 6e3a2d05a0cd..01da575b29a1 100644 --- a/apps/meteor/tests/data/chat.helper.js +++ b/apps/meteor/tests/data/chat.helper.js @@ -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'); } diff --git a/apps/meteor/tests/data/rooms.helper.js b/apps/meteor/tests/data/rooms.helper.js index 6e74509deab4..2fe988268b42 100644 --- a/apps/meteor/tests/data/rooms.helper.js +++ b/apps/meteor/tests/data/rooms.helper.js @@ -4,12 +4,12 @@ import { api, credentials, request } from './api-data'; export const createRoom = ({ name, type, - username, - token, - agentId, - members, - credentials: customCredentials, - extraData, + username = undefined, + token = undefined, + agentId = undefined, + members = undefined, + credentials: customCredentials = undefined, + extraData = undefined, voipCallDirection = 'inbound', }) => { if (!type) { diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts new file mode 100644 index 000000000000..763f68c6c8b0 --- /dev/null +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -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'])); + + (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); + }); + }); + }); +}); From 0d05e78256634e810a1e4019255b34c58ce91fc3 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 27 May 2024 16:32:31 -0300 Subject: [PATCH 06/25] fix typecheck --- apps/meteor/tests/data/rooms.helper.js | 14 +++++++------- apps/meteor/tests/data/uploads.helper.ts | 4 ---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/apps/meteor/tests/data/rooms.helper.js b/apps/meteor/tests/data/rooms.helper.js index 2fe988268b42..8c83495a3fae 100644 --- a/apps/meteor/tests/data/rooms.helper.js +++ b/apps/meteor/tests/data/rooms.helper.js @@ -2,15 +2,15 @@ import { resolve } from 'path'; import { api, credentials, request } from './api-data'; export const createRoom = ({ - name, + name = '', type, - username = undefined, - token = undefined, - agentId = undefined, - members = undefined, + username = '', + token = '', + agentId = '', + members = [], credentials: customCredentials = undefined, extraData = undefined, - voipCallDirection = 'inbound', + voipCallDirection = '', }) => { if (!type) { throw new Error('"type" is required in "createRoom.ts" test helper'); @@ -41,7 +41,7 @@ export const createRoom = ({ .set(customCredentials || credentials) .send({ ...params, - ...(members && { members }), + ...(members && members.length && { members }), ...(extraData && { extraData }), }); }; diff --git a/apps/meteor/tests/data/uploads.helper.ts b/apps/meteor/tests/data/uploads.helper.ts index eabd49e11a3b..4c1aad9b7451 100644 --- a/apps/meteor/tests/data/uploads.helper.ts +++ b/apps/meteor/tests/data/uploads.helper.ts @@ -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; From 01a51447c956c5ea52a75820a45563ef70f2c0f2 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 27 May 2024 17:09:33 -0300 Subject: [PATCH 07/25] test: fix default voip direction --- apps/meteor/tests/data/rooms.helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/rooms.helper.js b/apps/meteor/tests/data/rooms.helper.js index 8c83495a3fae..d768cbfa57fa 100644 --- a/apps/meteor/tests/data/rooms.helper.js +++ b/apps/meteor/tests/data/rooms.helper.js @@ -10,7 +10,7 @@ export const createRoom = ({ members = [], credentials: customCredentials = undefined, extraData = undefined, - voipCallDirection = '', + voipCallDirection = 'inbound', }) => { if (!type) { throw new Error('"type" is required in "createRoom.ts" test helper'); From 309ac035c049a6e9f5784fe30aaa6225914b701e Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 29 May 2024 14:01:13 -0300 Subject: [PATCH 08/25] improve: Add room type check to aggregation pipeline --- .../ee/server/lib/engagementDashboard/channels.ts | 7 +++++-- apps/meteor/server/models/raw/Rooms.ts | 12 ++++++++++-- packages/model-typings/src/models/IRoomsModel.ts | 3 ++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index e32df099ebb4..2e5bbd34d07d 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -2,6 +2,7 @@ import type { IDirectMessageRoom, IRoom } from '@rocket.chat/core-typings'; import { Rooms } from '@rocket.chat/models'; import moment from 'moment'; +import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator'; import { convertDateToInt, diffBetweenDaysInclusive } from './date'; export const findAllChannelsWithNumberOfMessages = async ({ @@ -34,8 +35,10 @@ export const findAllChannelsWithNumberOfMessages = async ({ const daysBetweenDates = diffBetweenDaysInclusive(end, start); const endOfLastWeek = moment(start).subtract(1, 'days').toDate(); const startOfLastWeek = moment(endOfLastWeek).subtract(daysBetweenDates, 'days').toDate(); + const roomTypes = roomCoordinator.getTypesToShowOnDashboard() as Array; - const channels = await Rooms.findChannelsWithNumberOfMessagesBetweenDate({ + const channels = await Rooms.findChannelsByTypesWithNumberOfMessagesBetweenDate({ + types: roomTypes, start: convertDateToInt(start), end: convertDateToInt(end), startOfLastWeek: convertDateToInt(startOfLastWeek), @@ -43,7 +46,7 @@ export const findAllChannelsWithNumberOfMessages = async ({ options, }).toArray(); - const total = await Rooms.countDocuments({}); + const total = await Rooms.countDocuments({ t: { $in: roomTypes } }); return { channels, diff --git a/apps/meteor/server/models/raw/Rooms.ts b/apps/meteor/server/models/raw/Rooms.ts index eaad105f2993..5ebaba4c6dc8 100644 --- a/apps/meteor/server/models/raw/Rooms.ts +++ b/apps/meteor/server/models/raw/Rooms.ts @@ -403,18 +403,25 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { } getChannelsWithNumberOfMessagesBetweenDateQuery({ + types, start, end, startOfLastWeek, endOfLastWeek, options, }: { + types: Array; start: number; end: number; startOfLastWeek: number; endOfLastWeek: number; options?: any; }) { + const typeMatch = { + $match: { + t: { $in: types }, + }, + }; const lookup = { $lookup: { from: 'rocketchat_analytics', @@ -494,7 +501,7 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { diffFromLastWeek: { $subtract: ['$messages', '$lastWeekMessages'] }, }, }; - const firstParams = [lookup, messagesProject, messagesUnwind, messagesGroup]; + const firstParams = [typeMatch, lookup, messagesProject, messagesUnwind, messagesGroup]; const lastParams = [lastWeekMessagesUnwind, lastWeekMessagesGroup, presentationProject]; const sort = { $sort: options?.sort || { messages: -1 } }; @@ -518,7 +525,8 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return params; } - findChannelsWithNumberOfMessagesBetweenDate(params: { + findChannelsByTypesWithNumberOfMessagesBetweenDate(params: { + types: Array; start: number; end: number; startOfLastWeek: number; diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index 8d7ccf4918e1..7e1a33557a06 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -86,7 +86,8 @@ export interface IRoomsModel extends IBaseModel { setTeamDefaultById(rid: IRoom['_id'], teamDefault: NonNullable, options?: UpdateOptions): Promise; - findChannelsWithNumberOfMessagesBetweenDate(params: { + findChannelsByTypesWithNumberOfMessagesBetweenDate(params: { + types: Array; start: number; end: number; startOfLastWeek: number; From 015775c3db64dd0fc3cd92d4d4f756e9d2367766 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 6 Jun 2024 17:58:35 -0300 Subject: [PATCH 09/25] improve: Added hideRoomsWithNoActivity param --- .../channels/useChannelsList.ts | 1 + .../api/engagementDashboard/channels.ts | 10 +- .../lib/engagementDashboard/channels.ts | 55 +++++++- apps/meteor/server/models/raw/Analytics.ts | 117 +++++++++++++++++- .../end-to-end/api/34-engagement-dashboard.ts | 93 +++++++++++++- .../src/models/IAnalyticsModel.ts | 9 ++ 6 files changed, 277 insertions(+), 8 deletions(-) diff --git a/apps/meteor/client/views/admin/engagementDashboard/channels/useChannelsList.ts b/apps/meteor/client/views/admin/engagementDashboard/channels/useChannelsList.ts index cd1a338eeab5..f45c620c8b49 100644 --- a/apps/meteor/client/views/admin/engagementDashboard/channels/useChannelsList.ts +++ b/apps/meteor/client/views/admin/engagementDashboard/channels/useChannelsList.ts @@ -24,6 +24,7 @@ export const useChannelsList = ({ period, offset, count }: UseChannelsListOption end: end.toISOString(), offset, count, + hideRoomsWithNoActivity: true, }); return response diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index b2a655f4a843..1b3d44db248d 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -3,14 +3,14 @@ import { check, Match } from 'meteor/check'; import { API } from '../../../../app/api/server'; import { getPaginationItems } from '../../../../app/api/server/helpers/getPaginationItems'; -import { findAllChannelsWithNumberOfMessages } from '../../lib/engagementDashboard/channels'; +import { findChannelsWithNumberOfMessages } from '../../lib/engagementDashboard/channels'; import { isDateISOString, mapDateForAPI } from '../../lib/engagementDashboard/date'; declare module '@rocket.chat/rest-typings' { // eslint-disable-next-line @typescript-eslint/naming-convention interface Endpoints { '/v1/engagement-dashboard/channels/list': { - GET: (params: { start: string; end: string; offset?: number; count?: number }) => { + GET: (params: { start: string; end: string; offset?: number; count?: number; hideRoomsWithNoActivity?: boolean }) => { channels: { room: { _id: IRoom['_id']; @@ -45,17 +45,19 @@ API.v1.addRoute( Match.ObjectIncluding({ start: Match.Where(isDateISOString), end: Match.Where(isDateISOString), + hideRoomsWithNoActivity: Match.Maybe(String), offset: Match.Maybe(String), count: Match.Maybe(String), }), ); - const { start, end } = this.queryParams; + const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); - const { channels, total } = await findAllChannelsWithNumberOfMessages({ + const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start), end: mapDateForAPI(end), + hideRoomsWithNoActivity: hideRoomsWithNoActivity === 'true' ?? false, options: { offset, count }, }); diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 2e5bbd34d07d..9cb7e6ee8f04 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -1,10 +1,63 @@ import type { IDirectMessageRoom, IRoom } from '@rocket.chat/core-typings'; -import { Rooms } from '@rocket.chat/models'; +import { Analytics, Rooms } from '@rocket.chat/models'; import moment from 'moment'; import { roomCoordinator } from '../../../../server/lib/rooms/roomCoordinator'; import { convertDateToInt, diffBetweenDaysInclusive } from './date'; +export const findChannelsWithNumberOfMessages = async ({ + start, + end, + hideRoomsWithNoActivity, + options = {}, +}: { + start: Date; + end: Date; + hideRoomsWithNoActivity: boolean; + options: { + offset?: number; + count?: number; + }; +}): Promise<{ + channels: { + room: { + _id: IRoom['_id']; + name: IRoom['name'] | IRoom['fname']; + ts: IRoom['ts']; + t: IRoom['t']; + _updatedAt: IRoom['_updatedAt']; + usernames?: IDirectMessageRoom['usernames']; + }; + messages: number; + lastWeekMessages: number; + diffFromLastWeek: number; + }[]; + total: number; +}> => { + if (!hideRoomsWithNoActivity) { + return findAllChannelsWithNumberOfMessages({ start, end, options }); + } + + const daysBetweenDates = diffBetweenDaysInclusive(end, start); + const endOfLastWeek = moment(start).subtract(1, 'days').toDate(); + const startOfLastWeek = moment(endOfLastWeek).subtract(daysBetweenDates, 'days').toDate(); + const roomTypes = roomCoordinator.getTypesToShowOnDashboard() as Array; + + const [{ channels, total }] = await Analytics.findRoomsByTypesWithNumberOfMessagesBetweenDate({ + types: roomTypes, + start: convertDateToInt(start), + end: convertDateToInt(end), + startOfLastWeek: convertDateToInt(startOfLastWeek), + endOfLastWeek: convertDateToInt(endOfLastWeek), + options, + }).toArray(); + + return { + channels, + total, + }; +}; + export const findAllChannelsWithNumberOfMessages = async ({ start, end, diff --git a/apps/meteor/server/models/raw/Analytics.ts b/apps/meteor/server/models/raw/Analytics.ts index 4e95cadebbcd..cfb237dab0e4 100644 --- a/apps/meteor/server/models/raw/Analytics.ts +++ b/apps/meteor/server/models/raw/Analytics.ts @@ -1,7 +1,7 @@ import type { IAnalytic, IRoom } from '@rocket.chat/core-typings'; -import type { IAnalyticsModel } from '@rocket.chat/model-typings'; +import type { IAnalyticsModel, IChannelsWithNumberOfMessagesBetweenDate } from '@rocket.chat/model-typings'; import { Random } from '@rocket.chat/random'; -import type { AggregationCursor, FindCursor, Db, IndexDescription, FindOptions, UpdateResult, Document } from 'mongodb'; +import type { AggregationCursor, FindCursor, Db, IndexDescription, FindOptions, UpdateResult, Document, Collection } from 'mongodb'; import { readSecondaryPreferred } from '../../database/readSecondaryPreferred'; import { BaseRaw } from './BaseRaw'; @@ -211,4 +211,117 @@ export class AnalyticsRaw extends BaseRaw implements IAnalyticsModel findByTypeBeforeDate({ type, date }: { type: IAnalytic['type']; date: IAnalytic['date'] }): FindCursor { return this.find({ type, date: { $lte: date } }); } + + getRoomsWithNumberOfMessagesBetweenDateQuery({ + types, + start, + end, + startOfLastWeek, + endOfLastWeek, + options, + }: { + types: Array; + start: number; + end: number; + startOfLastWeek: number; + endOfLastWeek: number; + options?: any; + }) { + const typeAndDateMatch = { + $match: { + 'type': 'messages', + 'room.t': { $in: types }, + 'date': { $gte: startOfLastWeek, $lte: end }, + }, + }; + const roomsGroup = { + $group: { + _id: '$room._id', + room: { $first: '$room' }, + messages: { $sum: { $cond: [{ $gte: ['$date', start] }, '$messages', 0] } }, + lastWeekMessages: { $sum: { $cond: [{ $lte: ['$date', endOfLastWeek] }, '$messages', 0] } }, + }, + }; + const lookup = { + $lookup: { + from: 'rocketchat_room', + localField: '_id', + foreignField: '_id', + as: 'room', + }, + }; + const roomsUnwind = { + $unwind: { + path: '$room', + preserveNullAndEmptyArrays: false, + }, + }; + const project = { + $project: { + _id: 0, + room: { + _id: '$room._id', + name: { $ifNull: ['$room.name', '$room.fname'] }, + ts: '$room.ts', + t: '$room.t', + _updatedAt: '$room._updatedAt', + usernames: '$room.usernames', + }, + messages: '$messages', + lastWeekMessages: '$lastWeekMessages', + diffFromLastWeek: { $subtract: ['$messages', '$lastWeekMessages'] }, + }, + }; + + const sort = { $sort: options?.sort || { messages: -1 } }; + const sortAndPaginationParams: Exclude['aggregate']>[0], undefined> = [sort]; + if (options?.offset) { + sortAndPaginationParams.push({ $skip: options.offset }); + } + + if (options?.count) { + sortAndPaginationParams.push({ $limit: options.count }); + } + const facet = { + $facet: { + channels: [...sortAndPaginationParams], + total: [{ $count: 'total' }], + }, + }; + const totalUnwind = { $unwind: '$total' }; + const totalProject = { + $project: { + channels: '$channels', + total: '$total.total', + }, + }; + + const params: Exclude['aggregate']>[0], undefined> = [ + typeAndDateMatch, + roomsGroup, + lookup, + roomsUnwind, + project, + facet, + totalUnwind, + totalProject, + ]; + + return params; + } + + findRoomsByTypesWithNumberOfMessagesBetweenDate(params: { + types: Array; + start: number; + end: number; + startOfLastWeek: number; + endOfLastWeek: number; + options?: any; + }): AggregationCursor<{ channels: IChannelsWithNumberOfMessagesBetweenDate[]; total: number }> { + const aggregationParams = this.getRoomsWithNumberOfMessagesBetweenDateQuery(params); + return this.col.aggregate<{ channels: IChannelsWithNumberOfMessagesBetweenDate[]; total: number }>(aggregationParams, { + allowDiskUse: true, + readPreference: readSecondaryPreferred(), + }); + } } diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts index 763f68c6c8b0..7a72e93e245e 100644 --- a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -148,6 +148,27 @@ describe('[Engagement Dashboard]', function () { }); }); + it('should not return empty rooms when the hideRoomsWithNoActivity param is provided', 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(), + hideRoomsWithNoActivity: true, + }) + .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.empty; + }); + }); + it('should correctly count messages in an empty room', async () => { await request .get(api('engagement-dashboard/channels/list')) @@ -183,8 +204,43 @@ describe('[Engagement Dashboard]', function () { }); }); - it('should correctly count messages diff compared to last week when there are messages in a room', async () => { + it('should correctly count messages diff compared to last week when the hideRoomsWithNoActivity param is provided and 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(), + hideRoomsWithNoActivity: true, + }) + .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 diff compared to last week when there are messages in a room', async () => { await request .get(api('engagement-dashboard/channels/list')) .set(credentials) @@ -218,6 +274,41 @@ describe('[Engagement Dashboard]', function () { }); }); + it('should correctly count messages from last week and diff when moving to the next week and providing the hideRoomsWithNoActivity param', 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(), + hideRoomsWithNoActivity: true, + }) + .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); + }); + }); + 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')) diff --git a/packages/model-typings/src/models/IAnalyticsModel.ts b/packages/model-typings/src/models/IAnalyticsModel.ts index 4d31f66a94d8..1ca4d7abf118 100644 --- a/packages/model-typings/src/models/IAnalyticsModel.ts +++ b/packages/model-typings/src/models/IAnalyticsModel.ts @@ -2,6 +2,7 @@ import type { IAnalytic, IRoom } from '@rocket.chat/core-typings'; import type { AggregationCursor, FindCursor, FindOptions, UpdateResult, Document } from 'mongodb'; import type { IBaseModel } from './IBaseModel'; +import type { IChannelsWithNumberOfMessagesBetweenDate } from './IRoomsModel'; export interface IAnalyticsModel extends IBaseModel { saveMessageSent({ room, date }: { room: IRoom; date: IAnalytic['date'] }): Promise; @@ -38,4 +39,12 @@ export interface IAnalyticsModel extends IBaseModel { users: number; }>; findByTypeBeforeDate({ type, date }: { type: IAnalytic['type']; date: IAnalytic['date'] }): FindCursor; + findRoomsByTypesWithNumberOfMessagesBetweenDate(params: { + types: Array; + start: number; + end: number; + startOfLastWeek: number; + endOfLastWeek: number; + options?: any; + }): AggregationCursor<{ channels: IChannelsWithNumberOfMessagesBetweenDate[]; total: number }>; } From 45653ba54f7638d83a7d05ec328080beef297dea Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Thu, 6 Jun 2024 18:14:36 -0300 Subject: [PATCH 10/25] Update changeset --- .changeset/proud-waves-bathe.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/proud-waves-bathe.md b/.changeset/proud-waves-bathe.md index c77887a3ad1f..556fa3af80e1 100644 --- a/.changeset/proud-waves-bathe.md +++ b/.changeset/proud-waves-bathe.md @@ -3,4 +3,4 @@ "@rocket.chat/model-typings": minor --- -Fixed Engagement Dashboard's "Channels" tab not loading when there are many rooms in the workspace +Improved Engagement Dashboard's "Channels" tab performance by not returning rooms that had no activity in the analyzed period From 1142b9d9312848b82c0c3072be192248e73e55e7 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Thu, 6 Jun 2024 18:17:44 -0300 Subject: [PATCH 11/25] Create changeset --- .changeset/many-tables-love.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/many-tables-love.md diff --git a/.changeset/many-tables-love.md b/.changeset/many-tables-love.md new file mode 100644 index 000000000000..8f37283c6a96 --- /dev/null +++ b/.changeset/many-tables-love.md @@ -0,0 +1,6 @@ +--- +"@rocket.chat/meteor": minor +"@rocket.chat/model-typings": minor +--- + +Fixed Livechat rooms being displayed in the Engagement Dashboard's "Channels" tab From 3e13306d1df7bb4685b2ca5b3ab8a682cd9ce461 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 6 Jun 2024 19:04:07 -0300 Subject: [PATCH 12/25] fix end-to-end test --- apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts index 7a72e93e245e..6a9885149645 100644 --- a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -165,7 +165,8 @@ describe('[Engagement Dashboard]', function () { 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.empty; + const channelRecord = res.body.channels.find(({ room }: { room: { _id: string } }) => room._id === testRoom._id); + expect(channelRecord).to.be.undefined; }); }); From 3ba5355eedc201acb6f4d71abdd15906afbbe96a Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 10 Jun 2024 18:28:40 -0300 Subject: [PATCH 13/25] deprecate new hideRoomsWithNoActivity param --- .../api/engagementDashboard/channels.ts | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index 1b3d44db248d..5a0c864dd732 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -3,6 +3,7 @@ import { check, Match } from 'meteor/check'; import { API } from '../../../../app/api/server'; import { getPaginationItems } from '../../../../app/api/server/helpers/getPaginationItems'; +import { apiDeprecationLogger } from '../../../../app/lib/server/lib/deprecationWarningLogger'; import { findChannelsWithNumberOfMessages } from '../../lib/engagementDashboard/channels'; import { isDateISOString, mapDateForAPI } from '../../lib/engagementDashboard/date'; @@ -54,10 +55,30 @@ API.v1.addRoute( const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); + if (hideRoomsWithNoActivity) { + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Setting the \`${parameter}\` param is deprecated in ${endpoint} and will be removed on version ${version}. This endpoint will behave as when \`${parameter}\` is set as \`true\` by default starting on ${version}`, + ); + } else { + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version}. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, + ); + } + const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start), end: mapDateForAPI(end), - hideRoomsWithNoActivity: hideRoomsWithNoActivity === 'true' ?? false, + hideRoomsWithNoActivity: hideRoomsWithNoActivity === 'true', options: { offset, count }, }); From 450ec1aa838e0b803e364e9796015d727e96e979 Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Tue, 11 Jun 2024 12:55:43 -0300 Subject: [PATCH 14/25] Update apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts Co-authored-by: Marcos Spessatto Defendi --- apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts index 6a9885149645..bfa073b3e099 100644 --- a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -17,7 +17,7 @@ describe('[Engagement Dashboard]', function () { before(() => updatePermission('view-engagement-dashboard', ['admin'])); - before(() => updatePermission('view-engagement-dashboard', ['admin'])); + after(() => updatePermission('view-engagement-dashboard', ['admin'])); (isEnterprise ? describe : describe.skip)('[/engagement-dashboard/channels/list]', () => { let testRoom: IRoom; From 787d0ff4de37e99f90339706b2baff94c18d719c Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Tue, 11 Jun 2024 17:21:19 -0300 Subject: [PATCH 15/25] Improve indexes --- apps/meteor/server/models/raw/Analytics.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/meteor/server/models/raw/Analytics.ts b/apps/meteor/server/models/raw/Analytics.ts index cfb237dab0e4..add9637a1ecb 100644 --- a/apps/meteor/server/models/raw/Analytics.ts +++ b/apps/meteor/server/models/raw/Analytics.ts @@ -14,7 +14,11 @@ export class AnalyticsRaw extends BaseRaw implements IAnalyticsModel } protected modelIndexes(): IndexDescription[] { - return [{ key: { date: 1 } }, { key: { 'room._id': 1, 'date': 1 }, unique: true, partialFilterExpression: { type: 'rooms' } }]; + return [ + { key: { date: 1 } }, + { key: { 'room._id': 1, 'date': 1 }, sparse: true, name: 'room-id-date-index' }, + { key: { 'room.t': 1, 'date': 1 }, sparse: true }, + ]; } saveMessageSent({ room, date }: { room: IRoom; date: IAnalytic['date'] }): Promise { From d832107202604940013f523294aeff6f606027fe Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Wed, 12 Jun 2024 15:44:37 -0300 Subject: [PATCH 16/25] Remove index fix (to be added in another PR) --- apps/meteor/server/models/raw/Analytics.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/server/models/raw/Analytics.ts b/apps/meteor/server/models/raw/Analytics.ts index add9637a1ecb..de4700834168 100644 --- a/apps/meteor/server/models/raw/Analytics.ts +++ b/apps/meteor/server/models/raw/Analytics.ts @@ -16,8 +16,8 @@ export class AnalyticsRaw extends BaseRaw implements IAnalyticsModel protected modelIndexes(): IndexDescription[] { return [ { key: { date: 1 } }, - { key: { 'room._id': 1, 'date': 1 }, sparse: true, name: 'room-id-date-index' }, - { key: { 'room.t': 1, 'date': 1 }, sparse: true }, + { key: { 'room._id': 1, 'date': 1 }, unique: true, partialFilterExpression: { type: 'rooms' } }, + { key: { 'room.t': 1, 'date': 1 }, partialFilterExpression: { type: 'messages' } }, ]; } From 68d67744d0ef8d046efbd4b7c49ea441b6083a16 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Mon, 17 Jun 2024 15:46:28 -0300 Subject: [PATCH 17/25] improve deprecation message --- .../api/engagementDashboard/channels.ts | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index 5a0c864dd732..09a5c89b1423 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -55,25 +55,14 @@ API.v1.addRoute( const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); - if (hideRoomsWithNoActivity) { - apiDeprecationLogger.deprecatedParameterUsage( - this.request.route, - 'hideRoomsWithNoActivity', - '7.0.0', - this.response, - ({ parameter, endpoint, version }) => - `Setting the \`${parameter}\` param is deprecated in ${endpoint} and will be removed on version ${version}. This endpoint will behave as when \`${parameter}\` is set as \`true\` by default starting on ${version}`, - ); - } else { - apiDeprecationLogger.deprecatedParameterUsage( - this.request.route, - 'hideRoomsWithNoActivity', - '7.0.0', - this.response, - ({ parameter, endpoint, version }) => - `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version}. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, - ); - } + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, + ); const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start), From 28ff4d9e9a53d83f83e1c32d4ffec3c990dbb411 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Fri, 21 Jun 2024 11:31:48 -0300 Subject: [PATCH 18/25] only display the deprecation message when param is sent --- .../server/api/engagementDashboard/channels.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index 09a5c89b1423..f5f3bd525ddc 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -55,14 +55,16 @@ API.v1.addRoute( const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); - apiDeprecationLogger.deprecatedParameterUsage( - this.request.route, - 'hideRoomsWithNoActivity', - '7.0.0', - this.response, - ({ parameter, endpoint, version }) => - `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, - ); + if (hideRoomsWithNoActivity) { + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, + ); + } const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start), From c405e02e9536354e73a440a3d62ba363a9de353e Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Tue, 16 Jul 2024 09:57:35 -0300 Subject: [PATCH 19/25] Always display deprecation message --- .../server/api/engagementDashboard/channels.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index f5f3bd525ddc..09a5c89b1423 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -55,16 +55,14 @@ API.v1.addRoute( const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); - if (hideRoomsWithNoActivity) { - apiDeprecationLogger.deprecatedParameterUsage( - this.request.route, - 'hideRoomsWithNoActivity', - '7.0.0', - this.response, - ({ parameter, endpoint, version }) => - `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, - ); - } + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, + ); const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start), From 29f8118bf0d757912d7a9800ffd00c3d060454d3 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Tue, 16 Jul 2024 11:41:07 -0300 Subject: [PATCH 20/25] update imports (converted to TS) --- apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts index bfa073b3e099..0861e4e3f634 100644 --- a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -3,8 +3,8 @@ 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 { getCredentials, api, request, credentials } from '../../data/api-data.ts'; +import { sendSimpleMessage } from '../../data/chat.helper.ts'; import { updatePermission } from '../../data/permissions.helper'; import { createRoom, deleteRoom } from '../../data/rooms.helper'; From 5b5e9dd38f8dd3ac680871175fcb534b4b513d23 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Tue, 16 Jul 2024 12:06:25 -0300 Subject: [PATCH 21/25] fix lint --- apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts index 0861e4e3f634..c1fc685d11f9 100644 --- a/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts +++ b/apps/meteor/tests/end-to-end/api/34-engagement-dashboard.ts @@ -3,8 +3,8 @@ 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.ts'; -import { sendSimpleMessage } from '../../data/chat.helper.ts'; +import { getCredentials, api, request, credentials } from '../../data/api-data'; +import { sendSimpleMessage } from '../../data/chat.helper'; import { updatePermission } from '../../data/permissions.helper'; import { createRoom, deleteRoom } from '../../data/rooms.helper'; From 7aea864869f6f0fd8bb6b1dfc4cd8e9a1246835a Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 18 Jul 2024 13:48:04 -0300 Subject: [PATCH 22/25] fix: handle case for when there are no messages analytics in the provided range --- .../meteor/ee/server/lib/engagementDashboard/channels.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 9cb7e6ee8f04..131a79075d87 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -43,6 +43,15 @@ export const findChannelsWithNumberOfMessages = async ({ const startOfLastWeek = moment(endOfLastWeek).subtract(daysBetweenDates, 'days').toDate(); const roomTypes = roomCoordinator.getTypesToShowOnDashboard() as Array; + const analyticsInDateRange = await Analytics.countDocuments({ + 'type': 'messages', + 'room.t': { $in: roomTypes }, + 'date': { $gte: convertDateToInt(startOfLastWeek), $lte: convertDateToInt(end) }, + }); + if (!analyticsInDateRange) { + return { channels: [], total: 0 }; + } + const [{ channels, total }] = await Analytics.findRoomsByTypesWithNumberOfMessagesBetweenDate({ types: roomTypes, start: convertDateToInt(start), From ea3344696e0c55fc9fb92217e39c80d4ae2ee4bd Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 18 Jul 2024 14:05:22 -0300 Subject: [PATCH 23/25] replace count by findOne --- apps/meteor/ee/server/lib/engagementDashboard/channels.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index 131a79075d87..ef6035d33f55 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -43,7 +43,7 @@ export const findChannelsWithNumberOfMessages = async ({ const startOfLastWeek = moment(endOfLastWeek).subtract(daysBetweenDates, 'days').toDate(); const roomTypes = roomCoordinator.getTypesToShowOnDashboard() as Array; - const analyticsInDateRange = await Analytics.countDocuments({ + const analyticsInDateRange = await Analytics.findOne({ 'type': 'messages', 'room.t': { $in: roomTypes }, 'date': { $gte: convertDateToInt(startOfLastWeek), $lte: convertDateToInt(end) }, From 5cebc20e9581fa022ac6c6dea53000bc392b356c Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 18 Jul 2024 14:20:14 -0300 Subject: [PATCH 24/25] fix: do not extract data if aggregation returns an empty result --- .../server/lib/engagementDashboard/channels.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts index ef6035d33f55..7d08086ee1e9 100644 --- a/apps/meteor/ee/server/lib/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/lib/engagementDashboard/channels.ts @@ -43,16 +43,7 @@ export const findChannelsWithNumberOfMessages = async ({ const startOfLastWeek = moment(endOfLastWeek).subtract(daysBetweenDates, 'days').toDate(); const roomTypes = roomCoordinator.getTypesToShowOnDashboard() as Array; - const analyticsInDateRange = await Analytics.findOne({ - 'type': 'messages', - 'room.t': { $in: roomTypes }, - 'date': { $gte: convertDateToInt(startOfLastWeek), $lte: convertDateToInt(end) }, - }); - if (!analyticsInDateRange) { - return { channels: [], total: 0 }; - } - - const [{ channels, total }] = await Analytics.findRoomsByTypesWithNumberOfMessagesBetweenDate({ + const aggregationResult = await Analytics.findRoomsByTypesWithNumberOfMessagesBetweenDate({ types: roomTypes, start: convertDateToInt(start), end: convertDateToInt(end), @@ -61,6 +52,12 @@ export const findChannelsWithNumberOfMessages = async ({ options, }).toArray(); + // The aggregation result may be undefined if there are no matching analytics or corresponding rooms in the period + if (!aggregationResult.length) { + return { channels: [], total: 0 }; + } + + const [{ channels, total }] = aggregationResult; return { channels, total, From f3ca6f01b981509e943ab8def7a0d580c48a71a0 Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Fri, 19 Jul 2024 18:50:35 -0300 Subject: [PATCH 25/25] chore: show deprecation only without param --- .../server/api/engagementDashboard/channels.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/meteor/ee/server/api/engagementDashboard/channels.ts b/apps/meteor/ee/server/api/engagementDashboard/channels.ts index 09a5c89b1423..0d2d140bd575 100644 --- a/apps/meteor/ee/server/api/engagementDashboard/channels.ts +++ b/apps/meteor/ee/server/api/engagementDashboard/channels.ts @@ -55,14 +55,16 @@ API.v1.addRoute( const { start, end, hideRoomsWithNoActivity } = this.queryParams; const { offset, count } = await getPaginationItems(this.queryParams); - apiDeprecationLogger.deprecatedParameterUsage( - this.request.route, - 'hideRoomsWithNoActivity', - '7.0.0', - this.response, - ({ parameter, endpoint, version }) => - `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, - ); + if (hideRoomsWithNoActivity === undefined) { + apiDeprecationLogger.deprecatedParameterUsage( + this.request.route, + 'hideRoomsWithNoActivity', + '7.0.0', + this.response, + ({ parameter, endpoint, version }) => + `Returning rooms that had no activity in ${endpoint} is deprecated and will be removed on version ${version} along with the \`${parameter}\` param. Set \`${parameter}\` as \`true\` to check how the endpoint will behave starting on ${version}`, + ); + } const { channels, total } = await findChannelsWithNumberOfMessages({ start: mapDateForAPI(start),