fix: display discussion name instead of id in composer mentions - #7565
fix: display discussion name instead of id in composer mentions#7565Rohit3523 wants to merge 4 commits into
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe composer now inserts discussion-room display titles, tracks their internal names, and resolves mention tokens before sending text messages. Tests cover draft persistence, token cleanup, prefix matching, same-name rooms, and duplicate display names. ChangesDiscussion room mention resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ComposerInput
participant MessageComposer
participant onSendMessage
ComposerInput->>ComposerInput: Insert display title and track internal name
MessageComposer->>ComposerInput: Resolve mention tokens
ComposerInput-->>MessageComposer: Replace titles with internal names
MessageComposer->>onSendMessage: Send resolved text
Suggested labels: Suggested reviewers: 🚥 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 |
GitHub Actions
Error
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/containers/MessageComposer/components/ComposerInput.tsx (1)
176-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit TypeScript annotations to both resolver implementations.
Both new callbacks rely on inferred return types, and the
ComposerInput.tsxcallback also relies on an inferred parameter type.
- app/containers/MessageComposer/components/ComposerInput.tsx#L176-L183: use
(text: string): string =>.- app/containers/MessageComposer/MessageComposer.tsx#L52-L52: use
(text: string): string => text.As per coding guidelines, TypeScript function parameters and return types must have explicit type 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/containers/MessageComposer/components/ComposerInput.tsx` around lines 176 - 183, Update both resolver callbacks with explicit TypeScript annotations: in app/containers/MessageComposer/components/ComposerInput.tsx lines 176-183, annotate the text parameter and string return type in resolveMentionRoomTokens; in app/containers/MessageComposer/MessageComposer.tsx line 52, annotate the identity resolver’s text parameter and string return type. Preserve the existing callback behavior.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/MessageComposer/components/ComposerInput.tsx`:
- Around line 176-183: Update resolveMentionRoomTokens to process mention titles
in descending length order before replacing tokens, ensuring longer titles such
as “foo bar” match before shorter prefixes such as “foo” while preserving the
existing escaping and boundary behavior.
- Around line 62-65: Scope mentionRoomTokensRef mappings to the active draft and
room by resetting or replacing them when the composer input/draft or room
changes, and remove entries when their associated mention is deleted. Ensure
selecting a discussion whose title equals its subtitle overwrites any existing
title mapping, and clear each mapping after it is consumed during message
submission.
- Around line 293-298: Update the mention-token mapping in ComposerInput so
duplicate discussion titles do not overwrite each other in
mentionRoomTokensRef.current. Use an occurrence-aware key or another unique
display token while retaining each room’s subtitle, and ensure the send-time
resolution uses the same keying scheme.
In `@app/containers/MessageComposer/MessageComposer.test.tsx`:
- Around line 575-594: Add an assertion in the “select # discussion shows fname
in composer but sends the room name” test after selecting the autocomplete item
and before sending, verifying the message-composer input displays “#My
Discussion”. Keep the existing send-payload assertion to confirm it still sends
“#aBcD123xyz”.
---
Nitpick comments:
In `@app/containers/MessageComposer/components/ComposerInput.tsx`:
- Around line 176-183: Update both resolver callbacks with explicit TypeScript
annotations: in app/containers/MessageComposer/components/ComposerInput.tsx
lines 176-183, annotate the text parameter and string return type in
resolveMentionRoomTokens; in app/containers/MessageComposer/MessageComposer.tsx
line 52, annotate the identity resolver’s text parameter and string return type.
Preserve the existing callback behavior.
🪄 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: 83660ba5-ef5f-458a-bbf8-d584571639c9
📒 Files selected for processing (4)
app/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.tsxapp/containers/MessageComposer/interfaces.ts
📜 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/interfaces.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.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/interfaces.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.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/interfaces.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
🧠 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/interfaces.tsapp/containers/MessageComposer/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.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/MessageComposer/MessageComposer.test.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/MessageComposer.test.tsxapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/ComposerInput.tsx
🔇 Additional comments (2)
app/containers/MessageComposer/interfaces.ts (1)
31-31: LGTM!app/containers/MessageComposer/MessageComposer.tsx (1)
177-177: LGTM!
| // Displays the readable fname for discussion-style rooms, but the server only resolves | ||
| // channel mentions by the room `name`, so the composer translates the title back to it | ||
| // when sending. Keyed by the exact title text inserted into the composer. | ||
| const mentionRoomTokensRef = useRef<Record<string, string>>({}); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Scope the mapping to the current draft and room.
mentionRoomTokensRef is populated when a discussion is selected, but no path clears it when the input is replaced or cleared. If the composer is reused for another draft or room, a later #title can still resolve to an old #name. Selecting an item with equal title and subtitle also leaves an older mapping unchanged. Store mappings with the current draft or clear them when the associated mention is removed and after the mapping is consumed.
Also applies to: 293-298
🤖 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/components/ComposerInput.tsx` around lines 62
- 65, Scope mentionRoomTokensRef mappings to the active draft and room by
resetting or replacing them when the composer input/draft or room changes, and
remove entries when their associated mention is deleted. Ensure selecting a
discussion whose title equals its subtitle overwrites any existing title
mapping, and clear each mapping after it is consumed during message submission.
|
@coderabbitai review |
|
GitHub Actions
Waiting
1 participant
Proposed changes
When a user mentions a discussion in the composer, the mention inserted into the composer showed the discussion ID instead of its display name. This fix shows the readable discussion name (
fname) in the composer while resolving it back to the roomnameon send, so the mention is delivered correctly to the server.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1316
How to test or reproduce
#nameofyourdiscussionin the composerScreenshots
Screen.Recording.2026-08-12.at.12.49.33.AM.mov
Screen.Recording.2026-08-12.at.1.23.01.AM.mov
Types of changes
Checklist
Further comments
The composer now stores a title→room-name mapping when a discussion autocomplete item is selected, translating it back on send since the server only resolves channel mentions by room
name.Summary by CodeRabbit
Bug Fixes
Tests