Skip to content

Conversation

@NAME-ASHWANIYADAV
Copy link
Contributor

@NAME-ASHWANIYADAV NAME-ASHWANIYADAV commented Nov 1, 2025

Closes #36885

Proposed changes

This pull request fixes the "ghost last message" bug (#36885).

Instead of just unsetting the lastMessage field, this fix implements the more robust solution suggested by the mentor.

When a user is deleted, this code now:

  1. Finds all rooms where the deleted user sent the last message.
  2. For each of those rooms, it searches for the new latest valid message.
  3. If a new last message is found, it updates the rocketchat_room.lastMessage field to this new message.
  4. If no new message is found (i.e., the room is now empty), it completely removes ($unset) the lastMessage field.

This ensures that room previews always show the correct, most-recent valid message, or nothing at all, preventing any ghost messages.

Steps to test or reproduce

  1. Log in as User A and post a message in #general (e.g., "This is an old message").
  2. Log in as User B and post a new message in #general (e.g., "This message will be a ghost").
  3. Log in as an Admin and delete User B.
  4. Expected behavior: The chat preview for #general should now correctly show User A's message ("This is an old message") as the last message, not the deleted ghost message.

Summary by CodeRabbit

  • Bug Fixes

    • Rooms referencing a deleted user's message now recompute and update to the next valid last message or clear it when none remain.
    • Room-change notifications now include all rooms affected by deletions and unlink flows so clients stay in sync.
  • Chores

    • Added a patch release entry documenting the fix.
    • Added a sparse index to improve lookup for rooms that reference message authors.

✏️ Tip: You can customize this high-level summary in your review settings.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 1, 2025

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

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 1, 2025

🦋 Changeset detected

Latest commit: cb91ea6

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

This PR includes changesets to release 40 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/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/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac 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/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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 1, 2025

Walkthrough

Aggregates rooms whose lastMessage pointed to messages removed during user deletion, computes each room's new latest message (or unsets lastMessage), updates those rooms, and merges affected room IDs into the post-deletion notify/cleanup flow.

Changes

Cohort / File(s) Summary
Delete user flow
apps/meteor/app/lib/server/functions/deleteUser.ts
Track deleted message IDs, find rooms whose lastMessage._id referenced deleted messages, compute per-room replacement lastMessage (or unset it), update rooms, and merge affected room IDs with original room IDs for downstream notifyOnRoomChangedById and cleanup. Remove extra boolean arg from relinquishRoomOwnerships call.
Room model index
packages/models/src/models/Rooms.ts
Add sparse index { 'lastMessage.u._id': 1 } to RoomsRaw.modelIndexes() to optimize lookups by last-message author.
Changeset (release note)
.changeset/eighty-pumas-float.md
Add patch changeset documenting the fix that updates or clears lastMessage when a user and their messages are deleted.

Sequence Diagram(s)

sequenceDiagram
  participant Admin as Admin/API
  participant Server as deleteUser()
  participant Messages as rocketchat_message
  participant Rooms as rocketchat_room
  participant Notifs as Notifications/Cleanup

  Admin->>Server: request delete user
  Server->>Messages: delete messages by userId
  Messages-->>Server: deleted message IDs
  Server->>Rooms: find rooms where lastMessage._id in deleted IDs
  Rooms-->>Server: affectedRoomIds
  alt room has remaining messages
    Server->>Messages: find latest message in room (sort ts desc, limit 1)
    Messages-->>Server: latestMessage
    Server->>Rooms: update room.lastMessage = latestMessage
  else no remaining messages
    Server->>Rooms: unset room.lastMessage
  end
  Server->>Server: mergedRoomIds = originalRoomIds ∪ affectedRoomIds
  Server->>Notifs: notifyOnRoomChangedById(mergedRoomIds) and run cleanup tasks
  Notifs-->>Server: cleanup complete
  Server->>Admin: deletion completed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • ggazzo
  • tassoevan
  • pierre-lehnen-rc

Poem

🐇 I hopped through rooms where ghost messages lay,
I nudged the last word back where it should stay,
Cleared crumbs of deleted notes with a twitch of my nose,
Now channels show truth where the old phantom rose.
🥕 Hooray — no more spectral prose!

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main change: updating lastMessage to a previous valid message when a user is deleted, which directly addresses the ghost message bug.
Linked Issues check ✅ Passed The PR implementation fully addresses the linked issue #36885 by finding affected rooms, updating lastMessage to the latest valid message, or unsetting it when no messages remain.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the ghost lastMessage bug: the main deletion logic in deleteUser.ts, a database index for query optimization, and the changeset entry.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (1)

71-104: Wrap the 'Delete' case in a block to fix variable scoping.

The Biome static analysis warning is valid: variables declared with const/let inside switch cases are accessible in all other cases unless wrapped in braces. This creates confusing scope and potential bugs where variables like store, cursor (lines 72-73), and roomsToUpdate (line 84) are technically accessible in the Unlink case below.

Apply this diff to wrap the entire case in a block:

 		const messageErasureType = settings.get<'Delete' | 'Unlink' | 'Keep'>('Message_ErasureType');
 		switch (messageErasureType) {
-			case 'Delete':
+			case 'Delete': {
 				const store = FileUpload.getStore('Uploads');
 				const cursor = Messages.findFilesByUserId(userId);
 
 				for await (const { file } of cursor) {
 					if (!file) {
 						continue;
 					}
 					await store.deleteById(file._id);
 				}
 
 				await Messages.removeByUserId(userId);
 
 				const roomsToUpdate = await Rooms.find({ 'lastMessage.u._id': userId }, { projection: { _id: 1 } }).toArray();
 				for await (const room of roomsToUpdate) {
 					const [newLastMessage] = await Messages.find({ rid: room._id }, { sort: { ts: -1 }, limit: 1 }).toArray();
 
 					if (newLastMessage) {
 						await Rooms.updateOne({ _id: room._id }, { $set: { lastMessage: newLastMessage } });
 					} else {
 						await Rooms.updateOne({ _id: room._id }, { $unset: { lastMessage: 1 } });
 					}
 				}
 
 				await ReadReceipts.removeByUserId(userId);
 
 				await ModerationReports.hideMessageReportsByUserId(
 					userId,
 					deletedBy || userId,
 					deletedBy === userId ? 'user deleted own account' : 'user account deleted',
 					'DELETE_USER',
 				);
 
 				break;
+			}
 			case 'Unlink':
🧹 Nitpick comments (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (1)

84-93: Consider performance optimization for large-scale deployments.

The core logic correctly fixes the ghost message bug by updating or unsetting lastMessage for affected rooms. However, the current implementation processes rooms sequentially, which could be slow for users who participated in many rooms.

Consider these optimizations:

  1. Use cursor iteration instead of loading all room IDs into memory:
const roomsCursor = Rooms.find({ 'lastMessage.u._id': userId }, { projection: { _id: 1 } });
for await (const room of roomsCursor) {
  // ... existing logic
}
  1. Batch updates using bulkWrite instead of individual updateOne calls:
const bulkOps = [];
const roomsCursor = Rooms.find({ 'lastMessage.u._id': userId }, { projection: { _id: 1 } });

for await (const room of roomsCursor) {
  const [newLastMessage] = await Messages.find({ rid: room._id }, { sort: { ts: -1 }, limit: 1 }).toArray();
  
  if (newLastMessage) {
    bulkOps.push({
      updateOne: {
        filter: { _id: room._id },
        update: { $set: { lastMessage: newLastMessage } }
      }
    });
  } else {
    bulkOps.push({
      updateOne: {
        filter: { _id: room._id },
        update: { $unset: { lastMessage: 1 } }
      }
    });
  }
}

if (bulkOps.length > 0) {
  await Rooms.bulkWrite(bulkOps);
}

These changes would reduce memory usage and improve performance when deleting users who participated in many rooms.

📜 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 9a18288 and cc97f4e.

📒 Files selected for processing (1)
  • apps/meteor/app/lib/server/functions/deleteUser.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🪛 Biome (2.1.2)
apps/meteor/app/lib/server/functions/deleteUser.ts

[error] 84-84: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
apps/meteor/app/lib/server/functions/deleteUser.ts (2)

84-93: Consider optimizing the room update queries for better performance.

The current implementation uses a sequential N+1 query pattern: one query to find affected rooms, then one query per room to find its latest message. For users active in many rooms, this could be slow.

If performance becomes a concern, consider using aggregation to compute all new lastMessages in a single pipeline, then bulk-update the rooms. However, given that user deletion is a rare administrative operation, the current straightforward approach is acceptable for most use cases.


104-104: Fix indentation for consistency.

The closing brace appears to have inconsistent indentation compared to the rest of the case block.

Apply this diff to align the indentation:

-				}
+			}
📜 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 cc97f4e and 617ee01.

📒 Files selected for processing (2)
  • .changeset/eighty-pumas-float.md (1 hunks)
  • apps/meteor/app/lib/server/functions/deleteUser.ts (3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • .changeset/eighty-pumas-float.md
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • .changeset/eighty-pumas-float.md
  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🔇 Additional comments (1)
.changeset/eighty-pumas-float.md (1)

1-5: LGTM!

The changeset follows the standard format and accurately describes the bug fix for handling lastMessage updates during user deletion.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 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 617ee01 and ce2683c.

📒 Files selected for processing (2)
  • .changeset/eighty-pumas-float.md (1 hunks)
  • apps/meteor/app/lib/server/functions/deleteUser.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .changeset/eighty-pumas-float.md
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (1)
apps/meteor/app/lib/server/lib/notifyListener.ts (1)
  • notifyOnRoomChangedById (70-78)
🔇 Additional comments (2)
apps/meteor/app/lib/server/functions/deleteUser.ts (2)

84-94: Good implementation of lastMessage cleanup logic.

The logic correctly addresses the PR objectives:

  1. Identifies rooms where the deleted user was the last message sender
  2. Computes the new latest message for each affected room
  3. Updates or unsets lastMessage appropriately

The query pattern (sort by timestamp descending, limit 1) efficiently retrieves the most recent remaining message. While this executes in a loop, it's acceptable for the infrequent user deletion operation.

Note: Once the scope issue at line 85 is fixed, this implementation will work as intended.


119-121: Correctly addresses past review but blocked by scope issue.

This implementation properly fixes the notification gap identified in the previous review by merging affectedRoomIds with the subscribed room IDs and using Set for deduplication. This ensures all affected rooms (both subscribed and non-subscribed) receive real-time lastMessage update notifications.

However, the critical scope issue at lines 85 and 120 must be resolved first for this notification logic to function correctly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @apps/meteor/app/lib/server/functions/deleteUser.ts:
- Around line 87-99: The Rooms query uses the field lastMessage.u._id and needs
an index to avoid full collection scans; open the RoomsRaw.modelIndexes()
function (in the Rooms model) and add an index definition for {
'lastMessage.u._id': 1 } with sparse: true so the database can efficiently
satisfy queries filtering by lastMessage.u._id; ensure the new index entry
follows the existing modelIndexes structure and includes the sparse option.
🧹 Nitpick comments (3)
apps/meteor/app/lib/server/functions/deleteUser.ts (3)

64-64: Remove non-English comment.

The comment is in Hindi. Per coding guidelines, code comments should be avoided, and when necessary, must be in English for consistency in an international codebase.

🧹 Proposed fix
-    const affectedRoomIds: string[] = []; // Yahan declare kar diya
+    const affectedRoomIds: string[] = [];

112-119: Consider updating lastMessage in Unlink case for consistency.

In the "Unlink" case, messages are updated to reference rocket.cat in the Messages collection, but room.lastMessage is not updated to reflect this change. This means lastMessage in rooms will still reference the original deleted user while the message in Messages collection shows rocket.cat.

While this doesn't cause "ghost messages" (since messages still exist), it creates an inconsistency between the cached lastMessage in rooms and the actual message in the Messages collection.

Note: This may be intentional or deferred work. The current PR's scope is specifically fixing ghost messages in the Delete case, which is correctly implemented.


124-127: Remove non-English comment; merge logic is correct.

Line 125 contains a Hindi comment ("Now affectedRoomIds is declared, so this line won't error"). Per coding guidelines, avoid comments in implementation, and when necessary, use English.

The merge logic correctly combines subscribed room IDs with affected room IDs (from lastMessage updates) and uses Set for deduplication.

🧹 Proposed fix
     const rids = subscribedRooms.map((room) => room.rid);
-    // Ab affectedRoomIds declared hai, toh ye line error nahi degi
     const allAffectedRids = [...new Set([...rids, ...affectedRoomIds])];
     void notifyOnRoomChangedById(allAffectedRids);
📜 Review details

Configuration used: Organization 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 424332d and 634e191.

📒 Files selected for processing (1)
  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧠 Learnings (11)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts

@NAME-ASHWANIYADAV NAME-ASHWANIYADAV requested a review from a team as a code owner January 9, 2026 08:40
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/models/src/models/Rooms.ts (1)

117-119: LGTM! Index addition supports efficient lastMessage queries.

The sparse index on lastMessage.u._id will optimize the query in deleteUser.ts that finds rooms where a deleted user sent the last message. This is essential for the ghost message fix.

Minor formatting adjustment for consistency
-			{ 
-				key: { 'lastMessage.u._id': 1 },
-			    sparse: true },
+			{
+				key: { 'lastMessage.u._id': 1 },
+				sparse: true,
+			},
📜 Review details

Configuration used: Organization 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 634e191 and a69b93b.

📒 Files selected for processing (2)
  • apps/meteor/app/lib/server/functions/deleteUser.ts
  • packages/models/src/models/Rooms.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • packages/models/src/models/Rooms.ts
  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧠 Learnings (11)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (3)
apps/meteor/app/lib/server/functions/getRoomsWithSingleOwner.ts (1)
  • getSubscribedRoomsForUserWithDetails (19-81)
apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts (1)
  • relinquishRoomOwnerships (63-84)
apps/meteor/app/lib/server/lib/notifyListener.ts (4)
  • notifyOnRoomChangedById (65-74)
  • notifyOnLivechatDepartmentAgentChanged (320-325)
  • notifyOnIntegrationChangedByUserId (189-198)
  • notifyOnUserChange (377-389)
🔇 Additional comments (2)
apps/meteor/app/lib/server/functions/deleteUser.ts (2)

88-98: LGTM! Core fix for ghost lastMessage on user deletion.

The logic correctly:

  1. Identifies rooms where the deleted user sent the last message (using the new index)
  2. Computes the next latest valid message after deletion
  3. Updates lastMessage or unsets it when no messages remain
  4. Tracks affected rooms for downstream notification

This directly addresses the PR objective to prevent ghost messages on Android.


123-125: LGTM! Correctly merges affected rooms for notifications.

The logic properly combines room IDs from subscriptions with rooms that had their lastMessage updated, deduplicates using a Set, and triggers notifications for all affected rooms. This ensures clients are informed about all relevant changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @apps/meteor/app/lib/server/functions/deleteUser.ts:
- Around line 118-129: The code contains stray leading backslashes causing
syntax errors in the delete user flow; remove the unwanted backslashes at the
start of the two lines inside the loop (the declaration of
roomsToUpdateUnlink/its loop start and the const [newLastMessage] = await
Messages.find(...) line) so that the for-await loop and the Messages.find call
compile correctly; ensure the block still pushes room._id to affectedRoomIds and
then updates Rooms with either $set lastMessage or $unset lastMessage as
originally intended (refer to the variables roomsToUpdateUnlink, room,
newLastMessage, Messages.find and Rooms.updateOne).
🧹 Nitpick comments (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (1)

74-132: Consider wrapping switch cases in blocks.

While the current code functions correctly, wrapping each case in braces would restrict variable scope to the case block and align with linting best practices, preventing variables like roomsToUpdate or roomsToUpdateUnlink from being accessible across cases.

Based on static analysis hints (Biome lint/correctness/noSwitchDeclarations).

♻️ Example refactor for case 'Delete'
-            case 'Delete': {
+            case 'Delete': {{
                 const store = FileUpload.getStore('Uploads');
                 // ... existing code ...
                 break;
-            }
+            }}

Apply similar wrapping to the 'Unlink' case.

📜 Review details

Configuration used: Organization 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 a69b93b and 54deff7.

📒 Files selected for processing (2)
  • apps/meteor/app/lib/server/functions/deleteUser.ts
  • packages/models/src/models/Rooms.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/models/src/models/Rooms.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧠 Learnings (11)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (5)
packages/core-typings/src/IUser.ts (1)
  • isUserFederated (267-267)
apps/meteor/app/lib/server/functions/getRoomsWithSingleOwner.ts (1)
  • getSubscribedRoomsForUserWithDetails (19-81)
apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts (1)
  • relinquishRoomOwnerships (63-84)
packages/models/src/index.ts (7)
  • ReadReceipts (188-188)
  • ModerationReports (214-214)
  • LivechatDepartmentAgents (163-163)
  • LivechatUnitMonitors (172-172)
  • LivechatVisitors (170-170)
  • Integrations (157-157)
  • FederationServers (150-150)
apps/meteor/app/lib/server/lib/notifyListener.ts (4)
  • notifyOnRoomChangedById (65-74)
  • notifyOnLivechatDepartmentAgentChanged (320-325)
  • notifyOnIntegrationChangedByUserId (189-198)
  • notifyOnUserChange (377-389)
🪛 Biome (2.1.2)
apps/meteor/app/lib/server/functions/deleteUser.ts

[error] 117-118: unexpected token \

(parse)


[error] 121-122: unexpected token \

(parse)


[error] 118-118: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (3)
apps/meteor/app/lib/server/functions/deleteUser.ts (3)

63-64: LGTM: Proper initialization of tracking arrays.

The affectedRoomIds array will correctly accumulate rooms that require lastMessage updates, and deletedRooms continues to track removed rooms.


87-98: LGTM: Correct logic for updating lastMessage in the Delete case.

The implementation properly handles the ghost message fix:

  • Identifies all rooms where the deleted user sent the last message
  • Fetches the new latest message (or determines the room has no messages)
  • Updates or unsets lastMessage accordingly

The query on line 88 should perform well with the new sparse index on lastMessage.u._id mentioned in the PR context.


137-139: LGTM: Proper deduplication and notification of affected rooms.

The Set-based deduplication correctly merges the original subscribed room IDs with the newly computed affected room IDs, ensuring all impacted rooms are notified exactly once.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In @apps/meteor/app/lib/server/functions/deleteUser.ts:
- Line 87: Remove the inline comment "// Find rooms where the user sent the last
message" in deleteUser and make the code self-documenting by renaming related
variables and functions to convey intent (for example, use names like
roomsWithUserAsLastSender, lastMessageSenderRoomQuery, or
findRoomsWhereUserWasLastSender) and, if needed, extract the logic into a
well-named helper (e.g., getRoomsWhereUserWasLastSender) so the flow is clear
without implementation comments.
- Around line 118-132: The switch case 'Unlink' contains lexical declarations
like const roomsToUpdateUnlink which must be inside a block to satisfy
Biome/noSwitchDeclarations; fix by wrapping the entire case 'Unlink' body in
curly braces { ... } (including the for-await loop, affectedRoomIds pushes, and
the Rooms/Messages update logic) so declarations such as roomsToUpdateUnlink and
newLastMessage have proper block scope and lint/CI will pass.
- Around line 63-65: The variable deletedRooms is never assigned and the result
of relinquishRoomOwnerships(...) is discarded; capture that return value into
deletedRooms (e.g., deletedRooms = await relinquishRoomOwnerships(...)) where it
is called, ensure affectedRoomIds still collects room IDs as intended, and
update the function's final return to return deletedRooms (or combine with
affectedRoomIds if intended) instead of always returning an empty array so the
function honors its declared return type.
- Around line 42-44: The user projection passed to Users.findOneById is missing
the name field, causing updateGroupDMsName and downstream consumers to receive
an incomplete IUser; update the projection in the Users.findOneById call to
include name (i.e., add name: 1) so updateGroupDMsName(user),
callbacks.run('afterDeleteUser', user) and
Apps.triggerEvent(AppEvents.IPostUserDeleted, { user }) receive the full
expected user object.
🧹 Nitpick comments (2)
apps/meteor/app/lib/server/functions/deleteUser.ts (2)

88-98: Perf: avoid toArray() + per-room Messages.find(...limit:1) (N+1 queries).

For users who are lastMessage author in many rooms, this can be very slow and memory-heavy. Prefer streaming the rooms cursor (no toArray()), and consider an aggregation to fetch “latest message per rid” in one pass + bulkWrite room updates.

Also applies to: 118-129


87-98: Deduplicate lastMessage recomputation logic between Delete and Unlink.

Both cases do the same “rooms where lastMessage.u._id === userId → recompute/unset” loop. Extract a small helper (e.g., recomputeLastMessageForUserRooms(userId, affectedRoomIds)) to reduce drift and make it easier to harden (projection, TOCTOU guard, cursor streaming) once.

Also applies to: 118-129

📜 Review details

Configuration used: Organization 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 54deff7 and 30954f0.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (1)
  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧠 Learnings (12)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-05T21:04:35.787Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37357
File: ee/packages/federation-matrix/src/setup.ts:103-120
Timestamp: 2025-11-05T21:04:35.787Z
Learning: In Rocket.Chat's federation-matrix setup (ee/packages/federation-matrix/src/setup.ts and apps/meteor/ee/server/startup/federation.ts), configureFederationMatrixSettings does not need to be called before setupFederationMatrix. The SDK's init() establishes infrastructure (database, event handlers, APIs) first, and the configuration can be applied later via settings watchers before actual federation events are processed. The config only matters when events actually occur, at which point all infrastructure is already configured.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (4)
packages/core-typings/src/IUser.ts (1)
  • isUserFederated (267-267)
apps/meteor/app/lib/server/functions/getRoomsWithSingleOwner.ts (1)
  • getSubscribedRoomsForUserWithDetails (19-81)
apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts (1)
  • relinquishRoomOwnerships (63-84)
apps/meteor/app/lib/server/lib/notifyListener.ts (4)
  • notifyOnRoomChangedById (65-74)
  • notifyOnLivechatDepartmentAgentChanged (320-325)
  • notifyOnIntegrationChangedByUserId (189-198)
  • notifyOnUserChange (377-389)
🪛 Biome (2.1.2)
apps/meteor/app/lib/server/functions/deleteUser.ts

[error] 118-118: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
apps/meteor/app/lib/server/functions/deleteUser.ts (3)

70-70: LGTM: Cleanup of redundant parameter.

The removeDirectMessages parameter defaults to true in relinquishRoomOwnerships, so explicitly passing true was redundant.


118-139: Unlink case correctly updates lastMessage after message reassignment.

After unlinkUserId reassigns messages to rocket.cat in the Messages collection (line 123), the denormalized lastMessage field in Rooms remains stale. Lines 126-136 correctly fetch the updated message and refresh room.lastMessage, with the same TOCTOU guard as the Delete case.

Optional: Extract lastMessage update logic into a helper function.

Lines 95-105 (Delete case) and lines 126-136 (Unlink case) are nearly identical. Consider extracting this pattern into a reusable helper:

async function updateLastMessageForRooms(userId: string, affectedRoomIds: string[]): Promise<void>

This would reduce duplication and make the logic easier to maintain.


221-222: Optional: Consider returning affectedRoomIds for observability.

The function currently returns { deletedRooms } but not affectedRoomIds. Including affected rooms in the return value would improve observability for testing and logging purposes:

return { deletedRooms, affectedRoomIds };

However, this changes the return type and may impact callers, so defer if broader compatibility is preferred.

📜 Review details

Configuration used: Organization 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 30954f0 and cb91ea6.

📒 Files selected for processing (1)
  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧠 Learnings (12)
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: The im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). Constructing slug-like identifiers like `user2${Users.userE2EE.data.username}` doesn't work for this endpoint.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-19T18:20:37.116Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: apps/meteor/server/services/media-call/service.ts:141-141
Timestamp: 2025-11-19T18:20:37.116Z
Learning: In apps/meteor/server/services/media-call/service.ts, the sendHistoryMessage method should use call.caller.id or call.createdBy?.id as the message author, not call.transferredBy?.id. Even for transferred calls, the message should appear in the DM between the two users who are calling each other, not sent by the person who transferred the call.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Avoid code comments in the implementation

Applied to files:

  • apps/meteor/app/lib/server/functions/deleteUser.ts
🧬 Code graph analysis (1)
apps/meteor/app/lib/server/functions/deleteUser.ts (2)
apps/meteor/app/lib/server/functions/relinquishRoomOwnerships.ts (1)
  • relinquishRoomOwnerships (63-84)
apps/meteor/app/lib/server/lib/notifyListener.ts (1)
  • notifyOnRoomChangedById (65-74)
🔇 Additional comments (4)
apps/meteor/app/lib/server/functions/deleteUser.ts (4)

43-43: LGTM: Projection includes necessary fields.

The extended projection correctly includes all fields used downstream in the deletion flow.


64-64: LGTM: Tracking affected rooms for lastMessage updates.

The affectedRoomIds array correctly tracks rooms where lastMessage will be updated, enabling proper notification downstream.


146-147: LGTM: Notification correctly includes all affected rooms.

The Set deduplication ensures rooms that appear in both subscribedRooms and affectedRoomIds are notified only once. All clients watching affected rooms will receive updates for the lastMessage changes.


74-117: Core fix correctly updates lastMessage after message deletion.

The logic is sound:

  1. All user messages are removed first (line 92)
  2. Rooms where the deleted user authored lastMessage are identified (line 95) using an indexed query on 'lastMessage.u._id'
  3. For each such room, the newest remaining message is fetched and lastMessage is updated (or unset if no messages remain)
  4. The TOCTOU guard { _id: room._id, 'lastMessage.u._id': userId } (line 99) ensures updates only apply if lastMessage still references the deleted user, preventing race conditions

Performance note: For users who authored lastMessage in many rooms, this loops through each room and queries the Messages collection. User deletion is infrequent, so this overhead is acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android app shows ghost lastMessage after user deletion (room.lastMessage not cleaned up)

1 participant