Repository navigation
feat(inbox): add unified inbox view with DMs and channels - #327
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces a comprehensive inbox feature with a new API endpoint for fetching unified inbox items (direct messages and channels), new dashboard pages for inbox views, a sidebar component with pagination and search capabilities, and navigation updates to route users to the inbox interface. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant InboxSidebar
participant APIServer as /api/inbox
participant Database
User->>InboxSidebar: Navigate to inbox or mount
InboxSidebar->>APIServer: GET /api/inbox?driveId=X&limit=20
APIServer->>Database: Query channels with last messages
APIServer->>Database: Query DMs with unread counts
APIServer->>Database: Fetch sender info and timestamps
Database-->>APIServer: Return aggregated results
APIServer-->>InboxSidebar: Return paginated InboxItem[]
InboxSidebar->>InboxSidebar: Normalize timestamps & render items
InboxSidebar-->>User: Display inbox with last messages & unread counts
User->>InboxSidebar: Scroll or type search query
InboxSidebar->>InboxSidebar: Filter items client-side or fetch more (pagination)
InboxSidebar-->>User: Update displayed items
User->>InboxSidebar: Click conversation
InboxSidebar->>User: Navigate to conversation page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
Add a unified inbox view that aggregates direct messages and channel conversations across all drives. The inbox sidebar shows recent activity and provides quick access to all communication in one place. Changes: - Add Inbox link to main sidebar navigation - Create InboxSidebar component for inbox-specific navigation - Create DashboardFooter with quick action links (Tasks, Activity, etc.) - Add API route for unified inbox with pagination - Add inbox pages for dashboard and drive-specific views - Update UserDropdown to remove items now in DashboardFooter - Add dashboardFooterCollapsed state to layout store Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
0691f63 to
a5c168b
Compare
![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
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/inbox/route.ts`:
- Around line 263-267: The hasMore calculation is unreliable because
filteredItems is bounded by the SQL LIMIT in the drive-specific path; change the
fetch logic to request limit + 1 items (so filteredItems can be one item larger
when there are more) then set hasMore = filteredItems.length > limit, set
paginatedItems = filteredItems.slice(0, limit) (so paginatedItems contains only
the real page) and compute nextCursor from the last element of paginatedItems if
any; update any code referencing hasMore, filteredItems, paginatedItems, limit,
and nextCursor accordingly.
- Around line 115-151: The DM query (dmResults, dm_data CTE and unread_counts)
currently ignores the incoming cursor parameter; modify the dm_data WHERE clause
to apply the same cursor filter used for channels so results are paginated
consistently — e.g., when a cursor is provided add a condition like AND
dd."lastMessageAt" < ${cursor} (matching the DESC ordering) or an equivalent
safe comparison (handle nulls if needed) so the final SELECT returns only
conversations before the cursor; keep the rest of the CTEs (unread_counts,
joins) intact.
In `@apps/web/src/components/layout/left-sidebar/InboxSidebar.tsx`:
- Around line 83-101: The loadMore function currently parses response JSON
without checking HTTP status; update the logic in loadMore (the block using
fetchWithAuth and the response variable) to verify response.ok before calling
response.json(), and handle non-ok cases by reading the error body/text and
throwing or logging a clear error so the catch block can run; ensure you don't
call setAllItems or setPagination when the response is an error and still clear
isLoadingMore in finally.
- Around line 114-119: The isItemActive function can produce false positives
because it uses pathname?.includes() to match IDs; change it to perform exact
path-segment matching instead (e.g., for dm items compare pathname to
`/messages/${item.id}` or ensure it starts with that plus a trailing slash, and
for other items compare the segment `/…/${item.id}` exactly or compare the last
path segment via pathname.split('/').pop()). Update isItemActive (and any usage
of InboxItem id matching) to use these exact equality/segment checks rather than
includes().
🧹 Nitpick comments (4)
apps/web/src/components/shared/UserDropdown.tsx (1)
106-109: Consider a more semantically appropriate icon for Account.
LayoutDashboardtypically represents dashboard/grid layouts. For an "Account" menu item, consider usingUserorUserCirclefrom lucide-react for better semantic clarity.💡 Suggested icon change
-import { LogOut, MessageSquareText, Settings, LayoutDashboard, Sun, Moon, Monitor, HardDrive, CreditCard } from 'lucide-react'; +import { LogOut, MessageSquareText, Settings, User, Sun, Moon, Monitor, HardDrive, CreditCard } from 'lucide-react';<DropdownMenuItem onClick={() => router.push('/settings/account')}> - <LayoutDashboard className="mr-2 h-4 w-4" /> + <User className="mr-2 h-4 w-4" /> <span>Account</span> </DropdownMenuItem>apps/web/src/app/api/inbox/route.ts (1)
86-94: DuplicateChannelRowinterface definition.The
ChannelRowinterface is defined identically in both the drive-specific branch (lines 86-94) and the dashboard branch (lines 215-223). Extract it to a single definition near the top of the file withInboxItemandDMRow.♻️ Proposed refactor
Move the interface near line 20 (after
InboxItem):+interface ChannelRow { + id: string; + name: string; + drive_id: string; + drive_name: string; + last_message: string | null; + last_message_at: string | null; + sender_name: string | null; +} + +interface DMRow { + id: string; + last_message_at: string | null; + last_message: string | null; + other_user_name: string; + other_user_display_name: string | null; + other_user_avatar_url: string | null; + unread_count: string; +}Then remove the duplicate definitions at lines 86-94, 153-161, and 215-223.
Also applies to: 215-223
apps/web/src/app/dashboard/[driveId]/inbox/page.tsx (1)
49-61: Suspense boundary provides limited value here.The
Suspensewrapper shows a skeleton fallback, butDriveInboxContentis a client component that handles its own loading state viaisLoadingfrom the store (lines 20-26). The Suspense boundary won't catch the store's async fetch.This is fine to keep for consistency with other pages, but note it won't actually suspend during the
fetchDrives()call.apps/web/src/components/layout/left-sidebar/InboxSidebar.tsx (1)
68-70: AddrevalidateOnFocus: falseto SWR config.Per coding guidelines, SWR should be configured with
revalidateOnFocus: falseto prevent unexpected refetches when the user returns to the tab.♻️ Proposed fix
const { data, error } = useSWR<InboxResponse>(apiUrl, fetcher, { refreshInterval: 10000, + revalidateOnFocus: false, });As per coding guidelines: "Use SWR for server state and caching with proper configuration including
revalidateOnFocus: false".
…cking - Fix Settings route in DashboardFooter (/dashboard/settings -> /settings) - Fix hasMore pagination by fetching limit+1 rows - Fix nextCursor null handling with composite cursor (id:<itemId>) - Add channel_read_status schema for tracking unread messages - Add channel read tracking endpoint (POST /api/channels/[pageId]/read) - Add real-time socket broadcasts for DM and channel updates - Replace polling with socket-based updates in InboxSidebar - Add UI refresh protection during editing sessions - Calculate actual channel unread counts from lastReadAt watermark Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add response.ok check in loadMore before parsing JSON - Fix isItemActive to use exact path matching instead of includes() Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove unused channelReadStatus import from read route - Remove unused useRef import from InboxSidebar - Remove unused InboxItem import from useInboxSocket Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code reviewFound 3 issues:
PageSpace/apps/web/src/hooks/useInboxSocket.ts Lines 47 to 65 in ba15a3f
PageSpace/apps/web/src/app/api/inbox/route.ts Lines 177 to 179 in ba15a3f
PageSpace/apps/web/src/app/api/channels/[pageId]/messages/route.ts Lines 162 to 190 in ba15a3f Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Add page-level permission check before broadcasting channel updates to prevent message preview leakage to unauthorized users - Add LIMIT to dashboard inbox queries to prevent unbounded memory usage - Handle new conversations in socket hook by triggering SWR revalidation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addressed code review issues in commit 378f46dIssue 1: New conversations not appearing in real-time inbox
Issue 2: Dashboard inbox unbounded queries
Issue 3: Permission check for inbox broadcasts
Generated with Claude Code |
- Add canUserViewPage check to inbox API route to filter channels by page-level permissions instead of just drive membership - Update InboxSidebar to preserve loaded pages when SWR revalidates by merging first-page updates with previously loaded items - Reset pagination state when driveId changes to avoid mixing contexts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Additional fixes in commit 677fb29Building on the earlier review feedback, this commit adds two more improvements: 1. Page permission filtering for inbox channels (P1 security fix)
2. Preserve paginated data on SWR revalidation (P2 UX fix)
Generated with Claude Code |
1 participant
Summary
/api/inbox) supporting dashboard-wide and drive-specific viewsChanges
apps/web/src/app/api/inbox/route.tsapps/web/src/app/dashboard/inbox/page.tsxapps/web/src/app/dashboard/[driveId]/inbox/page.tsxapps/web/src/components/layout/left-sidebar/InboxSidebar.tsxapps/web/src/components/layout/left-sidebar/DashboardFooter.tsxapps/web/src/components/layout/left-sidebar/MemoizedSidebar.tsxapps/web/src/components/layout/left-sidebar/index.tsxapps/web/src/components/shared/UserDropdown.tsxapps/web/src/stores/useLayoutStore.tsapps/web/src/app/dashboard/layout.tsxTest plan
/dashboard/inboxand verify empty state displays/dashboard/[driveId]/inboxand verify channels display🤖 Generated with Claude Code
Summary by CodeRabbit