fix: Show proper tab titles for DMs and channels from inbox#376
fix: Show proper tab titles for DMs and channels from inbox#3762witstudios merged 7 commits intomasterfrom
Conversation
Tab titles for DMs and channels opened from the inbox were showing the
raw route path instead of meaningful names. Now DMs show "DM - {name}"
and channels show "#{channel-name}".
- Add API endpoint to fetch single conversation metadata
- Update useTabMeta hook to handle inbox-dm and inbox-channel types
- Reuse page fetching logic for channels (since they're pages)
- Add DM conversation metadata fetching with participant name
https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
- DM page now fetches single conversation instead of all conversations - Add Hash and MessageCircle icons to TabItem ICON_MAP for channel/DM tabs https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
…ages - Add /account route support (shows "Account" with User icon) - Handle nested settings pages like /settings/integrations/google-calendar (shows "Integrations - Google Calendar") - Simplify settings page titles (e.g., "Account" instead of "Settings - Account") - Add /p/[pageId] public page link support with async title fetch https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds a GET API to fetch a single conversation by ID, updates the DM page to use that endpoint, removes the FriendsPage component, expands tab icon support, and extends tab metadata/routing (adds public-page and account, nests settings subpages, removes friends route). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Auth
participant DB
Client->>Server: GET /api/messages/conversations/:conversationId
Server->>Auth: validate session (read-only)
Auth-->>Server: userId
Server->>DB: SELECT conversation where id = :id AND participant = userId
DB-->>Server: conversation + other participant profile | or empty
alt conversation found
Server-->>Client: 200 { conversation: { ... , otherUser: { ... } } }
else not found
Server-->>Client: 404 { error: "Conversation not found" }
end
opt server error
Server-->>Client: 500 { error: "Server error" }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
The /friends page is an empty stub not linked anywhere in the app. The actual feature is /dashboard/connections which is already handled. https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
This was an empty placeholder page not linked anywhere in the app. The actual connections feature is at /dashboard/connections. https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
Removed dmConversations, eq, and, or imports that were not being used since the route uses raw SQL queries instead of Drizzle query builder. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/messages/conversations/`[conversationId]/route.ts:
- Around line 84-92: The response currently exposes otherUser.email (populated
from row.user_email) which may be unnecessary; remove the email field from the
otherUser object in route.ts (the object literal keyed as otherUser), drop
row.user_email from the SQL SELECT that populates the result, and remove the
corresponding email property from any TypeScript interface/type used for the
response (e.g., the conversation/otherUser response type). Ensure no callers
rely on otherUser.email; if they do, update them to use name/displayName
instead.
🧹 Nitpick comments (2)
apps/web/src/app/api/messages/conversations/[conversationId]/route.ts (1)
58-75: Type assertion pattern is acceptable but consider extracting the interface.The
ConversationRowinterface defined inline and theas unknown as ConversationRowcast is a common pattern for raw SQL queries with Drizzle. For better maintainability, consider moving this interface to a shared types file if it's reused elsewhere.apps/web/src/hooks/useTabMeta.ts (1)
191-227: Potential display inconsistency for channel tab titles.When the channel metadata is updated in the
useEffect(line 116-120), the rawpageData.titleis stored. However, when displaying, the title is prefixed with#(lines 196 and 206). This means the storedtab.titleis "channel-name" but displayed as "#channel-name".This works correctly but creates a subtle inconsistency: the DM title is stored as
"DM - {name}"(line 129), while the channel title is stored as"{name}"and then prefixed at display time.Consider either:
- Storing the prefixed title
#${pageData.title}for channels (like DMs store the full formatted title), or- Documenting this as intentional behavior for supporting different display contexts.
| otherUser: { | ||
| id: row.user_id, | ||
| name: row.user_name, | ||
| email: row.user_email, | ||
| image: row.user_image, | ||
| username: row.user_username, | ||
| displayName: row.user_display_name, | ||
| avatarUrl: row.user_avatar_url, | ||
| }, |
There was a problem hiding this comment.
Consider whether exposing email in the response is necessary.
The otherUser object includes the user's email address. If this endpoint is only used for displaying tab titles (which uses displayName or name), the email field may be unnecessary and could be a privacy concern. Consider whether this data is needed by the consuming client.
🔒 Proposed fix to remove email from response
otherUser: {
id: row.user_id,
name: row.user_name,
- email: row.user_email,
image: row.user_image,
username: row.user_username,
displayName: row.user_display_name,
avatarUrl: row.user_avatar_url,
},Also remove the corresponding SQL select and interface property if email is not needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| otherUser: { | |
| id: row.user_id, | |
| name: row.user_name, | |
| email: row.user_email, | |
| image: row.user_image, | |
| username: row.user_username, | |
| displayName: row.user_display_name, | |
| avatarUrl: row.user_avatar_url, | |
| }, | |
| otherUser: { | |
| id: row.user_id, | |
| name: row.user_name, | |
| image: row.user_image, | |
| username: row.user_username, | |
| displayName: row.user_display_name, | |
| avatarUrl: row.user_avatar_url, | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/src/app/api/messages/conversations/`[conversationId]/route.ts around
lines 84 - 92, The response currently exposes otherUser.email (populated from
row.user_email) which may be unnecessary; remove the email field from the
otherUser object in route.ts (the object literal keyed as otherUser), drop
row.user_email from the SQL SELECT that populates the result, and remove the
corresponding email property from any TypeScript interface/type used for the
response (e.g., the conversation/otherUser response type). Ensure no callers
rely on otherUser.email; if they do, update them to use name/displayName
instead.
- Update friends path test to expect 'unknown' type (route was removed) - Remove friends type test from getStaticTabMeta (type no longer exists) - Fix settings subpage test expectations to match actual behavior (returns just formatted page name, not "Settings - X") Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tab titles for DMs and channels opened from the inbox were showing the
raw route path instead of meaningful names. Now DMs show "DM - {name}"
and channels show "#{channel-name}".
https://claude.ai/code/session_018mPA9wWDxNNPtewkMYKMrh
Summary by CodeRabbit
New Features
Improvements
Removed