feat: speed improvements #1877
feat: speed improvements #1877
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces helper-based team access with direct Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant API as Endpoint
participant DB as Prisma/DB
U->>API: Request (teamId, resourceId)
API->>DB: userTeam.findUnique(userId, teamId)
alt unauthorized
DB-->>API: null
API-->>U: 401 Unauthorized
else authorized
API->>DB: resource.findUnique({ id, teamId })
alt not found
DB-->>API: null
API-->>U: 404 Not Found
else found
API->>DB: additional queries (findMany / groupBy)
DB-->>API: data + aggregated counts
API-->>U: 200/201 Response
end
end
sequenceDiagram
autonumber
actor U as User
participant API as Dataroom ViewerGroups GET
participant DB as Prisma/DB
U->>API: GET /datarooms/:id/groups
API->>DB: userTeam.findUnique(userId, teamId)
DB-->>API: teamAccess
API->>DB: dataroom.findUnique(id, teamId, select: { id })
DB-->>API: dataroom|null
alt dataroom missing
API-->>U: 404
else
API->>DB: viewerGroup.findMany(dataroomId, orderBy: createdAt desc)
par counts
API->>DB: viewerGroupMembership.groupBy(groupId, _count)
API->>DB: view.groupBy(groupId, _count)
and access controls (conditional)
API->>DB: accessControl.findMany(filter by itemType/itemId)
end
API-->>U: 200 viewerGroupsWithCounts and accessControls
end
sequenceDiagram
autonumber
actor U as User
participant API as Documents List/Search
participant DB as Prisma/DB
U->>API: GET /documents
API->>DB: userTeam.findUnique(userId, teamId)
DB-->>API: teamAccess
API->>DB: document.findMany(filters, no counts)
DB-->>API: documents[]
par counts
API->>DB: link.groupBy(documentId)
API->>DB: view.groupBy(documentId)
API->>DB: documentVersion.groupBy(documentId)
API->>DB: dataroomDocument.groupBy(documentId) %% where applicable
end
API->>API: Merge counts into documents as _count
API-->>U: 200 documentsWithCounts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
pages/api/teams/[teamId]/documents/[id]/overview.ts (2)
36-46: Bug: invalid use of findUnique with relation filter; switch to userTeam join and read plan from there.
prisma.team.findUniquecannot include a relation filter (users: { some: ... }) inwhere. This will not compile and breaks auth. UseuserTeamcomposite lookup and pullplanvia relation.Apply:
- const team = await prisma.team.findUnique({ - where: { - id: teamId, - users: { - some: { - userId, - }, - }, - }, - select: { plan: true }, - }); + const membership = await prisma.userTeam.findUnique({ + where: { userId_teamId: { userId, teamId } }, + select: { team: { select: { plan: true } } }, + }); + + if (!membership) { + return res.status(401).end("Unauthorized"); + } + + const teamPlan = membership.team?.plan ?? "free";And update downstream usages:
- if (primaryVersion && team.plan.includes("free")) { + if (primaryVersion && teamPlan.includes("free")) {- team: { - plan: team?.plan || "free", - isTrial: team?.plan.includes("drtrial") || false, - }, + team: { + plan: teamPlan, + isTrial: teamPlan.includes("drtrial"), + },
36-46: Replace prisma.team.findUnique calls that include relation filters with findFirst (or check membership separately).Prisma's findUnique accepts only unique identifier fields in where; relation filters (e.g., users: { some: ... }) should be handled with findFirst/findMany or by querying the team by id and checking the users relation. (prisma.io)
- Action: Replace patterns like
prisma.team.findUnique({ where: { id: teamId, users: { some: { userId } } }, ... })
with
prisma.team.findFirst({ where: { id: teamId, users: { some: { userId } } }, ... })- Alternative: keep findUnique({ where: { id: teamId } }) and then assert membership via a select on users (or a separate query) before proceeding.
- Scope: scan found this in multiple API routes (example: pages/api/teams/[teamId]/documents/[id]/overview.ts lines 36–46) — fix all occurrences under pages/api.
pages/api/teams/[teamId]/documents/[id]/annotations/[annotationId].ts (3)
87-101: Use findFirst (or a composite unique) for annotation fetch with multiple filtersSame concern:
findUniquewon’t acceptwhere: { id, documentId, teamId }unless a matching composite unique exists. PreferfindFirsthere since you already guard membership and document existence.Apply:
- const fullAnnotation = await prisma.documentAnnotation.findUnique({ - where: { id: annotationId, documentId: docId, teamId }, + const fullAnnotation = await prisma.documentAnnotation.findFirst({ + where: { id: annotationId, documentId: docId, teamId }, include: { images: true, createdBy: { select: { id: true, name: true, email: true }, }, }, });
108-121: Update selector must be unique — either rely on id after prior checks or add a composite uniqueYou previously verified the annotation belongs to the document/team. Use
idfor the update, or add a composite unique in schema and use that selector.Apply:
- const updatedAnnotation = await prisma.documentAnnotation.update({ - where: { id: annotationId, documentId: docId, teamId }, + const updatedAnnotation = await prisma.documentAnnotation.update({ + where: { id: annotationId }, data: validatedData, include: { images: true, createdBy: { select: { id: true, name: true, email: true }, }, }, });
126-128: Delete selector must be unique — use id after the earlier scope checksSame reasoning as the update path.
Apply:
- await prisma.documentAnnotation.delete({ - where: { id: annotationId, documentId: docId, teamId }, - }); + await prisma.documentAnnotation.delete({ + where: { id: annotationId }, + });pages/api/teams/[teamId]/documents/[id]/index.ts (1)
60-96: Prisma findUnique used with non-unique/relational filters — switch to findFirst
findUniquemust match a unique constraint. Filtering by{ id, teamId }is not a unique selector unless you expose a composite unique input; the current shape is invalid. UsefindFirstwith the filter.Apply:
- const document = await prisma.document.findUnique({ - where: { - id: docId, - teamId, - }, + const document = await prisma.document.findFirst({ + where: { id: docId, teamId }, include: { versions: { where: { isPrimary: true }, orderBy: { createdAt: "desc" }, take: 1, }, folder: { select: { name: true, path: true } }, datarooms: { select: { dataroom: { select: { id: true, name: true } }, folder: { select: { id: true, name: true, path: true } }, }, }, }, });pages/api/teams/[teamId]/folders/move.ts (2)
68-73: Don’t write the HTTP response inside a Prisma transactionReturning
res.status(...).json(...)inside$transactioncan lead to “headers already sent” and leaves the outer handler flow inconsistent. Detect duplicates before opening the transaction or throw and handle in the outer catch.Apply:
- if (duplicateNames.length > 0) { - return res.status(409).json({ - message: `Cannot move folders: Duplicate names found inside target folder - ${duplicateNames.join(", ")}`, - }); - } + if (duplicateNames.length > 0) { + throw new Error(`DUPLICATE_FOLDER_NAMES:${duplicateNames.join(",")}`); + }And in the catch block:
- } catch (error) { - return res.status(500).end(error); + } catch (error: any) { + if (typeof error.message === "string" && error.message.startsWith("DUPLICATE_FOLDER_NAMES:")) { + const list = error.message.split(":")[1]?.replaceAll(",", ", "); + return res.status(409).json({ + message: `Cannot move folders: Duplicate names found inside target folder - ${list}`, + }); + } + return res.status(500).end("Internal Server Error"); }
95-117:updatescontains nulls — Promise.all will throw; filter out falsy entriesMap can return
nullwhen no parentFolder; filter beforePromise.all.Apply:
- const updates = allSubfolders.map((subfolder) => { + const updates = allSubfolders.map((subfolder) => { // Find the parent folder it belongs to const parentFolder = foldersToMove.find((folder) => subfolder.path.startsWith(folder.path), ); if (!parentFolder) return null; // Get the new base path for the parent const newParentPath = folderPathUpdates.get(parentFolder.id); // Calculate the new subfolder path by replacing the old path with the new one const relativePath = subfolder.path .replace(parentFolder.path, "") .trim(); const newSubfolderPath = `${newParentPath}${relativePath}`; return prisma.folder.update({ where: { id: subfolder.id, teamId }, data: { path: newSubfolderPath }, }); - }); + }).filter(Boolean) as Promise<unknown>[];pages/api/teams/[teamId]/documents/search.ts (1)
41-45: Bug:containsreceivesundefinedwhenqueryis absentUnconditional filter will throw at runtime in Prisma when
queryis undefined. Make the filter conditional (and optionally trim).Apply:
- where: { - teamId: teamId, - name: { - contains: query, - mode: "insensitive", - }, - }, + where: { + teamId: teamId, + ...(query?.trim() + ? { + name: { + contains: query.trim(), + mode: "insensitive", + }, + } + : {}), + },pages/api/teams/[teamId]/documents/[id]/versions/index.ts (4)
143-146:teamis undefined inconversionQueue(team.plan)These trigger options reference
team.planbut noteamis fetched. Fetch the plan after authorization.if (!teamAccess) { return res.status(401).end("Unauthorized"); } + // Fetch team plan for queue selection + const team = await prisma.team.findUnique({ + where: { id: teamId }, + select: { plan: true }, + }); + if (!team) { + return res.status(404).json({ error: "Team not found" }); + }Also applies to: 169-171, 192-194, 64-67
89-92: Version number calc breaks when no prior versions
document.versionsis always truthy (empty array), causingversions[0]to beundefined.- const currentVersionNumber = document?.versions - ? document.versions[0].versionNumber - : 1; + const currentVersionNumber = document.versions[0]?.versionNumber ?? 0;
118-127: DuplicateupdateManydemotionThe second demotion block is redundant.
- // turn off isPrimary flag for all other versions - await prisma.documentVersion.updateMany({ - where: { - documentId: documentId, - id: { not: version.id }, - }, - data: { - isPrimary: false, - }, - });
219-221: Wrong Allow header and messageEndpoint supports POST, not GET.
- // We only allow GET requests - res.setHeader("Allow", ["GET"]); + // We only allow POST requests + res.setHeader("Allow", ["POST"]);pages/api/teams/[teamId]/datarooms/[id]/permission-groups/index.ts (1)
143-156:findUniquewith relational filter in POST authUse the same
userTeamcomposite check as GET; currentteam.findUniquefilter is invalid and inconsistent.- // Verify team membership - const team = await prisma.team.findUnique({ - where: { - id: teamId, - users: { - some: { userId }, - }, - }, - }); - - if (!team) { + // Verify team membership + const teamAccess = await prisma.userTeam.findUnique({ + where: { userId_teamId: { userId, teamId } }, + }); + + if (!teamAccess) { return res.status(401).end("Unauthorized"); }pages/api/teams/[teamId]/documents/index.ts (1)
193-221: Eliminate N+1 folder lookups when buildingfolderListBatch fetch unique folder paths once.
- documentsWithFolderList = await Promise.all( - documentsWithCounts.map(async (doc) => { - const folderNames = []; - const pathSegments = doc.folder?.path?.split("/") || []; - - if (pathSegments.length > 0) { - const folders = await prisma.folder.findMany({ - where: { - teamId, - path: { - in: pathSegments.map((_, index) => - pathSegments.slice(0, index + 1).join("/"), - ), - }, - }, - select: { - path: true, - name: true, - }, - orderBy: { - path: "asc", - }, - }); - folderNames.push(...folders.map((f) => f.name)); - } - return { ...doc, folderList: folderNames }; - }), - ); + // Collect all unique folder paths across docs + const allPaths = new Set<string>(); + for (const doc of documentsWithCounts) { + const segments = doc.folder?.path?.split("/") || []; + segments.forEach((_, i) => + allPaths.add(segments.slice(0, i + 1).join("/")), + ); + } + const folders = await prisma.folder.findMany({ + where: { teamId, path: { in: Array.from(allPaths).filter(Boolean) } }, + select: { path: true, name: true }, + }); + const nameByPath = new Map(folders.map((f) => [f.path, f.name])); + documentsWithFolderList = documentsWithCounts.map((doc) => { + const segments = doc.folder?.path?.split("/") || []; + const folderList = segments + .map((_, i) => segments.slice(0, i + 1).join("/")) + .map((p) => nameByPath.get(p)) + .filter(Boolean) as string[]; + return { ...doc, folderList }; + });pages/api/teams/[teamId]/datarooms/[id]/groups/index.ts (2)
45-55: Bug: prisma.dataroom.findUnique uses non-unique filter.
findUnique requires a unique field/composite. Passing both id and teamId at the top level is invalid unless you target a defined composite key. Use findFirst with a compound where, or target the composite unique name.Apply one of these:
- const dataroom = await prisma.dataroom.findUnique({ - where: { - id: dataroomId, - teamId: teamId, - }, - select: { - id: true, - teamId: true, - name: true, - }, - }); + const dataroom = await prisma.dataroom.findFirst({ + where: { id: dataroomId, teamId }, + select: { id: true, teamId: true, name: true }, + });Or, if a composite unique exists:
- const dataroom = await prisma.dataroom.findUnique({ - where: { id: dataroomId, teamId }, + const dataroom = await prisma.dataroom.findUnique({ + where: { teamId_id: { teamId, id: dataroomId } }, select: { id: true, teamId: true, name: true }, });
165-171: Validate dataroom existence/team match before create.
Prevents cross-team linkage and improves error clarity.const group = await prisma.viewerGroup.create({ - data: { - name: name, - dataroomId, - teamId, - }, - }); + // ensure dataroom exists and belongs to team + }); + const dr = await prisma.dataroom.findFirst({ where: { id: dataroomId, teamId } }); + if (!dr) { + return res.status(404).end("Dataroom not found"); + } + const group = await prisma.viewerGroup.create({ + data: { name, dataroomId, teamId }, + });
🧹 Nitpick comments (27)
pages/api/teams/[teamId]/documents/[id]/update-name.ts (3)
35-38: Use 403 for authenticated‑but‑not‑authorized and keep error shape consistentIf the user is logged in but lacks team access, prefer
403and JSON to match other errors.- if (!teamAccess) { - return res.status(401).end("Unauthorized"); - } + if (!teamAccess) { + return res.status(403).json({ error: "Forbidden" }); + }
55-57: Validate and bound the input nameGuard against empty/oversized names before persisting. The diff in the previous comment adds a minimal validation; tighten further if you already enforce different limits elsewhere.
15-15: Fix the route comment (GET → POST)The docstring says GET but the handler only allows POST.
- // GET /api/teams/:teamId/documents/:id/update-name + // POST /api/teams/:teamId/documents/:id/update-nameprisma/schema/document.prisma (1)
64-69: Consolidate and direction-align the new composite index.To fully support queries like
where { documentId, isPrimary } orderBy { createdAt: 'desc' }and avoid redundant indexes, set an explicit DESC oncreatedAthere and consider dropping the overlapping two-index combo in a follow-up migration.- @@index([documentId, isPrimary, createdAt]) // Optimize primary version lookups with ordering + @@index([documentId, isPrimary, createdAt(sort: Desc)]) // Optimize primary version lookups with orderingOptional follow-up (separate migration): drop either
@@index([documentId, isPrimary])and/or@@index([documentId, createdAt(sort: Desc)])if this composite fully covers their use cases.pages/api/teams/[teamId]/folders/[...name].ts (1)
89-91: Fix comment: method guard says POST but route allows GET.Tiny doc nit.
- // We only allow POST requests + // We only allow GET requestspages/api/teams/[teamId]/documents/[id]/overview.ts (2)
78-82: Trim the selected fields on versions for lighter payloads.You only need the latest primary version’s
id(and maybefileSize). Reduce selected fields to cut response size.versions: { where: { isPrimary: true }, orderBy: { createdAt: "desc" }, take: 1, + select: { id: true, fileSize: true, createdAt: true }, },
119-125: 404 on “no primary version” conflates states.Returning 404 when the document exists but has no primary version muddies error semantics. Consider 200 with
primaryVersion: nullandisEmpty: true, or a 409/422 to indicate incomplete state.pages/api/teams/[teamId]/folders/parents/[...name].ts (1)
33-35: Batch parent path lookups and type the accumulator.Avoid N queries in the loop and add a concrete type.
- const validatedName = nameValidation.data; - let folderNames = []; + const validatedName = nameValidation.data; + const folderNames: Array<{ name: string; path: string }> = []; @@ - for (let i = 0; i < validatedName.length; i++) { - const path = "/" + validatedName.slice(0, i + 1).join("/"); // construct the materialized path - - const folder = await prisma.folder.findUnique({ - where: { - teamId_path: { - teamId: teamId, - path: path, - }, - }, - select: { - id: true, - parentId: true, - name: true, - }, - }); + const paths = validatedName.map((_, i) => "/" + validatedName.slice(0, i + 1).join("/")); + const folders = await prisma.folder.findMany({ + where: { teamId, path: { in: paths } }, + select: { name: true, path: true }, + orderBy: { path: "asc" }, + }); @@ - folderNames.push({ name: folder.name, path: path }); + folders.forEach((f) => folderNames.push({ name: f.name, path: f.path }));Also applies to: 51-73
pages/api/teams/[teamId]/documents/[id]/annotations/index.ts (1)
110-116: Validate page numbers against document.numPages (defensive check).Prevent out‑of‑range pages from being stored.
- const document = await prisma.document.findUnique({ + const document = await prisma.document.findUnique({ where: { id: docId, teamId, }, - select: { id: true }, + select: { id: true, numPages: true }, }); @@ - const annotation = await prisma.documentAnnotation.create({ + // Validate pages are within bounds + if ( + document.numPages && + (validatedData.pages.some((p) => p < 1) || + Math.max(...validatedData.pages) > document.numPages) + ) { + return res.status(400).json({ + error: "Page out of bounds", + details: `Allowed range: 1..${document.numPages}`, + }); + } + + const annotation = await prisma.documentAnnotation.create({Also applies to: 122-130
pages/api/teams/[teamId]/documents/update.ts (1)
23-29: Add input validation and return 200 for updates (not 201).Validate
documentIdand coercion/bounds fornumPages; use 200/204 for update.+import { z } from "zod"; @@ - // Assuming data is an object with `name` and `description` properties - const { documentId, numPages } = req.body; + const updateSchema = z.object({ + documentId: z.string().min(1), + numPages: z.coerce.number().int().positive().max(20000), + }); + const { documentId, numPages } = updateSchema.parse(req.body); @@ - return res.status(201).json({ message: "Document updated successfully" }); + return res.status(200).json({ message: "Document updated successfully" });Also applies to: 56-71
pages/api/teams/[teamId]/documents/[id]/views/[viewId]/stats.ts (2)
66-71: Guard against empty duration data from Tinybird.Avoid reduce on undefined/null.
- const total_duration = duration.data.reduce( + const rows = Array.isArray(duration?.data) ? duration.data : []; + const total_duration = rows.reduce( (totalDuration, data) => totalDuration + data.sum_duration, 0, );
78-80: Fix comment: only GET is allowed.Header is
Allow: ["GET"]; update the comment for consistency.- // We only allow GET and POST requests + // We only allow GET requestspages/api/teams/[teamId]/documents/[id]/annotations/[annotationId]/images.ts (1)
77-89: Strengthen file validations (size bounds, stricter MIME).Bound
sizeand consider a MIME whitelist. Client‑providedmimeTypeis untrusted; prefer metadata from your storage callback when available.- if (!mimeType.startsWith("image/")) { + const allowedMime = new Set([ + "image/png", + "image/jpeg", + "image/webp", + "image/gif", + "image/svg+xml", + ]); + if (!allowedMime.has(mimeType)) { return res.status(400).json({ error: "Only image files are allowed" }); } + + if ( + size != null && + (!Number.isFinite(size) || size < 0 || size > 25 * 1024 * 1024) + ) { + return res.status(400).json({ error: "Image size must be between 0 and 25 MB" }); + }Also applies to: 90-101
prisma/migrations/20250917000000_add_performance_index_dataroom/migration.sql (1)
1-12: Plan index rollout to avoid locks on large tablesIf you’re on Postgres with sizable tables, consider out-of-band
CREATE INDEX CONCURRENTLYduring a maintenance window (Prisma Migrate can’t use CONCURRENTLY inside transactions). Otherwise, expect lock contention.pages/api/teams/[teamId]/documents/search.ts (2)
52-76: Skip count queries when there are no documentsAvoid three extra round-trips when
documentIds.length === 0.const documentIds = documents.map((d) => d.id); + if (documentIds.length === 0) { + return res.status(200).json([]); + }
79-87: Type the maps to tighten TS and preventanydriftSmall cleanup; no behavior change.
- const linkCountMap = new Map( + const linkCountMap = new Map<string, number>( linkCounts.map((lc) => [lc.documentId, lc._count.id]), ); - const viewCountMap = new Map( + const viewCountMap = new Map<string, number>( viewCounts.map((vc) => [vc.documentId, vc._count.id]), ); - const versionCountMap = new Map( + const versionCountMap = new Map<string, number>( versionCounts.map((vsc) => [vsc.documentId, vsc._count.id]), );pages/api/teams/[teamId]/documents/[id]/versions/index.ts (2)
88-116: Make create+demote atomic to prevent two primariesWrap creation + demotion in a transaction to avoid races under concurrent uploads.
- const version = await prisma.documentVersion.create({ - data: { - documentId: documentId, - file: url, - originalFile: url, - type: type, - storageType, - numPages: document?.advancedExcelEnabled ? 1 : numPages, - isPrimary: true, - versionNumber: currentVersionNumber + 1, - contentType, - fileSize, - }, - }); - - // turn off isPrimary flag for all other versions - await prisma.documentVersion.updateMany({ - where: { - documentId: documentId, - id: { not: version.id }, - }, - data: { - isPrimary: false, - }, - }); + const [version] = await prisma.$transaction([ + prisma.documentVersion.create({ + data: { + documentId: documentId, + file: url, + originalFile: url, + type, + storageType, + numPages: document.advancedExcelEnabled ? 1 : numPages, + isPrimary: true, + versionNumber: currentVersionNumber + 1, + contentType, + fileSize, + }, + }), + prisma.documentVersion.updateMany({ + where: { documentId: documentId }, + data: { isPrimary: false }, + }), + ]); + // Re-promote the created version + await prisma.documentVersion.update({ + where: { id: version.id }, + data: { isPrimary: true }, + });
158-161: FragiledocIdparsing from URL
url.split("/")[1]assumes a strict path layout. Prefer deriving from a known key format or passdocumentIddirectly when available.pages/api/teams/[teamId]/datarooms/[id]/permission-groups/index.ts (2)
66-68: Short-circuit when no groupsAvoid extra queries when
groupIds.length === 0.const groupIds = permissionGroups.map((g) => g.id); + if (groupIds.length === 0) { + return res.status(200).json([]); + }
88-96: Type the maps for clarityUse generics to avoid
any[].- const accessControlsMap = new Map<string, any[]>(); - const linksMap = new Map<string, any[]>(); + const accessControlsMap = new Map<string, typeof accessControls>(); + const linksMap = new Map<string, { id: string; name: string }[]>();Also applies to: 99-110
pages/api/teams/[teamId]/folders/documents/[...name].ts (2)
80-82: Skip count queries when no documentsTiny latency win on empty folders.
const documentIds = documents.map((d) => d.id); + if (documentIds.length === 0) { + return res.status(200).json([]); + }
115-127: Type the mapsNo behavior change; improves readability.
- const linkCountMap = new Map( + const linkCountMap = new Map<string, number>( linkCounts.map((lc) => [lc.documentId, lc._count.id]), ); - const viewCountMap = new Map( + const viewCountMap = new Map<string, number>( viewCounts.map((vc) => [vc.documentId, vc._count.id]), ); - const versionCountMap = new Map( + const versionCountMap = new Map<string, number>( versionCounts.map((vsc) => [vsc.documentId, vsc._count.id]), ); - const dataroomCountMap = new Map( + const dataroomCountMap = new Map<string, number>( dataroomCounts.map((dc) => [dc.documentId, dc._count.id]), );pages/api/teams/[teamId]/documents/index.ts (2)
130-164: Skip count queries when no documentsSave 4 queries on empty result sets.
const documentIds = documents.map((d) => d.id); + if (documentIds.length === 0) { + const documentsWithCounts = documents.map((document) => ({ + ...document, + _count: { links: 0, views: 0, versions: 0, datarooms: 0 }, + })); + return res.status(200).json({ + documents: documentsWithCounts, + ...(usePagination && { + pagination: { + total: totalDocuments, + pages: Math.ceil(totalDocuments! / limit!), + currentPage: page, + pageSize: limit, + }, + }), + }); + }
166-177: Type the mapsSmall TS improvement; consistent with other files.
- const linkCountMap = new Map( + const linkCountMap = new Map<string, number>( linkCounts.map((lc) => [lc.documentId, lc._count.id]), ); - const viewCountMap = new Map( + const viewCountMap = new Map<string, number>( viewCounts.map((vc) => [vc.documentId, vc._count.id]), ); - const versionCountMap = new Map( + const versionCountMap = new Map<string, number>( versionCounts.map((vsc) => [vsc.documentId, vsc._count.id]), ); - const dataroomCountMap = new Map( + const dataroomCountMap = new Map<string, number>( dataroomCounts.map((dc) => [dc.documentId, dc._count.id]), );pages/api/teams/[teamId]/datarooms/[id]/groups/index.ts (3)
27-29: Normalize documentId from req.query (string|string[]).
Next.js query params can be arrays; casting to string risks runtime errors in the include filter.- const documentId = req.query?.documentId as string; - const userId = (session.user as CustomUser).id; + const docRaw = req.query?.documentId; + const documentId = Array.isArray(docRaw) ? docRaw[0] : docRaw; + const userId = (session.user as CustomUser).id;
89-92: Guard empty IN lists to skip unnecessary groupBy calls.
Minor save and avoids edge-case SQL on some drivers.const groupIds = viewerGroups.map((g) => g.id); + if (groupIds.length === 0) { + return res.status(200).json([]); + }
175-177: Unify error handling and fix message.
Use shared errorhandler/log; correct resource name.- console.error("Request error", error); - res.status(500).json({ error: "Error creating folder" }); + log({ message: `Failed to create viewer group for dataroom ${dataroomId}. ${error}`, type: "error" }); + errorhandler(error, res);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
pages/api/teams/[teamId]/datarooms/[id]/groups/index.ts(5 hunks)pages/api/teams/[teamId]/datarooms/[id]/links.ts(0 hunks)pages/api/teams/[teamId]/datarooms/[id]/permission-groups/index.ts(3 hunks)pages/api/teams/[teamId]/documents/[id]/annotations/[annotationId].ts(1 hunks)pages/api/teams/[teamId]/documents/[id]/annotations/[annotationId]/images.ts(1 hunks)pages/api/teams/[teamId]/documents/[id]/annotations/index.ts(2 hunks)pages/api/teams/[teamId]/documents/[id]/index.ts(1 hunks)pages/api/teams/[teamId]/documents/[id]/links.ts(0 hunks)pages/api/teams/[teamId]/documents/[id]/overview.ts(2 hunks)pages/api/teams/[teamId]/documents/[id]/update-name.ts(1 hunks)pages/api/teams/[teamId]/documents/[id]/versions/index.ts(1 hunks)pages/api/teams/[teamId]/documents/[id]/views/[viewId]/stats.ts(2 hunks)pages/api/teams/[teamId]/documents/agreement.ts(1 hunks)pages/api/teams/[teamId]/documents/index.ts(5 hunks)pages/api/teams/[teamId]/documents/search.ts(2 hunks)pages/api/teams/[teamId]/documents/update.ts(2 hunks)pages/api/teams/[teamId]/folders/[...name].ts(1 hunks)pages/api/teams/[teamId]/folders/documents/[...name].ts(3 hunks)pages/api/teams/[teamId]/folders/index.ts(2 hunks)pages/api/teams/[teamId]/folders/move.ts(1 hunks)pages/api/teams/[teamId]/folders/parents/[...name].ts(1 hunks)prisma/migrations/20250917000000_add_performance_index_dataroom/migration.sql(1 hunks)prisma/schema/dataroom.prisma(2 hunks)prisma/schema/document.prisma(1 hunks)prisma/schema/link.prisma(1 hunks)
💤 Files with no reviewable changes (2)
- pages/api/teams/[teamId]/datarooms/[id]/links.ts
- pages/api/teams/[teamId]/documents/[id]/links.ts
⏰ 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). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (25)
pages/api/teams/[teamId]/documents/[id]/update-name.ts (1)
26-33: Membership check via composite key looks good; confirm role gating as neededUsing
userId_teamIdfor O(1) auth is solid. Please confirm this endpoint should allow any member to rename documents. If edit is role‑gated, add a role check in the same query/select.pages/api/teams/[teamId]/folders/parents/[...name].ts (1)
38-49: LGTM: standardized auth via userTeam composite key.Consistent with other endpoints; early 401 is good.
pages/api/teams/[teamId]/documents/[id]/annotations/index.ts (1)
43-66: LGTM: explicit membership + document existence checks improve clarity and performance.Early 401/404 returns are solid; good move away from the heavy helper.
Also applies to: 97-121
pages/api/teams/[teamId]/documents/update.ts (1)
31-43: LGTM: standardized auth flow.Two-phase membership + existence checks are consistent with the PR.
pages/api/teams/[teamId]/documents/[id]/views/[viewId]/stats.ts (1)
35-59: LGTM: explicit membership + doc check; early returns are correct.Consistent with the refactor across endpoints; good.
pages/api/teams/[teamId]/documents/[id]/annotations/[annotationId]/images.ts (1)
38-63: LGTM: explicit authorization + existence checks; solid validation.Good 401/404 handling and basic input checks.
pages/api/teams/[teamId]/folders/[...name].ts (1)
38-47: LGTM — composite key verifiedprisma/schema/team.prisma defines model UserTeam with @@id([userId, teamId]); prisma.userTeam.findUnique({ where: { userId_teamId: { userId, teamId } } }) is valid and more efficient.
pages/api/teams/[teamId]/documents/[id]/annotations/[annotationId].ts (1)
48-59: Good: lightweight, indexed team membership check via userTeam composite keyThis aligns with the PR’s pattern and reduces over-fetching.
pages/api/teams/[teamId]/documents/[id]/index.ts (1)
44-57: Good: cheap team membership gate before heavier document loadThis should cut unnecessary document queries for unauthorized users.
prisma/schema/dataroom.prisma (2)
70-71: Index on DataroomDocument.documentId looks right for document-scoped fetchesGood addition to speed up lookups by document.
128-131: Composite index on ViewerGroup(dataroomId, createdAt) is appropriate for sorted paginationSolid choice for time-ordered group listings per dataroom.
prisma/schema/link.prisma (1)
90-91: Index on permissionGroupId is appropriate for permission-group joinsMatches the new query patterns; low risk.
prisma/migrations/20250917000000_add_performance_index_dataroom/migration.sql (1)
1-12: Indexes match schema changes and target the new hot pathsLooks consistent with the Prisma schema updates.
pages/api/teams/[teamId]/folders/index.ts (2)
26-36: Good: switched to composite userTeam lookup for membershipLean check, avoids loading Team graph.
110-121: Same here: consistent, cheap membership gateMatches the new pattern; minimal data fetched.
pages/api/teams/[teamId]/datarooms/[id]/permission-groups/index.ts (1)
55-65: Two-phase fetch LGTMGood split of base entities and nested data; aligns with PR goals.
pages/api/teams/[teamId]/folders/documents/[...name].ts (1)
141-143: Fix error message to match endpointThe route fetches documents; message says “folders”.
[raise_nitpick_refactor]- return res.status(500).json({ error: "Error fetching folders" }); + return res.status(500).json({ error: "Error fetching documents" });pages/api/teams/[teamId]/documents/[id]/versions/index.ts (1)
323-336: ```shell
#!/bin/bash
set -euo pipefail
echo "Searching repo for improper prisma findUnique usages with relational filters..."1) Show all findUnique occurrences with context
echo -e "\n== All findUnique occurrences (context 3 lines) =="
rg -n --hidden --no-ignore -S -C3 "findUnique\s*(" || true2) Multiline search: findUnique(...) with where:{ ... some|every|none ... }
echo -e "\n== findUnique with 'some|every|none' inside where (multiline) =="
rg -nP --hidden --no-ignore -U "findUnique(\s*{(?:.|\n)?where\s:\s*{(?:.|\n)*?\b(?:some|every|none)\b" -C6 || true3) findUnique where clause referencing relation keys with an object (e.g. users: { some: ... } )
echo -e "\n== findUnique with relation keys (users|members|teams|participants|owners|memberships) as objects inside where =="
rg -nP --hidden --no-ignore -U "findUnique(\s*{(?:.|\n)?where\s:\s*{(?:.|\n)?\b(?:users|members|teams|participants|owners|memberships)\s:\s*{" -C6 || true4) As a fallback, list files containing 'findUnique' for manual inspection
echo -e "\n== Files containing 'findUnique' =="
rg -n --hidden --no-ignore -S "findUnique" --files-with-matches | sort -u || trueecho -e "\nDone."
</blockquote></details> <details> <summary>pages/api/teams/[teamId]/datarooms/[id]/groups/index.ts (7)</summary><blockquote> `4-4`: **Good add: ItemType import used for filtered include.** Keeps the include predicate explicit. --- `61-87`: **AccessControls exposure—confirm intended audience.** Including canView/canDownload for all team members may be sensitive; confirm this aligns with your RBAC. --- `120-123`: **_count field naming—ensure consistency with Prisma relation names.** Keys ‘members’ and ‘views’ should match client expectations; if other endpoints return `_count.memberships`, this will diverge. --- `152-160`: **POST auth LGTM—same composite check as GET.** Consistent with the new pattern. --- `40-42`: **401 vs 403 semantics—confirm consistency.** For authenticated-but-not-authorized, many APIs prefer 403. If 401 is a deliberate convention, ignore. Also applies to: 161-163 --- `126-126`: **Response shape changed — verify/update all callers** - Change: endpoint now returns an array of groups (viewerGroupsWithCounts) instead of a dataroom object with nested groups — callers reading dataroom.groups will break. - Evidence: sandbox ripgrep returned "No files were searched" so callers weren't discoverable here. - Action: run a local repo search and update all consumers, types and tests. Suggested commands: rg -n --hidden -uu -S '\bdataroom\.groups\b' -g '!**/dist/**' -g '!**/build/**' -C2 rg -n --hidden -uu -S 'viewerGroupsWithCounts' -C2 --- `31-38`: **Auth check OK — composite primary key exists.** model UserTeam declares @@id([userId, teamId]) in prisma/schema/team.prisma, so prisma.userTeam.findUnique({ where: { userId_teamId: { userId, teamId } } }) is valid. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pages/api/teams/[teamId]/documents/[id]/update-name.ts (1)
58-79: Fix Prisma misuse, remove side‑effects inside transactions, and collapse to a single atomic update.Problems:
findUnique({ where: { id: docId, teamId }})is invalid unless you use the generated compound key name; same forupdate({ where: { id, teamId }}). This likely won’t compile.- Calling
res.status(...).json(...)inside$transactionis unsafe (side‑effects may run multiple times on transaction retries) and leads to double‑send (you later send 200). This will throw “Cannot set headers after they are sent.”- Two queries (exists then update) create a TOCTOU window and extra round trip.
Solution: validate input once, then perform a single
updateManyscoped by{ id: docId, teamId }, checkcount, and respond outside of any transaction. This is atomic, retry‑safe, and index‑friendly.Apply this diff:
- // Atomic transaction to update document name - const result = await prisma.$transaction(async (tx) => { - // Perform atomic update with both teamId and id in the filter - const document = await tx.document.findUnique({ - where: { - id: docId, - teamId: teamId, // Ensure document belongs to the team - }, - select: { id: true }, - }); - - if (!document) { - return res.status(404).json({ error: "Document not found" }); - } - - const updateResult = await tx.document.update({ - where: { id: docId, teamId }, - data: { name: name }, - }); - - return updateResult; - }); - - // Check if any rows were affected - if (!result) { - return res.status(404).json({ error: "Document not found" }); - } + // Single atomic update scoped by teamId + id + const update = await prisma.document.updateMany({ + where: { id: docId, teamId }, + data: { name }, + }); + if (update.count === 0) { + return res.status(404).json({ error: "Document not found" }); + }If you do have a composite unique on
(teamId, id)and want the updated record back, you can alternatively use:await prisma.document.update({ where: { teamId_id: { teamId, id: docId } }, // adjust compound key name data: { name }, });Follow‑up: ensure an index/unique on
(teamId, id)exists for this hot path.#!/bin/bash # Check for composite index/unique on Document(teamId, id) fd -a schema.prisma | while read -r f; do echo ">>> $f" rg -nC2 -e 'model\s+Document\b' -e '@@unique\s*\(\s*\[\s*teamId\s*,\s*id\s*\]' -e '@@index\s*\(\s*teamId\s*,\s*id\s*\)' "$f" doneAlso applies to: 81-85
🧹 Nitpick comments (3)
prisma/schema/schema.prisma (1)
147-159: Optionally tighten indexes to match query shapesIf your hot paths filter per dataroom or fetch top-N by time within a group, consider these additions.
Option A — per‑dataroom grouping:
@@ model View { @@index([viewerId]) @@index([groupId]) // Performance optimization for groupBy queries on groupId + @@index([dataroomId, groupId]) // If queries commonly filter by dataroomId + groupId @@index([teamId])Option B — recent views per group:
@@ model View { - @@index([groupId]) // Performance optimization for groupBy queries on groupId + @@index([groupId]) // Performance optimization for groupBy queries on groupId + @@index([groupId, viewedAt(sort: Desc)]) // If you paginate/sort by viewedAt within a groupIf most analytics exclude archived rows, a partial index can help:
- Do in SQL migration (Prisma schema doesn’t express partial indexes); e.g., CREATE INDEX CONCURRENTLY IF NOT EXISTS "View_groupId_active_idx" ON "View" ("groupId") WHERE "isArchived" = false;
Would you like a tailored proposal after you share the exact WHERE/ORDER BY patterns from the impacted queries?
pages/api/teams/[teamId]/documents/[id]/update-name.ts (2)
11-16: Harden validation: trim and reject whitespace‑only names.Currently, " " passes
.min(1). Add.trim()to normalize and enforce non‑empty after trimming.Apply this diff:
-const updateNameSchema = z.object({ - name: z - .string() - .min(1, "Document name is required") - .max(255, "Document name too long"), -}); +const updateNameSchema = z.object({ + name: z + .string() + .trim() + .min(1, "Document name is required") + .max(255, "Document name too long"), +});
34-41: Consider a stable, UI‑friendly error shape.Returning raw
issuesis verbose and shape‑unstable across Zod versions. Prefer a flattened list or first error string to keep the API contract tight.Example:
- return res.status(400).json({ - error: "Invalid input", - details: validationResult.error.issues, - }); + const errors = validationResult.error.flatten().fieldErrors; + return res.status(400).json({ error: "Invalid input", errors });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pages/api/teams/[teamId]/documents/[id]/update-name.ts(2 hunks)prisma/migrations/20250917000000_add_performance_index_dataroom/migration.sql(1 hunks)prisma/schema/schema.prisma(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- prisma/migrations/20250917000000_add_performance_index_dataroom/migration.sql
🔇 Additional comments (4)
prisma/schema/schema.prisma (1)
152-152: LGTM — targeted FK index on View.groupId confirmedMigration(s) contain CREATE INDEX "View_groupId_idx"; no duplicate index on View.groupId found; package.json shows prisma and @prisma/client at ^6.5.0.
pages/api/teams/[teamId]/documents/[id]/update-name.ts (3)
5-5: LGTM on adding Zod.Good choice for concise validation with minimal overhead.
43-44: LGTM on using parsed data.Accessing
validationResult.dataavoids trustingreq.body. This is correct.
54-57: LGTM on 401 for missing membership.Short‑circuiting early is correct; keeps the hot path lean.
Summary by CodeRabbit
Refactor
Performance
Bug Fixes
Chores