Merged
Conversation
…crumbs - API: search folders by name when query is present, build parent path breadcrumbs (folderList), exclude hidden folders and children of hidden folders, return matching folders alongside documents - SWR hook: add FolderWithCountAndPath type, expose searchFolders from useDocuments hook - Documents page: display search-matched folders when filtered instead of empty array, pass correct loading state for filtered folders - Folder card: render path breadcrumb (Home > parent > folder) when search query is active, matching the existing document card pattern - Pagination: add optional extraInfo prop showing folder match count Co-authored-by: Marc Seitz <mfts@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughAdds folder path data and breadcrumb UI for search results: the API and hook now surface matching folders with ancestor paths, components accept the new folder shape, FolderCard renders breadcrumbs when searching, and Pagination can show an extraInfo string with folder counts. Changes
Possibly related PRs
🚥 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. 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
components/documents/folder-card.tsx (1)
71-74: Unused variablesortQuery.The
sortQueryvariable is extracted from query params but never used in this component.🧹 Remove unused variable
const router = useRouter(); const queryParams = router.query; const searchQuery = queryParams["search"]; - const sortQuery = queryParams["sort"]; const folderList = "folderList" in folder ? folder.folderList : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/documents/folder-card.tsx` around lines 71 - 74, The variable sortQuery is extracted from router.query but not used; remove the unused declaration to satisfy linting and reduce clutter by deleting the line that assigns sortQuery (the const sortQuery = queryParams["sort"]; statement) in the component where router.query and searchQuery are handled (leave router.query and searchQuery intact and keep folderList logic as-is).pages/api/teams/[teamId]/documents/index.ts (2)
281-307: N+1 query pattern for parent folder lookups.Each matching folder triggers a separate query to fetch its parent path hierarchy. For searches returning many folders, this could impact performance.
Consider batching: collect all unique parent paths from all matching folders first, then fetch them in a single query, and map results back to each folder.
⚡ Batch parent folder lookups
+ // Collect all unique parent paths across all folders + const allParentPaths = new Set<string>(); + for (const folder of folders) { + const parentPath = folder.path.substring(0, folder.path.lastIndexOf("/")); + if (parentPath) { + const pathSegments = parentPath.split("/").filter(Boolean); + for (let i = 0; i < pathSegments.length; i++) { + allParentPaths.add("/" + pathSegments.slice(0, i + 1).join("/")); + } + } + } + + // Single query for all parent folders + const allParentFolders = allParentPaths.size > 0 + ? await prisma.folder.findMany({ + where: { teamId, path: { in: Array.from(allParentPaths) } }, + select: { path: true, name: true }, + }) + : []; + const parentFolderMap = new Map(allParentFolders.map((f) => [f.path, f.name])); + + // Build folderList for each folder using the map matchingFolders = folders.map((folder) => { - // ... async parent lookup + const folderNames: string[] = []; + const parentPath = folder.path.substring(0, folder.path.lastIndexOf("/")); + if (parentPath) { + const pathSegments = parentPath.split("/").filter(Boolean); + for (let i = 0; i < pathSegments.length; i++) { + const segmentPath = "/" + pathSegments.slice(0, i + 1).join("/"); + const name = parentFolderMap.get(segmentPath); + if (name) folderNames.push(name); + } + } + return { ...folder, folderList: folderNames }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/teams/`[teamId]/documents/index.ts around lines 281 - 307, The current folders.map block causes an N+1 query pattern by calling prisma.folder.findMany for each folder to fetch parentPaths; instead, gather all unique parentPaths for every folder (use the same parentPath split logic to produce parentPaths arrays and flatten+dedupe), run a single prisma.folder.findMany({ where: { teamId, path: { in: allParentPaths } }, select: { path, name } }) to fetch all parent folders, then build a lookup map from path to name and map those names back into each original folder to set folderList (replace the per-folder prisma call inside the matchingFolders computation with the batched query + mapping).
251-251: Avoidany[]type for better type safety.Using
any[]loses type information. Consider defining or reusing a proper type.🛡️ Add explicit typing
- let matchingFolders: any[] = []; + let matchingFolders: Array<{ + id: string; + name: string; + path: string; + parentId: string | null; + _count: { documents: number; childFolders: number }; + folderList: string[]; + [key: string]: unknown; + }> = [];Alternatively, import and extend
FolderWithCountfrom the types if available server-side.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/api/teams/`[teamId]/documents/index.ts at line 251, The variable matchingFolders is typed as any[] which loses type safety; change it to a concrete folder type by importing or defining the appropriate type (e.g., FolderWithCount or an interface matching the folder shape) and declare matchingFolders with that type (for example: let matchingFolders: FolderWithCount[] = [];), then update any downstream uses to respect the new properties (or narrow types) and add/adjust imports for the type in the file; if a server-side type isn’t available, create a local interface (e.g., interface FolderWithCount { id: string; name: string; documentCount: number; /*...*/ }) and use that instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/documents/folder-card.tsx`:
- Around line 71-74: The variable sortQuery is extracted from router.query but
not used; remove the unused declaration to satisfy linting and reduce clutter by
deleting the line that assigns sortQuery (the const sortQuery =
queryParams["sort"]; statement) in the component where router.query and
searchQuery are handled (leave router.query and searchQuery intact and keep
folderList logic as-is).
In `@pages/api/teams/`[teamId]/documents/index.ts:
- Around line 281-307: The current folders.map block causes an N+1 query pattern
by calling prisma.folder.findMany for each folder to fetch parentPaths; instead,
gather all unique parentPaths for every folder (use the same parentPath split
logic to produce parentPaths arrays and flatten+dedupe), run a single
prisma.folder.findMany({ where: { teamId, path: { in: allParentPaths } },
select: { path, name } }) to fetch all parent folders, then build a lookup map
from path to name and map those names back into each original folder to set
folderList (replace the per-folder prisma call inside the matchingFolders
computation with the batched query + mapping).
- Line 251: The variable matchingFolders is typed as any[] which loses type
safety; change it to a concrete folder type by importing or defining the
appropriate type (e.g., FolderWithCount or an interface matching the folder
shape) and declare matchingFolders with that type (for example: let
matchingFolders: FolderWithCount[] = [];), then update any downstream uses to
respect the new properties (or narrow types) and add/adjust imports for the type
in the file; if a server-side type isn’t available, create a local interface
(e.g., interface FolderWithCount { id: string; name: string; documentCount:
number; /*...*/ }) and use that instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86af3754-4d4f-4fdb-aec6-1dac011c85db
📒 Files selected for processing (6)
components/documents/documents-list.tsxcomponents/documents/folder-card.tsxcomponents/documents/pagination.tsxlib/swr/use-documents.tspages/api/teams/[teamId]/documents/index.tspages/documents/index.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/api/teams/`[teamId]/documents/index.ts:
- Around line 253-287: The folders query using prisma.folder.findMany (assigning
to folders) is unbounded and should be limited/paginated like the documents
query: add a limit (take) and optional pagination (skip or cursor) parameters
and return pagination metadata; update the prisma.folder.findMany call to accept
query params (e.g., take/skip or cursor) and ensure ordering is stable (orderBy:
{ name: "asc", id: "asc" } or similar) so pagination is deterministic, and
mirror the same page-size/default cap used by the documents listing.
- Around line 305-338: The parentFolders query and subsequent
parentFolderNameByPath map can include hidden ancestors, leaking them into
folderList; update the prisma.folder.findMany call that populates parentFolders
to only return non-hidden ancestors (add where: { teamId, path: { in:
Array.from(allParentPaths) }, hiddenInAllDocuments: false }) or alternatively
filter parentFolders before building parentFolderNameByPath so
parentFolderNameByPath only contains folders with hiddenInAllDocuments ===
false, then leave the folders.map logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3031ddea-e4a3-418e-9724-97820b3fa9a3
📒 Files selected for processing (1)
pages/api/teams/[teamId]/documents/index.ts
Comment on lines
+253
to
+287
| const folders = await prisma.folder.findMany({ | ||
| where: { | ||
| teamId, | ||
| hiddenInAllDocuments: false, | ||
| name: { | ||
| contains: query, | ||
| mode: "insensitive", | ||
| }, | ||
| OR: [ | ||
| { parentId: null }, | ||
| { | ||
| parentFolder: { | ||
| hiddenInAllDocuments: false, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| include: { | ||
| _count: { | ||
| select: { | ||
| documents: { | ||
| where: { | ||
| hiddenInAllDocuments: false, | ||
| }, | ||
| }, | ||
| childFolders: { | ||
| where: { | ||
| hiddenInAllDocuments: false, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| orderBy: { name: "asc" }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Cap or paginate folder search results.
This query is unbounded, unlike the document query above. A broad term can return every matching folder plus all ancestor metadata in one request, which will dominate latency and payload size on large teams.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/teams/`[teamId]/documents/index.ts around lines 253 - 287, The
folders query using prisma.folder.findMany (assigning to folders) is unbounded
and should be limited/paginated like the documents query: add a limit (take) and
optional pagination (skip or cursor) parameters and return pagination metadata;
update the prisma.folder.findMany call to accept query params (e.g., take/skip
or cursor) and ensure ordering is stable (orderBy: { name: "asc", id: "asc" } or
similar) so pagination is deterministic, and mirror the same page-size/default
cap used by the documents listing.
Comment on lines
+305
to
+338
| const parentFolders = allParentPaths.size | ||
| ? await prisma.folder.findMany({ | ||
| where: { | ||
| teamId, | ||
| path: { in: Array.from(allParentPaths) }, | ||
| }, | ||
| select: { path: true, name: true }, | ||
| }) | ||
| : []; | ||
|
|
||
| const parentFolderNameByPath = new Map( | ||
| parentFolders.map((folder) => [folder.path, folder.name]), | ||
| ); | ||
|
|
||
| matchingFolders = folders.map((folder) => { | ||
| const folderNames: string[] = []; | ||
| const parentPath = folder.path.substring( | ||
| 0, | ||
| folder.path.lastIndexOf("/"), | ||
| ); | ||
|
|
||
| if (parentPath) { | ||
| const pathSegments = parentPath.split("/").filter(Boolean); | ||
| for (let index = 0; index < pathSegments.length; index++) { | ||
| const path = `/${pathSegments.slice(0, index + 1).join("/")}`; | ||
| const parentFolderName = parentFolderNameByPath.get(path); | ||
|
|
||
| if (parentFolderName) { | ||
| folderNames.push(parentFolderName); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return { ...folder, folderList: folderNames }; |
Contributor
There was a problem hiding this comment.
Don’t leak hidden ancestor names into folderList.
parentFolders is fetched without hiddenInAllDocuments: false, so search results can expose hidden ancestor names in the breadcrumb. That breaks the All Documents visibility rules.
Proposed fix
const parentFolders = allParentPaths.size
? await prisma.folder.findMany({
where: {
teamId,
+ hiddenInAllDocuments: false,
path: { in: Array.from(allParentPaths) },
},
select: { path: true, name: true },
})
: [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parentFolders = allParentPaths.size | |
| ? await prisma.folder.findMany({ | |
| where: { | |
| teamId, | |
| path: { in: Array.from(allParentPaths) }, | |
| }, | |
| select: { path: true, name: true }, | |
| }) | |
| : []; | |
| const parentFolderNameByPath = new Map( | |
| parentFolders.map((folder) => [folder.path, folder.name]), | |
| ); | |
| matchingFolders = folders.map((folder) => { | |
| const folderNames: string[] = []; | |
| const parentPath = folder.path.substring( | |
| 0, | |
| folder.path.lastIndexOf("/"), | |
| ); | |
| if (parentPath) { | |
| const pathSegments = parentPath.split("/").filter(Boolean); | |
| for (let index = 0; index < pathSegments.length; index++) { | |
| const path = `/${pathSegments.slice(0, index + 1).join("/")}`; | |
| const parentFolderName = parentFolderNameByPath.get(path); | |
| if (parentFolderName) { | |
| folderNames.push(parentFolderName); | |
| } | |
| } | |
| } | |
| return { ...folder, folderList: folderNames }; | |
| const parentFolders = allParentPaths.size | |
| ? await prisma.folder.findMany({ | |
| where: { | |
| teamId, | |
| hiddenInAllDocuments: false, | |
| path: { in: Array.from(allParentPaths) }, | |
| }, | |
| select: { path: true, name: true }, | |
| }) | |
| : []; | |
| const parentFolderNameByPath = new Map( | |
| parentFolders.map((folder) => [folder.path, folder.name]), | |
| ); | |
| matchingFolders = folders.map((folder) => { | |
| const folderNames: string[] = []; | |
| const parentPath = folder.path.substring( | |
| 0, | |
| folder.path.lastIndexOf("/"), | |
| ); | |
| if (parentPath) { | |
| const pathSegments = parentPath.split("/").filter(Boolean); | |
| for (let index = 0; index < pathSegments.length; index++) { | |
| const path = `/${pathSegments.slice(0, index + 1).join("/")}`; | |
| const parentFolderName = parentFolderNameByPath.get(path); | |
| if (parentFolderName) { | |
| folderNames.push(parentFolderName); | |
| } | |
| } | |
| } | |
| return { ...folder, folderList: folderNames }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pages/api/teams/`[teamId]/documents/index.ts around lines 305 - 338, The
parentFolders query and subsequent parentFolderNameByPath map can include hidden
ancestors, leaking them into folderList; update the prisma.folder.findMany call
that populates parentFolders to only return non-hidden ancestors (add where: {
teamId, path: { in: Array.from(allParentPaths) }, hiddenInAllDocuments: false })
or alternatively filter parentFolders before building parentFolderNameByPath so
parentFolderNameByPath only contains folders with hiddenInAllDocuments ===
false, then leave the folders.map logic unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add folder names and their paths to the "All Documents" search results to improve search functionality.
Summary by CodeRabbit