Skip to content

Commit

Permalink
regression: remove query field on dm messages listing (#33807)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ricardogarim
ricardogarim authored Oct 28, 2024
1 parent d1e14a0 commit 2bc9692
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 18 deletions.
6 changes: 6 additions & 0 deletions .changeset/tiny-rabbits-lie.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@rocket.chat/rest-typings': major
'@rocket.chat/meteor': major
---

Changes ims and dms messages listing endpoint by moving query params from the 'query' attribute to standard query parameters.
15 changes: 13 additions & 2 deletions apps/meteor/app/api/server/v1/im.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ API.v1.addRoute(
},
{
async get() {
const { room } = await findDirectMessageRoom(this.queryParams, this.userId);
const { roomId, username, mentionIds, starredIds, pinned } = this.queryParams;

const { room } = await findDirectMessageRoom({ ...(roomId ? { roomId } : { username }) }, this.userId);

const canAccess = await canAccessRoomIdAsync(room._id, this.userId);
if (!canAccess) {
Expand All @@ -388,7 +390,16 @@ API.v1.addRoute(
const { offset, count } = await getPaginationItems(this.queryParams);
const { sort, fields, query } = await this.parseJsonQuery();

const ourQuery = { rid: room._id, ...query };
const parseIds = (ids: string | undefined, field: string) =>
typeof ids === 'string' && ids ? { [field]: { $in: ids.split(',').map((id) => id.trim()) } } : {};

const ourQuery = {
rid: room._id,
...query,
...parseIds(mentionIds, 'mentions._id'),
...parseIds(starredIds, 'starred._id'),
...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}),
};
const sortObj = { ts: sort?.ts ?? -1 };

const { cursor, totalCount } = Messages.findPaginated(ourQuery, {
Expand Down
28 changes: 22 additions & 6 deletions apps/meteor/tests/data/chat.helper.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Credentials } from '@rocket.chat/api-client';
import type { IRoom, IMessage } from '@rocket.chat/core-typings';

import { api, credentials, request } from './api-data';
Expand Down Expand Up @@ -30,16 +31,31 @@ export const sendSimpleMessage = ({
return request.post(api('chat.sendMessage')).set(credentials).send({ message });
};

export const sendMessage = ({ message }: { message: { rid: IRoom['_id']; msg: string } & Partial<Omit<IMessage, 'rid' | 'msg'>> }) => {
return request.post(api('chat.sendMessage')).set(credentials).send({ message });
export const sendMessage = ({
message,
requestCredentials,
}: {
message: { rid: IRoom['_id']; msg: string } & Partial<Omit<IMessage, 'rid' | 'msg'>>;
requestCredentials?: Credentials;
}) => {
return request
.post(api('chat.sendMessage'))
.set(requestCredentials ?? credentials)
.send({ message });
};

export const starMessage = ({ messageId }: { messageId: IMessage['_id'] }) => {
return request.post(api('chat.starMessage')).set(credentials).send({ messageId });
export const starMessage = ({ messageId, requestCredentials }: { messageId: IMessage['_id']; requestCredentials?: Credentials }) => {
return request
.post(api('chat.starMessage'))
.set(requestCredentials ?? credentials)
.send({ messageId });
};

export const pinMessage = ({ messageId }: { messageId: IMessage['_id'] }) => {
return request.post(api('chat.pinMessage')).set(credentials).send({ messageId });
export const pinMessage = ({ messageId, requestCredentials }: { messageId: IMessage['_id']; requestCredentials?: Credentials }) => {
return request
.post(api('chat.pinMessage'))
.set(requestCredentials ?? credentials)
.send({ messageId });
};

export const deleteMessage = ({ roomId, msgId }: { roomId: IRoom['_id']; msgId: IMessage['_id'] }) => {
Expand Down
147 changes: 145 additions & 2 deletions apps/meteor/tests/end-to-end/api/direct-message.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { expect } from 'chai';
import { after, before, describe, it } from 'mocha';

import { getCredentials, api, request, credentials, apiUsername, apiEmail, methodCall } from '../../data/api-data';
import { pinMessage, sendMessage, starMessage } from '../../data/chat.helper';
import { updateSetting, updatePermission } from '../../data/permissions.helper';
import { deleteRoom } from '../../data/rooms.helper';
import { testFileUploads } from '../../data/uploads.helper';
Expand Down Expand Up @@ -369,12 +370,60 @@ describe('[Direct Messages]', () => {
});

describe('/im.messages', () => {
let testUser: IUser;
let testUserDMRoom: IRoom;
let testUserCredentials: Credentials;

before(async () => {
testUser = await createUser({ joinDefaultChannels: false, roles: ['admin'] });

testUserCredentials = await login(testUser.username, password);
await setUserStatus(testUserCredentials);

testUserDMRoom = (
await request
.post(api('im.create'))
.set(testUserCredentials)
.send({ username: `${testUser.username}` })
).body.room;

const messages = [
{
rid: testUserDMRoom._id,
msg: `@${adminUsername} youre being mentioned`,
mentions: [{ username: adminUsername, _id: adminUsername, name: adminUsername }],
},
{
rid: testUserDMRoom._id,
msg: `@${testUser.username} youre being mentioned`,
mentions: [{ username: testUser.username, _id: testUser._id, name: testUser.name }],
},
{
rid: testUserDMRoom._id,
msg: `A simple message`,
},
{
rid: testUserDMRoom._id,
msg: `A pinned simple message`,
},
];

const [, , starredMessage, pinnedMessage] = await Promise.all(
messages.map((message) => sendMessage({ message, requestCredentials: testUserCredentials })),
);

await Promise.all([
starMessage({ messageId: starredMessage.body.message._id, requestCredentials: testUserCredentials }),
pinMessage({ messageId: pinnedMessage.body.message._id, requestCredentials: testUserCredentials }),
]);
});

it('should return all DM messages that were sent to yourself using your username', (done) => {
void request
.get(api('im.messages'))
.set(credentials)
.set(testUserCredentials)
.query({
username: adminUsername,
username: testUser.username,
})
.expect('Content-Type', 'application/json')
.expect(200)
Expand All @@ -384,6 +433,100 @@ describe('[Direct Messages]', () => {
})
.end(done);
});

it('should return an error when trying to access a DM that does not belong to the current user', async () => {
await request
.get(api('im.messages'))
.set(credentials)
.query({ roomId: testUserDMRoom._id })
.expect('Content-Type', 'application/json')
.expect(403)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'unauthorized');
});
});

it('should return messages that mention a single user', async () => {
await request
.get(api('im.messages'))
.set(testUserCredentials)
.query({
roomId: testUserDMRoom._id,
mentionIds: adminUsername,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body.messages).to.have.lengthOf(1);
expect(res.body.messages[0]).to.have.nested.property('mentions').that.is.an('array').and.to.have.lengthOf(1);
expect(res.body.messages[0].mentions[0]).to.have.property('_id', adminUsername);
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
});
});

it('should return messages that mention multiple users', async () => {
await request
.get(api('im.messages'))
.set(testUserCredentials)
.query({
roomId: testUserDMRoom._id,
mentionIds: `${adminUsername},${testUser._id}`,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body.messages).to.have.lengthOf(2);
expect(res.body).to.have.property('count', 2);
expect(res.body).to.have.property('total', 2);

const mentionIds = res.body.messages.map((message: any) => message.mentions[0]._id);
expect(mentionIds).to.include.members([adminUsername, testUser._id]);
});
});

it('should return messages that are starred by a specific user', async () => {
await request
.get(api('im.messages'))
.set(testUserCredentials)
.query({
roomId: testUserDMRoom._id,
starredIds: testUser._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body.messages).to.have.lengthOf(1);
expect(res.body.messages[0]).to.have.nested.property('starred').that.is.an('array').and.to.have.lengthOf(1);
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
});
});

it('should return messages that are pinned', async () => {
await request
.get(api('im.messages'))
.set(testUserCredentials)
.query({
roomId: testUserDMRoom._id,
pinned: true,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body.messages).to.have.lengthOf(1);
expect(res.body.messages[0]).to.have.nested.property('pinned').that.is.an('boolean').and.to.be.true;
expect(res.body.messages[0]).to.have.nested.property('pinnedBy').that.is.an('object');
expect(res.body.messages[0].pinnedBy).to.have.property('_id', testUser._id);
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
});
});
});

describe('/im.messages.others', () => {
Expand Down
31 changes: 23 additions & 8 deletions packages/rest-typings/src/v1/dm/DmMessagesProps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,11 @@ import type { PaginatedRequest } from '../../helpers/PaginatedRequest';
const ajv = new Ajv({ coerceTypes: true });

export type DmMessagesProps = PaginatedRequest<
(
| {
roomId: string;
}
| {
username: string;
}
) & {
({ roomId: string } | { username: string }) & {
query?: string;
mentionIds?: string;
starredIds?: string;
pinned?: string;
fields?: string;
}
>;
Expand All @@ -25,6 +22,15 @@ export const isDmMessagesProps = ajv.compile<DmMessagesProps>({
roomId: {
type: 'string',
},
mentionIds: {
type: 'string',
},
starredIds: {
type: 'string',
},
pinned: {
type: 'string',
},
fields: {
type: 'string',
},
Expand All @@ -50,6 +56,15 @@ export const isDmMessagesProps = ajv.compile<DmMessagesProps>({
username: {
type: 'string',
},
mentionIds: {
type: 'string',
},
starredIds: {
type: 'string',
},
pinned: {
type: 'string',
},
query: {
type: 'string',
},
Expand Down

0 comments on commit 2bc9692

Please sign in to comment.