Skip to content

Commit 6f4429f

Browse files
fix: ui does not update after pruning only files (#36099)
Co-authored-by: gabriellsh <[email protected]>
1 parent d7d9a8c commit 6f4429f

File tree

11 files changed

+388
-68
lines changed

11 files changed

+388
-68
lines changed

.changeset/cold-insects-guess.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@rocket.chat/core-services': patch
3+
'@rocket.chat/ddp-client': patch
4+
'@rocket.chat/meteor': patch
5+
---
6+
7+
Fixes missing UI updates after pruning messages with "Files only" enabled.

apps/meteor/app/lib/server/functions/cleanRoomHistory.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { api } from '@rocket.chat/core-services';
2-
import type { IRoom, MessageAttachment } from '@rocket.chat/core-typings';
2+
import type { IRoom } from '@rocket.chat/core-typings';
33
import { Messages, Rooms, Subscriptions, ReadReceipts, Users } from '@rocket.chat/models';
44

55
import { deleteRoom } from './deleteRoom';
@@ -8,14 +8,6 @@ import { FileUpload } from '../../../file-upload/server';
88
import { notifyOnRoomChangedById, notifyOnSubscriptionChangedById } from '../lib/notifyListener';
99

1010
const FILE_CLEANUP_BATCH_SIZE = 1000;
11-
async function performFileAttachmentCleanupBatch(idsSet: Set<string>, replaceWith?: MessageAttachment) {
12-
if (idsSet.size === 0) return;
13-
14-
const ids = [...idsSet];
15-
await Messages.removeFileAttachmentsByMessageIds(ids, replaceWith);
16-
await Messages.clearFilesByMessageIds(ids);
17-
idsSet.clear();
18-
}
1911

2012
export async function cleanRoomHistory({
2113
rid = '',
@@ -55,6 +47,26 @@ export async function cleanRoomHistory({
5547
});
5648

5749
const targetMessageIdsForAttachmentRemoval = new Set<string>();
50+
const pruneMessageAttachment = { color: '#FD745E', text };
51+
52+
async function performFileAttachmentCleanupBatch() {
53+
if (targetMessageIdsForAttachmentRemoval.size === 0) return;
54+
55+
const ids = [...targetMessageIdsForAttachmentRemoval];
56+
await Messages.removeFileAttachmentsByMessageIds(ids, pruneMessageAttachment);
57+
await Messages.clearFilesByMessageIds(ids);
58+
void api.broadcast('notify.deleteMessageBulk', rid, {
59+
rid,
60+
excludePinned,
61+
ignoreDiscussion,
62+
ts,
63+
users: fromUsers,
64+
ids,
65+
filesOnly: true,
66+
replaceFileAttachmentsWith: pruneMessageAttachment,
67+
});
68+
targetMessageIdsForAttachmentRemoval.clear();
69+
}
5870

5971
for await (const document of cursor) {
6072
const uploadsStore = FileUpload.getStore('Uploads');
@@ -67,12 +79,12 @@ export async function cleanRoomHistory({
6779
}
6880

6981
if (targetMessageIdsForAttachmentRemoval.size >= FILE_CLEANUP_BATCH_SIZE) {
70-
await performFileAttachmentCleanupBatch(targetMessageIdsForAttachmentRemoval, { color: '#FD745E', text });
82+
await performFileAttachmentCleanupBatch();
7183
}
7284
}
7385

7486
if (targetMessageIdsForAttachmentRemoval.size > 0) {
75-
await performFileAttachmentCleanupBatch(targetMessageIdsForAttachmentRemoval, { color: '#FD745E', text });
87+
await performFileAttachmentCleanupBatch();
7688
}
7789

7890
if (filesOnly) {

apps/meteor/app/ui-utils/client/lib/LegacyRoomManager.ts

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { RoomManager } from '../../../../client/lib/RoomManager';
88
import { roomCoordinator } from '../../../../client/lib/rooms/roomCoordinator';
99
import { fireGlobalEvent } from '../../../../client/lib/utils/fireGlobalEvent';
1010
import { getConfig } from '../../../../client/lib/utils/getConfig';
11+
import { modifyMessageOnFilesDelete } from '../../../../client/lib/utils/modifyMessageOnFilesDelete';
1112
import { callbacks } from '../../../../lib/callbacks';
1213
import { Messages, Subscriptions } from '../../../models/client';
1314
import { sdk } from '../../../utils/client/lib/SDKClient';
@@ -110,6 +111,41 @@ function getOpenedRoomByRid(rid: IRoom['_id']) {
110111
.find((openedRoom) => openedRoom.rid === rid);
111112
}
112113

114+
function createDeleteQuery({
115+
excludePinned,
116+
ignoreDiscussion,
117+
rid,
118+
ts,
119+
users,
120+
ids,
121+
}: {
122+
rid: IMessage['rid'];
123+
excludePinned: boolean;
124+
ignoreDiscussion: boolean;
125+
ts: Record<string, Date>;
126+
users: string[];
127+
ids?: string[];
128+
}) {
129+
const query: Filter<IMessage> = { rid };
130+
131+
if (ids) {
132+
query._id = { $in: ids };
133+
} else {
134+
query.ts = ts;
135+
}
136+
if (excludePinned) {
137+
query.pinned = { $ne: true };
138+
}
139+
if (ignoreDiscussion) {
140+
query.drid = { $exists: false };
141+
}
142+
if (users?.length) {
143+
query['u.username'] = { $in: users };
144+
}
145+
146+
return query;
147+
}
148+
113149
const openRoom = (typeName: string, record: OpenedRoom) => {
114150
if (record.ready === true && record.streamActive === true) {
115151
return;
@@ -174,44 +210,28 @@ const openRoom = (typeName: string, record: OpenedRoom) => {
174210
({ tmid: _, ...record }) => record,
175211
);
176212
}),
177-
sdk.stream(
178-
'notify-room',
179-
[`${record.rid}/deleteMessageBulk`],
180-
({ rid, ts, excludePinned, ignoreDiscussion, users, ids, showDeletedStatus }) => {
181-
const query: Filter<IMessage> = { rid };
182-
183-
if (ids) {
184-
query._id = { $in: ids };
185-
} else {
186-
query.ts = ts;
187-
}
188-
if (excludePinned) {
189-
query.pinned = { $ne: true };
190-
}
191-
if (ignoreDiscussion) {
192-
query.drid = { $exists: false };
193-
}
194-
if (users?.length) {
195-
query['u.username'] = { $in: users };
196-
}
213+
sdk.stream('notify-room', [`${record.rid}/deleteMessageBulk`], async (params) => {
214+
const query = createDeleteQuery(params);
215+
const predicate = createPredicateFromFilter(query);
197216

198-
const predicate = createPredicateFromFilter(query);
199-
200-
if (showDeletedStatus) {
201-
return Messages.state.update(predicate, (record) => ({
202-
...record,
203-
t: 'rm',
204-
msg: '',
205-
urls: [],
206-
mentions: [],
207-
attachments: [],
208-
reactions: {},
209-
}));
210-
}
217+
if (params.filesOnly) {
218+
return Messages.state.update(predicate, (record) => modifyMessageOnFilesDelete(record, params.replaceFileAttachmentsWith));
219+
}
220+
221+
if (params.showDeletedStatus) {
222+
return Messages.state.update(predicate, (record) => ({
223+
...record,
224+
t: 'rm',
225+
msg: '',
226+
urls: [],
227+
mentions: [],
228+
attachments: [],
229+
reactions: {},
230+
}));
231+
}
211232

212-
return Messages.state.remove(predicate);
213-
},
214-
),
233+
return Messages.state.remove(predicate);
234+
}),
215235

216236
sdk.stream('notify-room', [`${record.rid}/messagesRead`], ({ tmid, until }) => {
217237
if (tmid) {

apps/meteor/client/hooks/lists/useStreamUpdatesForMessageList.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,30 @@
1-
import type { IMessage, IRoom, IUser } from '@rocket.chat/core-typings';
1+
import type { IMessage, IRoom, IUser, MessageAttachment } from '@rocket.chat/core-typings';
22
import { createPredicateFromFilter } from '@rocket.chat/mongo-adapter';
33
import { useStream } from '@rocket.chat/ui-contexts';
44
import type { Condition, Filter } from 'mongodb';
55
import { useEffect } from 'react';
66

77
import type { MessageList } from '../../lib/lists/MessageList';
8+
import { modifyMessageOnFilesDelete } from '../../lib/utils/modifyMessageOnFilesDelete';
89

9-
type NotifyRoomRidDeleteMessageBulkEvent = {
10+
type NotifyRoomRidDeleteBulkEvent = {
1011
rid: IMessage['rid'];
1112
excludePinned: boolean;
1213
ignoreDiscussion: boolean;
1314
ts: Condition<Date>;
1415
users: string[];
1516
ids?: string[]; // message ids have priority over ts
16-
showDeletedStatus?: boolean;
17-
};
17+
} & (
18+
| {
19+
filesOnly: true;
20+
replaceFileAttachmentsWith?: MessageAttachment;
21+
}
22+
| {
23+
filesOnly?: false;
24+
}
25+
);
1826

19-
const createDeleteCriteria = (params: NotifyRoomRidDeleteMessageBulkEvent): ((message: IMessage) => boolean) => {
27+
const createDeleteCriteria = (params: NotifyRoomRidDeleteBulkEvent): ((message: IMessage) => boolean) => {
2028
const query: Filter<IMessage> = {};
2129

2230
if (params.ids) {
@@ -59,6 +67,14 @@ export const useStreamUpdatesForMessageList = (messageList: MessageList, uid: IU
5967

6068
const unsubscribeFromDeleteMessageBulk = subscribeToNotifyRoom(`${rid}/deleteMessageBulk`, (params) => {
6169
const matchDeleteCriteria = createDeleteCriteria(params);
70+
if (params.filesOnly) {
71+
return messageList.batchHandle(async () => {
72+
const items = messageList.items.filter(matchDeleteCriteria).map((message) => {
73+
return modifyMessageOnFilesDelete(message, params.replaceFileAttachmentsWith);
74+
});
75+
return { items };
76+
});
77+
}
6278
messageList.prune(matchDeleteCriteria);
6379
});
6480

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import type { IMessage, MessageAttachment, FileAttachmentProps, MessageQuoteAttachment } from '@rocket.chat/core-typings';
2+
3+
import { modifyMessageOnFilesDelete } from './modifyMessageOnFilesDelete';
4+
5+
global.structuredClone = (val: any) => JSON.parse(JSON.stringify(val));
6+
7+
const fileAttachment: FileAttachmentProps = { title: 'Image', title_link: 'url', image_url: 'image.png', type: 'file' };
8+
const nonFileAttachment: MessageAttachment = { text: 'Non-file attachment', color: '#ff0000' };
9+
const quoteAttachment: MessageQuoteAttachment = {
10+
text: 'Quoted message',
11+
message_link: 'some-link',
12+
attachments: [{ text: 'Quoted inner message' }],
13+
author_icon: 'icon',
14+
author_link: 'link',
15+
author_name: 'name',
16+
};
17+
18+
const messageBase: IMessage = {
19+
_id: 'msg1',
20+
msg: 'Here is a file',
21+
file: {
22+
_id: 'file1',
23+
name: 'image.png',
24+
type: '',
25+
format: '',
26+
size: 0,
27+
},
28+
files: [
29+
{
30+
_id: 'file1',
31+
name: 'image.png',
32+
type: '',
33+
format: '',
34+
size: 0,
35+
},
36+
],
37+
attachments: [fileAttachment, nonFileAttachment, quoteAttachment],
38+
rid: '',
39+
ts: new Date().toISOString() as any,
40+
u: { username: 'username', _id: '12345' },
41+
_updatedAt: new Date().toISOString() as any,
42+
};
43+
44+
const createMessage = () => {
45+
const msg = structuredClone(messageBase);
46+
return msg as IMessage;
47+
};
48+
49+
describe('modifyMessageOnFilesDelete', () => {
50+
it('should remove `file`, empty `files`, and remove file-type attachments', () => {
51+
const message = createMessage();
52+
53+
const result = modifyMessageOnFilesDelete(message);
54+
55+
expect(result).not.toHaveProperty('file');
56+
expect(Array.isArray(result.files)).toBe(true);
57+
expect(result.files).toHaveLength(0);
58+
expect(result.attachments).toHaveLength(2);
59+
expect(result.attachments?.some((att) => att.text === 'Non-file attachment')).toBe(true);
60+
});
61+
62+
it('should replace file-type attachments if `replaceFileAttachmentsWith` is provided', () => {
63+
const message = createMessage();
64+
65+
const replacement: MessageAttachment = { text: 'File removed by prune' };
66+
67+
const result = modifyMessageOnFilesDelete(message, replacement);
68+
69+
expect(result).not.toHaveProperty('file');
70+
expect(Array.isArray(result.files)).toBe(true);
71+
expect(result.files).toHaveLength(0);
72+
expect(result.attachments?.some((att) => att.text === 'File removed by prune')).toBe(true);
73+
});
74+
75+
it('should not mutate the original message', () => {
76+
const message = createMessage();
77+
78+
const original = JSON.parse(JSON.stringify(message));
79+
modifyMessageOnFilesDelete(message);
80+
81+
expect(message).toEqual(original);
82+
});
83+
84+
it('should not remove non-file attachments such as text and quote', () => {
85+
const message = createMessage();
86+
87+
const result = modifyMessageOnFilesDelete(message);
88+
89+
expect(result.attachments?.some((att) => att.text === 'Non-file attachment')).toBe(true);
90+
expect(result.attachments?.some((att) => att.text === 'Quoted message')).toBe(true);
91+
});
92+
});
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { isFileAttachment } from '@rocket.chat/core-typings';
2+
import type { IMessage, MessageAttachment } from '@rocket.chat/core-typings';
3+
4+
/**
5+
* Clone a message and clear or replace its file attachments.
6+
*
7+
* - Performs a deep clone via `structuredClone` to avoid mutating the source.
8+
* - Removes the single-file `file` field and empties the `files` array.
9+
* - If `replaceFileAttachmentsWith` is given, swaps out any file-type
10+
* attachments; otherwise filters them out entirely.
11+
*
12+
* @param message The original `IMessage` to copy.
13+
* @param replaceFileAttachmentsWith
14+
* Optional attachment to substitute for each file attachment.
15+
* @returns A new message object with `file`, `files`, and `attachments` updated.
16+
*/
17+
18+
export const modifyMessageOnFilesDelete = <T extends IMessage = IMessage>(message: T, replaceFileAttachmentsWith?: MessageAttachment) => {
19+
const { attachments, file: _, ...rest } = message;
20+
21+
if (!attachments?.length) {
22+
return message;
23+
}
24+
25+
if (replaceFileAttachmentsWith) {
26+
return { ...rest, files: [], attachments: attachments?.map((att) => (isFileAttachment(att) ? replaceFileAttachmentsWith : att)) };
27+
}
28+
29+
return { ...rest, files: [], attachments: attachments?.filter((att) => !isFileAttachment(att)) };
30+
};

0 commit comments

Comments
 (0)