fix: API method rooms.delete deleting main team room

[tiagoevanp]

Proposed changes (including videos or screenshots)

What’s Changed

This PR fixes a critical bug where the API method rooms.delete inadvertently deletes the main (primary) room of a team. The change ensures the main team room remains protected and is not removed when this endpoint is invoked. Also

Background

On Rocket.Chat, teams often have a designated “main room” that serves as the central hub. Deleting this primary room via the rooms.delete API was unintended, potentially disrupting team workflows and causing data loss.

Testing

New test case specifically targeting rooms.delete to confirm the main team room remains intact.

Issue(s)

Steps to test or reproduce

Further comments

SUP-836

Summary by CodeRabbit

• Bug Fixes

  • Prevents deletion of a team’s main room via the API to avoid accidental loss of primary team spaces.
  • Improves validation when deleting rooms: invalid or non-existent rooms now return clear errors.
  • Increases reliability of room deletion by standardizing handling whether a room ID or room object is provided.

• Tests

  • Adds end-to-end test to assert main-team rooms cannot be deleted.

• Chores

  • Adds a patch changeset documenting the fix.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is targeting the wrong base branch. It should target 7.12.0, but it targets 7.11.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[changeset-bot]

🦋 Changeset detected

Latest commit: 655b39c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 42 packages

Name	Type
@rocket.chat/meteor	Patch
@rocket.chat/core-typings	Patch
@rocket.chat/rest-typings	Patch
@rocket.chat/uikit-playground	Patch
@rocket.chat/api-client	Patch
@rocket.chat/apps	Patch
@rocket.chat/core-services	Patch
@rocket.chat/cron	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/freeswitch	Patch
@rocket.chat/fuselage-ui-kit	Patch
@rocket.chat/gazzodown	Patch
@rocket.chat/http-router	Patch
@rocket.chat/livechat	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/ui-avatar	Patch
@rocket.chat/ui-client	Patch
@rocket.chat/ui-contexts	Patch
@rocket.chat/web-ui-registration	Patch
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/federation-service	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/stream-hub-service	Patch
@rocket.chat/federation-matrix	Patch
@rocket.chat/license	Patch
@rocket.chat/media-calls	Patch
@rocket.chat/omnichannel-services	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/models	Patch
@rocket.chat/network-broker	Patch
@rocket.chat/omni-core-ee	Patch
@rocket.chat/mock-providers	Patch
@rocket.chat/ui-video-conf	Patch
@rocket.chat/ui-voip	Patch
@rocket.chat/instance-status	Patch
@rocket.chat/omni-core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.58%. Comparing base (6f13304) to head (655b39c).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #36790      +/-   ##
===========================================
- Coverage    67.59%   67.58%   -0.02%     
===========================================
  Files         3341     3341              
  Lines       114018   114016       -2     
  Branches     20668    20669       +1     
===========================================
- Hits         77076    77055      -21     
- Misses       34266    34284      +18     
- Partials      2676     2677       +1     

Flag	Coverage Δ
e2e	57.32% <ø> (-0.02%)	⬇️
unit	71.52% <ø> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pierre-lehnen-rc]

I don't know if this is still the case, but worth mentioning:

Originally the teams system was supposed to allow executing operations like this in either entitity and it should then reflect on the other. So from that initial concept, you should be able to delete a team's main room, but doing so should cause the whole team to be deleted along with it.

[pierre-lehnen-rc]

Please add a test to the teams.delete endpoint as well, with the roomsToRemove param including the team's main room (in-between others). I don't know if this scenario should successfully delete the team or not, but I suspect the behavior may have changed by this PR so an additional test for it is warranted.

[abhinavkrin]

While testing the current behavior, I found that rooms.delete is able to delete a team’s main room, which results in an orphaned team document remaining in the database.

Although the team no longer appears in the sidebar, the orphaned team record still occupies space in the DB and also prevents the creation of a new team with the same name.

The PR prevents deletion of the team since it throws error when the code tries to delete team's main room. We should move the logic introduced in the eraseRoom function by this pr upwards to the api level.

[coderabbitai]

Walkthrough

Adds a pre-check in the rooms.delete API to fetch the room and block deletion if it's a team's main room. Updates eraseRoom to accept either a room id or an IRoom object and operate on the resolved room. Tests extended to cover the new guard and adjust setup/teardown.

Changes

Cohort / File(s)	Summary
API: rooms.delete route apps/meteor/app/api/server/v1/rooms.ts	Fetches room by id before deletion; throws error if not found; blocks deletion for team main rooms with a specific MeteorError; calls eraseRoom with the resolved room object; returns success on allowed deletion.
Server lib: eraseRoom apps/meteor/server/lib/eraseRoom.ts	Changes signature to `eraseRoom(roomOrId: string
E2E tests: rooms API apps/meteor/tests/end-to-end/api/rooms.ts	Updates setup to create two users and a public team and associate them with a channel; adjusts teardown to remove the created team before deleting the channel; adds a test asserting that deleting a team's main room fails.
Changeset .changeset/slimy-apples-complain.md	Adds a patch entry documenting the fix that rooms.delete no longer deletes a team's main room.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant API as rooms.delete (API)
  participant DB as Rooms Store
  participant Guard as teamMain check
  participant Eraser as eraseRoom()
  participant Delete as deleteRoom()

  C->>API: DELETE /api/v1/rooms.delete (roomId)
  API->>DB: findOneById(roomId)
  DB-->>API: room | null
  alt room not found
    API-->>C: error (invalid-room)
  else room found
    API->>Guard: isTeamMain(room)?
    alt is teamMain
      API-->>C: error (cannot-delete-team-main)
    else not teamMain
      API->>Eraser: eraseRoom(room, currentUserId)
      Eraser->>Delete: deleteRoom(room._id)
      Delete-->>Eraser: ok
      Eraser-->>API: done
      API-->>C: success: true
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I hop through code with whiskers bright,
Stopping mains from vanishing at night.
I pass the room, not just its name,
Tests now guard the team’s dear frame.
Thump-thump—safe rooms, and carrots for fame. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "fix: API method rooms.delete deleting main team room" accurately and concisely describes the main change in the changeset. The code modifications clearly implement a protection mechanism to prevent deletion of team main rooms through the rooms.delete API endpoint, with corresponding test coverage added to validate this behavior. The title is specific, clear, and directly reflects the primary objective of the changes.
Linked Issues Check	✅ Passed	The pull request addresses the core requirement from SUP-836 by implementing a guard in the rooms.delete API endpoint that prevents deletion of team main rooms, which directly fulfills objective 3 (ensure delete semantics are clear and consistent by protecting the main room from independent deletion). The implementation also contributes to objectives 1, 2, and 4 by preventing the scenario where team main rooms can be deleted independently, which was causing orphaned team documents in the database. The changes include appropriate test coverage validating that the main team room deletion attempt returns an error, confirming the fix is properly implemented.
Out of Scope Changes Check	✅ Passed	All code changes are directly scoped to addressing the prevention of main team room deletion. The changes to apps/meteor/app/api/server/v1/rooms.ts introduce the guard logic, the eraseRoom function signature update is a necessary supporting change to enable room validation before deletion, the test additions validate the new behavior, and the changeset documents the fix. No unrelated refactoring, cleanup, or new features outside the stated objectives are present in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/api-rooms.delete-team

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 61e9072 and 6cfb378.

📒 Files selected for processing (1)

• .changeset/slimy-apples-complain.md (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Builds matrix rust bindings against alpine
• GitHub Check: CodeQL-Build
• GitHub Check: CodeQL-Build

🔇 Additional comments (1)

.changeset/slimy-apples-complain.md (1)

1-5: Changeset entry is well-formed and accurately documents the fix.

The entry correctly specifies a patch-level bump for @rocket.chat/meteor and provides a clear, concise description of the bug fix aligned with the PR objectives.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[abhinavkrin]

Please add a test to the teams.delete endpoint as well, with the roomsToRemove param including the team's main room (in-between others). I don't know if this scenario should successfully delete the team or not, but I suspect the behavior may have changed by this PR so an additional test for it is warranted.

The current behavior first deletes the rooms listed in roomsToRemove, and then deletes the team’s main room.

If the team’s main room ID is included in roomsToRemove, it gets deleted in the first step, causing an error when the code later attempts to delete the same room (team's main room) again or calls Team.unsetTeamIdOfRooms().

To fix this, I’ll filter out the team’s main room ID from roomsToRemove before performing the deletions. It will eventually be deleted in the second step. I’ll also add appropriate tests to cover this scenario.

The fix related to this should in the pr #36807