Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/fix-room-rename-unread-alert.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fix: Room rename no longer triggers an unread alert for members if the "Room name changed" system message is hidden in room settings.
105 changes: 105 additions & 0 deletions apps/meteor/app/channel-settings/server/functions/saveRoomName.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { expect } from 'chai';
Comment thread
rish106-hub marked this conversation as resolved.
import { describe, it, beforeEach } from 'mocha';
import proxyquire from 'proxyquire';
import sinon from 'sinon';

const sandbox = sinon.createSandbox();

const mocks = {
Rooms: {
findOneById: sandbox.stub(),
setNameById: sandbox.stub().resolves({ modifiedCount: 1 }),
setFnameById: sandbox.stub().resolves({ modifiedCount: 1 }),
},
Subscriptions: {
updateNameAndAlertByRoomId: sandbox.stub().resolves({ modifiedCount: 1 }),
updateNameAndFnameByRoomId: sandbox.stub().resolves({ modifiedCount: 1 }),
updateFnameByRoomId: sandbox.stub().resolves({ modifiedCount: 1 }),
},
Integrations: {
updateRoomName: sandbox.stub().resolves(),
},
Message: {
saveSystemMessage: sandbox.stub().resolves(),
},
Room: {
beforeNameChange: sandbox.stub().resolves(),
},
roomCoordinator: {
getRoomDirectives: sandbox.stub().returns({ preventRenaming: () => false }),
},
checkUsernameAvailability: sandbox.stub().resolves(true),
getValidRoomName: sandbox.stub().callsFake((name: string) => Promise.resolve(name)),
notifyOnIntegrationChangedByChannels: sandbox.stub().resolves(),
notifyOnSubscriptionChangedByRoomId: sandbox.stub().resolves(),
callbacks: {
run: sandbox.stub().resolves(),
},
isRoomNativeFederated: sandbox.stub().returns(false),
};

const { saveRoomName } = proxyquire.noCallThru().load('./saveRoomName', {
'@rocket.chat/core-services': { Message: mocks.Message, Room: mocks.Room },
'@rocket.chat/core-typings': { isRoomNativeFederated: mocks.isRoomNativeFederated },
'@rocket.chat/models': { Integrations: mocks.Integrations, Rooms: mocks.Rooms, Subscriptions: mocks.Subscriptions },
'meteor/meteor': { Meteor: { Error: class MeteorError extends Error {} } },
'../../../../server/lib/callbacks': { callbacks: mocks.callbacks },
'../../../../server/lib/rooms/roomCoordinator': { roomCoordinator: mocks.roomCoordinator },
'../../../lib/server/functions/checkUsernameAvailability': { checkUsernameAvailability: mocks.checkUsernameAvailability },
'../../../lib/server/lib/notifyListener': {
notifyOnIntegrationChangedByChannels: mocks.notifyOnIntegrationChangedByChannels,
notifyOnSubscriptionChangedByRoomId: mocks.notifyOnSubscriptionChangedByRoomId,
},
'../../../utils/server/lib/getValidRoomName': { getValidRoomName: mocks.getValidRoomName },
});

const fakeUser = { _id: 'user1', username: 'user1' } as any;

describe('saveRoomName — sysMes unread alert behavior', () => {
beforeEach(() => {
sandbox.resetHistory();
mocks.checkUsernameAvailability.resolves(true);
mocks.isRoomNativeFederated.returns(false);
});

it('triggers unread alert when sysMes does not include "r"', async () => {
mocks.Rooms.findOneById.resolves({ _id: 'room1', t: 'c', name: 'old-name', sysMes: ['uj', 'ul'] });

await saveRoomName('room1', 'new-name', fakeUser);

expect(mocks.Rooms.setNameById.calledOnceWith('room1', 'new-name', 'new-name')).to.be.true;
expect(mocks.Subscriptions.updateNameAndAlertByRoomId.calledOnce).to.be.true;
expect(mocks.Subscriptions.updateNameAndFnameByRoomId.called).to.be.false;
});

it('skips unread alert when sysMes includes "r"', async () => {
mocks.Rooms.findOneById.resolves({ _id: 'room1', t: 'c', name: 'old-name', sysMes: ['uj', 'r', 'ul'] });

await saveRoomName('room1', 'new-name', fakeUser);

expect(mocks.Rooms.setNameById.calledOnceWith('room1', 'new-name', 'new-name')).to.be.true;
expect(mocks.Subscriptions.updateNameAndFnameByRoomId.calledOnce).to.be.true;
expect(mocks.Subscriptions.updateNameAndAlertByRoomId.called).to.be.false;
});

it('triggers unread alert when sysMes is undefined (safe fallback)', async () => {
mocks.Rooms.findOneById.resolves({ _id: 'room1', t: 'c', name: 'old-name' });

await saveRoomName('room1', 'new-name', fakeUser);

expect(mocks.Rooms.setNameById.calledOnceWith('room1', 'new-name', 'new-name')).to.be.true;
expect(mocks.Subscriptions.updateNameAndAlertByRoomId.calledOnce).to.be.true;
expect(mocks.Subscriptions.updateNameAndFnameByRoomId.called).to.be.false;
});

it('does not match partial tokens like "rp" as the "r" system message type', async () => {
mocks.Rooms.findOneById.resolves({ _id: 'room1', t: 'c', name: 'old-name', sysMes: ['rp', 'ru'] });

await saveRoomName('room1', 'new-name', fakeUser);

expect(mocks.Rooms.setNameById.calledOnceWith('room1', 'new-name', 'new-name')).to.be.true;
// 'rp'/'ru' are not 'r', so alert should still fire
expect(mocks.Subscriptions.updateNameAndAlertByRoomId.calledOnce).to.be.true;
expect(mocks.Subscriptions.updateNameAndFnameByRoomId.called).to.be.false;
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const updateFName = async (rid: string, displayName: string): Promise<(UpdateRes
return responses;
};

const updateRoomName = async (rid: string, displayName: string, slugifiedRoomName: string) => {
const updateRoomName = async (rid: string, displayName: string, slugifiedRoomName: string, triggerUnreadAlert = true) => {
// Check if the username is available
if (!(await checkUsernameAvailability(slugifiedRoomName))) {
throw new Meteor.Error('error-duplicate-handle', `A room, team or user with name '${slugifiedRoomName}' already exists`, {
Expand All @@ -30,10 +30,11 @@ const updateRoomName = async (rid: string, displayName: string, slugifiedRoomNam
});
}

const responses = await Promise.all([
Rooms.setNameById(rid, slugifiedRoomName, displayName),
Subscriptions.updateNameAndAlertByRoomId(rid, slugifiedRoomName, displayName),
]);
const subscriptionUpdate = triggerUnreadAlert
? Subscriptions.updateNameAndAlertByRoomId(rid, slugifiedRoomName, displayName)
: Subscriptions.updateNameAndFnameByRoomId(rid, slugifiedRoomName, displayName);

const responses = await Promise.all([Rooms.setNameById(rid, slugifiedRoomName, displayName), subscriptionUpdate]);

if (responses[1]?.modifiedCount) {
void notifyOnSubscriptionChangedByRoomId(rid);
Expand Down Expand Up @@ -84,7 +85,8 @@ export async function saveRoomName(
if (isDiscussion || isRoomNativeFederated(room)) {
update = await updateFName(rid, displayName);
} else {
update = await updateRoomName(rid, displayName, slugifiedRoomName);
const shouldAlert = !room.sysMes?.some((m) => m === 'r');
update = await updateRoomName(rid, displayName, slugifiedRoomName, shouldAlert);
}

if (!update) {
Expand Down
5 changes: 4 additions & 1 deletion apps/meteor/server/startup/ensureMessagesTextIndex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ export const ensureMessagesTextIndex = async (): Promise<void> => {
shape: desiredShape,
});
try {
const name = await Messages.col.createIndex(desiredShape === 'room-scoped' ? { rid: 1, msg: 'text' } : { msg: 'text' });
const index = desiredShape === 'room-scoped' ? { rid: 1, msg: 'text' } : { msg: 'text' };
const name = await Messages.col.createIndex(index, {
name: desiredShape === 'room-scoped' ? 'messages_room_text' : 'messages_text',
});
SystemLogger.startup({ msg: 'created messages text index', name, shape: desiredShape });
} catch (err) {
SystemLogger.error({ msg: 'failed to create messages text index', shape: desiredShape, err });
Expand Down