diff --git a/.changeset/wet-turtles-fix.md b/.changeset/wet-turtles-fix.md new file mode 100644 index 0000000000000..647a5ae3cd117 --- /dev/null +++ b/.changeset/wet-turtles-fix.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes reaction list modal showing blank entries (mobile) or usernames (web) instead of real names when `UI_Use_Real_Name` is enabled. The broadcast pipeline now enriches reactions with display names via batch query. diff --git a/apps/meteor/server/lib/notifyListener.ts b/apps/meteor/server/lib/notifyListener.ts index 3a10d7ea000fa..da99acf311320 100644 --- a/apps/meteor/server/lib/notifyListener.ts +++ b/apps/meteor/server/lib/notifyListener.ts @@ -431,6 +431,14 @@ const getUserNameCached = mem( const getSettingCached = mem(async (setting: string): Promise => Settings.getValueById(setting), { maxAge: 10000 }); +const getUsersByUsernamesCached = mem( + async (usernames: string[]): Promise> => { + const users = await Users.findByUsernames(usernames, { projection: { username: 1, name: 1 } }).toArray(); + return new Map(users.filter((u): u is IUser & { username: string } => !!u.username).map((u) => [u.username, u.name])); + }, + { maxAge: 10000, cacheKey: ([usernames]) => JSON.stringify([...usernames].sort()) }, +); + export async function getMessageToBroadcast({ id, data }: { id: IMessage['_id']; data?: IMessage }): Promise { const message = data ?? (await Messages.findOneById(id)); if (!message) { @@ -467,6 +475,16 @@ export async function getMessageToBroadcast({ id, data }: { id: IMessage['_id']; } } } + + if (message.reactions) { + const allUsernames = [...new Set(Object.values(message.reactions).flatMap((r) => r.usernames))]; + if (allUsernames.length > 0) { + const nameByUsername = await getUsersByUsernamesCached(allUsernames); + for (const reaction of Object.values(message.reactions)) { + reaction.names = reaction.usernames.map((username) => nameByUsername.get(username) || username); + } + } + } } return message; diff --git a/apps/meteor/tests/unit/server/lib/notifyListener.spec.ts b/apps/meteor/tests/unit/server/lib/notifyListener.spec.ts index d0c335df80107..bfa9075c9976c 100644 --- a/apps/meteor/tests/unit/server/lib/notifyListener.spec.ts +++ b/apps/meteor/tests/unit/server/lib/notifyListener.spec.ts @@ -6,6 +6,7 @@ import sinon from 'sinon'; describe('Message Broadcast Tests', () => { let getSettingValueByIdStub: sinon.SinonStub; let usersFindOneStub: sinon.SinonStub; + let usersFindByUsernamesStub: sinon.SinonStub; let messagesFindOneStub: sinon.SinonStub; let broadcastStub: sinon.SinonStub; let getMessageToBroadcast: any; @@ -29,6 +30,7 @@ describe('Message Broadcast Tests', () => { }, Users: { findOne: usersFindOneStub, + findByUsernames: usersFindByUsernamesStub, }, Settings: { getValueById: getSettingValueByIdStub, @@ -44,6 +46,7 @@ describe('Message Broadcast Tests', () => { beforeEach(() => { getSettingValueByIdStub = sinon.stub(); usersFindOneStub = sinon.stub(); + usersFindByUsernamesStub = sinon.stub(); messagesFindOneStub = sinon.stub(); broadcastStub = sinon.stub(); memStub = sinon.stub().callsFake((fn: any) => fn); @@ -99,6 +102,44 @@ describe('Message Broadcast Tests', () => { useRealName: true, expectedResult: { ...sampleMessage, u: { ...sampleMessage.u, name: 'Real User' } }, }, + { + description: 'should return the message with reactions real names if useRealName is true', + message: { + ...sampleMessage, + t: undefined, + reactions: { + ':smile:': { usernames: ['user1', 'user2'] }, + ':heart:': { usernames: ['user1', 'user3'] }, + }, + }, + hideSystemMessages: [], + useRealName: true, + expectedResult: { + ...sampleMessage, + t: undefined, + u: { ...sampleMessage.u, name: 'Real User' }, + reactions: { + ':smile:': { usernames: ['user1', 'user2'], names: ['Real User', 'Name for user2'] }, + ':heart:': { usernames: ['user1', 'user3'], names: ['Real User', 'Name for user3'] }, + }, + }, + }, + { + description: 'should return the message with empty reactions without querying users if useRealName is true', + message: { + ...sampleMessage, + t: undefined, + reactions: {}, + }, + hideSystemMessages: [], + useRealName: true, + expectedResult: { + ...sampleMessage, + t: undefined, + u: { ...sampleMessage.u, name: 'Real User' }, + reactions: {}, + }, + }, { description: 'should return the message with mentions real name if useRealName is true', message: { @@ -184,17 +225,37 @@ describe('Message Broadcast Tests', () => { getSettingValueByIdStub.withArgs('UI_Use_Real_Name').resolves(useRealName); if (useRealName) { - const realNames = - message.mentions && message.mentions.length > 0 - ? [message.u.name, ...message.mentions.map((mention) => mention.name)] - : [message.u.name]; + const realNames: (string | undefined)[] = [message.u.name]; + + if (message.mentions) { + message.mentions.forEach((mention) => realNames.push(mention.name)); + } realNames.forEach((user, index) => usersFindOneStub.onCall(index).resolves({ name: user })); + + if (message.reactions) { + const allUsernames = [...new Set(Object.values(message.reactions).flatMap((r) => r.usernames))]; + const users = allUsernames.map((username) => ({ + username, + name: username === message.u.username ? message.u.name : `Name for ${username}`, + })); + usersFindByUsernamesStub.returns({ toArray: () => Promise.resolve(users) }); + } } const result = await getMessageToBroadcast({ id: '123' }); expect(result).to.deep.equal(expectedResult); + + if (useRealName && message.reactions && Object.keys(message.reactions).length) { + const deduplicated = [...new Set(Object.values(message.reactions).flatMap((r) => r.usernames))]; + expect(usersFindByUsernamesStub.calledOnce).to.be.true; + expect(usersFindByUsernamesStub.calledWith(deduplicated, { projection: { username: 1, name: 1 } })).to.be.true; + } + + if (useRealName && message.reactions && !Object.keys(message.reactions).length) { + expect(usersFindByUsernamesStub.called).to.be.false; + } }); }); });