diff --git a/.changeset/ninety-bulldogs-dream.md b/.changeset/ninety-bulldogs-dream.md new file mode 100644 index 00000000000..1fe15fb7935 --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/lib/server/functions/deleteMessage.ts b/apps/meteor/app/lib/server/functions/deleteMessage.ts index a91e7785804..30044dadb81 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 { 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, userId: IUser['_id']): Promise => { if (!message?._id) { @@ -50,8 +50,8 @@ export async function deleteMessage(message: IMessage, user: IUser): Promise { + 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, + }); + } +} diff --git a/apps/meteor/server/models/raw/Messages.ts b/apps/meteor/server/models/raw/Messages.ts index e84abc3ed48..bec0afaafe4 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/apps/meteor/tests/data/chat.helper.ts b/apps/meteor/tests/data/chat.helper.ts index 5df28383120..2d0588a7f15 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 = ({ @@ -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 }); +}; diff --git a/apps/meteor/tests/data/rooms.helper.ts b/apps/meteor/tests/data/rooms.helper.ts index 410e6e7ca48..29059cf2f42 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 } 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 => + 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', + }), + }); +}; diff --git a/apps/meteor/tests/end-to-end/api/chat.ts b/apps/meteor/tests/end-to-end/api/chat.ts index 2e23e261377..43df270bbec 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 } 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; + 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', () => { diff --git a/packages/model-typings/src/models/IMessagesModel.ts b/packages/model-typings/src/models/IMessagesModel.ts index e50e71f179b..f5452e957e8 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; }