From aad915d84100c2d645db4e05963d2cd5b75dbeda Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 24 Feb 2026 17:43:00 +0530 Subject: [PATCH 01/18] fix: read receipts not turning blue when all active users have read Signed-off-by: Abhinav Kumar --- .../server/functions/setUserActiveStatus.ts | 21 +- .../lib/message-read-receipt/ReadReceipt.ts | 2 +- .../read-receipts-deactivated-users.spec.ts | 110 ++++++++ .../functions/setUserActiveStatus.spec.ts | 239 ++++++++++++++++++ .../model-typings/src/models/IRoomsModel.ts | 1 + .../src/models/ISubscriptionsModel.ts | 4 +- packages/models/src/models/Rooms.ts | 9 + packages/models/src/models/Subscriptions.ts | 43 +++- 8 files changed, 410 insertions(+), 19 deletions(-) create mode 100644 apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts create mode 100644 apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index 8ebfe0c7449bc..4727ab5df4b6a 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts @@ -15,7 +15,7 @@ import { settings } from '../../../settings/server'; import { notifyOnRoomChangedById, notifyOnRoomChangedByUserDM, - notifyOnSubscriptionChangedByNameAndRoomType, + notifyOnSubscriptionChangedByUserId, notifyOnUserChange, } from '../lib/notifyListener'; @@ -112,10 +112,23 @@ export async function setUserActiveStatus( await callbacks.run('afterDeactivateUser', user); } - if (user.username) { - const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, !active); + if (user.username && active === false) { + const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, true); if (modifiedCount) { - void notifyOnSubscriptionChangedByNameAndRoomType({ t: 'd', name: user.username }); + void notifyOnSubscriptionChangedByUserId(userId); + } + } + + if (user.username && active === true && !user.active) { + // When reactivating, only unarchive subscriptions to non-archived rooms + const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray(); + const roomIds = subscriptions.map((sub) => sub.rid); + const archivedRooms = await Rooms.findArchivedByRoomIds(roomIds, { projection: { _id: 1 } }).toArray(); + const archivedRoomIds = archivedRooms.map((room) => room._id); + + const { modifiedCount } = await Subscriptions.unarchiveByUsernameExcludingRoomIds(user.username, archivedRoomIds); + if (modifiedCount) { + void notifyOnSubscriptionChangedByUserId(userId); } } diff --git a/apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts b/apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts index 8be6bc78ab1b8..62122cab7c3ed 100644 --- a/apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts +++ b/apps/meteor/ee/server/lib/message-read-receipt/ReadReceipt.ts @@ -72,7 +72,7 @@ class ReadReceiptClass { } // mark message as read if the sender is the only one in the room - const isUserAlone = (await Subscriptions.countByRoomIdAndNotUserId(roomId, userId)) === 0; + const isUserAlone = (await Subscriptions.countUnarchivedByRoomIdAndNotUserId(roomId, userId)) === 0; if (isUserAlone) { const result = await Messages.setAsReadById(message._id); if (result.modifiedCount > 0) { diff --git a/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts new file mode 100644 index 0000000000000..07aaf8e9e1c40 --- /dev/null +++ b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts @@ -0,0 +1,110 @@ +import type { Page } from '@playwright/test'; + +import { IS_EE } from './config/constants'; +import { createAuxContext } from './fixtures/createAuxContext'; +import { Users } from './fixtures/userStates'; +import { HomeChannel } from './page-objects'; +import { createTargetChannel, deleteChannel, setSettingValueById } from './utils'; +import { expect, test } from './utils/test'; + +test.use({ storageState: Users.admin.state }); + +test.describe.serial('read-receipts-deactivated-users', () => { + let poHomeChannel: HomeChannel; + let targetChannel: string; + let user1Context: { page: Page; poHomeChannel: HomeChannel } | undefined; + let user2Context: { page: Page; poHomeChannel: HomeChannel } | undefined; + + test.skip(!IS_EE, 'Enterprise Only'); + + test.beforeAll(async ({ api }) => { + targetChannel = await createTargetChannel(api, { members: ['user1', 'user2'] }); + await setSettingValueById(api, 'Message_Read_Receipt_Enabled', true); + await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', true); + }); + + test.afterAll(async ({ api }) => { + await setSettingValueById(api, 'Message_Read_Receipt_Enabled', false); + await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', false); + + await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: true }); + await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: true }); + + await deleteChannel(api, targetChannel); + }); + + test.beforeEach(async ({ page }) => { + poHomeChannel = new HomeChannel(page); + await page.goto('/home'); + }); + + test.afterEach(async () => { + if (user1Context) { + await user1Context.page.close(); + } + if (user2Context) { + await user2Context.page.close(); + } + user1Context = undefined; + user2Context = undefined; + }); + + test('should correctly handle read receipts as users are deactivated', async ({ browser, api, page }) => { + const { page: page1 } = await createAuxContext(browser, Users.user1); + const user1Ctx = { page: page1, poHomeChannel: new HomeChannel(page1) }; + user1Context = user1Ctx; + + const { page: page2 } = await createAuxContext(browser, Users.user2); + const user2Ctx = { page: page2, poHomeChannel: new HomeChannel(page2) }; + user2Context = user2Ctx; + + await poHomeChannel.navbar.openChat(targetChannel); + await user1Ctx.poHomeChannel.navbar.openChat(targetChannel); + await user2Ctx.poHomeChannel.navbar.openChat(targetChannel); + + await test.step('when all users are active', async () => { + await poHomeChannel.content.sendMessage('Message 1: All three users active'); + + await expect(user1Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(); + await expect(user2Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(); + + await expect(poHomeChannel.content.lastUserMessage.getByRole('status', { name: 'Message viewed' })).toBeVisible(); + + await poHomeChannel.content.openLastMessageMenu(); + await page.locator('role=menuitem[name="Read receipts"]').click(); + await expect(page.getByRole('dialog').getByRole('listitem')).toHaveCount(3); + await page.getByRole('button', { name: 'Close' }).click(); + }); + + await test.step('when some users are deactivated', async () => { + await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: false }); + + await poHomeChannel.content.sendMessage('Message 2: User1 deactivated, two active users'); + + await expect(user2Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(); + + await expect(poHomeChannel.content.lastUserMessage.getByRole('status', { name: 'Message viewed' })).toBeVisible(); + + await poHomeChannel.content.openLastMessageMenu(); + await page.locator('role=menuitem[name="Read receipts"]').click(); + await expect(page.getByRole('dialog').getByRole('listitem')).toHaveCount(2); + await page.getByRole('button', { name: 'Close' }).click(); + }); + + await test.step('when only one user remains active (user alone in room)', async () => { + await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: false }); + + await poHomeChannel.content.sendMessage('Message 3: Only admin active'); + + await expect(poHomeChannel.content.lastUserMessage.getByRole('status', { name: 'Message viewed' })).toBeVisible(); + + await poHomeChannel.content.openLastMessageMenu(); + await page.locator('role=menuitem[name="Read receipts"]').click(); + await expect(page.getByRole('dialog').getByRole('listitem')).toHaveCount(1); + await page.getByRole('button', { name: 'Close' }).click(); + }); + + await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: true }); + await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: true }); + }); +}); diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts new file mode 100644 index 0000000000000..b0e47efb47e55 --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -0,0 +1,239 @@ +import { expect } from 'chai'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +describe('setUserActiveStatus', () => { + const userId = 'test-user-id'; + const username = 'testuser'; + + const stubs = { + Users: { + findOneById: sinon.stub(), + setUserActive: sinon.stub(), + findOneAdmin: sinon.stub(), + countActiveUsersInRoles: sinon.stub(), + unsetLoginTokens: sinon.stub(), + unsetReason: sinon.stub(), + findActiveByUserIds: sinon.stub(), + }, + Subscriptions: { + setArchivedByUsername: sinon.stub(), + findByUserId: sinon.stub(), + unarchiveByUsernameExcludingRoomIds: sinon.stub(), + }, + Rooms: { + findArchivedByRoomIds: sinon.stub(), + setDmReadOnlyByUserId: sinon.stub(), + getDirectConversationsByUserId: sinon.stub(), + }, + check: sinon.stub(), + callbacks: { + run: sinon.stub(), + }, + settings: { + get: sinon.stub(), + }, + notifyOnUserChange: sinon.stub(), + notifyOnSubscriptionChangedByUserId: sinon.stub(), + notifyOnRoomChangedByUserDM: sinon.stub(), + notifyOnRoomChangedById: sinon.stub(), + getSubscribedRoomsForUserWithDetails: sinon.stub(), + shouldRemoveOrChangeOwner: sinon.stub(), + getUserSingleOwnedRooms: sinon.stub(), + closeOmnichannelConversations: sinon.stub(), + relinquishRoomOwnerships: sinon.stub(), + Mailer: { + sendNoWrap: sinon.stub(), + }, + Accounts: { + emailTemplates: { + userActivated: { + subject: sinon.stub(), + html: sinon.stub(), + }, + }, + }, + isUserFederated: sinon.stub(), + }; + + const { setUserActiveStatus } = proxyquire.noCallThru().load('../../../../../../app/lib/server/functions/setUserActiveStatus', { + 'meteor/check': { check: stubs.check }, + 'meteor/meteor': { Meteor: { Error } }, + 'meteor/accounts-base': { Accounts: stubs.Accounts }, + '@rocket.chat/core-typings': { isUserFederated: stubs.isUserFederated, isDirectMessageRoom: sinon.stub() }, + '@rocket.chat/models': { Users: stubs.Users, Subscriptions: stubs.Subscriptions, Rooms: stubs.Rooms }, + './closeOmnichannelConversations': { closeOmnichannelConversations: stubs.closeOmnichannelConversations }, + './getRoomsWithSingleOwner': { + shouldRemoveOrChangeOwner: stubs.shouldRemoveOrChangeOwner, + getSubscribedRoomsForUserWithDetails: stubs.getSubscribedRoomsForUserWithDetails, + }, + './getUserSingleOwnedRooms': { getUserSingleOwnedRooms: stubs.getUserSingleOwnedRooms }, + './relinquishRoomOwnerships': { relinquishRoomOwnerships: stubs.relinquishRoomOwnerships }, + '../../../../server/lib/callbacks': { callbacks: stubs.callbacks }, + '../../../mailer/server/api': stubs.Mailer, + '../../../settings/server': { settings: stubs.settings }, + '../lib/notifyListener': { + notifyOnRoomChangedById: stubs.notifyOnRoomChangedById, + notifyOnRoomChangedByUserDM: stubs.notifyOnRoomChangedByUserDM, + notifyOnSubscriptionChangedByUserId: stubs.notifyOnSubscriptionChangedByUserId, + notifyOnUserChange: stubs.notifyOnUserChange, + }, + }); + + beforeEach(() => { + stubs.Users.findOneById.resolves({ _id: userId, username, active: true }); + stubs.Users.setUserActive.resolves(); + stubs.Users.unsetLoginTokens.resolves(); + stubs.Users.unsetReason.resolves(); + stubs.isUserFederated.returns(false); + stubs.Users.findOneAdmin.resolves(null); + stubs.Users.countActiveUsersInRoles.resolves(2); + stubs.Users.findActiveByUserIds.returns({ toArray: sinon.stub().resolves([]) }); + stubs.getSubscribedRoomsForUserWithDetails.resolves([]); + stubs.shouldRemoveOrChangeOwner.returns(false); + stubs.closeOmnichannelConversations.resolves(); + stubs.relinquishRoomOwnerships.resolves(); + stubs.callbacks.run.resolves(); + stubs.settings.get.returns(false); + stubs.Rooms.setDmReadOnlyByUserId.resolves({ modifiedCount: 0 }); + stubs.Rooms.getDirectConversationsByUserId.returns({ toArray: sinon.stub().resolves([]) }); + stubs.notifyOnRoomChangedById.returns(undefined); + stubs.notifyOnUserChange.returns(undefined); + stubs.notifyOnRoomChangedByUserDM.returns(undefined); + stubs.notifyOnSubscriptionChangedByUserId.returns(undefined); + }); + + afterEach(() => { + Object.values(stubs).forEach((stub) => { + if (typeof stub === 'object' && stub !== null) { + Object.values(stub).forEach((method) => { + if (typeof method?.reset === 'function') { + method.reset(); + } + }); + } else if (typeof stub?.reset === 'function') { + stub.reset(); + } + }); + }); + + describe('Subscription archiving on deactivation', () => { + it('should archive all user subscriptions when user is deactivated', async () => { + stubs.Subscriptions.setArchivedByUsername.resolves({ modifiedCount: 5 }); + + await setUserActiveStatus(userId, false); + + expect(stubs.Subscriptions.setArchivedByUsername.calledOnce).to.be.true; + expect(stubs.Subscriptions.setArchivedByUsername.calledWith(username, true)).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; + }); + + it('should not notify if no subscriptions were modified', async () => { + stubs.Subscriptions.setArchivedByUsername.resolves({ modifiedCount: 0 }); + + await setUserActiveStatus(userId, false); + + expect(stubs.Subscriptions.setArchivedByUsername.calledOnce).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; + }); + }); + + describe('Subscription unarchiving on reactivation', () => { + beforeEach(() => { + stubs.Users.findOneById.resolves({ _id: userId, username, active: false }); + }); + + it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { + const subscriptions = [{ rid: 'room1' }, { rid: 'room2' }, { rid: 'room3' }]; + const archivedRooms = [{ _id: 'room2' }]; + + stubs.Subscriptions.findByUserId.returns({ + toArray: sinon.stub().resolves(subscriptions), + }); + stubs.Rooms.findArchivedByRoomIds.returns({ + toArray: sinon.stub().resolves(archivedRooms), + }); + stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 2 }); + + await setUserActiveStatus(userId, true); + + expect(stubs.Subscriptions.findByUserId.calledWith(userId, { projection: { rid: 1 } })).to.be.true; + expect(stubs.Rooms.findArchivedByRoomIds.calledWith(['room1', 'room2', 'room3'], { projection: { _id: 1 } })).to.be.true; + expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledWith(username, ['room2'])).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; + }); + + it('should unarchive all subscriptions if no rooms are archived', async () => { + const subscriptions = [{ rid: 'room1' }, { rid: 'room2' }]; + + stubs.Subscriptions.findByUserId.returns({ + toArray: sinon.stub().resolves(subscriptions), + }); + stubs.Rooms.findArchivedByRoomIds.returns({ + toArray: sinon.stub().resolves([]), + }); + stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 2 }); + + await setUserActiveStatus(userId, true); + + expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledWith(username, [])).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; + }); + + it('should not notify if no subscriptions were modified during reactivation', async () => { + stubs.Subscriptions.findByUserId.returns({ + toArray: sinon.stub().resolves([{ rid: 'room1' }]), + }); + stubs.Rooms.findArchivedByRoomIds.returns({ + toArray: sinon.stub().resolves([]), + }); + stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 0 }); + + await setUserActiveStatus(userId, true); + + expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledOnce).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; + }); + }); + + describe('User without username', () => { + it('should not archive subscriptions for user without username', async () => { + stubs.Users.findOneById.resolves({ _id: userId, username: undefined, active: true }); + + await setUserActiveStatus(userId, false); + + expect(stubs.Subscriptions.setArchivedByUsername.called).to.be.false; + }); + + it('should not unarchive subscriptions for user without username', async () => { + stubs.Users.findOneById.resolves({ _id: userId, username: undefined, active: false }); + + await setUserActiveStatus(userId, true); + + expect(stubs.Subscriptions.findByUserId.called).to.be.false; + expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.called).to.be.false; + }); + }); + + describe('Error handling and validation', () => { + it('should return false if user is not found', async () => { + stubs.Users.findOneById.resolves(null); + + const result = await setUserActiveStatus(userId, false); + + expect(result).to.be.false; + expect(stubs.Subscriptions.setArchivedByUsername.called).to.be.false; + }); + + it('should throw error for federated users', async () => { + stubs.isUserFederated.returns(true); + + try { + await setUserActiveStatus(userId, false); + expect.fail('Should have thrown an error'); + } catch (error: any) { + expect(error.message).to.equal('error-user-is-federated'); + } + }); + }); +}); diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index 2639f6adcb3a7..b297d0163a25f 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -261,6 +261,7 @@ export interface IRoomsModel extends IBaseModel { addImportIds(rid: string, importIds: string[]): Promise; archiveById(rid: string): Promise; unarchiveById(rid: string): Promise; + findArchivedByRoomIds(roomIds: IRoom['_id'][], options?: FindOptions): FindCursor; setNameById(rid: string, name: string, fname: string): Promise; setIncMsgCountAndSetLastMessageUpdateQuery( inc: number, diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index 27a8e2dadc709..ae0a31a452398 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -38,6 +38,8 @@ export interface ISubscriptionsModel extends IBaseModel { countUnarchivedByRoomId(rid: string): Promise; + countUnarchivedByRoomIdAndNotUserId(rid: string, uid: string): Promise; + isUserInRole(uid: IUser['_id'], roleId: IRole['_id'], rid?: IRoom['_id']): Promise; setAsReadByRoomIdAndUserId( @@ -241,6 +243,7 @@ export interface ISubscriptionsModel extends IBaseModel { archiveByRoomId(roomId: string): Promise; unarchiveByRoomId(roomId: string): Promise; + unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; findByRoomIdWhenUsernameExists(rid: string, options?: FindOptions): FindCursor; setCustomFieldsDirectMessagesByUserId(userId: string, fields: Record): Promise; @@ -331,7 +334,6 @@ export interface ISubscriptionsModel extends IBaseModel { countByRoomId(roomId: string, options?: CountDocumentsOptions): Promise; countByUserIdExceptType(userId: string, typeException: ISubscription['t']): Promise; openByRoomIdAndUserId(roomId: string, userId: string): Promise; - countByRoomIdAndNotUserId(rid: string, uid: string): Promise; countByRoomIdWhenUsernameExists(rid: string): Promise; setE2EKeyByUserIdAndRoomId(userId: string, rid: string, key: string): Promise>; countUsersInRoles(roles: IRole['_id'][], rid: IRoom['_id'] | undefined): Promise; diff --git a/packages/models/src/models/Rooms.ts b/packages/models/src/models/Rooms.ts index 9bcac18cf72fe..a81cba82d05fc 100644 --- a/packages/models/src/models/Rooms.ts +++ b/packages/models/src/models/Rooms.ts @@ -1597,6 +1597,15 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.updateOne(query, update); } + findArchivedByRoomIds(roomIds: IRoom['_id'][], options?: FindOptions): FindCursor { + const query: Filter = { + _id: { $in: roomIds }, + archived: true, + }; + + return this.find(query, options || {}); + } + setNameById(_id: IRoom['_id'], name: IRoom['name'], fname: IRoom['fname']): Promise { const query: Filter = { _id }; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index f85013fd4bce4..9480c77b59bfe 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -67,6 +67,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { key: { rid: 1, ls: 1 } }, { key: { 'u._id': 1, 'autotranslate': 1 } }, { key: { 'v._id': 1, 'open': 1 } }, + { key: { rid: 1, archived: 1, ls: 1 } }, ]; } @@ -135,17 +136,6 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.find(query, options); } - countByRoomIdAndNotUserId(rid: string, uid: string): Promise { - const query = { - rid, - 'u._id': { - $ne: uid, - }, - }; - - return this.countDocuments(query); - } - findByLivechatRoomIdAndNotUserId(roomId: string, userId: string, options: FindOptions = {}): FindCursor { const query = { 'rid': roomId, @@ -176,6 +166,17 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.countDocuments(query); } + countUnarchivedByRoomIdAndNotUserId(rid: string, uid: string): Promise { + const query = { + rid, + 'archived': { $ne: true }, + 'u._id': { + $ne: uid, + }, + }; + return this.countDocuments(query); + } + async isUserInRole(uid: IUser['_id'], roleId: IRole['_id'], rid?: IRoom['_id']): Promise { if (rid == null) { return false; @@ -1283,6 +1284,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.findOne( { rid, + archived: { $ne: true }, }, { sort: { @@ -1324,6 +1326,22 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateMany(query, update); } + unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise { + const query: Filter = { + 'u.username': username, + 'rid': { $nin: excludeRoomIds }, + 'archived': true, + }; + + const update: UpdateFilter = { + $set: { + archived: false, + }, + }; + + return this.updateMany(query, update); + } + hideByRoomIdAndUserId(roomId: string, userId: string): Promise { const query = { 'rid': roomId, @@ -1736,8 +1754,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri setArchivedByUsername(username: string, archived: boolean): Promise { const query: Filter = { - t: 'd', - name: username, + 'u.username': username, }; const update: UpdateFilter = { From f24d4ea792ca02c56f6c1ec24384598a94cbf569 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 24 Feb 2026 17:44:45 +0530 Subject: [PATCH 02/18] added changeset Signed-off-by: Abhinav Kumar --- .changeset/hot-lies-divide.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/hot-lies-divide.md diff --git a/.changeset/hot-lies-divide.md b/.changeset/hot-lies-divide.md new file mode 100644 index 0000000000000..89b596fdba55a --- /dev/null +++ b/.changeset/hot-lies-divide.md @@ -0,0 +1,7 @@ +--- +'@rocket.chat/model-typings': patch +'@rocket.chat/models': patch +'@rocket.chat/meteor': patch +--- + +Fixes an issue where messages appeared as unread even when all active users had read them. Read receipts now correctly ignore deactivated users. From f6f0360bdec9c34d17fa2e9946e7f047c3a93c21 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Mon, 2 Mar 2026 20:54:14 +0530 Subject: [PATCH 03/18] unarchive subscription of only active users Signed-off-by: Abhinav Kumar --- .../app/lib/server/functions/unarchiveRoom.ts | 5 +- .../src/models/ISubscriptionsModel.ts | 2 +- packages/models/src/models/Subscriptions.ts | 50 +++++++++++++++---- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts index 699f9c3701b1c..635b976c9bb36 100644 --- a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts +++ b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts @@ -7,8 +7,9 @@ import { notifyOnRoomChangedById, notifyOnSubscriptionChangedByRoomId } from '.. export const unarchiveRoom = async function (rid: string, user: IMessage['u']): Promise { await Rooms.unarchiveById(rid); - const unarchiveResponse = await Subscriptions.unarchiveByRoomId(rid); - if (unarchiveResponse.modifiedCount) { + const hasUnarchived = await Subscriptions.unarchiveByRoomId(rid); + + if (hasUnarchived) { void notifyOnSubscriptionChangedByRoomId(rid); } diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index ae0a31a452398..21acc7200f651 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -242,7 +242,7 @@ export interface ISubscriptionsModel extends IBaseModel { findUnreadByUserId(userId: string): FindCursor; archiveByRoomId(roomId: string): Promise; - unarchiveByRoomId(roomId: string): Promise; + unarchiveByRoomId(roomId: string): Promise; unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; findByRoomIdWhenUsernameExists(rid: string, options?: FindOptions): FindCursor; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 9480c77b59bfe..8651712614419 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1312,18 +1312,48 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateMany(query, update); } - unarchiveByRoomId(roomId: string): Promise { - const query = { rid: roomId }; + async unarchiveByRoomId(roomId: string): Promise { + const hasArchived = await this.col.countDocuments({ rid: roomId, archived: true }, { limit: 1 }); + if (!hasArchived) { + return false; + } - const update: UpdateFilter = { - $set: { - alert: false, - open: true, - archived: false, - }, - }; + await this.col + .aggregate( + [ + { $match: { rid: roomId, archived: true } }, + { $project: { '_id': 1, 'u._id': 1 } }, + { + $lookup: { + from: Users.getCollectionName(), + localField: 'u._id', + foreignField: '_id', + as: '_user', + pipeline: [{ $project: { active: 1 } }], + }, + }, + { $match: { '_user.active': true } }, + { + $project: { + _id: 1, + archived: { $literal: false }, + open: { $literal: true }, + alert: { $literal: false }, + }, + }, + { + $merge: { + into: this.getCollectionName(), + whenMatched: 'merge', + whenNotMatched: 'discard', + }, + }, + ], + { allowDiskUse: true }, + ) + .toArray(); - return this.updateMany(query, update); + return true; } unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise { From 6b40a462e1a9c553621c5c907b17d9b71c5a8821 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Mon, 2 Mar 2026 21:17:38 +0530 Subject: [PATCH 04/18] minor change Signed-off-by: Abhinav Kumar --- .../lib/server/functions/setUserActiveStatus.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index 4727ab5df4b6a..b48b711d4ee1e 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts @@ -19,6 +19,13 @@ import { notifyOnUserChange, } from '../lib/notifyListener'; +async function getArchivedRoomIdsForUser(userId: string): Promise { + const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray(); + const roomIds = subscriptions.map((sub) => sub.rid); + const archivedRooms = await Rooms.findArchivedByRoomIds(roomIds, { projection: { _id: 1 } }).toArray(); + return archivedRooms.map((room) => room._id); +} + async function reactivateDirectConversations(userId: string) { // since both users can be deactivated at the same time, we should just reactivate rooms if both users are active // for that, we need to fetch the direct messages, fetch the users involved and then the ids of rooms we can reactivate @@ -120,12 +127,7 @@ export async function setUserActiveStatus( } if (user.username && active === true && !user.active) { - // When reactivating, only unarchive subscriptions to non-archived rooms - const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray(); - const roomIds = subscriptions.map((sub) => sub.rid); - const archivedRooms = await Rooms.findArchivedByRoomIds(roomIds, { projection: { _id: 1 } }).toArray(); - const archivedRoomIds = archivedRooms.map((room) => room._id); - + const archivedRoomIds = await getArchivedRoomIdsForUser(userId); const { modifiedCount } = await Subscriptions.unarchiveByUsernameExcludingRoomIds(user.username, archivedRoomIds); if (modifiedCount) { void notifyOnSubscriptionChangedByUserId(userId); From d081e06057f776f1a0a81556547d87c5db37ca85 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 10 Mar 2026 04:17:38 +0530 Subject: [PATCH 05/18] test fix Signed-off-by: Abhinav Kumar --- .../read-receipts-deactivated-users.spec.ts | 29 +++++---- apps/meteor/tests/e2e/read-receipts.spec.ts | 2 +- apps/meteor/tests/e2e/utils/user-helpers.ts | 60 ++++++++++++++++++- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts index 07aaf8e9e1c40..390df5a6a3d7e 100644 --- a/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts +++ b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts @@ -2,10 +2,13 @@ import type { Page } from '@playwright/test'; import { IS_EE } from './config/constants'; import { createAuxContext } from './fixtures/createAuxContext'; +import type { IUserState } from './fixtures/userStates'; import { Users } from './fixtures/userStates'; import { HomeChannel } from './page-objects'; import { createTargetChannel, deleteChannel, setSettingValueById } from './utils'; import { expect, test } from './utils/test'; +import type { ITestUser } from './utils/user-helpers'; +import { createTestUser, loginTestUser } from './utils/user-helpers'; test.use({ storageState: Users.admin.state }); @@ -14,11 +17,20 @@ test.describe.serial('read-receipts-deactivated-users', () => { let targetChannel: string; let user1Context: { page: Page; poHomeChannel: HomeChannel } | undefined; let user2Context: { page: Page; poHomeChannel: HomeChannel } | undefined; + let testUser1: ITestUser; + let testUser2: ITestUser; + let testUser1State: IUserState; + let testUser2State: IUserState; test.skip(!IS_EE, 'Enterprise Only'); test.beforeAll(async ({ api }) => { - targetChannel = await createTargetChannel(api, { members: ['user1', 'user2'] }); + testUser1 = await createTestUser(api); + testUser2 = await createTestUser(api); + + [testUser1State, testUser2State] = await Promise.all([loginTestUser(api, testUser1), loginTestUser(api, testUser2)]); + + targetChannel = await createTargetChannel(api, { members: [testUser1.data.username, testUser2.data.username] }); await setSettingValueById(api, 'Message_Read_Receipt_Enabled', true); await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', true); }); @@ -27,10 +39,8 @@ test.describe.serial('read-receipts-deactivated-users', () => { await setSettingValueById(api, 'Message_Read_Receipt_Enabled', false); await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', false); - await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: true }); - await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: true }); - await deleteChannel(api, targetChannel); + await Promise.all([testUser1?.delete(), testUser2?.delete()]); }); test.beforeEach(async ({ page }) => { @@ -50,11 +60,11 @@ test.describe.serial('read-receipts-deactivated-users', () => { }); test('should correctly handle read receipts as users are deactivated', async ({ browser, api, page }) => { - const { page: page1 } = await createAuxContext(browser, Users.user1); + const { page: page1 } = await createAuxContext(browser, testUser1State); const user1Ctx = { page: page1, poHomeChannel: new HomeChannel(page1) }; user1Context = user1Ctx; - const { page: page2 } = await createAuxContext(browser, Users.user2); + const { page: page2 } = await createAuxContext(browser, testUser2State); const user2Ctx = { page: page2, poHomeChannel: new HomeChannel(page2) }; user2Context = user2Ctx; @@ -77,7 +87,7 @@ test.describe.serial('read-receipts-deactivated-users', () => { }); await test.step('when some users are deactivated', async () => { - await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: false }); + await api.post('/users.setActiveStatus', { userId: testUser1.data._id, activeStatus: false }); await poHomeChannel.content.sendMessage('Message 2: User1 deactivated, two active users'); @@ -92,7 +102,7 @@ test.describe.serial('read-receipts-deactivated-users', () => { }); await test.step('when only one user remains active (user alone in room)', async () => { - await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: false }); + await api.post('/users.setActiveStatus', { userId: testUser2.data._id, activeStatus: false }); await poHomeChannel.content.sendMessage('Message 3: Only admin active'); @@ -103,8 +113,5 @@ test.describe.serial('read-receipts-deactivated-users', () => { await expect(page.getByRole('dialog').getByRole('listitem')).toHaveCount(1); await page.getByRole('button', { name: 'Close' }).click(); }); - - await api.post('/users.setActiveStatus', { userId: Users.user1.data._id, activeStatus: true }); - await api.post('/users.setActiveStatus', { userId: Users.user2.data._id, activeStatus: true }); }); }); diff --git a/apps/meteor/tests/e2e/read-receipts.spec.ts b/apps/meteor/tests/e2e/read-receipts.spec.ts index 5594484ff74d4..fbd091860a98d 100644 --- a/apps/meteor/tests/e2e/read-receipts.spec.ts +++ b/apps/meteor/tests/e2e/read-receipts.spec.ts @@ -30,7 +30,7 @@ test.describe.serial('read-receipts', () => { await poHomeChannel.navbar.openChat(targetChannel); await poHomeChannel.content.sendMessage('hello world'); await poHomeChannel.content.openLastMessageMenu(); - expect(page.locator('role=menuitem[name="Read receipts"]')).not.toBeVisible; + await expect(page.locator('role=menuitem[name="Read receipts"]')).not.toBeVisible(); }); }); diff --git a/apps/meteor/tests/e2e/utils/user-helpers.ts b/apps/meteor/tests/e2e/utils/user-helpers.ts index 267fbc6b6b9a9..10e5e4db32ce6 100644 --- a/apps/meteor/tests/e2e/utils/user-helpers.ts +++ b/apps/meteor/tests/e2e/utils/user-helpers.ts @@ -3,7 +3,8 @@ import type { APIResponse } from '@playwright/test'; import type { IUser } from '@rocket.chat/core-typings'; import type { BaseTest } from './test'; -import { DEFAULT_USER_CREDENTIALS } from '../config/constants'; +import { BASE_URL, DEFAULT_USER_CREDENTIALS } from '../config/constants'; +import type { IUserState } from '../fixtures/userStates'; export interface ICreateUserOptions { username?: string; @@ -62,6 +63,63 @@ export async function createTestUser(api: BaseTest['api'], options: ICreateUserO }; } +/** + * Logs in a test user via the REST API and returns an IUserState + * suitable for use with createAuxContext. + * + * Use this instead of the pre-baked Users.userN fixtures whenever the test + * will deactivate (or otherwise invalidate the session of) the user, so that + * shared fixture tokens are never corrupted. + */ +export async function loginTestUser(api: BaseTest['api'], user: ITestUser): Promise { + const response = await api.post('/login', { + username: user.data.username, + password: DEFAULT_USER_CREDENTIALS.password, + }); + const { + data: { userId, authToken }, + } = await response.json(); + + const expires = new Date(); + expires.setFullYear(expires.getFullYear() + 1); + + return { + data: { + _id: userId, + username: user.data.username, + loginToken: authToken, + loginExpire: expires, + hashedToken: '', + }, + state: { + cookies: [ + { sameSite: 'Lax', name: 'rc_uid', value: userId, domain: 'localhost', path: '/', expires: -1, httpOnly: false, secure: false }, + { + sameSite: 'Lax', + name: 'rc_token', + value: authToken, + domain: 'localhost', + path: '/', + expires: -1, + httpOnly: false, + secure: false, + }, + ], + origins: [ + { + origin: BASE_URL, + localStorage: [ + { name: 'userLanguage', value: 'en-US' }, + { name: 'Meteor.loginToken', value: authToken }, + { name: 'Meteor.loginTokenExpires', value: expires.toISOString() }, + { name: 'Meteor.userId', value: userId }, + ], + }, + ], + }, + }; +} + /** * Creates multiple test users at once */ From 5d4e979cffba7f6e563f2a3825994fc9f95f93ce Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 10 Mar 2026 22:00:01 +0530 Subject: [PATCH 06/18] requested changes Signed-off-by: Abhinav Kumar --- .../server/functions/setUserActiveStatus.ts | 26 +++------ .../functions/setUserActiveStatus.spec.ts | 54 ++----------------- .../model-typings/src/models/IRoomsModel.ts | 1 - .../src/models/ISubscriptionsModel.ts | 2 +- packages/models/src/models/Rooms.ts | 9 ---- packages/models/src/models/Subscriptions.ts | 37 ++++++++----- 6 files changed, 36 insertions(+), 93 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index b48b711d4ee1e..d0fef28912090 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts @@ -19,13 +19,6 @@ import { notifyOnUserChange, } from '../lib/notifyListener'; -async function getArchivedRoomIdsForUser(userId: string): Promise { - const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray(); - const roomIds = subscriptions.map((sub) => sub.rid); - const archivedRooms = await Rooms.findArchivedByRoomIds(roomIds, { projection: { _id: 1 } }).toArray(); - return archivedRooms.map((room) => room._id); -} - async function reactivateDirectConversations(userId: string) { // since both users can be deactivated at the same time, we should just reactivate rooms if both users are active // for that, we need to fetch the direct messages, fetch the users involved and then the ids of rooms we can reactivate @@ -119,17 +112,14 @@ export async function setUserActiveStatus( await callbacks.run('afterDeactivateUser', user); } - if (user.username && active === false) { - const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, true); - if (modifiedCount) { - void notifyOnSubscriptionChangedByUserId(userId); - } - } - - if (user.username && active === true && !user.active) { - const archivedRoomIds = await getArchivedRoomIdsForUser(userId); - const { modifiedCount } = await Subscriptions.unarchiveByUsernameExcludingRoomIds(user.username, archivedRoomIds); - if (modifiedCount) { + if (user.username) { + if (active === false) { + const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, true); + if (modifiedCount) { + void notifyOnSubscriptionChangedByUserId(userId); + } + } else if (active === true && !user.active) { + await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(userId); void notifyOnSubscriptionChangedByUserId(userId); } } diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts index b0e47efb47e55..7e7c39fe14fba 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -18,11 +18,9 @@ describe('setUserActiveStatus', () => { }, Subscriptions: { setArchivedByUsername: sinon.stub(), - findByUserId: sinon.stub(), - unarchiveByUsernameExcludingRoomIds: sinon.stub(), + unarchiveByUserIdExceptForArchivedRooms: sinon.stub(), }, Rooms: { - findArchivedByRoomIds: sinon.stub(), setDmReadOnlyByUserId: sinon.stub(), getDirectConversationsByUserId: sinon.stub(), }, @@ -144,56 +142,13 @@ describe('setUserActiveStatus', () => { }); it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { - const subscriptions = [{ rid: 'room1' }, { rid: 'room2' }, { rid: 'room3' }]; - const archivedRooms = [{ _id: 'room2' }]; - - stubs.Subscriptions.findByUserId.returns({ - toArray: sinon.stub().resolves(subscriptions), - }); - stubs.Rooms.findArchivedByRoomIds.returns({ - toArray: sinon.stub().resolves(archivedRooms), - }); - stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 2 }); - - await setUserActiveStatus(userId, true); - - expect(stubs.Subscriptions.findByUserId.calledWith(userId, { projection: { rid: 1 } })).to.be.true; - expect(stubs.Rooms.findArchivedByRoomIds.calledWith(['room1', 'room2', 'room3'], { projection: { _id: 1 } })).to.be.true; - expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledWith(username, ['room2'])).to.be.true; - expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; - }); - - it('should unarchive all subscriptions if no rooms are archived', async () => { - const subscriptions = [{ rid: 'room1' }, { rid: 'room2' }]; - - stubs.Subscriptions.findByUserId.returns({ - toArray: sinon.stub().resolves(subscriptions), - }); - stubs.Rooms.findArchivedByRoomIds.returns({ - toArray: sinon.stub().resolves([]), - }); - stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 2 }); + stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.resolves(); await setUserActiveStatus(userId, true); - expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledWith(username, [])).to.be.true; + expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.calledOnceWith(userId)).to.be.true; expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; }); - - it('should not notify if no subscriptions were modified during reactivation', async () => { - stubs.Subscriptions.findByUserId.returns({ - toArray: sinon.stub().resolves([{ rid: 'room1' }]), - }); - stubs.Rooms.findArchivedByRoomIds.returns({ - toArray: sinon.stub().resolves([]), - }); - stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.resolves({ modifiedCount: 0 }); - - await setUserActiveStatus(userId, true); - - expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.calledOnce).to.be.true; - expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; - }); }); describe('User without username', () => { @@ -210,8 +165,7 @@ describe('setUserActiveStatus', () => { await setUserActiveStatus(userId, true); - expect(stubs.Subscriptions.findByUserId.called).to.be.false; - expect(stubs.Subscriptions.unarchiveByUsernameExcludingRoomIds.called).to.be.false; + expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; }); }); diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index b297d0163a25f..2639f6adcb3a7 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -261,7 +261,6 @@ export interface IRoomsModel extends IBaseModel { addImportIds(rid: string, importIds: string[]): Promise; archiveById(rid: string): Promise; unarchiveById(rid: string): Promise; - findArchivedByRoomIds(roomIds: IRoom['_id'][], options?: FindOptions): FindCursor; setNameById(rid: string, name: string, fname: string): Promise; setIncMsgCountAndSetLastMessageUpdateQuery( inc: number, diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index 21acc7200f651..52fe464ab8991 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -243,7 +243,7 @@ export interface ISubscriptionsModel extends IBaseModel { archiveByRoomId(roomId: string): Promise; unarchiveByRoomId(roomId: string): Promise; - unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise; + unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; findByRoomIdWhenUsernameExists(rid: string, options?: FindOptions): FindCursor; setCustomFieldsDirectMessagesByUserId(userId: string, fields: Record): Promise; diff --git a/packages/models/src/models/Rooms.ts b/packages/models/src/models/Rooms.ts index a81cba82d05fc..9bcac18cf72fe 100644 --- a/packages/models/src/models/Rooms.ts +++ b/packages/models/src/models/Rooms.ts @@ -1597,15 +1597,6 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.updateOne(query, update); } - findArchivedByRoomIds(roomIds: IRoom['_id'][], options?: FindOptions): FindCursor { - const query: Filter = { - _id: { $in: roomIds }, - archived: true, - }; - - return this.find(query, options || {}); - } - setNameById(_id: IRoom['_id'], name: IRoom['name'], fname: IRoom['fname']): Promise { const query: Filter = { _id }; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 8651712614419..bad19ce9e3431 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1356,20 +1356,29 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return true; } - unarchiveByUsernameExcludingRoomIds(username: string, excludeRoomIds: string[]): Promise { - const query: Filter = { - 'u.username': username, - 'rid': { $nin: excludeRoomIds }, - 'archived': true, - }; - - const update: UpdateFilter = { - $set: { - archived: false, - }, - }; - - return this.updateMany(query, update); + async unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise { + await this.col + .aggregate([ + { $match: { 'u._id': userId, 'archived': true } }, + { + $lookup: { + from: Rooms.getCollectionName(), + localField: 'rid', + foreignField: '_id', + as: 'room', + }, + }, + { $match: { 'room.0.archived': { $ne: true } } }, + { + $merge: { + on: '_id', + into: this.getCollectionName(), + whenMatched: [{ $set: { archived: false } }], + whenNotMatched: 'discard', + }, + }, + ]) + .toArray(); } hideByRoomIdAndUserId(roomId: string, userId: string): Promise { From d1ce974e67a2f52dacc2411c1fcdb34654354bfa Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Wed, 11 Mar 2026 23:51:24 +0530 Subject: [PATCH 07/18] requested changes Signed-off-by: Abhinav Kumar --- .../server/functions/setUserActiveStatus.ts | 6 ++++-- .../functions/setUserActiveStatus.spec.ts | 14 +++++++++++++ .../src/models/ISubscriptionsModel.ts | 1 + packages/models/src/models/Subscriptions.ts | 20 +++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index d0fef28912090..cd5f2ed410aa9 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts @@ -119,8 +119,10 @@ export async function setUserActiveStatus( void notifyOnSubscriptionChangedByUserId(userId); } } else if (active === true && !user.active) { - await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(userId); - void notifyOnSubscriptionChangedByUserId(userId); + if (await Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId)) { + await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(userId); + void notifyOnSubscriptionChangedByUserId(userId); + } } } diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts index 7e7c39fe14fba..361a4ffa519e9 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -18,6 +18,7 @@ describe('setUserActiveStatus', () => { }, Subscriptions: { setArchivedByUsername: sinon.stub(), + hasArchivedSubscriptionsInNonArchivedRoomsByUserId: sinon.stub(), unarchiveByUserIdExceptForArchivedRooms: sinon.stub(), }, Rooms: { @@ -142,13 +143,25 @@ describe('setUserActiveStatus', () => { }); it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { + stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(true); stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.resolves(); await setUserActiveStatus(userId, true); + expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.calledOnceWith(userId)).to.be.true; expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; }); + + it('should not unarchive or notify if no archived subscriptions in non-archived rooms', async () => { + stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(false); + + await setUserActiveStatus(userId, true); + + expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; + expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; + expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; + }); }); describe('User without username', () => { @@ -165,6 +178,7 @@ describe('setUserActiveStatus', () => { await setUserActiveStatus(userId, true); + expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.called).to.be.false; expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; }); }); diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index 52fe464ab8991..72d3048b69d52 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -243,6 +243,7 @@ export interface ISubscriptionsModel extends IBaseModel { archiveByRoomId(roomId: string): Promise; unarchiveByRoomId(roomId: string): Promise; + hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise; unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; findByRoomIdWhenUsernameExists(rid: string, options?: FindOptions): FindCursor; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index bad19ce9e3431..0bc4d123e85b3 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1356,6 +1356,26 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return true; } + async hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise { + const result = await this.col + .aggregate([ + { $match: { 'u._id': userId, 'archived': true } }, + { + $lookup: { + from: Rooms.getCollectionName(), + localField: 'rid', + foreignField: '_id', + as: 'room', + }, + }, + { $match: { 'room.0.archived': { $ne: true } } }, + { $limit: 1 }, + ]) + .toArray(); + + return result.length > 0; + } + async unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise { await this.col .aggregate([ From 39dfaffe7a7558350433dbedfce54dc0fa6e6e53 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 12 Mar 2026 00:04:15 +0530 Subject: [PATCH 08/18] minor improvement Signed-off-by: Abhinav Kumar --- packages/models/src/models/Subscriptions.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 0bc4d123e85b3..4985c099eef3d 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1366,6 +1366,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri localField: 'rid', foreignField: '_id', as: 'room', + pipeline: [{ $project: { archived: 1 } }], }, }, { $match: { 'room.0.archived': { $ne: true } } }, @@ -1386,6 +1387,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri localField: 'rid', foreignField: '_id', as: 'room', + pipeline: [{ $project: { archived: 1 } }], }, }, { $match: { 'room.0.archived': { $ne: true } } }, From a8ae48d6b5ba2f026243dd0ea196bca4d533cd1a Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 12 Mar 2026 01:18:59 +0530 Subject: [PATCH 09/18] requested changes Signed-off-by: Abhinav Kumar --- .../server/functions/setUserActiveStatus.ts | 23 +---- .../app/lib/server/functions/unarchiveRoom.ts | 5 +- .../app/lib/server/hooks/afterUserActions.ts | 23 +++++ apps/meteor/app/lib/server/index.ts | 1 + .../functions/setUserActiveStatus.spec.ts | 78 +--------------- .../lib/server/hooks/afterUserActions.spec.ts | 88 +++++++++++++++++++ .../src/models/ISubscriptionsModel.ts | 5 +- packages/models/src/models/Subscriptions.ts | 28 +++--- 8 files changed, 134 insertions(+), 117 deletions(-) create mode 100644 apps/meteor/app/lib/server/hooks/afterUserActions.ts create mode 100644 apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index cd5f2ed410aa9..25a1a06f7bb8d 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts @@ -1,6 +1,6 @@ import type { IUser, IUserEmail } from '@rocket.chat/core-typings'; import { isUserFederated, isDirectMessageRoom } from '@rocket.chat/core-typings'; -import { Rooms, Users, Subscriptions } from '@rocket.chat/models'; +import { Rooms, Users } from '@rocket.chat/models'; import { Accounts } from 'meteor/accounts-base'; import { check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; @@ -12,12 +12,7 @@ import { relinquishRoomOwnerships } from './relinquishRoomOwnerships'; import { callbacks } from '../../../../server/lib/callbacks'; import * as Mailer from '../../../mailer/server/api'; import { settings } from '../../../settings/server'; -import { - notifyOnRoomChangedById, - notifyOnRoomChangedByUserDM, - notifyOnSubscriptionChangedByUserId, - notifyOnUserChange, -} from '../lib/notifyListener'; +import { notifyOnRoomChangedById, notifyOnRoomChangedByUserDM, notifyOnUserChange } from '../lib/notifyListener'; async function reactivateDirectConversations(userId: string) { // since both users can be deactivated at the same time, we should just reactivate rooms if both users are active @@ -112,20 +107,6 @@ export async function setUserActiveStatus( await callbacks.run('afterDeactivateUser', user); } - if (user.username) { - if (active === false) { - const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, true); - if (modifiedCount) { - void notifyOnSubscriptionChangedByUserId(userId); - } - } else if (active === true && !user.active) { - if (await Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId)) { - await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(userId); - void notifyOnSubscriptionChangedByUserId(userId); - } - } - } - if (active === false) { await Users.unsetLoginTokens(userId); await Rooms.setDmReadOnlyByUserId(userId, undefined, true, false); diff --git a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts index 635b976c9bb36..44a74479560d4 100644 --- a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts +++ b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts @@ -7,9 +7,8 @@ import { notifyOnRoomChangedById, notifyOnSubscriptionChangedByRoomId } from '.. export const unarchiveRoom = async function (rid: string, user: IMessage['u']): Promise { await Rooms.unarchiveById(rid); - const hasUnarchived = await Subscriptions.unarchiveByRoomId(rid); - - if (hasUnarchived) { + if (await Subscriptions.hasArchivedSubscriptionsByRoomId(rid)) { + await Subscriptions.unarchiveByRoomId(rid); void notifyOnSubscriptionChangedByRoomId(rid); } diff --git a/apps/meteor/app/lib/server/hooks/afterUserActions.ts b/apps/meteor/app/lib/server/hooks/afterUserActions.ts new file mode 100644 index 0000000000000..c56aae8ff4732 --- /dev/null +++ b/apps/meteor/app/lib/server/hooks/afterUserActions.ts @@ -0,0 +1,23 @@ +import type { IUser } from '@rocket.chat/core-typings'; +import { Subscriptions } from '@rocket.chat/models'; + +import { callbacks } from '../../../../server/lib/callbacks'; +import { notifyOnSubscriptionChangedByUserId } from '../lib/notifyListener'; + +const handleDeactivateUser = async (user: IUser): Promise => { + const { modifiedCount } = await Subscriptions.setArchivedByUserId(user._id, true); + if (modifiedCount) { + void notifyOnSubscriptionChangedByUserId(user._id); + } +}; + +const handleActivateUser = async (user: IUser): Promise => { + if (await Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId(user._id)) { + await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(user._id); + void notifyOnSubscriptionChangedByUserId(user._id); + } +}; + +callbacks.add('afterDeactivateUser', handleDeactivateUser, callbacks.priority.LOW, 'subscription-archive-on-deactivate'); + +callbacks.add('afterActivateUser', handleActivateUser, callbacks.priority.LOW, 'subscription-unarchive-on-activate'); diff --git a/apps/meteor/app/lib/server/index.ts b/apps/meteor/app/lib/server/index.ts index 9fb7c341b83e4..5705c22816eb1 100644 --- a/apps/meteor/app/lib/server/index.ts +++ b/apps/meteor/app/lib/server/index.ts @@ -42,5 +42,6 @@ import './methods/unarchiveRoom'; import './methods/unblockUser'; import './methods/updateMessage'; import './methods/checkFederationConfiguration'; +import './hooks/afterUserActions'; export * from './lib'; diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts index 361a4ffa519e9..756e60fae6c64 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -16,11 +16,6 @@ describe('setUserActiveStatus', () => { unsetReason: sinon.stub(), findActiveByUserIds: sinon.stub(), }, - Subscriptions: { - setArchivedByUsername: sinon.stub(), - hasArchivedSubscriptionsInNonArchivedRoomsByUserId: sinon.stub(), - unarchiveByUserIdExceptForArchivedRooms: sinon.stub(), - }, Rooms: { setDmReadOnlyByUserId: sinon.stub(), getDirectConversationsByUserId: sinon.stub(), @@ -33,7 +28,6 @@ describe('setUserActiveStatus', () => { get: sinon.stub(), }, notifyOnUserChange: sinon.stub(), - notifyOnSubscriptionChangedByUserId: sinon.stub(), notifyOnRoomChangedByUserDM: sinon.stub(), notifyOnRoomChangedById: sinon.stub(), getSubscribedRoomsForUserWithDetails: sinon.stub(), @@ -60,7 +54,7 @@ describe('setUserActiveStatus', () => { 'meteor/meteor': { Meteor: { Error } }, 'meteor/accounts-base': { Accounts: stubs.Accounts }, '@rocket.chat/core-typings': { isUserFederated: stubs.isUserFederated, isDirectMessageRoom: sinon.stub() }, - '@rocket.chat/models': { Users: stubs.Users, Subscriptions: stubs.Subscriptions, Rooms: stubs.Rooms }, + '@rocket.chat/models': { Users: stubs.Users, Rooms: stubs.Rooms }, './closeOmnichannelConversations': { closeOmnichannelConversations: stubs.closeOmnichannelConversations }, './getRoomsWithSingleOwner': { shouldRemoveOrChangeOwner: stubs.shouldRemoveOrChangeOwner, @@ -74,7 +68,6 @@ describe('setUserActiveStatus', () => { '../lib/notifyListener': { notifyOnRoomChangedById: stubs.notifyOnRoomChangedById, notifyOnRoomChangedByUserDM: stubs.notifyOnRoomChangedByUserDM, - notifyOnSubscriptionChangedByUserId: stubs.notifyOnSubscriptionChangedByUserId, notifyOnUserChange: stubs.notifyOnUserChange, }, }); @@ -99,7 +92,6 @@ describe('setUserActiveStatus', () => { stubs.notifyOnRoomChangedById.returns(undefined); stubs.notifyOnUserChange.returns(undefined); stubs.notifyOnRoomChangedByUserDM.returns(undefined); - stubs.notifyOnSubscriptionChangedByUserId.returns(undefined); }); afterEach(() => { @@ -116,73 +108,6 @@ describe('setUserActiveStatus', () => { }); }); - describe('Subscription archiving on deactivation', () => { - it('should archive all user subscriptions when user is deactivated', async () => { - stubs.Subscriptions.setArchivedByUsername.resolves({ modifiedCount: 5 }); - - await setUserActiveStatus(userId, false); - - expect(stubs.Subscriptions.setArchivedByUsername.calledOnce).to.be.true; - expect(stubs.Subscriptions.setArchivedByUsername.calledWith(username, true)).to.be.true; - expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; - }); - - it('should not notify if no subscriptions were modified', async () => { - stubs.Subscriptions.setArchivedByUsername.resolves({ modifiedCount: 0 }); - - await setUserActiveStatus(userId, false); - - expect(stubs.Subscriptions.setArchivedByUsername.calledOnce).to.be.true; - expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; - }); - }); - - describe('Subscription unarchiving on reactivation', () => { - beforeEach(() => { - stubs.Users.findOneById.resolves({ _id: userId, username, active: false }); - }); - - it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { - stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(true); - stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.resolves(); - - await setUserActiveStatus(userId, true); - - expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; - expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.calledOnceWith(userId)).to.be.true; - expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; - }); - - it('should not unarchive or notify if no archived subscriptions in non-archived rooms', async () => { - stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(false); - - await setUserActiveStatus(userId, true); - - expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; - expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; - expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; - }); - }); - - describe('User without username', () => { - it('should not archive subscriptions for user without username', async () => { - stubs.Users.findOneById.resolves({ _id: userId, username: undefined, active: true }); - - await setUserActiveStatus(userId, false); - - expect(stubs.Subscriptions.setArchivedByUsername.called).to.be.false; - }); - - it('should not unarchive subscriptions for user without username', async () => { - stubs.Users.findOneById.resolves({ _id: userId, username: undefined, active: false }); - - await setUserActiveStatus(userId, true); - - expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.called).to.be.false; - expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; - }); - }); - describe('Error handling and validation', () => { it('should return false if user is not found', async () => { stubs.Users.findOneById.resolves(null); @@ -190,7 +115,6 @@ describe('setUserActiveStatus', () => { const result = await setUserActiveStatus(userId, false); expect(result).to.be.false; - expect(stubs.Subscriptions.setArchivedByUsername.called).to.be.false; }); it('should throw error for federated users', async () => { diff --git a/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts b/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts new file mode 100644 index 0000000000000..db70fb35af64b --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts @@ -0,0 +1,88 @@ +import { expect } from 'chai'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +describe('afterUserActions hooks', () => { + const userId = 'test-user-id'; + + const stubs = { + Subscriptions: { + setArchivedByUserId: sinon.stub(), + hasArchivedSubscriptionsInNonArchivedRoomsByUserId: sinon.stub(), + unarchiveByUserIdExceptForArchivedRooms: sinon.stub(), + }, + callbacks: { + add: sinon.stub(), + priority: { LOW: 1 }, + }, + notifyOnSubscriptionChangedByUserId: sinon.stub(), + }; + + let handleDeactivateUser: (user: any) => Promise; + let handleActivateUser: (user: any) => Promise; + + stubs.callbacks.add.callsFake((event: string, handler: any) => { + if (event === 'afterDeactivateUser') { + handleDeactivateUser = handler; + } + if (event === 'afterActivateUser') { + handleActivateUser = handler; + } + }); + + proxyquire.noCallThru().load('../../../../../../app/lib/server/hooks/afterUserActions', { + '@rocket.chat/models': { Subscriptions: stubs.Subscriptions }, + '../../../../server/lib/callbacks': { callbacks: stubs.callbacks }, + '../lib/notifyListener': { notifyOnSubscriptionChangedByUserId: stubs.notifyOnSubscriptionChangedByUserId }, + }); + + afterEach(() => { + stubs.Subscriptions.setArchivedByUserId.reset(); + stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.reset(); + stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.reset(); + stubs.notifyOnSubscriptionChangedByUserId.reset(); + }); + + describe('afterDeactivateUser — subscription archiving', () => { + it('should archive all user subscriptions when user is deactivated', async () => { + stubs.Subscriptions.setArchivedByUserId.resolves({ modifiedCount: 5 }); + + await handleDeactivateUser({ _id: userId }); + + expect(stubs.Subscriptions.setArchivedByUserId.calledOnceWith(userId, true)).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; + }); + + it('should not notify if no subscriptions were modified', async () => { + stubs.Subscriptions.setArchivedByUserId.resolves({ modifiedCount: 0 }); + + await handleDeactivateUser({ _id: userId }); + + expect(stubs.Subscriptions.setArchivedByUserId.calledOnce).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; + }); + }); + + describe('afterActivateUser — subscription unarchiving', () => { + it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { + stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(true); + stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.resolves(); + + await handleActivateUser({ _id: userId }); + + expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; + expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.calledOnceWith(userId)).to.be.true; + expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; + }); + + it('should not unarchive or notify if no archived subscriptions in non-archived rooms', async () => { + stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(false); + + await handleActivateUser({ _id: userId }); + + expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; + expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; + expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; + }); + }); +}); diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index 72d3048b69d52..e8c6146e26f11 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -242,7 +242,8 @@ export interface ISubscriptionsModel extends IBaseModel { findUnreadByUserId(userId: string): FindCursor; archiveByRoomId(roomId: string): Promise; - unarchiveByRoomId(roomId: string): Promise; + hasArchivedSubscriptionsByRoomId(roomId: string): Promise; + unarchiveByRoomId(roomId: string): Promise; hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise; unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; @@ -286,7 +287,7 @@ export interface ISubscriptionsModel extends IBaseModel { removeRoleById(_id: string, role: string): Promise; updateDirectFNameByName(name: string, fname: string): Promise; - setArchivedByUsername(username: string, archived: boolean): Promise; + setArchivedByUserId(userId: string, archived: boolean): Promise; updateUserHighlights(userId: string, userHighlights: any): Promise; updateNotificationUserPreferences( userId: string, diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 4985c099eef3d..98c520ee4e5eb 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1312,12 +1312,11 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateMany(query, update); } - async unarchiveByRoomId(roomId: string): Promise { - const hasArchived = await this.col.countDocuments({ rid: roomId, archived: true }, { limit: 1 }); - if (!hasArchived) { - return false; - } + async hasArchivedSubscriptionsByRoomId(roomId: string): Promise { + return !!(await this.col.findOne({ rid: roomId, archived: true }, { projection: { _id: 1 } })); + } + async unarchiveByRoomId(roomId: string): Promise { await this.col .aggregate( [ @@ -1325,14 +1324,17 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { $project: { '_id': 1, 'u._id': 1 } }, { $lookup: { - from: Users.getCollectionName(), - localField: 'u._id', - foreignField: '_id', + from: 'users', + let: { userId: '$u._id' }, + pipeline: [ + { $match: { $expr: { $and: [{ $eq: ['$_id', '$$userId'] }, { $eq: ['$active', true] }] } } }, + { $limit: 1 }, + { $project: { _id: 1 } }, + ], as: '_user', - pipeline: [{ $project: { active: 1 } }], }, }, - { $match: { '_user.active': true } }, + { $match: { '_user.0': { $exists: true } } }, { $project: { _id: 1, @@ -1352,8 +1354,6 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { allowDiskUse: true }, ) .toArray(); - - return true; } async hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise { @@ -1813,9 +1813,9 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateOne(query, update); } - setArchivedByUsername(username: string, archived: boolean): Promise { + setArchivedByUserId(userId: string, archived: boolean): Promise { const query: Filter = { - 'u.username': username, + 'u._id': userId, }; const update: UpdateFilter = { From 2a3559f8b8592205d8c4fb471f2d59b1450059f1 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Fri, 13 Mar 2026 17:28:28 +0530 Subject: [PATCH 10/18] requested changes Signed-off-by: Abhinav Kumar --- .../src/models/ISubscriptionsModel.ts | 2 +- packages/models/src/models/Subscriptions.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index e8c6146e26f11..e1822fc3415ff 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -242,7 +242,7 @@ export interface ISubscriptionsModel extends IBaseModel { findUnreadByUserId(userId: string): FindCursor; archiveByRoomId(roomId: string): Promise; - hasArchivedSubscriptionsByRoomId(roomId: string): Promise; + hasArchivedSubscriptionsByRoomId(roomId: string): Promise; unarchiveByRoomId(roomId: string): Promise; hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise; unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 98c520ee4e5eb..1f7e3f6f36a52 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1312,8 +1312,8 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateMany(query, update); } - async hasArchivedSubscriptionsByRoomId(roomId: string): Promise { - return !!(await this.col.findOne({ rid: roomId, archived: true }, { projection: { _id: 1 } })); + hasArchivedSubscriptionsByRoomId(roomId: string): Promise { + return this.findOne({ rid: roomId, archived: true }, { projection: { _id: 1 } }); } async unarchiveByRoomId(roomId: string): Promise { @@ -1362,14 +1362,14 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { $match: { 'u._id': userId, 'archived': true } }, { $lookup: { - from: Rooms.getCollectionName(), + from: 'rocketchat_room', localField: 'rid', foreignField: '_id', as: 'room', - pipeline: [{ $project: { archived: 1 } }], + pipeline: [{ $project: { archived: 1 } }, { $match: { archived: { $ne: true } } }], }, }, - { $match: { 'room.0.archived': { $ne: true } } }, + { $match: { 'room.0': { $exists: true } } }, { $limit: 1 }, ]) .toArray(); @@ -1383,18 +1383,18 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { $match: { 'u._id': userId, 'archived': true } }, { $lookup: { - from: Rooms.getCollectionName(), + from: 'rocketchat_room', localField: 'rid', foreignField: '_id', as: 'room', - pipeline: [{ $project: { archived: 1 } }], + pipeline: [{ $project: { archived: 1 } }, { $match: { archived: { $ne: true } } }], }, }, - { $match: { 'room.0.archived': { $ne: true } } }, + { $match: { 'room.0': { $exists: true } } }, { $merge: { on: '_id', - into: this.getCollectionName(), + into: 'rocketchat_subscription', whenMatched: [{ $set: { archived: false } }], whenNotMatched: 'discard', }, From 0c6af049f0464ddb78c173b006e078541d77fdc3 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Fri, 13 Mar 2026 19:23:29 +0530 Subject: [PATCH 11/18] requested changes Signed-off-by: Abhinav Kumar --- packages/models/src/models/Subscriptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 1f7e3f6f36a52..9f3c4fed1be74 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1345,7 +1345,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri }, { $merge: { - into: this.getCollectionName(), + into: 'rocketchat_subscription', whenMatched: 'merge', whenNotMatched: 'discard', }, From 7deda6b7db6a8c80d8f1a724c1366392e2883361 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 19 Mar 2026 00:20:10 +0530 Subject: [PATCH 12/18] refactor: apply PR review feedback - Refactor setUserActiveStatus.spec.ts to use sinon sandbox and expand test coverage for administration actions - Optimize aggregations in Subscriptions.ts by updating the match pipeline stage --- .../functions/setUserActiveStatus.spec.ts | 109 +++++++++++------- packages/models/src/models/Subscriptions.ts | 4 +- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts index 756e60fae6c64..509564b647a35 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -6,47 +6,49 @@ describe('setUserActiveStatus', () => { const userId = 'test-user-id'; const username = 'testuser'; + const sandbox = sinon.createSandbox(); + const stubs = { Users: { - findOneById: sinon.stub(), - setUserActive: sinon.stub(), - findOneAdmin: sinon.stub(), - countActiveUsersInRoles: sinon.stub(), - unsetLoginTokens: sinon.stub(), - unsetReason: sinon.stub(), - findActiveByUserIds: sinon.stub(), + findOneById: sandbox.stub(), + setUserActive: sandbox.stub(), + findOneAdmin: sandbox.stub(), + countActiveUsersInRoles: sandbox.stub(), + unsetLoginTokens: sandbox.stub(), + unsetReason: sandbox.stub(), + findActiveByUserIds: sandbox.stub(), }, Rooms: { - setDmReadOnlyByUserId: sinon.stub(), - getDirectConversationsByUserId: sinon.stub(), + setDmReadOnlyByUserId: sandbox.stub(), + getDirectConversationsByUserId: sandbox.stub(), }, - check: sinon.stub(), + check: sandbox.stub(), callbacks: { - run: sinon.stub(), + run: sandbox.stub(), }, settings: { - get: sinon.stub(), + get: sandbox.stub(), }, - notifyOnUserChange: sinon.stub(), - notifyOnRoomChangedByUserDM: sinon.stub(), - notifyOnRoomChangedById: sinon.stub(), - getSubscribedRoomsForUserWithDetails: sinon.stub(), - shouldRemoveOrChangeOwner: sinon.stub(), - getUserSingleOwnedRooms: sinon.stub(), - closeOmnichannelConversations: sinon.stub(), - relinquishRoomOwnerships: sinon.stub(), + notifyOnUserChange: sandbox.stub(), + notifyOnRoomChangedByUserDM: sandbox.stub(), + notifyOnRoomChangedById: sandbox.stub(), + getSubscribedRoomsForUserWithDetails: sandbox.stub(), + shouldRemoveOrChangeOwner: sandbox.stub(), + getUserSingleOwnedRooms: sandbox.stub(), + closeOmnichannelConversations: sandbox.stub(), + relinquishRoomOwnerships: sandbox.stub(), Mailer: { - sendNoWrap: sinon.stub(), + sendNoWrap: sandbox.stub(), }, Accounts: { emailTemplates: { userActivated: { - subject: sinon.stub(), - html: sinon.stub(), + subject: sandbox.stub(), + html: sandbox.stub(), }, }, }, - isUserFederated: sinon.stub(), + isUserFederated: sandbox.stub(), }; const { setUserActiveStatus } = proxyquire.noCallThru().load('../../../../../../app/lib/server/functions/setUserActiveStatus', { @@ -95,16 +97,33 @@ describe('setUserActiveStatus', () => { }); afterEach(() => { - Object.values(stubs).forEach((stub) => { - if (typeof stub === 'object' && stub !== null) { - Object.values(stub).forEach((method) => { - if (typeof method?.reset === 'function') { - method.reset(); - } - }); - } else if (typeof stub?.reset === 'function') { - stub.reset(); - } + sandbox.reset(); + }); + + describe('Successful status changes', () => { + it('should deactivate a user successfully', async () => { + const result = await setUserActiveStatus(userId, false); + + expect(result).to.be.true; + expect(stubs.Users.setUserActive.calledWith(userId, false)).to.be.true; + expect(stubs.Users.unsetLoginTokens.calledWith(userId)).to.be.true; + expect(stubs.Rooms.setDmReadOnlyByUserId.calledWith(userId, undefined, true, false)).to.be.true; + expect(stubs.callbacks.run.calledWith('afterDeactivateUser', sinon.match({ _id: userId }))).to.be.true; + expect(stubs.notifyOnUserChange.calledWith(sinon.match({ clientAction: 'updated', id: userId, diff: { 'services.resume.loginTokens': [], active: false } }))).to.be.true; + expect(stubs.notifyOnRoomChangedByUserDM.calledWith(userId)).to.be.true; + }); + + it('should activate a user successfully', async () => { + stubs.Users.findOneById.resolves({ _id: userId, username, active: false }); + + const result = await setUserActiveStatus(userId, true); + + expect(result).to.be.true; + expect(stubs.callbacks.run.calledWith('beforeActivateUser', sinon.match({ _id: userId }))).to.be.true; + expect(stubs.Users.setUserActive.calledWith(userId, true)).to.be.true; + expect(stubs.callbacks.run.calledWith('afterActivateUser', sinon.match({ _id: userId }))).to.be.true; + expect(stubs.Users.unsetReason.calledWith(userId)).to.be.true; + expect(stubs.notifyOnUserChange.calledWith(sinon.match({ clientAction: 'updated', id: userId, diff: { active: true } }))).to.be.true; }); }); @@ -120,12 +139,22 @@ describe('setUserActiveStatus', () => { it('should throw error for federated users', async () => { stubs.isUserFederated.returns(true); - try { - await setUserActiveStatus(userId, false); - expect.fail('Should have thrown an error'); - } catch (error: any) { - expect(error.message).to.equal('error-user-is-federated'); - } + await expect(setUserActiveStatus(userId, false)).to.be.rejectedWith('error-user-is-federated'); + }); + + it('should throw error if deactivating the last active admin', async () => { + stubs.Users.findOneAdmin.resolves({ _id: userId }); + stubs.Users.countActiveUsersInRoles.resolves(1); + + await expect(setUserActiveStatus(userId, false)).to.be.rejectedWith('error-action-not-allowed'); + }); + + it('should throw error if user is the last owner of channels without confirmRelinquish', async () => { + stubs.getSubscribedRoomsForUserWithDetails.resolves([{ t: 'c' }]); + stubs.shouldRemoveOrChangeOwner.returns(true); + stubs.getUserSingleOwnedRooms.resolves(['room1']); + + await expect(setUserActiveStatus(userId, false)).to.be.rejectedWith('user-last-owner'); }); }); }); diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 9f3c4fed1be74..8749d4d334b52 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1366,7 +1366,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri localField: 'rid', foreignField: '_id', as: 'room', - pipeline: [{ $project: { archived: 1 } }, { $match: { archived: { $ne: true } } }], + pipeline: [{ $match: { archived: { $ne: true } } }, { $project: { _id: 1 } }], }, }, { $match: { 'room.0': { $exists: true } } }, @@ -1387,7 +1387,7 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri localField: 'rid', foreignField: '_id', as: 'room', - pipeline: [{ $project: { archived: 1 } }, { $match: { archived: { $ne: true } } }], + pipeline: [{ $match: { archived: { $ne: true } } }, { $project: { _id: 1 } }], }, }, { $match: { 'room.0': { $exists: true } } }, From f0aeb1fad43b19f40d76a2131269ac9770291011 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 19 Mar 2026 00:59:33 +0530 Subject: [PATCH 13/18] style: run prettier to fix line length in tests --- .../app/lib/server/functions/setUserActiveStatus.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts index 509564b647a35..c98e3003dc0d2 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserActiveStatus.spec.ts @@ -109,7 +109,11 @@ describe('setUserActiveStatus', () => { expect(stubs.Users.unsetLoginTokens.calledWith(userId)).to.be.true; expect(stubs.Rooms.setDmReadOnlyByUserId.calledWith(userId, undefined, true, false)).to.be.true; expect(stubs.callbacks.run.calledWith('afterDeactivateUser', sinon.match({ _id: userId }))).to.be.true; - expect(stubs.notifyOnUserChange.calledWith(sinon.match({ clientAction: 'updated', id: userId, diff: { 'services.resume.loginTokens': [], active: false } }))).to.be.true; + expect( + stubs.notifyOnUserChange.calledWith( + sinon.match({ clientAction: 'updated', id: userId, diff: { 'services.resume.loginTokens': [], 'active': false } }), + ), + ).to.be.true; expect(stubs.notifyOnRoomChangedByUserDM.calledWith(userId)).to.be.true; }); From 8422dd9bba1d8166b3ec38f7e6a92664ba1eb392 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Fri, 20 Mar 2026 00:31:14 +0530 Subject: [PATCH 14/18] Apply suggestion from @abhinavkrin --- packages/models/src/models/Subscriptions.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 8749d4d334b52..2b022da460a4c 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -67,7 +67,6 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri { key: { rid: 1, ls: 1 } }, { key: { 'u._id': 1, 'autotranslate': 1 } }, { key: { 'v._id': 1, 'open': 1 } }, - { key: { rid: 1, archived: 1, ls: 1 } }, ]; } From 05c8922457534d569041c14602ccd9cdde390dc9 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Thu, 2 Apr 2026 19:35:50 +0530 Subject: [PATCH 15/18] improved tests Signed-off-by: Abhinav Kumar --- .../read-receipts-deactivated-users.spec.ts | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts index 390df5a6a3d7e..6ad9a5cfdf614 100644 --- a/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts +++ b/apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts @@ -25,19 +25,22 @@ test.describe.serial('read-receipts-deactivated-users', () => { test.skip(!IS_EE, 'Enterprise Only'); test.beforeAll(async ({ api }) => { - testUser1 = await createTestUser(api); - testUser2 = await createTestUser(api); + [testUser1, testUser2] = await Promise.all([createTestUser(api), createTestUser(api)]); [testUser1State, testUser2State] = await Promise.all([loginTestUser(api, testUser1), loginTestUser(api, testUser2)]); targetChannel = await createTargetChannel(api, { members: [testUser1.data.username, testUser2.data.username] }); - await setSettingValueById(api, 'Message_Read_Receipt_Enabled', true); - await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', true); + await Promise.all([ + setSettingValueById(api, 'Message_Read_Receipt_Enabled', true), + setSettingValueById(api, 'Message_Read_Receipt_Store_Users', true), + ]); }); test.afterAll(async ({ api }) => { - await setSettingValueById(api, 'Message_Read_Receipt_Enabled', false); - await setSettingValueById(api, 'Message_Read_Receipt_Store_Users', false); + await Promise.all([ + setSettingValueById(api, 'Message_Read_Receipt_Enabled', false), + setSettingValueById(api, 'Message_Read_Receipt_Store_Users', false), + ]); await deleteChannel(api, targetChannel); await Promise.all([testUser1?.delete(), testUser2?.delete()]); @@ -49,12 +52,7 @@ test.describe.serial('read-receipts-deactivated-users', () => { }); test.afterEach(async () => { - if (user1Context) { - await user1Context.page.close(); - } - if (user2Context) { - await user2Context.page.close(); - } + await Promise.all([user1Context?.page.close(), user2Context?.page.close()]); user1Context = undefined; user2Context = undefined; }); @@ -68,15 +66,19 @@ test.describe.serial('read-receipts-deactivated-users', () => { const user2Ctx = { page: page2, poHomeChannel: new HomeChannel(page2) }; user2Context = user2Ctx; - await poHomeChannel.navbar.openChat(targetChannel); - await user1Ctx.poHomeChannel.navbar.openChat(targetChannel); - await user2Ctx.poHomeChannel.navbar.openChat(targetChannel); + await Promise.all([ + poHomeChannel.navbar.openChat(targetChannel), + user1Ctx.poHomeChannel.navbar.openChat(targetChannel), + user2Ctx.poHomeChannel.navbar.openChat(targetChannel), + ]); await test.step('when all users are active', async () => { await poHomeChannel.content.sendMessage('Message 1: All three users active'); - await expect(user1Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(); - await expect(user2Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(); + await Promise.all([ + expect(user1Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(), + expect(user2Ctx.poHomeChannel.content.lastUserMessage).toBeVisible(), + ]); await expect(poHomeChannel.content.lastUserMessage.getByRole('status', { name: 'Message viewed' })).toBeVisible(); From b8b2fc709dbc27336cde2a1e91d6f00b45ad60eb Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Wed, 15 Apr 2026 16:17:09 +0530 Subject: [PATCH 16/18] switched from aggregation to normal queries --- .../app/lib/server/functions/unarchiveRoom.ts | 32 +++++- .../functions/unarchiveUserSubscriptions.ts | 40 ++++++++ .../app/lib/server/hooks/afterUserActions.ts | 13 ++- .../model-typings/src/models/IRoomsModel.ts | 2 + .../src/models/ISubscriptionsModel.ts | 7 +- packages/models/src/models/Rooms.ts | 17 +++- packages/models/src/models/Subscriptions.ts | 98 +++---------------- 7 files changed, 113 insertions(+), 96 deletions(-) create mode 100644 apps/meteor/app/lib/server/functions/unarchiveUserSubscriptions.ts diff --git a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts index 44a74479560d4..353124a02037c 100644 --- a/apps/meteor/app/lib/server/functions/unarchiveRoom.ts +++ b/apps/meteor/app/lib/server/functions/unarchiveRoom.ts @@ -1,14 +1,40 @@ import { Message } from '@rocket.chat/core-services'; import type { IMessage } from '@rocket.chat/core-typings'; -import { Rooms, Subscriptions } from '@rocket.chat/models'; +import { Rooms, Subscriptions, Users } from '@rocket.chat/models'; import { notifyOnRoomChangedById, notifyOnSubscriptionChangedByRoomId } from '../lib/notifyListener'; +const BATCH_SIZE = 100_000; + +async function getActiveUserIds(userIds: string[]): Promise> { + const activeUserIds = new Set(); + + for (let i = 0; i < userIds.length; i += BATCH_SIZE) { + const batch = await Users.findActiveByIds(userIds.slice(i, i + BATCH_SIZE), { projection: { _id: 1 } }).toArray(); + for (const u of batch) { + activeUserIds.add(u._id); + } + } + + return activeUserIds; +} + +async function unarchiveSubscriptionsByIds(ids: string[]): Promise { + for (let i = 0; i < ids.length; i += BATCH_SIZE) { + await Subscriptions.unarchiveByIds(ids.slice(i, i + BATCH_SIZE)); + } +} + export const unarchiveRoom = async function (rid: string, user: IMessage['u']): Promise { await Rooms.unarchiveById(rid); - if (await Subscriptions.hasArchivedSubscriptionsByRoomId(rid)) { - await Subscriptions.unarchiveByRoomId(rid); + const archivedSubs = await Subscriptions.findArchivedByRoomId(rid, { projection: { 'u._id': 1 } }).toArray(); + + if (archivedSubs.length > 0) { + const activeUserIds = await getActiveUserIds(archivedSubs.map((s) => s.u._id)); + const idsToUnarchive = archivedSubs.filter((s) => activeUserIds.has(s.u._id)).map((s) => s._id); + + await unarchiveSubscriptionsByIds(idsToUnarchive); void notifyOnSubscriptionChangedByRoomId(rid); } diff --git a/apps/meteor/app/lib/server/functions/unarchiveUserSubscriptions.ts b/apps/meteor/app/lib/server/functions/unarchiveUserSubscriptions.ts new file mode 100644 index 0000000000000..aafb4389506c9 --- /dev/null +++ b/apps/meteor/app/lib/server/functions/unarchiveUserSubscriptions.ts @@ -0,0 +1,40 @@ +import { Rooms, Subscriptions } from '@rocket.chat/models'; + +const BATCH_SIZE = 100_000; + +async function getArchivedRoomIds(rids: string[]): Promise> { + const archivedRoomIds = new Set(); + + for (let i = 0; i < rids.length; i += BATCH_SIZE) { + const batch = await Rooms.findManyArchivedByRoomIds(rids.slice(i, i + BATCH_SIZE), { projection: { _id: 1 } }).toArray(); + for (const r of batch) { + archivedRoomIds.add(r._id); + } + } + + return archivedRoomIds; +} + +async function unarchiveSubscriptionsByIds(ids: string[]): Promise { + for (let i = 0; i < ids.length; i += BATCH_SIZE) { + await Subscriptions.unarchiveByIds(ids.slice(i, i + BATCH_SIZE)); + } +} + +export const unarchiveUserSubscriptions = async (userId: string): Promise => { + const archivedSubs = await Subscriptions.findArchivedByUserId(userId, { projection: { rid: 1 } }).toArray(); + + if (!archivedSubs.length) { + return false; + } + + const archivedRoomIds = await getArchivedRoomIds(archivedSubs.map((s) => s.rid)); + const idsToUnarchive = archivedSubs.filter((s) => !archivedRoomIds.has(s.rid)).map((s) => s._id); + + if (!idsToUnarchive.length) { + return false; + } + + await unarchiveSubscriptionsByIds(idsToUnarchive); + return true; +}; diff --git a/apps/meteor/app/lib/server/hooks/afterUserActions.ts b/apps/meteor/app/lib/server/hooks/afterUserActions.ts index c56aae8ff4732..965bd15e664f9 100644 --- a/apps/meteor/app/lib/server/hooks/afterUserActions.ts +++ b/apps/meteor/app/lib/server/hooks/afterUserActions.ts @@ -2,8 +2,13 @@ import type { IUser } from '@rocket.chat/core-typings'; import { Subscriptions } from '@rocket.chat/models'; import { callbacks } from '../../../../server/lib/callbacks'; +import { unarchiveUserSubscriptions } from '../functions/unarchiveUserSubscriptions'; import { notifyOnSubscriptionChangedByUserId } from '../lib/notifyListener'; +/** + * When a user is deactivated, archive all their subscriptions so they + * no longer appear in read-receipt counts and active member lists. + */ const handleDeactivateUser = async (user: IUser): Promise => { const { modifiedCount } = await Subscriptions.setArchivedByUserId(user._id, true); if (modifiedCount) { @@ -11,9 +16,13 @@ const handleDeactivateUser = async (user: IUser): Promise => { } }; +/** + * When a user is reactivated, restore their subscriptions — except for + * rooms that are themselves archived (those should stay archived). + */ const handleActivateUser = async (user: IUser): Promise => { - if (await Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId(user._id)) { - await Subscriptions.unarchiveByUserIdExceptForArchivedRooms(user._id); + const unarchived = await unarchiveUserSubscriptions(user._id); + if (unarchived) { void notifyOnSubscriptionChangedByUserId(user._id); } }; diff --git a/packages/model-typings/src/models/IRoomsModel.ts b/packages/model-typings/src/models/IRoomsModel.ts index 2639f6adcb3a7..76c8004d024a0 100644 --- a/packages/model-typings/src/models/IRoomsModel.ts +++ b/packages/model-typings/src/models/IRoomsModel.ts @@ -44,6 +44,8 @@ export interface IRoomsModel extends IBaseModel { findManyByRoomIds(roomIds: Array, options?: FindOptions): FindCursor; + findManyArchivedByRoomIds(roomIds: Array, options?: FindOptions): FindCursor; + findPaginatedByIds( roomIds: Array, options?: FindOptions, diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index e1822fc3415ff..b499d952b17b7 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -242,10 +242,9 @@ export interface ISubscriptionsModel extends IBaseModel { findUnreadByUserId(userId: string): FindCursor; archiveByRoomId(roomId: string): Promise; - hasArchivedSubscriptionsByRoomId(roomId: string): Promise; - unarchiveByRoomId(roomId: string): Promise; - hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise; - unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise; + findArchivedByRoomId(roomId: string, options?: FindOptions): FindCursor; + findArchivedByUserId(userId: string, options?: FindOptions): FindCursor; + unarchiveByIds(ids: string[]): Promise; updateNameAndAlertByRoomId(roomId: string, name: string, fname: string): Promise; findByRoomIdWhenUsernameExists(rid: string, options?: FindOptions): FindCursor; setCustomFieldsDirectMessagesByUserId(userId: string, fields: Record): Promise; diff --git a/packages/models/src/models/Rooms.ts b/packages/models/src/models/Rooms.ts index 9bcac18cf72fe..6d0dc1aa833a6 100644 --- a/packages/models/src/models/Rooms.ts +++ b/packages/models/src/models/Rooms.ts @@ -154,6 +154,17 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.find(query, options); } + findManyArchivedByRoomIds(roomIds: Array, options: FindOptions = {}): FindCursor { + const query: Filter = { + _id: { + $in: roomIds, + }, + archived: true, + }; + + return this.find(query, options); + } + findPaginatedByIds( roomIds: Array, options: FindOptions = {}, @@ -313,7 +324,7 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.find(query, options); } - findRoomsByNameOrFnameStarting(name: NonNullable, options: FindOptions = {}): FindCursor { + findRoomsByNameOrFnameStarting(name: NonNullable, options: FindOptions = {}): FindCursor { const nameRegex = new RegExp(`^${escapeRegExp(name).trim()}`, 'i'); const query: Filter = { @@ -609,7 +620,7 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { }); } - findOneByNameOrFname(name: NonNullable, options: FindOptions = {}): Promise { + findOneByNameOrFname(name: NonNullable, options: FindOptions = {}): Promise { const query = { $or: [ { @@ -633,7 +644,7 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { return this.findOne(query, options); } - async findOneByNonValidatedName(name: NonNullable, options: FindOptions = {}) { + async findOneByNonValidatedName(name: NonNullable, options: FindOptions = {}) { const room = await this.findOneByNameOrFname(name, options); if (room) { return room; diff --git a/packages/models/src/models/Subscriptions.ts b/packages/models/src/models/Subscriptions.ts index 2b022da460a4c..5702488983c54 100644 --- a/packages/models/src/models/Subscriptions.ts +++ b/packages/models/src/models/Subscriptions.ts @@ -1311,95 +1311,25 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.updateMany(query, update); } - hasArchivedSubscriptionsByRoomId(roomId: string): Promise { - return this.findOne({ rid: roomId, archived: true }, { projection: { _id: 1 } }); + findArchivedByRoomId(roomId: string, options?: FindOptions): FindCursor { + return this.find({ rid: roomId, archived: true }, options); } - async unarchiveByRoomId(roomId: string): Promise { - await this.col - .aggregate( - [ - { $match: { rid: roomId, archived: true } }, - { $project: { '_id': 1, 'u._id': 1 } }, - { - $lookup: { - from: 'users', - let: { userId: '$u._id' }, - pipeline: [ - { $match: { $expr: { $and: [{ $eq: ['$_id', '$$userId'] }, { $eq: ['$active', true] }] } } }, - { $limit: 1 }, - { $project: { _id: 1 } }, - ], - as: '_user', - }, - }, - { $match: { '_user.0': { $exists: true } } }, - { - $project: { - _id: 1, - archived: { $literal: false }, - open: { $literal: true }, - alert: { $literal: false }, - }, - }, - { - $merge: { - into: 'rocketchat_subscription', - whenMatched: 'merge', - whenNotMatched: 'discard', - }, - }, - ], - { allowDiskUse: true }, - ) - .toArray(); + findArchivedByUserId(userId: string, options?: FindOptions): FindCursor { + return this.find({ 'u._id': userId, 'archived': true }, options); } - async hasArchivedSubscriptionsInNonArchivedRoomsByUserId(userId: string): Promise { - const result = await this.col - .aggregate([ - { $match: { 'u._id': userId, 'archived': true } }, - { - $lookup: { - from: 'rocketchat_room', - localField: 'rid', - foreignField: '_id', - as: 'room', - pipeline: [{ $match: { archived: { $ne: true } } }, { $project: { _id: 1 } }], - }, - }, - { $match: { 'room.0': { $exists: true } } }, - { $limit: 1 }, - ]) - .toArray(); - - return result.length > 0; - } - - async unarchiveByUserIdExceptForArchivedRooms(userId: string): Promise { - await this.col - .aggregate([ - { $match: { 'u._id': userId, 'archived': true } }, - { - $lookup: { - from: 'rocketchat_room', - localField: 'rid', - foreignField: '_id', - as: 'room', - pipeline: [{ $match: { archived: { $ne: true } } }, { $project: { _id: 1 } }], - }, - }, - { $match: { 'room.0': { $exists: true } } }, - { - $merge: { - on: '_id', - into: 'rocketchat_subscription', - whenMatched: [{ $set: { archived: false } }], - whenNotMatched: 'discard', - }, + unarchiveByIds(ids: string[]): Promise { + return this.updateMany( + { _id: { $in: ids } }, + { + $set: { + archived: false, + open: true, + alert: false, }, - ]) - .toArray(); + }, + ); } hideByRoomIdAndUserId(roomId: string, userId: string): Promise { From 3f7a15c164fd6ba284bdcea500b9a613d3f9d7d8 Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Wed, 15 Apr 2026 16:35:39 +0530 Subject: [PATCH 17/18] update tests --- .../lib/server/hooks/afterUserActions.spec.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts b/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts index db70fb35af64b..a2d4529747871 100644 --- a/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/hooks/afterUserActions.spec.ts @@ -8,9 +8,8 @@ describe('afterUserActions hooks', () => { const stubs = { Subscriptions: { setArchivedByUserId: sinon.stub(), - hasArchivedSubscriptionsInNonArchivedRoomsByUserId: sinon.stub(), - unarchiveByUserIdExceptForArchivedRooms: sinon.stub(), }, + unarchiveUserSubscriptions: sinon.stub(), callbacks: { add: sinon.stub(), priority: { LOW: 1 }, @@ -33,13 +32,13 @@ describe('afterUserActions hooks', () => { proxyquire.noCallThru().load('../../../../../../app/lib/server/hooks/afterUserActions', { '@rocket.chat/models': { Subscriptions: stubs.Subscriptions }, '../../../../server/lib/callbacks': { callbacks: stubs.callbacks }, + '../functions/unarchiveUserSubscriptions': { unarchiveUserSubscriptions: stubs.unarchiveUserSubscriptions }, '../lib/notifyListener': { notifyOnSubscriptionChangedByUserId: stubs.notifyOnSubscriptionChangedByUserId }, }); afterEach(() => { stubs.Subscriptions.setArchivedByUserId.reset(); - stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.reset(); - stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.reset(); + stubs.unarchiveUserSubscriptions.reset(); stubs.notifyOnSubscriptionChangedByUserId.reset(); }); @@ -65,23 +64,20 @@ describe('afterUserActions hooks', () => { describe('afterActivateUser — subscription unarchiving', () => { it('should unarchive subscriptions excluding archived rooms when user is reactivated', async () => { - stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(true); - stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.resolves(); + stubs.unarchiveUserSubscriptions.resolves(true); await handleActivateUser({ _id: userId }); - expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; - expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.calledOnceWith(userId)).to.be.true; + expect(stubs.unarchiveUserSubscriptions.calledOnceWith(userId)).to.be.true; expect(stubs.notifyOnSubscriptionChangedByUserId.calledWith(userId)).to.be.true; }); it('should not unarchive or notify if no archived subscriptions in non-archived rooms', async () => { - stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.resolves(false); + stubs.unarchiveUserSubscriptions.resolves(false); await handleActivateUser({ _id: userId }); - expect(stubs.Subscriptions.hasArchivedSubscriptionsInNonArchivedRoomsByUserId.calledOnceWith(userId)).to.be.true; - expect(stubs.Subscriptions.unarchiveByUserIdExceptForArchivedRooms.called).to.be.false; + expect(stubs.unarchiveUserSubscriptions.calledOnceWith(userId)).to.be.true; expect(stubs.notifyOnSubscriptionChangedByUserId.called).to.be.false; }); }); From 10ff7ebc047d627d14c5c7884372ca09180f41ea Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Wed, 15 Apr 2026 17:10:05 +0530 Subject: [PATCH 18/18] minor changes Signed-off-by: Abhinav Kumar --- apps/meteor/app/lib/server/hooks/afterUserActions.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apps/meteor/app/lib/server/hooks/afterUserActions.ts b/apps/meteor/app/lib/server/hooks/afterUserActions.ts index 965bd15e664f9..f0b57def35975 100644 --- a/apps/meteor/app/lib/server/hooks/afterUserActions.ts +++ b/apps/meteor/app/lib/server/hooks/afterUserActions.ts @@ -5,10 +5,6 @@ import { callbacks } from '../../../../server/lib/callbacks'; import { unarchiveUserSubscriptions } from '../functions/unarchiveUserSubscriptions'; import { notifyOnSubscriptionChangedByUserId } from '../lib/notifyListener'; -/** - * When a user is deactivated, archive all their subscriptions so they - * no longer appear in read-receipt counts and active member lists. - */ const handleDeactivateUser = async (user: IUser): Promise => { const { modifiedCount } = await Subscriptions.setArchivedByUserId(user._id, true); if (modifiedCount) { @@ -16,10 +12,6 @@ const handleDeactivateUser = async (user: IUser): Promise => { } }; -/** - * When a user is reactivated, restore their subscriptions — except for - * rooms that are themselves archived (those should stay archived). - */ const handleActivateUser = async (user: IUser): Promise => { const unarchived = await unarchiveUserSubscriptions(user._id); if (unarchived) {