From 8596daf01ac84864caa63dd937971e557933d401 Mon Sep 17 00:00:00 2001 From: Tiago Evangelista Pinto Date: Thu, 20 Nov 2025 08:48:24 -0300 Subject: [PATCH] fix: Orphan team when last owner user deleted (#36807) Co-authored-by: Abhinav Kumar <15830206+abhinavkrin@users.noreply.github.com> Co-authored-by: Douglas Fabris <27704687+dougfabris@users.noreply.github.com> --- .changeset/brave-socks-battle.md | 6 + .changeset/stale-sloths-smoke.md | 5 + .../app/api/server/lib/eraseTeam.spec.ts | 272 ++++++++++++++++++ apps/meteor/app/api/server/lib/eraseTeam.ts | 96 +++++++ apps/meteor/app/api/server/v1/teams.ts | 25 +- apps/meteor/app/api/server/v1/users.ts | 4 +- .../app/lib/server/functions/deleteUser.ts | 9 +- .../functions/relinquishRoomOwnerships.ts | 53 +++- apps/meteor/server/services/team/service.ts | 11 +- apps/meteor/tests/end-to-end/api/teams.ts | 158 ++++++++++ .../core-services/src/types/ITeamService.ts | 2 +- 11 files changed, 598 insertions(+), 43 deletions(-) create mode 100644 .changeset/brave-socks-battle.md create mode 100644 .changeset/stale-sloths-smoke.md create mode 100644 apps/meteor/app/api/server/lib/eraseTeam.spec.ts create mode 100644 apps/meteor/app/api/server/lib/eraseTeam.ts diff --git a/.changeset/brave-socks-battle.md b/.changeset/brave-socks-battle.md new file mode 100644 index 00000000000..35e44f812dc --- /dev/null +++ b/.changeset/brave-socks-battle.md @@ -0,0 +1,6 @@ +--- +'@rocket.chat/core-services': minor +'@rocket.chat/meteor': minor +--- + +Adds a `deletedRooms` field to the `users.delete` endpoint response, indicating which rooms were deleted as part of the user deletion process. diff --git a/.changeset/stale-sloths-smoke.md b/.changeset/stale-sloths-smoke.md new file mode 100644 index 00000000000..533f66e9c9c --- /dev/null +++ b/.changeset/stale-sloths-smoke.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": minor +--- + +Fix issue where a team would become orphaned when its last owner was deleted. diff --git a/apps/meteor/app/api/server/lib/eraseTeam.spec.ts b/apps/meteor/app/api/server/lib/eraseTeam.spec.ts new file mode 100644 index 00000000000..041f5b90e2b --- /dev/null +++ b/apps/meteor/app/api/server/lib/eraseTeam.spec.ts @@ -0,0 +1,272 @@ +import { expect } from 'chai'; +import proxyquireRaw from 'proxyquire'; +import * as sinon from 'sinon'; + +const proxyquire = proxyquireRaw.noCallThru(); + +type Stubbed = { [k: string]: any }; + +describe('eraseTeam (TypeScript) module', () => { + let sandbox: sinon.SinonSandbox; + let stubs: Stubbed; + let subject: any; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + + stubs = { + 'Team': { + getMatchingTeamRooms: sandbox.stub().resolves([]), + unsetTeamIdOfRooms: sandbox.stub().resolves(), + removeAllMembersFromTeam: sandbox.stub().resolves(), + deleteById: sandbox.stub().resolves(), + }, + 'Users': { + findOneById: sandbox.stub().resolves(null), + }, + 'Rooms': { + findOneById: sandbox.stub().resolves(null), + }, + 'eraseRoomStub': sandbox.stub().resolves(true), + 'deleteRoomStub': sandbox.stub().resolves(), + '../../../../server/lib/logger/system': { + SystemLogger: { + error: sandbox.stub(), + }, + }, + '@rocket.chat/apps': { + AppEvents: { + IPreRoomDeletePrevent: 'IPreRoomDeletePrevent', + IPostRoomDeleted: 'IPostRoomDeleted', + }, + Apps: { + self: { isLoaded: () => false }, + getBridges: () => ({ + getListenerBridge: () => ({ + roomEvent: sandbox.stub().resolves(false), + }), + }), + }, + }, + '@rocket.chat/models': { + Rooms: { + findOneById: (...args: any[]) => stubs.Rooms.findOneById(...args), + }, + Users: { + findOneById: (...args: any[]) => stubs.Users.findOneById(...args), + }, + }, + '@rocket.chat/core-services': { + MeteorError: (function () { + class MeteorError extends Error { + public error: string | undefined; + + public details: any; + + constructor(message?: string, error?: string, details?: any) { + super(message); + this.error = error; + this.details = details; + } + } + return MeteorError; + })(), + }, + }; + + subject = proxyquire('./eraseTeam', { + '@rocket.chat/apps': stubs['@rocket.chat/apps'], + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + }); + + afterEach(() => { + sandbox.restore(); + }); + + describe('eraseTeamShared', () => { + it('throws when user is undefined', async () => { + // eslint-disable-next-line @typescript-eslint/no-empty-function + await expect(subject.eraseTeamShared(undefined, { _id: 'team1', roomId: 'teamRoom' }, [], () => {})).to.be.rejected; + }); + + it('erases provided rooms (excluding team.roomId) and cleans up team', async () => { + const team = { _id: 'team-id', roomId: 'team-room' }; + const user = { _id: 'user-1', username: 'u' }; + stubs.Team.getMatchingTeamRooms.resolves(['room-1', 'room-2', team.roomId]); + + const erased: Array<{ rid: string; user: any }> = []; + const eraseRoomFn = async (rid: string, user: any) => { + erased.push({ rid, user }); + }; + + await subject.eraseTeamShared(user, team, ['room-1', 'room-2', team.roomId], eraseRoomFn); + + expect(erased.some((r) => r.rid === 'room-1')).to.be.true; + expect(erased.some((r) => r.rid === 'room-2')).to.be.true; + sinon.assert.calledOnce(stubs.Team.unsetTeamIdOfRooms); + expect(erased.some((r) => r.rid === team.roomId)).to.be.true; + sinon.assert.calledOnce(stubs.Team.removeAllMembersFromTeam); + sinon.assert.calledOnce(stubs.Team.deleteById); + }); + }); + + describe('eraseTeam', () => { + it('calls eraseRoom for the team main room (via eraseTeamShared)', async () => { + const team = { _id: 't1', roomId: 't-room' }; + const user = { _id: 'u1', username: 'u', name: 'User' }; + stubs.Team.getMatchingTeamRooms.resolves([]); + const { eraseRoomStub } = stubs; + eraseRoomStub.resolves(true); + + await subject.eraseTeam(user, team, []); + + sinon.assert.calledWith(eraseRoomStub, team.roomId, 'u1'); + }); + }); + + describe('eraseTeamOnRelinquishRoomOwnerships', () => { + it('returns successfully deleted room ids only', async () => { + const team = { _id: 't1', roomId: 't-room' }; + stubs.Team.getMatchingTeamRooms.resolves(['r1', 'r2']); + + stubs.Rooms.findOneById.withArgs('r1').resolves({ _id: 'r1', federated: false }); + stubs.Rooms.findOneById.withArgs('r2').resolves(null); + + stubs.deleteRoomStub.withArgs('r1').resolves(); + stubs.deleteRoomStub.withArgs('r2').rejects(new Error('boom')); + + const base = proxyquire('./eraseTeam', { + '@rocket.chat/apps': stubs['@rocket.chat/apps'], + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + + const result: string[] = await base.eraseTeamOnRelinquishRoomOwnerships(team, ['r1', 'r2']); + expect(result).to.be.an('array').that.includes('r1').and.not.includes('r2'); + }); + }); + + describe('eraseRoomLooseValidation', () => { + let baseModule: any; + + beforeEach(() => { + baseModule = proxyquire('./eraseTeam', { + '@rocket.chat/apps': stubs['@rocket.chat/apps'], + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + }); + + it('returns false when room not found', async () => { + stubs.Rooms.findOneById.resolves(null); + const res = await baseModule.eraseRoomLooseValidation('does-not-exist'); + expect(res).to.be.false; + }); + + it('returns false when room.federated is true', async () => { + stubs.Rooms.findOneById.resolves({ _id: 'r', federated: true }); + const res = await baseModule.eraseRoomLooseValidation('r'); + expect(res).to.be.false; + }); + + it('returns false when app pre-delete prevents deletion', async () => { + const listenerStub = sandbox.stub().resolves(true); + const AppsStub = { + AppEvents: stubs['@rocket.chat/apps'].AppEvents, + Apps: { + self: { isLoaded: () => true }, + getBridges: () => ({ getListenerBridge: () => ({ roomEvent: listenerStub }) }), + }, + }; + + const m = proxyquire('./eraseTeam', { + '@rocket.chat/apps': AppsStub, + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + + stubs.Rooms.findOneById.resolves({ _id: 'r', federated: false }); + + const res = await m.eraseRoomLooseValidation('r'); + expect(listenerStub.calledOnce).to.be.true; + expect(res).to.be.false; + }); + + it('logs and returns false when deleteRoom throws', async () => { + stubs.Rooms.findOneById.resolves({ _id: 'r', federated: false }); + stubs.deleteRoomStub.rejects(new Error('boom')); + + const m = proxyquire('./eraseTeam', { + '@rocket.chat/apps': stubs['@rocket.chat/apps'], + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + + const res = await m.eraseRoomLooseValidation('r'); + expect(res).to.be.false; + sinon.assert.calledOnce(stubs['../../../../server/lib/logger/system'].SystemLogger.error); + }); + + it('calls post-deleted event and returns true on success', async () => { + const roomEventStub = sandbox.stub().onFirstCall().resolves(false).onSecondCall().resolves(); + const AppsStub = { + AppEvents: stubs['@rocket.chat/apps'].AppEvents, + Apps: { + self: { isLoaded: () => true }, + getBridges: () => ({ getListenerBridge: () => ({ roomEvent: roomEventStub }) }), + }, + }; + + stubs.deleteRoomStub.resolves(); + const m = proxyquire('./eraseTeam', { + '@rocket.chat/apps': AppsStub, + '@rocket.chat/models': stubs['@rocket.chat/models'], + '../../../../server/lib/eraseRoom': { __esModule: true, eraseRoom: stubs.eraseRoomStub }, + '../../../lib/server/functions/deleteRoom': { __esModule: true, deleteRoom: stubs.deleteRoomStub }, + '../../../../server/lib/logger/system': stubs['../../../../server/lib/logger/system'], + '@rocket.chat/core-services': { + MeteorError: stubs['@rocket.chat/core-services'].MeteorError, + Team: stubs.Team, + }, + }); + + stubs.Rooms.findOneById.resolves({ _id: 'r', federated: false }); + + const res = await m.eraseRoomLooseValidation('r'); + expect(res).to.be.true; + sinon.assert.calledTwice(roomEventStub); + }); + }); +}); diff --git a/apps/meteor/app/api/server/lib/eraseTeam.ts b/apps/meteor/app/api/server/lib/eraseTeam.ts new file mode 100644 index 00000000000..65b289ab998 --- /dev/null +++ b/apps/meteor/app/api/server/lib/eraseTeam.ts @@ -0,0 +1,96 @@ +import { AppEvents, Apps } from '@rocket.chat/apps'; +import { MeteorError, Team } from '@rocket.chat/core-services'; +import type { AtLeast, IRoom, ITeam, IUser } from '@rocket.chat/core-typings'; +import { Rooms } from '@rocket.chat/models'; + +import { eraseRoom } from '../../../../server/lib/eraseRoom'; +import { SystemLogger } from '../../../../server/lib/logger/system'; +import { deleteRoom } from '../../../lib/server/functions/deleteRoom'; + +type eraseRoomFnType = (rid: string, user: AtLeast) => Promise; + +export const eraseTeamShared = async ( + user: AtLeast, + team: ITeam, + roomsToRemove: IRoom['_id'][] = [], + eraseRoomFn: eraseRoomFnType, +) => { + const rooms: string[] = roomsToRemove.length + ? (await Team.getMatchingTeamRooms(team._id, roomsToRemove)).filter((roomId) => roomId !== team.roomId) + : []; + + if (!user) { + throw new MeteorError('Invalid user provided for erasing team', 'error-invalid-user', { + method: 'eraseTeamShared', + }); + } + + // If we got a list of rooms to delete along with the team, remove them first + await Promise.all(rooms.map((room) => eraseRoomFn(room, user))); + + // Move every other room back to the workspace + await Team.unsetTeamIdOfRooms(user, team); + + // Remove the team's main room + await eraseRoomFn(team.roomId, user); + + // Delete all team memberships + await Team.removeAllMembersFromTeam(team._id); + + // And finally delete the team itself + await Team.deleteById(team._id); +}; + +export const eraseTeam = async (user: AtLeast, team: ITeam, roomsToRemove: IRoom['_id'][]) => { + await eraseTeamShared(user, team, roomsToRemove, async (rid, user) => { + return eraseRoom(rid, user._id); + }); +}; + +/** + * @param team + * @param roomsToRemove + * @returns deleted room ids + */ +export const eraseTeamOnRelinquishRoomOwnerships = async (team: ITeam, roomsToRemove: IRoom['_id'][] = []): Promise => { + const deletedRooms = new Set(); + await eraseTeamShared({ _id: 'rocket.cat', username: 'rocket.cat', name: 'Rocket.Cat' }, team, roomsToRemove, async (rid) => { + const isDeleted = await eraseRoomLooseValidation(rid); + if (isDeleted) { + deletedRooms.add(rid); + } + }); + return Array.from(deletedRooms); +}; + +export async function eraseRoomLooseValidation(rid: string): Promise { + const room = await Rooms.findOneById(rid); + + if (!room) { + return false; + } + + if (room.federated) { + return false; + } + + if (Apps.self?.isLoaded()) { + const prevent = await Apps.getBridges()?.getListenerBridge().roomEvent(AppEvents.IPreRoomDeletePrevent, room); + if (prevent) { + return false; + } + } + + try { + await deleteRoom(rid); + } catch (e) { + SystemLogger.error(e); + return false; + } + + if (Apps.self?.isLoaded()) { + void Apps.getBridges()?.getListenerBridge().roomEvent(AppEvents.IPostRoomDeleted, room); + } + + return true; +} diff --git a/apps/meteor/app/api/server/v1/teams.ts b/apps/meteor/app/api/server/v1/teams.ts index ee5ec7e273f..9ce577522a2 100644 --- a/apps/meteor/app/api/server/v1/teams.ts +++ b/apps/meteor/app/api/server/v1/teams.ts @@ -22,6 +22,7 @@ import { hasPermissionAsync, hasAtLeastOnePermissionAsync } from '../../../autho import { removeUserFromRoom } from '../../../lib/server/functions/removeUserFromRoom'; import { API } from '../api'; import { getPaginationItems } from '../helpers/getPaginationItems'; +import { eraseTeam } from '../lib/eraseTeam'; API.v1.addRoute( 'teams.list', @@ -135,7 +136,7 @@ API.v1.addRoute( } } - await Promise.all([Team.unsetTeamIdOfRooms(this.userId, team._id), Team.removeAllMembersFromTeam(team._id)]); + await Promise.all([Team.unsetTeamIdOfRooms(this.user, team), Team.removeAllMembersFromTeam(team._id)]); await Team.deleteById(team._id); @@ -634,6 +635,7 @@ API.v1.addRoute( const { roomsToRemove = [] } = this.bodyParams; const team = await getTeamByIdOrName(this.bodyParams); + if (!team) { return API.v1.failure('team-does-not-exist'); } @@ -642,26 +644,7 @@ API.v1.addRoute( return API.v1.forbidden(); } - const rooms: string[] = await Team.getMatchingTeamRooms(team._id, roomsToRemove); - - // If we got a list of rooms to delete along with the team, remove them first - if (rooms.length) { - for await (const room of rooms) { - await eraseRoom(room, this.userId); - } - } - - // Move every other room back to the workspace - await Team.unsetTeamIdOfRooms(this.userId, team._id); - - // Remove the team's main room - await eraseRoom(team.roomId, this.userId); - - // Delete all team memberships - await Team.removeAllMembersFromTeam(team._id); - - // And finally delete the team itself - await Team.deleteById(team._id); + await eraseTeam(this.user, team, roomsToRemove); return API.v1.success(); }, diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index 048b289b3fc..d4f0012cd57 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -361,9 +361,9 @@ API.v1.addRoute( const user = await getUserFromParams(this.bodyParams); const { confirmRelinquish = false } = this.bodyParams; - await deleteUser(user._id, confirmRelinquish, this.userId); + const { deletedRooms } = await deleteUser(user._id, confirmRelinquish, this.userId); - return API.v1.success(); + return API.v1.success({ deletedRooms }); }, }, ); diff --git a/apps/meteor/app/lib/server/functions/deleteUser.ts b/apps/meteor/app/lib/server/functions/deleteUser.ts index a80ba716d4e..925dffb84ef 100644 --- a/apps/meteor/app/lib/server/functions/deleteUser.ts +++ b/apps/meteor/app/lib/server/functions/deleteUser.ts @@ -31,7 +31,7 @@ import { notifyOnUserChange, } from '../lib/notifyListener'; -export async function deleteUser(userId: string, confirmRelinquish = false, deletedBy?: IUser['_id']): Promise { +export async function deleteUser(userId: string, confirmRelinquish = false, deletedBy?: IUser['_id']): Promise<{ deletedRooms: string[] }> { if (userId === 'rocket.cat') { throw new Meteor.Error('error-action-not-allowed', 'Deleting the rocket.cat user is not allowed', { method: 'deleteUser', @@ -44,7 +44,7 @@ export async function deleteUser(userId: string, confirmRelinquish = false, dele }); if (!user) { - return; + return { deletedRooms: [] }; } if (isUserFederated(user)) { @@ -60,11 +60,12 @@ export async function deleteUser(userId: string, confirmRelinquish = false, dele throw new Meteor.Error('user-last-owner', '', rooms); } + let deletedRooms: string[] = []; // Users without username can't do anything, so there is nothing to remove if (user.username != null) { let userToReplaceWhenUnlinking: IUser | null = null; const nameAlias = i18n.t('Removed_User'); - await relinquishRoomOwnerships(userId, subscribedRooms); + deletedRooms = await relinquishRoomOwnerships(userId, subscribedRooms, true); const messageErasureType = settings.get<'Delete' | 'Unlink' | 'Keep'>('Message_ErasureType'); switch (messageErasureType) { @@ -176,4 +177,6 @@ export async function deleteUser(userId: string, confirmRelinquish = false, dele void notifyOnUserChange({ clientAction: 'removed', id: user._id }); await callbacks.run('afterDeleteUser', user); + + return { deletedRooms }; } diff --git a/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts b/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts index a62f893319a..a22556fb88f 100644 --- a/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts +++ b/apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts @@ -1,15 +1,42 @@ -import { Messages, Rooms, Subscriptions, ReadReceipts } from '@rocket.chat/models'; +import type { IRoom } from '@rocket.chat/core-typings'; +import { Messages, Rooms, Subscriptions, ReadReceipts, Team } from '@rocket.chat/models'; import type { SubscribedRoomsForUserWithDetails } from './getRoomsWithSingleOwner'; import { addUserRolesAsync } from '../../../../server/lib/roles/addUserRoles'; +import { eraseRoomLooseValidation, eraseTeamOnRelinquishRoomOwnerships } from '../../../api/server/lib/eraseTeam'; import { FileUpload } from '../../../file-upload/server'; import { notifyOnSubscriptionChanged } from '../lib/notifyListener'; -const bulkRoomCleanUp = async (rids: string[]): Promise => { +const bulkTeamCleanup = async (rids: IRoom['_id'][]) => { + const rooms = (await Rooms.findByIds(rids, { projection: { teamId: 1, teamMain: 1 } }).toArray()) as Pick< + IRoom, + '_id' | 'teamId' | 'teamMain' + >[]; + + const teamsToRemove = rooms.filter((room) => room.teamMain); + const teamIds = teamsToRemove.map((room) => room.teamId).filter((teamId) => teamId !== undefined); + const uniqueTeamIds = [...new Set(teamIds)]; + + const deletedRoomIds: string[] = []; + await Promise.all( + uniqueTeamIds.map(async (teamId) => { + const team = await Team.findOneById(teamId); + if (!team) { + return; + } + + const ids = await eraseTeamOnRelinquishRoomOwnerships(team, []); + ids.forEach((id) => deletedRoomIds.push(id)); + }), + ); + return deletedRoomIds; +}; + +const bulkRoomCleanUp = async (rids: string[]) => { // no bulk deletion for files await Promise.all(rids.map((rid) => FileUpload.removeFilesByRoomId(rid))); - return Promise.all([ + const [, , , deletedRoomIds] = await Promise.all([ Subscriptions.removeByRoomIds(rids, { async onTrash(doc) { void notifyOnSubscriptionChanged(doc, 'removed'); @@ -17,15 +44,27 @@ const bulkRoomCleanUp = async (rids: string[]): Promise => { }), Messages.removeByRoomIds(rids), ReadReceipts.removeByRoomIds(rids), - Rooms.removeByIds(rids), + bulkTeamCleanup(rids), ]); + + const restRidsToRemove = rids.filter((rid) => !deletedRoomIds.includes(rid)); + await Promise.all( + restRidsToRemove.map(async (rid) => { + const isDeleted = await eraseRoomLooseValidation(rid); + if (isDeleted) { + deletedRoomIds.push(rid); + } + }), + ); + + return deletedRoomIds; }; export const relinquishRoomOwnerships = async function ( userId: string, subscribedRooms: SubscribedRoomsForUserWithDetails[], removeDirectMessages = true, -): Promise { +) { // change owners const changeOwner = subscribedRooms.filter(({ shouldChangeOwner }) => shouldChangeOwner); @@ -41,7 +80,5 @@ export const relinquishRoomOwnerships = async function ( ); } - await bulkRoomCleanUp(roomIdsToRemove); - - return subscribedRooms; + return bulkRoomCleanUp(roomIdsToRemove); }; diff --git a/apps/meteor/server/services/team/service.ts b/apps/meteor/server/services/team/service.ts index cde46a95ce8..87c8ce18af4 100644 --- a/apps/meteor/server/services/team/service.ts +++ b/apps/meteor/server/services/team/service.ts @@ -416,29 +416,24 @@ export class TeamService extends ServiceClassInternal implements ITeamService { }; } - async unsetTeamIdOfRooms(uid: string, teamId: string): Promise { - if (!teamId) { - throw new Error('missing-teamId'); - } - - const team = await Team.findOneById>(teamId, { projection: { roomId: 1 } }); + async unsetTeamIdOfRooms(user: AtLeast, team: AtLeast): Promise { if (!team) { throw new Error('invalid-team'); } const room = await Rooms.findOneById>(team.roomId, { projection: { name: 1 } }); + if (!room) { throw new Error('invalid-room'); } - const user = await Users.findOneById>(uid, { projection: { username: 1, name: 1 } }); if (!user) { throw new Error('invalid-user'); } await Message.saveSystemMessage('user-converted-to-channel', team.roomId, room.name || '', user); - await Rooms.unsetTeamId(teamId); + await Rooms.unsetTeamId(team._id); } async updateRoom(uid: string, rid: string, isDefault: boolean, canUpdateAnyRoom = false): Promise { diff --git a/apps/meteor/tests/end-to-end/api/teams.ts b/apps/meteor/tests/end-to-end/api/teams.ts index 8623c97c7f8..195606d3099 100644 --- a/apps/meteor/tests/end-to-end/api/teams.ts +++ b/apps/meteor/tests/end-to-end/api/teams.ts @@ -1,6 +1,7 @@ import type { Credentials } from '@rocket.chat/api-client'; import type { IRole, IRoom, ITeam, IUser } from '@rocket.chat/core-typings'; import { TEAM_TYPE } from '@rocket.chat/core-typings'; +import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, afterEach, before, beforeEach, describe, it } from 'mocha'; @@ -1481,6 +1482,163 @@ describe('/teams.delete', () => { .catch(done); }); }); + + describe("delete team when team's main room id is provided in roomsToRemove", () => { + const tempTeamName = `temporaryTeam-${Random.id()}`; + const channel1Name = `${tempTeamName}-channel1`; + const channel2Name = `${tempTeamName}-channel2`; + let teamId: ITeam['_id']; + let channel1Id: IRoom['_id']; + let channel2Id: IRoom['_id']; + let teamMainRoomId: IRoom['_id']; + + before('create team', async () => { + await request + .post(api('teams.create')) + .set(credentials) + .send({ + name: tempTeamName, + type: 0, + }) + .then((response) => { + teamId = response.body.team._id; + teamMainRoomId = response.body.team.roomId; + }); + }); + + before('create channel 1', async () => { + await request + .post(api('channels.create')) + .set(credentials) + .send({ + name: channel1Name, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + channel1Id = res.body.channel._id; + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('channel._id'); + expect(res.body).to.have.nested.property('channel.name', channel1Name); + expect(res.body).to.have.nested.property('channel.t', 'c'); + expect(res.body).to.have.nested.property('channel.msgs', 0); + }); + }); + + before('add channel 1 to team', async () => { + await request + .post(api('teams.addRooms')) + .set(credentials) + .send({ + rooms: [channel1Id], + teamId, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('rooms'); + expect(res.body.rooms[0]).to.have.property('teamId', teamId); + expect(res.body.rooms[0]).to.not.have.property('teamDefault'); + }); + }); + + before('create channel 2', async () => { + await request + .post(api('channels.create')) + .set(credentials) + .send({ + name: channel2Name, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + channel2Id = res.body.channel._id; + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.nested.property('channel._id'); + expect(res.body).to.have.nested.property('channel.name', channel2Name); + expect(res.body).to.have.nested.property('channel.t', 'c'); + expect(res.body).to.have.nested.property('channel.msgs', 0); + }); + }); + + before('add channel 2 to team', async () => { + await request + .post(api('teams.addRooms')) + .set(credentials) + .send({ + rooms: [channel2Id], + teamId, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('rooms'); + expect(res.body.rooms[0]).to.have.property('teamId', teamId); + expect(res.body.rooms[0]).to.not.have.property('teamDefault'); + }); + }); + + after(() => deleteRoom({ type: 'c', roomId: channel1Id })); + + it('should delete the specified room and move the other back to the workspace', async () => { + await request + .post(api('teams.delete')) + .set(credentials) + .send({ + teamName: tempTeamName, + roomsToRemove: [channel2Id, teamMainRoomId], + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }); + + await request + .get(api('channels.info')) + .set(credentials) + .query({ + roomId: teamMainRoomId, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((response) => { + expect(response.body).to.have.property('success', false); + expect(response.body).to.have.property('error'); + expect(response.body.error).to.include('[error-room-not-found]'); + }); + await request + .get(api('channels.info')) + .set(credentials) + .query({ + roomId: channel2Id, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((response) => { + expect(response.body).to.have.property('success', false); + expect(response.body).to.have.property('error'); + expect(response.body.error).to.include('[error-room-not-found]'); + }); + + await request + .get(api('channels.info')) + .set(credentials) + .query({ + roomId: channel1Id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((response) => { + expect(response.body).to.have.property('success', true); + expect(response.body).to.have.property('channel'); + expect(response.body.channel).to.have.property('_id', channel1Id); + expect(response.body.channel).to.not.have.property('teamId'); + }); + }); + }); }); describe('/teams.addRooms', () => { diff --git a/packages/core-services/src/types/ITeamService.ts b/packages/core-services/src/types/ITeamService.ts index e3f77b3c7d2..7b357c26657 100644 --- a/packages/core-services/src/types/ITeamService.ts +++ b/packages/core-services/src/types/ITeamService.ts @@ -106,7 +106,7 @@ export interface ITeamService { getInfoById(teamId: string): Promise | null>; deleteById(teamId: string): Promise; deleteByName(teamName: string): Promise; - unsetTeamIdOfRooms(uid: string, teamId: string): void; + unsetTeamIdOfRooms(user: AtLeast, team: AtLeast): Promise; getOneById(teamId: string, options?: FindOptions): Promise; getOneById

(teamId: string, options?: FindOptions

): Promise; getOneByName(teamName: string | RegExp, options?: FindOptions): Promise;