Skip to content

Commit c4b3153

Browse files
cardosocardoso
authored
refactor: use chat.update endpoint instead of updateMessage method (RocketChat#37138)
1 parent 16da6b3 commit c4b3153

9 files changed

Lines changed: 269 additions & 82 deletions

File tree

‎apps/meteor/app/api/server/v1/chat.ts‎

Lines changed: 71 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,77 @@ const chatEndpoints = API.v1
282282

283283
return API.v1.success();
284284
},
285+
)
286+
.post(
287+
'chat.update',
288+
{
289+
authRequired: true,
290+
body: isChatUpdateProps,
291+
response: {
292+
400: validateBadRequestErrorResponse,
293+
401: validateUnauthorizedErrorResponse,
294+
200: ajv.compile<{ message: IMessage }>({
295+
type: 'object',
296+
properties: {
297+
message: { $ref: '#/components/schemas/IMessage' },
298+
success: {
299+
type: 'boolean',
300+
enum: [true],
301+
},
302+
},
303+
required: ['message', 'success'],
304+
additionalProperties: false,
305+
}),
306+
},
307+
},
308+
async function action() {
309+
const { bodyParams } = this;
310+
311+
const msg = await Messages.findOneById(bodyParams.msgId);
312+
313+
// Ensure the message exists
314+
if (!msg) {
315+
return API.v1.failure(`No message found with the id of "${bodyParams.msgId}".`);
316+
}
317+
318+
if (bodyParams.roomId !== msg.rid) {
319+
return API.v1.failure('The room id provided does not match where the message is from.');
320+
}
321+
322+
const hasContent = 'content' in bodyParams;
323+
324+
if (hasContent && msg.t !== 'e2e') {
325+
return API.v1.failure('Only encrypted messages can have content updated.');
326+
}
327+
328+
const updateData: Parameters<typeof executeUpdateMessage> = [
329+
this.userId,
330+
hasContent
331+
? {
332+
_id: msg._id,
333+
rid: msg.rid,
334+
content: bodyParams.content,
335+
...(bodyParams.e2eMentions && { e2eMentions: bodyParams.e2eMentions }),
336+
}
337+
: {
338+
_id: msg._id,
339+
rid: msg.rid,
340+
msg: bodyParams.text,
341+
...(bodyParams.customFields && { customFields: bodyParams.customFields }),
342+
},
343+
'previewUrls' in bodyParams ? bodyParams.previewUrls : undefined,
344+
];
345+
346+
// Permission checks are already done in the updateMessage method, so no need to duplicate them
347+
await applyAirGappedRestrictionsValidation(() => executeUpdateMessage(...updateData));
348+
349+
const updatedMessage = await Messages.findOneById(msg._id);
350+
const [message] = await normalizeMessagesForUser(updatedMessage ? [updatedMessage] : [], this.userId);
351+
352+
return API.v1.success({
353+
message,
354+
});
355+
},
285356
);
286357

287358
API.v1.addRoute(
@@ -421,48 +492,6 @@ API.v1.addRoute(
421492
},
422493
);
423494

424-
API.v1.addRoute(
425-
'chat.update',
426-
{ authRequired: true, validateParams: isChatUpdateProps },
427-
{
428-
async post() {
429-
const msg = await Messages.findOneById(this.bodyParams.msgId);
430-
431-
// Ensure the message exists
432-
if (!msg) {
433-
return API.v1.failure(`No message found with the id of "${this.bodyParams.msgId}".`);
434-
}
435-
436-
if (this.bodyParams.roomId !== msg.rid) {
437-
return API.v1.failure('The room id provided does not match where the message is from.');
438-
}
439-
440-
const msgFromBody = this.bodyParams.text;
441-
442-
// Permission checks are already done in the updateMessage method, so no need to duplicate them
443-
await applyAirGappedRestrictionsValidation(() =>
444-
executeUpdateMessage(
445-
this.userId,
446-
{
447-
_id: msg._id,
448-
msg: msgFromBody,
449-
rid: msg.rid,
450-
...(this.bodyParams.customFields && { customFields: this.bodyParams.customFields }),
451-
},
452-
this.bodyParams.previewUrls,
453-
),
454-
);
455-
456-
const updatedMessage = await Messages.findOneById(msg._id);
457-
const [message] = await normalizeMessagesForUser(updatedMessage ? [updatedMessage] : [], this.userId);
458-
459-
return API.v1.success({
460-
message,
461-
});
462-
},
463-
},
464-
);
465-
466495
API.v1.addRoute(
467496
'chat.react',
468497
{ authRequired: true, validateParams: isChatReactProps },

‎apps/meteor/app/lib/server/functions/updateMessage.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyLis
1111
import { validateCustomMessageFields } from '../lib/validateCustomMessageFields';
1212

1313
export const updateMessage = async function (
14-
message: AtLeast<IMessage, '_id' | 'rid' | 'msg' | 'customFields'>,
14+
message: AtLeast<IMessage, '_id' | 'rid' | 'msg' | 'customFields'> | AtLeast<IMessage, '_id' | 'rid' | 'content'>,
1515
user: IUser,
1616
originalMsg?: IMessage,
1717
previewUrls?: string[],

‎apps/meteor/app/lib/server/methods/updateMessage.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const allowedEditedFields = ['tshow', 'alias', 'attachments', 'avatar', 'emoji',
1515

1616
export async function executeUpdateMessage(
1717
uid: IUser['_id'],
18-
message: AtLeast<IMessage, '_id' | 'rid' | 'msg' | 'customFields'>,
18+
message: AtLeast<IMessage, '_id' | 'rid' | 'msg' | 'customFields'> | AtLeast<IMessage, '_id' | 'rid' | 'content'>,
1919
previewUrls?: string[],
2020
) {
2121
const originalMessage = await Messages.findOneById(message._id);

‎apps/meteor/client/lib/chats/data.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
isEncryptedMessageContent,
23
isOTRAckMessage,
34
isOTRMessage,
45
type IEditedMessage,
@@ -176,8 +177,24 @@ export const createDataAPI = ({ rid, tmid }: { rid: IRoom['_id']; tmid: IMessage
176177
Messages.state.store({ ...message, rid, ...(tmid && { tmid }) });
177178
};
178179

179-
const updateMessage = async (message: IEditedMessage, previewUrls?: string[]): Promise<void> =>
180-
sdk.call('updateMessage', message, previewUrls);
180+
const updateMessage = async (message: IEditedMessage, previewUrls?: string[]): Promise<void> => {
181+
const params = isEncryptedMessageContent(message)
182+
? {
183+
msgId: message._id,
184+
roomId: message.rid,
185+
content: message.content,
186+
e2eMentions: message.e2eMentions,
187+
}
188+
: {
189+
previewUrls,
190+
msgId: message._id,
191+
roomId: message.rid,
192+
customFields: message.customFields,
193+
text: message.msg,
194+
};
195+
196+
await sdk.rest.post('/v1/chat.update', params);
197+
};
181198

182199
const canDeleteMessage = async (message: IMessage): Promise<boolean> => {
183200
const uid = getUserId();

‎apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,4 +319,64 @@ test.describe('E2EE Encrypted Channels', () => {
319319
await lastStarredMessage.locator('role=button[name="More"]').click();
320320
await expect(page.locator('role=menuitem[name="Copy link"]')).toHaveClass(/disabled/);
321321
});
322+
323+
test('expect to edit encrypted message', async ({ page }) => {
324+
const channelName = faker.string.uuid();
325+
const originalMessage = 'This is the original encrypted message';
326+
const editedMessage = 'This is the edited encrypted message';
327+
328+
await poHomeChannel.sidenav.createEncryptedChannel(channelName);
329+
await expect(page).toHaveURL(`/group/${channelName}`);
330+
await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();
331+
332+
await poHomeChannel.content.sendMessage(originalMessage);
333+
334+
await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(originalMessage);
335+
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
336+
337+
await poHomeChannel.content.openLastMessageMenu();
338+
await poHomeChannel.content.btnOptionEditMessage.click();
339+
await poHomeChannel.content.inputMessage.fill(editedMessage);
340+
341+
await page.keyboard.press('Enter');
342+
343+
await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(editedMessage);
344+
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
345+
});
346+
347+
test('expect to edit encrypted message to include mention', async ({ page }) => {
348+
const channelName = faker.string.uuid();
349+
const originalMessage = 'This is the original encrypted message';
350+
const editedMessage = 'This is the edited encrypted message with a mention to @user1 and #general';
351+
const displayedMessage = 'This is the edited encrypted message with a mention to user1 and general';
352+
353+
await poHomeChannel.sidenav.createEncryptedChannel(channelName);
354+
await expect(page).toHaveURL(`/group/${channelName}`);
355+
await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();
356+
357+
await poHomeChannel.content.sendMessage(originalMessage);
358+
await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(originalMessage);
359+
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
360+
361+
await poHomeChannel.content.openLastMessageMenu();
362+
await poHomeChannel.content.btnOptionEditMessage.click();
363+
await poHomeChannel.content.inputMessage.fill(editedMessage);
364+
365+
await page.keyboard.press('Enter');
366+
367+
await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(displayedMessage);
368+
await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible();
369+
370+
const userMention = page.getByRole('button', {
371+
name: 'user1',
372+
});
373+
374+
await expect(userMention).toBeVisible();
375+
376+
const channelMention = page.getByRole('button', {
377+
name: 'general',
378+
});
379+
380+
await expect(channelMention).toBeVisible();
381+
});
322382
});

‎apps/meteor/tests/e2e/messaging.spec.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,7 @@ test.describe('Messaging', () => {
173173

174174
await test.step('send edited message', async () => {
175175
const editPromise = page.waitForResponse(
176-
(response) =>
177-
/api\/v1\/method.call\/updateMessage/.test(response.url()) &&
178-
response.status() === 200 &&
179-
response.request().method() === 'POST',
176+
(response) => /api\/v1\/chat.update/.test(response.url()) && response.status() === 200 && response.request().method() === 'POST',
180177
);
181178

182179
await poHomeChannel.content.sendMessage('edited msg2', false);
@@ -187,10 +184,7 @@ test.describe('Messaging', () => {
187184

188185
await test.step('stress test on message editions', async () => {
189186
const editPromise = page.waitForResponse(
190-
(response) =>
191-
/api\/v1\/method.call\/updateMessage/.test(response.url()) &&
192-
response.status() === 200 &&
193-
response.request().method() === 'POST',
187+
(response) => /api\/v1\/chat.update/.test(response.url()) && response.status() === 200 && response.request().method() === 'POST',
194188
);
195189

196190
for (const element of ['edited msg2 a', 'edited msg2 b', 'edited msg2 c', 'edited msg2 d', 'edited msg2 e']) {

‎apps/meteor/tests/e2e/page-objects/fragments/home-content.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
import fs from 'fs/promises';
2+
import { resolve, join, relative } from 'node:path';
23

34
import type { Locator, Page } from '@playwright/test';
45

56
import { expect } from '../../utils/test';
67

8+
const FIXTURES_PATH = relative(process.cwd(), resolve(__dirname, '../../fixtures/files'));
9+
10+
export function getFilePath(fileName: string): string {
11+
return join(FIXTURES_PATH, fileName);
12+
}
13+
714
export class HomeContent {
815
protected readonly page: Page;
916

@@ -379,7 +386,7 @@ export class HomeContent {
379386
}
380387

381388
async dragAndDropTxtFile(): Promise<void> {
382-
const contract = await fs.readFile('./tests/e2e/fixtures/files/any_file.txt', 'utf-8');
389+
const contract = await fs.readFile(getFilePath('any_file.txt'), 'utf-8');
383390
const dataTransfer = await this.page.evaluateHandle((contract) => {
384391
const data = new DataTransfer();
385392
const file = new File([`${contract}`], 'any_file.txt', {
@@ -395,7 +402,7 @@ export class HomeContent {
395402
}
396403

397404
async dragAndDropLstFile(): Promise<void> {
398-
const contract = await fs.readFile('./tests/e2e/fixtures/files/lst-test.lst', 'utf-8');
405+
const contract = await fs.readFile(getFilePath('lst-test.lst'), 'utf-8');
399406
const dataTransfer = await this.page.evaluateHandle((contract) => {
400407
const data = new DataTransfer();
401408
const file = new File([`${contract}`], 'lst-test.lst', {
@@ -411,7 +418,7 @@ export class HomeContent {
411418
}
412419

413420
async dragAndDropTxtFileToThread(): Promise<void> {
414-
const contract = await fs.readFile('./tests/e2e/fixtures/files/any_file.txt', 'utf-8');
421+
const contract = await fs.readFile(getFilePath('any_file.txt'), 'utf-8');
415422
const dataTransfer = await this.page.evaluateHandle((contract) => {
416423
const data = new DataTransfer();
417424
const file = new File([`${contract}`], 'any_file.txt', {
@@ -427,7 +434,7 @@ export class HomeContent {
427434
}
428435

429436
async sendFileMessage(fileName: string): Promise<void> {
430-
await this.page.locator('input[type=file]').setInputFiles(`./tests/e2e/fixtures/files/${fileName}`);
437+
await this.page.locator('input[type=file]').setInputFiles(getFilePath(fileName));
431438
}
432439

433440
async openLastMessageMenu(): Promise<void> {

‎apps/meteor/tests/end-to-end/api/chat.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,6 +1734,27 @@ describe('[Chat]', () => {
17341734
});
17351735
});
17361736

1737+
it('should fail updating a message with "content" if it is not encrypted', (done) => {
1738+
void request
1739+
.post(api('chat.update'))
1740+
.set(credentials)
1741+
.send({
1742+
roomId: testChannel._id,
1743+
msgId: message._id,
1744+
content: {
1745+
algorithm: 'rc.v1.aes-sha2',
1746+
ciphertext: 'U2FsdGVkX1+u3j0u2+oXg4o3kw5y4t7D9sdfsdff==',
1747+
},
1748+
})
1749+
.expect('Content-Type', 'application/json')
1750+
.expect(400)
1751+
.expect((res) => {
1752+
expect(res.body).to.have.property('success', false);
1753+
expect(res.body).to.have.property('error', 'Only encrypted messages can have content updated.');
1754+
})
1755+
.end(done);
1756+
});
1757+
17371758
it('should update a message successfully', (done) => {
17381759
void request
17391760
.post(api('chat.update'))

0 commit comments

Comments
 (0)