From 6d478224692c53aebae5c646b87283cd972dbb61 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 5 Aug 2024 14:58:22 -0600 Subject: [PATCH 01/19] Refactor setReaction --- apps/meteor/app/api/server/v1/chat.ts | 8 +- .../server/hooks/afterUnsetReaction.js | 2 +- .../lib/server/functions/isTheLastMessage.ts | 4 +- .../reactions/client/methods/setReaction.ts | 4 - .../app/reactions/server/setReaction.ts | 86 +++++++++---------- .../app/slackbridge/server/RocketAdapter.js | 18 ++-- 6 files changed, 55 insertions(+), 67 deletions(-) diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index 3ccc9caeafa0..cf3303bdd3b4 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -364,19 +364,13 @@ API.v1.addRoute( throw new Meteor.Error('error-messageid-param-not-provided', 'The required "messageId" param is missing.'); } - const msg = await Messages.findOneById(this.bodyParams.messageId); - - if (!msg) { - throw new Meteor.Error('error-message-not-found', 'The provided "messageId" does not match any existing message.'); - } - const emoji = 'emoji' in this.bodyParams ? this.bodyParams.emoji : (this.bodyParams as { reaction: string }).reaction; if (!emoji) { throw new Meteor.Error('error-emoji-param-not-provided', 'The required "emoji" param is missing.'); } - await executeSetReaction(this.userId, emoji, msg._id, this.bodyParams.shouldReact); + await executeSetReaction(this.userId, emoji, this.bodyParams.messageId, this.bodyParams.shouldReact); return API.v1.success(); }, diff --git a/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js b/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js index 51181d88ab9e..5243e3259ae9 100644 --- a/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js +++ b/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js @@ -6,7 +6,7 @@ import { getFederationDomain } from '../lib/getFederationDomain'; import { clientLogger } from '../lib/logger'; async function afterUnsetReaction(message, { user, reaction }) { - const room = Rooms.findOneById(message.rid, { fields: { federation: 1 } }); + const room = await Rooms.findOneById(message.rid, { fields: { federation: 1 } }); // If there are not federated users on this room, ignore it if (!hasExternalDomain(room)) { diff --git a/apps/meteor/app/lib/server/functions/isTheLastMessage.ts b/apps/meteor/app/lib/server/functions/isTheLastMessage.ts index f8e5be94002c..2ff40d4828ae 100644 --- a/apps/meteor/app/lib/server/functions/isTheLastMessage.ts +++ b/apps/meteor/app/lib/server/functions/isTheLastMessage.ts @@ -1,7 +1,7 @@ -import type { IMessage, IRoom } from '@rocket.chat/core-typings'; +import type { AtLeast, IMessage, IRoom } from '@rocket.chat/core-typings'; import { settings } from '../../../settings/server'; // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export const isTheLastMessage = (room: IRoom, message: Pick) => +export const isTheLastMessage = (room: AtLeast, message: Pick) => settings.get('Store_Last_Message') && (!room.lastMessage || room.lastMessage._id === message._id); diff --git a/apps/meteor/app/reactions/client/methods/setReaction.ts b/apps/meteor/app/reactions/client/methods/setReaction.ts index ed15cda9ab8e..1744d49c0ceb 100644 --- a/apps/meteor/app/reactions/client/methods/setReaction.ts +++ b/apps/meteor/app/reactions/client/methods/setReaction.ts @@ -3,7 +3,6 @@ import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Meteor } from 'meteor/meteor'; import { roomCoordinator } from '../../../../client/lib/rooms/roomCoordinator'; -import { callbacks } from '../../../../lib/callbacks'; import { emoji } from '../../../emoji/client'; import { Messages, ChatRoom, Subscriptions } from '../../../models/client'; @@ -55,10 +54,8 @@ Meteor.methods({ if (!message.reactions || typeof message.reactions !== 'object' || Object.keys(message.reactions).length === 0) { delete message.reactions; Messages.update({ _id: messageId }, { $unset: { reactions: 1 } }); - await callbacks.run('unsetReaction', messageId, reaction); } else { Messages.update({ _id: messageId }, { $set: { reactions: message.reactions } }); - await callbacks.run('setReaction', messageId, reaction); } } else { if (!message.reactions) { @@ -72,7 +69,6 @@ Meteor.methods({ message.reactions[reaction].usernames.push(user.username); Messages.update({ _id: messageId }, { $set: { reactions: message.reactions } }); - await callbacks.run('setReaction', messageId, reaction); } }, }); diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index e35103e9d333..1ac765538ddb 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -4,7 +4,6 @@ import type { IMessage, IRoom, IUser } from '@rocket.chat/core-typings'; import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Messages, EmojiCustom, Rooms, Users } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; -import _ from 'underscore'; import { callbacks } from '../../../lib/callbacks'; import { i18n } from '../../../server/lib/i18n'; @@ -20,18 +19,22 @@ const removeUserReaction = (message: IMessage, reaction: string, username: strin } message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(username), 1); - if (message.reactions[reaction].usernames.length === 0) { + if (!message.reactions[reaction].usernames.length) { delete message.reactions[reaction]; } return message; }; -async function setReaction(room: IRoom, user: IUser, message: IMessage, reaction: string, shouldReact?: boolean) { - reaction = `:${reaction.replace(/:/g, '')}:`; - - if (!emoji.list[reaction] && (await EmojiCustom.findByNameOrAlias(reaction, {}).count()) === 0) { - throw new Meteor.Error('error-not-allowed', 'Invalid emoji provided.', { - method: 'setReaction', +async function setReaction( + room: Pick, + user: IUser, + message: IMessage, + reaction: string, + userAlreadyReacted?: boolean, +) { + if (Array.isArray(room.muted) && room.muted.includes(user.username as string)) { + throw new Meteor.Error('error-not-allowed', i18n.t('You_have_been_muted', { lng: user.language }), { + rid: room._id, }); } @@ -42,49 +45,24 @@ async function setReaction(room: IRoom, user: IUser, message: IMessage, reaction } } - if (Array.isArray(room.muted) && room.muted.indexOf(user.username as string) !== -1) { - throw new Meteor.Error('error-not-allowed', i18n.t('You_have_been_muted', { lng: user.language }), { - rid: room._id, - }); - } - - // if (!('reactions' in message)) { - // return; - // } - - const userAlreadyReacted = - message.reactions && - Boolean(message.reactions[reaction]) && - message.reactions[reaction].usernames.indexOf(user.username as string) !== -1; - // When shouldReact was not informed, toggle the reaction. - if (shouldReact === undefined) { - shouldReact = !userAlreadyReacted; - } - - if (userAlreadyReacted === shouldReact) { - return; - } - let isReacted; if (userAlreadyReacted) { const oldMessage = JSON.parse(JSON.stringify(message)); removeUserReaction(message, reaction, user.username as string); - if (_.isEmpty(message.reactions)) { + if (Object.keys(message.reactions || {}).length === 0) { delete message.reactions; + await Messages.unsetReactions(message._id); if (isTheLastMessage(room, message)) { await Rooms.unsetReactionsInLastMessage(room._id); - void notifyOnRoomChangedById(room._id); } - await Messages.unsetReactions(message._id); } else { await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); } } - await callbacks.run('unsetReaction', message._id, reaction); - await callbacks.run('afterUnsetReaction', message, { user, reaction, shouldReact, oldMessage }); + void callbacks.run('afterUnsetReaction', message, { user, reaction, shouldReact: false, oldMessage }); isReacted = false; } else { @@ -100,24 +78,31 @@ async function setReaction(room: IRoom, user: IUser, message: IMessage, reaction await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); - void notifyOnRoomChangedById(room._id); } - await callbacks.run('setReaction', message._id, reaction); - await callbacks.run('afterSetReaction', message, { user, reaction, shouldReact }); + void callbacks.run('afterSetReaction', message, { user, reaction, shouldReact: true }); isReacted = true; } - await Apps.self?.triggerEvent(AppEvents.IPostMessageReacted, message, user, reaction, isReacted); + void Apps.self?.triggerEvent(AppEvents.IPostMessageReacted, message, user, reaction, isReacted); + void notifyOnRoomChangedById(room._id); void notifyOnMessageChange({ id: message._id, }); } export async function executeSetReaction(userId: string, reaction: string, messageId: IMessage['_id'], shouldReact?: boolean) { - const user = await Users.findOneById(userId); + // Check if the emoji is valid before proceeding + reaction = `:${reaction.replace(/:/g, '')}:`; + if (!emoji.list[reaction] && (await EmojiCustom.findByNameOrAlias(reaction, {}).count()) === 0) { + throw new Meteor.Error('error-not-allowed', 'Invalid emoji provided.', { + method: 'setReaction', + }); + } + + const user = await Users.findOneById(userId); if (!user) { throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setReaction' }); } @@ -127,7 +112,22 @@ export async function executeSetReaction(userId: string, reaction: string, messa throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' }); } - const room = await Rooms.findOneById(message.rid); + const userAlreadyReacted = + message.reactions && Boolean(message.reactions[reaction]) && message.reactions[reaction].usernames.includes(user.username as string); + + // When shouldReact was not informed, toggle the reaction. + if (shouldReact === undefined) { + shouldReact = !userAlreadyReacted; + } + + if (userAlreadyReacted === shouldReact) { + return; + } + + const room = await Rooms.findOneById>( + message.rid, + { projection: { _id: 1, ro: 1, muted: 1, reactWhenReadOnly: 1, lastMessage: 1, t: 1, prid: 1 } }, + ); if (!room) { throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' }); } @@ -136,7 +136,7 @@ export async function executeSetReaction(userId: string, reaction: string, messa throw new Meteor.Error('not-authorized', 'Not Authorized', { method: 'setReaction' }); } - return setReaction(room, user, message, reaction, shouldReact); + return setReaction(room, user, message, reaction, userAlreadyReacted); } declare module '@rocket.chat/ddp-client' { diff --git a/apps/meteor/app/slackbridge/server/RocketAdapter.js b/apps/meteor/app/slackbridge/server/RocketAdapter.js index f76c33fa1f81..f0f59c0b4cb5 100644 --- a/apps/meteor/app/slackbridge/server/RocketAdapter.js +++ b/apps/meteor/app/slackbridge/server/RocketAdapter.js @@ -45,16 +45,16 @@ export default class RocketAdapter { rocketLogger.debug('Register for events'); callbacks.add('afterSaveMessage', this.onMessage.bind(this), callbacks.priority.LOW, 'SlackBridge_Out'); callbacks.add('afterDeleteMessage', this.onMessageDelete.bind(this), callbacks.priority.LOW, 'SlackBridge_Delete'); - callbacks.add('setReaction', this.onSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_SetReaction'); - callbacks.add('unsetReaction', this.onUnSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_UnSetReaction'); + callbacks.add('afterSetReaction', this.onSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_SetReaction'); + callbacks.add('afterUnsetReaction', this.onUnSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_UnSetReaction'); } unregisterForEvents() { rocketLogger.debug('Unregister for events'); callbacks.remove('afterSaveMessage', 'SlackBridge_Out'); callbacks.remove('afterDeleteMessage', 'SlackBridge_Delete'); - callbacks.remove('setReaction', 'SlackBridge_SetReaction'); - callbacks.remove('unsetReaction', 'SlackBridge_UnSetReaction'); + callbacks.remove('afterSetReaction', 'SlackBridge_SetReaction'); + callbacks.remove('afterUnsetReaction', 'SlackBridge_UnSetReaction'); } async onMessageDelete(rocketMessageDeleted) { @@ -72,7 +72,7 @@ export default class RocketAdapter { } } - async onSetReaction(rocketMsgID, reaction) { + async onSetReaction(rocketMsg, { reaction }) { try { if (!this.slackBridge.isReactionsEnabled) { return; @@ -80,12 +80,11 @@ export default class RocketAdapter { rocketLogger.debug('onRocketSetReaction'); - if (rocketMsgID && reaction) { + if (rocketMsg._id && reaction) { if (this.slackBridge.reactionsMap.delete(`set${rocketMsgID}${reaction}`)) { // This was a Slack reaction, we don't need to tell Slack about it return; } - const rocketMsg = await Messages.findOneById(rocketMsgID); if (rocketMsg) { for await (const slack of this.slackAdapters) { const slackChannel = slack.getSlackChannel(rocketMsg.rid); @@ -101,7 +100,7 @@ export default class RocketAdapter { } } - async onUnSetReaction(rocketMsgID, reaction) { + async onUnSetReaction(rocketMsg, { reaction }) { try { if (!this.slackBridge.isReactionsEnabled) { return; @@ -109,13 +108,12 @@ export default class RocketAdapter { rocketLogger.debug('onRocketUnSetReaction'); - if (rocketMsgID && reaction) { + if (rocketMsg._id && reaction) { if (this.slackBridge.reactionsMap.delete(`unset${rocketMsgID}${reaction}`)) { // This was a Slack unset reaction, we don't need to tell Slack about it return; } - const rocketMsg = await Messages.findOneById(rocketMsgID); if (rocketMsg) { for await (const slack of this.slackAdapters) { const slackChannel = slack.getSlackChannel(rocketMsg.rid); From 26d44f532e284acd37c008bee4b5654c66b8c534 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 6 Aug 2024 11:18:43 -0600 Subject: [PATCH 02/19] Change find with countDocuments and add tests --- .../app/reactions/server/setReaction.ts | 9 +- apps/meteor/server/models/raw/EmojiCustom.ts | 10 +- .../app/reactions/server/setReaction.spec.ts | 406 ++++++++++++++++++ 3 files changed, 420 insertions(+), 5 deletions(-) create mode 100644 apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index 1ac765538ddb..2cce1b7ed7d1 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -13,7 +13,7 @@ import { emoji } from '../../emoji/server'; import { isTheLastMessage } from '../../lib/server/functions/isTheLastMessage'; import { notifyOnRoomChangedById, notifyOnMessageChange } from '../../lib/server/lib/notifyListener'; -const removeUserReaction = (message: IMessage, reaction: string, username: string) => { +export const removeUserReaction = (message: IMessage, reaction: string, username: string) => { if (!message.reactions) { return message; } @@ -25,7 +25,7 @@ const removeUserReaction = (message: IMessage, reaction: string, username: strin return message; }; -async function setReaction( +export async function setReaction( room: Pick, user: IUser, message: IMessage, @@ -94,9 +94,10 @@ async function setReaction( export async function executeSetReaction(userId: string, reaction: string, messageId: IMessage['_id'], shouldReact?: boolean) { // Check if the emoji is valid before proceeding - reaction = `:${reaction.replace(/:/g, '')}:`; + const reactionWithoutColons = reaction.replace(/:/g, ''); + reaction = `:${reactionWithoutColons}:`; - if (!emoji.list[reaction] && (await EmojiCustom.findByNameOrAlias(reaction, {}).count()) === 0) { + if (!emoji.list[reaction] && (await EmojiCustom.countByNameOrAlias(reactionWithoutColons)) === 0) { throw new Meteor.Error('error-not-allowed', 'Invalid emoji provided.', { method: 'setReaction', }); diff --git a/apps/meteor/server/models/raw/EmojiCustom.ts b/apps/meteor/server/models/raw/EmojiCustom.ts index 300721f60d7b..1b45b5ed2b00 100644 --- a/apps/meteor/server/models/raw/EmojiCustom.ts +++ b/apps/meteor/server/models/raw/EmojiCustom.ts @@ -1,6 +1,6 @@ import type { IEmojiCustom, RocketChatRecordDeleted } from '@rocket.chat/core-typings'; import type { IEmojiCustomModel, InsertionModel } from '@rocket.chat/model-typings'; -import type { Collection, FindCursor, Db, FindOptions, IndexDescription, InsertOneResult, UpdateResult, WithId } from 'mongodb'; +import type { Collection, FindCursor, Db, FindOptions, IndexDescription, InsertOneResult, UpdateResult, WithId, CountDocumentsOptions } from 'mongodb'; import { BaseRaw } from './BaseRaw'; @@ -72,4 +72,12 @@ export class EmojiCustomRaw extends BaseRaw implements IEmojiCusto create(data: InsertionModel): Promise>> { return this.insertOne(data); } + + countByNameOrAlias(name: string): Promise { + const query = { + $or: [{ name }, { aliases: name }], + }; + + return this.countDocuments(query); + } } diff --git a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts new file mode 100644 index 000000000000..8c5fcc1b5c4b --- /dev/null +++ b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts @@ -0,0 +1,406 @@ +import { expect } from 'chai'; +import { beforeEach, describe, it } from 'mocha'; +import p from 'proxyquire'; +import sinon from 'sinon'; + +const meteorMethodsMock = sinon.stub(); +const emojiList: Record = {}; +const modelsMock = { + EmojiCustom: { + countByNameOrAlias: sinon.stub(), + }, + Users: { + findOneById: sinon.stub(), + }, + Messages: { + findOneById: sinon.stub(), + setReactions: sinon.stub(), + unsetReactions: sinon.stub(), + }, + Rooms: { + findOneById: sinon.stub(), + unsetReactionsInLastMessage: sinon.stub(), + setReactionsInLastMessage: sinon.stub(), + }, +}; +const canAccessRoomAsyncMock = sinon.stub(); +const isTheLastMessageMock = sinon.stub(); +const notifyOnRoomChangedByIdMock = sinon.stub(); +const notifyOnMessageChangeMock = sinon.stub(); +const hasPermissionAsyncMock = sinon.stub(); +const i18nMock = { t: sinon.stub() }; +const callbacksRunMock = sinon.stub(); +const meteorErrorMock = class extends Error { + constructor(message: string) { + super(message); + } +}; + +const { removeUserReaction, executeSetReaction, setReaction } = p.noCallThru().load('../../../../../app/reactions/server/setReaction.ts', { + '@rocket.chat/models': modelsMock, + 'meteor/meteor': { Meteor: { methods: meteorMethodsMock, Error: meteorErrorMock } }, + '../../../lib/callbacks': { callbacks: { run: callbacksRunMock } }, + '../../../server/lib/i18n': { i18n: i18nMock }, + '../../authorization/server': { canAccessRoomAsync: canAccessRoomAsyncMock }, + '../../authorization/server/functions/hasPermission': { hasPermissionAsync: hasPermissionAsyncMock }, + '../../emoji/server': { emoji: { list: emojiList } }, + '../../lib/server/functions/isTheLastMessage': { isTheLastMessage: isTheLastMessageMock }, + '../../lib/server/lib/notifyListener': { + notifyOnRoomChangedById: notifyOnRoomChangedByIdMock, + notifyOnMessageChange: notifyOnMessageChangeMock, + }, +}); + +describe('Reactions', () => { + describe('removeUserReaction', () => { + it('should return the message unmodified when no reactions exist', () => { + const message = {}; + + const result = removeUserReaction(message as any, 'test', 'test'); + expect(result).to.equal(message); + }); + it('should remove the reaction from a message', () => { + const message = { + reactions: { + test: { + usernames: ['test', 'test2'], + }, + }, + }; + + const result = removeUserReaction(message as any, 'test', 'test'); + expect(result.reactions.test.usernames).to.not.include('test'); + expect(result.reactions.test.usernames).to.include('test2'); + }); + it('should remove the reaction from a message when the user is the last one on the array', () => { + const message = { + reactions: { + test: { + usernames: ['test'], + }, + }, + }; + + const result = removeUserReaction(message as any, 'test', 'test'); + expect(result.reactions.test).to.be.undefined; + }); + it('should remove username only from the reaction thats passed in', () => { + const message = { + reactions: { + test: { + usernames: ['test', 'test2'], + }, + other: { + usernames: ['test', 'test2'], + }, + }, + }; + + const result = removeUserReaction(message as any, 'test', 'test'); + expect(result.reactions.test.usernames).to.not.include('test'); + expect(result.reactions.test.usernames).to.include('test2'); + expect(result.reactions.other.usernames).to.include('test'); + expect(result.reactions.other.usernames).to.include('test2'); + }); + }); + describe('executeSetReaction', () => { + beforeEach(() => { + modelsMock.EmojiCustom.countByNameOrAlias.reset(); + }); + it('should throw an error if reaction is not on emojione list', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(0); + await expect(executeSetReaction('test', 'test', 'test')).to.be.rejectedWith('error-not-allowed'); + }); + it('should fail if user does not exist', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + await expect(executeSetReaction('test', 'test', 'test')).to.be.rejectedWith('error-invalid-user'); + }); + it('should fail if message does not exist', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + await expect(executeSetReaction('test', 'test', 'test')).to.be.rejectedWith('error-not-allowed'); + }); + it('should return nothing if user already reacted and its trying to react again', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Messages.findOneById.resolves({ reactions: { ':test:': { usernames: ['test'] } } }); + expect(await executeSetReaction('test', 'test', 'test', true)).to.be.undefined; + }); + it('should return nothing if user hasnt reacted and its trying to unreact', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Messages.findOneById.resolves({ reactions: { ':test:': { usernames: ['testxxxx'] } } }); + expect(await executeSetReaction('test', 'test', 'test', false)).to.be.undefined; + }); + it('should fail if room does not exist', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Messages.findOneById.resolves({ reactions: { ':test:': { usernames: ['test'] } } }); + modelsMock.Rooms.findOneById.resolves(undefined); + await expect(executeSetReaction('test', 'test', 'test')).to.be.rejectedWith('error-not-allowed'); + }); + it('should fail if user doesnt have acccess to the room', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Messages.findOneById.resolves({ reactions: { ':test:': { usernames: ['test'] } } }); + modelsMock.Rooms.findOneById.resolves({ t: 'd' }); + canAccessRoomAsyncMock.resolves(false); + await expect(executeSetReaction('test', 'test', 'test')).to.be.rejectedWith('not-authorized'); + }); + it('should call setReaction with correct params', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Messages.findOneById.resolves({ reactions: { ':test:': { usernames: ['test'] } } }); + modelsMock.Rooms.findOneById.resolves({ t: 'c' }); + canAccessRoomAsyncMock.resolves(true); + + const res = await executeSetReaction('test', 'test', 'test'); + expect(res).to.be.undefined; + }); + }); + describe('setReaction', () => { + beforeEach(() => { + canAccessRoomAsyncMock.reset(); + hasPermissionAsyncMock.reset(); + isTheLastMessageMock.reset(); + modelsMock.Messages.setReactions.reset(); + modelsMock.Rooms.setReactionsInLastMessage.reset(); + modelsMock.Rooms.unsetReactionsInLastMessage.reset(); + modelsMock.Messages.unsetReactions.reset(); + callbacksRunMock.reset(); + }); + it('should throw an error if user is muted from the room', async () => { + const room = { + muted: ['test'], + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + await expect(setReaction(room, user, message, ':test:')).to.be.rejectedWith('error-not-allowed'); + }); + it('should throw an error if room is readonly and cannot be reacted when readonly and user trying doesnt have permissions and user is not unmuted from room', async () => { + const room = { + ro: true, + reactWhenReadOnly: false, + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + canAccessRoomAsyncMock.resolves(false); + await expect(setReaction(room, user, message, ':test:')).to.be.rejectedWith("You can't send messages because the room is readonly."); + }); + it('should remove the user reaction if userAlreadyReacted is true and call unsetReaction if reaction is the last one on message', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + await setReaction(room, user, message, reaction, true); + expect(modelsMock.Messages.unsetReactions.calledWith(message._id)).to.be.true; + }); + it('should call Rooms.unsetReactionsInLastMessage when userAlreadyReacted is true and reaction is the last one on message', async () => { + const room = { + _id: 'test', + lastMessage: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + isTheLastMessageMock.resolves(true); + + await setReaction(room, user, message, reaction, true); + expect(modelsMock.Messages.unsetReactions.calledWith(message._id)).to.be.true; + expect(modelsMock.Rooms.unsetReactionsInLastMessage.calledWith(room._id)).to.be.true; + }); + it('should update the reactions object when userAlreadyReacted is true and there is more reactions on message', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + ':test2:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + await setReaction(room, user, message, reaction, true); + expect(modelsMock.Messages.setReactions.calledWith(message._id, sinon.match({ ':test2:': { usernames: ['test'] } }))).to.be.true; + }); + it('should call Rooms.setReactionsInLastMessage when userAlreadyReacted is true and reaction is not the last one on message', async () => { + const room = { + _id: 'test', + lastMessage: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + ':test2:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + isTheLastMessageMock.resolves(true); + + await setReaction(room, user, message, reaction, true); + expect(modelsMock.Messages.setReactions.calledWith(message._id, sinon.match({ ':test2:': { usernames: ['test'] } }))).to.be.true; + expect(modelsMock.Rooms.setReactionsInLastMessage.calledWith(room._id, sinon.match({ ':test2:': { usernames: ['test'] } }))).to.be + .true; + }); + it('should call afterUnsetReaction callback when userAlreadyReacted is true', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + await setReaction(room, user, message, reaction, true); + expect( + callbacksRunMock.calledWith( + 'afterUnsetReaction', + sinon.match({ _id: 'test' }), + sinon.match({ user, reaction, shouldReact: false, oldMessage: message }), + ), + ).to.be.true; + }); + it('should set reactions when userAlreadyReacted is false', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + const reaction = ':test:'; + await setReaction(room, user, message, reaction, false); + expect(modelsMock.Messages.setReactions.calledWith(message._id, sinon.match({ ':test:': { usernames: ['test'] } }))).to.be.true; + }); + it('should properly add username to the list of reactions when userAlreadyReacted is false', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test2', + }; + const message = { + _id: 'test', + reactions: { + ':test:': { + usernames: ['test'], + }, + }, + }; + const reaction = ':test:'; + + await setReaction(room, user, message, reaction, false); + expect(modelsMock.Messages.setReactions.calledWith(message._id, sinon.match({ ':test:': { usernames: ['test', 'test2'] } }))).to.be + .true; + }); + it('should call Rooms.setReactionInLastMessage when userAlreadyReacted is false', async () => { + const room = { + _id: 'x5', + lastMessage: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + const reaction = ':test:'; + + isTheLastMessageMock.resolves(true); + + await setReaction(room, user, message, reaction, false); + expect(modelsMock.Messages.setReactions.calledWith(message._id, sinon.match({ ':test:': { usernames: ['test'] } }))).to.be.true; + expect(modelsMock.Rooms.setReactionsInLastMessage.calledWith(room._id, sinon.match({ ':test:': { usernames: ['test'] } }))).to.be + .true; + }); + it('should call afterSetReaction callback when userAlreadyReacted is false', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + const reaction = ':test:'; + + await setReaction(room, user, message, reaction, false); + expect( + callbacksRunMock.calledWith('afterSetReaction', sinon.match({ _id: 'test' }), sinon.match({ user, reaction, shouldReact: true })), + ).to.be.true; + }); + it('should return undefined on a successful reaction', async () => { + const room = { + _id: 'test', + }; + const user = { + username: 'test', + }; + const message = { + _id: 'test', + }; + const reaction = ':test:'; + + expect(await setReaction(room, user, message, reaction, false)).to.be.undefined; + }); + }); +}); From b6bda018f4daff6735d56de0c5661667f1fb555e Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 6 Aug 2024 12:16:12 -0600 Subject: [PATCH 03/19] ts --- apps/meteor/server/models/raw/EmojiCustom.ts | 2 +- packages/model-typings/src/models/IEmojiCustomModel.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/meteor/server/models/raw/EmojiCustom.ts b/apps/meteor/server/models/raw/EmojiCustom.ts index 1b45b5ed2b00..ec3c6390f64d 100644 --- a/apps/meteor/server/models/raw/EmojiCustom.ts +++ b/apps/meteor/server/models/raw/EmojiCustom.ts @@ -1,6 +1,6 @@ import type { IEmojiCustom, RocketChatRecordDeleted } from '@rocket.chat/core-typings'; import type { IEmojiCustomModel, InsertionModel } from '@rocket.chat/model-typings'; -import type { Collection, FindCursor, Db, FindOptions, IndexDescription, InsertOneResult, UpdateResult, WithId, CountDocumentsOptions } from 'mongodb'; +import type { Collection, FindCursor, Db, FindOptions, IndexDescription, InsertOneResult, UpdateResult, WithId } from 'mongodb'; import { BaseRaw } from './BaseRaw'; diff --git a/packages/model-typings/src/models/IEmojiCustomModel.ts b/packages/model-typings/src/models/IEmojiCustomModel.ts index fba5f1c3ea10..30f0323c1ec7 100644 --- a/packages/model-typings/src/models/IEmojiCustomModel.ts +++ b/packages/model-typings/src/models/IEmojiCustomModel.ts @@ -10,4 +10,5 @@ export interface IEmojiCustomModel extends IBaseModel { setAliases(_id: string, aliases: string[]): Promise; setExtension(_id: string, extension: string): Promise; create(data: InsertionModel): Promise>>; + countByNameOrAlias(name: string): Promise; } From 17ced603f39b6f32e3e959a4adc5c970d70ef08b Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 6 Aug 2024 13:40:47 -0600 Subject: [PATCH 04/19] lint --- apps/meteor/app/slackbridge/server/RocketAdapter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/slackbridge/server/RocketAdapter.js b/apps/meteor/app/slackbridge/server/RocketAdapter.js index f0f59c0b4cb5..8ba2a76dcbc2 100644 --- a/apps/meteor/app/slackbridge/server/RocketAdapter.js +++ b/apps/meteor/app/slackbridge/server/RocketAdapter.js @@ -81,7 +81,7 @@ export default class RocketAdapter { rocketLogger.debug('onRocketSetReaction'); if (rocketMsg._id && reaction) { - if (this.slackBridge.reactionsMap.delete(`set${rocketMsgID}${reaction}`)) { + if (this.slackBridge.reactionsMap.delete(`set${rocketMsg._id}${reaction}`)) { // This was a Slack reaction, we don't need to tell Slack about it return; } @@ -109,7 +109,7 @@ export default class RocketAdapter { rocketLogger.debug('onRocketUnSetReaction'); if (rocketMsg._id && reaction) { - if (this.slackBridge.reactionsMap.delete(`unset${rocketMsgID}${reaction}`)) { + if (this.slackBridge.reactionsMap.delete(`unset${rocketMsg._id}${reaction}`)) { // This was a Slack unset reaction, we don't need to tell Slack about it return; } From 1aa1cd52b5b574f2ade0f0bdea21f8a3a4c1b961 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 12 Aug 2024 08:16:40 -0600 Subject: [PATCH 05/19] cr --- apps/meteor/app/api/server/v1/chat.ts | 8 ++++++- .../app/reactions/server/setReaction.ts | 13 +++++++--- apps/meteor/lib/callbacks.ts | 2 -- .../app/reactions/server/setReaction.spec.ts | 24 +++++++++++++++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index cf3303bdd3b4..d04d1a2418b5 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -364,13 +364,19 @@ API.v1.addRoute( throw new Meteor.Error('error-messageid-param-not-provided', 'The required "messageId" param is missing.'); } + const msg = await Messages.findOneById(this.bodyParams.messageId); + + if (!msg) { + throw new Meteor.Error('error-message-not-found', 'The provided "messageId" does not match any existing message.'); + } + const emoji = 'emoji' in this.bodyParams ? this.bodyParams.emoji : (this.bodyParams as { reaction: string }).reaction; if (!emoji) { throw new Meteor.Error('error-emoji-param-not-provided', 'The required "emoji" param is missing.'); } - await executeSetReaction(this.userId, emoji, this.bodyParams.messageId, this.bodyParams.shouldReact); + await executeSetReaction(this.userId, emoji, msg, this.bodyParams.shouldReact); return API.v1.success(); }, diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index 2cce1b7ed7d1..14f13bbb5b39 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -18,7 +18,14 @@ export const removeUserReaction = (message: IMessage, reaction: string, username return message; } - message.reactions[reaction].usernames.splice(message.reactions[reaction].usernames.indexOf(username), 1); + const idx = message.reactions[reaction].usernames.indexOf(username); + + // user not found in reaction array + if (idx === -1) { + return message; + } + + message.reactions[reaction].usernames.splice(idx, 1); if (!message.reactions[reaction].usernames.length) { delete message.reactions[reaction]; } @@ -92,7 +99,7 @@ export async function setReaction( }); } -export async function executeSetReaction(userId: string, reaction: string, messageId: IMessage['_id'], shouldReact?: boolean) { +export async function executeSetReaction(userId: string, reaction: string, messageParam: IMessage['_id'] | IMessage, shouldReact?: boolean) { // Check if the emoji is valid before proceeding const reactionWithoutColons = reaction.replace(/:/g, ''); reaction = `:${reactionWithoutColons}:`; @@ -108,7 +115,7 @@ export async function executeSetReaction(userId: string, reaction: string, messa throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setReaction' }); } - const message = await Messages.findOneById(messageId); + const message = typeof messageParam === 'string' ? await Messages.findOneById(messageParam) : messageParam; if (!message) { throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' }); } diff --git a/apps/meteor/lib/callbacks.ts b/apps/meteor/lib/callbacks.ts index eb8e032804f7..83222cf7b5cf 100644 --- a/apps/meteor/lib/callbacks.ts +++ b/apps/meteor/lib/callbacks.ts @@ -254,10 +254,8 @@ export type Hook = | 'onValidateLogin' | 'openBroadcast' | 'renderNotification' - | 'setReaction' | 'streamMessage' | 'streamNewMessage' - | 'unsetReaction' | 'userAvatarSet' | 'userConfirmationEmailRequested' | 'userForgotPasswordEmailRequested' diff --git a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts index 8c5fcc1b5c4b..ba134cb35d0e 100644 --- a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts +++ b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts @@ -102,6 +102,20 @@ describe('Reactions', () => { expect(result.reactions.other.usernames).to.include('test'); expect(result.reactions.other.usernames).to.include('test2'); }); + it('should do nothing if username is not in the reaction', () => { + const message = { + reactions: { + test: { + usernames: ['test', 'test2'], + }, + }, + }; + + const result = removeUserReaction(message as any, 'test', 'test3'); + expect(result.reactions.test.usernames).to.not.include('test3'); + expect(result.reactions.test.usernames).to.include('test'); + expect(result.reactions.test.usernames).to.include('test2'); + }); }); describe('executeSetReaction', () => { beforeEach(() => { @@ -157,6 +171,16 @@ describe('Reactions', () => { const res = await executeSetReaction('test', 'test', 'test'); expect(res).to.be.undefined; }); + it('should use the message from param when the type is not an string', async () => { + modelsMock.EmojiCustom.countByNameOrAlias.resolves(1); + modelsMock.Users.findOneById.resolves({ username: 'test' }); + modelsMock.Rooms.findOneById.resolves({ t: 'c' }); + canAccessRoomAsyncMock.resolves(true); + + await executeSetReaction('test', 'test', { reactions: { ':test:': { usernames: ['test'] } } }); + expect(modelsMock.Messages.findOneById.calledOnce).to.be.false; + }); + }); describe('setReaction', () => { beforeEach(() => { From 3cc2055dfb338e6e41b1c604d56ee3f0538bca6a Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 12 Aug 2024 10:32:00 -0600 Subject: [PATCH 06/19] lint --- apps/meteor/app/reactions/server/setReaction.ts | 7 ++++++- .../tests/unit/app/reactions/server/setReaction.spec.ts | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index 14f13bbb5b39..b0bc4f42f6f2 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -99,7 +99,12 @@ export async function setReaction( }); } -export async function executeSetReaction(userId: string, reaction: string, messageParam: IMessage['_id'] | IMessage, shouldReact?: boolean) { +export async function executeSetReaction( + userId: string, + reaction: string, + messageParam: IMessage['_id'] | IMessage, + shouldReact?: boolean, +) { // Check if the emoji is valid before proceeding const reactionWithoutColons = reaction.replace(/:/g, ''); reaction = `:${reactionWithoutColons}:`; diff --git a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts index ba134cb35d0e..e4a819b3ce2b 100644 --- a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts +++ b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts @@ -180,7 +180,6 @@ describe('Reactions', () => { await executeSetReaction('test', 'test', { reactions: { ':test:': { usernames: ['test'] } } }); expect(modelsMock.Messages.findOneById.calledOnce).to.be.false; }); - }); describe('setReaction', () => { beforeEach(() => { From 4a0d65d5e7c4d10fd40d31da8736a330890bbac2 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Wed, 21 Aug 2024 09:51:30 -0600 Subject: [PATCH 07/19] accept at least federated key --- apps/meteor/app/reactions/server/setReaction.ts | 9 ++++----- .../services/messages/hooks/BeforeFederationActions.ts | 3 ++- apps/meteor/server/services/messages/service.ts | 3 ++- packages/core-services/src/types/IMessageService.ts | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index b03c4c257ba4..90eb5c90e54c 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -33,7 +33,7 @@ export const removeUserReaction = (message: IMessage, reaction: string, username }; export async function setReaction( - room: Pick, + room: Pick, user: IUser, message: IMessage, reaction: string, @@ -138,10 +138,9 @@ export async function executeSetReaction( return; } - const room = await Rooms.findOneById>( - message.rid, - { projection: { _id: 1, ro: 1, muted: 1, reactWhenReadOnly: 1, lastMessage: 1, t: 1, prid: 1 } }, - ); + const room = await Rooms.findOneById< + Pick + >(message.rid, { projection: { _id: 1, ro: 1, muted: 1, reactWhenReadOnly: 1, lastMessage: 1, t: 1, prid: 1, federated: 1 } }); if (!room) { throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setReaction' }); } diff --git a/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts b/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts index a954e4899970..19f42626c216 100644 --- a/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts +++ b/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts @@ -1,9 +1,10 @@ +import type { AtLeast } from '@rocket.chat/core-typings'; import { type IMessage, type IRoom, isMessageFromMatrixFederation, isRoomFederated } from '@rocket.chat/core-typings'; import { isFederationEnabled, isFederationReady } from '../../federation/utils'; export class FederationActions { - public static shouldPerformAction(message: IMessage, room: IRoom): boolean { + public static shouldPerformAction(message: IMessage, room: AtLeast): boolean { if (isMessageFromMatrixFederation(message) || isRoomFederated(room)) { return isFederationEnabled() && isFederationReady(); } diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts index b20b5236b7fe..a7c6801f5c58 100644 --- a/apps/meteor/server/services/messages/service.ts +++ b/apps/meteor/server/services/messages/service.ts @@ -1,5 +1,6 @@ import type { IMessageService } from '@rocket.chat/core-services'; import { Authorization, ServiceClassInternal } from '@rocket.chat/core-services'; +import type { AtLeast } from '@rocket.chat/core-typings'; import { type IMessage, type MessageTypesValues, type IUser, type IRoom, isEditedMessage } from '@rocket.chat/core-typings'; import { Messages, Rooms } from '@rocket.chat/models'; @@ -244,7 +245,7 @@ export class MessageService extends ServiceClassInternal implements IMessageServ // await Room.join({ room, user }); // } - async beforeReacted(message: IMessage, room: IRoom) { + async beforeReacted(message: IMessage, room: AtLeast) { if (!FederationActions.shouldPerformAction(message, room)) { throw new FederationMatrixInvalidConfigurationError('Unable to react to message'); } diff --git a/packages/core-services/src/types/IMessageService.ts b/packages/core-services/src/types/IMessageService.ts index ca84f78ea677..29da139ef63c 100644 --- a/packages/core-services/src/types/IMessageService.ts +++ b/packages/core-services/src/types/IMessageService.ts @@ -1,4 +1,4 @@ -import type { IMessage, MessageTypesValues, IUser, IRoom } from '@rocket.chat/core-typings'; +import type { IMessage, MessageTypesValues, IUser, IRoom, AtLeast } from '@rocket.chat/core-typings'; export interface IMessageService { sendMessage({ fromId, rid, msg }: { fromId: string; rid: string; msg: string }): Promise; @@ -21,6 +21,6 @@ export interface IMessageService { deleteMessage(user: IUser, message: IMessage): Promise; updateMessage(message: IMessage, user: IUser, originalMsg?: IMessage): Promise; reactToMessage(userId: string, reaction: string, messageId: IMessage['_id'], shouldReact?: boolean): Promise; - beforeReacted(message: IMessage, room: IRoom): Promise; + beforeReacted(message: IMessage, room: AtLeast): Promise; beforeDelete(message: IMessage, room: IRoom): Promise; } From 0349cf48ea6daa2573271de5b48a4a0d6915e62a Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 16 Sep 2024 15:16:55 -0600 Subject: [PATCH 08/19] Update afterUnsetReaction.js --- apps/meteor/app/federation/server/hooks/afterUnsetReaction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js b/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js index 5243e3259ae9..51181d88ab9e 100644 --- a/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js +++ b/apps/meteor/app/federation/server/hooks/afterUnsetReaction.js @@ -6,7 +6,7 @@ import { getFederationDomain } from '../lib/getFederationDomain'; import { clientLogger } from '../lib/logger'; async function afterUnsetReaction(message, { user, reaction }) { - const room = await Rooms.findOneById(message.rid, { fields: { federation: 1 } }); + const room = Rooms.findOneById(message.rid, { fields: { federation: 1 } }); // If there are not federated users on this room, ignore it if (!hasExternalDomain(room)) { From 9d6077ffe20303ac38703f8a8fee1d0201b8bd03 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 17 Sep 2024 10:59:49 -0600 Subject: [PATCH 09/19] Update isTheLastMessage.ts --- apps/meteor/app/lib/server/functions/isTheLastMessage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/isTheLastMessage.ts b/apps/meteor/app/lib/server/functions/isTheLastMessage.ts index 2ff40d4828ae..f8e5be94002c 100644 --- a/apps/meteor/app/lib/server/functions/isTheLastMessage.ts +++ b/apps/meteor/app/lib/server/functions/isTheLastMessage.ts @@ -1,7 +1,7 @@ -import type { AtLeast, IMessage, IRoom } from '@rocket.chat/core-typings'; +import type { IMessage, IRoom } from '@rocket.chat/core-typings'; import { settings } from '../../../settings/server'; // eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export const isTheLastMessage = (room: AtLeast, message: Pick) => +export const isTheLastMessage = (room: IRoom, message: Pick) => settings.get('Store_Last_Message') && (!room.lastMessage || room.lastMessage._id === message._id); From b1d3b609e49aed5a57e98f4b815498b7265fe6c1 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 17 Sep 2024 11:00:40 -0600 Subject: [PATCH 10/19] Update BeforeFederationActions.ts --- .../server/services/messages/hooks/BeforeFederationActions.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts b/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts index 19f42626c216..a954e4899970 100644 --- a/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts +++ b/apps/meteor/server/services/messages/hooks/BeforeFederationActions.ts @@ -1,10 +1,9 @@ -import type { AtLeast } from '@rocket.chat/core-typings'; import { type IMessage, type IRoom, isMessageFromMatrixFederation, isRoomFederated } from '@rocket.chat/core-typings'; import { isFederationEnabled, isFederationReady } from '../../federation/utils'; export class FederationActions { - public static shouldPerformAction(message: IMessage, room: AtLeast): boolean { + public static shouldPerformAction(message: IMessage, room: IRoom): boolean { if (isMessageFromMatrixFederation(message) || isRoomFederated(room)) { return isFederationEnabled() && isFederationReady(); } From c3629a039bb43baa1015b5ad8dc11ec548f2ee45 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 17 Sep 2024 11:01:21 -0600 Subject: [PATCH 11/19] Update service.ts --- apps/meteor/server/services/messages/service.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/meteor/server/services/messages/service.ts b/apps/meteor/server/services/messages/service.ts index a7c6801f5c58..b20b5236b7fe 100644 --- a/apps/meteor/server/services/messages/service.ts +++ b/apps/meteor/server/services/messages/service.ts @@ -1,6 +1,5 @@ import type { IMessageService } from '@rocket.chat/core-services'; import { Authorization, ServiceClassInternal } from '@rocket.chat/core-services'; -import type { AtLeast } from '@rocket.chat/core-typings'; import { type IMessage, type MessageTypesValues, type IUser, type IRoom, isEditedMessage } from '@rocket.chat/core-typings'; import { Messages, Rooms } from '@rocket.chat/models'; @@ -245,7 +244,7 @@ export class MessageService extends ServiceClassInternal implements IMessageServ // await Room.join({ room, user }); // } - async beforeReacted(message: IMessage, room: AtLeast) { + async beforeReacted(message: IMessage, room: IRoom) { if (!FederationActions.shouldPerformAction(message, room)) { throw new FederationMatrixInvalidConfigurationError('Unable to react to message'); } From 0a304beb93b5044a206779e882dc8234bddbd47d Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Tue, 17 Sep 2024 11:01:50 -0600 Subject: [PATCH 12/19] Update IMessageService.ts --- packages/core-services/src/types/IMessageService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core-services/src/types/IMessageService.ts b/packages/core-services/src/types/IMessageService.ts index 29da139ef63c..ca84f78ea677 100644 --- a/packages/core-services/src/types/IMessageService.ts +++ b/packages/core-services/src/types/IMessageService.ts @@ -1,4 +1,4 @@ -import type { IMessage, MessageTypesValues, IUser, IRoom, AtLeast } from '@rocket.chat/core-typings'; +import type { IMessage, MessageTypesValues, IUser, IRoom } from '@rocket.chat/core-typings'; export interface IMessageService { sendMessage({ fromId, rid, msg }: { fromId: string; rid: string; msg: string }): Promise; @@ -21,6 +21,6 @@ export interface IMessageService { deleteMessage(user: IUser, message: IMessage): Promise; updateMessage(message: IMessage, user: IUser, originalMsg?: IMessage): Promise; reactToMessage(userId: string, reaction: string, messageId: IMessage['_id'], shouldReact?: boolean): Promise; - beforeReacted(message: IMessage, room: AtLeast): Promise; + beforeReacted(message: IMessage, room: IRoom): Promise; beforeDelete(message: IMessage, room: IRoom): Promise; } From 9e502b160b4ddf9770c21d1ff4bde1f1bc7f3202 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Wed, 18 Sep 2024 08:48:10 -0600 Subject: [PATCH 13/19] Discard changes to apps/meteor/app/reactions/client/methods/setReaction.ts --- apps/meteor/app/reactions/client/methods/setReaction.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/meteor/app/reactions/client/methods/setReaction.ts b/apps/meteor/app/reactions/client/methods/setReaction.ts index 1744d49c0ceb..ed15cda9ab8e 100644 --- a/apps/meteor/app/reactions/client/methods/setReaction.ts +++ b/apps/meteor/app/reactions/client/methods/setReaction.ts @@ -3,6 +3,7 @@ import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Meteor } from 'meteor/meteor'; import { roomCoordinator } from '../../../../client/lib/rooms/roomCoordinator'; +import { callbacks } from '../../../../lib/callbacks'; import { emoji } from '../../../emoji/client'; import { Messages, ChatRoom, Subscriptions } from '../../../models/client'; @@ -54,8 +55,10 @@ Meteor.methods({ if (!message.reactions || typeof message.reactions !== 'object' || Object.keys(message.reactions).length === 0) { delete message.reactions; Messages.update({ _id: messageId }, { $unset: { reactions: 1 } }); + await callbacks.run('unsetReaction', messageId, reaction); } else { Messages.update({ _id: messageId }, { $set: { reactions: message.reactions } }); + await callbacks.run('setReaction', messageId, reaction); } } else { if (!message.reactions) { @@ -69,6 +72,7 @@ Meteor.methods({ message.reactions[reaction].usernames.push(user.username); Messages.update({ _id: messageId }, { $set: { reactions: message.reactions } }); + await callbacks.run('setReaction', messageId, reaction); } }, }); From 995e7c3deb91eede982c0cb0141eec45089417f5 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Wed, 18 Sep 2024 08:48:24 -0600 Subject: [PATCH 14/19] Discard changes to apps/meteor/app/slackbridge/server/RocketAdapter.js --- .../app/slackbridge/server/RocketAdapter.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/meteor/app/slackbridge/server/RocketAdapter.js b/apps/meteor/app/slackbridge/server/RocketAdapter.js index 8ba2a76dcbc2..f76c33fa1f81 100644 --- a/apps/meteor/app/slackbridge/server/RocketAdapter.js +++ b/apps/meteor/app/slackbridge/server/RocketAdapter.js @@ -45,16 +45,16 @@ export default class RocketAdapter { rocketLogger.debug('Register for events'); callbacks.add('afterSaveMessage', this.onMessage.bind(this), callbacks.priority.LOW, 'SlackBridge_Out'); callbacks.add('afterDeleteMessage', this.onMessageDelete.bind(this), callbacks.priority.LOW, 'SlackBridge_Delete'); - callbacks.add('afterSetReaction', this.onSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_SetReaction'); - callbacks.add('afterUnsetReaction', this.onUnSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_UnSetReaction'); + callbacks.add('setReaction', this.onSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_SetReaction'); + callbacks.add('unsetReaction', this.onUnSetReaction.bind(this), callbacks.priority.LOW, 'SlackBridge_UnSetReaction'); } unregisterForEvents() { rocketLogger.debug('Unregister for events'); callbacks.remove('afterSaveMessage', 'SlackBridge_Out'); callbacks.remove('afterDeleteMessage', 'SlackBridge_Delete'); - callbacks.remove('afterSetReaction', 'SlackBridge_SetReaction'); - callbacks.remove('afterUnsetReaction', 'SlackBridge_UnSetReaction'); + callbacks.remove('setReaction', 'SlackBridge_SetReaction'); + callbacks.remove('unsetReaction', 'SlackBridge_UnSetReaction'); } async onMessageDelete(rocketMessageDeleted) { @@ -72,7 +72,7 @@ export default class RocketAdapter { } } - async onSetReaction(rocketMsg, { reaction }) { + async onSetReaction(rocketMsgID, reaction) { try { if (!this.slackBridge.isReactionsEnabled) { return; @@ -80,11 +80,12 @@ export default class RocketAdapter { rocketLogger.debug('onRocketSetReaction'); - if (rocketMsg._id && reaction) { - if (this.slackBridge.reactionsMap.delete(`set${rocketMsg._id}${reaction}`)) { + if (rocketMsgID && reaction) { + if (this.slackBridge.reactionsMap.delete(`set${rocketMsgID}${reaction}`)) { // This was a Slack reaction, we don't need to tell Slack about it return; } + const rocketMsg = await Messages.findOneById(rocketMsgID); if (rocketMsg) { for await (const slack of this.slackAdapters) { const slackChannel = slack.getSlackChannel(rocketMsg.rid); @@ -100,7 +101,7 @@ export default class RocketAdapter { } } - async onUnSetReaction(rocketMsg, { reaction }) { + async onUnSetReaction(rocketMsgID, reaction) { try { if (!this.slackBridge.isReactionsEnabled) { return; @@ -108,12 +109,13 @@ export default class RocketAdapter { rocketLogger.debug('onRocketUnSetReaction'); - if (rocketMsg._id && reaction) { - if (this.slackBridge.reactionsMap.delete(`unset${rocketMsg._id}${reaction}`)) { + if (rocketMsgID && reaction) { + if (this.slackBridge.reactionsMap.delete(`unset${rocketMsgID}${reaction}`)) { // This was a Slack unset reaction, we don't need to tell Slack about it return; } + const rocketMsg = await Messages.findOneById(rocketMsgID); if (rocketMsg) { for await (const slack of this.slackAdapters) { const slackChannel = slack.getSlackChannel(rocketMsg.rid); From b3182153c7bfdaf9d3f9e3b70e9cf4cd7e88e5f5 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Wed, 18 Sep 2024 08:48:30 -0600 Subject: [PATCH 15/19] Discard changes to apps/meteor/lib/callbacks.ts --- apps/meteor/lib/callbacks.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/meteor/lib/callbacks.ts b/apps/meteor/lib/callbacks.ts index 4d4a00a4fa90..7eaa9ed7595d 100644 --- a/apps/meteor/lib/callbacks.ts +++ b/apps/meteor/lib/callbacks.ts @@ -254,8 +254,10 @@ export type Hook = | 'onValidateLogin' | 'openBroadcast' | 'renderNotification' + | 'setReaction' | 'streamMessage' | 'streamNewMessage' + | 'unsetReaction' | 'userAvatarSet' | 'userConfirmationEmailRequested' | 'userForgotPasswordEmailRequested' From 0cf7b6793d185110e6efcd74045212ff1da3e670 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 19 Sep 2024 12:10:17 -0600 Subject: [PATCH 16/19] Update setReaction.ts --- apps/meteor/app/reactions/server/setReaction.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index 9e42f3d60e52..6069d642ff5b 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -68,6 +68,7 @@ export async function setReaction( await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); + void notifyOnRoomChangedById(room._id); } } void callbacks.run('afterUnsetReaction', message, { user, reaction, shouldReact: false, oldMessage }); @@ -86,6 +87,7 @@ export async function setReaction( await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); + void notifyOnRoomChangedById(room._id); } void callbacks.run('afterSetReaction', message, { user, reaction, shouldReact: true }); @@ -95,7 +97,6 @@ export async function setReaction( void Apps.self?.triggerEvent(AppEvents.IPostMessageReacted, message, user, reaction, isReacted); - void notifyOnRoomChangedById(room._id); void notifyOnMessageChange({ id: message._id, }); From 8784d7fcd73c54c6b45157c0608d268132a3382c Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 19 Sep 2024 16:41:42 -0600 Subject: [PATCH 17/19] Update setReaction.ts --- apps/meteor/app/reactions/server/setReaction.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index 6069d642ff5b..d65b92826f93 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -63,6 +63,7 @@ export async function setReaction( await Messages.unsetReactions(message._id); if (isTheLastMessage(room, message)) { await Rooms.unsetReactionsInLastMessage(room._id); + void notifyOnRoomChangedById(room._id); } } else { await Messages.setReactions(message._id, message.reactions); From 71cc1aa610aabba69b7a637241e73264c4df5bf2 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 19 Sep 2024 16:45:52 -0600 Subject: [PATCH 18/19] Update setReaction.ts --- apps/meteor/app/reactions/server/setReaction.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/apps/meteor/app/reactions/server/setReaction.ts b/apps/meteor/app/reactions/server/setReaction.ts index d65b92826f93..be6e5aed4a54 100644 --- a/apps/meteor/app/reactions/server/setReaction.ts +++ b/apps/meteor/app/reactions/server/setReaction.ts @@ -11,7 +11,7 @@ import { canAccessRoomAsync } from '../../authorization/server'; import { hasPermissionAsync } from '../../authorization/server/functions/hasPermission'; import { emoji } from '../../emoji/server'; import { isTheLastMessage } from '../../lib/server/functions/isTheLastMessage'; -import { notifyOnRoomChangedById, notifyOnMessageChange } from '../../lib/server/lib/notifyListener'; +import { notifyOnMessageChange } from '../../lib/server/lib/notifyListener'; export const removeUserReaction = (message: IMessage, reaction: string, username: string) => { if (!message.reactions) { @@ -63,13 +63,11 @@ export async function setReaction( await Messages.unsetReactions(message._id); if (isTheLastMessage(room, message)) { await Rooms.unsetReactionsInLastMessage(room._id); - void notifyOnRoomChangedById(room._id); } } else { await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); - void notifyOnRoomChangedById(room._id); } } void callbacks.run('afterUnsetReaction', message, { user, reaction, shouldReact: false, oldMessage }); @@ -88,7 +86,6 @@ export async function setReaction( await Messages.setReactions(message._id, message.reactions); if (isTheLastMessage(room, message)) { await Rooms.setReactionsInLastMessage(room._id, message.reactions); - void notifyOnRoomChangedById(room._id); } void callbacks.run('afterSetReaction', message, { user, reaction, shouldReact: true }); From 9a2fc139e6f98e2991f238f21f1b4625c4ed94d3 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Thu, 19 Sep 2024 16:46:18 -0600 Subject: [PATCH 19/19] Update setReaction.spec.ts --- apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts index ed710eb39f70..e267825a6a18 100644 --- a/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts +++ b/apps/meteor/tests/unit/app/reactions/server/setReaction.spec.ts @@ -25,7 +25,6 @@ const modelsMock = { }; const canAccessRoomAsyncMock = sinon.stub(); const isTheLastMessageMock = sinon.stub(); -const notifyOnRoomChangedByIdMock = sinon.stub(); const notifyOnMessageChangeMock = sinon.stub(); const hasPermissionAsyncMock = sinon.stub(); const i18nMock = { t: sinon.stub() }; @@ -47,7 +46,6 @@ const { removeUserReaction, executeSetReaction, setReaction } = p.noCallThru().l '../../emoji/server': { emoji: { list: emojiList } }, '../../lib/server/functions/isTheLastMessage': { isTheLastMessage: isTheLastMessageMock }, '../../lib/server/lib/notifyListener': { - notifyOnRoomChangedById: notifyOnRoomChangedByIdMock, notifyOnMessageChange: notifyOnMessageChangeMock, }, });