chore: fix notification UI display #866
Conversation
…comments notifications
WalkthroughThis update introduces explicit handling for "reply" notifications throughout the comment and notification system. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant NotificationLib
participant DB
User->>WebApp: Submit new comment or reply
WebApp->>NotificationLib: createNotification({type, commentId, parentCommentId, ...})
alt type == "reply" and parentCommentId present
NotificationLib->>DB: Fetch parent comment's author
NotificationLib->>DB: Check for duplicate reply notification
alt No duplicate and recipient valid
NotificationLib->>DB: Insert reply notification for parent author
NotificationLib->>WebApp: Trigger dashboard revalidation
end
else type == "comment" or "reaction"
NotificationLib->>DB: Check notification preferences and duplicates
alt No duplicate and recipient valid
NotificationLib->>DB: Insert notification for video owner
NotificationLib->>WebApp: Trigger dashboard revalidation
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
apps/web/lib/Notification.ts (1)
24-28:parentCommentIdnow leaks into every notification typeAdding
parentCommentIdun-conditionally to all author-based notifications means even top-level “comment”, “reaction”, or “view” rows will carry an empty / undefined field innotifications.data. If downstream code later assumes the presence of this key only for"reply"types you’ll get false positives.Consider restricting the extra key to the
"reply"branch only:-: Omit<D, "author"> & { authorId: string } & { parentCommentId?: string } +: D["type"] extends "reply" + ? Omit<D, "author"> & { authorId: string; parentCommentId: string } + : Omit<D, "author"> & { authorId: string }apps/web/actions/videos/new-comment.ts (1)
52-58: Avoid persisting meaninglessparentCommentIdvalues
parentCommentIdis forwarded even for root comments as an empty string, which ends up stored innotifications.data.
Preferundefined/nullso downstream"reply"logic (if (notification.parentCommentId) …) remains explicit and data stays clean.- parentCommentId, + ...(parentCommentId ? { parentCommentId } : {}),apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
30-41: Collapse duplicateuseSearchParams()calls
useSearchParams()is inexpensive but returns a stable instance; still, calling it twice per render is redundant and hurts readability.- const commentParams = useSearchParams().get("comment"); - const replyParams = useSearchParams().get("reply"); + const searchParams = useSearchParams(); + const commentParams = searchParams.get("comment"); + const replyParams = searchParams.get("reply");apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx (1)
70-78: Minor styling bug: unwanted leading space in colour string
backgroundColorfallback value has a leading space (" #F9F9F9").
This is benign in most browsers but technically invalid and may break minifiers.- backgroundColor: (commentParams || replyParams) === comment.id ? ["#F9F9F9", "#EDF6FF"] : " #F9F9F9", + backgroundColor: (commentParams || replyParams) === comment.id ? ["#F9F9F9", "#EDF6FF"] : "#F9F9F9",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/actions/videos/new-comment.ts(1 hunks)apps/web/app/(org)/dashboard/_components/Notifications/NotificationItem.tsx(3 hunks)apps/web/app/(org)/dashboard/_components/Notifications/index.tsx(2 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx(2 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx(1 hunks)apps/web/app/s/[videoId]/page.tsx(4 hunks)apps/web/lib/Notification.ts(6 hunks)
🔇 Additional comments (3)
apps/web/app/s/[videoId]/page.tsx (1)
632-636: Comment ordering silently flips to oldest-firstPrevious implementation used
desc(comments.createdAt).
The new fallbackcomments.createdAtreturns ascending order, changing UX and making the newest comments disappear off-screen.Restore explicit ordering:
- : comments.createdAt + : sql`${comments.createdAt} DESC`apps/web/app/(org)/dashboard/_components/Notifications/NotificationItem.tsx (2)
65-70: Conditional render looks goodGrouping the
commentandreplychecks and addingh-fit+line-clamp-2cleanly achieves the two-line preview.
97-101: Confirm URL format (& trailing slash) for reply linksBoth the
replyandcommentpaths include a slash right before the query string (/s/${id}/?reply=).
If the route is defined without the trailing slash (/s/[videoId]), this will redirect or 404 depending on Next.js config.
Double-check that the trailing slash is intentional and consistent across the app.
This PR:
Summary by CodeRabbit
New Features
Improvements