Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces comprehensive backend integration for resume, cover letter, and interview management. It adds API client modules, DTO type definitions, and utility functions for error handling and HTTP requests. The workspace and interview creation UIs are refactored to fetch, display, and mutate data using these APIs. Member session management, login/logout, and detailed document dialogs are implemented, replacing mock data with live data flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorkspacePage
participant APIClient (resume/coverletter)
participant Server
User->>WorkspacePage: Loads workspace
WorkspacePage->>APIClient: fetch resume list (memberId)
WorkspacePage->>APIClient: fetch cover letter list (memberId)
APIClient->>Server: GET /resumes/, /coverletters/me
Server-->>APIClient: Resume/Cover letter data
APIClient-->>WorkspacePage: Lists
WorkspacePage->>User: Shows document cards
User->>WorkspacePage: Clicks "Upload Resume"
WorkspacePage->>ResumeUploadDialog: Opens dialog
User->>ResumeUploadDialog: Selects file, clicks upload
ResumeUploadDialog->>APIClient: getPresignedUploadUrl(fileName)
APIClient->>Server: GET /resumes/upload
Server-->>APIClient: Presigned URL
APIClient-->>ResumeUploadDialog: URL
ResumeUploadDialog->>Server: Uploads file to S3 via presigned URL
ResumeUploadDialog->>APIClient: saveResume(metadata)
APIClient->>Server: POST /resumes/upload
Server-->>APIClient: Save result
APIClient-->>ResumeUploadDialog: Result
ResumeUploadDialog->>WorkspacePage: onSuccess (refresh list)
sequenceDiagram
participant User
participant CoverLetterForm
participant APIClient
participant Server
User->>CoverLetterForm: Fills form, submits
CoverLetterForm->>APIClient: createCoverletter(data)
APIClient->>Server: POST /coverletters
Server-->>APIClient: Response
APIClient-->>CoverLetterForm: Result
CoverLetterForm->>User: Shows success/updates list
sequenceDiagram
participant User
participant Step2
participant APIClient
participant Server
User->>Step2: Opens interview step 2
Step2->>APIClient: Fetch resumes/cover letters
APIClient->>Server: GET /resumes/, /coverletters/me
Server-->>APIClient: Data
APIClient-->>Step2: Lists
User->>Step2: Selects resume/cover letter
Step2->>APIClient: Fetch details (optional)
APIClient->>Server: GET /resumes/{id}, /coverletters/{id}
Server-->>APIClient: Detail data
APIClient-->>Step2: Detail
Step2->>User: Shows preview
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR establishes the API foundation and wires up various endpoints for resumes, cover letters, and interview creation.
- Introduces shared error classes and date conversion utility
- Implements member session context and Next.js login/workspace integration
- Adds API modules, types, and connects them in hooks and pages
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/error/error.ts | Added custom HTTP error classes |
| utils/date/convertDate.ts | Created a human-friendly date formatter |
| mock/exResumes.ts | Seed data for resume list (unused) |
| mock/exCoverLetters.ts | Seed data for cover letter list (unused) |
| hooks/useInterviewWizard.ts | Hook updated to build and submit interview payload |
| components/ui/button-logout.tsx | Logout button wired to session context |
| components/member-session-context.tsx | Provider and hooks for member session |
| components/interview/steps/Step3.tsx | Expanded voice options |
| components/cover-letter-form.tsx | Form updated to include memberId and Q&A payload |
| app/workspace/page.tsx | Workspace page wired to API queries/mutations |
| app/providers.tsx | Added MemberSessionProvider around app |
| app/login/page.tsx | Login page now sets session on temporary login |
| api/types/*.ts | Added DTO definitions for resume, interview, cover letter |
| api/resume.ts | Resume upload/detail/list API calls |
| api/interview.ts | Interview creation and management API calls |
| api/coverletter.ts | Cover letter create/list/detail API calls |
| api/api-test.ts | Test harness for all API functions |
| api/ai-interview.ts | AI interview question generation endpoint wrapper |
Comments suppressed due to low confidence (2)
hooks/useInterviewWizard.ts:70
- This debug log will run in production. Remove or guard it behind a development flag to avoid unnecessary console noise.
console.log("🧪 validate", step, ok, form); // ← 추가
mock/exResumes.ts:1
- This mock data file isn’t imported anywhere in the application. If it's no longer needed, consider removing it to reduce dead code.
export const resumes = [
|
|
||
| export class UnProcessableError extends Error { | ||
| constructor(message?: string) { | ||
| super(message || "UnProcessable"); |
There was a problem hiding this comment.
The default error message "UnProcessable" is misspelled/mismatched with the class name. Consider using "Unprocessable" for consistency.
| super(message || "UnProcessable"); | |
| super(message || "Unprocessable"); |
| import { useMemberSession } from '../member-session-context' | ||
|
|
||
| export function ButtonLogout() { | ||
| const { logout } = useMemberSession() | ||
| return ( | ||
| <button | ||
| onClick={() => { | ||
| logout() | ||
| window.location.href = '/login' |
There was a problem hiding this comment.
Directly setting window.location.href causes a full reload. Consider using Next.js router.push for smoother client-side navigation.
| import { useMemberSession } from '../member-session-context' | |
| export function ButtonLogout() { | |
| const { logout } = useMemberSession() | |
| return ( | |
| <button | |
| onClick={() => { | |
| logout() | |
| window.location.href = '/login' | |
| import { useRouter } from 'next/router' | |
| import { useMemberSession } from '../member-session-context' | |
| export function ButtonLogout() { | |
| const { logout } = useMemberSession() | |
| const router = useRouter() | |
| return ( | |
| <button | |
| onClick={() => { | |
| logout() | |
| router.push('/login') |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🔭 Outside diff range comments (1)
app/workspace/page.tsx (1)
247-266: 🛠️ Refactor suggestion
doc.type === "file"branch is unreachableThe mapping logic assigns
"resume"or"manual"totype, but the menu checks for"file". This means the download action never shows.- {"type" in doc && doc.type === "file" ? ( + {doc.type === "resume" ? (Adjust downstream checks consistently (
resume➜ download,manual➜ detail) or standardise the enum.
🧹 Nitpick comments (21)
mock/exResumes.ts (1)
1-23: Consider updating mock dates for better realism.The mock data structure is well-designed and consistent. However, the dates (2023-03-10 to 2023-05-15) are quite old relative to the current time, which might make the mock data feel outdated during development and testing.
Consider updating the dates to more recent values:
export const resumes = [ { id: 1, name: "신입 개발자 이력서.pdf", - date: "2023-05-15", + date: "2024-12-15", size: "1.2MB", type: "resume", }, { id: 2, name: "포트폴리오_2023.pdf", - date: "2023-04-22", + date: "2024-11-22", size: "3.5MB", type: "resume", }, { id: 3, name: "경력기술서_최종.docx", - date: "2023-03-10", + date: "2024-10-10", size: "890KB", type: "resume", }, ];components/ui/button-logout.tsx (2)
7-10: Consider using Next.js router for navigation instead of window.location.href.Using
window.location.hrefbypasses Next.js client-side routing and causes a full page reload. Consider using Next.js router for better performance and user experience.+import { useRouter } from 'next/navigation' + export function ButtonLogout() { const { logout } = useMemberSession() + const router = useRouter() return ( <button onClick={() => { logout() - window.location.href = '/login' + router.push('/login') }}
8-8: Add error handling for logout operation.The logout operation could potentially fail. Consider adding error handling to provide better user feedback.
onClick={() => { - logout() + try { + logout() + } catch (error) { + console.error('Logout failed:', error) + // Handle error appropriately + } router.push('/login') }}api/ai-interview.ts (1)
3-3: Remove or update the TODO comment.The comment suggests that the endpoint and types need adjustment, but since this is being reviewed for production, either implement the proper types or create a tracking issue.
Would you like me to help create a GitHub issue to track the type definition work, or should these types be implemented now?
mock/exCoverLetters.ts (2)
1-34: Add TypeScript interface for type safety.Consider defining a TypeScript interface for the cover letter mock data to ensure type consistency and catch potential errors.
+interface CoverLetter { + id: number + name: string + date: string + category: 'coverLetter' + type: 'file' | 'manual' + size?: string + questions?: number +} + -export const coverLetters = [ +export const coverLetters: CoverLetter[] = [ { id: 1,
4-5: Consider using ISO date format for consistency.The dates are in YYYY-MM-DD format which is good, but consider using ISO 8601 format with time for better consistency with typical API responses.
For example:
"2023-05-10T00:00:00Z"instead of"2023-05-10"Also applies to: 12-13, 28-29
components/interview/steps/Step3.tsx (2)
25-32: Consider more diverse sample phrases for better user experience.All voice options use very similar sample phrases. Consider using more diverse phrases that better demonstrate the different voice characteristics (age/gender).
{[ - { value: "female20", label: "👩🦰 여성(20대)", sample: "안녕하세요! 면접을 시작하겠습니다." }, - { value: "female30", label: "👩 여성(30대)", sample: "안녕하세요! 준비되셨나요?" }, - { value: "female40", label: "👩🦳 여성(40대)", sample: "안녕하세요! 편안하게 답변해주세요." }, - { value: "female50", label: "👵 여성(50대)", sample: "안녕하세요! 좋은 면접이 되길 바랍니다." }, + { value: "female20", label: "👩🦰 여성(20대)", sample: "안녕하세요! 오늘 면접 잘 부탁드려요!" }, + { value: "female30", label: "👩 여성(30대)", sample: "반갑습니다. 편안하게 대화해보시죠." }, + { value: "female40", label: "👩🦳 여성(40대)", sample: "안녕하세요. 차분하게 답변해주세요." }, + { value: "female50", label: "👵 여성(50대)", sample: "좋은 시간이 되길 바랍니다." },
25-32: Consider accessibility improvements for emoji usage.The emoji icons provide visual distinction but may not be accessible to screen readers. Consider adding aria-labels or additional text descriptions.
<CardContent className="p-4 text-center"> - <h3 className="font-semibold mb-2">{voice.label}</h3> + <h3 className="font-semibold mb-2" aria-label={voice.label.replace(/👩🦰|👩|👩🦳|👵|👨🦱|👨|👨🦳|👴/g, '')}>{voice.label}</h3>api/api-test.ts (1)
21-26: Use dynamic test data instead of hardcoded values.The hardcoded
fileUrland other test values may not accurately reflect real usage scenarios and could cause test failures.Consider generating dynamic test data:
const resumeResult = await saveResume({ memberId: 1, - fileName: "test.pdf", - fileUrl: "https://example.com/test.pdf", - fileSize: 12345, + fileName: `test-${Date.now()}.pdf`, + fileUrl: presigned.data?.uploadUrl || "https://example.com/test.pdf", + fileSize: Math.floor(Math.random() * 100000) + 1000, });utils/error/error.ts (1)
1-20: Consider adding more HTTP error classes and standardizing constructor patterns.The current implementation covers basic error cases but could be expanded for better error handling coverage.
Consider adding more error types and standardizing the constructor pattern:
+ export abstract class HttpError extends Error { + public readonly statusCode: number; + public readonly context?: Record<string, any>; + + constructor(statusCode: number, message: string, context?: Record<string, any>) { + super(message); + this.statusCode = statusCode; + this.context = context; + this.name = this.constructor.name; + } + } - export class BadRequestError extends Error { + export class BadRequestError extends HttpError { - constructor(message?: string) { - super(message || "Bad Request"); - this.name = "BadRequestError"; + constructor(message?: string, context?: Record<string, any>) { + super(400, message || "Bad Request", context); } } + export class UnauthorizedError extends HttpError { + constructor(message?: string, context?: Record<string, any>) { + super(401, message || "Unauthorized", context); + } + } + + export class InternalServerError extends HttpError { + constructor(message?: string, context?: Record<string, any>) { + super(500, message || "Internal Server Error", context); + } + }utils/fetch/fetch.ts (1)
22-22: Remove or make logging configurable.Console logging in production code can be problematic for performance and security. Consider making it configurable or removing it.
+ interface FetchConfig { + baseURL: string; + debug?: boolean; + } - function createFetch(baseURL: string) { + function createFetch(config: FetchConfig | string) { + const { baseURL, debug } = typeof config === 'string' + ? { baseURL: config, debug: false } + : config; const get = async <T>(/*...*/) => { - console.log("GET : ", url); + if (debug) console.log("GET : ", url); // ... };Also applies to: 54-54, 99-99
components/member-session-context.tsx (1)
5-9: Consider adding session expiration and additional context data.The current session context is minimal. Consider adding session expiration and user metadata for better session management.
interface MemberSessionContextProps { memberId: number | null + memberData?: { + name?: string; + email?: string; + role?: string; + } | null + sessionExpiry?: Date | null + isSessionValid: boolean login: (id: number) => void + loginWithData: (id: number, data: any, expiryHours?: number) => void logout: () => void + refreshSession: () => Promise<void> }This would allow for richer session management including automatic session validation and user metadata caching.
hooks/useInterviewWizard.ts (1)
76-77: Consider using more specific error types.While the error message is user-friendly, consider creating specific error types for better error handling and debugging.
Example implementation:
class ValidationError extends Error { constructor(message: string) { super(message); this.name = 'ValidationError'; } } if (!form.resumeId || !form.coverLetterId) { throw new ValidationError("이력서/자기소개서가 선택되지 않았습니다."); }api/resume.ts (2)
34-34: Remove trailing slash from the endpoint URL.The endpoint
/resumes/has a trailing slash which might cause issues depending on the server configuration. For consistency with other endpoints, remove the trailing slash.Apply this diff:
- return serverFetch.get<ApiResponseResumeListDTO>("/resumes/", { memberId }); + return serverFetch.get<ApiResponseResumeListDTO>("/resumes", { memberId });
12-14: Consider adding error handling for API calls.While the functions are clean, they don't include explicit error handling. Consider wrapping these calls in try-catch blocks or documenting that error handling should be done by the caller.
Would you like me to generate a wrapper utility that adds consistent error handling and logging for all API calls? This could help standardize error handling across the application.
Also applies to: 27-29, 34-34
components/cover-letter-form.tsx (1)
62-62: Consider adding null check for memberId.While
useRequireMemberIdshould handle authentication, adding an explicit check would improve defensive programming.const handleSubmit = () => { + if (!memberId) { + console.error('Member ID is required'); + return; + } onSubmit({ memberId: memberId!, corporateName, jobName, qnaDTOList: questionAnswerPairs.map(({ question, answer }) => ({ question, answer })), }) }api/types/resume-types.ts (1)
38-38: Consider expanding supported file types.The
fileTypeis currently restricted to only "PDF". This might be limiting if the application needs to support other resume formats in the future.Consider making it more flexible:
- fileType?: "PDF"; + fileType?: "PDF" | "DOC" | "DOCX";Or if PDF-only is a business requirement, consider adding a comment explaining this restriction:
- fileType?: "PDF"; + fileType?: "PDF"; // Currently only PDF format is supportedapi/types/interview-types.ts (1)
75-80: DTO interface names deviate from PascalCase convention
changeMemberStatusDTO(andendInterviewRequestDTObelow) start with a lower-case letter, breaking the otherwise consistent PascalCase naming in this file.-export interface changeMemberStatusDTO { +export interface ChangeMemberStatusDTO { memberId: number; status: "NO_SHOW" | "SCHEDULED" | "IN_PROGRESS" | "DONE"; }(Apply the same treatment to
endInterviewRequestDTO➜EndInterviewRequestDTO.)components/interview/steps/Step2.tsx (2)
254-272: File-upload XHR lacks abort handling & leaks if the dialog closesIf the user closes the dialog mid-upload or the component unmounts, the
XMLHttpRequestkeeps running, potentially causing state updates on an unmounted component and wasted bandwidth.A minimal, self-contained fix:
-const xhr = new XMLHttpRequest() +const xhr = new XMLHttpRequest() +// Allow cancelling when the component unmounts +const abort = () => { try { xhr.abort() } catch (_) {} } +// Cleanup +window.addEventListener('beforeunload', abort) +// ... +xhr.onload = () => { + window.removeEventListener('beforeunload', abort) + // existing logic … +} +xhr.onerror = () => { + window.removeEventListener('beforeunload', abort) + // existing logic … +}Alternatively, switch to
fetchwith anAbortController; you already lose progress events for fetch so the current approach is acceptable if the abort is addressed.🧰 Tools
🪛 Biome (1.9.4)
[error] 258-259: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
80-89:coverLetterSchemais declared but never usedDead code increases bundle size and confuses future readers. If you intend to validate the dialog’s form, wire this schema into
zodResolver&react-hook-form; otherwise, please remove it.app/workspace/page.tsx (1)
397-468: Duplicated resume-upload component across files – extract to a shared module
ResumeUploadDialoghere is virtually identical to the one inStep2.tsx. Duplication invites drift and double maintenance when S3 logic changes. Export a single<ResumeUploadDialog>component fromcomponents/resume/resume-upload-dialog.tsxand reuse it in both places.Benefits:
• one-time bug fixes
• lighter bundle (tree-shakable)
• consistent UX🧰 Tools
🪛 Biome (1.9.4)
[error] 426-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
api/ai-interview.ts(1 hunks)api/api-test.ts(1 hunks)api/coverletter.ts(1 hunks)api/interview.ts(1 hunks)api/resume.ts(1 hunks)api/types/coverletter-types.ts(1 hunks)api/types/interview-types.ts(1 hunks)api/types/resume-types.ts(1 hunks)app/login/page.tsx(2 hunks)app/providers.tsx(1 hunks)app/workspace/page.tsx(9 hunks)components/cover-letter-form.tsx(3 hunks)components/interview/steps/Step2.tsx(2 hunks)components/interview/steps/Step3.tsx(1 hunks)components/member-session-context.tsx(1 hunks)components/ui/button-logout.tsx(1 hunks)hooks/useInterviewWizard.ts(3 hunks)mock/exCoverLetters.ts(1 hunks)mock/exResumes.ts(1 hunks)utils/date/convertDate.ts(1 hunks)utils/error/error.ts(1 hunks)utils/fetch/fetch.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
app/login/page.tsx (1)
components/member-session-context.tsx (1)
useMemberSession(39-43)
components/ui/button-logout.tsx (1)
components/member-session-context.tsx (1)
useMemberSession(39-43)
app/providers.tsx (1)
components/member-session-context.tsx (1)
MemberSessionProvider(13-37)
api/ai-interview.ts (1)
utils/fetch/fetch.ts (1)
aiFetch(135-135)
api/api-test.ts (4)
api/resume.ts (2)
getPresignedUploadUrl(11-15)saveResume(18-23)api/coverletter.ts (3)
createCoverletter(10-15)getCoverletterDetail(18-22)findMyCoverletter(25-29)api/interview.ts (3)
createMemberInterview(22-30)changeParticipantsStatus(33-41)terminateInterview(44-52)api/ai-interview.ts (1)
generateInterviewQuestions(4-6)
hooks/useInterviewWizard.ts (1)
api/types/interview-types.ts (1)
InterviewCreateDTO(1-30)
utils/fetch/fetch.ts (2)
utils/error/error.ts (3)
BadRequestError(1-6)NotFoundError(8-13)UnProcessableError(15-20)lib/api.ts (1)
post(1-15)
api/coverletter.ts (2)
api/types/coverletter-types.ts (4)
CreateCoverletterDTO(6-11)ApiResponseCreateCoverletterDTO(13-18)ApiResponseCoverletterDetailDTO(33-38)ApiResponseMyCoverletterListDTO(51-56)utils/fetch/fetch.ts (1)
serverFetch(135-135)
api/resume.ts (2)
utils/fetch/fetch.ts (1)
serverFetch(135-135)api/types/resume-types.ts (5)
ApiResponsePresignedUploadDTO(26-31)ResumeCreateDTO(1-6)ApiResponseCreateResumeResultDTO(13-18)ApiResponseResumeDetailDTO(43-48)ApiResponseResumeListDTO(60-65)
api/interview.ts (2)
api/types/interview-types.ts (8)
InterviewCreateDTO(1-30)ApiResponseInterviewCreateResultDTO(37-42)CreateMemberInterviewDTO(57-61)ApiResponseCreateMemberInterviewDTO(63-68)changeMemberStatusDTO(75-78)ApiResponseMemberInterviewStatusDTO(80-85)endInterviewRequestDTO(93-95)ApiResponseInterviewEndResultDTO(97-102)utils/fetch/fetch.ts (1)
serverFetch(135-135)
🪛 Biome (1.9.4)
components/interview/steps/Step2.tsx
[error] 258-259: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
app/workspace/page.tsx
[error] 426-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (17)
app/providers.tsx (1)
9-9: LGTM! Proper integration of session management.The
MemberSessionProvideris correctly imported and integrated into the provider hierarchy. The nesting withinQueryClientProviderensures that session context is available throughout the application while maintaining proper provider ordering.Also applies to: 17-19
components/interview/steps/Step3.tsx (1)
38-38:Details
❓ Verification inconclusive
Remove unsafe type casting and ensure proper typing.
The
as anytype assertion bypasses TypeScript's type checking and should be avoided. Ensure the voice values match the expectedvoiceTypetype definition.- onClick={() => patch({ voiceType: voice.value as any })} + onClick={() => patch({ voiceType: voice.value })}Please verify that the voice values (
female20,male30, etc.) match the expectedvoiceTypeenum or union type definition:
🏁 Script executed:
#!/bin/bash # Search for voiceType type definition to verify allowed values ast-grep --pattern 'voiceType: $_' rg -A 5 -B 5 "voiceType.*="Length of output: 1249
🏁 Script executed:
#!/bin/bash # Show the voices array definition and related types in Step3.tsx rg -n "const voices" -A 10 -B 5 components/interview/steps/Step3.tsx # Search for a VoiceType type or enum definition elsewhere in the repo rg -n "type VoiceType" || rg -n "enum VoiceType"Length of output: 120
🏁 Script executed:
#!/bin/bash # 1. Find where the useInterview hook is defined and show its signature rg -n "export function useInterview" -g "*.ts" -g "*.tsx" -A10 # 2. Look for any interface/type definitions that include a voiceType property rg -n "interface .*{[^}]*voiceType" -g "*.ts" -g "*.tsx" -A5 rg -n "type .* =.*voiceType" -g "*.ts" -g "*.tsx" -A5Length of output: 4071
#!/bin/bash # Locate initialFormState definition to inspect default values including voiceType rg -n "initialFormState" -A5 -B5 hooks/useInterviewWizard.ts # Locate the InterviewFormState type or interface to see the voiceType property type rg -n "InterviewFormState" -A5 -B5 hooks/useInterviewWizard.ts
Ensure proper typing for
voiceTypebefore removing the castI wasn’t able to locate a
voiceTypetype or enum in the codebase—please confirm that your form state definesvoiceTypeas a union including the new values ("female20","male30", etc.). Once that’s in place, you can safely remove the unsafe assertion:- onClick={() => patch({ voiceType: voice.value as any })} + onClick={() => patch({ voiceType: voice.value })}• Verify
InterviewFormStateinhooks/useInterviewWizard.tslists all voice options.
• Update the type definition if needed to include the new values.
• Then drop theas anycast incomponents/interview/steps/Step3.tsx.hooks/useInterviewWizard.ts (1)
8-9: LGTM! Clean import organization.The import changes correctly reflect the new centralized API structure, improving modularity and maintainability.
api/coverletter.ts (2)
1-30: Well-structured API client module!The module follows a clean and consistent pattern with proper TypeScript typing and clear function naming. The use of typed DTOs ensures type safety across the application.
25-29:Details
✅ Verification successful
Verify the query parameter structure for GET requests.
The current implementation passes
memberIdas the second parameter toserverFetch.get(). Ensure that theserverFetchutility correctly handles this as a query parameter.
🏁 Script executed:
#!/bin/bash # Description: Verify how serverFetch handles query parameters for GET requests # Search for the serverFetch implementation to understand parameter handling ast-grep --pattern 'const serverFetch = { $$$ get($$$) { $$$ } $$$ }' # Also check for any usage patterns of serverFetch.get with parameters rg -A 5 'serverFetch\.get.*\(' --glob '*.{ts,tsx,js,jsx}'Length of output: 1163
To confirm how query parameters are applied, let’s inspect the
serverFetchimplementation:
🏁 Script executed:
#!/bin/bash # Locate and display the serverFetch implementation to verify how `get` handles query params file=$(fd serverFetch.ts 2>/dev/null || fd serverFetch.js 2>/dev/null) echo "Found serverFetch at: $file" sed -n '1,200p' "$file"Length of output: 146
Let’s locate the
serverFetchimplementation to confirm how.get()constructs query parameters:
🏁 Script executed:
#!/bin/bash # Search for exported serverFetch definition file=$(rg -lP "export\s+const\s+serverFetch\s*=" . | head -1) # Fallback to non-exported const if not found if [[ -z "$file" ]]; then file=$(rg -lP "const\s+serverFetch\s*=" . | head -1) fi if [[ -n "$file" ]]; then echo "Found serverFetch in: $file" sed -n '1,200p' "$file" else echo "⚠️ serverFetch implementation not found by patterns; please verify manually." fiLength of output: 3660
Query parameters are handled correctly by
serverFetch.getThe
serverFetch.getmethod serializes its optional second argument into a URL query string viaURLSearchParams, so calling:serverFetch.get<ApiResponseMyCoverletterListDTO>( "/coverletters/me", { memberId } );will result in a request to
/coverletters/me?memberId=<number>. No changes are needed here.api/resume.ts (1)
1-36: Clean and well-organized API client implementation!The module maintains consistency with other API modules and provides a clear interface for resume-related operations. The separation of concerns between presigned URL generation and metadata saving is well-designed.
api/types/coverletter-types.ts (2)
1-11: Excellent type structure for cover letter creation.The DTOs are well-structured with clear separation between creation and retrieval interfaces. The naming convention is consistent and follows good TypeScript practices.
13-18: Consistent API response wrapper pattern.Good use of consistent API response wrapper interfaces across all endpoints. This provides standardized error handling and response structure throughout the application.
Also applies to: 33-38, 51-56
api/interview.ts (2)
1-11: Well-organized imports and type definitions.Good use of explicit imports for all required types from the interview-types module. This ensures type safety across the API client.
13-19: Excellent implementation of typed API functions.The functions are well-structured with proper generic typing, clear parameter definitions, and appropriate HTTP methods for each operation. The use of template literals for dynamic URLs is also well-implemented.
Also applies to: 21-30, 32-41, 43-52
components/cover-letter-form.tsx (4)
11-11: Good integration with member session context.The use of
useRequireMemberIdhook properly ensures that only authenticated users can access the form functionality.Also applies to: 29-29
14-25: Excellent refactoring of form data structure.The change from a single title field to separate
corporateNameandjobNamefields aligns well with the API requirements and provides better data granularity.
70-75: Comprehensive form validation logic.The validation properly checks all required fields including both the new corporate/job name fields and ensures all Q&A pairs are complete.
80-99: Clear and user-friendly form fields.The separate input fields for corporate name and job name provide better UX with appropriate placeholder text and labels.
api/types/resume-types.ts (3)
1-18: Well-structured resume creation types.The DTOs properly separate creation data from result data, and the API response wrapper follows the established pattern.
20-31: Good support for presigned upload workflow.The presigned upload DTOs properly support modern file upload patterns with S3 or similar storage services.
50-65: Consistent API response structure across all endpoints.The resume list and detail response types maintain consistency with the established API response pattern, which will simplify error handling and data processing.
utils/date/convertDate.ts
Outdated
| if (diffHours < 24) { | ||
| return `${diffHours}시간 전`; | ||
| } else if (diffMinutes < 60) { | ||
| return `${diffMinutes}분 전`; | ||
| } else if (diffSeconds < 60) { | ||
| return `${diffSeconds}초 전`; | ||
| } |
There was a problem hiding this comment.
Fix the conditional logic flow.
The current conditional logic has a fundamental flaw in the order of checks. The conditions should be structured to handle time units from largest to smallest, and the current logic allows fall-through to smaller units even when larger units should take precedence.
- if (diffHours < 24) {
- return `${diffHours}시간 전`;
- } else if (diffMinutes < 60) {
- return `${diffMinutes}분 전`;
- } else if (diffSeconds < 60) {
- return `${diffSeconds}초 전`;
- }
+ if (diffHours >= 24) {
+ return `${year}년 ${month}월 ${day}일`;
+ } else if (diffHours > 0) {
+ return `${diffHours}시간 전`;
+ } else if (diffMinutes > 0) {
+ return `${diffMinutes}분 전`;
+ } else {
+ return `${diffSeconds}초 전`;
+ }
-
- return `${year}년 ${month}월 ${day}일`;📝 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.
| if (diffHours < 24) { | |
| return `${diffHours}시간 전`; | |
| } else if (diffMinutes < 60) { | |
| return `${diffMinutes}분 전`; | |
| } else if (diffSeconds < 60) { | |
| return `${diffSeconds}초 전`; | |
| } | |
| if (diffHours >= 24) { | |
| return `${year}년 ${month}월 ${day}일`; | |
| } else if (diffHours > 0) { | |
| return `${diffHours}시간 전`; | |
| } else if (diffMinutes > 0) { | |
| return `${diffMinutes}분 전`; | |
| } else { | |
| return `${diffSeconds}초 전`; | |
| } |
🤖 Prompt for AI Agents
In utils/date/convertDate.ts around lines 14 to 20, the conditional checks for
time differences are ordered incorrectly, causing smaller units like minutes or
seconds to be returned even when larger units like hours apply. Reorder the
conditions to check from largest to smallest time units, starting with hours,
then minutes, and finally seconds, ensuring the correct time unit is returned
based on the difference.
| </div> | ||
|
|
||
| <Button variant="outline" className="w-full" onClick={() => (window.location.href = "/workspace")}> | ||
| <Button variant="outline" className="w-full" onClick={() => { login(1); window.location.href = "/workspace" }}> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address hard-coded member ID and improve navigation.
The onClick handler has several issues that should be addressed:
- Hard-coded member ID: Using
login(1)with a fixed ID appears to be temporary development code - Navigation method: Using
window.location.hrefbypasses Next.js routing and doesn't provide the benefits of client-side navigation - Missing error handling: No error handling for the login operation
Consider this improved implementation:
- <Button variant="outline" className="w-full" onClick={() => { login(1); window.location.href = "/workspace" }}>
+ <Button variant="outline" className="w-full" onClick={async () => {
+ try {
+ // TODO: Replace with actual user ID from authentication
+ await login(1);
+ router.push("/workspace");
+ } catch (error) {
+ console.error("Login failed:", error);
+ // TODO: Show error message to user
+ }
+ }}>You'll also need to import the router:
"use client"
import Link from "next/link"
+import { useRouter } from "next/navigation"
import { Button } from "@/components/ui/button"And add it to the component:
export default function LoginPage() {
+ const router = useRouter()
const { login } = useMemberSession()🤖 Prompt for AI Agents
In app/login/page.tsx at line 44, replace the hard-coded login call login(1)
with a dynamic or state-driven member ID to avoid fixed values. Instead of using
window.location.href for navigation, use Next.js's useRouter hook and its
router.push method for client-side routing. Add proper error handling around the
login call to manage failures gracefully. Import useRouter from
'next/navigation', initialize it in the component, and update the onClick
handler to await login, handle errors, and navigate using router.push.
| export async function generateInterviewQuestions(payload: any) { | ||
| return aiFetch.post<any, any>("/ai/interview/questions", payload); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any types with proper TypeScript interfaces.
Using any types defeats the purpose of TypeScript's type safety. Define proper interfaces for the request payload and response.
+interface InterviewQuestionPayload {
+ resumeId?: string
+ coverLetterId?: string
+ jobPosition?: string
+ interviewStyle?: 'personality' | 'technical'
+ questionCount?: number
+}
+
+interface InterviewQuestionResponse {
+ questions: Array<{
+ id: string
+ question: string
+ category?: string
+ }>
+}
+
-export async function generateInterviewQuestions(payload: any) {
- return aiFetch.post<any, any>("/ai/interview/questions", payload);
+export async function generateInterviewQuestions(payload: InterviewQuestionPayload) {
+ return aiFetch.post<InterviewQuestionResponse, InterviewQuestionPayload>("/ai/interview/questions", payload);
}📝 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.
| export async function generateInterviewQuestions(payload: any) { | |
| return aiFetch.post<any, any>("/ai/interview/questions", payload); | |
| } | |
| interface InterviewQuestionPayload { | |
| resumeId?: string | |
| coverLetterId?: string | |
| jobPosition?: string | |
| interviewStyle?: 'personality' | 'technical' | |
| questionCount?: number | |
| } | |
| interface InterviewQuestionResponse { | |
| questions: Array<{ | |
| id: string | |
| question: string | |
| category?: string | |
| }> | |
| } | |
| export async function generateInterviewQuestions(payload: InterviewQuestionPayload) { | |
| return aiFetch.post<InterviewQuestionResponse, InterviewQuestionPayload>( | |
| "/ai/interview/questions", | |
| payload | |
| ); | |
| } |
🤖 Prompt for AI Agents
In api/ai-interview.ts around lines 4 to 6, the function
generateInterviewQuestions uses `any` types for both the payload parameter and
the response type, which undermines type safety. Define appropriate TypeScript
interfaces for the request payload structure and the expected response format,
then replace the `any` types with these interfaces in the function signature and
the aiFetch.post call to ensure proper type checking.
| // 4. 자기소개서 상세 조회 | ||
| const coverletterDetail = await getCoverletterDetail(1); | ||
| console.log("coverletterDetail:", coverletterDetail); | ||
|
|
||
| // 5. 내 자기소개서 리스트 조회 | ||
| const myCoverletters = await findMyCoverletter(1); | ||
| console.log("myCoverletters:", myCoverletters); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoded entity IDs in dependent API calls.
Using hardcoded coverletterId: 1 may cause test failures if this entity doesn't exist in the test environment.
Use the ID from the previously created cover letter:
// 4. 자기소개서 상세 조회
- const coverletterDetail = await getCoverletterDetail(1);
+ const coverletterDetail = await getCoverletterDetail(
+ coverletterResult.data?.coverletterId || 1
+ );
console.log("coverletterDetail:", coverletterDetail);
// 5. 내 자기소개서 리스트 조회
const myCoverletters = await findMyCoverletter(1);📝 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.
| // 4. 자기소개서 상세 조회 | |
| const coverletterDetail = await getCoverletterDetail(1); | |
| console.log("coverletterDetail:", coverletterDetail); | |
| // 5. 내 자기소개서 리스트 조회 | |
| const myCoverletters = await findMyCoverletter(1); | |
| console.log("myCoverletters:", myCoverletters); | |
| // 4. 자기소개서 상세 조회 | |
| const coverletterDetail = await getCoverletterDetail( | |
| coverletterResult.data?.coverletterId || 1 | |
| ); | |
| console.log("coverletterDetail:", coverletterDetail); | |
| // 5. 내 자기소개서 리스트 조회 | |
| const myCoverletters = await findMyCoverletter(1); | |
| console.log("myCoverletters:", myCoverletters); |
🤖 Prompt for AI Agents
In api/api-test.ts around lines 38 to 44, avoid using the hardcoded
coverletterId value of 1 in the getCoverletterDetail and findMyCoverletter
calls. Instead, capture the ID from the cover letter created earlier in the test
and use that dynamic ID in these calls to ensure the tests do not fail due to
missing entities.
| // 내 인터뷰 리스트 조회 | ||
| export async function getMyInterviewList() { | ||
| return serverFetch.get<any>("/interviews"); | ||
| } |
There was a problem hiding this comment.
Fix missing type definition for interview list response.
The function uses any type which breaks type safety. Based on the pattern in other files, there should be a proper response type defined.
-export async function getMyInterviewList() {
- return serverFetch.get<any>("/interviews");
-}
+export async function getMyInterviewList() {
+ return serverFetch.get<ApiResponseMyInterviewListDTO>("/interviews");
+}You'll need to define the corresponding types in api/types/interview-types.ts:
export interface MyInterviewDTO {
interviewId?: number;
name?: string;
corporateName?: string;
jobName?: string;
status?: string;
createdAt?: string;
// Add other relevant fields
}
export interface MyInterviewListDTO {
interviews?: MyInterviewDTO[];
}
export interface ApiResponseMyInterviewListDTO {
isSuccess?: boolean;
code?: string;
message?: string;
result?: MyInterviewListDTO;
}🤖 Prompt for AI Agents
In api/interview.ts around lines 54 to 57, the getMyInterviewList function
currently returns a response typed as any, which breaks type safety. To fix
this, define the appropriate response types in api/types/interview-types.ts as
suggested (MyInterviewDTO, MyInterviewListDTO, ApiResponseMyInterviewListDTO)
and update the function's return type to use ApiResponseMyInterviewListDTO
instead of any. This will ensure proper typing and consistency with other API
response patterns.
| export interface ApiResponseCreateMemberInterviewDTO { | ||
| isSuccess?: boolean; | ||
| code?: string; | ||
| message?: string; | ||
| result?: CreateMemberInterviewDTO; | ||
| } |
There was a problem hiding this comment.
ApiResponseCreateMemberInterviewDTO returns the wrong result type
The API response should wrap CreateMemberInterviewResultDTO, not the request DTO. Returning the input shape loses server-generated fields such as memberInterviewId.
export interface ApiResponseCreateMemberInterviewDTO {
isSuccess?: boolean;
code?: string;
message?: string;
- result?: CreateMemberInterviewDTO;
+ result?: CreateMemberInterviewResultDTO;
}📝 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.
| export interface ApiResponseCreateMemberInterviewDTO { | |
| isSuccess?: boolean; | |
| code?: string; | |
| message?: string; | |
| result?: CreateMemberInterviewDTO; | |
| } | |
| export interface ApiResponseCreateMemberInterviewDTO { | |
| isSuccess?: boolean; | |
| code?: string; | |
| message?: string; | |
| result?: CreateMemberInterviewResultDTO; | |
| } |
🤖 Prompt for AI Agents
In api/types/interview-types.ts between lines 63 and 68, the
ApiResponseCreateMemberInterviewDTO interface incorrectly uses
CreateMemberInterviewDTO as the type for the result property. Update the result
property type to CreateMemberInterviewResultDTO to correctly represent the
server response including generated fields like memberInterviewId.
| export interface CreateMemberInterviewDTO { | ||
| memberId: number; | ||
| resumeId: number; | ||
| coverletterId: number; | ||
| } | ||
|
|
||
| export interface ApiResponseCreateMemberInterviewDTO { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent camel-casing – coverletterId vs coverLetterId used elsewhere
Most of the codebase (e.g. Step2.tsx, workspace/page.tsx) uses coverLetterId (capital “L”) while this DTO defines coverletterId (lower-case “l”). This small mismatch silently defeats type-safety and complicates auto-completion.
export interface CreateMemberInterviewDTO {
memberId: number;
resumeId: number;
- coverletterId: number;
+ coverLetterId: number;
}📝 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.
| export interface CreateMemberInterviewDTO { | |
| memberId: number; | |
| resumeId: number; | |
| coverletterId: number; | |
| } | |
| export interface ApiResponseCreateMemberInterviewDTO { | |
| export interface CreateMemberInterviewDTO { | |
| memberId: number; | |
| resumeId: number; | |
| coverLetterId: number; | |
| } | |
| export interface ApiResponseCreateMemberInterviewDTO { |
🤖 Prompt for AI Agents
In api/types/interview-types.ts around lines 57 to 63, the property name
coverletterId in CreateMemberInterviewDTO uses inconsistent camel-casing
compared to coverLetterId used elsewhere. Rename coverletterId to coverLetterId
to match the rest of the codebase, ensuring consistent camel-casing for type
safety and better auto-completion.
| const queryClient = useQueryClient() | ||
| const memberId = typeof window !== 'undefined' ? Number(localStorage.getItem('memberId')) : undefined | ||
| // Dialog 상태 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
memberId extraction may yield 0 (truthy-falsey bug) and duplicates context logic
Number(localStorage.getItem('memberId')) returns 0 (falsy) when the key is absent, disabling the queries even though the value exists but is invalid. Moreover, the rest of the app already offers useRequireMemberId() for this purpose.
-const memberId = typeof window !== 'undefined' ? Number(localStorage.getItem('memberId')) : undefined
+const memberId = useRequireMemberId() // reuse the centralised hook
+if (!memberId) throw new Error('memberId missing – user should be redirected by the hook')This removes duplicated logic, prevents the accidental 0 case, and keeps the retrieval strategy consistent.
🤖 Prompt for AI Agents
In components/interview/steps/Step2.tsx around lines 35 to 37, replace the
manual extraction of memberId from localStorage using
Number(localStorage.getItem('memberId')) with the existing useRequireMemberId()
hook from the app. This avoids the falsy-zero bug and removes duplicated logic
by using the centralized memberId retrieval method, ensuring consistent and
correct memberId handling across the app.
| enabled: !!memberId, | ||
| select: (data) => { | ||
| const resumes = data.result?.resumes | ||
| return resumes?.map((resume) => ({ | ||
| id: resume.resumeId, | ||
| name: resume.fileName, | ||
| date: null, // 상세조회에서 가져올 수 있음 | ||
| size: resume.fileSize ? `${(resume.fileSize / 1024).toFixed(1)}KB` : '-', | ||
| type: "resume", | ||
| })) | ||
| }, |
There was a problem hiding this comment.
Resume items set date: null but later pass it to convertDate
convertDate(null) will likely yield "Invalid Date" or throw, leading to runtime issues.
- date: null, // 상세조회에서 가져올 수 있음
+ date: resume.createdAt ?? '', // preserve ISO string or empty for safetyConsider falling back to the detailed API only when the field is missing.
📝 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.
| enabled: !!memberId, | |
| select: (data) => { | |
| const resumes = data.result?.resumes | |
| return resumes?.map((resume) => ({ | |
| id: resume.resumeId, | |
| name: resume.fileName, | |
| date: null, // 상세조회에서 가져올 수 있음 | |
| size: resume.fileSize ? `${(resume.fileSize / 1024).toFixed(1)}KB` : '-', | |
| type: "resume", | |
| })) | |
| }, | |
| enabled: !!memberId, | |
| select: (data) => { | |
| const resumes = data.result?.resumes | |
| return resumes?.map((resume) => ({ | |
| id: resume.resumeId, | |
| name: resume.fileName, | |
| - date: null, // 상세조회에서 가져올 수 있음 | |
| + date: resume.createdAt ?? '', // preserve ISO string or empty for safety | |
| size: resume.fileSize ? `${(resume.fileSize / 1024).toFixed(1)}KB` : '-', | |
| type: "resume", | |
| })) | |
| }, |
🤖 Prompt for AI Agents
In app/workspace/page.tsx around lines 70 to 80, the resume items set the date
field to null, which causes runtime issues when passed to convertDate. Instead
of setting date to null, modify the code to omit the date or provide a valid
fallback value, and only call the detailed API to fetch the date if it is
missing. This prevents convertDate from receiving null and avoids invalid date
errors.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
app/workspace/page.tsx (1)
76-76:⚠️ Potential issueCritical issue: Passing null to convertDate function
This is a known issue from previous reviews. Setting
date: nullwill cause runtime errors whenconvertDate(null)is called on line 236.Apply this fix to handle the missing date properly:
- date: null, // 상세조회에서 가져올 수 있음 + date: resume.createdAt ?? '',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/workspace/page.tsx(9 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
app/workspace/page.tsx
[error] 426-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
app/workspace/page.tsx (1)
206-206: Good fix: Using unique ID for React keysThe change to use
doc.idinstead ofdoc.namefor React keys addresses the previous review concern about potential key collisions.
| {/* 기업 배지 */} | ||
| {doc.type === 'manual' && ( | ||
| <Badge className="bg-blue-100 text-blue-800 whitespace-nowrap hover:bg-blue-100"> | ||
| {(doc as any).corporateName} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for corporate name access
Using (doc as any).corporateName bypasses TypeScript's type checking and could lead to runtime errors.
Consider defining proper types for your document objects:
interface CoverLetterDocument {
id: number;
name: string;
corporateName: string;
jobName: string;
date: string | null;
size: string;
type: 'manual';
}
interface ResumeDocument {
id: number;
name: string;
date: string | null;
size: string;
type: 'resume';
}
type DocumentType = CoverLetterDocument | ResumeDocument;Then use type guards instead of type assertions:
- {doc.type === 'manual' && (
- <Badge className="bg-blue-100 text-blue-800 whitespace-nowrap hover:bg-blue-100">
- {(doc as any).corporateName}
- </Badge>
- )}
+ {doc.type === 'manual' && 'corporateName' in doc && (
+ <Badge className="bg-blue-100 text-blue-800 whitespace-nowrap hover:bg-blue-100">
+ {doc.corporateName}
+ </Badge>
+ )}🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 221, avoid using (doc as any).corporateName to
improve type safety. Define proper TypeScript interfaces for your document
objects, such as CoverLetterDocument and ResumeDocument, and create a union type
DocumentType. Then implement type guards to safely access corporateName only
when doc is of type CoverLetterDocument, replacing the any cast with proper type
checking.
| try { | ||
| // 1. presigned url 발급 | ||
| const presigned = await getPresignedUploadUrl(file.name) | ||
| const { presignedUrl, fileUrl } = presigned.result! |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix optional chaining issue
The static analysis tool correctly identifies that accessing presigned.result! properties could be safer with optional chaining.
Apply this fix to improve null safety:
- const { presignedUrl, fileUrl } = presigned.result!
+ const { presignedUrl, fileUrl } = presigned.result ?? {}Also add validation to ensure the URLs exist before proceeding:
+ if (!presignedUrl || !fileUrl) {
+ throw new Error('Presigned URL generation failed')
+ }📝 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 { presignedUrl, fileUrl } = presigned.result! | |
| // replace non-null assertion with safe fallback | |
| - const { presignedUrl, fileUrl } = presigned.result! | |
| + const { presignedUrl, fileUrl } = presigned.result ?? {} | |
| // ensure URLs are present before proceeding | |
| + if (!presignedUrl || !fileUrl) { | |
| + throw new Error('Presigned URL generation failed') | |
| + } |
🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 418, replace the non-null assertion on
presigned.result with optional chaining to safely access presignedUrl and
fileUrl. Additionally, add validation checks to confirm that both URLs exist
before using them, preventing runtime errors from undefined values.
| // 2. S3 업로드 | ||
| await new Promise<void>((resolve, reject) => { | ||
| const xhr = new XMLHttpRequest() | ||
| xhr.open('PUT', presignedUrl!, true) |
There was a problem hiding this comment.
Fix potential null pointer access
The presignedUrl could be undefined, causing a runtime error.
Apply this fix:
- xhr.open('PUT', presignedUrl!, true)
+ xhr.open('PUT', presignedUrl, true)This change works together with the validation added in the previous suggestion.
📝 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.
| xhr.open('PUT', presignedUrl!, true) | |
| xhr.open('PUT', presignedUrl, true) |
🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 422, the code uses presignedUrl without
checking if it is undefined, which can cause a runtime error. Add a conditional
check before this line to ensure presignedUrl is defined; if it is undefined,
handle the error appropriately or return early to prevent calling xhr.open with
an invalid URL.
|
|
||
| return ( | ||
| <div className="space-y-4"> | ||
| <Input type="file" accept="application/pdf" onChange={handleFileChange} disabled={isUploading} /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add file validation for security
The file input should validate file types and sizes to prevent security issues and improve user experience.
Add file validation:
const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => {
if (e.target.files && e.target.files[0]) {
- setFile(e.target.files[0])
- setError(null)
+ const selectedFile = e.target.files[0]
+
+ // Validate file type
+ if (selectedFile.type !== 'application/pdf') {
+ setError('PDF 파일만 업로드 가능합니다.')
+ return
+ }
+
+ // Validate file size (e.g., 10MB limit)
+ if (selectedFile.size > 10 * 1024 * 1024) {
+ setError('파일 크기는 10MB 이하여야 합니다.')
+ return
+ }
+
+ setFile(selectedFile)
+ setError(null)
}
}📝 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.
| <Input type="file" accept="application/pdf" onChange={handleFileChange} disabled={isUploading} /> | |
| const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| if (e.target.files && e.target.files[0]) { | |
| const selectedFile = e.target.files[0] | |
| // Validate file type | |
| if (selectedFile.type !== 'application/pdf') { | |
| setError('PDF 파일만 업로드 가능합니다.') | |
| return | |
| } | |
| // Validate file size (e.g., 10MB limit) | |
| if (selectedFile.size > 10 * 1024 * 1024) { | |
| setError('파일 크기는 10MB 이하여야 합니다.') | |
| return | |
| } | |
| setFile(selectedFile) | |
| setError(null) | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 456, the file input lacks validation for file
type and size, which can lead to security risks and poor user experience.
Enhance the handleFileChange function to check that the selected file is a PDF
and its size is within acceptable limits before proceeding. If the file does not
meet these criteria, prevent further processing and provide appropriate user
feedback.
| <div className="grid md:grid-cols-2 lg:grid-cols-3 gap-4"> | ||
| {filteredDocuments.length > 0 ? ( | ||
| filteredDocuments.map((doc) => ( | ||
| {filteredDocuments && filteredDocuments?.length > 0 ? ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add loading states for better UX
The component should show loading indicators while data is being fetched.
Add loading state handling:
+ {(coverLetterListLoading || resumeListLoading) && (
+ <div className="col-span-full py-12 text-center">
+ <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-[#8FD694] mx-auto"></div>
+ <p className="mt-2 text-gray-500">문서를 불러오는 중...</p>
+ </div>
+ )}
{filteredDocuments && filteredDocuments?.length > 0 ? (📝 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.
| {filteredDocuments && filteredDocuments?.length > 0 ? ( | |
| {(coverLetterListLoading || resumeListLoading) && ( | |
| <div className="col-span-full py-12 text-center"> | |
| <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-[#8FD694] mx-auto"></div> | |
| <p className="mt-2 text-gray-500">문서를 불러오는 중...</p> | |
| </div> | |
| )} | |
| {filteredDocuments && filteredDocuments?.length > 0 ? ( |
🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 204, the component currently does not handle
loading states, which can lead to poor user experience during data fetching. Add
a loading state variable to track when data is being fetched and render a
loading indicator or message when this state is true. Ensure the loading
indicator is displayed before filteredDocuments are available, and only show the
documents list once loading is complete.
| setProgress(100) | ||
| onSuccess() | ||
| alert('이력서가 성공적으로 업로드되었습니다.') | ||
| } catch (e: any) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in upload function
The error handling could be more specific and user-friendly.
Enhance error handling:
} catch (e: any) {
- setError(e.message || '업로드 실패')
- alert('업로드에 실패했습니다. 다시 시도해주세요.')
+ const errorMessage = e.message || '업로드 실패'
+ setError(errorMessage)
+
+ // More specific error messages
+ if (errorMessage.includes('S3')) {
+ alert('파일 업로드에 실패했습니다. 네트워크 연결을 확인해주세요.')
+ } else if (errorMessage.includes('Presigned')) {
+ alert('업로드 준비에 실패했습니다. 잠시 후 다시 시도해주세요.')
+ } else {
+ alert('업로드에 실패했습니다. 다시 시도해주세요.')
+ }
} finally {📝 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.
| } catch (e: any) { | |
| } catch (e: any) { | |
| const errorMessage = e.message || '업로드 실패' | |
| setError(errorMessage) | |
| // More specific error messages | |
| if (errorMessage.includes('S3')) { | |
| alert('파일 업로드에 실패했습니다. 네트워크 연결을 확인해주세요.') | |
| } else if (errorMessage.includes('Presigned')) { | |
| alert('업로드 준비에 실패했습니다. 잠시 후 다시 시도해주세요.') | |
| } else { | |
| alert('업로드에 실패했습니다. 다시 시도해주세요.') | |
| } | |
| } finally { |
🤖 Prompt for AI Agents
In app/workspace/page.tsx at line 446, the catch block currently catches errors
as a generic any type, which limits specific error handling and user-friendly
messages. Refine the error handling by narrowing the error type if possible, and
provide more descriptive, user-friendly error messages or handling logic based
on the error type or properties. This may include checking error codes or
messages and responding accordingly to improve clarity and user experience.
| const { data: resumeList, isLoading: resumeListLoading } = useQuery({ | ||
| queryKey: ["resumeList", memberId], | ||
| queryFn: () => getResumeList(memberId!), | ||
| enabled: !!memberId, | ||
| select: (data) => { | ||
| const resumes = data.result?.resumes | ||
| return resumes?.map((resume) => ({ | ||
| id: resume.resumeId, | ||
| name: resume.fileName, | ||
| date: null, // 상세조회에서 가져올 수 있음 | ||
| size: resume.fileSize ? `${(resume.fileSize / 1024).toFixed(1)}KB` : '-', | ||
| type: "resume", | ||
| })) | ||
| }, | ||
| { | ||
| id: 2, | ||
| name: "네이버 지원 자소서.docx", | ||
| date: "2023-04-18", | ||
| size: "450KB", | ||
| type: "file", | ||
| category: "coverLetter", | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply same error handling pattern to resume list
The resume list query also needs proper error handling for consistency.
Add similar error handling:
const { data: resumeList, isLoading: resumeListLoading, error: resumeError } = useQuery({
queryKey: ["resumeList", memberId],
queryFn: () => getResumeList(memberId!),
enabled: !!memberId,
+ retry: 2,
+ staleTime: 5 * 60 * 1000,
select: (data) => {
const resumes = data.result?.resumes
return resumes?.map((resume) => ({
id: resume.resumeId,
name: resume.fileName,
date: resume.createdAt ?? '',
size: resume.fileSize ? `${(resume.fileSize / 1024).toFixed(1)}KB` : '-',
type: "resume",
}))
},
})
+ if (resumeError) {
+ console.error('Failed to load resumes:', resumeError)
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/workspace/page.tsx around lines 67 to 81, the useQuery call for fetching
the resume list lacks error handling. Add an onError callback to the useQuery
options that handles errors consistently, such as logging the error or showing a
user-friendly message, following the same pattern used in other queries in this
file.
| const { data: coverLetterList, isLoading: coverLetterListLoading } = useQuery({ | ||
| queryKey: ["coverLetterList", memberId], | ||
| queryFn: () => findMyCoverletter(memberId!), | ||
| enabled: !!memberId, | ||
| select: (data) => { | ||
| const coverLetters = data.result?.coverletters; | ||
| return coverLetters?.map((coverLetter) => ({ | ||
| id: coverLetter.coverletterId, | ||
| name: `${coverLetter.corporateName}-${coverLetter.jobName}`, | ||
| corporateName: coverLetter.corporateName, | ||
| jobName: coverLetter.jobName, | ||
| date: coverLetter.createdAt, | ||
| size: "0KB", | ||
| type: "manual", | ||
| })) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider error handling for API failures
The cover letter list query doesn't handle API errors, which could lead to poor user experience.
Add error handling to the query:
const { data: coverLetterList, isLoading: coverLetterListLoading, error: coverLetterError } = useQuery({
queryKey: ["coverLetterList", memberId],
queryFn: () => findMyCoverletter(memberId!),
enabled: !!memberId,
+ retry: 2,
+ staleTime: 5 * 60 * 1000, // 5 minutes
select: (data) => {
const coverLetters = data.result?.coverletters;
return coverLetters?.map((coverLetter) => ({
id: coverLetter.coverletterId,
name: `${coverLetter.corporateName}-${coverLetter.jobName}`,
corporateName: coverLetter.corporateName,
jobName: coverLetter.jobName,
date: coverLetter.createdAt,
size: "0KB",
type: "manual",
}))
}
})
+ if (coverLetterError) {
+ console.error('Failed to load cover letters:', coverLetterError)
+ }📝 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 { data: coverLetterList, isLoading: coverLetterListLoading } = useQuery({ | |
| queryKey: ["coverLetterList", memberId], | |
| queryFn: () => findMyCoverletter(memberId!), | |
| enabled: !!memberId, | |
| select: (data) => { | |
| const coverLetters = data.result?.coverletters; | |
| return coverLetters?.map((coverLetter) => ({ | |
| id: coverLetter.coverletterId, | |
| name: `${coverLetter.corporateName}-${coverLetter.jobName}`, | |
| corporateName: coverLetter.corporateName, | |
| jobName: coverLetter.jobName, | |
| date: coverLetter.createdAt, | |
| size: "0KB", | |
| type: "manual", | |
| })) | |
| } | |
| }) | |
| const { data: coverLetterList, isLoading: coverLetterListLoading, error: coverLetterError } = useQuery({ | |
| queryKey: ["coverLetterList", memberId], | |
| queryFn: () => findMyCoverletter(memberId!), | |
| enabled: !!memberId, | |
| retry: 2, | |
| staleTime: 5 * 60 * 1000, // 5 minutes | |
| select: (data) => { | |
| const coverLetters = data.result?.coverletters; | |
| return coverLetters?.map((coverLetter) => ({ | |
| id: coverLetter.coverletterId, | |
| name: `${coverLetter.corporateName}-${coverLetter.jobName}`, | |
| corporateName: coverLetter.corporateName, | |
| jobName: coverLetter.jobName, | |
| date: coverLetter.createdAt, | |
| size: "0KB", | |
| type: "manual", | |
| })); | |
| } | |
| }); | |
| if (coverLetterError) { | |
| console.error('Failed to load cover letters:', coverLetterError); | |
| } |
🤖 Prompt for AI Agents
In app/workspace/page.tsx around lines 48 to 64, the useQuery call for fetching
the cover letter list lacks error handling, which can cause issues if the API
call fails. Add an onError callback to the useQuery options to handle API errors
gracefully, such as logging the error or showing a user-friendly message. This
will improve the user experience by managing failures properly.
📌 작업 내용
📎 관련 이슈
--> CCI-37
Summary by CodeRabbit
New Features
Bug Fixes
Chores