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

.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,23 @@ 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) {
+			const userIdsThatAreWatchingTheThread = [...new Set([user._id, ...(message.replies || [])])];
+			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,
+		});
+	}
+}

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 = ({

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<ISubscription> =>
+	new Promise((resolve) => {
+		void request
+			.get(api('subscriptions.getOne'))
+			.set(userCredentials)
+			.query({ roomId })
+			.end((_err, res) => {
+				resolve(res.body.subscription);
+			});
+	});

apps/meteor/tests/end-to-end/api/chat.ts

@@ -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';
@@ -2005,6 +2005,50 @@ describe('[Chat]', () => {
 				})
 				.end(done);
 		});
+
+		describe('when deleting a thread message', () => {
+			let otherUser: TestUser<IUser>;
+			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);
+					});
+
+				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([]);
+
+				// 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');
+			});
+		});
 	});
 
 	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>;
 }