Conversation
|
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 |
🦋 Changeset detectedLatest commit: dd3e4a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR implements per-room message draft persistence with a "Drafts" sidebar section. It adds a new API endpoint for server-side draft storage, refactors the composer to use injectable draft persistence callbacks, introduces a draft management hook, and gates the sidebar drafts section behind a feature preview toggle. Changes
Sequence DiagramsequenceDiagram
actor User
participant Composer as Message Composer
participant useDraft as useDraft Hook
participant LocalStorage as Local Storage
participant Server as /v1/subscriptions.saveDraft
participant DB as Database
User->>Composer: Types message in room
Composer->>useDraft: persistLocal(draftText)
useDraft->>useDraft: Update draftRef
useDraft->>LocalStorage: Save draft to local storage
User->>Composer: Switches rooms / closes tab
Composer->>useDraft: flushDraft()
useDraft->>Server: POST { rid, draft }
Server->>DB: updateDraftById(subscriptionId, draft)
DB-->>Server: Success
Server-->>useDraft: Response
useDraft->>useDraft: Clear pending draft flag
User->>Composer: Returns to same room
useDraft->>LocalStorage: Read local draft (fallback)
LocalStorage-->>useDraft: Initial value
useDraft-->>Composer: Restore draft text
Composer-->>User: Display draft in textarea
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38225 +/- ##
===========================================
+ Coverage 70.73% 70.84% +0.10%
===========================================
Files 3195 3209 +14
Lines 113105 113522 +417
Branches 20510 20572 +62
===========================================
+ Hits 80010 80422 +412
- Misses 31049 31050 +1
- Partials 2046 2050 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
e8ba5fd to
e3bc7b8
Compare
97440d3 to
4f1855b
Compare
There was a problem hiding this comment.
1 issue found across 7 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="apps/meteor/server/settings/accounts.ts">
<violation number="1" location="apps/meteor/server/settings/accounts.ts:752">
P3: Add an i18n entry for the new `Drafts` sidebar section; otherwise locales without a translation will show the raw key.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/client/sidebar/hooks/useDraftRoomIds.ts`:
- Around line 61-64: The storage event handler (handleStorageChange) misses
same-tab writes because window "storage" doesn't fire in the writing document;
update useDraftRoomIds to also listen for a same-document custom event (e.g.,
"messagebox:draft:updated") or subscribe to the
composer/Accounts.storageLocation notification API and call callback() when that
event is received; additionally, when drafts are written via
Accounts.storageLocation.setItem (or composer API), emit the same custom event
so the active tab's handler (handleStorageChange) can pick up updates and keep
sidebar draft indicators in sync.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/meteor/public/images/featurePreview/sidebar-drafts.pngis excluded by!**/*.png
📒 Files selected for processing (6)
.changeset/red-maps-wink.mdapps/meteor/client/sidebar/hooks/useDraftRoomIds.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/server/settings/accounts.tspackages/i18n/src/locales/en.i18n.jsonpackages/ui-client/src/hooks/useFeaturePreviewList.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). (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/client/sidebar/hooks/useDraftRoomIds.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/server/settings/accounts.ts
🧠 Learnings (14)
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: Rocket.Chat repo context: When a workspace manifest on develop already pins a dependency version (e.g., packages/web-ui-registration → "rocket.chat/ui-contexts": "27.0.1"), a lockfile change in a feature PR that upgrades only that dependency’s resolution is considered a manifest-driven sync and can be kept, preferably as a small "chore: sync yarn.lock with manifests" commit.
Applied to files:
.changeset/red-maps-wink.md
📚 Learning: 2026-02-24T19:09:09.561Z
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:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
.changeset/red-maps-wink.md
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
.changeset/red-maps-wink.mdpackages/i18n/src/locales/en.i18n.json
📚 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:
.changeset/red-maps-wink.mdapps/meteor/client/sidebar/hooks/useDraftRoomIds.ts
📚 Learning: 2026-02-24T19:05:56.710Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 0
File: :0-0
Timestamp: 2026-02-24T19:05:56.710Z
Learning: In Rocket.Chat PRs, keep feature PRs free of unrelated lockfile-only dependency bumps; prefer reverting lockfile drift or isolating such bumps into a separate "chore" commit/PR, and always use yarn install --immutable with the Yarn version pinned in package.json via Corepack.
Applied to files:
.changeset/red-maps-wink.md
📚 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:
.changeset/red-maps-wink.mdpackages/i18n/src/locales/en.i18n.json
📚 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/client/sidebar/hooks/useDraftRoomIds.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/client/sidebar/hooks/useDraftRoomIds.tsapps/meteor/server/settings/accounts.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/sidebar/hooks/useDraftRoomIds.tsapps/meteor/client/sidebar/hooks/useRoomList.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/client/sidebar/hooks/useDraftRoomIds.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/server/settings/accounts.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/client/sidebar/hooks/useDraftRoomIds.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/server/settings/accounts.ts
📚 Learning: 2026-01-19T18:08:05.314Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38169
File: packages/ui-client/src/hooks/useGoToDirectMessage.ts:20-27
Timestamp: 2026-01-19T18:08:05.314Z
Learning: Rule of Hooks: In React, hooks (including custom hooks like useGoToDirectMessage and useUserSubscriptionByName) must be called unconditionally and in the same order on every render. Do not conditionally call hooks based on values; instead, derive safe inputs if a value may be undefined (e.g., pass an empty string or undefined) and handle variations in logic outside the hook invocation. This preserves hook order and avoids violations. Apply this guideline to all files under packages/ui-client/src/hooks where hooks are used.
Applied to files:
packages/ui-client/src/hooks/useFeaturePreviewList.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/client/sidebar/hooks/useRoomList.ts
📚 Learning: 2025-11-17T22:38:48.631Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37505
File: packages/i18n/src/locales/en.i18n.json:3765-3765
Timestamp: 2025-11-17T22:38:48.631Z
Learning: Rocket.Chat i18n copy: Keep sentence case for the value of "Notification_Desktop_show_voice_calls" in packages/i18n/src/locales/en.i18n.json (“Show desktop notifications for voice calls”) per design directive; do not change to Title Case even if nearby labels differ.
Applied to files:
packages/i18n/src/locales/en.i18n.json
🧬 Code graph analysis (1)
apps/meteor/client/sidebar/hooks/useRoomList.ts (1)
apps/meteor/client/sidebar/hooks/useDraftRoomIds.ts (1)
useDraftRoomIds(15-77)
🔇 Additional comments (5)
apps/meteor/server/settings/accounts.ts (1)
746-759: Looks good:Draftsis in the correct default position.Putting
'Drafts'after'Unread'and before'Favorites'matches the intended priority behavior for sidebar grouping.packages/i18n/src/locales/en.i18n.json (1)
1860-1861: Looks good — keys are correctly added for the feature preview copy.Line 1860 and Line 1861 add a complete label/description pair with consistent key naming and appropriate user-facing text.
Based on learnings: In this repository, new translation keys should be added to
packages/i18n/src/locales/en.i18n.jsononly; other locale files are populated via the external translation pipeline and/or fall back to English..changeset/red-maps-wink.md (1)
1-8: Changeset entry is clear and release-scoped.Package bump declarations and the feature-preview note are consistent with the PR scope.
packages/ui-client/src/hooks/useFeaturePreviewList.ts (1)
3-3: Feature preview registration forsidebarDraftsis wired correctly.The new union value and default preview entry are internally consistent and ready to be consumed by feature-preview consumers.
Also applies to: 31-39
apps/meteor/client/sidebar/hooks/useRoomList.ts (1)
88-98: Draft precedence logic matches the required grouping priority.The new flow correctly places rooms in
Unreadfirst, thenDrafts, thenFavorites, and only materializes theDraftsgroup when enabled and non-empty.Also applies to: 136-136
There was a problem hiding this comment.
1 issue found across 11 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/ui-message/client/messageBox/createComposerAPI.ts">
<violation number="1" location="apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts:275">
P2: The `ready` guard is ineffective with debouncing: the initialization `input` event is queued while `ready` is false, but executes after `ready = true`, so `initialDraft` is still persisted.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| skipFocus: true, | ||
| }); | ||
|
|
||
| ready = true; |
There was a problem hiding this comment.
P2: The ready guard is ineffective with debouncing: the initialization input event is queued while ready is false, but executes after ready = true, so initialDraft is still persisted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts, line 275:
<comment>The `ready` guard is ineffective with debouncing: the initialization `input` event is queued while `ready` is false, but executes after `ready = true`, so `initialDraft` is still persisted.</comment>
<file context>
@@ -269,10 +268,12 @@ export const createComposerAPI = (
skipFocus: true,
});
+ ready = true;
+
// Gets the text that is connected to the cursor and replaces it with the given text
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts (1)
39-44: Initializereadyafter draft setup to prevent unnecessary deferred persistence.The
persistcallback uses trailing debounce (defaultimmediate: false), so callingsetText(initialDraft)during initialization triggers an 'input' event that schedules a debouncedpersistDraft()to execute 300ms later—afterready = trueis set. This causes unnecessary draft writes for initialization-time state changes rather than user edits.Move
ready = truebeforesetText(initialDraft)or cancel the debounced call during initialization:Current flow (lines 39–275)
let ready = false; const persist = withDebouncing({ wait: 300 })(() => { if (ready) { persistDraft(input.value); } }); input.addEventListener('input', persist); // ... setText(initialDraft, { skipFocus: true }); // triggers 'input' event → schedules persist ready = true; // now ready flag is true when debounced persist executes🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts` around lines 39 - 44, The debounced persist callback (created via withDebouncing) can be scheduled by the initialization call setText(initialDraft) because ready is set to true afterwards; to fix, ensure ready is true before any initialization that triggers input events or temporarily disable/cancel the debounced handler around initialization: specifically, either move ready = true to occur before calling setText(initialDraft) (and keep input.addEventListener('input', persist) as-is) or call persist.cancel() (or a provided cancel API from withDebouncing) before setText(initialDraft) and re-enable it after; update references in this file to ready, persist, withDebouncing, input.addEventListener, setText, and persistDraft accordingly.
🤖 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/client/views/room/composer/messageBox/hooks/useDraft.ts`:
- Around line 9-10: initialValueRef is initialized once with useRef(serverDraft
|| localDraft) inside the useDraft hook and therefore becomes stale when tmid
changes without remounting (ComposerMessage is keyed by room._id); update
initialValueRef.current whenever the inputs change by adding a useEffect in
useDraft that sets initialValueRef.current = serverDraft || localDraft and also
reset draftRef.current (if applicable) when serverDraft, localDraft or tmid
change so the composer re-initializes to the correct thread draft.
---
Nitpick comments:
In `@apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts`:
- Around line 39-44: The debounced persist callback (created via withDebouncing)
can be scheduled by the initialization call setText(initialDraft) because ready
is set to true afterwards; to fix, ensure ready is true before any
initialization that triggers input events or temporarily disable/cancel the
debounced handler around initialization: specifically, either move ready = true
to occur before calling setText(initialDraft) (and keep
input.addEventListener('input', persist) as-is) or call persist.cancel() (or a
provided cancel API from withDebouncing) before setText(initialDraft) and
re-enable it after; update references in this file to ready, persist,
withDebouncing, input.addEventListener, setText, and persistDraft accordingly.
🪄 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: ce37be42-c870-4f07-a1c1-130e390904fb
📒 Files selected for processing (10)
apps/meteor/app/api/server/v1/subscriptions.tsapps/meteor/app/ui-message/client/messageBox/createComposerAPI.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.tsapps/meteor/lib/publishFields.tspackages/core-typings/src/ISubscription.tspackages/model-typings/src/models/ISubscriptionsModel.tspackages/models/src/models/Subscriptions.tspackages/rest-typings/src/v1/subscriptionsEndpoints.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core-typings/src/ISubscription.ts
- apps/meteor/lib/publishFields.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). (2)
- GitHub Check: cubic · AI code reviewer
- 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:
packages/model-typings/src/models/ISubscriptionsModel.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/app/api/server/v1/subscriptions.tspackages/models/src/models/Subscriptions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/ui-message/client/messageBox/createComposerAPI.tspackages/rest-typings/src/v1/subscriptionsEndpoints.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.ts
🧠 Learnings (19)
📚 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/ISubscriptionsModel.tsapps/meteor/app/api/server/v1/subscriptions.tspackages/models/src/models/Subscriptions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/ui-message/client/messageBox/createComposerAPI.tspackages/rest-typings/src/v1/subscriptionsEndpoints.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.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/ISubscriptionsModel.tsapps/meteor/app/api/server/v1/subscriptions.tspackages/models/src/models/Subscriptions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/ui-message/client/messageBox/createComposerAPI.tspackages/rest-typings/src/v1/subscriptionsEndpoints.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.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/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.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/views/room/composer/messageBox/MessageBox.tsx
📚 Learning: 2026-03-12T10:26:26.697Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39340
File: apps/meteor/app/api/server/v1/im.ts:1349-1398
Timestamp: 2026-03-12T10:26:26.697Z
Learning: In `apps/meteor/app/api/server/v1/im.ts` (PR `#39340`), the `DmEndpoints` type intentionally includes temporary stub entries for `/v1/im.kick`, `/v1/dm.kick`, `/v1/im.leave`, and `/v1/dm.leave` (using `DmKickProps` and `DmLeaveProps`) even though no route handlers exist for them yet. These stubs were added to preserve type compatibility after removing the original `DmLeaveProps` and related files. They are planned for cleanup in a follow-up PR. Do not flag these as missing implementations when reviewing this file until the follow-up is merged.
Applied to files:
apps/meteor/app/api/server/v1/subscriptions.tspackages/rest-typings/src/v1/subscriptionsEndpoints.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.ts
📚 Learning: 2026-03-14T14:58:58.834Z
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Applied to files:
apps/meteor/app/api/server/v1/subscriptions.ts
📚 Learning: 2026-03-20T13:52:29.575Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/api/server/v1/stats.ts:98-117
Timestamp: 2026-03-20T13:52:29.575Z
Learning: In `apps/meteor/app/api/server/v1/stats.ts`, the `statistics.telemetry` POST endpoint intentionally has no `body` AJV schema in its route options. The proper request body shape (a `params` array of telemetry event objects) has not been formally defined yet, so body validation is deferred to a follow-up. Do not flag the missing body schema for this endpoint during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/subscriptions.tspackages/rest-typings/src/v1/subscriptionsEndpoints.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
Applied to files:
apps/meteor/app/api/server/v1/subscriptions.tspackages/rest-typings/src/v1/subscriptionsEndpoints.ts
📚 Learning: 2026-03-16T21:50:42.118Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Applied to files:
apps/meteor/app/api/server/v1/subscriptions.tspackages/rest-typings/src/v1/subscriptionsEndpoints.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/app/api/server/v1/subscriptions.tsapps/meteor/client/sidebar/hooks/useRoomList.tsapps/meteor/app/ui-message/client/messageBox/createComposerAPI.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/subscriptions.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/subscriptions.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/client/sidebar/hooks/useRoomList.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/sidebar/hooks/useRoomList.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.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/ui-message/client/messageBox/createComposerAPI.ts
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/app/ui-message/client/messageBox/createComposerAPI.tsapps/meteor/client/views/room/composer/messageBox/hooks/useDraft.ts
📚 Learning: 2026-02-24T19:16:35.307Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 39003
File: apps/meteor/client/lib/chats/flows/sendMessage.ts:39-45
Timestamp: 2026-02-24T19:16:35.307Z
Learning: In apps/meteor/client/lib/chats/flows/sendMessage.ts, when sdk.call('sendMessage', ...) throws an error, the message is intentionally left with temp: true (not deleted or cleaned up) to support a future retry UI feature. This allows users to retry sending failed messages rather than losing them.
Applied to files:
apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts
📚 Learning: 2026-03-10T08:13:52.153Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39414
File: apps/meteor/app/api/server/v1/rooms.ts:1241-1297
Timestamp: 2026-03-10T08:13:52.153Z
Learning: In the RocketChat/Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1/rooms.ts, the pattern `ajv.compile<void>({...})` is intentionally used for the 200 response schema even when the endpoint returns `{ success: true }`. This is an established convention across all migrated endpoints (rooms.leave, rooms.favorite, rooms.delete, rooms.muteUser, rooms.unmuteUser). Do not flag this as a type mismatch during reviews of these migration PRs.
Applied to files:
packages/rest-typings/src/v1/subscriptionsEndpoints.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/subscriptionsEndpoints.ts
🔇 Additional comments (6)
packages/models/src/models/Subscriptions.ts (1)
719-725: Draft field set/unset logic looks correct.The method cleanly handles both persistence (
$set) and clear ($unset) paths fordraft.apps/meteor/client/sidebar/hooks/useRoomList.ts (1)
89-91: Drafts grouping precedence is correctly wired.
Unreadis evaluated beforeDrafts, andDraftsbeforeFavorites, matching the required sidebar precedence while staying feature-preview gated.Also applies to: 133-134
packages/rest-typings/src/v1/subscriptionsEndpoints.ts (1)
109-126: Endpoint typing + validator addition is consistent.The new request shape and route entry for
'/v1/subscriptions.saveDraft'are aligned and well-scoped.Also applies to: 149-151
packages/model-typings/src/models/ISubscriptionsModel.ts (1)
154-154: Model typing extension is aligned with implementation.The new
updateDraftByIdcontract is consistent with the model layer and API usage.apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (1)
130-132: Composer integration with draft hook is cleanly connected.The new
useDraftinputs/outputs are correctly threaded into composer initialization and teardown paths.Also applies to: 136-145
apps/meteor/app/api/server/v1/subscriptions.ts (1)
121-143:subscriptions.saveDraftroute is safely scoped.Auth, payload validation, and per-user subscription ownership checks are correctly applied before persistence.
| const initialValueRef = useRef(serverDraft || localDraft); | ||
| const draftRef = useRef<string | null>(null); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate MessageBox usage and verify whether remount is forced via a room/thread-based key.
rg -n --type=tsx '<MessageBox\b|MessageBox\(' apps/meteor -C3
rg -n --type=tsx 'key=\{[^}]*(_id|rid|tmid)' apps/meteor -C2
# 2) Inspect where useRoomSubscription is consumed to confirm whether subscription can be undefined initially then populated.
rg -n --type=tsx '\buseRoomSubscription\s*\(' apps/meteor -C3
# 3) Confirm all useDraft call sites and their argument sources.
rg -n --type=ts --type=tsx '\buseDraft\s*\(' apps/meteor packages -C3Repository: RocketChat/Rocket.Chat
Length of output: 93
🏁 Script executed:
# 1) Locate MessageBox usage and verify whether remount is forced via a room/thread-based key.
rg -n -t ts -t js 'MessageBox' apps/meteor/client/views/room --max-count=20 -A 3 -B 1
# 2) Find and read the useDraft.ts hook implementation
cat -n apps/meteor/client/views/room/composer/messageBox/hooks/useDraft.ts
# 3) Locate useDraft call sites
rg -n -t ts -t js 'useDraft\s*\(' apps/meteor packages -C 3
# 4) Find MessageBox component definition
find apps/meteor -name 'MessageBox*' -type fRepository: RocketChat/Rocket.Chat
Length of output: 32226
🏁 Script executed:
# Check if there are any useEffect dependencies on tmid that reset the composer state
rg -n -t ts -t js 'useEffect.*tmid|tmid.*useEffect' apps/meteor/client/views/room/composer --max-count=20 -A 5
# Check MessageBox component for any useEffect that resets state when tmid changes
sed -n '97,200p' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx | head -100
# Check if ChatContext or chat.composer handles thread changes
rg -n -t ts -t js 'chat\.composer.*tmid|setComposer.*tmid' apps/meteor/client/views/room --max-count=10 -B 2 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 2996
🏁 Script executed:
# Check what happens when tmid changes - look for any state reset logic
sed -n '97,400p' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx | grep -n "tmid\|useEffect\|composer"
# Check createComposerAPI to see how initialValue is used
rg -n 'createComposerAPI' apps/meteor --max-count=5 -A 20 -B 2Repository: RocketChat/Rocket.Chat
Length of output: 9446
🏁 Script executed:
# Check if there's any cleanup or reset logic when tmid changes in chat.composer or ChatContext
rg -n 'setComposerAPI\|clear.*composer\|reset.*composer' apps/meteor/client/views/room/contexts/ChatContext.ts -A 5 -B 5
# Check if there's a useEffect that handles room or thread changes to reset composer
sed -n '1,500p' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx | grep -n "useEffect\|cleanup\|return"
# Verify where chat.composer gets cleared/reset when navigating between threads
rg -n 'chat\.setComposerAPI\|composer.*clear' apps/meteor/client/views/room -A 3 -B 1 | head -50Repository: RocketChat/Rocket.Chat
Length of output: 996
🏁 Script executed:
# Look at the full ChatContext implementation to understand composer lifecycle
cat -n apps/meteor/client/views/room/contexts/ChatContext.ts | head -100
# Check if there's logic that clears composer state when switching threads
rg -n 'clearComposerAPI\|setComposerAPI\|null' apps/meteor/client/views/room/contexts/ChatContext.ts -B 2 -A 2
# Look for where initialValue is actually used in the composer
cat -n apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts | sed -n '1,80p'Repository: RocketChat/Rocket.Chat
Length of output: 3343
🏁 Script executed:
# Continue reading createComposerAPI to see how initialDraft is used
cat -n apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts | sed -n '80,180p'
# Search for where the initialDraft parameter is applied to the input
rg -n 'initialDraft\|input\.value\s*=' apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts -A 2 -B 2Repository: RocketChat/Rocket.Chat
Length of output: 2947
🏁 Script executed:
# Search for initialDraft usage - it should be set somewhere on the input
rg -n 'initialDraft' apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts
# Look at the return statement of createComposerAPI to see the full API
cat -n apps/meteor/app/ui-message/client/messageBox/createComposerAPI.ts | tail -100Repository: RocketChat/Rocket.Chat
Length of output: 3241
🏁 Script executed:
# Check the full callbackRef useCallback and see what happens when it's recreated
sed -n '130,160p' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
# Check if there's any useEffect that monitors tmid/room changes and resets composer state
sed -n '200,300p' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx | grep -n "useEffect\|tmid"
# Search for any cleanup when MessageBox unmounts or when room/tmid changes
rg -n 'useEffect.*\[\]|useEffect.*\[room\|useEffect.*\[tmid' apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx -A 8Repository: RocketChat/Rocket.Chat
Length of output: 967
🏁 Script executed:
# Check ComposerMessage more carefully - is MessageBox keyed by both room AND tmid?
sed -n '80,100p' apps/meteor/client/views/room/composer/ComposerMessage.tsx
# Look for any other MessageBox usages to see if they use keys
rg -n '<MessageBox' apps/meteor/client/views/room -B 2 -A 2Repository: RocketChat/Rocket.Chat
Length of output: 10716
Stale initialValue when switching threads without room change.
useRef(serverDraft || localDraft) on line 9 initializes once. When tmid changes within the same room (thread switching), MessageBox is NOT remounted because it's keyed only by room._id (see ComposerMessage.tsx:93). The useDraft hook receives new parameters, but initialValueRef.current never updates. This causes the composer to initialize with the previous thread's draft value instead of the current thread's draft.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/views/room/composer/messageBox/hooks/useDraft.ts` around
lines 9 - 10, initialValueRef is initialized once with useRef(serverDraft ||
localDraft) inside the useDraft hook and therefore becomes stale when tmid
changes without remounting (ComposerMessage is keyed by room._id); update
initialValueRef.current whenever the inputs change by adding a useEffect in
useDraft that sets initialValueRef.current = serverDraft || localDraft and also
reset draftRef.current (if applicable) when serverDraft, localDraft or tmid
change so the composer re-initializes to the correct thread draft.
Proposed changes (including videos or screenshots)
Important
This change is under feature preview
Issue(s)
Closes #38217
Steps to test or reproduce
Further comments
CORE-1747
Summary by CodeRabbit