fix: show discussion names in quoted, forwarded and thread start messages - #7566
fix: show discussion names in quoted, forwarded and thread start messages#7566Rohit3523 wants to merge 5 commits into
Conversation
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-04-30T17:07:51.020ZApplied to files:
📚 Learning: 2026-06-25T18:37:25.526ZApplied to files:
🔇 Additional comments (1)
WalkthroughThe PR adds channel mention formatting that prefers ChangesChannel mention rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant Description
participant useQuotedMessageChannels
participant getMessageIdFromPermalink
participant getMessageById
participant formatChannelMentions
participant MarkdownPreview
Description->>useQuotedMessageChannels: Load channels for quoted message link
useQuotedMessageChannels->>getMessageIdFromPermalink: Extract msg parameter
useQuotedMessageChannels->>getMessageById: Fetch quoted message
Description->>formatChannelMentions: Format quoted text with channels
formatChannelMentions-->>MarkdownPreview: Return channel-aware text
MarkdownPreview-->>Description: Render Markdown
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/lib/hooks/usePreviewFormatText/index.tsx (1)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
usePreviewFormatText.Declare the string result in the hook signature.
Proposed fix
-const usePreviewFormatText = (msg: string, channels?: TMentionableChannel[]) => { +const usePreviewFormatText = (msg: string, channels?: TMentionableChannel[]): string => {As per coding guidelines,
**/*.{ts,tsx}requires explicit parameter and return annotations.🤖 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/hooks/usePreviewFormatText/index.tsx` at line 8, Update the usePreviewFormatText hook signature to explicitly declare its string return type, while retaining the existing msg and channels parameter annotations and implementation behavior.Source: Coding guidelines
app/lib/methods/helpers/formatChannelMentions.ts (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an interface for
TMentionableChannel.This declaration defines an object shape. Replace the type alias with an interface.
Proposed fix
-export type TMentionableChannel = { name: string; fname?: string }; +export interface TMentionableChannel { + name: string; + fname?: string; +}As per coding guidelines,
**/*.{ts,tsx}requires interfaces for object shapes.🤖 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/methods/helpers/formatChannelMentions.ts` around lines 3 - 4, Replace the TMentionableChannel object-shape type alias with an interface, preserving its existing name, required name property, and optional fname property.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.
Inline comments:
In `@app/containers/message/hooks/useQuotedMessageChannels.ts`:
- Around line 22-32: Update the load function’s getMessageById lookup to handle
rejected promises with try/catch, preserving the existing isActive guard and
channel assignment on success. Leave channels undefined when the lookup or its
setup fails so the raw room ID remains visible, and ensure no unhandled
rejection escapes load.
---
Nitpick comments:
In `@app/lib/hooks/usePreviewFormatText/index.tsx`:
- Line 8: Update the usePreviewFormatText hook signature to explicitly declare
its string return type, while retaining the existing msg and channels parameter
annotations and implementation behavior.
In `@app/lib/methods/helpers/formatChannelMentions.ts`:
- Around line 3-4: Replace the TMentionableChannel object-shape type alias with
an interface, preserving its existing name, required name property, and optional
fname property.
🪄 Autofix
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: 7c96e65b-af98-455e-8dc8-ca0825c52fea
📒 Files selected for processing (21)
app/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.tsxapp/containers/markdown/interfaces.tsapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/components/Content/PreviewContent.tsxapp/containers/message/components/__tests__/Reply.test.tsxapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/hooks/useQuotedMessageChannels.tsapp/definitions/IMessage.tsapp/lib/hooks/usePreviewFormatText/index.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/lib/methods/helpers/formatChannelMentions.tsapp/lib/methods/helpers/getMessageIdFromPermalink.test.tsapp/lib/methods/helpers/getMessageIdFromPermalink.tsapp/views/ThreadMessagesView/Item.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: E2E Shard Preflight
🧰 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/markdown/interfaces.tsapp/containers/message/components/Content/PreviewContent.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/definitions/IMessage.tsapp/lib/methods/helpers/getMessageIdFromPermalink.tsapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/lib/hooks/usePreviewFormatText/index.tsxapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/hooks/useQuotedMessageChannels.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/components/__tests__/Reply.test.tsxapp/lib/methods/helpers/getMessageIdFromPermalink.test.tsapp/views/ThreadMessagesView/Item.tsxapp/lib/methods/helpers/formatChannelMentions.tsapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.tsx
**/*.{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/markdown/interfaces.tsapp/containers/message/components/Content/PreviewContent.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/definitions/IMessage.tsapp/lib/methods/helpers/getMessageIdFromPermalink.tsapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/lib/hooks/usePreviewFormatText/index.tsxapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/hooks/useQuotedMessageChannels.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/components/__tests__/Reply.test.tsxapp/lib/methods/helpers/getMessageIdFromPermalink.test.tsapp/views/ThreadMessagesView/Item.tsxapp/lib/methods/helpers/formatChannelMentions.tsapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.
Follow Oxlint rules configured in.oxlintrc.json, including the import, React, Jest, TypeScript, and React Native plugins.
Files:
app/containers/MessageComposer/components/Quotes/Quote.tsxapp/containers/markdown/interfaces.tsapp/containers/message/components/Content/PreviewContent.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/definitions/IMessage.tsapp/lib/methods/helpers/getMessageIdFromPermalink.tsapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/lib/hooks/usePreviewFormatText/index.tsxapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/hooks/useQuotedMessageChannels.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/components/__tests__/Reply.test.tsxapp/lib/methods/helpers/getMessageIdFromPermalink.test.tsapp/views/ThreadMessagesView/Item.tsxapp/lib/methods/helpers/formatChannelMentions.tsapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.tsx
🧠 Learnings (4)
📚 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/markdown/interfaces.tsapp/containers/message/components/Content/PreviewContent.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/definitions/IMessage.tsapp/lib/methods/helpers/getMessageIdFromPermalink.tsapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/lib/hooks/usePreviewFormatText/index.tsxapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/hooks/useQuotedMessageChannels.tsapp/containers/message/hooks/useMessageAccessibilityLabel.tsapp/containers/message/components/__tests__/Reply.test.tsxapp/lib/methods/helpers/getMessageIdFromPermalink.test.tsapp/views/ThreadMessagesView/Item.tsxapp/lib/methods/helpers/formatChannelMentions.tsapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.tsx
📚 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/message/components/Content/PreviewContent.tsxapp/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/lib/hooks/usePreviewFormatText/index.tsxapp/containers/message/components/Attachments/Reply.tsxapp/containers/message/components/__tests__/Reply.test.tsxapp/views/ThreadMessagesView/Item.tsxapp/containers/markdown/components/Preview.tsxapp/containers/markdown/components/mentions/Hashtag.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/containers/markdown/MarkdownPreview.channels.test.tsxapp/containers/markdown/Markdown.textStyle.test.tsxapp/containers/MessageComposer/components/Quotes/Quote.test.tsxapp/lib/methods/helpers/formatChannelMentions.test.tsapp/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsxapp/containers/message/hooks/__tests__/useQuotedMessageChannels.test.tsapp/containers/message/components/__tests__/Reply.test.tsxapp/lib/methods/helpers/getMessageIdFromPermalink.test.ts
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/ThreadMessagesView/Item.tsx
🪛 ast-grep (0.45.1)
app/lib/methods/helpers/formatChannelMentions.ts
[warning] 34-34: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(#(${pattern})(?![\\w-]), 'g')
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🪛 React Doctor (0.9.3)
app/containers/message/hooks/useQuotedMessageChannels.ts
[error] 17-17: 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 (22)
app/definitions/IMessage.ts (1)
44-45: LGTM!app/containers/markdown/interfaces.ts (1)
11-11: LGTM!app/containers/message/hooks/useMessageAccessibilityLabel.ts (2)
6-6: LGTM!
27-27: LGTM!app/containers/message/hooks/__tests__/useMessageAccessibilityLabel.test.tsx (2)
88-98: LGTM!
100-113: LGTM!app/containers/markdown/components/Preview.tsx (2)
6-15: LGTM!
17-19: LGTM!app/containers/markdown/components/mentions/Hashtag.tsx (1)
28-33: LGTM!Also applies to: 52-52, 63-63
app/containers/message/components/Content/PreviewContent.tsx (2)
3-10: LGTM!
22-22: LGTM!app/views/ThreadMessagesView/Item.tsx (1)
91-91: LGTM!app/containers/markdown/Markdown.textStyle.test.tsx (2)
43-58: LGTM!
60-64: LGTM!app/containers/markdown/MarkdownPreview.channels.test.tsx (1)
1-45: LGTM!app/containers/MessageComposer/components/Quotes/Quote.test.tsx (1)
1-53: LGTM!app/lib/methods/helpers/formatChannelMentions.test.ts (1)
1-79: LGTM!app/lib/methods/helpers/getMessageIdFromPermalink.ts (1)
1-7: LGTM!app/lib/methods/helpers/getMessageIdFromPermalink.test.ts (1)
1-40: LGTM!app/containers/message/components/Attachments/Reply.tsx (1)
17-18: LGTM!Also applies to: 111-112, 128-128
app/containers/message/components/__tests__/Reply.test.tsx (1)
13-13: LGTM!Also applies to: 25-28, 52-52, 93-124
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
49-49: LGTM!
….com/RocketChat/Rocket.Chat.ReactNative into fix/discussion-mention-quote-forward
Proposed changes
Fixes discussion mentions rendered as their room ID in surfaces that show plain-text previews: quoted messages (the quoted messages
channelsare read back off the local cache), forwarded/quote composer previews, thread start messages, and accessibility labels. A newformatChannelMentionshelper rewrites#<name>to#<fname>for discussions;useQuotedMessageChannelsresolves a quotes permalink back to its cached message.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1315
How to test or reproduce
#nameofyourdiscussionin the composer and send — it should show the discussion name, not the ID#general) must render unchangedScreenshots
N/A
Types of changes
Checklist
Summary by CodeRabbit
Bug Fixes
Tests