fix: Deletion of 1st message in a thread causes notification to be up without showing why. (#34165)

.changeset/ninety-bulldogs-dream.md

@@ -0,0 +1,5 @@
+---
+"@rocket.chat/meteor": patch
+---
+
+Fixes an issue where removing the only message of a thread would keep the unread thread messages badge

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 { 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';
 
 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, notifyOnSubscriptionChangedByRoomIdAndUserIds } from '../lib/notifyListener';
 
 export const deleteMessageValidatingPermission = async (message: AtLeast<IMessage, '_id'>, userId: IUser['_id']): Promise<void> => {
 	if (!message?._id) {
@@ -50,8 +50,8 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise<voi
 		}
 	}
 
-	if (deletedMsg?.tmid) {
-		await Messages.decreaseReplyCountById(deletedMsg.tmid, -1);
+	if (deletedMsg && isThreadMessage(deletedMsg)) {
+		await deleteThreadMessage(deletedMsg, user, room);
 	}
 
 	const files = (message.files || [message.file]).filter(Boolean); // Keep compatibility with old messages
@@ -107,3 +107,25 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise<voi
 		void bridges.getListenerBridge().messageEvent(AppEvents.IPostMessageDeleted, deletedMsg, user);
 	}
 }
+
+async function deleteThreadMessage(message: IThreadMessage, user: IUser, room: IRoom | null): Promise<void> {
+	const { value: updatedParentMessage } = await Messages.decreaseReplyCountById(message.tmid, -1);
+
+	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 (updatedParentMessage && updatedParentMessage.tcount === 0) {
+		void notifyOnMessageChange({
+			id: message.tmid,
+		});
+	}
+}

apps/meteor/server/models/raw/Messages.ts

@@ -1779,13 +1779,13 @@ export class MessagesRaw extends BaseRaw<IMessage> implements IMessagesModel {
 		return this.col.countDocuments(query);
 	}
 
-	decreaseReplyCountById(_id: string, inc = -1): Promise<UpdateResult> {
+	decreaseReplyCountById(_id: string, inc = -1): Promise<ModifyResult<IMessage>> {
 		const query = { _id };
 		const update: UpdateFilter<IMessage> = {
 			$inc: {
 				tcount: inc,
 			},
 		};
-		return this.updateOne(query, update);
+		return this.findOneAndUpdate(query, update, { returnDocument: 'after' });
 	}
 }

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 = ({
@@ -87,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 });
+};

apps/meteor/tests/data/rooms.helper.ts

@@ -1,7 +1,7 @@
 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';
+import { api, credentials, methodCall, request } from './api-data';
 
 type CreateRoomParams = {
 	name?: IRoom['name'];
@@ -108,3 +108,36 @@ 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<ISubscription> =>
+	new Promise((resolve) => {
+		void request
+			.get(api('subscriptions.getOne'))
+			.set(userCredentials)
+			.query({ roomId })
+			.end((_err, res) => {
+				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',
+			}),
+		});
+};

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 } 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';
@@ -2005,6 +2005,52 @@ describe('[Chat]', () => {
 				})
 				.end(done);
 		});
+
+		describe('when deleting a thread message', () => {
+			let otherUser: TestUser<IUser>;
+			let otherUserCredentials: Credentials;
+			let parentThreadId: IMessage['_id'];
+
+			before(async () => {
+				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 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;
+
+				await followMessage({ msgId: parentThreadId, requestCredentials: otherUserCredentials });
+				await deleteMessage({ msgId: childrenMessageId, roomId: testChannel._id });
+
+				const userWhoWasFollowingTheThreadSubscription = await getSubscriptionByRoomId(testChannel._id, otherUserCredentials);
+
+				expectNoUnreadThreadMessages(userWhoWasFollowingTheThreadSubscription);
+			});
+		});
 	});
 
 	describe('/chat.search', () => {

packages/model-typings/src/models/IMessagesModel.ts

@@ -290,7 +290,7 @@ export interface IMessagesModel extends IBaseModel<IMessage> {
 	removeThreadFollowerByThreadId(tmid: string, userId: string): Promise<UpdateResult>;
 
 	findThreadsByRoomId(rid: string, skip: number, limit: number): FindCursor<IMessage>;
-	decreaseReplyCountById(_id: string, inc?: number): Promise<UpdateResult>;
+	decreaseReplyCountById(_id: string, inc?: number): Promise<ModifyResult<IMessage>>;
 	countPinned(options?: CountDocumentsOptions): Promise<number>;
 	countStarred(options?: CountDocumentsOptions): Promise<number>;
 }