-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New Feature: Folders #2455
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
base: main
Are you sure you want to change the base?
New Feature: Folders #2455
Conversation
📝 WalkthroughWalkthroughAdds folder functionality across the stack: a Prisma migration and Folder model; server FolderModule, controller, and service (create/read/update/delete/move); DTOs and barrel exports; client routes, pages, components, dialogs, and React Query hooks with cache synchronization; dialog-store and resume services updated to support folders and moving resumes. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser UI
participant Dialog as DialogStore
participant Client as React Query
participant Server as REST API
participant DB as Prisma/Database
Note over User,Dialog: Open folder dialog or choose "Move to folder"
User->>Dialog: open("folder", mode)
Dialog->>User: render FolderDialog / Move UI
alt create / update / delete
User->>Client: mutate create/update/deleteFolder
Client->>Server: POST/PATCH/DELETE /folder...
Server->>DB: persist changes
DB-->>Server: folder/resume payload
Server-->>Client: response
Client->>Client: update FOLDERS_KEY and ["folder",{id}] caches
Client-->>User: mutation resolved (close dialog / refresh UI)
else move-resume
User->>Client: mutate moveResumeToFolder
Client->>Server: PATCH /folder/move-resume/:id
Server->>DB: update resume.folderId, adjust source/target relations
DB-->>Server: source/target folder + resume
Server-->>Client: response (source/target/resume)
Client->>Client: update folder caches, invalidate RESUMES_KEY
Client-->>User: mutation resolved (close dialog / refresh UI)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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: 14
🧹 Nitpick comments (24)
libs/dto/src/folder/folder.ts (1)
9-18: AlignFolderDTO with DB model and enforce basicnamevalidationTwo things to consider here:
userIdis required infolderSchema, but intools/prisma/schema.prismatheFolder.userIdfield is nullable (String?). If any folder can exist without a user (now or in future migrations), this DTO will reject those records at runtime. Either (a) makeFolder.userIdnon-null in Prisma if folders must always belong to a user, or (b) relax the DTO touserId: idSchema.optional()so both layers agree.name: z.string()allows empty or arbitrarily long names. For consistency withuserSchema.nameand to avoid blank-folder entries, consider something like.min(1).max(255).Example DTO tweak if you want stricter validation while keeping userId required:
-export const folderSchema = z.object({ - id: idSchema, - name: z.string(), - userId: idSchema, +export const folderSchema = z.object({ + id: idSchema, + name: z.string().min(1).max(255), + userId: idSchema, user: userSchema.optional(), resumes: resumeSchema.array().optional(), resumesCount: z.number().optional(), createdAt: dateSchema.optional(), updatedAt: dateSchema.optional(), });apps/client/src/services/folder/folders.ts (1)
8-26: Folders query hook looks solid; only minor optional cleanupsThe fetch + hook wiring with
FOLDERS_KEYanduseQueryis idiomatic and should behave as expected.If you want to trim a bit of typing noise, Axios can infer the response wrapper type, so this is equivalent:
-export const fetchFolders = async () => { - const response = await axios.get<FolderDto[], AxiosResponse<FolderDto[]>>("/folder"); - - return response.data; -}; +export const fetchFolders = async () => { + const response = await axios.get<FolderDto[]>("/folder"); + return response.data; +};libs/dto/src/folder/move-resume.ts (1)
5-10: Clarify and possibly renameidinMoveResumeToFolderDtoAs written, the schema:
export const moveResumeToFolderSchema = z.object({ id: idSchema.optional(), resumeId: idSchema, });doesn’t make it obvious what
idrepresents (folder id? something else), and making it optional is odd for a “move to folder” operation.If
idis the target folder id, consider making that explicit and required in the DTO (and usingnull/absence at the API level to represent “remove from folder” if you support that):-export const moveResumeToFolderSchema = z.object({ - id: idSchema.optional(), - resumeId: idSchema, -}); +export const moveResumeToFolderSchema = z.object({ + folderId: idSchema, + resumeId: idSchema, +});That makes the API self-describing and lines up better with the rest of the schema (
folderIdonResume,Folder.idin Prisma).tools/prisma/schema.prisma (1)
36-87: Revisit Folder ownership nullability and relation naming in Prisma schemaA few things worth double‑checking in this block:
Folder.userIdis nullable (userId String?,User User?), while thefolderSchemaDTO requiresuserId. If folders are always user‑owned (which the feature description suggests), makinguserIdnon‑nullable here (andUsernon‑optional) will better reflect the invariant and avoid DTO/runtime mismatches. Alternatively, relax the DTO to allowuserIdto be null if you truly intend system/global folders.- The relation field on
Resumeis namedFolder(capital F):
Folder Folder? @relation(fields: [folderId], references: [id])
This is purely stylistic, but renaming it tofolderwould align with existing patterns likeuser Userand keep Prisma Client reads (resume.folder) more conventional.- With
folderIdoptional and no explicitonDeletereferential action, Prisma defaults toonDelete: SetNullfor this relation. That nicely matches “delete folder but keep resumes (move them to no folder)”, but it also means “delete folder and delete its resumes” must be implemented explicitly in service code (e.g.,deleteManyon resumes before deleting the folder). Make sure the FolderService logic matches the UX (“with or without their resumes”) and tests cover both paths.None of these are blockers, but aligning the invariants across DTOs, Prisma, and service logic will make the new feature much more predictable.
apps/client/src/providers/dialog.tsx (1)
14-14: LGTM! FolderDialog correctly integrated into DialogProvider.The FolderDialog is properly imported and rendered in the correct position (before the resume-loaded conditional block), consistent with other non-resume-specific dialogs.
Optional future consideration: As the DialogProvider grows (now 17+ dialogs), consider implementing lazy loading for dialogs to improve initial bundle size. This is not urgent for this PR but worth considering for future optimization.
Also applies to: 37-37
apps/client/src/services/resume/delete.ts (1)
32-41: Cache invalidation logic is correct.The folder cache update properly handles the case when a resume belonging to a folder is deleted, ensuring cache consistency across both the global resumes list and the folder-specific cache.
Consider using the spread operator for a more modern and readable approach:
queryClient.setQueryData<FolderDto>(["folder", { id: data.folderId }], (cache) => { if (!cache) return cache; if (!cache.resumes) return cache; - return Object.assign({}, cache, { + return { + ...cache, resumes: cache.resumes.filter((resume) => resume.id !== data.id), - }); + }; });libs/dto/src/folder/move-resume-response.ts (1)
7-11: Consider making theresumefield required in the response.The
resumefield is marked as optional, which is unusual for a "move resume" operation response. If the operation successfully moves a resume to a folder, the response should include the updated resume data to confirm the operation succeeded and provide the client with the current state.Making it optional could:
- Hide errors where the resume wasn't actually moved
- Force clients to make an additional request to fetch the updated resume
- Create ambiguity about whether the operation fully succeeded
Consider making the resume field required unless there's a specific use case for it being optional:
export const moveResumeResponseSchema = z.object({ sourceFolder: folderSchema.optional(), targetFolder: folderSchema, - resume: resumeSchema.optional(), + resume: resumeSchema, });If there are valid scenarios where
resumemight not be included, please document those cases.apps/client/src/pages/dashboard/folders/_layouts/grid/index.tsx (1)
11-12: Remove redundant variable alias.Line 12 creates an unnecessary alias
loadingforfoldersLoadingthat's only used once. You can usefoldersLoadingdirectly in the conditional on line 19.Apply this diff to simplify the code:
export const GridView = () => { const { folders, loading: foldersLoading } = useFolders(); - const loading = foldersLoading; return ( <div className="grid grid-cols-1 gap-8 sm:grid-cols-3 xl:grid-cols-4 2xl:grid-cols-5"> <motion.div initial={{ opacity: 0, x: -50 }} animate={{ opacity: 1, x: 0 }}> <CreateFolderCard /> </motion.div> - {loading && + {foldersLoading &&apps/client/src/services/resume/update.tsx (1)
38-47: Folder cache sync on resume update looks good; guard logic can be simplifiedThe new
onSuccesscorrectly keeps the folder cache in sync and avoids touching missing caches, but the nested guards are a bit verbose. You could simplify them and keep behavior identical by collapsing to a single check:onSuccess: (data) => { if (data.folderId) { queryClient.setQueryData<FolderDto>(["folder", { id: data.folderId }], (cache) => { if (!cache?.resumes) return cache; return { ...cache, resumes: cache.resumes.map((resume) => (resume.id === data.id ? data : resume)), }; }); } }This keeps the same semantics and slightly improves readability.
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
17-28: Avoid mutatingfolder.resumesin-place when sorting
.sortmutates theresumesarray, which comes directly from the React Query cache; mutating cached data in-place can cause subtle bugs and makes updates harder to reason about. Safer to sort a shallow copy:{resumes && ( <AnimatePresence> {[...resumes] .sort((a, b) => sortByDate(a, b, "updatedAt")) .map((resume, index) => ( <motion.div key={resume.id} initial={{ opacity: 0, y: -50 }} animate={{ opacity: 1, y: 0, transition: { delay: (index + 2) * 0.1 } }} exit={{ opacity: 0, filter: "blur(8px)", transition: { duration: 0.5 } }} > <ResumeListItem resume={resume} /> </motion.div> ))} </AnimatePresence> )}apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
14-27: Consider adding basic button semantics for clickable cards
BaseCardis clickable viaonClickbut has no role/tabIndex, so it won’t be keyboard-focusable by default. For cards that act like buttons/links, you could optionally add basic semantics whenonClickis provided:<Card ref={ref} role={onClick ? "button" : undefined} tabIndex={onClick ? 0 : undefined} className={cn( "relative flex aspect-[1/1.4142] scale-100 cursor-pointer items-center justify-center bg-secondary/50 p-0 transition-transform active:scale-95", className, )} onClick={onClick} >That keeps current behavior while improving keyboard accessibility.
apps/client/src/pages/dashboard/folders/folder-details.tsx (1)
16-86: Handle “folder not found” / error state explicitlyAfter
loadingis false, iffolderisnull/undefined(e.g., invalid id or server error), the page still renders withfolder?.name, resulting in titles likeFolder - undefinedand an empty content area. A small guard after the loading branch would make this nicer:const { folder, loading, error } = useFolder(id!); if (loading) { // skeletons... } // Optionally use `error` as well. if (!folder) { return ( <> <Helmet> <title>{t`Folder`} - {t`Not found`}</title> </Helmet> <p className="text-sm opacity-70">{t`This folder could not be found or is no longer available.`}</p> </> ); }This avoids confusing “undefined” titles and gives users feedback when the folder doesn’t exist.
apps/client/src/services/folder/create.ts (1)
18-32: Folder creation cache updates are correct; you may want to guard against duplicatesThe
onSuccesshandler correctly seeds the detail cache and appends the new folder toFOLDERS_KEY. To be a bit more defensive against races or stale caches, you could skip appending if the folder is already present:queryClient.setQueryData<FolderDto[]>(FOLDERS_KEY, (cache) => { if (!cache) return [data]; if (cache.some((folder) => folder.id === data.id)) return cache; return [...cache, data]; });Not required, but it prevents accidental duplicates in the folders list.
apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
13-19: Consider accessibility improvements for the clickable container.The clickable
divlacks keyboard navigation support and semantic meaning. Users relying on keyboard navigation cannot interact with this component, and screen readers won't announce it as clickable.Consider one of these solutions:
Option 1: Use a button element (if the item triggers an action):
- <div + <button + type="button" className={cn( "flex cursor-pointer items-center rounded p-4 transition-colors hover:bg-secondary/30", className, )} onClick={onClick} >Option 2: Add keyboard support (if a div is required):
<div + role="button" + tabIndex={0} + onKeyDown={(e) => { + if ((e.key === 'Enter' || e.key === ' ') && onClick) { + e.preventDefault(); + onClick(); + } + }} className={cn( - "flex cursor-pointer items-center rounded p-4 transition-colors hover:bg-secondary/30", + "flex cursor-pointer items-center rounded p-4 transition-colors hover:bg-secondary/30 focus:outline-none focus:ring-2 focus:ring-secondary", className, )} onClick={onClick} >apps/client/src/pages/dashboard/folders/_layouts/grid/_components/create-folder.tsx (1)
33-33: Remove the unused empty paragraph element.The empty
<p>element serves no purpose and can be removed to clean up the code.</h4> - - <p className="text-xs opacity-75"></p> </div>apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
8-34: Consider displaying loading and error states.The component fetches
loadinganderrorfromuseFolderbut doesn't use them. Users will see an empty grid while data loads or if an error occurs.Add loading and error handling:
export default function FolderResumesGridView() { const { id } = useParams<{ id: string }>(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const { folder } = useFolder(id!); + const { folder, loading, error } = useFolder(id!); const resumes = folder?.resumes; + if (loading) { + return ( + <div className="grid grid-cols-1 gap-8 sm:grid-cols-3 xl:grid-cols-4 2xl:grid-cols-5"> + {Array.from({ length: 4 }).map((_, i) => ( + <div key={i} className="h-64 animate-pulse rounded bg-secondary/40" /> + ))} + </div> + ); + } + + if (error) { + return <div>Error loading folder</div>; + } return (apps/client/src/pages/dashboard/folders/_layouts/list/index.tsx (1)
11-12: Remove redundant variable assignment.The
loadingvariable is assigned fromfoldersLoadingwithout any transformation. You can usefoldersLoadingdirectly throughout the component.export const ListView = () => { - const { folders, loading: foldersLoading } = useFolders(); - const loading = foldersLoading; + const { folders, loading } = useFolders(); return (apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (1)
1-54: Address ESLint import sorting.The static analysis tool flagged unsorted imports. While this is a minor issue, maintaining consistent import order improves code readability.
Based on static analysis hints.
Run the autofix command suggested by ESLint:
eslint --fix apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsxapps/client/src/services/folder/update.ts (1)
9-37: Folder update mutation and cache wiring look correctPATCH call typing, single-folder cache update, and FOLDERS_KEY list refresh are consistent and safe. Optionally, consider extracting a small helper for the “filter out then append updated folder” pattern used across folder services to avoid duplication.
apps/client/src/services/folder/move-resume.ts (1)
1-46: moveResumeToFolder hook and cache updates look consistent with existing folder servicesThe mutation wiring, axios call, and cache updates for both the individual folder and the folders list are consistent with the existing
create/updateservices, and invalidatingRESUMES_KEYafter a move makes sense for keeping the main resumes view in sync. Only optional nit:updateFolderInCachere-appends the updated folder to the end of the folders list, which may slightly change ordering; if ordering matters (e.g., by creation date), consider preserving the previous index instead of always pushing.apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx (3)
72-88: Consider explicit error handling for mutation failures in onSubmit
onSubmitawaitscreateFolder,updateFolder, anddeleteFolder(via theirmutateAsyncwrappers) without any try/catch. If a mutation rejects, the error will bubble out ofonSubmit; depending on howreact-hook-formwrapshandleSubmit, this can result in unhandled promise rejections and the dialog staying open with no feedback.Consider wrapping the body of
onSubmitin a try/catch to keep the dialog stable and surface errors (e.g., via a toast, form-level error, or leaving the dialog open):const onSubmit = async (values: FormValues) => { try { // existing create/update/delete branches close(); } catch (error) { // surface error and do not close dialog } };
90-142: Delete dialog: add loading handling and wire form state more idiomaticallyTwo smaller points in the delete branch:
- The
Deleteaction button isn’t disabled whiledeleteLoadingis true, so users can trigger multiple deletion requests by clicking repeatedly:<AlertDialogAction variant="error" onClick={form.handleSubmit(onSubmit)} disabled={deleteLoading} > {t`Delete`} </AlertDialogAction>
- The
CheckboxforisDeleteResumesis manually wired viasetValue/watch. For consistency with the rest of the form, you might prefer to drive it throughFormField/Controller, which will also make it respectdefaultValuesand validation more cleanly.These are not blockers, but they will make the delete UX more robust and consistent.
145-203: Main dialog: stray{mode}text and submit button UX
- Line 149 renders
{mode}directly inside the dialog content; this looks like leftover debug output and will show raw"create"/"update"text in the UI. It’s safe to remove.- <Form {...form}> - {mode} + <Form {...form}>
- Optional: the primary submit
<Button>is only disabled byloading. You may also want to respect form validity and submission state (form.formState.isValid,form.formState.isSubmitting) so users can’t submit an invalid or already-submitting form.Neither issue is critical, but cleaning them up will polish the dialog UX.
tools/prisma/migrations/20251115145627_add_resumes_folders/migration.sql (1)
10-37: Composite unique indexes on(userId, id)and(folderId, id)may be redundantIf
Folder.idandResume.idare already primary keys (as they appear to be), the unique indexes:
Folder_userId_id_keyon("userId", "id")Resume_folderId_id_keyon("folderId", "id")don’t add additional uniqueness guarantees beyond what the PKs already enforce, but they do add index maintenance overhead on writes.
If you’re not relying on these specific composites for Prisma
@@uniqueaccess patterns or query optimization, consider dropping them from the Prisma schema before generating migrations to keep the index set leaner.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
apps/client/src/constants/query-keys.ts(1 hunks)apps/client/src/pages/dashboard/_components/sidebar.tsx(3 hunks)apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/create-folder.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-card.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/index.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/create-item.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/index.tsx(1 hunks)apps/client/src/pages/dashboard/folders/folder-details.tsx(1 hunks)apps/client/src/pages/dashboard/folders/page.tsx(1 hunks)apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx(5 hunks)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx(3 hunks)apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx(4 hunks)apps/client/src/providers/dialog.tsx(2 hunks)apps/client/src/router/index.tsx(2 hunks)apps/client/src/services/folder/create.ts(1 hunks)apps/client/src/services/folder/delete.ts(1 hunks)apps/client/src/services/folder/folder.ts(1 hunks)apps/client/src/services/folder/folders.ts(1 hunks)apps/client/src/services/folder/index.ts(1 hunks)apps/client/src/services/folder/move-resume.ts(1 hunks)apps/client/src/services/folder/update.ts(1 hunks)apps/client/src/services/resume/delete.ts(2 hunks)apps/client/src/services/resume/update.tsx(2 hunks)apps/client/src/stores/dialog.ts(1 hunks)apps/server/src/app.module.ts(2 hunks)apps/server/src/folder/folder.controller.ts(1 hunks)apps/server/src/folder/folder.module.ts(1 hunks)apps/server/src/folder/folder.service.ts(1 hunks)apps/server/src/folder/types.ts(1 hunks)apps/server/src/resume/resume.service.ts(1 hunks)libs/dto/src/folder/create.ts(1 hunks)libs/dto/src/folder/delete.ts(1 hunks)libs/dto/src/folder/folder.ts(1 hunks)libs/dto/src/folder/index.ts(1 hunks)libs/dto/src/folder/move-resume-response.ts(1 hunks)libs/dto/src/folder/move-resume.ts(1 hunks)libs/dto/src/folder/update.ts(1 hunks)libs/dto/src/index.ts(1 hunks)libs/dto/src/resume/resume.ts(1 hunks)tools/prisma/migrations/20251115145627_add_resumes_folders/migration.sql(1 hunks)tools/prisma/schema.prisma(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (33)
apps/client/src/services/folder/folders.ts (3)
apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/constants/query-keys.ts (1)
FOLDERS_KEY(12-12)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (3)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-23)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx (1)
ResumeListItem(36-195)
libs/dto/src/folder/update.ts (2)
libs/dto/src/folder/folder.ts (1)
folderSchema(9-18)libs/dto/src/resume/resume.ts (1)
resumeSchema(8-20)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/create-folder.tsx (4)
apps/client/src/stores/dialog.ts (1)
useDialog(34-51)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/utils/src/namespaces/style.ts (1)
cn(14-14)libs/ui/src/components/shortcut.tsx (1)
KeyboardShortcut(15-50)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/create-item.tsx (4)
apps/client/src/stores/dialog.ts (1)
useDialog(34-51)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
BaseListItem(12-30)libs/ui/src/components/shortcut.tsx (1)
KeyboardShortcut(15-50)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-card.tsx (5)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/stores/dialog.ts (1)
useDialog(34-51)libs/ui/src/components/dropdown-menu.tsx (5)
DropdownMenu(6-6)DropdownMenuTrigger(8-8)DropdownMenuContent(58-76)DropdownMenuItem(80-95)DropdownMenuSeparator(161-170)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/utils/src/namespaces/style.ts (1)
cn(14-14)
apps/client/src/pages/dashboard/folders/_layouts/grid/index.tsx (5)
apps/client/src/services/folder/folders.ts (1)
useFolders(14-26)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/create-folder.tsx (1)
CreateFolderCard(10-37)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-card.tsx (1)
FolderCard(23-78)
libs/dto/src/folder/move-resume-response.ts (2)
libs/dto/src/folder/folder.ts (1)
folderSchema(9-18)libs/dto/src/resume/resume.ts (1)
resumeSchema(8-20)
apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx (2)
libs/ui/src/components/dropdown-menu.tsx (1)
DropdownMenuItem(80-95)libs/ui/src/components/context-menu.tsx (1)
ContextMenuItem(74-89)
apps/client/src/services/folder/update.ts (4)
libs/dto/src/folder/update.ts (1)
UpdateFolderDto(10-10)apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/constants/query-keys.ts (1)
FOLDERS_KEY(12-12)
apps/client/src/router/index.tsx (2)
apps/client/src/pages/dashboard/folders/page.tsx (1)
FolderPage(12-62)apps/client/src/pages/dashboard/folders/folder-details.tsx (1)
FolderDetailsPage(16-87)
apps/client/src/services/folder/delete.ts (5)
libs/dto/src/folder/delete.ts (1)
DeleteFolderDto(10-10)apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)libs/dto/src/resume/resume.ts (1)
ResumeDto(22-22)apps/client/src/constants/query-keys.ts (1)
RESUMES_KEY(9-9)
apps/client/src/services/folder/move-resume.ts (5)
libs/dto/src/folder/move-resume.ts (1)
MoveResumeToFolderDto(10-10)apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/move-resume-response.ts (1)
MoveResumeResponseDto(13-13)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/constants/query-keys.ts (2)
FOLDERS_KEY(12-12)RESUMES_KEY(9-9)
apps/client/src/pages/dashboard/folders/folder-details.tsx (5)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-23)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/ui/src/components/scroll-area.tsx (1)
ScrollArea(5-40)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
FolderResumesGridView(8-35)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
FolderResumesListView(8-33)
apps/server/src/folder/folder.service.ts (5)
libs/dto/src/folder/create.ts (1)
CreateFolderDto(11-11)apps/server/src/folder/types.ts (1)
FolderCountResume(3-5)libs/dto/src/folder/update.ts (1)
UpdateFolderDto(10-10)libs/dto/src/folder/move-resume.ts (1)
MoveResumeToFolderDto(10-10)libs/dto/src/resume/resume.ts (1)
ResumeDto(22-22)
apps/client/src/stores/dialog.ts (1)
libs/schema/src/sections/index.ts (1)
SectionKey(93-93)
libs/dto/src/folder/create.ts (1)
libs/dto/src/resume/resume.ts (1)
resumeSchema(8-20)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (4)
apps/client/src/services/folder/folders.ts (1)
useFolders(14-26)apps/client/src/services/folder/move-resume.ts (1)
useMoveResumeToFolder(29-46)libs/ui/src/components/dialog.tsx (5)
Dialog(6-6)DialogContent(32-53)DialogHeader(57-59)DialogTitle(72-81)DialogDescription(85-94)libs/utils/src/namespaces/style.ts (1)
cn(14-14)
apps/client/src/services/resume/update.tsx (1)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (2)
apps/client/src/constants/parallax-tilt.ts (1)
defaultTiltProps(3-11)libs/utils/src/namespaces/style.ts (1)
cn(14-14)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx (5)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/stores/dialog.ts (1)
useDialog(34-51)libs/ui/src/components/dropdown-menu.tsx (4)
DropdownMenu(6-6)DropdownMenuTrigger(8-8)DropdownMenuContent(58-76)DropdownMenuItem(80-95)libs/ui/src/components/context-menu.tsx (5)
ContextMenuSeparator(155-164)ContextMenu(6-6)ContextMenuTrigger(8-8)ContextMenuContent(56-70)ContextMenuItem(74-89)apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
BaseListItem(12-30)
apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx (10)
libs/dto/src/folder/folder.ts (2)
folderSchema(9-18)FolderDto(20-20)apps/client/src/stores/dialog.ts (1)
useDialog(34-51)apps/client/src/services/folder/create.ts (2)
useCreateFolder(18-36)createFolder(9-16)apps/client/src/services/folder/update.ts (2)
useUpdateFolder(18-38)updateFolder(9-16)apps/client/src/services/folder/delete.ts (1)
useDeleteFolder(23-51)libs/ui/src/components/alert-dialog.tsx (8)
AlertDialog(8-8)AlertDialogContent(34-49)AlertDialogHeader(53-58)AlertDialogTitle(74-83)AlertDialogDescription(87-96)AlertDialogFooter(62-70)AlertDialogCancel(117-126)AlertDialogAction(104-113)libs/ui/src/components/checkbox.tsx (1)
Checkbox(6-22)libs/ui/src/components/dialog.tsx (6)
Dialog(6-6)DialogContent(32-53)DialogHeader(57-59)DialogTitle(72-81)DialogDescription(85-94)DialogFooter(63-68)libs/ui/src/components/form.tsx (6)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormDescription(73-87)FormMessage(91-112)libs/utils/src/namespaces/style.ts (1)
cn(14-14)
apps/client/src/pages/dashboard/folders/page.tsx (3)
libs/ui/src/components/scroll-area.tsx (1)
ScrollArea(5-40)apps/client/src/pages/dashboard/folders/_layouts/grid/index.tsx (1)
GridView(10-49)apps/client/src/pages/dashboard/folders/_layouts/list/index.tsx (1)
ListView(10-48)
apps/client/src/services/folder/create.ts (4)
libs/dto/src/folder/create.ts (1)
CreateFolderDto(11-11)apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/constants/query-keys.ts (1)
FOLDERS_KEY(12-12)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
libs/utils/src/namespaces/style.ts (1)
cn(14-14)
libs/dto/src/folder/folder.ts (3)
libs/dto/src/user/user.ts (1)
userSchema(17-32)libs/dto/src/resume/resume.ts (1)
resumeSchema(8-20)libs/utils/src/namespaces/date.ts (1)
dateSchema(4-7)
apps/client/src/providers/dialog.tsx (1)
apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx (1)
FolderDialog(49-203)
apps/server/src/folder/folder.controller.ts (4)
apps/server/src/user/decorators/user.decorator.ts (1)
User(5-12)libs/dto/src/folder/create.ts (1)
CreateFolderDto(11-11)libs/dto/src/folder/update.ts (1)
UpdateFolderDto(10-10)libs/dto/src/folder/move-resume.ts (1)
MoveResumeToFolderDto(10-10)
apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (1)
libs/ui/src/components/dropdown-menu.tsx (1)
DropdownMenuItem(80-95)
apps/client/src/pages/dashboard/folders/_layouts/list/index.tsx (5)
apps/client/src/services/folder/folders.ts (1)
useFolders(14-26)apps/client/src/pages/dashboard/folders/_layouts/list/_components/create-item.tsx (1)
CreateFolderListItem(10-29)apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
BaseListItem(12-30)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx (1)
FolderListItem(33-123)
apps/client/src/services/resume/delete.ts (1)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (3)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-23)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (1)
ResumeCard(32-135)
apps/client/src/services/folder/folder.ts (2)
apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)
🪛 ESLint
apps/client/src/services/folder/delete.ts
[error] 42-42: Use for…of instead of .forEach(…).
(unicorn/no-array-for-each)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx
[error] 1-54: Run autofix to sort these imports!
(simple-import-sort/imports)
🔇 Additional comments (20)
apps/client/src/constants/query-keys.ts (1)
11-12:FOLDERS_KEYaddition is consistent and clearThe new
FOLDERS_KEY: ["folders"]follows the existing convention (RESUMES_KEY,USER_KEY, etc.) and should work well with the new folder React Query hooks.apps/server/src/app.module.ts (1)
14-14: LGTM! FolderModule properly integrated.The FolderModule import and registration follow NestJS conventions and are correctly placed in the module configuration.
Also applies to: 53-53
libs/dto/src/index.ts (1)
5-5: LGTM! Folder DTO exports properly added.The folder DTO barrel export follows the existing pattern and correctly exposes folder-related DTOs to consumers.
apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (1)
60-62: LGTM! Move to folder action properly implemented.The "Move to folder" menu item follows the existing pattern for dialog actions and is correctly positioned in the dropdown menu with appropriate icon and translation.
Also applies to: 112-115
apps/client/src/pages/dashboard/_components/sidebar.tsx (1)
78-81: LGTM! Folders navigation seamlessly integrated.The Folders sidebar item and keyboard shortcut (Shift+F) are correctly implemented and follow the existing patterns for navigation items. The placement between Resumes and Settings is logical and consistent with the feature hierarchy.
Also applies to: 95-100
apps/client/src/services/resume/delete.ts (1)
28-30: Good refactor to use filter for cache update.The change from removing the item to filtering the cache is cleaner and more functional in style.
apps/server/src/folder/types.ts (1)
3-5: LGTM!The type definition correctly uses Prisma's type system to represent a Folder with a nested count of resumes.
apps/client/src/pages/dashboard/folders/_layouts/grid/index.tsx (1)
14-46: LGTM!The grid layout and animation implementation looks good. The staggered animation delays create a nice progressive reveal effect, and the
(index + 2)offset in the folder card animations appropriately accounts for the CreateFolderCard position.apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx (1)
3-3: LGTM!The "Move to Folder" functionality is cleanly integrated:
- Follows existing patterns for menu actions
- Properly added to both DropdownMenu and ContextMenu for consistent UX
- Event handling with
stopPropagation()matches other actions- Icon usage and styling are consistent with the rest of the component
Also applies to: 55-57, 102-110, 172-175
apps/server/src/folder/folder.module.ts (1)
7-11: FolderService export is not required.The verification shows FolderService is only used internally by FolderController within the same module. No other modules import or depend on FolderService, so exporting it from FolderModule is unnecessary. The current module configuration is correct.
Likely an incorrect or invalid review comment.
apps/client/src/stores/dialog.ts (1)
4-7: Dialog type extensions for folders and move-resume look consistentExtending
DialogNamewith"folder"andDialogModewith"move-resume"aligns with existing patterns and should integrate cleanly with the shareduseDialogAPI and new folder/move-resume dialogs.apps/client/src/pages/dashboard/folders/page.tsx (1)
11-61: FolderPage layout/tabs implementation looks solidThe grid/list layout state, Tabs wiring, and ScrollArea usage are clean and consistent with the rest of the dashboard (mirrors the folder-details layout), and should be easy to extend if more layouts are added later.
apps/client/src/pages/dashboard/folders/_layouts/list/_components/create-item.tsx (1)
10-28: CreateFolderListItem integrates cleanly with the dialog systemThe list item correctly opens the
"folder"dialog in"create"mode, reusesBaseListItemstyling, and includes a localized title/description plus a keyboard shortcut hint. This is consistent with the grid version and fits the shared dialog API.apps/client/src/router/index.tsx (1)
13-14: LGTM!The folder routes are properly integrated into the routing structure. They follow the existing patterns for dashboard routes and are correctly protected by the
AuthGuard.Also applies to: 69-70
apps/client/src/services/folder/folder.ts (1)
6-23: LGTM!The folder service implementation is clean and follows established patterns in the codebase. The React Query integration is correct, and type safety is properly maintained with
FolderDto.libs/dto/src/folder/index.ts (1)
1-6: LGTM!The barrel export file properly consolidates folder-related DTOs, providing a clean import interface for consumers.
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (2)
141-150: LGTM!The
onMoveToFolderhandler is correctly implemented with proper null checks and dialog cleanup after the move operation.
196-236: Good UX implementation for move-to-folder dialog.The dialog properly handles edge cases:
- Disables the button for the folder currently containing the resume
- Provides a helpful message when no folders exist
- Prevents multiple submissions during loading
apps/client/src/services/folder/index.ts (1)
1-6: Barrel re-exports are straightforward and cohesiveCentralizing folder service exports here is clean and matches the new folder service surface.
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-card.tsx (1)
23-56: Confirm dayjs relative time plugin is configured
lastUpdatedusesdayjs().to(folder.updatedAt), which depends on therelativeTimeplugin. Please ensure this plugin is initialized once in a shared setup file; otherwise this will throw at runtime (toundefined).
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
Outdated
Show resolved
Hide resolved
| const lastUpdated = dayjs().to(folder.updatedAt); | ||
|
|
||
| const onOpen = () => { | ||
| void navigate(`/dashboard/folders/${folder.id}`); | ||
| }; | ||
|
|
||
| const onUpdate = () => { | ||
| open("update", { id: "folder", item: folder }); | ||
| }; | ||
|
|
||
| const onDelete = () => { | ||
| open("delete", { id: "folder", item: folder }); | ||
| }; | ||
|
|
||
| const dropdownMenu = ( | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild className="aspect-square"> | ||
| <Button size="icon" variant="ghost"> | ||
| <DotsThreeVertical /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent> | ||
| <DropdownMenuItem | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| onOpen(); | ||
| }} | ||
| > | ||
| <FolderOpen size={14} className="mr-2" /> | ||
| {t`Open`} | ||
| </DropdownMenuItem> | ||
| <DropdownMenuItem | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| onUpdate(); | ||
| }} | ||
| > | ||
| <PencilSimple size={14} className="mr-2" /> | ||
| {t`Rename`} | ||
| </DropdownMenuItem> | ||
|
|
||
| <ContextMenuSeparator /> | ||
| <DropdownMenuItem | ||
| className="text-error" | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| onDelete(); | ||
| }} | ||
| > | ||
| <TrashSimple size={14} className="mr-2" /> | ||
| {t`Delete`} | ||
| </DropdownMenuItem> |
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.
🧩 Analysis chain
Use the correct separator component in the dropdown and confirm dayjs setup
- Line 78 uses
ContextMenuSeparatorinsideDropdownMenuContent. This mixes context‑menu primitives into a dropdown. Prefer importing and usingDropdownMenuSeparatorhere to match the rest of the dropdown API and avoid any Radix edge cases. - As in
FolderCard,dayjs().to(folder.updatedAt)relies on therelativeTimeplugin being configured globally; please confirm that initialization exists.
Example adjustment:
-import {
- Button,
- ContextMenu,
- ContextMenuContent,
- ContextMenuItem,
- ContextMenuSeparator,
- ContextMenuTrigger,
- DropdownMenu,
- DropdownMenuContent,
- DropdownMenuItem,
- DropdownMenuTrigger,
-} from "@reactive-resume/ui";
+import {
+ Button,
+ ContextMenu,
+ ContextMenuContent,
+ ContextMenuItem,
+ ContextMenuSeparator,
+ ContextMenuTrigger,
+ DropdownMenu,
+ DropdownMenuContent,
+ DropdownMenuItem,
+ DropdownMenuSeparator,
+ DropdownMenuTrigger,
+} from "@reactive-resume/ui";
@@
- <ContextMenuSeparator />
+ <DropdownMenuSeparator />🏁 Script executed:
#!/bin/bash
# Verify folder-item.tsx structure, imports, and line 78
echo "=== Checking folder-item.tsx imports and line 78 ==="
find . -type f -name "folder-item.tsx" | head -1 | xargs cat | head -100Length of output: 2539
🏁 Script executed:
#!/bin/bash
# Search for dayjs configuration and relativeTime plugin setup
echo "=== Searching for dayjs configuration ==="
rg -n "dayjs.*relativeTime|extend.*relativeTime" --type=ts --type=tsxLength of output: 69
🏁 Script executed:
#!/bin/bash
# Check @reactive-resume/ui exports for separators
echo "=== Looking for separator components in @reactive-resume/ui ==="
find . -path "*/reactive-resume/ui*" -name "*.ts" -o -name "*.tsx" | xargs grep -l "DropdownMenuSeparator\|ContextMenuSeparator" 2>/dev/null | head -5Length of output: 443
🏁 Script executed:
#!/bin/bash
# Verify the index file of @reactive-resume/ui to see what's exported
echo "=== Checking @reactive-resume/ui index/exports ==="
find . -path "*/reactive-resume/ui*" \( -name "index.ts" -o -name "index.tsx" \) | xargs cat 2>/dev/null | grep -E "Separator|export"Length of output: 50
🏁 Script executed:
#!/bin/bash
# Search for dayjs.extend or plugin initialization
echo "=== Searching for dayjs plugin setup ==="
rg -n "dayjs.*extend|relativeTime" -A2 -B2Length of output: 824
🏁 Script executed:
#!/bin/bash
# Find @reactive-resume/ui package and check structure
echo "=== Locating @reactive-resume/ui package ==="
find . -type d -name "ui" | grep reactive-resume | head -3Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check one file that uses separators to see the pattern
echo "=== Checking resume-item.tsx for separator usage ==="
cat ./apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx | grep -A3 -B3 "Separator"Length of output: 628
🏁 Script executed:
#!/bin/bash
# Search for DropdownMenuSeparator in the entire codebase
echo "=== Searching for DropdownMenuSeparator usage ==="
rg -n "DropdownMenuSeparator" -B3 -A1Length of output: 7503
Use the correct separator component in the dropdown
Line 78 uses ContextMenuSeparator inside DropdownMenuContent. This mixes context-menu primitives into a dropdown. Import and use DropdownMenuSeparator here to match the rest of the dropdown API and the pattern used in the related folder-card.tsx component.
Example adjustment:
import {
Button,
ContextMenu,
ContextMenuContent,
ContextMenuItem,
ContextMenuSeparator,
ContextMenuTrigger,
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
+ DropdownMenuSeparator,
DropdownMenuTrigger,
} from "@reactive-resume/ui";- <ContextMenuSeparator />
+ <DropdownMenuSeparator />📝 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.
| const lastUpdated = dayjs().to(folder.updatedAt); | |
| const onOpen = () => { | |
| void navigate(`/dashboard/folders/${folder.id}`); | |
| }; | |
| const onUpdate = () => { | |
| open("update", { id: "folder", item: folder }); | |
| }; | |
| const onDelete = () => { | |
| open("delete", { id: "folder", item: folder }); | |
| }; | |
| const dropdownMenu = ( | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild className="aspect-square"> | |
| <Button size="icon" variant="ghost"> | |
| <DotsThreeVertical /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent> | |
| <DropdownMenuItem | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onOpen(); | |
| }} | |
| > | |
| <FolderOpen size={14} className="mr-2" /> | |
| {t`Open`} | |
| </DropdownMenuItem> | |
| <DropdownMenuItem | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onUpdate(); | |
| }} | |
| > | |
| <PencilSimple size={14} className="mr-2" /> | |
| {t`Rename`} | |
| </DropdownMenuItem> | |
| <ContextMenuSeparator /> | |
| <DropdownMenuItem | |
| className="text-error" | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onDelete(); | |
| }} | |
| > | |
| <TrashSimple size={14} className="mr-2" /> | |
| {t`Delete`} | |
| </DropdownMenuItem> | |
| const lastUpdated = dayjs().to(folder.updatedAt); | |
| const onOpen = () => { | |
| void navigate(`/dashboard/folders/${folder.id}`); | |
| }; | |
| const onUpdate = () => { | |
| open("update", { id: "folder", item: folder }); | |
| }; | |
| const onDelete = () => { | |
| open("delete", { id: "folder", item: folder }); | |
| }; | |
| const dropdownMenu = ( | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild className="aspect-square"> | |
| <Button size="icon" variant="ghost"> | |
| <DotsThreeVertical /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent> | |
| <DropdownMenuItem | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onOpen(); | |
| }} | |
| > | |
| <FolderOpen size={14} className="mr-2" /> | |
| {t`Open`} | |
| </DropdownMenuItem> | |
| <DropdownMenuItem | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onUpdate(); | |
| }} | |
| > | |
| <PencilSimple size={14} className="mr-2" /> | |
| {t`Rename`} | |
| </DropdownMenuItem> | |
| <DropdownMenuSeparator /> | |
| <DropdownMenuItem | |
| className="text-error" | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| onDelete(); | |
| }} | |
| > | |
| <TrashSimple size={14} className="mr-2" /> | |
| {t`Delete`} | |
| </DropdownMenuItem> |
🤖 Prompt for AI Agents
In
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx
around lines 37 to 88, the dropdown uses ContextMenuSeparator inside
DropdownMenuContent which mixes context-menu primitives into a dropdown; replace
the ContextMenuSeparator usage with DropdownMenuSeparator and update imports
accordingly (import DropdownMenuSeparator from the dropdown menu primitives and
remove ContextMenuSeparator if no longer used) so the component consistently
uses the DropdownMenu API like folder-card.tsx.
| export const createFolderSchema = z.object({ | ||
| name: z.string().min(1), | ||
| resumes: resumeSchema.partial().array().optional(), | ||
| }); |
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.
🧩 Analysis chain
Clarify the use case for including resumes during folder creation.
The resumes field in the creation schema uses a partial resume schema, which makes all resume fields optional. This design is unusual because:
- Folders are typically created empty, with resumes moved into them afterward.
- If the intent is to associate existing resumes during creation, only resume IDs would be needed rather than partial resume objects.
- If the intent is to create new resumes within the folder, a full resume creation schema would be more appropriate.
Please verify the intended use case and ensure the server-side service properly handles this field. Consider whether this should be:
- An array of resume IDs:
z.array(idSchema) - Removed entirely, with resumes added via a separate move operation
- Or if the current design is intentional, document the expected usage
Run the following script to check how this DTO is used in the service layer:
🏁 Script executed:
#!/bin/bash
# Description: Check how CreateFolderDto and its resumes field are used
# Find usages of CreateFolderDto
ast-grep --pattern 'CreateFolderDto'
# Check folder creation service implementation
rg -n --type=ts -A10 'create.*folder' apps/server/src/folder/folder.service.tsLength of output: 320
---
🏁 Script executed:
```shell
#!/bin/bash
# Get folder service implementation - search for create method
rg -n --type=ts -B2 -A20 "^\s*(async\s+)?create.*\(" apps/server/src/folder/folder.service.ts | head -50
# Get folder controller implementation - search for POST method
rg -n --type=ts -B2 -A15 "@Post\|post.*create" apps/server/src/folder/folder.controller.ts | head -50
# Search for any reference to resumes in folder service
rg -n --type=ts "resumes" apps/server/src/folder/folder.service.ts
# Check if CreateFolderDto has documentation or usage patterns
rg -n --type=ts -B5 -A15 "interface CreateFolderDto|type CreateFolderDto" libs/dto/src/folder
Length of output: 1503
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full implementation of getResumesIds method
rg -n --type=ts -A8 "private getResumesIds" apps/server/src/folder/folder.service.ts
# Verify the actual ID extraction logic
rg -n --type=ts -B1 -A5 "resumes.map" apps/server/src/folder/folder.service.ts
Length of output: 635
Simplify the resumes field in the creation schema to accept resume IDs only.
The schema accepts Partial<ResumeDto>[], but verification shows the service only extracts the id field via getResumesIds() (lines 148-157 in folder.service.ts). This design is unnecessarily permissive and creates API confusion.
Update the schema to:
export const createFolderSchema = z.object({
name: z.string().min(1),
resumeIds: z.array(z.string()).optional(),
});Then update the service to use createFolderDto.resumeIds directly in the Prisma connect call. This clarifies the intent and prevents clients from sending unnecessary partial resume data.
🤖 Prompt for AI Agents
In libs/dto/src/folder/create.ts around lines 6–9, the createFolderSchema
currently accepts partial resume objects but the service only needs IDs; change
the schema to replace the `resumes` field with `resumeIds:
z.array(z.string()).optional()`. Then update the folder service (around lines
148–157 in folder.service.ts) to stop calling getResumesIds() and instead use
createFolderDto.resumeIds directly when building the Prisma connect payload (map
each id to the `{ id }` connect shape and guard for undefined), and update any
related DTO/type usages to reflect the renamed field.
libs/dto/src/resume/resume.ts
Outdated
| visibility: z.enum(["private", "public"]).default("private"), | ||
| locked: z.boolean().default(false), | ||
| userId: idSchema, | ||
| folderId: idSchema, |
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.
Critical: folderId must be nullable to match the data model.
The folderId field is defined as a required idSchema, but the server code in apps/server/src/resume/resume.service.ts filters resumes by folderId: null (Line 65), indicating resumes can exist without a folder. This type mismatch will cause validation errors and runtime issues when handling resumes not assigned to folders.
Apply this diff to make folderId nullable:
- folderId: idSchema,
+ folderId: idSchema.nullable(),📝 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.
| folderId: idSchema, | |
| folderId: idSchema.nullable(), |
🤖 Prompt for AI Agents
In libs/dto/src/resume/resume.ts around line 16, the folderId field is currently
defined as a required idSchema but must be nullable to match the server logic
that allows resumes with folderId === null; change the schema to make folderId
nullable (e.g., use idSchema.nullable()) so the DTO accepts null folderId values
and update any related type annotations if necessary.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 3
🧹 Nitpick comments (1)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
29-29: Consider simplifying animation delay calculation.The delay formula
(index + 2) * 0.1adds an offset of 2 to each index, meaning the first resume appears at 0.2s. If this offset is intentional (e.g., to stagger after other animations), consider adding a comment explaining why. Otherwise, simplify toindex * 0.1.If the offset is not needed:
- animate={{ opacity: 1, x: 0, transition: { delay: (index + 2) * 0.1 } }} + animate={{ opacity: 1, x: 0, transition: { delay: index * 0.1 } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (3)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-23)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (1)
ResumeCard(32-135)
🪛 Biome (2.1.2)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
[error] 15-15: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ESLint
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
[error] 12-12: String not marked for translation. Wrap it with .
(lingui/no-unlocalized-strings)
[error] 15-15: React Hook "useFolder" is called conditionally. React Hooks must be called in the exact same order in every component render.
(react-hooks/rules-of-hooks)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
Outdated
Show resolved
Hide resolved
| const { id } = useParams<{ id: string }>(); | ||
|
|
||
| if (!id) { | ||
| return <div>Folder not found</div>; |
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.
Wrap string in translation macro.
The "Folder not found" message should be wrapped in a translation function for internationalization support.
Apply this diff:
- return <div>Folder not found</div>;
+ return <div>{t`Folder not found`}</div>;Note: Ensure the t macro is imported from @lingui/macro.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 12-12: String not marked for translation. Wrap it with .
(lingui/no-unlocalized-strings)
🤖 Prompt for AI Agents
In
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
around line 12, the literal string "Folder not found" is rendered without
internationalization; wrap this string in the t macro (e.g., t`Folder not
found`) and ensure you import t from '@lingui/macro' at the top of the file.
Update the JSX to use the translated string and add the import if missing.
| const { folder } = useFolder(id); | ||
| const resumes = folder?.resumes; |
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.
Handle loading and error states.
The component destructures only folder from useFolder, ignoring loading and error states. Users will see an empty grid with no feedback while data is loading or if an error occurs, degrading the user experience.
Consider adding loading and error handling:
- const { folder } = useFolder(id);
+ const { folder, loading, error } = useFolder(id);
const resumes = folder?.resumes;
+
+ if (loading) {
+ return <div>{t`Loading...`}</div>;
+ }
+
+ if (error) {
+ return <div>{t`Error loading folder`}</div>;
+ }🧰 Tools
🪛 Biome (2.1.2)
[error] 15-15: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 ESLint
[error] 15-15: React Hook "useFolder" is called conditionally. React Hooks must be called in the exact same order in every component render.
(react-hooks/rules-of-hooks)
🤖 Prompt for AI Agents
In
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
around lines 15-16, change the current useFolder destructure to also pull out
loading and error, then handle those states before using folder.resumes: if
loading, return or render a loading skeleton/spinner placeholder; if error,
return or render a concise error UI/message (and optionally a retry action);
also handle the empty state when folder exists but resumes is empty by rendering
an empty-state message instead of an empty grid.
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.
The error state is handled by the app’s error boundary component in apps/client/src/router/index.tsx on line 29.
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the error boundary setup in the router file
cat apps/client/src/router/index.tsx | head -n 50
Length of output: 2406
🏁 Script executed:
#!/bin/bash
# Check how the FolderDetailsPage handles loading state for comparison
cat apps/client/src/pages/dashboard/folders/folder-details.tsx
Length of output: 3642
🏁 Script executed:
#!/bin/bash
# Check the list view version of folder resumes to see the pattern
cat apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx
Length of output: 1210
@elanizi43998, thank you for clarifying! You're right that the error boundary handles errors, and I can see the parent component FolderDetailsPage already handles the loading state with skeleton loaders (lines 21-33).
However, there's still a critical React Hooks violation in this component that needs to be fixed. The issue is that useFolder(id) is being called after the conditional return on line 11, which violates the Rules of Hooks. This causes the linter errors from both Biome and ESLint and can lead to runtime issues.
The fix is simple—move the hook call before the conditional check:
export default function FolderResumesGridView() {
const { id } = useParams<{ id: string }>();
const { folder } = useFolder(id ?? "");
if (!id) {
return <div>Folder not found</div>;
}
const resumes = folder?.resumes;
// ... rest of componentThis same pattern should also be applied to FolderResumesListView (which has the identical issue).
✏️ Learnings added
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In the Reactive Resume codebase (apps/client), error handling is managed by an error boundary configured in apps/client/src/router/index.tsx at line 29 using `<Route errorElement={<ErrorPage />}>`. Child components do not need explicit error state handling as errors bubble up to this boundary.
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In apps/client/src/pages/dashboard/folders/folder-details.tsx, the parent FolderDetailsPage component handles loading states for folder data using skeleton loaders (lines 21-33). Child components like FolderResumesGridView and FolderResumesListView inherit this pattern and do not need to duplicate loading state handling.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 1
🧹 Nitpick comments (1)
libs/dto/src/folder/update.ts (1)
3-4: Remove unused imports flagged by ESLint
resumeSchemaandfolderSchemaare not used anywhere in this file (ESLint:unused-imports/no-unused-imports), so they should be removed.Apply this diff to clean them up:
-import { resumeSchema } from "../resume"; -import { folderSchema } from "./folder";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/dto/src/folder/update.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
libs/dto/src/folder/update.ts
[error] 3-3: 'resumeSchema' is defined but never used.
(unused-imports/no-unused-imports)
[error] 4-4: 'folderSchema' is defined but never used.
(unused-imports/no-unused-imports)
🔇 Additional comments (1)
libs/dto/src/folder/update.ts (1)
6-10: Narrow update schema looks correct and saferThe
updateFolderSchemanow only exposes an optionalnamefield, which avoids accidental updates to ownership, system fields, or relations and aligns with the earlier security/data-integrity feedback.
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: 2
♻️ Duplicate comments (2)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (1)
227-227: Past review comment already addressed.The previous review flagged this line for using a relative path, but the current code already uses an absolute path with a leading slash (
"/dashboard/folders"), so that concern has been resolved.apps/server/src/folder/folder.service.ts (1)
155-164: Defensively filter resume IDs before Prismaconnect.The
getResumesIdshelper mapsresume.idwithout checking if it's defined. If any entry is missingid, Prisma will receive{ id: undefined }and throw an error.As previously suggested, harden this method:
private getResumesIds(resumes?: Partial<ResumeDto>[]) { - if (!Array.isArray(resumes) || resumes.length === 0) { - return []; - } - const resume = resumes.map((resume) => ({ - id: resume.id, - })); - - return resume; + if (!Array.isArray(resumes) || resumes.length === 0) return []; + + return resumes + .filter((r): r is { id: string } => typeof r.id === "string" && r.id.length > 0) + .map((r) => ({ id: r.id })); }
🧹 Nitpick comments (8)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (3)
69-69: Consider displaying loading and error states for folder fetching.While the conditional fetch optimization is good, the
loadinganderrorstates fromuseFoldersare not displayed in the UI. Users might see an empty list briefly before folders load, or no feedback if fetching fails.Consider adding a loading skeleton or spinner in the folder list area (lines 205-232) and an error message if
erroris present:- const { folders } = useFolders(isOpen && isMoveResumeToFolder); + const { folders, loading: foldersLoading, error: foldersError } = useFolders(isOpen && isMoveResumeToFolder);Then in the UI section, show loading/error states:
<div className="flex h-64 flex-col gap-2 space-y-2 overflow-y-auto"> {foldersLoading ? ( <div className="flex items-center justify-center h-full"> <span>Loading folders...</span> </div> ) : foldersError ? ( <DialogDescription className="text-destructive"> Failed to load folders. Please try again. </DialogDescription> ) : folders && folders.length > 0 ? ( // existing folder buttons... ) : ( // existing empty state... )} </div>
205-205: Consider usingoverflow-y-autoinstead ofoverflow-scroll.The
overflow-scrollclass forces scrollbars to always be visible. Usingoverflow-y-autoprovides a better UX by showing scrollbars only when content overflows.Apply this diff:
- <div className="flex h-64 flex-col gap-2 space-y-2 overflow-scroll"> + <div className="flex h-64 flex-col gap-2 space-y-2 overflow-y-auto">
206-206: Remove redundant null check.The condition
folders && folders?.length > 0contains a redundant check since optional chaining (folders?.length) already handles the null/undefined case.Apply this diff:
- {folders && folders?.length > 0 ? ( + {folders?.length > 0 ? (apps/client/src/pages/dashboard/folders/folder-details.tsx (3)
16-21: Avoid non‑null assertion onidand the ESLint overrideRelying on
id!plus an ESLint disable means a misconfigured route (or programmatic navigation without:id) will still calluseFolderwithundefined, likely surfacing as a backend or query error.You can keep the behavior and remove the assertion by guarding early:
- const { id } = useParams<{ id: string }>(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const { folder, loading } = useFolder(id!); + const { id } = useParams<{ id: string }>(); + if (!id) { + return null; // or reuse the "Folder not found" branch + } + const { folder, loading } = useFolder(id);This keeps types happy without suppressing lint and fails more gracefully if the route is ever misused.
63-71: Helmet title could safely assumefolderis presentBy the time this block runs,
folderis guaranteed truthy due to the preceding!loading && !folderguard, soconst name = folder?.name;is slightly more defensive than needed. Not an issue, just a minor readability nit—folder!.nameor destructuring fromfolderinside this branch would better convey the invariant.
72-111: Tabs + layout switching are correct; consider accessibility for icon‑only triggersThe controlled
Tabssetup withlayout: Layoutand matchingTabsContentvalues is type‑safe and straightforward;ScrollAreasizing also aligns with the rest of the dashboard.On small screens, though, the “Grid/List” labels are
hidden sm:block, so the tab triggers become icon‑only. Unless the Tabs implementation derives an accessible name from the value, this may leave screen‑reader users with unlabeled controls.You could add an accessible label without changing the visual design, for example:
- <TabsTrigger value="grid" className="size-8 p-0 sm:h-8 sm:w-auto sm:px-4"> - <SquaresFour /> - <span className="ml-2 hidden sm:block">{t`Grid`}</span> - </TabsTrigger> + <TabsTrigger + value="grid" + aria-label={t`Grid layout`} + className="size-8 p-0 sm:h-8 sm:w-auto sm:px-4" + > + <SquaresFour /> + <span className="ml-2 hidden sm:block">{t`Grid`}</span> + </TabsTrigger>and similarly for the list tab.
apps/server/src/folder/folder.service.ts (1)
17-17: Consider markingresumesServiceasreadonlyfor consistency.The
prismaproperty is markedreadonlybutresumesServiceis not, creating an inconsistency.Apply this diff:
- private resumesService: ResumeService, + private readonly resumesService: ResumeService,apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx (1)
52-90: De-duplicate menu actions by extracting to a shared componentThe Open/Rename/Delete actions are duplicated between the dropdown and context menu (lines 60-89 and 108-120). Extract these as a reusable component (e.g.
FolderMenuItems) to centralize markup and keep behavior in sync across both menus.Also applies to:
apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx(lines 75-142 and 160-191)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx(1 hunks)apps/client/src/pages/dashboard/folders/folder-details.tsx(1 hunks)apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx(5 hunks)apps/client/src/services/folder/delete.ts(1 hunks)apps/server/src/folder/folder.controller.ts(1 hunks)apps/server/src/folder/folder.service.ts(1 hunks)libs/dto/src/resume/resume.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/server/src/folder/folder.controller.ts
- apps/client/src/services/folder/delete.ts
- libs/dto/src/resume/resume.ts
🧰 Additional context used
🧬 Code graph analysis (4)
apps/client/src/pages/dashboard/folders/folder-details.tsx (5)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-23)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/ui/src/components/scroll-area.tsx (1)
ScrollArea(5-40)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
FolderResumesGridView(8-39)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
FolderResumesListView(8-33)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx (5)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)apps/client/src/stores/dialog.ts (1)
useDialog(34-51)libs/ui/src/components/dropdown-menu.tsx (5)
DropdownMenu(6-6)DropdownMenuTrigger(8-8)DropdownMenuContent(58-76)DropdownMenuItem(80-95)DropdownMenuSeparator(161-170)libs/ui/src/components/context-menu.tsx (5)
ContextMenu(6-6)ContextMenuTrigger(8-8)ContextMenuContent(56-70)ContextMenuItem(74-89)ContextMenuSeparator(155-164)apps/client/src/pages/dashboard/folders/_layouts/list/_components/base-item.tsx (1)
BaseListItem(12-30)
apps/server/src/folder/folder.service.ts (5)
libs/dto/src/folder/create.ts (1)
CreateFolderDto(11-11)apps/server/src/folder/types.ts (1)
FolderCountResume(3-5)libs/dto/src/folder/update.ts (1)
UpdateFolderDto(10-10)libs/dto/src/folder/move-resume.ts (1)
MoveResumeToFolderDto(10-10)libs/dto/src/resume/resume.ts (1)
ResumeDto(22-22)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (2)
apps/client/src/services/folder/folders.ts (1)
useFolders(14-26)apps/client/src/services/folder/move-resume.ts (1)
useMoveResumeToFolder(29-46)
🪛 ESLint
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx
[error] 1-54: Run autofix to sort these imports!
(simple-import-sort/imports)
🔇 Additional comments (12)
apps/client/src/pages/dashboard/resumes/_dialogs/resume.tsx (3)
76-81: LGTM!The loading state correctly aggregates all async operations, including the new
moveResumeToFolderLoading.
141-150: LGTM!The
onMoveToFolderhandler correctly validates the payload, invokes the move operation, and closes the dialog. Error handling is appropriately delegated to theuseMoveResumeToFolderhook.
196-236: LGTM! Well-structured move-resume UI.The implementation correctly:
- Conditionally renders based on mode
- Displays folders as clickable buttons
- Disables the button for the resume's current folder (lines 214-217) providing clear visual feedback
- Handles the empty state with a helpful link to create folders
- Closes the dialog after a successful move
The overall flow integrates well with the existing dialog structure.
apps/client/src/pages/dashboard/folders/folder-details.tsx (2)
21-35: Loading skeleton implementation looks solidThe 4‑card skeleton matches the grid mental model and the staggered
animationDelaywill give a nice progressive entrance without complicating the layout. No functional concerns here.
37-61: Not‑found UX and title handling are well doneThe dedicated not‑found view with a localized title, explanatory copy, and a clear CTA back to
/dashboard/foldersis clean and consistent. Helmet usage for"Folder Not Found"is also correct.apps/server/src/folder/folder.service.ts (6)
35-54: Excellent fix to scope folders by userId!The critical data-leakage issue flagged in the previous review has been properly addressed. The
where: { userId }clause on line 41 ensures that only folders belonging to the authenticated user are returned.
56-60: LGTM!The helper cleanly transforms the Prisma
_countstructure to a more intuitiveresumesCountproperty.
62-72: Excellent fix using the composite key for authorization!The ownership vulnerability flagged in the previous review has been properly addressed. Using
userId_id: { userId, id }on lines 64-65 ensures that users can only access their own folders.
74-93: LGTM!The update method properly authorizes access using the composite key and correctly transforms the result with
resumesCount.
95-153: Excellent fix to validate resume ownership!The critical authorization vulnerability flagged in the previous review has been properly addressed. Lines 130-135 now validate that the resume belongs to the user before connecting it to the target folder, preventing cross-user data access.
166-177: Excellent implementation of cascading delete with proper authorization!The ownership vulnerability flagged in the previous review has been fixed. The composite key on line 175 ensures proper authorization. Additionally, using
Promise.allSettledon lines 170-172 is a good choice—it allows the folder deletion to proceed even if individual resume deletions fail.apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx (1)
23-23: ✓ Dayjs relativeTime plugin is properly initialized globallyThe
relativeTimeplugin is safely configured and will be available whendayjs().to()is called. The setup is initialized at app startup through the root<Providers />component in the router, which loads before any components render:
apps/client/src/libs/dayjs.ts:8extends the plugin via a side-effect import inproviders/locale.tsx:1Providerswraps all routes at the root level inrouter/index.tsx:28- All components, including
folder-item.tsx, receive the globally extended dayjs instanceNo action is needed.
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx
Show resolved
Hide resolved
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: 2
🧹 Nitpick comments (4)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
21-21: Consider removing the +2 offset in animation delay.The delay calculation
(index + 2) * 0.1adds a 0.2s offset before the first item animates in. Unless this offset is intentional to coordinate with other page animations, consider usingindex * 0.1for more immediate visual feedback.Apply this diff if the offset is not needed:
- animate={{ opacity: 1, y: 0, transition: { delay: (index + 2) * 0.1 } }} + animate={{ opacity: 1, y: 0, transition: { delay: index * 0.1 } }}apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
23-23: Animation delay offset matches list view.Like the list view component, this uses
(index + 2) * 0.1for the delay calculation. If you decide to remove the +2 offset in one component, consider updating both for consistency.apps/client/src/pages/dashboard/folders/folder-details.tsx (1)
22-36: Loading skeleton layout doesn't match actual content.The skeleton uses
flex items-center justify-betweenwhich creates a horizontal flex layout, but the actual content renders either a grid (multi-column) or list (vertical) layout. This creates a noticeable layout shift when loading completes.Consider matching the skeleton to the actual layout:
if (loading) { return ( - <div className="flex items-center justify-between"> + <div className="grid grid-cols-1 gap-8 sm:grid-cols-3 xl:grid-cols-4 2xl:grid-cols-5"> {Array.from({ length: 4 }).map((_, i) => ( <div key={i} className="duration-300 animate-in fade-in" style={{ animationFillMode: "backwards", animationDelay: `${i * 300}ms` }} > <BaseCard /> </div> ))} </div> ); }apps/server/src/folder/folder.service.ts (1)
91-149: Consider wrapping move operation in a transaction.The
moveResumeToFoldermethod performs multiple database mutations (disconnect from source, connect to target) without transaction isolation. If the target folder update fails after successfully disconnecting from the source, the resume is left orphaned.Wrap the mutations in a Prisma transaction to ensure atomicity:
async moveResumeToFolder(userId: string, id: string, moveResumeDto: MoveResumeToFolderDto) { const resumeId = moveResumeDto.resumeId; - let mappedSourceFolder; + // Validate resume ownership first const resume = await this.prisma.resume.findUnique({ where: { userId_id: { userId, id: resumeId } }, }); if (!resume) { throw new ForbiddenException(); } + // Perform all mutations within a transaction + const result = await this.prisma.$transaction(async (tx) => { + let mappedSourceFolder; + - const sourceFolder = await this.prisma.folder.findFirst({ + const sourceFolder = await tx.folder.findFirst({ where: { userId, resumes: { some: { id: resumeId }, }, }, include: { resumes: true, }, }); if (sourceFolder) { - const updatedSourceFolder = await this.prisma.folder.update({ + const updatedSourceFolder = await tx.folder.update({ where: { userId_id: { userId, id: sourceFolder.id } }, data: { resumes: { disconnect: { id: resumeId }, }, }, include: { _count: { select: { resumes: true, }, }, resumes: true, }, }); mappedSourceFolder = this.mapFolderResumesCount(updatedSourceFolder); } - const targetFolder = await this.prisma.folder.update({ + + const targetFolder = await tx.folder.update({ where: { userId_id: { userId, id } }, data: { resumes: { connect: { id: resumeId } }, }, include: { _count: { select: { resumes: true, }, }, resumes: true, }, }); const mappedTargetFolder = this.mapFolderResumesCount(targetFolder); return { sourceFolder: mappedSourceFolder, targetFolder: mappedTargetFolder }; + }); + + return result; - return { sourceFolder: mappedSourceFolder, targetFolder: mappedTargetFolder }; }This ensures either both operations succeed or both are rolled back.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx(1 hunks)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx(1 hunks)apps/client/src/pages/dashboard/folders/folder-details.tsx(1 hunks)apps/client/src/services/folder/delete.ts(1 hunks)apps/client/src/services/folder/folder.ts(1 hunks)apps/server/src/folder/folder.service.ts(1 hunks)libs/dto/src/folder/create.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-item.tsx
- libs/dto/src/folder/create.ts
- apps/client/src/pages/dashboard/folders/_dialogs/folder.tsx
- apps/client/src/services/folder/delete.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In apps/client/src/pages/dashboard/folders/folder-details.tsx, the parent FolderDetailsPage component handles loading states for folder data using skeleton loaders (lines 21-33). Child components like FolderResumesGridView and FolderResumesListView inherit this pattern and do not need to duplicate loading state handling.
📚 Learning: 2025-11-20T13:57:06.492Z
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In apps/client/src/pages/dashboard/folders/folder-details.tsx, the parent FolderDetailsPage component handles loading states for folder data using skeleton loaders (lines 21-33). Child components like FolderResumesGridView and FolderResumesListView inherit this pattern and do not need to duplicate loading state handling.
Applied to files:
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsxapps/server/src/folder/folder.service.tsapps/client/src/services/folder/folder.tsapps/client/src/pages/dashboard/folders/folder-details.tsxapps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
📚 Learning: 2025-11-20T13:57:06.492Z
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In the Reactive Resume codebase (apps/client), error handling is managed by an error boundary configured in apps/client/src/router/index.tsx at line 29 using `<Route errorElement={<ErrorPage />}>`. Child components do not need explicit error state handling as errors bubble up to this boundary.
Applied to files:
apps/client/src/pages/dashboard/folders/folder-details.tsxapps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx
🧬 Code graph analysis (5)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (3)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/resumes/_layouts/list/_components/resume-item.tsx (1)
ResumeListItem(36-195)
apps/server/src/folder/folder.service.ts (5)
libs/dto/src/folder/create.ts (1)
CreateFolderDto(8-8)apps/server/src/folder/types.ts (1)
FolderCountResume(3-5)libs/dto/src/folder/update.ts (1)
UpdateFolderDto(10-10)libs/dto/src/folder/move-resume.ts (1)
MoveResumeToFolderDto(10-10)libs/dto/src/resume/resume.ts (1)
ResumeDto(22-22)
apps/client/src/services/folder/folder.ts (2)
apps/client/src/libs/axios.ts (1)
axios(15-15)libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)
apps/client/src/pages/dashboard/folders/folder-details.tsx (5)
apps/client/src/services/folder/folder.ts (1)
useFolder(12-24)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/base-card.tsx (1)
BaseCard(14-29)libs/ui/src/components/scroll-area.tsx (1)
ScrollArea(5-40)apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
FolderResumesGridView(9-33)apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
FolderResumesListView(9-31)
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (3)
libs/dto/src/folder/folder.ts (1)
FolderDto(20-20)libs/utils/src/namespaces/date.ts (1)
sortByDate(9-16)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (1)
ResumeCard(32-135)
🔇 Additional comments (5)
apps/client/src/pages/dashboard/folders/_layouts/list/_components/folder-resumes.tsx (1)
9-31: LGTM! Clean list view implementation.The component correctly sorts resumes by update date and applies smooth entrance/exit animations. The conditional rendering safely handles cases where resumes might be undefined.
apps/client/src/services/folder/folder.ts (1)
6-24: LGTM! Well-structured data fetching logic.The
findFolderByIdfunction anduseFolderhook follow React Query best practices:
- The
enabled: Boolean(id)guard prevents unnecessary queries when id is undefined or empty- Clear separation between the fetch function and the hook
- Proper typing with FolderDto
apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx (1)
9-33: LGTM! Grid view implementation is solid.The component correctly:
- Accepts folder as a prop (avoiding the previous conditional hook issue)
- Sorts resumes by update date
- Implements responsive grid layout with proper breakpoints
- Applies smooth animations with layout transitions
apps/client/src/pages/dashboard/folders/folder-details.tsx (1)
16-114: LGTM! Well-structured folder details page.The component properly handles all states:
- ✅ Loading state with animated skeleton loaders
- ✅ Not found state with helpful messaging and navigation
- ✅ Success state with grid/list toggle and proper data flow
- ✅ Hooks called unconditionally (no Rules of Hooks violations)
- ✅ Proper i18n throughout with the
tmacro- ✅ Dynamic page titles via Helmet
The use of
id ?? ""withuseFolderis safe because the hook'senabled: Boolean(id)guard prevents queries when id is falsy.Based on learnings
apps/server/src/folder/folder.service.ts (1)
162-174: LGTM: Remove method properly scoped and handles edge cases.The deletion logic correctly:
- Uses the composite key
userId_idfor ownership enforcement (line 172)- Calls the ownership-aware
findOnewhen deleting resumes (line 164)- Handles null folder with optional chaining (line 165)
Note:
Promise.allSettled(line 166) allows the folder deletion to proceed even if some resume deletions fail. This appears intentional, prioritizing folder removal over individual resume deletion failures.
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
🧹 Nitpick comments (1)
libs/dto/src/folder/update.ts (1)
1-10: DTO is lean and focused; consider droppingidfrom the body for cleaner API semanticsThis DTO now only exposes
idandname, which is a big improvement over the earlier broad.partial()usage. From an API design perspective, though, it’s usually preferable for the folder id to come exclusively from the route parameter (/folders/:id) and for the update body to only contain updatable fields (here, justname). Accepting anidin the payload can introduce ambiguity (what if route param and body disagree?) and requires extra care in the service layer.If you don’t have a concrete need for
idin the body, I’d suggest simplifying to:-import { idSchema } from "@reactive-resume/schema"; import { createZodDto } from "nestjs-zod/dto"; import { z } from "zod"; -const updateFolderSchema = z.object({ - id: idSchema.optional(), - name: z.string().min(1).optional(), -}); +const updateFolderSchema = z.object({ + name: z.string().min(1).optional(), +});This keeps the DTO aligned with “PATCH by id in path, fields in body” semantics and avoids any future confusion, while preserving the nice minimal shape you already have.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/server/src/folder/folder.service.ts(1 hunks)libs/dto/src/folder/update.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/server/src/folder/folder.service.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: elanizi43998
Repo: AmruthPillai/Reactive-Resume PR: 2455
File: apps/client/src/pages/dashboard/folders/_layouts/grid/_components/folder-resumes.tsx:15-16
Timestamp: 2025-11-20T13:57:06.492Z
Learning: In apps/client/src/pages/dashboard/folders/folder-details.tsx, the parent FolderDetailsPage component handles loading states for folder data using skeleton loaders (lines 21-33). Child components like FolderResumesGridView and FolderResumesListView inherit this pattern and do not need to duplicate loading state handling.
Hello Dear Reactive Resume community!
I have been using this app for almost a year, and it has helped me a lot in my career.
As I created multiple resumes for different versions, job applications, and languages, it became harder to find and update the right one. Sometimes I even updated the wrong resume by mistake. To fix this, I added a way to organize resumes using folders.
Folders feature
This pull request adds the Folders feature, which lets you:
I also improved the user experience: the main resumes page now shows only resumes that are not in any folder, such as new resumes or those from deleted folders.
Media
folders-gridfolders-grid-mobilefolders-listfolders-list-mobilepart-1(video)part-2 sidebar(video)Check out the attached media or try it in your own environment to see it in action.
Folders Feature Preview
Videos
Part 1
part-1.mp4
Part 2
part-2.mp4
Screenshots
Folders Grid
Folders List
Folders Grid Mobile & Folders List Mobile
PS: Apologies for the video quality, I had to compress them to meet file size limits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.