diff --git a/apps/meteor/server/services/room/service.ts b/apps/meteor/server/services/room/service.ts index a860e965a9842..aa3985a8920af 100644 --- a/apps/meteor/server/services/room/service.ts +++ b/apps/meteor/server/services/room/service.ts @@ -36,7 +36,7 @@ import { removeRoomOwner } from '../../methods/removeRoomOwner'; export class RoomService extends ServiceClassInternal implements IRoomService { protected name = 'room'; - async updateDirectMessageRoomName(room: IRoom): Promise { + async updateDirectMessageRoomName(room: IRoom, ignoreStatusFromSubs?: string[]): Promise { const subs = await Subscriptions.findByRoomId(room._id, { projection: { u: 1, status: 1 } }).toArray(); const uids = subs.map((sub) => sub.u._id); @@ -47,7 +47,7 @@ export class RoomService extends ServiceClassInternal implements IRoomService { for await (const sub of subs) { // don't update the name if the user is invited but hasn't accepted yet - if (sub.status === 'INVITED') { + if (!ignoreStatusFromSubs?.includes(sub._id) && sub.status === 'INVITED') { continue; } await Subscriptions.updateOne({ _id: sub._id }, { $set: roomNames[sub.u._id] }); diff --git a/ee/packages/federation-matrix/src/FederationMatrix.ts b/ee/packages/federation-matrix/src/FederationMatrix.ts index 52c1cebfc6727..bb47f5b2d9fab 100644 --- a/ee/packages/federation-matrix/src/FederationMatrix.ts +++ b/ee/packages/federation-matrix/src/FederationMatrix.ts @@ -291,13 +291,9 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS continue; } - if (!isUserNativeFederated(member)) { - continue; - } - try { await federationSDK.inviteUserToRoom( - userIdSchema.parse(member.username), + isUserNativeFederated(member) ? userIdSchema.parse(member.username) : `@${member.username}:${this.serverName}`, roomIdSchema.parse(matrixRoomResult.room_id), userIdSchema.parse(actualMatrixUserId), ); @@ -919,8 +915,6 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS if (action === 'accept') { await federationSDK.acceptInvite(room.federation.mrid, matrixUserId); - - await Room.performAcceptRoomInvite(room, subscription, user); } if (action === 'reject') { diff --git a/ee/packages/federation-matrix/src/events/member.ts b/ee/packages/federation-matrix/src/events/member.ts index 5b643b40e03bd..8de384253281a 100644 --- a/ee/packages/federation-matrix/src/events/member.ts +++ b/ee/packages/federation-matrix/src/events/member.ts @@ -215,7 +215,7 @@ async function handleJoin({ // update room name for DMs if (room.t === 'd') { - await Room.updateDirectMessageRoomName(room); + await Room.updateDirectMessageRoomName(room, [subscription._id]); } if (!subscription.status) { diff --git a/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts index 78e38469169b8..083d695f4b989 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/dms.spec.ts @@ -1097,7 +1097,7 @@ const waitForRoomEvent = async ( }); }); - it.failing('should accept the invitation by the Rocket.Chat user', async () => { + it('should accept the invitation by the Rocket.Chat user', async () => { const response = await acceptRoomInvite(rcRoom._id, rcUserConfig2); expect(response.success).toBe(true); }); @@ -1204,22 +1204,7 @@ const waitForRoomEvent = async ( }); beforeAll(async () => { - // Create non-federated DM between rcUser1 and rcUser2 - const nonFedDmResponse = await rcUser1.config.request - .post(api('dm.create')) - .set(rcUser1.config.credentials) - .send({ - username: rcUser2.username, - }) - .expect(200); - - expect(nonFedDmResponse.body).toHaveProperty('success', true); - - const roomInfoNonFed = await getRoomInfo(nonFedDmResponse.body.room._id, rcUser1.config); - - rcRoomNonFed = roomInfoNonFed.room as IRoomNativeFederated; - - // Create federated DM between rcUser1 and rcUser2 and the Synapse user + // First create a federated DM between rcUser1 and rcUser2 and the Synapse user, so this is the oldest room const fedDmResponse = await rcUser1.config.request .post(api('dm.create')) .set(rcUser1.config.credentials) @@ -1260,6 +1245,21 @@ const waitForRoomEvent = async ( // After acceptance, should display the Synapse user's ID expect(sub).toHaveProperty('fname', `${federationConfig.hs1.adminMatrixUserId}, ${rcUser1.fullName}`); }); + + // Then create non-federated DM between rcUser1 and rcUser2 which should be returned on duplication + const nonFedDmResponse = await rcUser1.config.request + .post(api('dm.create')) + .set(rcUser1.config.credentials) + .send({ + username: rcUser2.username, + }) + .expect(200); + + expect(nonFedDmResponse.body).toHaveProperty('success', true); + + const roomInfoNonFed = await getRoomInfo(nonFedDmResponse.body.room._id, rcUser1.config); + + rcRoomNonFed = roomInfoNonFed.room as IRoomNativeFederated; }); it('should have two DMs with same users', async () => { @@ -1322,9 +1322,6 @@ const waitForRoomEvent = async ( expect(response.body).toHaveProperty('success', true); - // expect(response.body).toHaveProperty('room._id', rcRoom._id); - - // TODO this is currently wrong, the correct value should be rcRoom._id expect(response.body).toHaveProperty('room._id', rcRoomNonFed._id); }); }); @@ -1435,7 +1432,7 @@ const waitForRoomEvent = async ( }); }); - it.failing('should create a federated DM between rcUser1 and rcUser2 and the Synapse user', async () => { + it('should create a federated DM between rcUser1 and rcUser2 and the Synapse user', async () => { const fedDmResponse = await rcUser1.config.request .post(api('dm.create')) .set(rcUser1.config.credentials) @@ -1486,13 +1483,21 @@ const waitForRoomEvent = async ( { delayMs: 100 }, ); - // TODO accept not working const response = await acceptRoomInvite(rcRoom._id, rcUser2.config); expect(response.success).toBe(true); + + await retry( + 'wait for the join to be processed', + async () => { + const sub = await getSubscriptionByRoomId(rcRoom._id, rcUser2.config.credentials, rcUser2.config.request); + + expect(sub).not.toHaveProperty('status'); + }, + { delayMs: 100 }, + ); }); - // TODO this is failing because we cannot accept the invite properly on the line above - it.failing('should have two DMs with same users', async () => { + it('should have a single DM with same users before one leaves', async () => { const roomsResponse = await rcUser1.config.request.get(api('rooms.get')).set(rcUser1.config.credentials).expect(200); const dmRooms = roomsResponse.body.update.filter( @@ -1506,7 +1511,9 @@ const waitForRoomEvent = async ( // at this time there should be only one DM with only two users (the non-federated one) expect(dmRooms.length).toBe(1); + }); + it('should leave the federated DM with three users', async () => { // now the rc user leaves the federated DM const response = await rcUser2.config.request .post(api('rooms.leave')) @@ -1518,19 +1525,21 @@ const waitForRoomEvent = async ( expect(response.body).toHaveProperty('success', true); - await retry( - 'wait for the leave to be processed', - async () => { - expect(hs1Room1.getMyMembership()).toBe('leave'); + // Verify room is no longer accessible to RC users + await retry('waiting for room cleanup', async () => { + const roomsResponse = await rcUser2.config.request.get(api('rooms.get')).set(rcUser2.config.credentials).expect(200); - const sub = await getSubscriptionByRoomId(rcRoom._id, rcUser1.config.credentials, rcUser1.config.request); + expect(roomsResponse.body).toHaveProperty('update'); + expect(roomsResponse.body.update).toBeInstanceOf(Array); - // After leave, should display only the RC user's full name - expect(sub).toHaveProperty('fname', rcUser2.fullName); - }, - { delayMs: 100 }, - ); + const room = roomsResponse.body.update?.find((r: IRoom) => r._id === rcRoom._id); + + // Room should not be in active rooms list + expect(room).toBeUndefined(); + }); + }); + it('should have two DMs with same users', async () => { // now there should be two DMs with the same users const roomsResponseAfterLeave = await rcUser1.config.request .get(api('rooms.get')) @@ -1560,9 +1569,6 @@ const waitForRoomEvent = async ( expect(response.body).toHaveProperty('success', true); - // expect(response.body).toHaveProperty('room._id', rcRoom._id); - - // TODO this is currently wrong, the correct value should be rcRoom._id expect(response.body).toHaveProperty('room._id', rcRoom1on1._id); }); }); diff --git a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts index bfade515738a8..f0b2f169f752d 100644 --- a/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts +++ b/ee/packages/federation-matrix/tests/end-to-end/room.spec.ts @@ -14,6 +14,7 @@ import { } from '../../../../../apps/meteor/tests/data/rooms.helper'; import { type IRequestConfig, getRequestConfig, createUser, deleteUser } from '../../../../../apps/meteor/tests/data/users.helper'; import { IS_EE } from '../../../../../apps/meteor/tests/e2e/config/constants'; +import { retry } from '../../../../../apps/meteor/tests/end-to-end/api/helpers/retry'; import { federationConfig } from '../helper/config'; import { createDDPListener } from '../helper/ddp-listener'; import { SynapseClient } from '../helper/synapse-client'; @@ -1612,13 +1613,20 @@ import { SynapseClient } from '../helper/synapse-client'; describe('It should reflect all the members and messagens on the rocket.chat side', () => { it('It should show all the three users in the members list', async () => { - const members = await getRoomMembers(rid, rc1AdminRequestConfig); - expect(members.members.length).toBe(3); - expect(members.members.find((member: IUser) => member.username === federationConfig.rc1.adminUser)).not.toBeNull(); - expect( - members.members.find((member: IUser) => member.username === federationConfig.rc1.additionalUser1.username), - ).not.toBeNull(); - expect(members.members.find((member: IUser) => member.username === federationConfig.hs1.adminMatrixUserId)).not.toBeNull(); + await retry( + 'Getting room members until all are present', + async () => { + const members = await getRoomMembers(rid, rc1AdminRequestConfig); + + expect(members.members.length).toBe(3); + expect(members.members.find((member: IUser) => member.username === federationConfig.rc1.adminUser)).not.toBeNull(); + expect( + members.members.find((member: IUser) => member.username === federationConfig.rc1.additionalUser1.username), + ).not.toBeNull(); + expect(members.members.find((member: IUser) => member.username === federationConfig.hs1.adminMatrixUserId)).not.toBeNull(); + }, + { delayMs: 200 }, + ); }); }); }); diff --git a/packages/core-services/src/types/IRoomService.ts b/packages/core-services/src/types/IRoomService.ts index 76f909b97d61c..62ccf27bcde6f 100644 --- a/packages/core-services/src/types/IRoomService.ts +++ b/packages/core-services/src/types/IRoomService.ts @@ -69,5 +69,5 @@ export interface IRoomService { status?: 'INVITED'; roles?: ISubscription['roles']; }): Promise; - updateDirectMessageRoomName(room: IRoom): Promise; + updateDirectMessageRoomName(room: IRoom, ignoreStatusFromSubs?: string[]): Promise; } diff --git a/packages/models/src/models/Rooms.ts b/packages/models/src/models/Rooms.ts index 8c94564b01812..81edc91ae703e 100644 --- a/packages/models/src/models/Rooms.ts +++ b/packages/models/src/models/Rooms.ts @@ -897,7 +897,13 @@ export class RoomsRaw extends BaseRaw implements IRoomsModel { uids: { $size: uid.length, $all: uid }, }; - return this.findOne(query, options); + return this.findOne(query, { + ...options, + sort: { + federated: 1, + ts: 1, + }, + }); } findFederatedRooms(options: FindOptions = {}): FindCursor {