fix: desktop notifications not respecting privacy settings

.changeset/silent-dolphins-argue.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes an issue where the `Show Message in Notification` and `Show Channel/Group/Username in Notification` setting was ignored in desktop notifications. Also ensures users are redirected to the correct room on interacting with the notifications.

apps/meteor/.mocharc.js

@@ -41,5 +41,6 @@ module.exports = {
 		'tests/unit/server/**/*.spec.ts',
 		'app/api/server/lib/**/*.spec.ts',
 		'app/file-upload/server/**/*.spec.ts',
+		'app/lib/server/functions/notifications/**/*.spec.ts',
 	],
 };

apps/meteor/app/lib/server/functions/notifications/desktop.spec.ts

@@ -0,0 +1,231 @@
+import type { IRoom } from '@rocket.chat/core-typings';
+import { UserStatus } from '@rocket.chat/core-typings';
+import { expect } from 'chai';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+const broadcastStub = sinon.stub();
+const settingsGetStub = sinon.stub();
+
+const { buildNotificationDetails } = proxyquire.noCallThru().load('../../../../../server/lib/rooms/buildNotificationDetails.ts', {
+	'../../../app/settings/server': { settings: { get: settingsGetStub } },
+});
+const { roomCoordinator } = proxyquire.noCallThru().load('../../../../../server/lib/rooms/roomCoordinator.ts', {
+	'../../../app/settings/server': { settings: { get: settingsGetStub } },
+	'./buildNotificationDetails': { buildNotificationDetails },
+});
+
+['public', 'private', 'voip', 'livechat'].forEach((type) => {
+	proxyquire.noCallThru().load(`../../../../../server/lib/rooms/roomTypes/${type}.ts`, {
+		'../../../../app/settings/server': { settings: { get: settingsGetStub } },
+		'../roomCoordinator': { roomCoordinator },
+		'../buildNotificationDetails': { buildNotificationDetails },
+	});
+});
+
+proxyquire.noCallThru().load('../../../../../server/lib/rooms/roomTypes/direct.ts', {
+	'../../../../app/settings/server': { settings: { get: settingsGetStub } },
+	'../roomCoordinator': { roomCoordinator },
+	'../buildNotificationDetails': { buildNotificationDetails },
+	'meteor/meteor': { Meteor: { userId: () => 'user123' } },
+	'@rocket.chat/models': {
+		Subscription: {
+			findOneByRoomIdAndUserId: () => ({ name: 'general', fname: 'general' }),
+		},
+	},
+});
+
+const { notifyDesktopUser } = proxyquire.noCallThru().load('./desktop', {
+	'../../../../settings/server': { settings: { get: settingsGetStub } },
+	'../../../../metrics/server': {
+		metrics: {
+			notificationsSent: { inc: sinon.stub() },
+		},
+	},
+	'@rocket.chat/core-services': { api: { broadcast: broadcastStub } },
+	'../../lib/sendNotificationsOnMessage': {},
+	'../../../../../server/lib/rooms/roomCoordinator': { roomCoordinator },
+});
+
+const fakeUserId = 'user123';
+
+const createTestData = (t: IRoom['t'] = 'c', showPushMessage = false, showUserOrRoomName = false, groupDM = false) => {
+	const sender = { _id: 'sender123', name: 'Alice', username: 'alice' };
+	let uids: string[] | undefined;
+	if (t === 'd') {
+		uids = groupDM ? ['sender123', 'user123', 'otherUser123'] : ['sender123', 'user123'];
+	}
+
+	const room: Partial<IRoom> = {
+		t,
+		_id: 'room123',
+		msgs: 0,
+		_updatedAt: new Date(),
+		u: sender,
+		usersCount: uids ? uids.length : 2,
+		fname: uids?.length === 2 ? sender.name : 'general',
+		name: uids?.length === 2 ? sender.username : 'general',
+		uids,
+	};
+
+	const message = {
+		_id: 'msg123',
+		rid: 'room123',
+		tmid: null,
+		u: sender,
+		msg: 'Fake message here',
+	};
+
+	const receiver = {
+		_id: 'user123',
+		language: 'en',
+		username: 'receiver-username',
+		emails: [{ address: 'receiver@example.com', verified: true }],
+		active: true,
+		status: UserStatus.OFFLINE,
+		statusConnection: 'offline',
+	};
+
+	let expectedTitle: string | undefined;
+	if (showUserOrRoomName) {
+		switch (t) {
+			case 'c':
+			case 'p':
+				expectedTitle = `#${room.name}`;
+				break;
+			case 'l':
+			case 'v':
+				expectedTitle = `[Omnichannel] ${room.name}`;
+				break;
+			case 'd':
+				expectedTitle = room.name;
+				break;
+		}
+	}
+
+	let expectedNotificationMessage: string;
+
+	if (!showPushMessage) {
+		expectedNotificationMessage = 'You have a new message';
+	} else if (!showUserOrRoomName) {
+		// No prefix if showUserOrRoomName is false
+		expectedNotificationMessage = message.msg;
+	} else if (t === 'd' && uids && uids.length > 2) {
+		expectedNotificationMessage = `${sender.username}: ${message.msg}`;
+	} else {
+		switch (t) {
+			case 'c':
+			case 'p':
+				expectedNotificationMessage = `${sender.username}: ${message.msg}`;
+				break;
+			case 'l':
+			case 'v':
+			case 'd':
+				expectedNotificationMessage = message.msg;
+				break;
+		}
+	}
+
+	return {
+		room: room as IRoom,
+		user: sender,
+		message,
+		receiver,
+		expectedTitle,
+		expectedNotificationMessage,
+	};
+};
+
+describe('notifyDesktopUser privacy settings across all room types', () => {
+	const roomTypes: Array<{ t: IRoom['t']; isGroupDM?: boolean }> = [
+		{ t: 'c' },
+		{ t: 'p' },
+		{ t: 'l' },
+		{ t: 'v' },
+		{ t: 'd', isGroupDM: false },
+		{ t: 'd', isGroupDM: true },
+	];
+
+	afterEach(() => {
+		broadcastStub.resetHistory();
+		settingsGetStub.resetHistory();
+	});
+
+	roomTypes.forEach(({ t, isGroupDM = false }) => {
+		let roomLabel: string;
+		if (t === 'c') {
+			roomLabel = 'channel';
+		} else if (t === 'p') {
+			roomLabel = 'private';
+		} else if (t === 'l') {
+			roomLabel = 'livechat';
+		} else if (t === 'v') {
+			roomLabel = 'voip';
+		} else if (t === 'd' && isGroupDM) {
+			roomLabel = 'direct (group DM)';
+		} else {
+			roomLabel = 'direct (1:1 DM)';
+		}
+
+		describe(`when room type is "${roomLabel}"`, () => {
+			[
+				{ showPushMessage: false, showUserOrRoomName: true },
+				{ showPushMessage: true, showUserOrRoomName: false },
+				{ showPushMessage: false, showUserOrRoomName: false },
+				{ showPushMessage: true, showUserOrRoomName: true },
+			].forEach(({ showPushMessage, showUserOrRoomName }) => {
+				const label = `Push_show_message=${
+					showPushMessage ? 'true' : 'false'
+				} and Push_show_username_room=${showUserOrRoomName ? 'true' : 'false'}`;
+
+				it(`should handle settings: ${label}`, async () => {
+					const { room, user, message, receiver, expectedTitle, expectedNotificationMessage } = createTestData(
+						t,
+						showPushMessage,
+						showUserOrRoomName,
+						isGroupDM,
+					);
+
+					settingsGetStub.withArgs('Push_show_message').returns(showPushMessage);
+					settingsGetStub.withArgs('Push_show_username_room').returns(showUserOrRoomName);
+					settingsGetStub.withArgs('UI_Use_Real_Name').returns(false);
+
+					const duration = 4000;
+					const notificationMessage = message.msg;
+
+					await notifyDesktopUser({
+						userId: fakeUserId,
+						user,
+						room,
+						message,
+						duration,
+						notificationMessage,
+						receiver,
+					});
+
+					expect(broadcastStub.calledOnce).to.be.true;
+					const [eventName, targetUserId, payload] = broadcastStub.firstCall.args as [string, string, any];
+
+					expect(eventName).to.equal('notify.desktop');
+					expect(targetUserId).to.equal(fakeUserId);
+
+					expect(payload.text).to.equal(expectedNotificationMessage);
+
+					if (showPushMessage) {
+						expect(payload.payload.message?.msg).to.equal(message.msg);
+					} else {
+						expect(!!payload.payload.message?.msg).to.equal(false);
+					}
+
+					if (showUserOrRoomName) {
+						expect(payload.title).to.equal(expectedTitle);
+						expect(payload.payload.name).to.equal(room.name);
+					} else {
+						expect(!!payload.title).to.equal(false, `Found title to be ${payload.title} when expected falsy`);
+						expect(!!payload.payload.name).to.equal(false, `Found name to be ${payload.name} when expected falsy`);
+					}
+				});
+			});
+		});
+	});
+});

apps/meteor/app/lib/server/functions/notifications/desktop.ts

@@ -4,6 +4,7 @@ import type { IMessage, IRoom, IUser, AtLeast } from '@rocket.chat/core-typings'
 import { roomCoordinator } from '../../../../../server/lib/rooms/roomCoordinator';
 import { metrics } from '../../../../metrics/server';
 import { settings } from '../../../../settings/server';
+import type { SubscriptionAggregation } from '../../lib/sendNotificationsOnMessage';
 
 /**
  * Send notification to user
@@ -22,17 +23,21 @@ export async function notifyDesktopUser({
 	room,
 	duration,
 	notificationMessage,
+	receiver,
 }: {
 	userId: string;
 	user: AtLeast<IUser, '_id' | 'name' | 'username'>;
 	message: IMessage | Pick<IMessage, 'u'>;
 	room: IRoom;
 	duration?: number;
 	notificationMessage: string;
+	receiver?: SubscriptionAggregation['receiver'][number];
 }): Promise<void> {
 	const { title, text, name } = await roomCoordinator
 		.getRoomDirectives(room.t)
-		.getNotificationDetails(room, user, notificationMessage, userId);
+		.getNotificationDetails(room, user, notificationMessage, userId, receiver?.language);
+
+	const showPushMessage = settings.get<boolean>('Push_show_message');
 
 	const payload = {
 		title: title || '',
@@ -51,7 +56,7 @@ export async function notifyDesktopUser({
 			sender: message.u,
 			type: room.t,
 			message: {
-				msg: 'msg' in message ? message.msg : '',
+				msg: 'msg' in message && showPushMessage ? message.msg : '',
 				...('t' in message && {
 					t: message.t,
 				}),

apps/meteor/app/lib/server/lib/sendNotificationsOnMessage.ts

@@ -24,7 +24,7 @@ import { getEmailData, shouldNotifyEmail } from '../functions/notifications/emai
 import { messageContainsHighlight } from '../functions/notifications/messageContainsHighlight';
 import { getPushData, shouldNotifyMobile } from '../functions/notifications/mobile';
 
-type SubscriptionAggregation = {
+export type SubscriptionAggregation = {
 	receiver: [Pick<IUser, 'active' | 'emails' | 'language' | 'status' | 'statusConnection' | 'username' | 'settings'> | null];
 } & Pick<
 	ISubscription,
@@ -134,6 +134,7 @@ export const sendNotification = async ({
 			user: sender,
 			message,
 			room,
+			receiver,
 		});
 	}
 

apps/meteor/client/hooks/notification/useNotification.ts

@@ -22,7 +22,7 @@ export const useNotification = () => {
 			return;
 		}
 
-		const { rid } = notification.payload;
+		const { rid, name, _id: msgId } = notification.payload;
 		if (!rid) {
 			return;
 		}
@@ -63,56 +63,54 @@ export const useNotification = () => {
 			n.close();
 			window.focus();
 
-			if (!notification.payload._id || !notification.payload.rid || !notification.payload.name) {
-				return;
-			}
+			const jump = msgId && { jump: msgId };
 
 			switch (notification.payload?.type) {
 				case 'd':
 					router.navigate({
 						pattern: '/direct/:rid/:tab?/:context?',
 						params: {
-							rid: notification.payload.rid,
+							rid,
 							...(notification.payload.tmid && {
 								tab: 'thread',
 								context: notification.payload.tmid,
 							}),
 						},
-						search: { ...router.getSearchParameters(), jump: notification.payload._id },
+						search: { ...router.getSearchParameters(), ...jump },
 					});
 					break;
 				case 'c':
 					return router.navigate({
 						pattern: '/channel/:name/:tab?/:context?',
 						params: {
-							name: notification.payload.name,
+							name: name || rid,
 							...(notification.payload.tmid && {
 								tab: 'thread',
 								context: notification.payload.tmid,
 							}),
 						},
-						search: { ...router.getSearchParameters(), jump: notification.payload._id },
+						search: { ...router.getSearchParameters(), ...jump },
 					});
 				case 'p':
 					return router.navigate({
 						pattern: '/group/:name/:tab?/:context?',
 						params: {
-							name: notification.payload.name,
+							name: name || rid,
 							...(notification.payload.tmid && {
 								tab: 'thread',
 								context: notification.payload.tmid,
 							}),
 						},
-						search: { ...router.getSearchParameters(), jump: notification.payload._id },
+						search: { ...router.getSearchParameters(), ...jump },
 					});
 				case 'l':
 					return router.navigate({
 						pattern: '/live/:id/:tab?/:context?',
 						params: {
-							id: notification.payload.rid,
+							id: rid,
 							tab: 'room-info',
 						},
-						search: { ...router.getSearchParameters(), jump: notification.payload._id },
+						search: { ...router.getSearchParameters(), ...jump },
 					});
 			}
 		};

apps/meteor/definition/IRoomTypeConfig.ts

@@ -105,7 +105,8 @@ export interface IRoomTypeServerDirectives {
 		sender: AtLeast<IUser, '_id' | 'name' | 'username'>,
 		notificationMessage: string,
 		userId: string,
-	) => Promise<{ title: string | undefined; text: string; name: string | undefined }>;
+		language?: string,
+	) => Promise<{ title?: string; text: string; name?: string }>;
 	getMsgSender: (message: IMessage) => Promise<IUser | null>;
 	includeInRoomSearch: () => boolean;
 	getReadReceiptsExtraData: (message: IMessage) => Partial<ReadReceipt>;