Skip to content

test: Enhance end-to-end tests for DMs #37813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 6 commits into from
Dec 15, 2025
Merged

test: Enhance end-to-end tests for DMs #37813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c95bb92
test: Enhance end-to-end tests for adding users to rooms, including d…
ggazzo Dec 15, 2025
4e2a42f
test: Add end-to-end tests for "kick" command in direct message rooms
ggazzo Dec 15, 2025
8ff0b23
test: Add end-to-end tests for "leave" command in direct message rooms
ggazzo Dec 15, 2025
866abd4
test: Add end-to-end tests for "invite" command in direct message rooms
ggazzo Dec 15, 2025
d899963
refactor: Add TODO comments for direct room restrictions and error ha…
ggazzo Dec 15, 2025
ff1c945
fix: Correct error handling response in API route to return success i…
ggazzo Dec 15, 2025
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
2 changes: 1 addition & 1 deletion apps/meteor/app/lib/server/methods/addUsersToRoom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const addUsersToRoomMethod = async (userId: string, data: { rid: string;
});
const userInRoom = subscription != null;

// Can't add to direct room ever
// TODO: Can't add to direct room ever, unless it's a federated room
if (room.t === 'd') {
throw new Meteor.Error('error-cant-invite-for-direct-room', "Can't invite user to direct rooms", {
method: 'addUsersToRoom',
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/app/slashcommands-invite/server/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ slashCommands.add({
await Promise.all(
usersFiltered.map(async (user) => {
try {
// TODO: Refactor this to return an error if some user fails to be added
return await addUsersToRoomMethod(
userId,
{
Expand Down
7 changes: 4 additions & 3 deletions apps/meteor/app/slashcommands-leave/server/leave.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ const Leave = async function Leave({ message, userId }: SlashCommandCallbackPara
return;
}
await leaveRoomMethod(user, message.rid);
} catch ({ error }: any) {
if (typeof error !== 'string') {
return;
} catch (error: any) {
if (typeof error.error !== 'string') {
throw error;
}
const user = await Users.findOneById(userId);
void api.broadcast('notify.ephemeralMessage', userId, message.rid, {
msg: i18n.t(error, { lng: user?.language || settings.get('Language') || 'en' }),
});
throw new Meteor.Error(error.error, error.message);
}
Comment on lines +21 to 30
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Find the leave.ts file and examine its full context
find . -path "*/slashcommands-leave/server/leave.ts" -type f

Repository: RocketChat/Rocket.Chat

Length of output: 119

🏁 Script executed:

# Examine the full content of the leave.ts file with line numbers
cat -n "apps/meteor/app/slashcommands-leave/server/leave.ts"

Repository: RocketChat/Rocket.Chat

Length of output: 1838

🏁 Script executed:

# Check what i18n.t expects - examine i18n module signature
fd -t f "i18n\\.ts$" | head -20

Repository: RocketChat/Rocket.Chat

Length of output: 207

🏁 Script executed:

# Search for how i18n.t is used elsewhere in the codebase (passing object vs string)
rg "i18n\.t\(" --max-count 10 -A 2 -B 1

Repository: RocketChat/Rocket.Chat

Length of output: 50378

🏁 Script executed:

# Check if Meteor is imported or used as global in similar files
rg "import.*Meteor" apps/meteor/app/slashcommands-leave/server/ || echo "No Meteor import found"

Repository: RocketChat/Rocket.Chat

Length of output: 88

🏁 Script executed:

# Check how error handling is typically done in this codebase
rg "catch.*error.*:" apps/meteor/app/slashcommands-leave/ -A 5 | head -30

Repository: RocketChat/Rocket.Chat

Length of output: 581

🏁 Script executed:

# Check the error structure more closely - see how leaveRoomMethod throws errors
rg "leaveRoomMethod" --max-count 5 -B 2 -A 10 | head -50

Repository: RocketChat/Rocket.Chat

Length of output: 3977

🏁 Script executed:

# Check how other catch blocks handle similar error patterns
rg "catch.*error.*any" apps/meteor/app/slashcommands-invite/server/server.ts -A 15 | head -40

Repository: RocketChat/Rocket.Chat

Length of output: 833

Change i18n.t(error, …) to i18n.t(error.error, …) on line 27

The code validates that error.error is a string (line 22), but then passes the entire error object to i18n.t() instead of the error code. This will fail to translate because i18n.t() expects a string translation key, not an object. Use error.error to pass the actual error code string, matching the string key validation on line 22 and consistent with the error code used in Meteor.Error on line 29.

Optionally, import Meteor from 'meteor/meteor' for explicitness; it currently works as a global but being explicit improves type safety.

🤖 Prompt for AI Agents 
In apps/meteor/app/slashcommands-leave/server/leave.ts around lines 21 to 30,
the catch block validates that error.error is a string but incorrectly calls
i18n.t(error, ...) with the whole error object; change the call to
i18n.t(error.error, { lng: ... }) so the translation key is the string code, and
keep the rest of the broadcast and Meteor.Error construction unchanged;
optionally add an explicit import for Meteor from 'meteor/meteor' at the top for
clearer typing.

};

Expand Down
112 changes: 112 additions & 0 deletions apps/meteor/tests/end-to-end/api/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,4 +521,116 @@ describe('[Commands]', () => {
});
});
});
describe('Command "kick"', function () {
let directMessageRoom: IRoom;
let user1: TestUser<IUser>;
let user1Credentials: Credentials;
this.beforeAll(async () => {
user1 = await createUser();

[user1Credentials] = await Promise.all([login(user1.username, password)]);
});

this.beforeAll(async () => {
const [response1] = await Promise.all([
createRoom({ type: 'd', name: `room1-${Date.now()}.${Random.id()}`, username: user1.username }),
]);
directMessageRoom = response1.body.room;
});

this.afterAll(async () => {
await Promise.all([deleteRoom({ type: 'd', roomId: directMessageRoom._id })]);
await Promise.all([deleteUser(user1)]);
});

it('should fail when trying to kick a user from a direct message room', (done) => {
void request
.post(api('commands.run'))
.set(user1Credentials)
.send({ command: 'kick', roomId: directMessageRoom._id, params: user1.username })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error').that.is.a('string');
})
.end(done);
});
});
describe('Command "leave"', function () {
let directMessageRoom: IRoom;
let user1: TestUser<IUser>;
let user2: TestUser<IUser>;
let user1Credentials: Credentials;
this.beforeAll(async () => {
user1 = await createUser();
user2 = await createUser();

[user1Credentials] = await Promise.all([login(user1.username, password)]);
});

this.beforeAll(async () => {
const [response1] = await Promise.all([
createRoom({ type: 'd', name: `room1-${Date.now()}.${Random.id()}`, username: user1.username }),
]);
directMessageRoom = response1.body.room;
});

this.afterAll(async () => {
await Promise.all([deleteRoom({ type: 'd', roomId: directMessageRoom._id })]);
await Promise.all([deleteUser(user1), deleteUser(user2)]);
});

it('should fail when trying to leave a direct message room', (done) => {
void request
.post(api('commands.run'))
.set(user1Credentials)
.send({ command: 'leave', roomId: directMessageRoom._id })
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error').that.is.a('string');
})
.end(done);
});
});

// TODO: invite never fails
describe.skip('Command "invite"', function () {
let directMessageRoom: IRoom;
let user1: TestUser<IUser>;
let user2: TestUser<IUser>;
let user1Credentials: Credentials;
this.beforeAll(async () => {
user1 = await createUser();
user2 = await createUser();

[user1Credentials] = await Promise.all([login(user1.username, password)]);
});

this.beforeAll(async () => {
const [response1] = await Promise.all([
createRoom({ type: 'd', name: `room1-${Date.now()}.${Random.id()}`, username: user1.username }),
]);
directMessageRoom = response1.body.room;
});

this.afterAll(async () => {
await Promise.all([deleteRoom({ type: 'd', roomId: directMessageRoom._id })]);
await Promise.all([deleteUser(user1), deleteUser(user2)]);
});

it('should fail when trying to invite a user to a direct message room', (done) => {
void request
.post(api('commands.run'))
.set(user1Credentials)
.send({ command: 'invite', roomId: directMessageRoom._id, params: 'g1' })
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error').that.is.a('string');
expect(res.body.error).to.equal('unauthorized');
})
.end(done);
});
});
});
Loading
Loading