RocketChat · kodiakhq · Feb 20, 2025 · Feb 3, 2025 · Feb 4, 2025 · Feb 5, 2025
diff --git a/.changeset/five-cherries-hang.md b/.changeset/five-cherries-hang.md
@@ -0,0 +1,20 @@
+---
+'@rocket.chat/omnichannel-transcript': patch
+'@rocket.chat/authorization-service': patch
+'@rocket.chat/stream-hub-service': patch
+'@rocket.chat/network-broker': patch
+'@rocket.chat/presence-service': patch
+'@rocket.chat/fuselage-ui-kit': patch
+'@rocket.chat/account-service': patch
+'@rocket.chat/ui-video-conf': patch
+'@rocket.chat/core-typings': patch
+'@rocket.chat/ddp-streamer': patch
+'@rocket.chat/queue-worker': patch
+'@rocket.chat/apps-engine': patch
+'@rocket.chat/ui-client': patch
+'@rocket.chat/apps': patch
+'@rocket.chat/i18n': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes behavior of app updates that would save undesired field changes to documents
@@ -5,7 +5,7 @@ import type { ITypingDescriptor } from '@rocket.chat/apps-engine/server/bridges/
 import { MessageBridge } from '@rocket.chat/apps-engine/server/bridges/MessageBridge';
 import { api } from '@rocket.chat/core-services';
 import type { IMessage } from '@rocket.chat/core-typings';
-import { Users, Subscriptions, Messages } from '@rocket.chat/models';
+import { Users, Subscriptions } from '@rocket.chat/models';
 
 import { deleteMessage } from '../../../lib/server/functions/deleteMessage';
 import { updateMessage } from '../../../lib/server/functions/updateMessage';
@@ -44,12 +44,8 @@ export class AppMessageBridge extends MessageBridge {
 			throw new Error('Invalid editor assigned to the message for the update.');
 		}
 
-		if (!message.id || !(await Messages.findOneById(message.id))) {
-			throw new Error('A message must exist to update.');
-		}
-
 		// #TODO: #AppsEngineTypes - Remove explicit types and typecasts once the apps-engine definition/implementation mismatch is fixed.
-		const msg: IMessage | undefined = await this.orch.getConverters()?.get('messages').convertAppMessage(message);
+		const msg = await this.orch.getConverters()?.get('messages').convertAppMessage(message, true);
 		const editor = await Users.findOneById(message.editor.id);
 
 		if (!editor) {

@@ -163,13 +163,13 @@ export class AppRoomBridge extends RoomBridge {
 	protected async update(room: IRoom, members: Array<string> = [], appId: string): Promise<void> {
 		this.orch.debugLog(`The App ${appId} is updating a room.`);
 
-		if (!room.id || !(await Rooms.findOneById(room.id))) {
-			throw new Error('A room must exist to update.');
-		}
+		const rm = await this.orch.getConverters()?.get('rooms').convertAppRoom(room, true);
 
-		const rm = await this.orch.getConverters()?.get('rooms').convertAppRoom(room);
+		const updateResult = await Rooms.updateOne({ _id: room.id }, { $set: rm });
 
-		await Rooms.updateOne({ _id: rm._id }, { $set: rm as Partial<ICoreRoom> });
+		if (!updateResult.matchedCount) {
+			throw new Error('Room id not found');
+		}
 
 		for await (const username of members) {
 			const member = await Users.findOneByUsername(username, {});
@@ -178,7 +178,7 @@ export class AppRoomBridge extends RoomBridge {
 				continue;
 			}
 
-			await addUserToRoom(rm._id, member);
+			await addUserToRoom(room.id, member);
 		}
 	}
 

@@ -137,19 +137,23 @@ export class AppMessagesConverter {
 		return transformMappedData(msgObj, map);
 	}
 
-	async convertAppMessage(message) {
-		if (!message || !message.room) {
+	async convertAppMessage(message, isPartial = false) {
+		if (!message) {
 			return undefined;
 		}
 
-		const room = await Rooms.findOneById(message.room.id);
+		let rid;
+		if (message.room?.id) {
+			const room = await Rooms.findOneById(message.room.id, { projection: { _id: 1 } });
+			rid = room?._id;
+		}
 
-		if (!room) {
+		if (!rid && !isPartial) {
 			throw new Error('Invalid room provided on the message.');
 		}
 
 		let u;
-		if (message.sender && message.sender.id) {
+		if (message.sender?.id) {
 			const user = await Users.findOneById(message.sender.id);
 
 			if (user) {
@@ -178,14 +182,27 @@ export class AppMessagesConverter {
 
 		const attachments = this._convertAppAttachments(message.attachments);
 
+		let _id = message.id;
+		let ts = message.createdAt;
+
+		if (!isPartial) {
+			if (!message.id) {
+				_id = Random.id();
+			}
+
+			if (!message.createdAt) {
+				ts = new Date();
+			}
+		}
+
 		const newMessage = {
-			_id: message.id || Random.id(),
+			_id,
 			...('threadId' in message && { tmid: message.threadId }),
-			rid: room._id,
+			rid,
 			u,
 			msg: message.text,
-			ts: message.createdAt || new Date(),
-			_updatedAt: message.updatedAt || new Date(),
+			ts,
+			_updatedAt: message.updatedAt,
 			...(editedBy && { editedBy }),
 			...('editedAt' in message && { editedAt: message.editedAt }),
 			...('emoji' in message && { emoji: message.emoji }),
@@ -200,7 +217,17 @@ export class AppMessagesConverter {
 			...('token' in message && { token: message.token }),
 		};
 
-		return Object.assign(newMessage, message._unmappedProperties_);
+		if (isPartial) {
+			Object.entries(newMessage).forEach(([key, value]) => {
+				if (typeof value === 'undefined') {
+					delete newMessage[key];
+				}
+			});
+		} else {
+			Object.assign(newMessage, message._unmappedProperties_);
+		}
+
+		return newMessage;
 	}
 
 	_convertAppAttachments(attachments) {

@@ -20,7 +20,7 @@ export class AppRoomsConverter {
 		return this.convertRoom(room);
 	}
 
-	async convertAppRoom(room) {
+	async convertAppRoom(room, isPartial = false) {
 		if (!room) {
 			return undefined;
 		}
@@ -88,24 +88,44 @@ export class AppRoomsConverter {
 			contactId = contact._id;
 		}
 
+		let _default;
+		if (typeof room.isDefault !== 'undefined') {
+			_default = room.isDefault;
+		}
+
+		let ro;
+		if (typeof room.isReadOnly !== 'undefined') {
+			ro = room.isReadOnly;
+		}
+
+		let sysMes;
+		if (typeof room.displaySystemMessages !== 'undefined') {
+			sysMes = room.displaySystemMessages;
+		}
+
+		let msgs;
+		if (typeof room.messageCount !== 'undefined') {
+			msgs = room.messageCount;
+		}
+
 		const newRoom = {
 			...(room.id && { _id: room.id }),
 			fname: room.displayName,
 			name: room.slugifiedName,
 			t: room.type,
 			u,
 			v,
+			ro,
+			sysMes,
+			msgs,
 			departmentId,
 			servedBy,
 			closedBy,
 			members: room.members,
 			uids: room.userIds,
-			default: typeof room.isDefault === 'undefined' ? false : room.isDefault,
-			ro: typeof room.isReadOnly === 'undefined' ? false : room.isReadOnly,
-			sysMes: typeof room.displaySystemMessages === 'undefined' ? true : room.displaySystemMessages,
+			default: _default,
 			waitingResponse: typeof room.isWaitingResponse === 'undefined' ? undefined : !!room.isWaitingResponse,
 			open: typeof room.isOpen === 'undefined' ? undefined : !!room.isOpen,
-			msgs: room.messageCount || 0,
 			ts: room.createdAt,
 			_updatedAt: room.updatedAt,
 			closedAt: room.closedAt,
@@ -122,7 +142,17 @@ export class AppRoomsConverter {
 			}),
 		};
 
-		return Object.assign(newRoom, room._unmappedProperties_);
+		if (isPartial) {
+			Object.entries(newRoom).forEach(([key, value]) => {
+				if (typeof value === 'undefined') {
+					delete newRoom[key];
+				}
+			});
+		} else {
+			Object.assign(newRoom, room._unmappedProperties_);
+		}
+
+		return newRoom;
 	}
 
 	async convertRoom(originalRoom) {
@@ -245,6 +275,7 @@ export class AppRoomsConverter {
 				if (originalRoom.closer === 'user') {
 					return this.orch.getConverters().get('users').convertById(closedBy._id);
 				}
+
 				return this.orch.getConverters().get('visitors').convertById(closedBy._id);
 			},
 			servedBy: async (room) => {

@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import proxyquire from 'proxyquire';
 
-import { appMessageMock, appMessageInvalidRoomMock } from './mocks/data/messages.data';
+import { appMessageMock, appMessageInvalidRoomMock, appPartialMessageMock } from './mocks/data/messages.data';
 import { MessagesMock } from './mocks/models/Messages.mock';
 import { RoomsMock } from './mocks/models/Rooms.mock';
 import { UsersMock } from './mocks/models/Users.mock';
@@ -134,13 +134,39 @@ describe('The AppMessagesConverter instance', () => {
 			});
 		});
 
+		it('should return a proper schema when receiving a partial object', async () => {
+			const rocketchatMessage = await messagesConverter.convertAppMessage(appPartialMessageMock, true);
+
+			expect(rocketchatMessage).to.have.property('_id', 'appPartialMessageMock');
+			expect(rocketchatMessage).to.have.property('groupable', false);
+			expect(rocketchatMessage).to.have.property('emoji', ':smirk:');
+			expect(rocketchatMessage).to.have.property('alias', 'rocket.feline');
+
+			expect(rocketchatMessage).to.not.have.property('ts');
+			expect(rocketchatMessage).to.not.have.property('u');
+			expect(rocketchatMessage).to.not.have.property('rid');
+			expect(rocketchatMessage).to.not.have.property('_updatedAt');
+		});
+
 		it('should merge `_unmappedProperties_` into the returned message', async () => {
 			const rocketchatMessage = await messagesConverter.convertAppMessage(appMessageMock);
 
 			expect(rocketchatMessage).not.to.have.property('_unmappedProperties_');
 			expect(rocketchatMessage).to.have.property('t', 'uj');
 		});
 
+		it('should not merge `_unmappedProperties_` into the returned message when receiving a partial object', async () => {
+			const invalidPartialMessage = structuredClone(appPartialMessageMock);
+			invalidPartialMessage._unmappedProperties_ = {
+				t: 'uj',
+			};
+
+			const rocketchatMessage = await messagesConverter.convertAppMessage(invalidPartialMessage, true);
+
+			expect(rocketchatMessage).to.not.have.property('_unmappedProperties_');
+			expect(rocketchatMessage).to.not.have.property('t');
+		});
+
 		it('should throw if message has an invalid room', async () => {
 			try {
 				await messagesConverter.convertAppMessage(appMessageInvalidRoomMock);

@@ -56,6 +56,14 @@ export const appMessageMock = {
 	},
 };
 
+export const appPartialMessageMock = {
+	id: 'appPartialMessageMock',
+	text: 'rocket.cat',
+	groupable: false,
+	emoji: ':smirk:',
+	alias: 'rocket.feline',
+};
+
 export const appMessageInvalidRoomMock = {
 	id: 'appMessageInvalidRoomMock',
 	text: 'rocket.cat',

@@ -108,6 +108,13 @@ export class RoomsMock extends BaseModelMock {
 			customFields: {},
 		},
 
+		GENERALPartial: {
+			id: 'GENERAL',
+			slugifiedName: 'general',
+			displaySystemMessages: true,
+			updatedAt: new Date('2019-04-10T17:44:34.931Z'),
+		},
+
 		LivechatRoom: {
 			id: 'LivechatRoom',
 			slugifiedName: undefined,