-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: dataroom without documents show timestamp 0 #1836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdded dataroom.createdAt to dataroom link data retrieval and updated two view pages to seed lastUpdatedAt calculations with the dataroom creation timestamp instead of 0. No control-flow or API signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/api/links/link-data.ts (1)
108-116: Add missingcreatedAt(andallowBulkDownload) to theLinkWithDataroomtypeThe
fetchDataroomLinkDatacalls in your API routes are now selecting bothallowBulkDownloadandcreatedAt, but the TypeScript interface forLinkWithDataroomhasn’t been updated to reflect these fields. You’ll get a type mismatch whenever you try to access them at runtime.Please update
lib/types.ts—around line 125—to include both new properties on thedataroomobject:export interface LinkWithDataroom extends Link { dataroom: { id: string; name: string; teamId: string; + allowBulkDownload: boolean; // <- add this + createdAt: string; // <- add this documents: { // … }; }; }This ensures that consumers of
link.dataroom.createdAt(andallowBulkDownload) will type‐check correctly.pages/view/domains/[domain]/[slug]/index.tsx (1)
170-176: Use the computed documents array for lastUpdatedAt (ensures doc.updatedAt is considered)You correctly seed with createdAt. However, the reduction currently reads from link.dataroom.documents, which ignores your per-document merged updatedAt (max of dataroomDocument.updatedAt and version.updatedAt). Reduce over the computed documents to avoid underreporting lastUpdatedAt.
- const lastUpdatedAt = link.dataroom.documents.reduce((max, doc) => { - return Math.max( - max, - new Date(doc.document.versions[0].updatedAt).getTime(), - ); - }, new Date(link.dataroom.createdAt).getTime()); + const baseCreatedAt = new Date(link.dataroom.createdAt as any).getTime(); + const lastUpdatedAt = documents.reduce((max, doc) => { + return Math.max( + max, + new Date(doc.versions[0].updatedAt as any).getTime(), + ); + }, Number.isFinite(baseCreatedAt) ? baseCreatedAt : 0);pages/view/[linkId]/index.tsx (1)
188-194: Use the computed documents array for lastUpdatedAt (keeps logic consistent with merged per-doc timestamps)Same rationale as the domain page: read from documents, not link.dataroom.documents, so lastUpdatedAt reflects the max of (dataroomDocument.updatedAt vs version.updatedAt) per file.
- const lastUpdatedAt = link.dataroom.documents.reduce((max, doc) => { - return Math.max( - max, - new Date(doc.document.versions[0].updatedAt).getTime(), - ); - }, new Date(link.dataroom.createdAt).getTime()); + const baseCreatedAt = new Date(link.dataroom.createdAt as any).getTime(); + const lastUpdatedAt = documents.reduce((max, doc) => { + return Math.max( + max, + new Date(doc.versions[0].updatedAt as any).getTime(), + ); + }, Number.isFinite(baseCreatedAt) ? baseCreatedAt : 0);
🧹 Nitpick comments (5)
lib/api/links/link-data.ts (1)
108-116: Consider also selecting dataroom.updatedAt for a more accurate “last updated” baselineIf dataroom metadata (branding, settings, folder structure) updates should influence lastUpdatedAt, exposing updatedAt here will let the UI compute max(createdAt, updatedAt, doc updates).
dataroom: { select: { id: true, name: true, teamId: true, allowBulkDownload: true, + updatedAt: true, createdAt: true,pages/view/domains/[domain]/[slug]/index.tsx (2)
175-175: Add a defensive fallback if createdAt is missing or invalidIf createdAt were ever absent (older payloads, partial fetch), new Date(undefined).getTime() yields NaN and poisons the reduction. The above diff guards this; alternatively:
- }, new Date(link.dataroom.createdAt).getTime()); + }, Date.parse(String(link.dataroom.createdAt)) || 0);
146-166: Deduplicate shared compute logic between domain and ID viewsBoth pages build documents the same way and compute lastUpdatedAt similarly. Extract a small helper (e.g., lib/view/compute-last-updated.ts) to DRY and reduce divergence risk.
// lib/view/compute-last-updated.ts export function computeLastUpdatedAt( createdAt: string | Date, docs: Array<{ versions: Array<{ updatedAt: string | Date }> }> ) { const base = Date.parse(String(createdAt)) || 0; return docs.reduce((m, d) => Math.max(m, Date.parse(String(d.versions[0].updatedAt)) || 0), base); }Then use:
- const lastUpdatedAt = /* reduction inline */ + const lastUpdatedAt = computeLastUpdatedAt(link.dataroom.createdAt, documents);Also applies to: 170-176
pages/view/[linkId]/index.tsx (2)
193-193: Defensive default for createdAt parsingMirror the same guard here to avoid NaN in case of unexpected payloads.
- }, new Date(link.dataroom.createdAt).getTime()); + }, Date.parse(String(link.dataroom.createdAt)) || 0);
164-184: Optional: centralize shared logicBoth pages share the documents mapping and lastUpdatedAt calculation. Consider extracting a shared helper as suggested in the domain view comment to reduce duplication.
Also applies to: 188-194
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
lib/api/links/link-data.ts(1 hunks)pages/view/[linkId]/index.tsx(1 hunks)pages/view/domains/[domain]/[slug]/index.tsx(1 hunks)
Summary by CodeRabbit