Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/meteor/app/reactions/server/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
import './setReaction';
import './notifications';
107 changes: 107 additions & 0 deletions apps/meteor/app/reactions/server/notifications.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { Subscriptions, Users } from '@rocket.chat/models';

import { callbacks } from '../../../server/lib/callbacks';
import { i18n } from '../../../server/lib/i18n';
import { notifyOnSubscriptionChangedByRoomIdAndUserId } from '../../lib/server/lib/notifyListener';
import { settings } from '../../settings/server';
import { notifyDesktopUser } from '../../lib/server/functions/notifications/desktop';
import { emoji } from '../../emoji/server';

callbacks.add(
'afterSetReaction',
async (message, { user, reaction, room }) => {
try {
if (settings.get<boolean>('Troubleshoot_Disable_Notifications') === true) {
return;
}

if (!message.u?._id || message.u._id === user._id) {
return;
}

const recipient = await Users.findOneById(message.u._id, {
projection: {
'active': 1,
'status': 1,
'settings.preferences.receiveReactionNotifications': 1,
'language': 1,
},
});

if (!recipient || !recipient.active) {
return;
}

const receiveReactionNotifications =
recipient.settings?.preferences?.receiveReactionNotifications ??
settings.get<boolean>('Accounts_Default_User_Preferences_receiveReactionNotifications');

if (!receiveReactionNotifications) {
return;
}

await Subscriptions.incUserMentionsAndUnreadForRoomIdAndUserIds(room._id, [recipient._id], 0, 1);
void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipient._id);

const useRealName = settings.get<boolean>('UI_Use_Real_Name');
const reactorName = (useRealName && user.name) || user.username;

const emojione = (emoji.packages as any).emojione;
const emojiActual =
(emojione &&
(emoji.list[reaction] as any)?.uc_output &&
emojione.convert((emoji.list[reaction] as any).uc_output.toUpperCase())) ||
reaction;
const msgText = message.msg || i18n.t('Attachment', { lng: recipient.language });

const notificationMessage = i18n.t('Reaction_Notification', {
name: reactorName,
reaction: emojiActual,
message: msgText,
lng: recipient.language || settings.get<string>('Language') || 'en',
});

await notifyDesktopUser({
userId: recipient._id,
user,
message,
room,
notificationMessage,
});
Comment on lines +64 to +70
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm notifyDesktopUser still accepts `icon` and check for any reaction-specific icon helpers/assets.
rg -nP --type=ts -C5 'function notifyDesktopUser' apps/meteor
rg -nP --type=ts -C2 'icon\?\s*:\s*string' apps/meteor/app/lib/server/functions/notifications
rg -nP --type=ts -C2 'getEmojiUrl|emojiUrl|reactionIcon' apps/meteor packages

Repository: RocketChat/Rocket.Chat

Length of output: 3042


🏁 Script executed:

#!/bin/bash
# Check full implementation of notifyDesktopUser including how icon is used
cat -n apps/meteor/app/lib/server/functions/notifications/desktop.ts | head -80

# Search for server-side emoji or reaction icon utilities
rg -n 'getEmojiUrl|reactionIcon|emojiUrl' apps/meteor/app --type=ts | head -20

# Check for any constant assets or icons related to reactions
rg -n 'icon|emoji' apps/meteor/app/reactions/server --type=ts -A 2 -B 2

Repository: RocketChat/Rocket.Chat

Length of output: 6004


🏁 Script executed:

#!/bin/bash
# Read the full reactions/server/notifications.ts file to see the complete context
cat -n apps/meteor/app/reactions/server/notifications.ts

Repository: RocketChat/Rocket.Chat

Length of output: 4613


Pass an icon parameter to distinguish reaction notifications from message notifications.

The notifyDesktopUser function accepts an optional icon?: string parameter, but the call at line 64–70 doesn't provide one. Since emoji data is already being extracted (lines 49–54), consider passing a reaction icon URL or asset to help users visually distinguish reaction notifications from regular message notifications at the OS level.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/reactions/server/notifications.ts` around lines 64 - 70, The
notifyDesktopUser call is missing the optional icon?: string parameter; update
the invocation of notifyDesktopUser (the call that passes userId, user, message,
room, notificationMessage) to include the reaction icon using the emoji-related
variable already extracted earlier (e.g., emoji, emojiUrl, or reactionEmoji from
lines 49–54) so the desktop notification uses that icon to distinguish reaction
notifications from regular message notifications.

} catch (e) {
console.error('Error sending reaction notification', e);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
callbacks.priority.LOW,
'RecordReactionNotification',
);

callbacks.add(
'afterUnsetReaction',
async (message, { user, room }) => {
try {
if (!message.u?._id || message.u._id === user._id) {
return;
}

const recipientId = message.u._id;

await Subscriptions.updateOne(
{
rid: room._id,
'u._id': recipientId,
unread: { $gt: 0 },
},
{
$inc: { unread: -1 },
},
);

void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipientId);
} catch (e) {
console.error('Error handling reaction notification removal', e);
}
},
callbacks.priority.LOW,
'RecordReactionNotificationRemoval',
);
Comment on lines +79 to +130
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Counter drift when preference/active state changes between set and unset.

The set and unset handlers gate on the recipient's active flag and receiveReactionNotifications preference. If those evaluate true at set time (counter +1) and false at unset time (handler returns early before decrementing), the reactions counter drifts upward permanently for that subscription — leaving a stale reaction badge that can never be cleared via this flow. Equivalent drift occurs in the reverse direction if the user toggles the preference on between set and unset.

Since the increment is the side effect that creates user-visible state, the symmetric guard for unset should arguably be "did we previously increment?" rather than "would we increment now?". A few options:

  • Always decrement on unset (after the self-reaction and Troubleshoot_Disable_Notifications guards), since reactions: { $gt: 0 } already prevents going negative and the counter is reaction-specific (not aliased with unread anymore).
  • Or, persist a per-reaction marker so unset only decrements reactions whose set was actually counted.

The first option is the smallest change and matches the existing $gt: 0 safety net.

🛠️ Minimal fix: drop the active/preference gate in unset
 callbacks.add(
 	'afterUnsetReaction',
 	async (message, { user, room }) => {
 		try {
 			if (settings.get<boolean>('Troubleshoot_Disable_Notifications') === true) {
 				return;
 			}
 
 			if (!message.u?._id || message.u._id === user._id) {
 				return;
 			}
 
-			const recipient = await Users.findOneById(message.u._id, {
-				projection: {
-					'active': 1,
-					'settings.preferences.receiveReactionNotifications': 1,
-				},
-			});
-
-			if (!recipient || !recipient.active) {
-				return;
-			}
-
-			const receiveReactionNotifications =
-				recipient.settings?.preferences?.receiveReactionNotifications ??
-				settings.get<boolean>('Accounts_Default_User_Preferences_receiveReactionNotifications');
-
-			if (!receiveReactionNotifications) {
-				return;
-			}
-
 			const recipientId = message.u._id;
 
 			await Subscriptions.updateOne(
 				{
 					rid: room._id,
 					'u._id': recipientId,
 					reactions: { $gt: 0 },
 				},
 				{
 					$inc: { reactions: -1 },
 				},
 			);
 
 			void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipientId);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
callbacks.add(
'afterUnsetReaction',
async (message, { user, room }) => {
try {
if (settings.get<boolean>('Troubleshoot_Disable_Notifications') === true) {
return;
}
if (!message.u?._id || message.u._id === user._id) {
return;
}
const recipient = await Users.findOneById(message.u._id, {
projection: {
'active': 1,
'settings.preferences.receiveReactionNotifications': 1,
},
});
if (!recipient || !recipient.active) {
return;
}
const receiveReactionNotifications =
recipient.settings?.preferences?.receiveReactionNotifications ??
settings.get<boolean>('Accounts_Default_User_Preferences_receiveReactionNotifications');
if (!receiveReactionNotifications) {
return;
}
const recipientId = message.u._id;
await Subscriptions.updateOne(
{
rid: room._id,
'u._id': recipientId,
reactions: { $gt: 0 },
},
{
$inc: { reactions: -1 },
},
);
void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipientId);
} catch (e) {
SystemLogger.error({ msg: 'Error handling reaction notification removal', err: e });
}
},
callbacks.priority.LOW,
'RecordReactionNotificationRemoval',
);
callbacks.add(
'afterUnsetReaction',
async (message, { user, room }) => {
try {
if (settings.get<boolean>('Troubleshoot_Disable_Notifications') === true) {
return;
}
if (!message.u?._id || message.u._id === user._id) {
return;
}
const recipientId = message.u._id;
await Subscriptions.updateOne(
{
rid: room._id,
'u._id': recipientId,
reactions: { $gt: 0 },
},
{
$inc: { reactions: -1 },
},
);
void notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipientId);
} catch (e) {
SystemLogger.error({ msg: 'Error handling reaction notification removal', err: e });
}
},
callbacks.priority.LOW,
'RecordReactionNotificationRemoval',
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/app/reactions/server/notifications.ts` around lines 79 - 130, The
unset handler (callbacks.add 'afterUnsetReaction') currently gates decrementing
on recipient.active and
recipient.settings.preferences.receiveReactionNotifications which can cause
counter drift; update the handler to keep the early returns for
Troubleshoot_Disable_Notifications and self-reactions but remove the checks that
read recipient.active and receiveReactionNotifications so it always runs the
Subscriptions.updateOne({ rid: room._id, 'u._id': recipientId, reactions: { $gt:
0 } }, { $inc: { reactions: -1 } }) and calls
notifyOnSubscriptionChangedByRoomIdAndUserId(room._id, recipientId); keep the
existing $gt: 0 safety and existing try/catch and logging.

Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ const PreferencesNotificationsSection = () => {
</FieldRow>
</Field>
)}
<Field>
<FieldRow>
<FieldLabel>{t('Receive_Reaction_Notifications')}</FieldLabel>
<Controller
name='receiveReactionNotifications'
control={control}
render={({ field: { value, ...field } }) => <ToggleSwitch {...field} checked={value} />}
/>
</FieldRow>
</Field>
</FieldGroup>
</AccordionItem>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type AccountPreferencesData = {
notificationsSoundVolume?: number;
voipRingerVolume?: number;
desktopNotificationVoiceCalls?: boolean;
receiveReactionNotifications?: boolean;
};

export const useAccountPreferencesValues = (): AccountPreferencesData => {
Expand All @@ -55,6 +56,7 @@ export const useAccountPreferencesValues = (): AccountPreferencesData => {
const receiveLoginDetectionEmail = useUserPreference<boolean>('receiveLoginDetectionEmail', true);
const notifyCalendarEvents = useUserPreference<boolean>('notifyCalendarEvents');
const enableMobileRinging = useUserPreference<boolean>('enableMobileRinging');
const receiveReactionNotifications = useUserPreference<boolean>('receiveReactionNotifications');

const unreadAlert = useUserPreference<boolean>('unreadAlert');
const showThreadsInMainChannel = useUserPreference<boolean>('showThreadsInMainChannel');
Expand Down Expand Up @@ -111,5 +113,6 @@ export const useAccountPreferencesValues = (): AccountPreferencesData => {
notificationsSoundVolume,
voipRingerVolume,
desktopNotificationVoiceCalls,
receiveReactionNotifications,
};
};
6 changes: 6 additions & 0 deletions apps/meteor/server/settings/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,12 @@ export const createAccountSettings = () =>
type: 'string',
public: true,
});

await this.add('Accounts_Default_User_Preferences_receiveReactionNotifications', true, {
type: 'boolean',
public: true,
i18nLabel: 'Receive_Reaction_Notifications',
});
});

await this.section('Avatar', async function () {
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -4340,6 +4340,7 @@
"React_when_read_only_changed_successfully": "Allow reacting when read only changed successfully",
"React_with__reaction__": "Reacted with {{reaction}}",
"Reacted_with": "Reacted with",
"Reaction_Notification": "{{reaction}} to your message: {{message}}",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
"Reactions": "Reactions",
"Read_Receipts": "Read receipts",
"Read_by": "Read by",
Expand All @@ -4360,6 +4361,7 @@
"Receive_Group_Mentions": "Receive @all and @here mentions",
"Receive_Login_Detection_Emails": "Receive login detection emails",
"Receive_Login_Detection_Emails_Description": "Receive an email each time a new login is detected on your account.",
"Receive_Reaction_Notifications": "Receive Reaction Notifications",
"Receive_alerts": "Receive alerts",
"Receive_login_notifications": "Receive login notifications",
"Recent": "Recent",
Expand Down