Skip to content
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

Jump to bottom

chore!: remove query field on messages listing #33801

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
+188 −9
Draft

chore!: remove query field on messages listing #33801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 15 additions & 5 deletions apps/meteor/app/api/server/v1/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,15 +279,25 @@ API.v1.addRoute(
},
{
async get() {
const { roomId } = this.queryParams;
const { roomId, mentionIds, starredIds, pinned } = this.queryParams;
const { offset, count } = await getPaginationItems(this.queryParams);
const { sort, fields, query } = await this.parseJsonQuery();

const findResult = await findChannelByIdOrName({
params: { roomId },
checkedArchived: false,
});
const { offset, count } = await getPaginationItems(this.queryParams);
const { sort, fields, query } = await this.parseJsonQuery();

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

const ourQuery = {
...query,
rid: findResult._id,
...parseIds(mentionIds, 'mentions._id'),
...parseIds(starredIds, 'starred._id'),
...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}),
};

// Special check for the permissions
if (
Expand All @@ -297,7 +307,7 @@ API.v1.addRoute(
return API.v1.unauthorized();
}

const { cursor, totalCount } = await Messages.findPaginated(ourQuery, {
const { cursor, totalCount } = Messages.findPaginated(ourQuery, {
sort: sort || { ts: -1 },
skip: offset,
limit: count,
Expand Down
13 changes: 13 additions & 0 deletions apps/meteor/tests/data/chat.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,26 @@ export const sendSimpleMessage = ({
rid: roomId,
text,
};

if (tmid) {
message.tmid = tmid;
}

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 starMessage = ({ messageId }: { messageId: IMessage['_id'] }) => {
return request.post(api('chat.starMessage')).set(credentials).send({ messageId });
};

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

export const deleteMessage = ({ roomId, msgId }: { roomId: IRoom['_id']; msgId: IMessage['_id'] }) => {
if (!roomId) {
throw new Error('"roomId" is required in "deleteMessage" test helper');
Expand Down
142 changes: 141 additions & 1 deletion apps/meteor/tests/end-to-end/api/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { expect, assert } from 'chai';
import { after, before, describe, it } from 'mocha';

import { getCredentials, api, request, credentials, reservedWords } from '../../data/api-data';
import { pinMessage, sendMessage, starMessage } from '../../data/chat.helper';
import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants';
import { createIntegration, removeIntegration } from '../../data/integration.helper';
import { updatePermission, updateSetting } from '../../data/permissions.helper';
Expand Down Expand Up @@ -2461,9 +2462,45 @@ describe('[Channels]', () => {

describe('[/channels.messages]', () => {
let testChannel: IRoom;
let emptyChannel: IRoom;
let firstUser: IUser;
let secondUser: IUser;

before(async () => {
await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']);
emptyChannel = (await createRoom({ type: 'c', name: `channels.messages.empty.test.${Date.now()}` })).body.channel;
testChannel = (await createRoom({ type: 'c', name: `channels.messages.test.${Date.now()}` })).body.channel;

firstUser = await createUser({ joinDefaultChannels: false });
secondUser = await createUser({ joinDefaultChannels: false });

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

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

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

after(async () => {
Expand All @@ -2476,7 +2513,7 @@ describe('[Channels]', () => {
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._id,
roomId: emptyChannel._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
Expand All @@ -2488,6 +2525,26 @@ describe('[Channels]', () => {
});
});

it('should return an array of messages when inspecting a room with messages', async () => {
await request
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._id,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('messages').and.to.be.an('array').that.has.lengthOf(5);
expect(res.body).to.have.property('count', 5);
expect(res.body).to.have.property('total', 5);

const pinnedMessage = res.body.messages.find((message: any) => message.t === 'message_pinned');
expect(pinnedMessage).to.not.be.undefined;
});
});

it('should not return message when the user does NOT have the necessary permission', async () => {
await updatePermission('view-c-room', []);
await request
Expand All @@ -2502,6 +2559,89 @@ describe('[Channels]', () => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'User does not have the permissions required for this action [error-unauthorized]');
});
await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']);
});

it('should return messages that mention a single user', async () => {
await request
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._id,
mentionIds: firstUser._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('mentions').that.is.an('array').and.to.have.lengthOf(1);
expect(res.body.messages[0].mentions[0]).to.have.property('_id', firstUser._id);
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('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._id,
mentionIds: `${firstUser._id},${secondUser._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([firstUser._id, secondUser._id]);
});
});

it('should return messages that are starred by a specific user', async () => {
await request
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._id,
starredIds: 'rocketchat.internal.admin.test',
})
.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);
});
});

// Return messages that are pinned
it('should return messages that are pinned', async () => {
await request
.get(api('channels.messages'))
.set(credentials)
.query({
roomId: testChannel._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', 'rocketchat.internal.admin.test');
expect(res.body).to.have.property('count', 1);
expect(res.body).to.have.property('total', 1);
});
});
});
});
22 changes: 19 additions & 3 deletions packages/rest-typings/src/v1/channels/ChannelsMessagesProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,34 @@ import type { PaginatedRequest } from '../../helpers/PaginatedRequest';

const ajv = new Ajv({ coerceTypes: true });

// query: { 'mentions._id': { $in: string[] } } | { 'starred._id': { $in: string[] } } | { pinned: boolean };
export type ChannelsMessagesProps = PaginatedRequest<{ roomId: IRoom['_id'] }, 'ts'>;
export type ChannelsMessagesProps = PaginatedRequest<
{
roomId: IRoom['_id'];
mentionIds?: string;
starredIds?: string;
pinned?: boolean;
query?: Record<string, any>;
},
'ts'
>;

const channelsMessagesPropsSchema = {
type: 'object',
properties: {
roomId: {
type: 'string',
},
mentionIds: {
type: 'string',
},
starredIds: {
type: 'string',
},
pinned: {
type: 'string',
},
query: {
type: 'string',
nullable: true,
},
count: {
type: 'number',
Expand Down
Loading