fix: desktop notifications not respecting privacy settings (#36156)
Abhinav Kumar
7 months ago
committed by
GitHub
parent
3779de0e8c
commit
d2ac7f0988
@ -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. |
||||
@ -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`); |
||||
} |
||||
}); |
||||
}); |
||||
}); |
||||
}); |
||||
}); |
||||
@ -0,0 +1,97 @@ |
||||
import { expect } from 'chai'; |
||||
import proxyquire from 'proxyquire'; |
||||
import sinon from 'sinon'; |
||||
|
||||
const getUserDisplayNameStub = sinon.stub(); |
||||
const i18nTranslateStub = sinon.stub(); |
||||
|
||||
const settingsGetStub = sinon.stub(); |
||||
|
||||
const { buildNotificationDetails } = proxyquire.noCallThru().load('./buildNotificationDetails.ts', { |
||||
'../../../app/settings/server': { |
||||
settings: { get: settingsGetStub }, |
||||
}, |
||||
'@rocket.chat/core-typings': { |
||||
getUserDisplayName: getUserDisplayNameStub, |
||||
}, |
||||
'../i18n': { |
||||
i18n: { t: i18nTranslateStub }, |
||||
}, |
||||
}); |
||||
|
||||
describe('buildNotificationDetails', () => { |
||||
const room = { name: 'general' }; |
||||
const sender = { _id: 'id1', name: 'Alice', username: 'alice' }; |
||||
|
||||
beforeEach(() => { |
||||
settingsGetStub.reset(); |
||||
getUserDisplayNameStub.reset(); |
||||
i18nTranslateStub.reset(); |
||||
}); |
||||
|
||||
const languageFallback = 'You_have_a_new_message'; |
||||
|
||||
const testCases = [ |
||||
{ showPushMessage: true, showName: true, expectPrefix: true }, |
||||
{ showPushMessage: true, showName: false, expectPrefix: false }, |
||||
{ showPushMessage: false, showName: true, expectPrefix: false }, |
||||
{ showPushMessage: false, showName: false, expectPrefix: false }, |
||||
]; |
||||
|
||||
testCases.forEach(({ showPushMessage, showName, expectPrefix }) => { |
||||
const label = `Push_show_message=${showPushMessage}, Push_show_username_room=${showName}`; |
||||
|
||||
it(`should return correct fields when ${label}`, () => { |
||||
settingsGetStub.withArgs('Push_show_message').returns(showPushMessage); |
||||
settingsGetStub.withArgs('Push_show_username_room').returns(showName); |
||||
settingsGetStub.withArgs('UI_Use_Real_Name').returns(false); |
||||
settingsGetStub.withArgs('Language').returns('en'); |
||||
|
||||
const expectedMessage = 'Test message'; |
||||
const expectedTitle = 'Some Room Title'; |
||||
const senderDisplayName = 'Alice'; |
||||
|
||||
getUserDisplayNameStub.returns(senderDisplayName); |
||||
i18nTranslateStub.withArgs('You_have_a_new_message', { lng: 'en' }).returns(languageFallback); |
||||
|
||||
const result = buildNotificationDetails({ |
||||
expectedNotificationMessage: expectedMessage, |
||||
expectedTitle, |
||||
room, |
||||
sender, |
||||
language: undefined, |
||||
senderNameExpectedInMessage: true, |
||||
}); |
||||
|
||||
const shouldShowTitle = showName; |
||||
const shouldPrefix = showPushMessage && showName && expectPrefix; |
||||
|
||||
expect(result.title).to.equal(shouldShowTitle ? expectedTitle : undefined); |
||||
expect(result.name).to.equal(shouldShowTitle ? room.name : undefined); |
||||
|
||||
if (!showPushMessage) { |
||||
expect(result.text).to.equal(languageFallback); |
||||
} else if (shouldPrefix) { |
||||
expect(result.text).to.equal(`${senderDisplayName}: ${expectedMessage}`); |
||||
} else { |
||||
expect(result.text).to.equal(expectedMessage); |
||||
} |
||||
}); |
||||
}); |
||||
|
||||
it('should respect provided language if supplied', () => { |
||||
settingsGetStub.withArgs('Push_show_message').returns(false); |
||||
settingsGetStub.withArgs('Push_show_username_room').returns(false); |
||||
i18nTranslateStub.withArgs('You_have_a_new_message', { lng: 'fr' }).returns('Vous avez un nouveau message'); |
||||
|
||||
const result = buildNotificationDetails({ |
||||
expectedNotificationMessage: 'ignored', |
||||
room, |
||||
sender, |
||||
language: 'fr', |
||||
senderNameExpectedInMessage: false, |
||||
}); |
||||
|
||||
expect(result.text).to.equal('Vous avez un nouveau message'); |
||||
}); |
||||
}); |
||||
@ -0,0 +1,64 @@ |
||||
import { getUserDisplayName } from '@rocket.chat/core-typings'; |
||||
import type { AtLeast, IRoom, IUser } from '@rocket.chat/core-typings'; |
||||
|
||||
import { settings } from '../../../app/settings/server'; |
||||
import { i18n } from '../i18n'; |
||||
|
||||
/** |
||||
* Options for building notification details. |
||||
*/ |
||||
export type BuildNotificationDetailsOptions = { |
||||
/** The message to display for the notification. */ |
||||
expectedNotificationMessage: string; |
||||
/** The precomputed title for the notification. */ |
||||
expectedTitle?: string; |
||||
/** The room object with at least a name property. */ |
||||
room: AtLeast<IRoom, 'name'>; |
||||
/** The user sending the notification. */ |
||||
sender: AtLeast<IUser, '_id' | 'name' | 'username'>; |
||||
/** Optional language code used for translation (defaults to server setting or 'en'). */ |
||||
language?: string; |
||||
/** Whether to prefix the message with senderName: (defaults to false). */ |
||||
senderNameExpectedInMessage?: boolean; |
||||
}; |
||||
|
||||
/** |
||||
* Builds the notification payload fields (title, text, name) based on settings and provided expectations. |
||||
* |
||||
* @param options - Precomputed values and context for the notification. |
||||
* @returns An object containing the notification title, text, and user/room label. |
||||
*/ |
||||
export function buildNotificationDetails({ |
||||
expectedNotificationMessage, |
||||
expectedTitle, |
||||
room, |
||||
sender, |
||||
language, |
||||
senderNameExpectedInMessage = false, |
||||
}: BuildNotificationDetailsOptions): { title?: string; text: string; name?: string } { |
||||
const showPushMessage = settings.get<boolean>('Push_show_message'); |
||||
const showUserOrRoomName = settings.get<boolean>('Push_show_username_room'); |
||||
|
||||
let text: string; |
||||
if (showPushMessage) { |
||||
if (senderNameExpectedInMessage && showUserOrRoomName) { |
||||
const useRealName = settings.get<boolean>('UI_Use_Real_Name'); |
||||
const senderName = getUserDisplayName(sender.name, sender.username, useRealName); |
||||
text = `${senderName}: ${expectedNotificationMessage}`; |
||||
} else { |
||||
text = expectedNotificationMessage; |
||||
} |
||||
} else { |
||||
const lng = language || settings.get('Language') || 'en'; |
||||
text = i18n.t('You_have_a_new_message', { lng }); |
||||
} |
||||
|
||||
let title: string | undefined; |
||||
let name: string | undefined; |
||||
if (showUserOrRoomName) { |
||||
name = room.name; |
||||
title = expectedTitle; |
||||
} |
||||
|
||||
return { title, text, name }; |
||||
} |
||||
Loading…
Reference in new issue