From baa7fcfd7f0044d18092e63336cc82362daf5922 Mon Sep 17 00:00:00 2001 From: gustrb Date: Wed, 11 Dec 2024 16:19:11 -0300 Subject: [PATCH 01/11] fix: Notifying the client when we remove a message from a thread Now, everytime a message has a tmid(is a thread), and it gets deleted, we decrease the replyCount of the parent message then remove the unread thread unread, to make sure all the subscriptions are up to date (and notify to the client) At the end, we check if the parent message had all of its thread messages deleted, if so, we notify the change in the parent message itself --- .../app/lib/server/functions/deleteMessage.ts | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/deleteMessage.ts b/apps/meteor/app/lib/server/functions/deleteMessage.ts index a91e77858043..cb5e43f11cff 100644 --- a/apps/meteor/app/lib/server/functions/deleteMessage.ts +++ b/apps/meteor/app/lib/server/functions/deleteMessage.ts @@ -1,14 +1,14 @@ import { AppEvents, Apps } from '@rocket.chat/apps'; import { api, Message } from '@rocket.chat/core-services'; -import type { AtLeast, IMessage, IUser } from '@rocket.chat/core-typings'; -import { Messages, Rooms, Uploads, Users, ReadReceipts } from '@rocket.chat/models'; +import type { AtLeast, IMessage, IRoom, IUser } from '@rocket.chat/core-typings'; +import { Messages, Rooms, Uploads, Users, ReadReceipts, Subscriptions } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; import { callbacks } from '../../../../lib/callbacks'; import { canDeleteMessageAsync } from '../../../authorization/server/functions/canDeleteMessage'; import { FileUpload } from '../../../file-upload/server'; import { settings } from '../../../settings/server'; -import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener'; +import { notifyOnRoomChangedById, notifyOnMessageChange, notifyOnSubscriptionChangedByRoomIdAndUserId } from '../lib/notifyListener'; export const deleteMessageValidatingPermission = async (message: AtLeast, userId: IUser['_id']): Promise => { if (!message?._id) { @@ -51,7 +51,7 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise>, user, room); } const files = (message.files || [message.file]).filter(Boolean); // Keep compatibility with old messages @@ -107,3 +107,22 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise>, user: IUser, room: IRoom | null): Promise { + await Messages.decreaseReplyCountById(message.tmid, -1); + + if (room) { + const { modifiedCount } = await Subscriptions.removeUnreadThreadsByRoomId(room._id, [message.tmid]); + if (modifiedCount > 0) { + void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, user._id); + } + } + + // Check if this was the last reply in the thread + const thread = await Messages.findOneById(message.tmid, { projection: { tcount: 1 } }); + if (thread?.tcount === 0) { + void notifyOnMessageChange({ + id: message.tmid, + }); + } +} From ec8e9cd092ec622e7e4bb889f358cddc050968b4 Mon Sep 17 00:00:00 2001 From: Gustavo Reis Bauer Date: Wed, 11 Dec 2024 16:33:59 -0300 Subject: [PATCH 02/11] Create ninety-bulldogs-dream.md --- .changeset/ninety-bulldogs-dream.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ninety-bulldogs-dream.md diff --git a/.changeset/ninety-bulldogs-dream.md b/.changeset/ninety-bulldogs-dream.md new file mode 100644 index 000000000000..873a0c76a26d --- /dev/null +++ b/.changeset/ninety-bulldogs-dream.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": minor +--- + +Fixes an issue where removing the only thread message of a message would show as if there were unread messages. From 9f528a1f7f81d810097cb04a5f7eb449842603ec Mon Sep 17 00:00:00 2001 From: gustrb Date: Thu, 12 Dec 2024 10:29:47 -0300 Subject: [PATCH 03/11] test: add api tests to ensure the behavior --- apps/meteor/tests/data/chat.helper.ts | 4 ++- apps/meteor/tests/end-to-end/api/chat.ts | 44 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/chat.helper.ts b/apps/meteor/tests/data/chat.helper.ts index 5df283831203..2d5aaaf15ad3 100644 --- a/apps/meteor/tests/data/chat.helper.ts +++ b/apps/meteor/tests/data/chat.helper.ts @@ -7,10 +7,12 @@ export const sendSimpleMessage = ({ roomId, text = 'test message', tmid, + userCredentials = credentials, }: { roomId: IRoom['_id']; text?: string; tmid?: IMessage['_id']; + userCredentials?: Credentials; }) => { if (!roomId) { throw new Error('"roomId" is required in "sendSimpleMessage" test helper'); @@ -28,7 +30,7 @@ export const sendSimpleMessage = ({ message.tmid = tmid; } - return request.post(api('chat.sendMessage')).set(credentials).send({ message }); + return request.post(api('chat.sendMessage')).set(userCredentials).send({ message }); }; export const sendMessage = ({ diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index 2e23e2613779..023155f6420f 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -2005,6 +2005,50 @@ describe('[Chat]', () => { }) .end(done); }); + + describe('when deleting a thread message', () => { + let otherUser: TestUser; + let otherUserCredentials: Credentials; + let parentThreadId: IMessage['_id']; + + before(async () => { + otherUser = await createUser(); + otherUserCredentials = await login(otherUser.username, password); + parentThreadId = (await sendSimpleMessage({ roomId: testChannel._id })).body.message._id; + }); + + after(() => Promise.all([deleteUser(otherUser), deleteMessage({ msgId: parentThreadId, roomId: testChannel._id })])); + + it('should reset the unread counter if the message was removed', async () => { + const { body } = await sendSimpleMessage({ roomId: testChannel._id, tmid: parentThreadId, userCredentials: otherUserCredentials }); + const msgId = body.message._id; + await request + .post(api('chat.delete')) + .set(credentials) + .send({ + roomId: testChannel._id, + msgId, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }); + await request + .get(api('subscriptions.getOne')) + .set(credentials) + .query({ + roomId: testChannel._id, + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body.subscription).to.have.property('tunread'); + expect(res.body.subscription.tunread).to.be.an('array'); + expect(res.body.subscription.tunread).to.deep.equal([]); + }); + }); + }); }); describe('/chat.search', () => { From c2b5efb736e0bc9ded9d484ae0288a897f9e5cd8 Mon Sep 17 00:00:00 2001 From: gustrb Date: Fri, 13 Dec 2024 11:22:08 -0300 Subject: [PATCH 04/11] chore: using the notifyOnSubscriptionChangedByRoomIdAndUserIds to notify everyone that is on the replies list --- .changeset/ninety-bulldogs-dream.md | 4 ++-- .../app/lib/server/functions/deleteMessage.ts | 21 ++++++++++--------- apps/meteor/server/models/raw/Messages.ts | 4 ++-- .../src/models/IMessagesModel.ts | 2 +- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/.changeset/ninety-bulldogs-dream.md b/.changeset/ninety-bulldogs-dream.md index 873a0c76a26d..1fe15fb79350 100644 --- a/.changeset/ninety-bulldogs-dream.md +++ b/.changeset/ninety-bulldogs-dream.md @@ -1,5 +1,5 @@ --- -"@rocket.chat/meteor": minor +"@rocket.chat/meteor": patch --- -Fixes an issue where removing the only thread message of a message would show as if there were unread messages. +Fixes an issue where removing the only message of a thread would keep the unread thread messages badge diff --git a/apps/meteor/app/lib/server/functions/deleteMessage.ts b/apps/meteor/app/lib/server/functions/deleteMessage.ts index cb5e43f11cff..7b105e81c77e 100644 --- a/apps/meteor/app/lib/server/functions/deleteMessage.ts +++ b/apps/meteor/app/lib/server/functions/deleteMessage.ts @@ -1,6 +1,6 @@ import { AppEvents, Apps } from '@rocket.chat/apps'; import { api, Message } from '@rocket.chat/core-services'; -import type { AtLeast, IMessage, IRoom, IUser } from '@rocket.chat/core-typings'; +import { isThreadMessage, type AtLeast, type IMessage, type IRoom, type IThreadMessage, type IUser } from '@rocket.chat/core-typings'; import { Messages, Rooms, Uploads, Users, ReadReceipts, Subscriptions } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; @@ -8,7 +8,7 @@ import { callbacks } from '../../../../lib/callbacks'; import { canDeleteMessageAsync } from '../../../authorization/server/functions/canDeleteMessage'; import { FileUpload } from '../../../file-upload/server'; import { settings } from '../../../settings/server'; -import { notifyOnRoomChangedById, notifyOnMessageChange, notifyOnSubscriptionChangedByRoomIdAndUserId } from '../lib/notifyListener'; +import { notifyOnRoomChangedById, notifyOnMessageChange, notifyOnSubscriptionChangedByRoomIdAndUserIds } from '../lib/notifyListener'; export const deleteMessageValidatingPermission = async (message: AtLeast, userId: IUser['_id']): Promise => { if (!message?._id) { @@ -50,8 +50,8 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise>, user, room); + if (deletedMsg && isThreadMessage(deletedMsg)) { + await deleteThreadMessage(deletedMsg, user, room); } const files = (message.files || [message.file]).filter(Boolean); // Keep compatibility with old messages @@ -108,19 +108,20 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise>, user: IUser, room: IRoom | null): Promise { - await Messages.decreaseReplyCountById(message.tmid, -1); +async function deleteThreadMessage(message: IThreadMessage, user: IUser, room: IRoom | null): Promise { + const { value: updatedParentMessage } = await Messages.decreaseReplyCountById(message.tmid, -1); if (room) { const { modifiedCount } = await Subscriptions.removeUnreadThreadsByRoomId(room._id, [message.tmid]); if (modifiedCount > 0) { - void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, user._id); + const userIdsThatAreWatchingTheThread = [...new Set([user._id, ...(message.replies || [])])]; + void notifyOnSubscriptionChangedByRoomIdAndUserIds(room._id, userIdsThatAreWatchingTheThread); } } - // Check if this was the last reply in the thread - const thread = await Messages.findOneById(message.tmid, { projection: { tcount: 1 } }); - if (thread?.tcount === 0) { + // If we could not find the parent message, there is no need to notify whoever is listening + // about the change in the parent message + if (updatedParentMessage && updatedParentMessage.tcount === 0) { void notifyOnMessageChange({ id: message.tmid, }); diff --git a/apps/meteor/server/models/raw/Messages.ts b/apps/meteor/server/models/raw/Messages.ts index e84abc3ed481..bec0afaafe43 100644 --- a/apps/meteor/server/models/raw/Messages.ts +++ b/apps/meteor/server/models/raw/Messages.ts @@ -1779,13 +1779,13 @@ export class MessagesRaw extends BaseRaw implements IMessagesModel { return this.col.countDocuments(query); } - decreaseReplyCountById(_id: string, inc = -1): Promise { + decreaseReplyCountById(_id: string, inc = -1): Promise> { const query = { _id }; const update: UpdateFilter = { $inc: { tcount: inc, }, }; - return this.updateOne(query, update); + return this.findOneAndUpdate(query, update, { returnDocument: 'after' }); } } diff --git a/packages/model-typings/src/models/IMessagesModel.ts b/packages/model-typings/src/models/IMessagesModel.ts index e50e71f179bc..f5452e957e8f 100644 --- a/packages/model-typings/src/models/IMessagesModel.ts +++ b/packages/model-typings/src/models/IMessagesModel.ts @@ -290,7 +290,7 @@ export interface IMessagesModel extends IBaseModel { removeThreadFollowerByThreadId(tmid: string, userId: string): Promise; findThreadsByRoomId(rid: string, skip: number, limit: number): FindCursor; - decreaseReplyCountById(_id: string, inc?: number): Promise; + decreaseReplyCountById(_id: string, inc?: number): Promise>; countPinned(options?: CountDocumentsOptions): Promise; countStarred(options?: CountDocumentsOptions): Promise; } From b14fae4a37945665a2b641aedf624202db3e11c2 Mon Sep 17 00:00:00 2001 From: gustrb Date: Fri, 13 Dec 2024 14:15:29 -0300 Subject: [PATCH 05/11] feat: add test method to fetch subscription by roomId --- apps/meteor/tests/data/rooms.helper.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/rooms.helper.ts b/apps/meteor/tests/data/rooms.helper.ts index 410e6e7ca48c..0e2855c4bfd4 100644 --- a/apps/meteor/tests/data/rooms.helper.ts +++ b/apps/meteor/tests/data/rooms.helper.ts @@ -1,5 +1,5 @@ import type { Credentials } from '@rocket.chat/api-client'; -import type { IRoom } from '@rocket.chat/core-typings'; +import type { IRoom, ISubscription } from '@rocket.chat/core-typings'; import { api, credentials, request } from './api-data'; @@ -108,3 +108,14 @@ export function actionRoom({ action, type, roomId, overrideCredentials = credent export const deleteRoom = ({ type, roomId }: { type: ActionRoomParams['type']; roomId: IRoom['_id'] }) => actionRoom({ action: 'delete', type, roomId, overrideCredentials: credentials }); + +export const getSubscriptionByRoomId = (roomId: IRoom['_id'], userCredentials = credentials): Promise => + new Promise((resolve) => { + void request + .get(api('subscriptions.getOne')) + .set(userCredentials) + .query({ roomId }) + .end((_err, res) => { + resolve(res.body.subscription); + }); + }); From 5ec9527786f0304758b3bd37439de94d0cf65760 Mon Sep 17 00:00:00 2001 From: gustrb Date: Fri, 13 Dec 2024 14:15:51 -0300 Subject: [PATCH 06/11] test: ensure the subscription has no unread threads from both users --- apps/meteor/tests/end-to-end/api/chat.ts | 28 +++++++++++------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index 023155f6420f..7c851d1c2778 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -1,5 +1,5 @@ import type { Credentials } from '@rocket.chat/api-client'; -import type { IMessage, IRoom, IThreadMessage, IUser } from '@rocket.chat/core-typings'; +import type { IMessage, IRoom, ISubscription, IThreadMessage, IUser } from '@rocket.chat/core-typings'; import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, before, beforeEach, describe, it } from 'mocha'; @@ -9,7 +9,7 @@ import { getCredentials, api, request, credentials } from '../../data/api-data'; import { sendSimpleMessage, deleteMessage } from '../../data/chat.helper'; import { imgURL } from '../../data/interactions'; import { updatePermission, updateSetting } from '../../data/permissions.helper'; -import { createRoom, deleteRoom } from '../../data/rooms.helper'; +import { createRoom, deleteRoom, getSubscriptionByRoomId } from '../../data/rooms.helper'; import { password } from '../../data/user'; import type { TestUser } from '../../data/users.helper'; import { createUser, deleteUser, login } from '../../data/users.helper'; @@ -2034,19 +2034,17 @@ describe('[Chat]', () => { .expect((res) => { expect(res.body).to.have.property('success', true); }); - await request - .get(api('subscriptions.getOne')) - .set(credentials) - .query({ - roomId: testChannel._id, - }) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - expect(res.body.subscription).to.have.property('tunread'); - expect(res.body.subscription.tunread).to.be.an('array'); - expect(res.body.subscription.tunread).to.deep.equal([]); - }); + + const userWhoCreatedTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id); + const userWhoDeletedTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id, otherUserCredentials); + const expectThreadUnreadToBeEmpty = (subscription: ISubscription) => { + expect(subscription).to.have.property('tunread'); + expect(subscription.tunread).to.be.an('array'); + expect(subscription.tunread).to.deep.equal([]); + }; + + expectThreadUnreadToBeEmpty(userWhoCreatedTheThreadSubscription); + expectThreadUnreadToBeEmpty(userWhoDeletedTheThreadSubscription); }); }); }); From 961ee549daf79a06b386d2d95caf421143ed8219 Mon Sep 17 00:00:00 2001 From: gustrb Date: Fri, 13 Dec 2024 14:48:59 -0300 Subject: [PATCH 07/11] chore: fix test --- apps/meteor/tests/end-to-end/api/chat.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index 7c851d1c2778..a6210918dd89 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -1,5 +1,5 @@ import type { Credentials } from '@rocket.chat/api-client'; -import type { IMessage, IRoom, ISubscription, IThreadMessage, IUser } from '@rocket.chat/core-typings'; +import type { IMessage, IRoom, IThreadMessage, IUser } from '@rocket.chat/core-typings'; import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, before, beforeEach, describe, it } from 'mocha'; @@ -2035,16 +2035,18 @@ describe('[Chat]', () => { expect(res.body).to.have.property('success', true); }); - const userWhoCreatedTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id); - const userWhoDeletedTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id, otherUserCredentials); - const expectThreadUnreadToBeEmpty = (subscription: ISubscription) => { - expect(subscription).to.have.property('tunread'); - expect(subscription.tunread).to.be.an('array'); - expect(subscription.tunread).to.deep.equal([]); - }; + const [userWhoCreatedTheThreadSubscription, userWhoDeletedTheThreadSubscription] = await Promise.all([ + getSubscriptionByRoomId(testChannel._id), + getSubscriptionByRoomId(testChannel._id, otherUserCredentials), + ]); + + expect(userWhoCreatedTheThreadSubscription).to.have.property('tunread'); + expect(userWhoCreatedTheThreadSubscription.tunread).to.be.an('array'); + expect(userWhoCreatedTheThreadSubscription.tunread).to.deep.equal([]); - expectThreadUnreadToBeEmpty(userWhoCreatedTheThreadSubscription); - expectThreadUnreadToBeEmpty(userWhoDeletedTheThreadSubscription); + // The user who deleted the thread should not have the thread marked as unread, since + // there was never a message in the thread that was not read by the user + expect(userWhoDeletedTheThreadSubscription).to.not.have.property('tunread'); }); }); }); From b38fb2ea7ee3c7469b16e5d4e949cff1dc512b06 Mon Sep 17 00:00:00 2001 From: gustrb Date: Thu, 19 Dec 2024 11:38:52 -0300 Subject: [PATCH 08/11] chore: explain the logic --- apps/meteor/app/lib/server/functions/deleteMessage.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/deleteMessage.ts b/apps/meteor/app/lib/server/functions/deleteMessage.ts index 7b105e81c77e..30044dadb81c 100644 --- a/apps/meteor/app/lib/server/functions/deleteMessage.ts +++ b/apps/meteor/app/lib/server/functions/deleteMessage.ts @@ -114,13 +114,15 @@ async function deleteThreadMessage(message: IThreadMessage, user: IUser, room: I if (room) { const { modifiedCount } = await Subscriptions.removeUnreadThreadsByRoomId(room._id, [message.tmid]); if (modifiedCount > 0) { + // The replies array contains the ids of all the users that are following the thread (everyone that is involved + the ones who are following) + // Technically, user._id is already in the message.replies array, but since we don't have any strong + // guarantees of it, we are adding again to make sure it is there. const userIdsThatAreWatchingTheThread = [...new Set([user._id, ...(message.replies || [])])]; + // So they can decrement the unread threads count void notifyOnSubscriptionChangedByRoomIdAndUserIds(room._id, userIdsThatAreWatchingTheThread); } } - // If we could not find the parent message, there is no need to notify whoever is listening - // about the change in the parent message if (updatedParentMessage && updatedParentMessage.tcount === 0) { void notifyOnMessageChange({ id: message.tmid, From 9a1f19e2e50ae129b309984ca32c79601676c9b9 Mon Sep 17 00:00:00 2001 From: gustrb Date: Thu, 19 Dec 2024 11:39:10 -0300 Subject: [PATCH 09/11] feat: add helper to follow a message thread --- apps/meteor/tests/data/chat.helper.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/meteor/tests/data/chat.helper.ts b/apps/meteor/tests/data/chat.helper.ts index 2d5aaaf15ad3..2d0588a7f158 100644 --- a/apps/meteor/tests/data/chat.helper.ts +++ b/apps/meteor/tests/data/chat.helper.ts @@ -89,3 +89,10 @@ export const getMessageById = ({ msgId }: { msgId: IMessage['_id'] }) => { }); }); }; + +export const followMessage = ({ msgId, requestCredentials }: { msgId: IMessage['_id']; requestCredentials?: Credentials }) => { + return request + .post(api('chat.followMessage')) + .set(requestCredentials ?? credentials) + .send({ mid: msgId }); +}; From 65ae2130e29cf4aaba215a0839cf2a39cb182177 Mon Sep 17 00:00:00 2001 From: gustrb Date: Thu, 19 Dec 2024 11:39:30 -0300 Subject: [PATCH 10/11] feat: added a helper to add a user to a room --- apps/meteor/tests/data/rooms.helper.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/data/rooms.helper.ts b/apps/meteor/tests/data/rooms.helper.ts index 0e2855c4bfd4..29059cf2f42b 100644 --- a/apps/meteor/tests/data/rooms.helper.ts +++ b/apps/meteor/tests/data/rooms.helper.ts @@ -1,7 +1,7 @@ import type { Credentials } from '@rocket.chat/api-client'; import type { IRoom, ISubscription } from '@rocket.chat/core-typings'; -import { api, credentials, request } from './api-data'; +import { api, credentials, methodCall, request } from './api-data'; type CreateRoomParams = { name?: IRoom['name']; @@ -119,3 +119,25 @@ export const getSubscriptionByRoomId = (roomId: IRoom['_id'], userCredentials = resolve(res.body.subscription); }); }); + +export const addUserToRoom = ({ + usernames, + rid, + userCredentials, +}: { + usernames: string[]; + rid: IRoom['_id']; + userCredentials?: Credentials; +}) => { + return request + .post(methodCall('addUsersToRoom')) + .set(userCredentials ?? credentials) + .send({ + message: JSON.stringify({ + method: 'addUsersToRoom', + params: [{ rid, users: usernames }], + id: 'id', + msg: 'method', + }), + }); +}; From 664599fbf885f9939041dd62354c1435c19b96bc Mon Sep 17 00:00:00 2001 From: gustrb Date: Thu, 19 Dec 2024 11:39:57 -0300 Subject: [PATCH 11/11] test: add a test case validating followers --- apps/meteor/tests/end-to-end/api/chat.ts | 56 ++++++++++++------------ 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index a6210918dd89..43df270bbec1 100644 --- a/apps/meteor/tests/end-to-end/api/chat.ts +++ b/apps/meteor/tests/end-to-end/api/chat.ts @@ -1,15 +1,15 @@ import type { Credentials } from '@rocket.chat/api-client'; -import type { IMessage, IRoom, IThreadMessage, IUser } from '@rocket.chat/core-typings'; +import type { IMessage, IRoom, ISubscription, IThreadMessage, IUser } from '@rocket.chat/core-typings'; import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, before, beforeEach, describe, it } from 'mocha'; import type { Response } from 'supertest'; import { getCredentials, api, request, credentials } from '../../data/api-data'; -import { sendSimpleMessage, deleteMessage } from '../../data/chat.helper'; +import { followMessage, sendSimpleMessage, deleteMessage } from '../../data/chat.helper'; import { imgURL } from '../../data/interactions'; import { updatePermission, updateSetting } from '../../data/permissions.helper'; -import { createRoom, deleteRoom, getSubscriptionByRoomId } from '../../data/rooms.helper'; +import { addUserToRoom, createRoom, deleteRoom, getSubscriptionByRoomId } from '../../data/rooms.helper'; import { password } from '../../data/user'; import type { TestUser } from '../../data/users.helper'; import { createUser, deleteUser, login } from '../../data/users.helper'; @@ -2012,41 +2012,43 @@ describe('[Chat]', () => { let parentThreadId: IMessage['_id']; before(async () => { - otherUser = await createUser(); + const username = `user${+new Date()}`; + otherUser = await createUser({ username }); otherUserCredentials = await login(otherUser.username, password); parentThreadId = (await sendSimpleMessage({ roomId: testChannel._id })).body.message._id; + await addUserToRoom({ rid: testChannel._id, usernames: [otherUser.username] }); }); after(() => Promise.all([deleteUser(otherUser), deleteMessage({ msgId: parentThreadId, roomId: testChannel._id })])); + const expectNoUnreadThreadMessages = (s: ISubscription) => { + expect(s).to.have.property('tunread'); + expect(s.tunread).to.be.an('array'); + expect(s.tunread).to.deep.equal([]); + }; + it('should reset the unread counter if the message was removed', async () => { const { body } = await sendSimpleMessage({ roomId: testChannel._id, tmid: parentThreadId, userCredentials: otherUserCredentials }); - const msgId = body.message._id; - await request - .post(api('chat.delete')) - .set(credentials) - .send({ - roomId: testChannel._id, - msgId, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - }); + const childrenMessageId = body.message._id; + + await followMessage({ msgId: parentThreadId, requestCredentials: otherUserCredentials }); + await deleteMessage({ msgId: childrenMessageId, roomId: testChannel._id }); + + const userWhoCreatedTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id); + + expectNoUnreadThreadMessages(userWhoCreatedTheThreadSubscription); + }); + + it('should reset the unread counter of users who followed the thread', async () => { + const { body } = await sendSimpleMessage({ roomId: testChannel._id, tmid: parentThreadId }); + const childrenMessageId = body.message._id; - const [userWhoCreatedTheThreadSubscription, userWhoDeletedTheThreadSubscription] = await Promise.all([ - getSubscriptionByRoomId(testChannel._id), - getSubscriptionByRoomId(testChannel._id, otherUserCredentials), - ]); + await followMessage({ msgId: parentThreadId, requestCredentials: otherUserCredentials }); + await deleteMessage({ msgId: childrenMessageId, roomId: testChannel._id }); - expect(userWhoCreatedTheThreadSubscription).to.have.property('tunread'); - expect(userWhoCreatedTheThreadSubscription.tunread).to.be.an('array'); - expect(userWhoCreatedTheThreadSubscription.tunread).to.deep.equal([]); + const userWhoWasFollowingTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id, otherUserCredentials); - // The user who deleted the thread should not have the thread marked as unread, since - // there was never a message in the thread that was not read by the user - expect(userWhoDeletedTheThreadSubscription).to.not.have.property('tunread'); + expectNoUnreadThreadMessages(userWhoWasFollowingTheThreadSubscription); }); }); });