fix: cleanRoomHistory with filesOnly=true removes all attachments including quotes (#35797)

Author: abhinavkrin

.changeset/thirty-carrots-decide.md

@@ -0,0 +1,7 @@
+---
+'@rocket.chat/model-typings': patch
+'@rocket.chat/models': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes the room history pruning behavior when filesOnly is true to ensure only file-type attachments are removed, preserving quotes and non-file attachments.

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

@@ -1,12 +1,22 @@
 import { api } from '@rocket.chat/core-services';
-import type { IRoom } from '@rocket.chat/core-typings';
+import type { IRoom, MessageAttachment } from '@rocket.chat/core-typings';
 import { Messages, Rooms, Subscriptions, ReadReceipts, Users } from '@rocket.chat/models';
 
 import { deleteRoom } from './deleteRoom';
 import { i18n } from '../../../../server/lib/i18n';
 import { FileUpload } from '../../../file-upload/server';
 import { notifyOnRoomChangedById, notifyOnSubscriptionChangedById } from '../lib/notifyListener';
 
+const FILE_CLEANUP_BATCH_SIZE = 1000;
+async function performFileAttachmentCleanupBatch(idsSet: Set<string>, replaceWith?: MessageAttachment) {
+	if (idsSet.size === 0) return;
+
+	const ids = [...idsSet];
+	await Messages.removeFileAttachmentsByMessageIds(ids, replaceWith);
+	await Messages.clearFilesByMessageIds(ids);
+	idsSet.clear();
+}
+
 export async function cleanRoomHistory({
 	rid = '',
 	latest = new Date(),
@@ -44,15 +54,25 @@ export async function cleanRoomHistory({
 		limit,
 	});
 
+	const targetMessageIdsForAttachmentRemoval = new Set<string>();
+
 	for await (const document of cursor) {
 		const uploadsStore = FileUpload.getStore('Uploads');
 
 		document.files && (await Promise.all(document.files.map((file) => uploadsStore.deleteById(file._id))));
 
 		fileCount++;
 		if (filesOnly) {
-			await Messages.updateOne({ _id: document._id }, { $unset: { file: 1 }, $set: { attachments: [{ color: '#FD745E', text }] } });
+			targetMessageIdsForAttachmentRemoval.add(document._id);
 		}
+
+		if (targetMessageIdsForAttachmentRemoval.size >= FILE_CLEANUP_BATCH_SIZE) {
+			await performFileAttachmentCleanupBatch(targetMessageIdsForAttachmentRemoval, { color: '#FD745E', text });
+		}
+	}
+
+	if (targetMessageIdsForAttachmentRemoval.size > 0) {
+		await performFileAttachmentCleanupBatch(targetMessageIdsForAttachmentRemoval, { color: '#FD745E', text });
 	}
 
 	if (filesOnly) {

apps/meteor/tests/end-to-end/api/rooms.ts

@@ -2,8 +2,18 @@ import fs from 'fs';
 import path from 'path';
 
 import type { Credentials } from '@rocket.chat/api-client';
-import type { IMessage, IRole, IRoom, ITeam, IUpload, IUser, ImageAttachmentProps, SettingValue } from '@rocket.chat/core-typings';
-import { TEAM_TYPE } from '@rocket.chat/core-typings';
+import type {
+	IMessage,
+	IRole,
+	IRoom,
+	ITeam,
+	IUpload,
+	IUser,
+	ImageAttachmentProps,
+	MessageAttachment,
+	SettingValue,
+} from '@rocket.chat/core-typings';
+import { isFileAttachment, isQuoteAttachment, TEAM_TYPE } from '@rocket.chat/core-typings';
 import { Random } from '@rocket.chat/random';
 import { assert, expect } from 'chai';
 import { after, afterEach, before, beforeEach, describe, it } from 'mocha';
@@ -1176,6 +1186,116 @@ describe('[Rooms]', () => {
 				})
 				.end(done);
 		});
+
+		it('should remove only files and file attachments when filesOnly is set to true', async () => {
+			const message1Response = await sendSimpleMessage({ roomId: publicChannel._id });
+
+			const mediaUploadResponse = await request
+				.post(api(`rooms.media/${publicChannel._id}`))
+				.set(credentials)
+				.attach('file', imgURL)
+				.expect(200);
+
+			const message2Response = await request
+				.post(api(`rooms.mediaConfirm/${publicChannel._id}/${mediaUploadResponse.body.file._id}`))
+				.set(credentials)
+				.send({ msg: 'message with file only' })
+				.expect(200);
+
+			await request
+				.post(api('rooms.cleanHistory'))
+				.set(credentials)
+				.send({
+					roomId: publicChannel._id,
+					latest: '9999-12-31T23:59:59.000Z',
+					oldest: '0001-01-01T00:00:00.000Z',
+					filesOnly: true,
+				})
+				.expect(200);
+
+			const res = await request.get(api('channels.messages')).set(credentials).query({ roomId: publicChannel._id }).expect(200);
+
+			expect(res.body.messages).to.be.an('array');
+			const messageIds = res.body.messages.map((m: IMessage) => m._id);
+			expect(messageIds).to.contain(message1Response.body.message._id);
+			expect(messageIds).to.contain(message2Response.body.message._id);
+			const cleanedMessage = res.body.messages.find((m: { _id: any }) => m._id === message2Response.body.message._id);
+			expect(cleanedMessage).to.exist;
+			expect(cleanedMessage.file).to.be.undefined;
+			expect(cleanedMessage.files?.length ?? 0).to.equal(0);
+			expect((cleanedMessage.attachments ?? []).find((a: MessageAttachment) => isFileAttachment(a))).to.be.undefined;
+
+			await request
+				.get(api('channels.files'))
+				.set(credentials)
+				.query({
+					roomId: publicChannel._id,
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('files').and.to.be.an('array');
+					expect(res.body.files).to.have.lengthOf(0);
+				});
+		});
+
+		it('should not remove quote attachments when filesOnly is set to true', async () => {
+			const siteUrl = await getSettingValueById('Site_Url');
+			const message1Response = await sendSimpleMessage({ roomId: publicChannel._id });
+			const mediaResponse = await request
+				.post(api(`rooms.media/${publicChannel._id}`))
+				.set(credentials)
+				.attach('file', imgURL)
+				.expect('Content-Type', 'application/json')
+				.expect(200);
+
+			const message2Response = await request
+				.post(api(`rooms.mediaConfirm/${publicChannel._id}/${mediaResponse.body.file._id}`))
+				.set(credentials)
+				.send({
+					msg: new URL(`/${publicChannel.fname}?msg=${message1Response.body.message._id}`, siteUrl as string).toString(),
+				})
+				.expect(200);
+
+			await request
+				.post(api('rooms.cleanHistory'))
+				.set(credentials)
+				.send({
+					roomId: publicChannel._id,
+					latest: '9999-12-31T23:59:59.000Z',
+					oldest: '0001-01-01T00:00:00.000Z',
+					filesOnly: true,
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+				});
+
+			await request
+				.get(api('channels.messages'))
+				.set(credentials)
+				.query({
+					roomId: publicChannel._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');
+					const message = (res.body.messages.find((m: { _id: any }) => m._id === message2Response.body.message._id) as IMessage) || null;
+					expect(message).not.to.be.null;
+					expect(message).to.have.property('attachments');
+					const fileAttachment = message.attachments?.find((f) => isFileAttachment(f)) || null;
+					expect(fileAttachment, 'Expected file attachments to be removed').to.be.null;
+					const quoteAttachment = message.attachments?.find((f) => isQuoteAttachment(f)) || null;
+					expect(quoteAttachment, 'Expected quote attachments to be present').not.to.be.null;
+					expect(message.file).to.be.undefined;
+					expect(message.files).to.satisfy((files: IMessage['files']) => files === undefined || files.length === 0);
+				});
+		});
+
 		it('should return success when send a valid private channel', (done) => {
 			void request
 				.post(api('rooms.cleanHistory'))

packages/model-typings/src/models/IMessagesModel.ts

@@ -293,4 +293,6 @@ export interface IMessagesModel extends IBaseModel<IMessage> {
 	decreaseReplyCountById(_id: string, inc?: number): Promise<IMessage | null>;
 	countPinned(options?: CountDocumentsOptions): Promise<number>;
 	countStarred(options?: CountDocumentsOptions): Promise<number>;
+	removeFileAttachmentsByMessageIds(_ids: string[], replaceWith?: MessageAttachment): Promise<Document | UpdateResult>;
+	clearFilesByMessageIds(_ids: string[]): Promise<Document | UpdateResult>;
 }

packages/models/src/models/Messages.ts

@@ -1783,4 +1783,48 @@ export class MessagesRaw extends BaseRaw<IMessage> implements IMessagesModel {
 		};
 		return this.findOneAndUpdate(query, update, { returnDocument: 'after' });
 	}
+
+	removeFileAttachmentsByMessageIds(_ids: string[], replaceWith?: MessageAttachment) {
+		if (!_ids || _ids.length === 0) {
+			return Promise.resolve({ acknowledged: true, modifiedCount: 0, upsertedId: null, upsertedCount: 0, matchedCount: 0 });
+		}
+		const setAttachments = replaceWith
+			? {
+					$map: {
+						input: '$attachments',
+						as: 'att',
+						in: {
+							$cond: [{ $eq: ['$$att.type', 'file'] }, replaceWith, '$$att'],
+						},
+					},
+				}
+			: {
+					$filter: {
+						input: '$attachments',
+						as: 'att',
+						cond: { $ne: ['$$att.type', 'file'] },
+					},
+				};
+
+		return this.updateMany({ _id: { $in: _ids } }, [
+			{
+				$set: {
+					attachments: setAttachments,
+				},
+			},
+		]);
+	}
+
+	clearFilesByMessageIds(_ids: string[]) {
+		if (!_ids || _ids.length === 0) {
+			return Promise.resolve({ acknowledged: true, modifiedCount: 0, upsertedId: null, upsertedCount: 0, matchedCount: 0 });
+		}
+		return this.updateMany(
+			{ _id: { $in: _ids } },
+			{
+				$set: { files: [] },
+				$unset: { file: 1 },
+			},
+		);
+	}
 }