fix: quote has no effect on older thread messages - #7535
Conversation
GitHub Actions
Error
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
WalkthroughThread-aware quoting now passes ChangesThread-aware quoting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MessageComposer
participant prepareQuoteMessage
participant getMessageById
participant getThreadMessageById
MessageComposer->>prepareQuoteMessage: pass selected messages and tmid
prepareQuoteMessage->>getMessageById: load message with tmid
getMessageById->>getThreadMessageById: check thread message
getThreadMessageById-->>getMessageById: return thread message or no result
getMessageById-->>prepareQuoteMessage: return quoted message
prepareQuoteMessage-->>MessageComposer: prepare quote
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/containers/MessageComposer/hooks/useMessage.ts (1)
7-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTrack
tmidin the effect lifecycle.The effect closes over
tmid, but Line 19 does not include it in the dependency list. If the room thread scope changes after mount, the hook does not reload the message. Guard the state update with a cleanup flag so an earlier lookup cannot overwrite a newer result.Proposed fix
useEffect(() => { + let isCurrent = true; const load = async () => { const result = await getMessageById(messageId, tmid); - if (result) { + if (isCurrent && result) { setMessage(result); } }; load(); - }, [messageId]); + return () => { + isCurrent = false; + }; + }, [messageId, tmid]);🤖 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 `@app/containers/MessageComposer/hooks/useMessage.ts` around lines 7 - 19, Update the useMessage effect to include tmid in its dependency list and add a cleanup flag that prevents an earlier getMessageById lookup from calling setMessage after the effect is superseded. Preserve updates for the currently active messageId and thread scope.Sources: Coding guidelines, Linters/SAST tools
🤖 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 `@app/lib/database/services/Message.ts`:
- Around line 8-18: Update getMessageById to reject only a missing messageId,
allowing calls without tmid to continue to the main messages collection lookup.
Keep getThreadMessageById conditional on tmid being present, then preserve the
existing main-collection fallback for non-thread messages.
---
Outside diff comments:
In `@app/containers/MessageComposer/hooks/useMessage.ts`:
- Around line 7-19: Update the useMessage effect to include tmid in its
dependency list and add a cleanup flag that prevents an earlier getMessageById
lookup from calling setMessage after the effect is superseded. Preserve updates
for the currently active messageId and thread scope.
🪄 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 Plus
Run ID: 8c03ad2b-2b73-4ea6-ba10-9f8479da35dd
📒 Files selected for processing (8)
app/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.tsapp/containers/MessageComposer/hooks/useMessage.tsapp/lib/database/services/Message.test.tsapp/lib/database/services/Message.tsapp/lib/methods/getPermalinks.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/hooks/useMessage.tsapp/containers/MessageComposer/MessageComposer.tsxapp/lib/database/services/Message.test.tsapp/lib/methods/getPermalinks.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.tsapp/lib/database/services/Message.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/hooks/useMessage.tsapp/containers/MessageComposer/MessageComposer.tsxapp/lib/database/services/Message.test.tsapp/lib/methods/getPermalinks.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.tsapp/lib/database/services/Message.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Before committing changes to JavaScript or TypeScript files, runpnpm prettier-lintandTZ=UTC pnpm testfor the modified files.
Use the local-first data flow: the UI reads from WatermelonDB, while sagas synchronize data with the server.
Use Redux and Redux-Saga for global or server state, and use Zustand for feature-local stores; do not assume all state is in Redux.
Files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/hooks/useMessage.tsapp/containers/MessageComposer/MessageComposer.tsxapp/lib/database/services/Message.test.tsapp/lib/methods/getPermalinks.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.tsapp/lib/database/services/Message.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.ts
🧠 Learnings (3)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/hooks/useMessage.tsapp/containers/MessageComposer/MessageComposer.tsxapp/lib/database/services/Message.test.tsapp/lib/methods/getPermalinks.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.tsapp/lib/database/services/Message.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.ts
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/MessageComposer/MessageComposer.tsx
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/lib/database/services/Message.test.tsapp/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts
🪛 React Doctor (0.9.1)
app/containers/MessageComposer/hooks/useMessage.ts
[error] 11-11: This setter runs after await, so overlapping re-runs of the effect can resolve out of order and write stale state; gate it behind a cancellation/ignore flag or return a cleanup that cancels the work.
In a useEffect whose dependencies can change, guard any setter call that runs after an await behind a cancellation/ignore flag, or return a cleanup that cancels the async work.
(no-set-state-after-await-in-effect)
🔇 Additional comments (6)
app/lib/database/services/Message.test.ts (1)
32-100: LGTM!app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
16-17: LGTM!app/containers/MessageComposer/helpers/prepareQuoteMessage.ts (1)
6-14: LGTM!app/lib/methods/getPermalinks.ts (1)
2-2: LGTM!Also applies to: 21-21
app/containers/MessageComposer/helpers/prepareQuoteMessage.test.ts (1)
84-84: LGTM!Also applies to: 165-204
app/containers/MessageComposer/MessageComposer.tsx (1)
128-128: LGTM!Also applies to: 152-152
GitHub Actions
Inactive
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/lib/database/services/Message.ts (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
getMessageById.The function declares parameter types but relies on inference for its exported return type. Add the concrete
Promise<... | null>type that covers both message-model results.As per coding guidelines,
**/*.{ts,tsx}requires explicit type annotations for function parameters and return types.🤖 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 `@app/lib/database/services/Message.ts` at line 8, Update the exported getMessageById function with an explicit Promise return type covering both possible message-model result types and null, while preserving its existing behavior and parameter types.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@app/lib/database/services/Message.ts`:
- Line 8: Update the exported getMessageById function with an explicit Promise
return type covering both possible message-model result types and null, while
preserving its existing behavior and parameter types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 78fedddf-d8c0-4725-9873-529ed4421d1f
📒 Files selected for processing (1)
app/lib/database/services/Message.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/database/services/Message.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/database/services/Message.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Before committing changes to JavaScript or TypeScript files, runpnpm prettier-lintandTZ=UTC pnpm testfor the modified files.
Use the local-first data flow: the UI reads from WatermelonDB, while sagas synchronize data with the server.
Use Redux and Redux-Saga for global or server state, and use Zustand for feature-local stores; do not assume all state is in Redux.
Files:
app/lib/database/services/Message.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/database/services/Message.ts
🔇 Additional comments (2)
app/lib/database/services/Message.ts (2)
9-18: LGTM!
8-18: 📐 Maintainability & Code QualityConfirm the required TypeScript validation commands.
Run
pnpm prettier-lintandTZ=UTC pnpm testfor the modified files before commit.As per coding guidelines,
**/*.{js,jsx,ts,tsx}requires both commands before committing changes.Source: Coding guidelines
GitHub Actions
Inactive
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.75.0.109471 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNQEp3T7dOmpxE8rSBxyXhNBgGhWPC9Mp-A1Kf-Izsg_2V74D4kDeBGn_sEzfDLpzDarDC7pQppdEhH6zAoi |
|
iOS Build Available Rocket.Chat 4.75.0.109473 |
GitHub Actions
Error
GitHub Actions
Error
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.maestro/tests/room/quote-thread-message.yaml (1)
29-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse contains patterns for all Maestro selectors.
Replace each exact
idortextselector with.*value.*. Add the missing leading.*tocomposer-quote-.*andcomposer-quote-remove-.*.This makes the flow consistent with the Maestro selector convention and prevents failures when the rendered value has additional text.
Example change
- tapOn: - id: 'room-view-messages' + id: '.*room-view-messages.*'Based on learnings, use
.*keyword.*for all Maestrotextandidselectors.Also applies to: 69-77, 81-110
🤖 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 @.maestro/tests/room/quote-thread-message.yaml around lines 29 - 50, Update all Maestro selectors in the quote-thread flow, including the referenced later ranges, to use contains patterns by wrapping every id and text value with .* on both sides. Ensure composer-quote-.* and composer-quote-remove-.* include the missing leading wildcard, while preserving the existing actions and flow.Source: Learnings
🤖 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.
Nitpick comments:
In @.maestro/tests/room/quote-thread-message.yaml:
- Around line 29-50: Update all Maestro selectors in the quote-thread flow,
including the referenced later ranges, to use contains patterns by wrapping
every id and text value with .* on both sides. Ensure composer-quote-.* and
composer-quote-remove-.* include the missing leading wildcard, while preserving
the existing actions and flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c4ef10b2-d107-4270-90ff-f9a638e556ab
📒 Files selected for processing (1)
.maestro/tests/room/quote-thread-message.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-05T14:28:10.004Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6997
File: .maestro/tests/room/message-markdown-click.yaml:28-39
Timestamp: 2026-03-05T14:28:10.004Z
Learning: In Maestro YAML selector fields (text, id) within the Rocket.Chat React Native repository, use the contains pattern '.*keyword.*' (leading and trailing '.*') for matching text. The pattern '.*keyword*.' is incorrect and will fail to match cases where the keyword appears at the end of the element's text. This guideline applies to all Maestro YAML selector fields across the codebase.
Applied to files:
.maestro/tests/room/quote-thread-message.yaml
📚 Learning: 2026-03-17T19:15:26.536Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:26.536Z
Learning: In YAML test files under .maestro/tests/room, use tapping the empty area (e.g., tapOn: point: 5%,10%) to dismiss both the bottom sheet and keyboard when needed. Do not rely on action-sheet-handle alone if the keyboard also needs to be dismissed in the same step. This pattern is acceptable for tests where a single tap should close both UI elements.
Applied to files:
.maestro/tests/room/quote-thread-message.yaml
🔇 Additional comments (1)
.maestro/tests/room/quote-thread-message.yaml (1)
1-22: LGTM!Also applies to: 53-66
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Waiting
GitHub Actions
Error
GitHub Actions
Waiting
GitHub Actions
Waiting
1 participant
Proposed changes
Quoting a message inside a thread only worked for the newest handful of replies (~5–7). For anything older, tapping Quote did nothing: no preview in the composer, and no quote in the sent message. It affected both your own messages and other users'.
The cause is that thread replies are persisted in the thread_messages table, while the quote flow resolved ids through getMessageById, which only reads messages. A reply lands in messages only if it happened to arrive via the room stream, so which messages were quotable depended on how the room had been loaded rather than on anything the user did.
getMessageById now accepts an optional tmid and prefers thread_messages when it's set, falling back to messages so the thread's parent message still resolves. The composer passes tmid through to both the preview (Quote.tsx → useMessage) and the outgoing message (prepareQuoteMessage), including the attachment-caption path. getPermalinkMessage was widened from TMessageModel to TAnyMessageModel — a parameter-type widening only, no runtime change. All four code paths that create a thread_messages row set subscription_id to the room's rid, so permalinks resolve to the room and not the thread.
Issue(s)
https://rocketchat.atlassian.net/browse/SUP-1092
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Tests