feat(sidebar): add personal room categories#40665
Open
artyoomi wants to merge 5 commits into
Open
Conversation
|
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
Contributor
WalkthroughAdds per-user room categories: types and Mongo model, six authenticated REST endpoints, a React Query hook, multiple modals for create/rename/assign/manage, sidebar group rendering and menus, menu action integration points, and English/Russian i18n entries. ChangesUser Room Categories Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Modal/Menu UI
participant Hook as useUserRoomCategories
participant API as REST Endpoints
participant Model as UserRoomCategoriesRaw
User->>UI: Create category (name)
UI->>Hook: addCategory(name)
Hook->>API: POST /v1/user-room-categories
API->>Model: createCategory(userId, name)
Model->>Model: Validate uniqueness, upsert with $push
Model-->>API: UpdateResult
API-->>Hook: {success: true}
Hook->>Hook: invalidate query
Hook-->>UI: refetch categories
UI-->>User: toast success + close
User->>UI: Assign room to category
UI->>Hook: addRoomToCategory(categoryName, roomId)
Hook->>API: POST /v1/user-room-categories/add-room
API->>Model: addRoomToCategory(userId, categoryName, roomId)
Model->>Model: remove roomId from all categories, add to target, dedupe
Model-->>API: UpdateResult
API-->>Hook: {success: true}
Hook->>Hook: invalidate query
Hook-->>UI: refetch + update sidebar
UI-->>User: toast success + close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
|
Backend: * Add model to manage user room categories * Add API methods to manager user room categories * Add client hook to manage user room categories Frontend: * Expose "Create room category" in Create New * Allow assigning/removing rooms via room menus * Show empty custom categories in the sidebar * Support deleting categories from the sidebar header * Support renaming categories from the sidebar header
artyoomi
force-pushed
the
feat/custom-sidebar-sections
branch
from
385d7b5 to
ee74735
Compare
coderabbitai
Bot
added
the
type: feature
![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: 10
🧹 Nitpick comments (3)
packages/i18n/src/locales/ru.i18n.json (1)
4040-4040: ⚡ Quick winUse consistent Russian term for “sidebar”.
Line 4040, Line 4045, and Line 4048 use “сайдбар”, while this locale already standardizes on “боковая панель” (e.g., key
Sidebar). Please align these strings for consistency.Suggested wording update
- "User_room_category_add_description": "Выберите пользовательскую категорию сайдбара для этой комнаты. Комната будет убрана из любой другой личной категории.", + "User_room_category_add_description": "Выберите пользовательскую категорию боковой панели для этой комнаты. Комната будет убрана из любой другой личной категории.", - "User_room_category_delete_confirm": "Удалить категорию \"{{name}}\"? Комнаты останутся в сайдбаре в своих обычных разделах.", + "User_room_category_delete_confirm": "Удалить категорию \"{{name}}\"? Комнаты останутся в боковой панели в своих обычных разделах.", - "User_room_category_manage": "Пользовательская категория сайдбара", + "User_room_category_manage": "Пользовательская категория боковой панели",Also applies to: 4045-4045, 4048-4048
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/i18n/src/locales/ru.i18n.json` at line 4040, Strings use the informal term "сайдбар" but the locale standardizes on "боковая панель"; update the value of "User_room_category_add_description" and the two adjacent locale entries referenced (near lines 4045 and 4048) to replace "сайдбар" with "боковая панель" so all sidebar references use the same Russian term.packages/rest-typings/src/index.ts (1)
46-47: ⚡ Quick winExport the new endpoint typings from the root barrel as well.
EndpointsincludesUserRoomCategoriesEndpoints, but./v1/userRoomCategoriesis not re-exported in the package root export list.Proposed fix
export * from './v1/twoFactorChallenges'; +export * from './v1/userRoomCategories';Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rest-typings/src/index.ts` around lines 46 - 47, The root barrel is missing re-exports for the new endpoint typings referenced in the Endpoints union: add explicit re-exports for the modules that define those types (e.g. export type { UserRoomCategoriesEndpoints } from './v1/userRoomCategories' and export type { VideoConferenceEndpoints } from './v1/videoConference') so the named types referenced by Endpoints are available from the package root; update the export list to include each missing endpoint typing symbol.apps/meteor/client/sidebar/RoomList/RoomListCollapser.tsx (1)
10-13: ⚡ Quick winRemove inline implementation comments from props type.
Please drop these inline comments and keep the type self-descriptive to match repository style.
As per coding guidelines
**/*.{ts,tsx,js}: “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/sidebar/RoomList/RoomListCollapser.tsx` around lines 10 - 13, Remove the inline implementation comments on the props type in RoomListCollapser: delete the comment lines preceding displayTitle and ariaTitle in the props/interface so the type remains self-descriptive (keep the properties displayTitle: ReactNode and ariaTitle: string as-is) to comply with the repository guideline "Avoid code comments in the implementation."
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/app/api/server/v1/user-room-categories.ts`:
- Around line 149-155: The two route registrations for
'user-room-categories/remove-category' and
'user-room-categories/rename-category' are missing the explicit
rateLimiterOptions: false setting used by other user-room-categories endpoints;
update the API.v1.post calls that register these routes to include
rateLimiterOptions: false in the options object so their throttling behavior
matches the other routes (match pattern used by the existing
user-room-categories routes in the file).
In `@apps/meteor/client/hooks/useRoomMenuActions.ts`:
- Around line 118-121: The catch block inside useRoomMenuActions currently
converts the caught error to a string before calling dispatchToastMessage;
instead pass the raw caught error object to preserve structured info so
getErrorMessage can extract details. Update the catch handler around the
dispatchToastMessage call (the one that currently builds message = error
instanceof Error ? error.message : String(error)) to call dispatchToastMessage({
type: 'error', message: error || t('Something_went_wrong') }) so the original
error object is forwarded; keep the fallback to t('Something_went_wrong') if
error is falsy.
In `@apps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsx`:
- Around line 75-77: The catch blocks in useUserRoomCategoryForRoomModal.tsx
normalize the caught error into a string before calling dispatchToastMessage;
remove that normalization and pass the raw caught value to dispatchToastMessage
so the ToastMessagesProvider can format it via getErrorMessage. Locate the two
catch blocks around the code that currently builds message with error instanceof
Error ? error.message : String(error) and replace that expression with the
original caught error object (the variable named error) when calling
dispatchToastMessage({ type: 'error', message: ... }). Ensure no other
normalization logic remains in the function so dispatchToastMessage receives the
unknown value directly.
In
`@apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsx`:
- Around line 52-55: In the catch block inside CreateUserRoomCategoryModal where
dispatchToastMessage is called, stop converting err to a string and instead pass
the original error object so ToastMessagesProvider can use getErrorMessage;
replace the current message construction with dispatchToastMessage({ type:
'error', message: err ?? t('User_room_category_create_failed') }) (preserving
the existing t(...) fallback) so dispatchToastMessage receives the actual
Error/failed object.
In `@apps/meteor/client/sidebar/hooks/useRoomList.ts`:
- Around line 175-176: The groupsList return type is incorrect: fullSidebarOrder
can contain arbitrary user strings from customCategoryOrder/userRoomCategories,
yet groupsList is typed as TranslationKey[] and the reducer casts keys as
TranslationKey; change groupsList to string[] (update the reducer accumulator
and any signatures in useRoomList.ts that reference groupsList), stop casting
key as TranslationKey inside the reduce, and ensure rendering still translates
only built-in keys by keeping the SIDEBAR_BUILTIN_GROUP_KEYS check (e.g., call
t(groupTitle) only when group is in SIDEBAR_BUILTIN_GROUP_KEYS and treat other
group titles as raw strings).
In `@apps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx`:
- Around line 49-52: In the catch block of RenameUserRoomCategoryModal.tsx, stop
stringifying the caught error; instead pass the raw error through to
dispatchToastMessage so structured error data is preserved by the
ToastMessagesContext. Replace the current message computation (error instanceof
Error ? error.message : String(error)) with using the raw error (e.g.
dispatchToastMessage({ type: 'error', message: error ??
t('Something_went_wrong') })) so the toast receives the original error object
while still falling back to t('Something_went_wrong') when error is nullish.
In `@apps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsx`:
- Around line 24-27: The catch block is stringifying the caught error before
calling dispatchToastMessage which expects message: unknown; preserve and pass
the raw caught error instead. Replace the current construction of const message
= error instanceof Error ? error.message : String(error) with passing error
directly to dispatchToastMessage (e.g., dispatchToastMessage({ type: 'error',
message: error ?? t('Something_went_wrong') })) so the toast receives the
original Error object while still falling back to t('Something_went_wrong') when
error is null/undefined; keep references to dispatchToastMessage and t
unchanged.
In `@packages/model-typings/src/models/IUserRoomCategoriesModel.ts`:
- Around line 6-13: IUserRoomCategoriesModel is missing the renameCategory
declaration even though UserRoomCategoriesRaw implements it; add a
renameCategory method signature to the IUserRoomCategoriesModel interface
matching the implementation (e.g., renameCategory(userId: string, oldName:
string, newName: string): Promise<UpdateResult>) so the interface contract
aligns with UserRoomCategoriesRaw and proxied model consumers can rely on the
typed API.
In `@packages/models/src/models/UserRoomCategories.ts`:
- Around line 41-69: The current addRoomToCategory implementation reads the
whole document and $set's the entire categories array which causes lost updates
under concurrency; change it to perform a targeted atomic update using Mongo
update operators (e.g., $addToSet / $pull with arrayFilters or positional
operator) instead of replacing the full categories array: locate
addRoomToCategory (and the similar methods noted) and replace the
read-modify-write logic that builds categoriesWithoutRoom with an updateOne that
uses $pull to remove the roomId from other categories if needed and $addToSet
with an appropriate filter/arrayFilters to add the trimmedRoomId only to the
matched category, preserving other categories atomically. Ensure you still
validate existence (using findByUserId or a filtered update result) and trim
inputs (trimmedCategoryName/trimmedRoomId) before the single atomic update.
- Around line 20-38: createCategory currently accepts raw names allowing
blank/whitespace and inconsistent matching; trim the incoming name, validate
it's non-empty (throw a descriptive error if empty), and use the trimmed value
for the duplicate check (existing?.categories.some(c => c.name === trimmedName))
and for the $push payload so stored names are normalized; keep the rest of the
updateOne/upsert logic unchanged.
---
Nitpick comments:
In `@apps/meteor/client/sidebar/RoomList/RoomListCollapser.tsx`:
- Around line 10-13: Remove the inline implementation comments on the props type
in RoomListCollapser: delete the comment lines preceding displayTitle and
ariaTitle in the props/interface so the type remains self-descriptive (keep the
properties displayTitle: ReactNode and ariaTitle: string as-is) to comply with
the repository guideline "Avoid code comments in the implementation."
In `@packages/i18n/src/locales/ru.i18n.json`:
- Line 4040: Strings use the informal term "сайдбар" but the locale standardizes
on "боковая панель"; update the value of "User_room_category_add_description"
and the two adjacent locale entries referenced (near lines 4045 and 4048) to
replace "сайдбар" with "боковая панель" so all sidebar references use the same
Russian term.
In `@packages/rest-typings/src/index.ts`:
- Around line 46-47: The root barrel is missing re-exports for the new endpoint
typings referenced in the Endpoints union: add explicit re-exports for the
modules that define those types (e.g. export type { UserRoomCategoriesEndpoints
} from './v1/userRoomCategories' and export type { VideoConferenceEndpoints }
from './v1/videoConference') so the named types referenced by Endpoints are
available from the package root; update the export list to include each missing
endpoint typing symbol.
🪄 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: 4868e231-890d-4ed7-b3ea-de36230d4677
📒 Files selected for processing (26)
apps/meteor/app/api/server/index.tsapps/meteor/app/api/server/v1/user-room-categories.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/hooks/useCreateNewMenu.tsxapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsxapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/sidebar/RoomList/RoomListCollapser.tsxapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/views/room/contextualBar/Info/ManageUserRoomCategoryModal.tsxapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.tsapps/meteor/server/models.tspackages/core-typings/src/IUserRoomCategories.tspackages/core-typings/src/index.tspackages/i18n/src/locales/en.i18n.jsonpackages/i18n/src/locales/ru.i18n.jsonpackages/model-typings/src/index.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tspackages/models/src/index.tspackages/models/src/modelClasses.tspackages/models/src/models/UserRoomCategories.tspackages/rest-typings/src/index.tspackages/rest-typings/src/v1/userRoomCategories.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). (1)
- GitHub Check: cubic · AI code reviewer
🧰 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/core-typings/src/index.tspackages/core-typings/src/IUserRoomCategories.tspackages/rest-typings/src/index.tsapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxpackages/models/src/modelClasses.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/app/api/server/index.tspackages/model-typings/src/index.tsapps/meteor/server/models.tspackages/rest-typings/src/v1/userRoomCategories.tspackages/models/src/index.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/RoomList/RoomListCollapser.tsxapps/meteor/app/api/server/v1/user-room-categories.tsapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxpackages/models/src/models/UserRoomCategories.tsapps/meteor/client/navbar/NavBarPagesGroup/hooks/useCreateNewMenu.tsxapps/meteor/client/views/room/contextualBar/Info/ManageUserRoomCategoryModal.tsxapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.tsapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx
🧠 Learnings (10)
📚 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:
packages/core-typings/src/index.tspackages/core-typings/src/IUserRoomCategories.tspackages/rest-typings/src/index.tspackages/models/src/modelClasses.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/app/api/server/index.tspackages/model-typings/src/index.tsapps/meteor/server/models.tspackages/rest-typings/src/v1/userRoomCategories.tspackages/models/src/index.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.tsapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.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:
packages/core-typings/src/index.tspackages/core-typings/src/IUserRoomCategories.tspackages/rest-typings/src/index.tspackages/models/src/modelClasses.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/app/api/server/index.tspackages/model-typings/src/index.tsapps/meteor/server/models.tspackages/rest-typings/src/v1/userRoomCategories.tspackages/models/src/index.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.tsapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/core-typings/src/index.tspackages/core-typings/src/IUserRoomCategories.tspackages/rest-typings/src/index.tsapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxpackages/models/src/modelClasses.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/app/api/server/index.tspackages/model-typings/src/index.tsapps/meteor/server/models.tspackages/rest-typings/src/v1/userRoomCategories.tspackages/models/src/index.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/RoomList/RoomListCollapser.tsxapps/meteor/app/api/server/v1/user-room-categories.tsapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxpackages/models/src/models/UserRoomCategories.tsapps/meteor/client/navbar/NavBarPagesGroup/hooks/useCreateNewMenu.tsxapps/meteor/client/views/room/contextualBar/Info/ManageUserRoomCategoryModal.tsxapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.tsapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx
📚 Learning: 2026-05-11T23:14:59.316Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40469
File: packages/rest-typings/src/v1/users.ts:337-337
Timestamp: 2026-05-11T23:14:59.316Z
Learning: In Rocket.Chat REST endpoint typings (e.g., packages/rest-typings/src/v1/users.ts and other rest-typings files), keep the established convention of deriving field types from the domain model (e.g., use IUser indexed access like IUser['statusExpiresAt']) rather than swapping individual fields to serialized primitives (like string) in an ad-hoc way. If a truly different “serialized” representation is needed, perform the refactor consistently across the codebase (not just a single endpoint/field) and ensure all related REST typings stay aligned with the shared serialization types.
Applied to files:
packages/rest-typings/src/index.tspackages/rest-typings/src/v1/userRoomCategories.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/sidebar/RoomList/RoomListCollapser.tsxapps/meteor/client/sidebar/RoomList/RoomList.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/hooks/useCreateNewMenu.tsxapps/meteor/client/views/room/contextualBar/Info/ManageUserRoomCategoryModal.tsxapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.ts
📚 Learning: 2026-04-14T21:10:31.855Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36292
File: apps/meteor/client/hooks/useHasValidLocationHash.ts:7-12
Timestamp: 2026-04-14T21:10:31.855Z
Learning: When reviewing files in apps/meteor/client/hooks/, do not treat JSDoc-style comments on React hooks (especially exported hooks) as a violation of any “avoid code comments in implementation” guideline. It’s acceptable to use JSDoc to document the public API of exported hooks (e.g., parameter/return types, intended usage), as long as it documents behavior/contracts rather than adding narrative implementation comments.
Applied to files:
apps/meteor/client/hooks/useUserRoomCategories.tsapps/meteor/client/hooks/useRoomMenuActions.ts
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/user-room-categories.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/user-room-categories.ts
🔇 Additional comments (16)
packages/i18n/src/locales/en.i18n.json (1)
484-484: LGTM!Also applies to: 1606-1606, 5745-5762
packages/i18n/src/locales/ru.i18n.json (1)
330-330: LGTM!Also applies to: 1219-1219, 4041-4058, 5005-5005
packages/core-typings/src/IUserRoomCategories.ts (1)
1-10: LGTM!packages/core-typings/src/index.ts (1)
38-38: LGTM!packages/model-typings/src/index.ts (1)
64-64: LGTM!packages/rest-typings/src/v1/userRoomCategories.ts (1)
1-25: LGTM!packages/models/src/index.ts (1)
64-64: LGTM!Also applies to: 109-109, 201-201, 219-219
packages/models/src/modelClasses.ts (1)
63-63: LGTM!apps/meteor/app/api/server/v1/user-room-categories.ts (1)
1-147: LGTM!Also applies to: 179-196
apps/meteor/app/api/server/index.ts (1)
46-46: LGTM!apps/meteor/server/models.ts (1)
72-72: LGTM!Also applies to: 162-162
apps/meteor/client/hooks/useUserRoomCategories.ts (1)
4-84: LGTM!apps/meteor/client/sidebar/RoomList/RoomList.tsx (1)
56-91: LGTM!apps/meteor/client/views/room/contextualBar/Info/ManageUserRoomCategoryModal.tsx (1)
78-80: ⚡ Quick winRevert the suggestion to pass the raw caught
errordirectly todispatchToastMessage(lines 78-80 and 95-97).
dispatchToastMessageacceptsunknown, butToastMessagesProviderrenderstype: 'error'by callinggetErrorMessage(message)without a fallbackdefaultMessage. If the thrown value is an object withoutmessage/reason/errorstring fields (or has an emptymessage),getErrorMessagecan throw and you also lose the currentt('Something_went_wrong')fallback—so the existingmessage || t('Something_went_wrong')behavior should remain.> Likely an incorrect or invalid review comment.apps/meteor/client/navbar/NavBarPagesGroup/hooks/useCreateNewMenu.tsx (1)
1-34: LGTM!apps/meteor/client/views/room/contextualBar/Info/hooks/useRoomActions.ts (1)
2-4: LGTM!Also applies to: 6-6, 23-28, 66-75, 121-134
Comment on lines
+41
to
+69
| async addRoomToCategory(userId: string, categoryName: string, roomId: string): Promise<UpdateResult> { | ||
| const doc = await this.findByUserId(userId); | ||
| if (!doc) { | ||
| throw new Error('User categories document not found'); | ||
| } | ||
|
|
||
| const trimmedCategoryName = categoryName.trim(); | ||
| const trimmedRoomId = roomId.trim(); | ||
|
|
||
| const categoryExists = doc.categories.some((c) => c.name === trimmedCategoryName); | ||
| if (!categoryExists) { | ||
| throw new Error('Category not found'); | ||
| } | ||
|
|
||
| const categoriesWithoutRoom = doc.categories.map((c) => ({ | ||
| ...c, | ||
| roomIds: (c.roomIds ?? []).filter((rid) => rid !== trimmedRoomId), | ||
| })); | ||
|
|
||
| const targetIndex = categoriesWithoutRoom.findIndex((c) => c.name === trimmedCategoryName); | ||
| const target = categoriesWithoutRoom[targetIndex]; | ||
|
|
||
| categoriesWithoutRoom[targetIndex] = { | ||
| ...target, | ||
| roomIds: Array.from(new Set([...(target?.roomIds ?? []), trimmedRoomId])), | ||
| }; | ||
|
|
||
| return this.updateOne({ userId }, { $set: { categories: categoriesWithoutRoom } }); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid full-array read-modify-write updates for categories.
These methods read the document and then $set the entire categories array. Concurrent requests can overwrite each other (lost updates), especially when add/remove/rename happen close together.
Also applies to: 71-97, 99-109, 111-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/models/src/models/UserRoomCategories.ts` around lines 41 - 69, The
current addRoomToCategory implementation reads the whole document and $set's the
entire categories array which causes lost updates under concurrency; change it
to perform a targeted atomic update using Mongo update operators (e.g.,
$addToSet / $pull with arrayFilters or positional operator) instead of replacing
the full categories array: locate addRoomToCategory (and the similar methods
noted) and replace the read-modify-write logic that builds categoriesWithoutRoom
with an updateOne that uses $pull to remove the roomId from other categories if
needed and $addToSet with an appropriate filter/arrayFilters to add the
trimmedRoomId only to the matched category, preserving other categories
atomically. Ensure you still validate existence (using findByUserId or a
filtered update result) and trim inputs (trimmedCategoryName/trimmedRoomId)
before the single atomic update.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 26 files
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="packages/models/src/models/UserRoomCategories.ts">
<violation number="1" location="packages/models/src/models/UserRoomCategories.ts:20">
P2: Race condition in createCategory allows duplicate category names under concurrent requests</violation>
</file>
<file name="apps/meteor/client/sidebar/hooks/useRoomList.ts">
<violation number="1" location="apps/meteor/client/sidebar/hooks/useRoomList.ts:162">
P1: Custom category names can silently overwrite built-in sidebar group keys in the `groups` Map because `categoryName` is used as a Map key without guarding against reserved built-in section names like 'Favorites', 'Channels', 'Teams', etc.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| favoritesEnabled && favorite.size && groups.set('Favorites', favorite); | ||
| customCategoryRooms.forEach((rooms, categoryName) => { | ||
| // Show empty categories as groups, so they are visible/manageable right after creation. | ||
| groups.set(categoryName, rooms); |
Contributor
There was a problem hiding this comment.
P1: Custom category names can silently overwrite built-in sidebar group keys in the groups Map because categoryName is used as a Map key without guarding against reserved built-in section names like 'Favorites', 'Channels', 'Teams', etc.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/hooks/useRoomList.ts, line 162:
<comment>Custom category names can silently overwrite built-in sidebar group keys in the `groups` Map because `categoryName` is used as a Map key without guarding against reserved built-in section names like 'Favorites', 'Channels', 'Teams', etc.</comment>
<file context>
@@ -133,6 +157,10 @@ export const useRoomList = ({ collapsedGroups }: { collapsedGroups?: string[] })
favoritesEnabled && favorite.size && groups.set('Favorites', favorite);
+ customCategoryRooms.forEach((rooms, categoryName) => {
+ // Show empty categories as groups, so they are visible/manageable right after creation.
+ groups.set(categoryName, rooms);
+ });
</file context>
| return this.findOne({ userId }); | ||
| } | ||
|
|
||
| async createCategory(userId: string, name: string): Promise<UpdateResult> { |
Contributor
There was a problem hiding this comment.
P2: Race condition in createCategory allows duplicate category names under concurrent requests
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/models/src/models/UserRoomCategories.ts, line 20:
<comment>Race condition in createCategory allows duplicate category names under concurrent requests</comment>
<file context>
@@ -0,0 +1,137 @@
+ return this.findOne({ userId });
+ }
+
+ async createCategory(userId: string, name: string): Promise<UpdateResult> {
+ const existing = await this.findByUserId(userId);
+
</file context>
- Add rateLimiterOptions: false to remove-category and rename-category endpoints - Add renameCategory to IUserRoomCategoriesModel interface - Pass raw error to dispatchToastMessage instead of stringifying in catch blocks - Change groupsList type from TranslationKey[] to string[] in useRoomList - Trim and validate name in createCategory method
coderabbitai
Bot
removed
the
type: feature
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/models/src/models/UserRoomCategories.ts (1)
27-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake
createCategoryduplicate prevention atomic (and fix the MongoDB filter).
UserRoomCategoriesRaw.createCategorystill does read-then-write (findByUserId+$push), so concurrent requests can both insert the same categoryname.- The suggested filter using
{ 'categories.name': { $ne: trimmedName } }won’t work for an array of objects: MongoDB can match when any element has a differentname. Use an “absence of element” predicate instead, e.g.{ userId, categories: { $not: { $elemMatch: { name: trimmedName } } } }, and throw whenmodifiedCount === 0andupsertedCount === 0.addRoomToCategory,removeRoomFromCategory,removeCategory, andrenameCategoryall rebuild the fullcategoriesarray and persist via$set: { categories }, so concurrent calls can clobber each other (and bypass the duplicate checks inrenameCategory).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/models/src/models/UserRoomCategories.ts` around lines 27 - 44, createCategory currently does a non-atomic read-then-write allowing duplicate names; replace the findByUserId + unconditional $push with a single updateOne that uses the filter { userId, categories: { $not: { $elemMatch: { name: trimmedName } } } } and the same $push payload, then throw if result.modifiedCount === 0 && result.upsertedCount === 0 so concurrent inserts fail deterministically; additionally, avoid rebuilding the entire categories array in addRoomToCategory, removeRoomFromCategory, removeCategory, and renameCategory—use atomic array operators (e.g., $addToSet/$pull/$set with arrayFilters and positional updates) so concurrent calls don’t clobber each other or bypass duplicate checks (update renameCategory to check existence via an atomic filter or use an $elemMatch-based conditional update).
♻️ Duplicate comments (1)
packages/models/src/models/UserRoomCategories.ts (1)
47-75:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid full-array read-modify-write in category mutation methods.
These methods read
doc.categories, mutate in memory, then$setthe entire array. Concurrent requests can overwrite each other and lose updates. Use targeted atomic operators ($addToSet,$pull, positional/arrayFilters) per method instead of replacing the full array.Also applies to: 77-103, 105-115, 117-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/models/src/models/UserRoomCategories.ts` around lines 47 - 75, The addRoomToCategory method currently reads the whole doc.categories, mutates it in memory and writes back the entire array which risks lost concurrent updates; change it to use targeted atomic updates instead: use updateOne with $pull to remove roomId from any category.roomIds that contain it and $addToSet with arrayFilters or the positional operator to add the trimmedRoomId only to the target category (match by userId and category name) without replacing the full categories array. Locate addRoomToCategory plus the other category-mutating methods referenced and replace their read-modify-write logic (the findByUserId → updateOne pattern) with equivalent atomic Mongo updates using $addToSet, $pull and arrayFilters/positional updates against the categories field.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/models/src/models/UserRoomCategories.ts`:
- Around line 27-44: createCategory currently does a non-atomic read-then-write
allowing duplicate names; replace the findByUserId + unconditional $push with a
single updateOne that uses the filter { userId, categories: { $not: {
$elemMatch: { name: trimmedName } } } } and the same $push payload, then throw
if result.modifiedCount === 0 && result.upsertedCount === 0 so concurrent
inserts fail deterministically; additionally, avoid rebuilding the entire
categories array in addRoomToCategory, removeRoomFromCategory, removeCategory,
and renameCategory—use atomic array operators (e.g., $addToSet/$pull/$set with
arrayFilters and positional updates) so concurrent calls don’t clobber each
other or bypass duplicate checks (update renameCategory to check existence via
an atomic filter or use an $elemMatch-based conditional update).
---
Duplicate comments:
In `@packages/models/src/models/UserRoomCategories.ts`:
- Around line 47-75: The addRoomToCategory method currently reads the whole
doc.categories, mutates it in memory and writes back the entire array which
risks lost concurrent updates; change it to use targeted atomic updates instead:
use updateOne with $pull to remove roomId from any category.roomIds that contain
it and $addToSet with arrayFilters or the positional operator to add the
trimmedRoomId only to the target category (match by userId and category name)
without replacing the full categories array. Locate addRoomToCategory plus the
other category-mutating methods referenced and replace their read-modify-write
logic (the findByUserId → updateOne pattern) with equivalent atomic Mongo
updates using $addToSet, $pull and arrayFilters/positional updates against the
categories field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10c6dabc-9472-4fcc-844d-a7b1918e7382
📒 Files selected for processing (9)
apps/meteor/app/api/server/v1/user-room-categories.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsxapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/sidebar/hooks/useRoomList.tspackages/model-typings/src/models/IUserRoomCategoriesModel.tspackages/models/src/models/UserRoomCategories.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). (1)
- GitHub Check: cubic · AI code reviewer
🧰 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/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsxapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.ts
🧠 Learnings (9)
📚 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:
packages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.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:
packages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/model-typings/src/models/IUserRoomCategoriesModel.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsxapps/meteor/app/api/server/v1/user-room-categories.tspackages/models/src/models/UserRoomCategories.ts
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/hooks/useRoomList.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/sidebar/hooks/useRoomList.ts
📚 Learning: 2026-04-14T21:10:31.855Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36292
File: apps/meteor/client/hooks/useHasValidLocationHash.ts:7-12
Timestamp: 2026-04-14T21:10:31.855Z
Learning: When reviewing files in apps/meteor/client/hooks/, do not treat JSDoc-style comments on React hooks (especially exported hooks) as a violation of any “avoid code comments in implementation” guideline. It’s acceptable to use JSDoc to document the public API of exported hooks (e.g., parameter/return types, intended usage), as long as it documents behavior/contracts rather than adding narrative implementation comments.
Applied to files:
apps/meteor/client/hooks/useRoomMenuActions.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsxapps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsxapps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsxapps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/user-room-categories.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/user-room-categories.ts
🔇 Additional comments (8)
packages/model-typings/src/models/IUserRoomCategoriesModel.ts (1)
13-13: LGTM!apps/meteor/client/hooks/useRoomMenuActions.ts (1)
119-119: LGTM!apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateUserRoomCategoryModal.tsx (1)
53-53: LGTM!apps/meteor/client/hooks/useUserRoomCategoryForRoomModal.tsx (1)
76-76: LGTM!Also applies to: 92-92
apps/meteor/client/sidebar/RoomList/UserRoomCategoryGroupMenu.tsx (1)
25-25: LGTM!apps/meteor/client/sidebar/hooks/useRoomList.ts (1)
4-4: LGTM!Also applies to: 39-39, 184-184
apps/meteor/client/sidebar/RoomList/RenameUserRoomCategoryModal.tsx (1)
50-50: LGTM!apps/meteor/app/api/server/v1/user-room-categories.ts (1)
153-153: LGTM!Also applies to: 177-177
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.
Backend:
Frontend:
Proposed changes (including videos or screenshots)
Issue(s)
#38663
Steps to test or reproduce
Further comments
Summary by CodeRabbit