diff --git a/.changeset/fast-phones-poke.md b/.changeset/fast-phones-poke.md new file mode 100644 index 0000000000000..c0255f88396a8 --- /dev/null +++ b/.changeset/fast-phones-poke.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes `channels.messages`, `groups.messages`, `dm.messages` and `im.messages` APIs to filter out deleted messages. diff --git a/apps/meteor/app/api/server/v1/channels.ts b/apps/meteor/app/api/server/v1/channels.ts index 77d19384fe3fe..b5574e239ee4b 100644 --- a/apps/meteor/app/api/server/v1/channels.ts +++ b/apps/meteor/app/api/server/v1/channels.ts @@ -311,6 +311,7 @@ API.v1.addRoute( ...parseIds(mentionIds, 'mentions._id'), ...parseIds(starredIds, 'starred._id'), ...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}), + _hidden: { $ne: true }, }; if (!(await canAccessRoomAsync(findResult, { _id: this.userId }))) { diff --git a/apps/meteor/app/api/server/v1/groups.ts b/apps/meteor/app/api/server/v1/groups.ts index 20a11b5268d70..311e383232171 100644 --- a/apps/meteor/app/api/server/v1/groups.ts +++ b/apps/meteor/app/api/server/v1/groups.ts @@ -792,6 +792,7 @@ API.v1.addRoute( ...parseIds(mentionIds, 'mentions._id'), ...parseIds(starredIds, 'starred._id'), ...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}), + _hidden: { $ne: true }, }; const { cursor, totalCount } = Messages.findPaginated(ourQuery, { diff --git a/apps/meteor/app/api/server/v1/im.ts b/apps/meteor/app/api/server/v1/im.ts index 4cf7c5f242b28..099d17141f8b4 100644 --- a/apps/meteor/app/api/server/v1/im.ts +++ b/apps/meteor/app/api/server/v1/im.ts @@ -455,6 +455,7 @@ API.v1.addRoute( ...parseIds(mentionIds, 'mentions._id'), ...parseIds(starredIds, 'starred._id'), ...(pinned && pinned.toLowerCase() === 'true' ? { pinned: true } : {}), + _hidden: { $ne: true }, }; const sortObj = sort || { ts: -1 }; diff --git a/apps/meteor/tests/data/chat.helper.ts b/apps/meteor/tests/data/chat.helper.ts index 2d0588a7f1582..7bcab192285ec 100644 --- a/apps/meteor/tests/data/chat.helper.ts +++ b/apps/meteor/tests/data/chat.helper.ts @@ -53,9 +53,17 @@ export const starMessage = ({ messageId, requestCredentials }: { messageId: IMes .send({ messageId }); }; -export const pinMessage = ({ messageId, requestCredentials }: { messageId: IMessage['_id']; requestCredentials?: Credentials }) => { +export const pinMessage = ({ + messageId, + requestCredentials, + unpin = false, +}: { + messageId: IMessage['_id']; + requestCredentials?: Credentials; + unpin?: boolean; +}) => { return request - .post(api('chat.pinMessage')) + .post(api(unpin ? 'chat.unPinMessage' : 'chat.pinMessage')) .set(requestCredentials ?? credentials) .send({ messageId }); }; @@ -96,3 +104,20 @@ export const followMessage = ({ msgId, requestCredentials }: { msgId: IMessage[' .set(requestCredentials ?? credentials) .send({ mid: msgId }); }; + +export const updateMessage = ({ + msgId, + requestCredentials, + updatedMessage, + roomId, +}: { + msgId: IMessage['_id']; + requestCredentials?: Credentials; + updatedMessage: string; + roomId?: IRoom['_id']; +}) => { + return request + .post(api('chat.update')) + .set(requestCredentials ?? credentials) + .send({ msgId, text: updatedMessage, roomId }); +}; diff --git a/apps/meteor/tests/end-to-end/api/channels.ts b/apps/meteor/tests/end-to-end/api/channels.ts index 3b122a176738e..f4f66bf7a10f5 100644 --- a/apps/meteor/tests/end-to-end/api/channels.ts +++ b/apps/meteor/tests/end-to-end/api/channels.ts @@ -5,7 +5,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 { pinMessage, sendMessage, starMessage, updateMessage } 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'; @@ -3946,6 +3946,7 @@ describe('[Channels]', () => { let emptyChannel: IRoom; let firstUser: IUser; let secondUser: IUser; + let pinnedMessageId: IMessage['_id']; before(async () => { await updatePermission('view-c-room', ['admin', 'user', 'bot', 'app', 'anonymous']); @@ -3982,6 +3983,7 @@ describe('[Channels]', () => { starMessage({ messageId: starredMessage.body.message._id }), pinMessage({ messageId: pinnedMessage.body.message._id }), ]); + pinnedMessageId = pinnedMessage.body.message._id; }); after(async () => { @@ -4125,6 +4127,79 @@ describe('[Channels]', () => { }); }); + describe('_hidden messages behavior when Message_KeepHistory is enabled', async () => { + before(async () => { + await updateSetting('Message_KeepHistory', true); + await pinMessage({ messageId: pinnedMessageId, unpin: true }); + }); + + after(async () => { + await updateSetting('Message_KeepHistory', false); + }); + + it('should return all messages, without any pinned 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'); + expect(res.body.messages).to.have.lengthOf(5); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('pinned', true); + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + + it('should return no pinned messages', 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(0); + expect(res.body).to.have.property('count', 0); + expect(res.body).to.have.property('total', 0); + }); + }); + + it('should not return old message when updating a message', async () => { + await updateMessage({ msgId: pinnedMessageId, updatedMessage: 'message was unpinned', roomId: testChannel._id }); + + 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'); + expect(res.body.messages).to.have.lengthOf(5); + + const updatedMessage = res.body.messages.find((msg: IMessage) => msg._id === pinnedMessageId); + + expect(updatedMessage).to.have.property('msg', 'message was unpinned'); + expect(updatedMessage).to.have.property('editedAt'); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + }); + describe('Additional Visibility Tests', () => { let outsiderUser: IUser; let insideUser: IUser; diff --git a/apps/meteor/tests/end-to-end/api/direct-message.ts b/apps/meteor/tests/end-to-end/api/direct-message.ts index 0ece886ad3b05..c449ee29e8ac9 100644 --- a/apps/meteor/tests/end-to-end/api/direct-message.ts +++ b/apps/meteor/tests/end-to-end/api/direct-message.ts @@ -5,7 +5,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 { pinMessage, sendMessage, starMessage, updateMessage } from '../../data/chat.helper'; import { updateSetting, updatePermission } from '../../data/permissions.helper'; import { deleteRoom } from '../../data/rooms.helper'; import { testFileUploads } from '../../data/uploads.helper'; @@ -482,6 +482,7 @@ describe('[Direct Messages]', () => { let testUserDMRoom: IRoom; let testUserCredentials: Credentials; let testUser2Credentials: Credentials; + let pinnedMessageId: IMessage['_id']; let messages: Pick[] = []; @@ -538,6 +539,7 @@ describe('[Direct Messages]', () => { starMessage({ messageId: starredMessage.body.message._id, requestCredentials: testUserCredentials }), pinMessage({ messageId: pinnedMessage.body.message._id, requestCredentials: testUserCredentials }), ]); + pinnedMessageId = pinnedMessage.body.message._id; }); after(async () => Promise.all([deleteUser(testUser), deleteUser(testUser2)])); @@ -702,6 +704,82 @@ describe('[Direct Messages]', () => { expect(res.body).to.have.property('total', 1); }); }); + + describe('_hidden messages behavior when Message_KeepHistory is enabled', async () => { + before(async () => { + await updateSetting('Message_KeepHistory', true); + await pinMessage({ messageId: pinnedMessageId, unpin: true, requestCredentials: testUserCredentials }); + }); + + after(async () => { + await updateSetting('Message_KeepHistory', false); + }); + + it('should return all messages, without any pinned messages', async () => { + await request + .get(api('im.messages')) + .set(testUserCredentials) + .query({ roomId: testUserDMRoom._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'); + expect(res.body.messages).to.have.lengthOf(5); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('pinned', true); + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + + it('should return no pinned messages', 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(0); + expect(res.body).to.have.property('count', 0); + expect(res.body).to.have.property('total', 0); + }); + }); + + it('should not return old message when updating a message', async () => { + await updateMessage({ + msgId: pinnedMessageId, + updatedMessage: 'message was unpinned', + roomId: testUserDMRoom._id, + requestCredentials: testUser2Credentials, + }); + + await request + .get(api('im.messages')) + .set(testUserCredentials) + .query({ roomId: testUserDMRoom._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'); + expect(res.body.messages).to.have.lengthOf(5); + const updatedMessage = res.body.messages.find((msg: IMessage) => msg._id === pinnedMessageId); + expect(updatedMessage).to.have.property('msg', 'message was unpinned'); + expect(updatedMessage).to.have.property('editedAt'); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + }); }); describe('/im.messages.others', () => { diff --git a/apps/meteor/tests/end-to-end/api/groups.ts b/apps/meteor/tests/end-to-end/api/groups.ts index d911ee9c6b4aa..b92ac79bd3046 100644 --- a/apps/meteor/tests/end-to-end/api/groups.ts +++ b/apps/meteor/tests/end-to-end/api/groups.ts @@ -4,7 +4,7 @@ import { assert, expect } from 'chai'; import { after, before, describe, it } from 'mocha'; import { getCredentials, api, request, credentials, apiPrivateChannelName } from '../../data/api-data'; -import { pinMessage, starMessage, sendMessage } from '../../data/chat.helper'; +import { pinMessage, starMessage, sendMessage, updateMessage } from '../../data/chat.helper'; import { CI_MAX_ROOMS_PER_GUEST as maxRoomsPerGuest } from '../../data/constants'; import { createGroup, deleteGroup } from '../../data/groups.helper'; import { createIntegration, removeIntegration } from '../../data/integration.helper'; @@ -500,6 +500,7 @@ describe('[Groups]', () => { let testGroup: IRoom; let firstUser: IUser; let secondUser: IUser; + let pinnedMessageId: IMessage['_id']; before(async () => { testGroup = (await createGroup({ name: `test-group-${Date.now()}` })).body.group; @@ -533,6 +534,8 @@ describe('[Groups]', () => { starMessage({ messageId: starredMessage.body.message._id }), pinMessage({ messageId: pinnedMessage.body.message._id }), ]); + + pinnedMessageId = pinnedMessage.body.message._id; }); after(async () => { @@ -670,6 +673,79 @@ describe('[Groups]', () => { await Promise.all([deleteGroup({ roomName: secondGroup.name }), deleteGroup({ roomName: thirdGroup.name })]); } }); + + describe('_hidden messages behavior when Message_KeepHistory is enabled', async () => { + before(async () => { + await updateSetting('Message_KeepHistory', true); + await pinMessage({ messageId: pinnedMessageId, unpin: true }); + }); + + after(async () => { + await updateSetting('Message_KeepHistory', false); + }); + + it('should return all messages, without any pinned messages', async () => { + await request + .get(api('groups.messages')) + .set(credentials) + .query({ roomId: testGroup._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'); + expect(res.body.messages).to.have.lengthOf(5); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('pinned', true); + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + + it('should return no pinned messages', async () => { + await request + .get(api('groups.messages')) + .set(credentials) + .query({ + roomId: testGroup._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(0); + expect(res.body).to.have.property('count', 0); + expect(res.body).to.have.property('total', 0); + }); + }); + + it('should not return old message when updating a message', async () => { + await updateMessage({ msgId: pinnedMessageId, updatedMessage: 'message was unpinned', roomId: testGroup._id }); + + await request + .get(api('groups.messages')) + .set(credentials) + .query({ roomId: testGroup._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'); + expect(res.body.messages).to.have.lengthOf(5); + + const updatedMessage = res.body.messages.find((msg: IMessage) => msg._id === pinnedMessageId); + + expect(updatedMessage).to.have.property('msg', 'message was unpinned'); + expect(updatedMessage).to.have.property('editedAt'); + + res.body.messages.forEach((msg: IMessage) => { + expect(msg).to.not.have.property('_hidden'); + }); + }); + }); + }); }); describe('/groups.invite', async () => {