fix: read receipts not turning blue when all active users have read

.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.

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,
-	notifyOnSubscriptionChangedByNameAndRoomType,
-	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,13 +107,6 @@ export async function setUserActiveStatus(
 		await callbacks.run('afterDeactivateUser', user);
 	}
 
-	if (user.username) {
-		const { modifiedCount } = await Subscriptions.setArchivedByUsername(user.username, !active);
-		if (modifiedCount) {
-			void notifyOnSubscriptionChangedByNameAndRoomType({ t: 'd', name: user.username });
-		}
-	}
-
 	if (active === false) {
 		await Users.unsetLoginTokens(userId);
 		await Rooms.setDmReadOnlyByUserId(userId, undefined, true, false);

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<Set<string>> {
+	const activeUserIds = new Set<string>();
+
+	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<void> {
+	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<void> {
 	await Rooms.unarchiveById(rid);
 
-	const unarchiveResponse = await Subscriptions.unarchiveByRoomId(rid);
-	if (unarchiveResponse.modifiedCount) {
+	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);
 	}
 

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<Set<string>> {
+	const archivedRoomIds = new Set<string>();
+
+	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<void> {
+	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<boolean> => {
+	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;
+};

apps/meteor/app/lib/server/hooks/afterUserActions.ts

@@ -0,0 +1,24 @@
+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';
+
+const handleDeactivateUser = async (user: IUser): Promise<void> => {
+	const { modifiedCount } = await Subscriptions.setArchivedByUserId(user._id, true);
+	if (modifiedCount) {
+		void notifyOnSubscriptionChangedByUserId(user._id);
+	}
+};
+
+const handleActivateUser = async (user: IUser): Promise<void> => {
+	const unarchived = await unarchiveUserSubscriptions(user._id);
+	if (unarchived) {
+		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');

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';

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) {

apps/meteor/tests/e2e/read-receipts-deactivated-users.spec.ts

@@ -0,0 +1,119 @@
+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 });
+
+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;
+	let testUser1: ITestUser;
+	let testUser2: ITestUser;
+	let testUser1State: IUserState;
+	let testUser2State: IUserState;
+
+	test.skip(!IS_EE, 'Enterprise Only');
+
+	test.beforeAll(async ({ 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 Promise.all([
+			setSettingValueById(api, 'Message_Read_Receipt_Enabled', true),
+			setSettingValueById(api, 'Message_Read_Receipt_Store_Users', true),
+		]);
+	});
+
+	test.afterAll(async ({ api }) => {
+		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()]);
+	});
+
+	test.beforeEach(async ({ page }) => {
+		poHomeChannel = new HomeChannel(page);
+		await page.goto('/home');
+	});
+
+	test.afterEach(async () => {
+		await Promise.all([user1Context?.page.close(), 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, testUser1State);
+		const user1Ctx = { page: page1, poHomeChannel: new HomeChannel(page1) };
+		user1Context = user1Ctx;
+
+		const { page: page2 } = await createAuxContext(browser, testUser2State);
+		const user2Ctx = { page: page2, poHomeChannel: new HomeChannel(page2) };
+		user2Context = user2Ctx;
+
+		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 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();
+
+			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: testUser1.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: testUser2.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();
+		});
+	});
+});

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();
 		});
 	});
 

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<IUserState> {
+	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
  */