regression: Allow ban actions for non-federated rooms as well#39987
Merged
sampaiodiego merged 11 commits intorelease-8.3.0from Apr 2, 2026
Merged
regression: Allow ban actions for non-federated rooms as well#39987sampaiodiego merged 11 commits intorelease-8.3.0from
sampaiodiego merged 11 commits intorelease-8.3.0from
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR refactors the unban user flow to centralize authorization checks in the server-side Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
|
aleksandernsilva
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-8.3.0 #39987 +/- ##
=================================================
+ Coverage 70.58% 70.59% +0.01%
=================================================
Files 3258 3258
Lines 115863 115858 -5
Branches 21036 21079 +43
=================================================
+ Hits 81784 81793 +9
+ Misses 32026 32007 -19
- Partials 2053 2058 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
aleksandernsilva
force-pushed
the
fix/ban-kick-slashcommand-restrictions
branch
2 times, most recently
from
ac27bfa to
512537c
Compare
aleksandernsilva
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/app/slashcommands-ban/server/ban.ts (1)
21-34: Consider extracting the shared room-eligibility guard used by/banand/unban.The same lookup + eligibility + ephemeral error flow appears in both command handlers; a shared helper would reduce drift risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/slashcommands-ban/server/ban.ts` around lines 21 - 34, Extract the repeated room-eligibility guard into a shared helper (e.g., ensureFederatedRoom) that encapsulates the Rooms.findOneById lookup, the isRoomNativeFederated check, and the api.broadcast ephemeral error responses using i18n.t and settings.get; have ensureFederatedRoom accept the roomId (message.rid) and userId and return the Room when valid (or null/false after sending the appropriate ephemeral message), then replace the duplicated blocks in the /ban and /unban handlers with calls to ensureFederatedRoom so they early-return when the helper indicates the room is invalid or not federated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/i18n/src/locales/en.i18n.json`:
- Line 6213: The message for the localization key "error-ban-not-supported" is
ban-specific but is reused for both ban and unban; update the string for the key
error-ban-not-supported to be action-neutral (e.g., "This action is not
supported for this room type.") so it reads correctly for both /ban and /unban;
locate the key error-ban-not-supported in the en.i18n.json and replace the
existing ban-specific sentence with a neutral message.
---
Nitpick comments:
In `@apps/meteor/app/slashcommands-ban/server/ban.ts`:
- Around line 21-34: Extract the repeated room-eligibility guard into a shared
helper (e.g., ensureFederatedRoom) that encapsulates the Rooms.findOneById
lookup, the isRoomNativeFederated check, and the api.broadcast ephemeral error
responses using i18n.t and settings.get; have ensureFederatedRoom accept the
roomId (message.rid) and userId and return the Room when valid (or null/false
after sending the appropriate ephemeral message), then replace the duplicated
blocks in the /ban and /unban handlers with calls to ensureFederatedRoom so they
early-return when the helper indicates the room is invalid or not federated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d24efa9-ac9e-4ed1-ac18-fec3e7713621
📒 Files selected for processing (5)
apps/meteor/app/lib/server/functions/banUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/server/lib/unbanUserFromRoom.tspackages/i18n/src/locales/en.i18n.json
📜 Review details
⏰ 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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.ts
🧠 Learnings (16)
📓 Common learnings
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.
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.
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.
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.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/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.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/server/lib/unbanUserFromRoom.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/lib/unbanUserFromRoom.tsapps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.tsapps/meteor/app/lib/server/functions/banUserFromRoom.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/app/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.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/slashcommands-ban/server/unban.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/slashcommands-ban/server/unban.tsapps/meteor/app/slashcommands-ban/server/ban.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/slashcommands-ban/server/ban.ts
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
🔇 Additional comments (4)
apps/meteor/app/lib/server/functions/banUserFromRoom.ts (1)
75-75: Good change: awaited callback now surfaces post-ban failures.Awaiting
afterBanFromRoomCallback.run(...)prevents silent failures in downstream ban propagation hooks.apps/meteor/server/lib/unbanUserFromRoom.ts (1)
19-21: Nice hardening: room-type directives are now enforced for unban.This closes a gap by validating
BANmember action support at the room-directive layer before unbanning.apps/meteor/app/slashcommands-ban/server/unban.ts (1)
21-34: Validation flow looks correct for/unban.Room existence plus
isRoomNativeFederated(room)gating with explicit ephemeral errors cleanly prevents unsupported-room execution.apps/meteor/app/slashcommands-ban/server/ban.ts (1)
21-34: Correct guardrails added for/ban.The invalid-room and native-federation checks enforce the intended command scope and provide clear user feedback.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/lib/server/functions/banUserFromRoom.ts">
<violation number="1">
P1: The post-ban callback is no longer awaited, so callback failures are not propagated and ban side effects can run out-of-band.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
sampaiodiego
requested changes
Apr 2, 2026
aleksandernsilva
force-pushed
the
fix/ban-kick-slashcommand-restrictions
branch
from
5ac8b4b to
e37509f
Compare
scuciatto
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/server/lib/banUserFromRoom.ts">
<violation number="1" location="apps/meteor/server/lib/banUserFromRoom.ts:18">
P1: This new room-type check restricts a shared ban method to native federated rooms, which breaks `rooms.banUser` for regular rooms.</violation>
</file>
<file name="apps/meteor/app/slashcommands-ban/server/ban.ts">
<violation number="1">
P1: The `/ban` command no longer validates room existence/type before executing the ban, which reintroduces unsupported-room ban attempts.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts (1)
38-39: Avoid duplicatingisRoomNativeFederatedlogic inside the unit test double.Re-implementing the predicate here couples this spec to core-typings internals. Prefer a controllable stub (
sinon.stub()) and set return values per test case to keep this focused onbanUserFromRoomMethodbehavior only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts` around lines 38 - 39, Replace the inline predicate implementation for isRoomNativeFederated in the test double with a controllable Sinon stub: create a sinon.stub() for isRoomNativeFederated and inject/replace that into the module or dependency used by banUserFromRoomMethod, then set the stub's return value per test case (e.g., stub.returns(true) or stub.returns(false)) to exercise the different branches of banUserFromRoomMethod; this removes coupling to core-typings internals and keeps the spec focused on banUserFromRoomMethod behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/server/lib/banUserFromRoom.ts`:
- Around line 18-23: The current hard-gate in banUserFromRoom (the if checking
!room || !isRoomNativeFederated(room)) blocks bans for non-federated rooms even
before permission checks; remove or narrow that gate so banUserFromRoomMethod
first validates the room exists and then runs the policy check via
roomCoordinator.getRoomDirectives(room.t).allowMemberAction(...), and only apply
isRoomNativeFederated checks at slash-command entry points (or other UI-specific
handlers) if needed; ensure afterBanFromRoomCallback and federation logic
continue using FederationActions.shouldPerformFederationAction(room) to decide
whether to invoke federation hooks.
---
Nitpick comments:
In `@apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts`:
- Around line 38-39: Replace the inline predicate implementation for
isRoomNativeFederated in the test double with a controllable Sinon stub: create
a sinon.stub() for isRoomNativeFederated and inject/replace that into the module
or dependency used by banUserFromRoomMethod, then set the stub's return value
per test case (e.g., stub.returns(true) or stub.returns(false)) to exercise the
different branches of banUserFromRoomMethod; this removes coupling to
core-typings internals and keeps the spec focused on banUserFromRoomMethod
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59e2b21f-0b8b-4a41-97ce-a80ec8248ae2
📒 Files selected for processing (3)
apps/meteor/server/lib/banUserFromRoom.tsapps/meteor/server/lib/unbanUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/server/lib/unbanUserFromRoom.ts
📜 Review details
⏰ 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: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
🧠 Learnings (21)
📓 Common learnings
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.
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.
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.
📚 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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-09T18:39:21.178Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:21.178Z
Learning: In apps/meteor/server/methods/addAllUserToRoom.ts, the implementation uses a single cursor pass (Users.find(userFilter).batchSize(100)) that collects both the full user objects (collectedUsers: IUser[]) and their usernames (usernames: string[]) in one iteration. `beforeAddUserToRoom` is then called once with the full usernames batch (preserving batch-validation semantics), and the subsequent subscription/message processing loop iterates over the same stable `collectedUsers` array — no second DB query is made. This avoids any race condition between validation and processing while preserving the original batch-validation behavior.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-19T13:59:40.678Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 38357
File: apps/meteor/app/apps/server/converters/uploads.ts:45-49
Timestamp: 2026-03-19T13:59:40.678Z
Learning: In `apps/meteor/app/apps/server/converters/uploads.ts`, the `room` async handler in `convertToApp` uses non-null assertions (`upload.rid!` and `result!`) intentionally. The data flow guarantees that any upload reaching this point must have a `rid`; if it does not, throwing an error is the desired behavior (fail-fast / data integrity guard). Do not flag these non-null assertions as unsafe during code review.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.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/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/lib/banUserFromRoom.tsapps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.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 apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-06T18:02:20.381Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.ts
Comment on lines
18
to
23
| if (!room || !isRoomNativeFederated(room)) { | ||
| throw new Error('Invalid room'); | ||
| } | ||
|
|
||
| if (!(await roomCoordinator.getRoomDirectives(room.t).allowMemberAction(room, RoomMemberActions.BAN, fromId))) { | ||
| throw new Error('Not allowed'); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find all usages of banUserFromRoomMethod"
rg -nP --type=ts -C3 '\bbanUserFromRoomMethod\s*\('
echo
echo "2) Find method/API wiring that may route non-slash flows here"
rg -nP --type=ts -C3 "banUserFromRoom(Method)?|['\"]ban-user['\"]|['\"]banUserFromRoom['\"]"
echo
echo "3) Locate slash-command handlers to confirm intended restriction boundary"
rg -nP --type=ts -C3 '/ban|/unban|slash.?command|SlashCommand'Repository: RocketChat/Rocket.Chat
Length of output: 50379
🏁 Script executed:
cat -n apps/meteor/server/lib/banUserFromRoom.tsRepository: RocketChat/Rocket.Chat
Length of output: 2771
🏁 Script executed:
cat -n apps/meteor/app/lib/server/functions/banUserFromRoom.ts | head -80Repository: RocketChat/Rocket.Chat
Length of output: 3665
🏁 Script executed:
# Check when the isRoomNativeFederated check was added to banUserFromRoomMethod
git log -p --follow -S 'isRoomNativeFederated' -- 'apps/meteor/server/lib/banUserFromRoom.ts' | head -150Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Check the REST API endpoint for ban context
sed -n '1290,1320p' apps/meteor/app/api/server/v1/rooms.tsRepository: RocketChat/Rocket.Chat
Length of output: 816
🏁 Script executed:
# Check the full endpoint definition context
sed -n '1270,1295p' apps/meteor/app/api/server/v1/rooms.tsRepository: RocketChat/Rocket.Chat
Length of output: 651
🏁 Script executed:
# Check if there are any federation-specific comments or PR context
git log --oneline -20 -- apps/meteor/server/lib/banUserFromRoom.tsRepository: RocketChat/Rocket.Chat
Length of output: 154
🏁 Script executed:
# Check the commit that introduced this change
git show e37509f698 -- apps/meteor/server/lib/banUserFromRoom.ts | head -200Repository: RocketChat/Rocket.Chat
Length of output: 2852
🏁 Script executed:
# Check what was in the slash command before the refactor
git show e37509f698^ -- apps/meteor/app/slashcommands-ban/server/ban.ts | head -100Repository: RocketChat/Rocket.Chat
Length of output: 99
🏁 Script executed:
# Check the isRoomNativeFederated function definition
rg -nA5 'export.*function isRoomNativeFederated|export const isRoomNativeFederated' --type=tsRepository: RocketChat/Rocket.Chat
Length of output: 554
🏁 Script executed:
# Check federation-related hooks for ban handling
fd -t f banUserFromRoomRepository: RocketChat/Rocket.Chat
Length of output: 406
🏁 Script executed:
# Check if there are federation hooks for banning
rg -n 'ban.*federation|federation.*ban' apps/meteor/ee/server/hooks/federation/ --type=ts -iRepository: RocketChat/Rocket.Chat
Length of output: 521
🏁 Script executed:
# Check the federation hooks context around ban
sed -n '200,240p' apps/meteor/ee/server/hooks/federation/index.tsRepository: RocketChat/Rocket.Chat
Length of output: 1397
🏁 Script executed:
# Check FederationActions.shouldPerformFederationAction implementation
rg -nA10 'shouldPerformFederationAction' --type=ts | head -50Repository: RocketChat/Rocket.Chat
Length of output: 4352
🏁 Script executed:
# Check if there's any documentation or prior implementation of ban for non-federated rooms
rg -n 'banUser|ban.*room' apps/meteor/app/slashcommands-ban/server/ --type=ts -B3 -A3 | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2742
🏁 Script executed:
# Check the test expectations for the federated requirement
sed -n '77,82p' apps/meteor/tests/unit/server/methods/banUserFromRoom.spec.tsRepository: RocketChat/Rocket.Chat
Length of output: 375
Native-federation requirement at line 18 restricts ban to federated rooms only, including REST API calls.
The hard gate if (!room || !isRoomNativeFederated(room)) at line 18 rejects all non-federated rooms before the room-type policy check at line 22. This affects both the /ban slash command and the POST /api/v1/rooms.ban REST API endpoint, both of which call banUserFromRoomMethod. The test at line 77–82 explicitly confirms this behavior rejects non-federated channels.
If ban was previously supported for non-federated public channels, this constitutes a regression. The federation hooks (afterBanFromRoomCallback) gracefully handle non-federated rooms by checking FederationActions.shouldPerformFederationAction(room) rather than completely blocking them. Consider whether the native-federation requirement should be removed or scoped only to slash-command entry points.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/server/lib/banUserFromRoom.ts` around lines 18 - 23, The current
hard-gate in banUserFromRoom (the if checking !room ||
!isRoomNativeFederated(room)) blocks bans for non-federated rooms even before
permission checks; remove or narrow that gate so banUserFromRoomMethod first
validates the room exists and then runs the policy check via
roomCoordinator.getRoomDirectives(room.t).allowMemberAction(...), and only apply
isRoomNativeFederated checks at slash-command entry points (or other UI-specific
handlers) if needed; ensure afterBanFromRoomCallback and federation logic
continue using FederationActions.shouldPerformFederationAction(room) to decide
whether to invoke federation hooks.
sampaiodiego
force-pushed
the
fix/ban-kick-slashcommand-restrictions
branch
from
e37509f to
4f01f82
Compare
sampaiodiego
force-pushed
the
fix/ban-kick-slashcommand-restrictions
branch
from
4f01f82 to
4be38c0
Compare
sampaiodiego
changed the title
sampaiodiego
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Allow UI actions for ban/unban users for non-federated rooms.
Issue(s)
FGA-54
FGA-55
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Refactor