feat: add viewerName if available #1941
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR adds optional Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/analytics/visitors-table.tsx (1)
252-273: IncludeviewerNamein exported data.The export functionality doesn't include the
viewerNamefield, which means exported CSV files won't contain visitor names even when available.Apply this diff:
const exportData = visitors.map((visitor) => ({ + Name: visitor.viewerName || "", Email: visitor.email, "Total Views": visitor.totalViews, "Unique Documents": visitor.uniqueDocuments, "Total Duration": durationFormat(visitor.totalDuration), "Last Active": new Date(visitor.lastActive).toISOString(), Verified: visitor.verified ? "Yes" : "No", }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/analytics/visitors-table.tsx(2 hunks)components/visitors/contacts-table.tsx(2 hunks)components/visitors/dataroom-viewers.tsx(2 hunks)components/visitors/dataroom-visitors-table.tsx(2 hunks)components/visitors/visitors-table.tsx(3 hunks)pages/api/analytics/index.ts(2 hunks)pages/api/teams/[teamId]/datarooms/[id]/viewers/index.ts(2 hunks)pages/api/teams/[teamId]/viewers/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/analytics/visitors-table.tsx (1)
components/ui/tooltip.tsx (1)
BadgeTooltip(53-107)
components/visitors/visitors-table.tsx (2)
components/ui/table.tsx (1)
TableCell(116-116)components/visitors/visitor-avatar.tsx (1)
VisitorAvatar(10-95)
🔇 Additional comments (14)
pages/api/analytics/index.ts (1)
482-493: LGTM!The logic correctly derives
viewerNamefrom the most recent view containing a name and includes proper null handling.components/analytics/visitors-table.tsx (2)
54-54: LGTM!The optional
viewerNamefield is properly typed.
67-78: LGTM!The rendering logic correctly prioritizes
viewerNameover email and conditionally displays both when available.components/visitors/contacts-table.tsx (2)
42-42: LGTM!The optional
viewerNamefield is properly typed.
153-159: LGTM!The rendering logic correctly displays
viewerNamewith proper fallback to email and conditional secondary line.pages/api/teams/[teamId]/datarooms/[id]/viewers/index.ts (2)
68-73: LGTM!The
viewerNamefield is correctly included in the view selection.
102-110: LGTM!The logic correctly derives
viewerNamefrom the most recent view and includes it in the response with proper null handling.components/visitors/dataroom-visitors-table.tsx (2)
106-106: LGTM!The display logic correctly prioritizes
viewerNameoverviewerEmail.
144-148: LGTM!The conditional rendering properly displays the email as a secondary line when both
viewerNameandviewerEmailexist.components/visitors/visitors-table.tsx (3)
179-189: LGTM!The archived view correctly displays
viewerNamewith proper fallback and conditional email line.
287-287: LGTM!The display logic in the collapsible row correctly prioritizes
viewerName.
346-350: LGTM!The conditional rendering properly shows the email as a secondary line when both fields exist.
pages/api/teams/[teamId]/viewers/index.ts (2)
87-102: Verify query performance with large datasets.The correlated subquery for
viewerNamecould have performance implications when dealing with a large number of views. Each row in the outer query triggers a separate subquery execution.Consider monitoring query performance or testing with realistic data volumes. If performance becomes an issue, you might want to use a window function or a different join strategy:
-- Alternative approach using window function ( SELECT "viewerId", "viewerName", ROW_NUMBER() OVER (PARTITION BY "viewerId" ORDER BY "viewedAt" DESC) as rn FROM "View" WHERE "viewerName" IS NOT NULL ) as recent_names ON v.id = recent_names."viewerId" AND recent_names.rn = 1However, the current implementation is correct and may be sufficient for typical dataset sizes.
121-141: LGTM!The type definition and response mapping correctly include
viewerNamewith proper null handling.
| {viewer.email ? ( | ||
| <> | ||
| {viewer.email}{" "} | ||
| {(viewer as any).viewerName || viewer.email}{" "} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add proper type definition instead of using any cast.
The type assertion (viewer as any).viewerName bypasses TypeScript's type safety. Consider defining a proper type or extending the viewer interface to include the optional viewerName field.
Apply this pattern (similar to other components in this PR):
// At the top of the file or in a types file
interface ViewerWithName {
viewerName?: string | null;
// ... other viewer properties
}Then use:
-{(viewer as any).viewerName || viewer.email}{" "}
+{(viewer as ViewerWithName).viewerName || viewer.email}{" "}Or better yet, ensure the useDataroomViewers hook returns viewers with the proper type that includes viewerName.
🤖 Prompt for AI Agents
In components/visitors/dataroom-viewers.tsx around line 80, the code uses
(viewer as any).viewerName which bypasses TypeScript safety; define a proper
interface (e.g., ViewerWithName with optional viewerName?: string | null plus
existing viewer fields) either at the top of this file or in a shared types
file, update the component and the useDataroomViewers hook to return viewers
typed as ViewerWithName (or narrow the local viewer variable to that type) and
replace the any cast with viewer.viewerName so TypeScript enforces the optional
field correctly.
| {(viewer as any).viewerName && viewer.email && ( | ||
| <p className="text-xs text-muted-foreground/60"> | ||
| {viewer.email} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use proper type definition for consistency.
The same type safety concern applies here as on Line 80.
🤖 Prompt for AI Agents
components/visitors/dataroom-viewers.tsx around lines 110 to 114: currently the
code uses (viewer as any).viewerName which bypasses type checking; replace the
any cast by updating the viewer prop/type so viewer has a proper interface that
includes viewerName and email (or use a narrowed union type) and then reference
viewer.viewerName directly; alternatively add a small type guard function
(isViewerWithName) to narrow the union before accessing viewerName; update the
component/prop type definitions where viewer is declared so the compiler knows
viewerName exists and remove the unsafe any cast.
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.