From 34d49bb8fe22eef4b0f1dcc124d5590a2a647d2f Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Fri, 25 Apr 2025 11:21:29 -0600 Subject: [PATCH 1/5] apps update should not introduce undefined values to document --- apps/meteor/app/apps/server/converters/rooms.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/apps/server/converters/rooms.js b/apps/meteor/app/apps/server/converters/rooms.js index 5fc1c5ab57bf8..4ea39bbcc13f8 100644 --- a/apps/meteor/app/apps/server/converters/rooms.js +++ b/apps/meteor/app/apps/server/converters/rooms.js @@ -137,12 +137,12 @@ export class AppRoomsConverter { const newRoom = { ...(room.id && { _id: room.id }), - t: room.type, - ts: room.createdAt, - msgs: room.messageCount || 0, - _updatedAt: room.updatedAt, + ...(typeof room.type !== 'undefined' && { t: room.type }), + ...(typeof room.ts !== 'undefined' && { ts: room.createdAt }), + ...(typeof room.msgs !== 'undefined' && { msgs: room.messageCount || 0 }), + ...(typeof room._updatedAt !== 'undefined' && { _updatedAt: room.updatedAt }), ...(room.displayName && { fname: room.displayName }), - ...(room.type !== 'd' && { name: room.slugifiedName }), + ...(room.type !== 'd' && room.slugifiedName && { name: room.slugifiedName }), ...(room.members && { members: room.members }), ...(typeof room.isDefault !== 'undefined' && { default: room.isDefault }), ...(typeof room.isReadOnly !== 'undefined' && { ro: room.isReadOnly }), @@ -170,7 +170,11 @@ export class AppRoomsConverter { }; if (!isPartial) { - Object.assign(newRoom, room._unmappedProperties_); + Object.assign( + newRoom, + { t: room.type, ts: room.createdAt, msgs: room.messageCount || 0, _updatedAt: room.updatedAt }, + room._unmappedProperties_, + ); } return newRoom; From 1279c0a1566a7797134e325b12cf4c5b731774d8 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Fri, 25 Apr 2025 13:17:13 -0600 Subject: [PATCH 2/5] tests --- .../meteor/app/apps/server/converters/rooms.js | 4 ++-- .../app/apps/server/mocks/models/Rooms.mock.js | 9 +++++++++ .../tests/unit/app/apps/server/rooms.tests.ts | 18 +++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/apps/meteor/app/apps/server/converters/rooms.js b/apps/meteor/app/apps/server/converters/rooms.js index 4ea39bbcc13f8..f190c20b893ac 100644 --- a/apps/meteor/app/apps/server/converters/rooms.js +++ b/apps/meteor/app/apps/server/converters/rooms.js @@ -139,8 +139,8 @@ export class AppRoomsConverter { ...(room.id && { _id: room.id }), ...(typeof room.type !== 'undefined' && { t: room.type }), ...(typeof room.ts !== 'undefined' && { ts: room.createdAt }), - ...(typeof room.msgs !== 'undefined' && { msgs: room.messageCount || 0 }), - ...(typeof room._updatedAt !== 'undefined' && { _updatedAt: room.updatedAt }), + ...(typeof room.messageCount !== 'undefined' && { msgs: room.messageCount || 0 }), + ...(typeof room.updatedAt !== 'undefined' && { _updatedAt: room.updatedAt }), ...(room.displayName && { fname: room.displayName }), ...(room.type !== 'd' && room.slugifiedName && { name: room.slugifiedName }), ...(room.members && { members: room.members }), diff --git a/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js b/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js index 6cf0370f1754e..632aa7cbf2ba9 100644 --- a/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js +++ b/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js @@ -115,6 +115,15 @@ export class RoomsMock extends BaseModelMock { updatedAt: new Date('2019-04-10T17:44:34.931Z'), }, + GENERALPartialWithOptionalProps: { + id: 'GENERAL', + slugifiedName: 'general', + displaySystemMessages: true, + updatedAt: new Date('2019-04-10T17:44:34.931Z'), + messageCount: 40, + type: 'c', + }, + LivechatRoom: { id: 'LivechatRoom', slugifiedName: undefined, diff --git a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts index 0e7508556cf74..18ddbdc975c44 100644 --- a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts +++ b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts @@ -112,9 +112,25 @@ describe('The AppMessagesConverter instance', () => { expect(rocketchatRoom).to.have.property('_id', appRoom.id); expect(rocketchatRoom).to.have.property('name', appRoom.slugifiedName); expect(rocketchatRoom).to.have.property('sysMes', appRoom.displaySystemMessages); - expect(rocketchatRoom).to.have.property('msgs', 0); expect(rocketchatRoom).to.have.property('_updatedAt', appRoom.updatedAt); + expect(rocketchatRoom).to.not.have.property('msgs'); + expect(rocketchatRoom).to.not.have.property('ro'); + expect(rocketchatRoom).to.not.have.property('default'); + expect(rocketchatRoom).to.not.have.property('t'); + }); + + it('should return a proper schema when receiving a partial object', async () => { + const appRoom = RoomsMock.convertedData.GENERALPartialWithOptionalProps as unknown as IAppsRoom; + const rocketchatRoom = await roomConverter.convertAppRoom(appRoom, true); + + expect(rocketchatRoom).to.have.property('_id', appRoom.id); + expect(rocketchatRoom).to.have.property('name', appRoom.slugifiedName); + expect(rocketchatRoom).to.have.property('sysMes', appRoom.displaySystemMessages); + expect(rocketchatRoom).to.have.property('_updatedAt', appRoom.updatedAt); + expect(rocketchatRoom).to.have.property('msgs', appRoom.messageCount); + expect(rocketchatRoom).to.have.property('t', 'c'); + expect(rocketchatRoom).to.not.have.property('ro'); expect(rocketchatRoom).to.not.have.property('default'); }); From 6f8626db2413d7e3037460a410c0496525186954 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Fri, 25 Apr 2025 15:32:42 -0600 Subject: [PATCH 3/5] tests --- apps/meteor/app/apps/server/converters/rooms.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/apps/server/converters/rooms.js b/apps/meteor/app/apps/server/converters/rooms.js index f190c20b893ac..045c341186810 100644 --- a/apps/meteor/app/apps/server/converters/rooms.js +++ b/apps/meteor/app/apps/server/converters/rooms.js @@ -138,7 +138,7 @@ export class AppRoomsConverter { const newRoom = { ...(room.id && { _id: room.id }), ...(typeof room.type !== 'undefined' && { t: room.type }), - ...(typeof room.ts !== 'undefined' && { ts: room.createdAt }), + ...(typeof room.createdAt !== 'undefined' && { ts: room.createdAt }), ...(typeof room.messageCount !== 'undefined' && { msgs: room.messageCount || 0 }), ...(typeof room.updatedAt !== 'undefined' && { _updatedAt: room.updatedAt }), ...(room.displayName && { fname: room.displayName }), @@ -170,11 +170,7 @@ export class AppRoomsConverter { }; if (!isPartial) { - Object.assign( - newRoom, - { t: room.type, ts: room.createdAt, msgs: room.messageCount || 0, _updatedAt: room.updatedAt }, - room._unmappedProperties_, - ); + Object.assign(newRoom, room._unmappedProperties_); } return newRoom; From fb2469e6a343ed84b44957888e1beee86656bbb1 Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 28 Apr 2025 08:12:28 -0600 Subject: [PATCH 4/5] test --- .../apps/server/mocks/models/Rooms.mock.js | 4 ++ .../tests/unit/app/apps/server/rooms.tests.ts | 41 ++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js b/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js index 632aa7cbf2ba9..de59997324c67 100644 --- a/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js +++ b/apps/meteor/tests/unit/app/apps/server/mocks/models/Rooms.mock.js @@ -124,6 +124,10 @@ export class RoomsMock extends BaseModelMock { type: 'c', }, + UpdatedRoom: { + customFields: { custom: 'field' }, + }, + LivechatRoom: { id: 'LivechatRoom', slugifiedName: undefined, diff --git a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts index 18ddbdc975c44..636b2cfc3defc 100644 --- a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts +++ b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts @@ -22,7 +22,7 @@ const { AppRoomsConverter } = proxyquire.noCallThru().load('../../../../../app/a }, }); -describe('The AppMessagesConverter instance', () => { +describe.only('The AppMessagesConverter instance', () => { let roomConverter: IAppRoomsConverter; let roomsMock: RoomsMock; @@ -134,5 +134,44 @@ describe('The AppMessagesConverter instance', () => { expect(rocketchatRoom).to.not.have.property('ro'); expect(rocketchatRoom).to.not.have.property('default'); }); + + it('should not include properties that are not present in the app room', async () => { + const appRoom = RoomsMock.convertedData.UpdatedRoom as unknown as IAppsRoom; + const rocketchatRoom = await roomConverter.convertAppRoom(appRoom, true); + + expect(rocketchatRoom).to.have.property('customFields'); + expect(rocketchatRoom).to.not.have.property('_id'); + expect(rocketchatRoom).to.not.have.property('t'); + }); + + it('should not include name as undefined if the room doesnt have a name property', async () => { + const appRoom = RoomsMock.convertedData.UpdatedRoom as unknown as IAppsRoom; + const rocketchatRoom = await roomConverter.convertAppRoom(appRoom, true); + + expect(rocketchatRoom.name).to.be.undefined; + }); + + it('should include a name if the source room has slugifiedName property', async () => { + const appRoom = RoomsMock.convertedData.GENERALPartialWithOptionalProps as unknown as IAppsRoom; + const rocketchatRoom = await roomConverter.convertAppRoom(appRoom, true); + + expect(rocketchatRoom.name).to.equal(appRoom.slugifiedName); + }); + + it('should not use _unmappedProperties when the room is a partial object', async () => { + const appRoom = RoomsMock.convertedData.GENERALPartialWithOptionalProps as unknown as IAppsRoom; + // @ts-expect-error - _unmappedProperties + const rocketchatRoom = await roomConverter.convertAppRoom({ ...appRoom, _unmappedProperties_: { unmapped: 'property' } }, true); + + expect(rocketchatRoom).to.not.have.property('unmapped'); + }); + + it('should use _unmappedProperties when the room is a partial object', async () => { + const appRoom = RoomsMock.convertedData.GENERALPartialWithOptionalProps as unknown as IAppsRoom; + // @ts-expect-error - _unmappedProperties + const rocketchatRoom = await roomConverter.convertAppRoom({ ...appRoom, _unmappedProperties_: { unmapped: 'property' } }, false); + + expect(rocketchatRoom).to.have.property('unmapped', 'property'); + }); }); }); From d252bf3ab1afd1f9ea5586d5cd16cccb2ca0c63b Mon Sep 17 00:00:00 2001 From: Kevin Aleman Date: Mon, 28 Apr 2025 08:19:27 -0600 Subject: [PATCH 5/5] monday --- apps/meteor/tests/unit/app/apps/server/rooms.tests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts index 636b2cfc3defc..fa2dabcb20313 100644 --- a/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts +++ b/apps/meteor/tests/unit/app/apps/server/rooms.tests.ts @@ -22,7 +22,7 @@ const { AppRoomsConverter } = proxyquire.noCallThru().load('../../../../../app/a }, }); -describe.only('The AppMessagesConverter instance', () => { +describe('The AppMessagesConverter instance', () => { let roomConverter: IAppRoomsConverter; let roomsMock: RoomsMock;