#115 feat: 대량 데이터 업로드 및 일괄 처리 기능 도입#121
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthrough이 PR은 과제 폼 관리를 훅 기반으로 중앙화하고, 테스트케이스 대량 업로드를 추가하며, 학생 관리 UI를 제어 컴포넌트로 리팩토링합니다. 강의 생성/수정 시 학생 등록 실패를 추적하고, Zod 스키마에 유연성을 추가합니다. ChangesAssignment and Testcase Management
Student Enrollment and Management
Course Creation and Enrollment Integration
Shared UI Component Enhancements
Sequence Diagram(s)sequenceDiagram
participant User
participant AssignmentForm as Assignment Form Page
participant useAssignmentForm as useAssignmentForm Hook
participant TestcaseField as Testcase Field
participant uploadAPI as uploadTestcasesBulk API
User->>AssignmentForm: 과제 작성/수정
AssignmentForm->>useAssignmentForm: useAssignmentForm(assignmentId)
useAssignmentForm-->>AssignmentForm: 폼 핸들러, watch, setValue
User->>TestcaseField: 테스트케이스 JSON 파일 선택
TestcaseField->>TestcaseField: 파일 파싱 및 정규화
TestcaseField->>uploadAPI: 파일 대량 업로드
uploadAPI-->>TestcaseField: 업로드 성공
TestcaseField-->>AssignmentForm: 테스트케이스 상태 업데이트
User->>AssignmentForm: 제출
AssignmentForm->>useAssignmentForm: handleSubmit (생성 또는 수정)
useAssignmentForm-->>AssignmentForm: 성공 토스트, 페이지 이동
sequenceDiagram
participant User
participant StudentMgmt as Student Management Page
participant useStudentMgmt as useStudentManagement Hook
participant StudentTable as StudentTable
participant deleteAPI as deleteEnrollmentsBulk API
User->>StudentMgmt: 학생 관리 페이지 접근
StudentMgmt->>useStudentMgmt: useStudentManagement(courseId)
useStudentMgmt-->>StudentMgmt: 학생 목록, 선택 상태, 핸들러
User->>StudentTable: 학생 선택 (체크박스)
StudentTable->>StudentMgmt: onSelectionChange(selectedIds)
User->>StudentMgmt: 삭제 버튼 클릭
StudentMgmt->>useStudentMgmt: handleDelete()
useStudentMgmt->>deleteAPI: 선택된 학생 ID 목록 전송
deleteAPI-->>useStudentMgmt: 삭제 완료
useStudentMgmt-->>StudentMgmt: 쿼리 무효화, 선택 초기화, 토스트
sequenceDiagram
participant User
participant CourseForm as Create/Edit Course Hook
participant courseMutations as courseMutations API
participant enrollmentAPI as addEnrollment API
User->>CourseForm: 강의 생성/수정 제출 (학생 ID 포함)
CourseForm->>courseMutations: createCourse/updateCourse (students 배열)
courseMutations->>courseMutations: 강의 생성/수정
courseMutations->>courseMutations: 기존 등록 확인
courseMutations->>courseMutations: 누락된 학생 ID 계산
courseMutations->>enrollmentAPI: 누락된 학생 대량 등록 (병렬)
enrollmentAPI-->>courseMutations: 등록 결과 (성공/실패)
courseMutations-->>CourseForm: { course, failedIds }
CourseForm->>CourseForm: failedIds 검사
alt failedIds 있음
CourseForm->>User: 등록 실패 학생 ID 알림
else 모두 성공
CourseForm->>User: 성공 토스트
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 이 PR은 여러 도메인(과제, 학생, 강의)에 걸친 광범위한 리팩토링으로, 기존 컴포넌트의 상태 관리 패턴 변화(제어 컴포넌트), 새로운 훅 통합, API 및 스키마 업데이트를 동시에 처리합니다. 특히 Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (12)
src/entities/assignment/model/schemas.ts (1)
36-52: ⚡ Quick win
.passthrough()의 의도를 명확히 문서화하세요.
.passthrough()를 사용했으므로 API가 반환할 수 있는 추가 필드에 대해 설명하는 주석을 추가하는 것이 좋습니다. 현재 구조에서TAssignmentDetail타입은 명시된 필드들만 포함하므로 타입 안전성이 손상되지 않습니다. 다만.passthrough()를 사용하는 이유(예: 향후 API 응답에 새로운 필드가 추가될 가능성 대비)를 코드에 명시하면, 다른 개발자들이 의도를 쉽게 이해할 수 있습니다.export const assignmentDetailSchema = z .object({ // ... fields }) // API 응답에 예상치 못한 필드가 있을 수 있으므로 통과 허용 .passthrough();또한 Zod 문서의
.passthrough()설명을 참고하면 이러한 접근 방식이 API 호환성을 유지하면서도 타입 검증을 보장하는 좋은 패턴임을 알 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entities/assignment/model/schemas.ts` around lines 36 - 52, The assignmentDetailSchema currently calls .passthrough(); add a short inline comment immediately above or beside the .passthrough() call (in the assignmentDetailSchema declaration) explaining the intent — e.g., that unexpected/additional fields from the API are allowed to preserve forward compatibility and avoid breaking changes — and reference Zod's .passthrough() behavior; keep the comment brief and mention assignmentDetailSchema so future readers know why extra properties are permitted.src/entities/student/api/studentApi.ts (1)
36-36: ⚡ Quick win불필요한 Content-Type 헤더 설정입니다.
FormData를 사용할 때 Axios는 자동으로Content-Type: multipart/form-data를 설정하고 적절한 boundary를 추가합니다. 수동으로 설정하면 오히려 boundary가 누락되어 요청이 실패할 수 있습니다.♻️ 불필요한 헤더 제거
const res = await privateAxios.post( ENDPOINTS.ENROLLMENTS.BULK(courseId), - formData, - { - headers: {'Content-Type': 'multipart/form-data'}, - } + formData );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entities/student/api/studentApi.ts` at line 36, Remove the manual Content-Type header so Axios can set multipart boundaries automatically: in the function that sends FormData (look for the request using headers: {'Content-Type': 'multipart/form-data'} in studentApi.ts), delete that header key or stop passing a headers object when body is a FormData instance to avoid missing boundary errors; ensure the code still passes the FormData as the request body and let Axios set Content-Type.src/pages/admin/student/model/useStudentManagement.ts (1)
43-50: ⚡ Quick win네이티브 대화상자 대신 UI 컴포넌트를 사용하세요.
alert()와confirm()은 다음 문제가 있습니다:
- 브라우저를 차단하여 UX가 나쁨
- 스타일링 불가능
- 접근성 지원 부족
- 테스트 어려움
프로젝트에 이미 토스트 시스템(
useToastStore)이 있으니, 확인 다이얼로그도 추가하는 것을 권장합니다.♻️ 개선 제안
// 임시 방안 (간단한 구현이 필요하다면) const handleDelete = () => { if (selectedIds.size === 0) { showToast('삭제할 학생을 선택해주세요.', 'warning'); return; } // confirm은 당장 대체하기 어렵다면 남겨둘 수 있지만, // 추후 모달 컴포넌트로 교체 권장 if (confirm(`정말 선택한 ${selectedIds.size}명의 학생을 삭제하시겠습니까?`)) { deleteEnrollments(Array.from(selectedIds)); } };장기적 해결책:
@/shared/ui/ConfirmDialog같은 재사용 가능한 컴포넌트 도입을 고려하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/student/model/useStudentManagement.ts` around lines 43 - 50, Replace native alert/confirm in the deletion flow with the app's UI components: when selectedIds.size === 0, call the toast helper from useToastStore (e.g., showToast or addToast) instead of alert; for confirmation, open the shared ConfirmDialog/modal component (suggested component name: ConfirmDialog or a hook like useConfirm) and only call deleteEnrollments(Array.from(selectedIds)) after the dialog resolves with user confirmation. Update the handler (e.g., handleDelete or the function wrapping deleteEnrollments) to trigger the toast/dialog and handle the confirmed callback, removing alert() and confirm() usage.src/pages/admin/student/StudentManagementPage.tsx (1)
58-62: 💤 Low value비활성화된 확인 버튼이 혼란을 줄 수 있습니다.
현재 구조는 다음과 같은 UX 문제가 있습니다:
- "확인" 버튼이 항상 비활성화되어 있음
onConfirm이 빈 함수- 실제로는 "선택 삭제" 버튼만 동작
AssignmentFormLayout이 두 개의 액션 버튼을 강제하는 것 같은데, 이 페이지에서는 삭제 버튼만 필요합니다.💡 개선 옵션
옵션 1: 레이아웃 컴포넌트에 단일 액션 모드 추가
<AssignmentFormLayout // ... singleAction={{ label: '선택 삭제', onClick: handleDelete, variant: 'danger' }} />옵션 2: 확인 버튼을 "새로고침"이나 "닫기" 같은 의미있는 액션으로 변경
현재 구현도 동작하지만, 사용자가 비활성화된 버튼을 보고 혼란스러워할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/student/StudentManagementPage.tsx` around lines 58 - 62, The current footer renders two action buttons with confirmDisabled={true} and a no-op onConfirm, which confuses users because only the cancel button (onCancel={handleDelete}) works; update the usage of AssignmentFormLayout to present a single actionable delete button by either using a single-action API on AssignmentFormLayout (e.g., pass singleAction={{ label: '선택 삭제', onClick: handleDelete, variant: 'danger' }}) or, if that prop is unavailable, set onConfirm={handleDelete}, confirmLabel='선택 삭제', remove/omit the cancel action or rename it to a secondary non-disabled action so the UI shows only meaningful enabled controls; reference AssignmentFormLayout, handleDelete, onConfirm and onCancel when making the change.src/pages/admin/assignments/ui/TestcaseField.tsx (3)
54-57: 💤 Low valueisPublic 로직을 단순화할 수 있습니다.
Lines 54-57의 삼항 연산자는 조금 복잡합니다. Boolean 강제 변환과 nullish coalescing을 사용하면 더 명확합니다.
✨ 가독성 개선
return { testcase: String(input), answer: String(output), - isPublic: - typeof testcase.isPublic === 'boolean' - ? testcase.isPublic - : testcase.public === true, + isPublic: testcase.isPublic ?? testcase.public ?? false, };
??연산자는undefined/null일 때만 다음 값을 사용하므로,false값도 올바르게 처리됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/ui/TestcaseField.tsx` around lines 54 - 57, The isPublic ternary in TestcaseField (using testcase.isPublic ? testcase.isPublic : testcase.public === true) can be simplified: replace it with a nullish-coalescing approach and explicit boolean coercion so false is preserved, e.g. compute isPublic as Boolean(testcase.isPublic ?? testcase.public); update the object/prop that sets isPublic accordingly to use this expression referencing the testcase variable.
99-119: ⚡ Quick win파일 업로드 전에 유효성 검사를 추가하세요.
파일 크기와 타입을 검증하지 않아 대용량 파일이나 잘못된 형식의 파일이 브라우저를 느리게 만들 수 있습니다. 파일 처리 전에 기본적인 검증을 추가하는 것이 좋습니다.
🛡️ 파일 검증 추가
const handleTestcaseFileChange = async (file: File | null) => { if (!file) return; + // 파일 크기 검증 (예: 5MB 제한) + const MAX_FILE_SIZE = 5 * 1024 * 1024; + if (file.size > MAX_FILE_SIZE) { + showToast('파일 크기는 5MB를 초과할 수 없습니다.'); + return; + } + + // 파일 타입 검증 + if (!file.type.includes('json') && !file.name.endsWith('.json')) { + showToast('JSON 파일만 업로드할 수 있습니다.'); + return; + } try { const parsed = JSON.parse(await file.text());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/ui/TestcaseField.tsx` around lines 99 - 119, Add client-side validation in handleTestcaseFileChange: before reading file.text(), check file.type (e.g., 'application/json' or .json extension) and file.size against a max (e.g., 5MB) and if invalid call showToast or alert and return; only proceed to JSON.parse, normalizeTestcaseJson, onChange and uploadTestcasesBulk after the checks pass; keep uploadTestcasesBulk({assignmentId, file}) behavior unchanged but ensure it is not invoked for invalid files.
112-118: ⚡ Quick win에러 표시를 Toast로 통일하세요.
Line 117에서 JSON 파싱 에러에
alert()를 사용하는데, 성공 시에는showToast()를 사용합니다 (Line 81, 110). 일관된 UX를 위해 모든 알림을 Toast로 통일하세요.🎨 개선 제안
} catch (error) { const message = error instanceof Error ? error.message : '테스트 케이스 JSON을 불러오지 못했습니다.'; - alert(message); + showToast(message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/ui/TestcaseField.tsx` around lines 112 - 118, In TestcaseField.tsx replace the alert(...) in the catch block with the same showToast used elsewhere so errors use the unified Toast UX: keep the existing message selection logic (error instanceof Error ? error.message : '테스트 케이스 JSON을 불러오지 못했습니다.'), and call showToast with that message and an error/negative status (matching how showToast is used on success at the top of the file) inside the catch in the TestcaseField component.src/pages/admin/assignments/model/useAssignmentForm.ts (3)
23-23: 💤 Low value기본 공개 설정이 TestcaseField와 다릅니다.
Line 23에서 기본 testcase의
isPublic: true로 설정되어 있는데,TestcaseField.tsx의 Line 93에서는 마지막 테스트케이스 삭제 시isPublic: false로 생성합니다. 의도된 차이인지 확인하고, 일관성이 필요하다면 통일하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/model/useAssignmentForm.ts` at line 23, The default `isPublic` for new testcases is inconsistent: `useAssignmentForm` initializes testcases with `isPublic: true` while `TestcaseField` creates a new testcase with `isPublic: false` when the last testcase is deleted; pick the intended default and make both places consistent by updating either the initial value in useAssignmentForm (testcases: [{ testcase: '', answer: '', isPublic: ... }]) or the creation logic in TestcaseField (the code path that appends a new testcase after deletion) so both use the same boolean default for `isPublic`.
57-57: ⚡ Quick win에러 처리를 Toast로 통일하세요.
Lines 57, 72에서 에러 발생 시
alert()를 사용하는데, 성공 시에는showToast()를 사용합니다 (Lines 54, 69). 일관된 사용자 경험을 위해 에러도 Toast로 표시하거나,handleApiError유틸을 사용하는 것이 좋습니다.🎨 개선 제안
onError: () => { - alert('문제 등록에 실패했습니다.') + showToast('문제 등록에 실패했습니다.'); }또는 TestcaseField.tsx (line 84)처럼
handleApiError를 사용:+ import {handleApiError} from '@/shared/lib/handleApiError'; - onError: () => alert('문제 등록에 실패했습니다.') + onError: (error) => handleApiError(error, '문제 등록에 실패했습니다.')Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/model/useAssignmentForm.ts` at line 57, The onError handlers in useAssignmentForm.ts (the onError callbacks at the current onError entries around the onError at line ~57 and the other at ~72) use alert('문제 등록에 실패했습니다.') which is inconsistent with successful-path showToast usage; replace those alert() calls with the same toast mechanism (e.g., call showToast with an error type and a helpful message) or delegate to the existing handleApiError utility (accepting the error param) so errors are shown via Toast consistently; update the onError signatures to accept the error argument and pass it to showToast or handleApiError accordingly.
7-7: ⚡ Quick win타입 전용 임포트를 사용하세요.
Line 7에서
TAssignmentForm은 타입으로만 사용되므로import type을 사용하는 것이 좋습니다. 이는 번들 크기를 줄이고 빌드 최적화에 도움이 됩니다.🔧 수정 제안
- import type {TAssignmentForm} from '@/entities/assignment/model/schemas'; + import type {TAssignmentForm} from '@/entities/assignment/model/schemas';실제로 이미
type키워드를 사용하고 있네요. 확인 부탁드립니다!As per coding guidelines: type-only import 적극 사용
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/model/useAssignmentForm.ts` at line 7, The import for TAssignmentForm should be a type-only import: ensure the declaration in useAssignmentForm.ts uses "import type { TAssignmentForm }" (and that TAssignmentForm is not used at runtime anywhere in functions like useAssignmentForm or related exports); if any runtime reference exists, convert it to a type-only usage or refactor so TAssignmentForm remains purely a compile-time type to keep the import type-only.src/entities/assignment/api/assignmentApi.ts (1)
102-102: ⚡ Quick win응답 스키마를 구체적으로 정의하세요.
Line 102에서
z.unknown()을 사용하면 응답 데이터의 유효성을 검증하지 않아 런타임 오류가 발생할 수 있습니다. 다른 API 함수들처럼 명확한 스키마를 정의하는 것이 좋습니다.💡 개선 제안
src/entities/assignment/model/schemas.ts에 응답 스키마를 추가하세요:export const testcaseBulkUploadResponseSchema = z.object({ count: z.number(), testcases: z.array(testcaseSchema), // 또는 적절한 스키마 });그리고 이곳에서 사용:
- const parsed = apiResponseSchema(z.unknown()).parse(response.data); + const parsed = apiResponseSchema(testcaseBulkUploadResponseSchema).parse(response.data);As per coding guidelines: API 응답 타입은 XXXResponse 네이밍을 사용하고, 명확한 타입 정의가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/entities/assignment/api/assignmentApi.ts` at line 102, The call using apiResponseSchema(z.unknown()) doesn't validate the API response; replace it with a concrete response schema (e.g., testcaseBulkUploadResponseSchema) and use that to parse response.data; add/export testcaseBulkUploadResponseSchema (named per XXXResponse guideline) in src/entities/assignment/model/schemas.ts that includes count: z.number() and testcases: z.array(testcaseSchema) (or appropriate shape) and then import and use that schema in assignmentApi.ts in place of apiResponseSchema(z.unknown()) so parsed has the correct typed shape.src/pages/admin/assignments/AssignmentFormPage.tsx (1)
7-19: ⚡ Quick win수정 모드에서 로딩 상태를 표시하세요.
수정 모드에서 기존 데이터를 불러오는 동안 로딩 상태가 표시되지 않습니다.
useAssignmentForm훅에서isLoading상태를 반환하고, 로딩 중에는 스켈레톤이나 스피너를 표시하는 것이 좋은 UX입니다.💡 개선 제안
useAssignmentForm.ts에서 로딩 상태 노출:const {data, isLoading} = useQuery({ ...assignmentQueries.getAssignment(assignmentId!), enabled: isEditMode, }); return { register, handleSubmit: handleSubmit(onSubmit), watch, setValue, isEditMode, isPending: isCreating || isUpdating, + isLoading, navigate, };그리고 이 페이지에서:
const { register, handleSubmit, watch, setValue, isEditMode, + isLoading, navigate, } = useAssignmentForm(assignmentId); + if (isLoading) { + return <div>로딩 중...</div>; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/admin/assignments/AssignmentFormPage.tsx` around lines 7 - 19, The page doesn't show a loading state when editing; update AssignmentFormPage to read the isLoading flag returned from useAssignmentForm and render a skeleton/spinner while isEditMode && isLoading; specifically, update the destructure to include isLoading from useAssignmentForm and add a conditional render early in AssignmentFormPage that returns a loading UI (spinner/skeleton) when isEditMode && isLoading, otherwise continue rendering the form; ensure the hook useAssignmentForm actually exposes isLoading (add it there if missing) so the page can rely on that flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/entities/assignment/api/assignmentApi.ts`:
- Around line 95-101: The request is manually setting the Content-Type header
when posting FormData which prevents the browser from adding the required
multipart boundary; in the privateAxios.post call that sends formData to
ENDPOINTS.TESTCASES.BULK(assignmentId) remove the explicit headers object (or at
least the 'Content-Type' entry) so axios/browsers can auto-set Content-Type with
the correct boundary when calling privateAxios.post(..., formData).
In `@src/entities/course/api/courseMutations.ts`:
- Around line 23-33: The enrollment-check step can reject the whole mutation if
getEnrollments fails and misdetect missing students due to ID type mismatches;
change the logic around the getEnrollments call (used when computing
enrolledStudentIds and missingStudents) to use a non-throwing pattern (e.g.,
Promise.allSettled or try/catch) so that a failed enrollment fetch is
logged/handled but does not cause the outer course create/update to fail, and
normalize IDs before comparing (coerce both
enrollments.response.students[].studentId and incoming students[].studentId to
the same type/string form) so Set.has comparisons are reliable; ensure
enrolledStudentIds, missingStudents, and any downstream branches handle the case
where enrollment data is unavailable.
In `@src/entities/student/api/studentApi.ts`:
- Line 25: The mutation functions addEnrollment, addEnrollmentsBulk, and
deleteEnrollment currently use apiResponseSchema(z.unknown()) which loses type
safety—define and export a specific response Zod schema (e.g.,
enrollmentMutationResponseSchema) in src/entities/student/model/schemas.ts that
matches the backend mutation response (success:boolean, optional message, and
any payload fields), then replace z.unknown() with that schema in those three
functions so runtime validation and typing match
getEnrollments/getEnrollmentById; also remove the explicit headers:
{'Content-Type': 'multipart/form-data'} from addEnrollmentsBulk since axios
handles FormData automatically.
In `@src/entities/student/api/studentMutations.ts`:
- Around line 17-21: The bulk delete in deleteEnrollmentsBulk uses Promise.all
which fails the whole operation if any deleteEnrollment(courseId, id) rejects;
change mutationFn to handle partial failures by using Promise.allSettled over
memberIds.map(id => deleteEnrollment(courseId, id)), collect the indices/ids
whose results are rejected, and return an object like { total: memberIds.length,
failed: number[] } (or otherwise surface the failed ids) so callers can present
partial-success feedback; alternatively implement a single backend bulk-delete
endpoint and call that from deleteEnrollmentsBulk for transactional behavior.
In `@src/pages/admin/assignments/AssignmentFormPage.tsx`:
- Line 31: Add client-side validation to the title and description fields by
passing validation rules to react-hook-form's register (e.g. required messages,
optional minLength) when calling {...register('title', {...})} and
{...register('description', {...})}; ensure you destructure formState: { errors
} from useAssignmentForm(assignmentId) and pass errors.title?.message and
errors.description?.message into the corresponding LabeledInput error props so
validation messages display to the user (update the register calls and
LabeledInput usages accordingly).
In `@src/pages/admin/assignments/ui/TestcaseField.tsx`:
- Around line 124-128: The list uses index-based keys (key={idx}) in
TestcaseField.tsx which causes React reuse bugs; update the data shape
(TestcaseValue) to include a stable id and use that for the key in the
TestcaseRow (e.g., testcase.id), and when creating new testcases assign a unique
id (UUID or timestamp+counter) so TestcaseRow, addTestcase handler (or wherever
new items are pushed) and any mapping that renders TestcaseRow are changed to
rely on that stable id; alternatively, if you cannot add ids, use a stable
composite key derived from immutable testcase properties (e.g.,
`${testcase.input}-${testcase.output}`) instead of idx.
In `@src/pages/admin/student/model/useStudentManagement.ts`:
- Around line 22-28: The current useQuery call is loading all students with
pageSize: 1000 and then client-slicing (see useQuery and the client-side slice
around currentPage), causing mismatch between server 0-based paging and client
1-based currentPage and poor scalability; change the query to use the
component's pageSize and map the client page to the server page by passing page:
currentPage - 1 (instead of hardcoded 0) and pageSize: pageSize (instead of
1000), remove the client-side slicing logic (the slice that trims results by
currentPage), and rely on the server response's items + total to render pages
(or add/handle a total field on the backend if missing) so pagination is
entirely server-driven (references: useQuery calling
studentQueries.getEnrollments and the client slice around currentPage).
In `@src/pages/admin/student/StudentManagementPage.tsx`:
- Around line 11-12: The code reads the courseId URL param with useParams and
converts it to numericCourseId via Number(courseId) without validation, which
can produce NaN; add explicit validation after const { courseId } =
useParams(...) to check for undefined or non-numeric values (e.g., test courseId
presence and that Number(courseId) is finite), and handle invalid params by
returning early/redirecting/rendering an error state before making API calls;
update any callers that rely on numericCourseId (the numericCourseId variable)
so they only run when the validated numeric id is available.
In `@src/shared/ui/FileUpload.tsx`:
- Around line 90-92: The parent onKeyDown handler (the one that triggers
inputRef.current?.click()) is firing for keyboard events from nested controls;
update those onKeyDown handlers to ignore events not originating on the
container and to prevent Space default: at the top of the handler check if
e.target !== e.currentTarget and return if true, then when handling keys use if
(e.key === 'Enter' || e.key === ' ') { e.preventDefault();
inputRef.current?.click(); } — apply the same change to both onKeyDown handlers
referenced (the container onKeyDowns that call inputRef.current?.click()) so
Enter/Space on the delete button no longer bubbles to the parent and Space
doesn't scroll the page.
---
Nitpick comments:
In `@src/entities/assignment/api/assignmentApi.ts`:
- Line 102: The call using apiResponseSchema(z.unknown()) doesn't validate the
API response; replace it with a concrete response schema (e.g.,
testcaseBulkUploadResponseSchema) and use that to parse response.data;
add/export testcaseBulkUploadResponseSchema (named per XXXResponse guideline) in
src/entities/assignment/model/schemas.ts that includes count: z.number() and
testcases: z.array(testcaseSchema) (or appropriate shape) and then import and
use that schema in assignmentApi.ts in place of apiResponseSchema(z.unknown())
so parsed has the correct typed shape.
In `@src/entities/assignment/model/schemas.ts`:
- Around line 36-52: The assignmentDetailSchema currently calls .passthrough();
add a short inline comment immediately above or beside the .passthrough() call
(in the assignmentDetailSchema declaration) explaining the intent — e.g., that
unexpected/additional fields from the API are allowed to preserve forward
compatibility and avoid breaking changes — and reference Zod's .passthrough()
behavior; keep the comment brief and mention assignmentDetailSchema so future
readers know why extra properties are permitted.
In `@src/entities/student/api/studentApi.ts`:
- Line 36: Remove the manual Content-Type header so Axios can set multipart
boundaries automatically: in the function that sends FormData (look for the
request using headers: {'Content-Type': 'multipart/form-data'} in
studentApi.ts), delete that header key or stop passing a headers object when
body is a FormData instance to avoid missing boundary errors; ensure the code
still passes the FormData as the request body and let Axios set Content-Type.
In `@src/pages/admin/assignments/AssignmentFormPage.tsx`:
- Around line 7-19: The page doesn't show a loading state when editing; update
AssignmentFormPage to read the isLoading flag returned from useAssignmentForm
and render a skeleton/spinner while isEditMode && isLoading; specifically,
update the destructure to include isLoading from useAssignmentForm and add a
conditional render early in AssignmentFormPage that returns a loading UI
(spinner/skeleton) when isEditMode && isLoading, otherwise continue rendering
the form; ensure the hook useAssignmentForm actually exposes isLoading (add it
there if missing) so the page can rely on that flag.
In `@src/pages/admin/assignments/model/useAssignmentForm.ts`:
- Line 23: The default `isPublic` for new testcases is inconsistent:
`useAssignmentForm` initializes testcases with `isPublic: true` while
`TestcaseField` creates a new testcase with `isPublic: false` when the last
testcase is deleted; pick the intended default and make both places consistent
by updating either the initial value in useAssignmentForm (testcases: [{
testcase: '', answer: '', isPublic: ... }]) or the creation logic in
TestcaseField (the code path that appends a new testcase after deletion) so both
use the same boolean default for `isPublic`.
- Line 57: The onError handlers in useAssignmentForm.ts (the onError callbacks
at the current onError entries around the onError at line ~57 and the other at
~72) use alert('문제 등록에 실패했습니다.') which is inconsistent with successful-path
showToast usage; replace those alert() calls with the same toast mechanism
(e.g., call showToast with an error type and a helpful message) or delegate to
the existing handleApiError utility (accepting the error param) so errors are
shown via Toast consistently; update the onError signatures to accept the error
argument and pass it to showToast or handleApiError accordingly.
- Line 7: The import for TAssignmentForm should be a type-only import: ensure
the declaration in useAssignmentForm.ts uses "import type { TAssignmentForm }"
(and that TAssignmentForm is not used at runtime anywhere in functions like
useAssignmentForm or related exports); if any runtime reference exists, convert
it to a type-only usage or refactor so TAssignmentForm remains purely a
compile-time type to keep the import type-only.
In `@src/pages/admin/assignments/ui/TestcaseField.tsx`:
- Around line 54-57: The isPublic ternary in TestcaseField (using
testcase.isPublic ? testcase.isPublic : testcase.public === true) can be
simplified: replace it with a nullish-coalescing approach and explicit boolean
coercion so false is preserved, e.g. compute isPublic as
Boolean(testcase.isPublic ?? testcase.public); update the object/prop that sets
isPublic accordingly to use this expression referencing the testcase variable.
- Around line 99-119: Add client-side validation in handleTestcaseFileChange:
before reading file.text(), check file.type (e.g., 'application/json' or .json
extension) and file.size against a max (e.g., 5MB) and if invalid call showToast
or alert and return; only proceed to JSON.parse, normalizeTestcaseJson, onChange
and uploadTestcasesBulk after the checks pass; keep
uploadTestcasesBulk({assignmentId, file}) behavior unchanged but ensure it is
not invoked for invalid files.
- Around line 112-118: In TestcaseField.tsx replace the alert(...) in the catch
block with the same showToast used elsewhere so errors use the unified Toast UX:
keep the existing message selection logic (error instanceof Error ?
error.message : '테스트 케이스 JSON을 불러오지 못했습니다.'), and call showToast with that
message and an error/negative status (matching how showToast is used on success
at the top of the file) inside the catch in the TestcaseField component.
In `@src/pages/admin/student/model/useStudentManagement.ts`:
- Around line 43-50: Replace native alert/confirm in the deletion flow with the
app's UI components: when selectedIds.size === 0, call the toast helper from
useToastStore (e.g., showToast or addToast) instead of alert; for confirmation,
open the shared ConfirmDialog/modal component (suggested component name:
ConfirmDialog or a hook like useConfirm) and only call
deleteEnrollments(Array.from(selectedIds)) after the dialog resolves with user
confirmation. Update the handler (e.g., handleDelete or the function wrapping
deleteEnrollments) to trigger the toast/dialog and handle the confirmed
callback, removing alert() and confirm() usage.
In `@src/pages/admin/student/StudentManagementPage.tsx`:
- Around line 58-62: The current footer renders two action buttons with
confirmDisabled={true} and a no-op onConfirm, which confuses users because only
the cancel button (onCancel={handleDelete}) works; update the usage of
AssignmentFormLayout to present a single actionable delete button by either
using a single-action API on AssignmentFormLayout (e.g., pass singleAction={{
label: '선택 삭제', onClick: handleDelete, variant: 'danger' }}) or, if that prop is
unavailable, set onConfirm={handleDelete}, confirmLabel='선택 삭제', remove/omit the
cancel action or rename it to a secondary non-disabled action so the UI shows
only meaningful enabled controls; reference AssignmentFormLayout, handleDelete,
onConfirm and onCancel when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4102c864-fccb-4d90-ba09-85bae434fa98
📒 Files selected for processing (20)
src/entities/assignment/api/assignmentApi.tssrc/entities/assignment/api/assignmentMutations.tssrc/entities/assignment/model/schemas.tssrc/entities/course/api/courseMutations.tssrc/entities/course/model/schemas.tssrc/entities/student/api/studentApi.tssrc/entities/student/api/studentMutations.tssrc/entities/student/model/schemas.tssrc/entities/student/ui/StudentTable.tsxsrc/features/course/create-course/model/useCreateCourse.tssrc/features/course/edit-course/model/useEditCourse.tssrc/pages/admin/assignments/AssignmentFormPage.tsxsrc/pages/admin/assignments/model/useAssignmentForm.tssrc/pages/admin/assignments/ui/TestcaseField.tsxsrc/pages/admin/assignments/ui/TestcaseRow.tsxsrc/pages/admin/student/StudentManagementPage.tsxsrc/pages/admin/student/model/useStudentManagement.tssrc/shared/config/endpoints.tssrc/shared/ui/FileUpload.tsxsrc/widgets/course-form/ui/CourseForm.tsx
| export const addEnrollment = async (courseId: number, studentId: string) => { | ||
| const res = await privateAxios.post(ENDPOINTS.ENROLLMENTS.BY_COURSE(courseId), {studentId}); | ||
| return apiResponseSchema(z.string()).parse(res.data); | ||
| return apiResponseSchema(z.unknown()).parse(res.data); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 다른 API 함수들의 응답 스키마 패턴 확인
rg -n "apiResponseSchema" --type=ts -C2Repository: 2025-snowCode/snowCode-Client
Length of output: 15714
🏁 Script executed:
# Check the full studentApi.ts file to understand context and available schemas
cat -n src/entities/student/api/studentApi.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 2075
🏁 Script executed:
# Check if there are any enrollment-related schemas defined
rg -n "enrollment|Enrollment" src/shared/model/schemas.ts src/entities/student/ --type=ts -iRepository: 2025-snowCode/snowCode-Client
Length of output: 4236
🏁 Script executed:
# Check if deleteEnrollment and other functions have comments or documentation about response
rg -B5 -A2 "deleteEnrollment|addEnrollmentsBulk" src/entities/student/api/studentApi.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 695
🏁 Script executed:
# Check what schemas are defined in the student model
cat -n src/entities/student/model/schemas.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 2620
🏁 Script executed:
# Check how the mutations are used in components
rg -B3 -A5 "addEnrollment|deleteEnrollment|addEnrollmentsBulk" src/entities/student/ui/ src/features/ --type=ts -iRepository: 2025-snowCode/snowCode-Client
Length of output: 809
🏁 Script executed:
# Check studentMutations to see if there's any type information
cat -n src/entities/student/api/studentMutations.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 923
🏁 Script executed:
# Check if there's any backend documentation or API definition
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "ENROLLMENTS.BY_COURSE\|ENROLLMENTS.BULK\|ENROLLMENTS.DETAIL" | head -5Repository: 2025-snowCode/snowCode-Client
Length of output: 114
🏁 Script executed:
# Check the ENDPOINTS definition to understand the API structure
rg -n "ENROLLMENTS" src/shared/config/endpoints.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 93
🏁 Script executed:
# Get the full ENDPOINTS definition
cat -n src/shared/config/endpoints.ts | grep -A 10 "ENROLLMENTS"Repository: 2025-snowCode/snowCode-Client
Length of output: 470
세 개의 등록금 변경 함수에서 타입 안정성이 손실되고 있습니다.
addEnrollment, addEnrollmentsBulk, deleteEnrollment 함수들이 z.unknown()을 사용하고 있어 다음 문제가 발생합니다:
- 응답 구조에 대한 타입 정보가 완전히 사라집니다
- 런타임 검증이 실질적으로 작동하지 않습니다
- 호출하는 쪽에서 응답을 안전하게 처리할 수 없습니다
같은 파일의 getEnrollments, getEnrollmentById 함수들은 enrollmentListSchema, studentDetailSchema를 사용하고 있으므로, 변경 작업(mutation)도 일관되게 처리해야 합니다.
백엔드 API 응답 구조를 확인하고 src/entities/student/model/schemas.ts에 적절한 스키마를 정의해주세요. 예를 들어:
export const enrollmentMutationResponseSchema = z.object({
success: z.boolean(),
message: z.string().optional(),
// 필요한 필드들...
});참고로 addEnrollmentsBulk 함수의 36번 라인에서 headers: {'Content-Type': 'multipart/form-data'}는 axios가 FormData를 감지하면 자동으로 처리하므로 제거해도 됩니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/entities/student/api/studentApi.ts` at line 25, The mutation functions
addEnrollment, addEnrollmentsBulk, and deleteEnrollment currently use
apiResponseSchema(z.unknown()) which loses type safety—define and export a
specific response Zod schema (e.g., enrollmentMutationResponseSchema) in
src/entities/student/model/schemas.ts that matches the backend mutation response
(success:boolean, optional message, and any payload fields), then replace
z.unknown() with that schema in those three functions so runtime validation and
typing match getEnrollments/getEnrollmentById; also remove the explicit headers:
{'Content-Type': 'multipart/form-data'} from addEnrollmentsBulk since axios
handles FormData automatically.
| onChange={(e) => setScore(e.target.value)} | ||
| label='문제 이름' | ||
| placeholder='입력하세요' | ||
| {...register('title')} |
There was a problem hiding this comment.
필수 필드에 대한 유효성 검사를 추가하세요.
Lines 31, 40에서 title과 description 필드에 유효성 검사 규칙이 없습니다. 사용자가 빈 값을 제출하면 서버 에러가 발생할 수 있습니다. react-hook-form의 register 옵션으로 최소한의 클라이언트 검증을 추가하세요.
🛡️ 유효성 검사 추가 예시
<LabeledInput
label='문제 이름'
placeholder='입력하세요'
- {...register('title')}
+ {...register('title', { required: '문제 이름을 입력하세요' })}
/>
<LabeledInput
label='문제 설명'
placeholder='입력하세요'
- {...register('description')}
+ {...register('description', { required: '문제 설명을 입력하세요' })}
/>에러 메시지 표시도 추가:
const { register, handleSubmit, watch, setValue, formState: { errors } } = useAssignmentForm(assignmentId);
// LabeledInput에 error prop 전달
<LabeledInput
label='문제 이름'
error={errors.title?.message}
{...register('title', { required: '문제 이름을 입력하세요' })}
/>React Hook Form 공식 문서: https://react-hook-form.com/api/useform/register
Also applies to: 40-40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/admin/assignments/AssignmentFormPage.tsx` at line 31, Add
client-side validation to the title and description fields by passing validation
rules to react-hook-form's register (e.g. required messages, optional minLength)
when calling {...register('title', {...})} and {...register('description',
{...})}; ensure you destructure formState: { errors } from
useAssignmentForm(assignmentId) and pass errors.title?.message and
errors.description?.message into the corresponding LabeledInput error props so
validation messages display to the user (update the register calls and
LabeledInput usages accordingly).
Comment on lines
+124
to
+128
| {value.map((tc, idx) => ( | ||
| <TestcaseRow | ||
| key={idx} | ||
| index={idx} | ||
| testcase={tc.testcase} |
There was a problem hiding this comment.
리스트 아이템에 안정적인 key를 사용하세요.
Line 126에서 key={idx}를 사용하면 테스트케이스를 삭제하거나 재정렬할 때 React가 컴포넌트를 잘못 재사용할 수 있습니다. 각 테스트케이스에 고유 ID를 부여하거나, 임시 ID를 생성하는 것이 좋습니다.
🔑 안정적인 key 사용 방법
방법 1: 임시 ID 추가
TestcaseValue 타입을 확장:
export type TestcaseValue = TAssignmentForm['testcases'][number] & {
id?: string;
};새 테스트케이스 추가 시 ID 생성:
const handleAddTestcase = () => {
- onChange([...value, {testcase: '', answer: '', isPublic: true}]);
+ onChange([...value, {
+ id: crypto.randomUUID(),
+ testcase: '',
+ answer: '',
+ isPublic: true
+ }]);
};key 속성 변경:
- {value.map((tc, idx) => (
+ {value.map((tc) => (
<TestcaseRow
- key={idx}
+ key={tc.id ?? tc.testcase + tc.answer}방법 2: 안정적인 복합 키
ID 추가 없이 콘텐츠 기반 키:
<TestcaseRow
- key={idx}
+ key={`${tc.testcase}-${tc.answer}-${idx}`}React 공식 문서 - 리스트의 Key: https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/admin/assignments/ui/TestcaseField.tsx` around lines 124 - 128, The
list uses index-based keys (key={idx}) in TestcaseField.tsx which causes React
reuse bugs; update the data shape (TestcaseValue) to include a stable id and use
that for the key in the TestcaseRow (e.g., testcase.id), and when creating new
testcases assign a unique id (UUID or timestamp+counter) so TestcaseRow,
addTestcase handler (or wherever new items are pushed) and any mapping that
renders TestcaseRow are changed to rely on that stable id; alternatively, if you
cannot add ids, use a stable composite key derived from immutable testcase
properties (e.g., `${testcase.input}-${testcase.output}`) instead of idx.
Comment on lines
+22
to
+28
| const {data, isLoading} = useQuery( | ||
| studentQueries.getEnrollments(courseId, { | ||
| page: 0, | ||
| pageSize: 1000, | ||
| studentId: searchValue || undefined, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
서버/클라이언트 페이지네이션 전략이 불일치합니다.
현재 구현에는 다음 문제가 있습니다:
- 페이지 번호 불일치: 서버는 0-based (
page: 0), 클라이언트는 1-based (currentPage = 1) - 대량 데이터 일괄 로드:
pageSize: 1000으로 모든 데이터를 한 번에 가져온 후 - 클라이언트 사이드 페이징: Lines 54-56에서 다시 슬라이싱
이는 성능과 확장성에 심각한 문제를 일으킵니다:
- 학생이 1000명 이상이면 데이터가 잘립니다
- 불필요한 네트워크 트래픽과 메모리 사용
- 서버 페이지네이션의 이점을 전혀 활용하지 못함
🔧 서버 사이드 페이지네이션으로 수정
const {data, isLoading} = useQuery(
studentQueries.getEnrollments(courseId, {
- page: 0,
- pageSize: 1000,
+ page: currentPage - 1, // 0-based로 변환
+ pageSize: 10,
studentId: searchValue || undefined,
})
);
- const pageSize = 10;
- const paginatedStudents =
- data?.students.slice((currentPage - 1) * pageSize, currentPage * pageSize) ??
- [];
return {
students: data?.students ?? [],
- paginatedStudents,
+ paginatedStudents: data?.students ?? [], // 서버에서 이미 페이징됨
// ...
- totalItems: data?.students.length ?? 0,
+ totalItems: data?.total ?? 0, // 서버 응답의 전체 개수 사용
- pageSize,
+ pageSize: 10,
};백엔드 응답에 total 필드가 없다면 추가가 필요합니다.
Also applies to: 53-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/admin/student/model/useStudentManagement.ts` around lines 22 - 28,
The current useQuery call is loading all students with pageSize: 1000 and then
client-slicing (see useQuery and the client-side slice around currentPage),
causing mismatch between server 0-based paging and client 1-based currentPage
and poor scalability; change the query to use the component's pageSize and map
the client page to the server page by passing page: currentPage - 1 (instead of
hardcoded 0) and pageSize: pageSize (instead of 1000), remove the client-side
slicing logic (the slice that trims results by currentPage), and rely on the
server response's items + total to render pages (or add/handle a total field on
the backend if missing) so pagination is entirely server-driven (references:
useQuery calling studentQueries.getEnrollments and the client slice around
currentPage).
Comment on lines
11
to
+12
| const {courseId} = useParams<{courseId: string}>(); | ||
| const [currentPage, setCurrentPage] = useState(1); | ||
| const {register, watch} = useForm<{studentSearch: string}>(); | ||
| const searchValue = watch('studentSearch'); | ||
|
|
||
| useEffect(() => { | ||
| setCurrentPage(1); | ||
| }, [searchValue]); | ||
| const numericCourseId = Number(courseId); |
There was a problem hiding this comment.
URL 파라미터 검증이 누락되었습니다.
Number(courseId)는 courseId가 undefined이거나 숫자로 변환할 수 없는 경우 NaN을 반환합니다. 이는 API 호출 시 오류를 발생시킵니다.
🛡️ 검증 추가
const {courseId} = useParams<{courseId: string}>();
+
+ if (!courseId || isNaN(Number(courseId))) {
+ return <div>올바르지 않은 강의 ID입니다.</div>;
+ }
+
const numericCourseId = Number(courseId);또는 React Router의 loader에서 사전 검증하는 것도 좋은 방법입니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const {courseId} = useParams<{courseId: string}>(); | |
| const [currentPage, setCurrentPage] = useState(1); | |
| const {register, watch} = useForm<{studentSearch: string}>(); | |
| const searchValue = watch('studentSearch'); | |
| useEffect(() => { | |
| setCurrentPage(1); | |
| }, [searchValue]); | |
| const numericCourseId = Number(courseId); | |
| const {courseId} = useParams<{courseId: string}>(); | |
| if (!courseId || isNaN(Number(courseId))) { | |
| return <div>올바르지 않은 강의 ID입니다.</div>; | |
| } | |
| const numericCourseId = Number(courseId); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/admin/student/StudentManagementPage.tsx` around lines 11 - 12, The
code reads the courseId URL param with useParams and converts it to
numericCourseId via Number(courseId) without validation, which can produce NaN;
add explicit validation after const { courseId } = useParams(...) to check for
undefined or non-numeric values (e.g., test courseId presence and that
Number(courseId) is finite), and handle invalid params by returning
early/redirecting/rendering an error state before making API calls; update any
callers that rely on numericCourseId (the numericCourseId variable) so they only
run when the validated numeric id is available.
Comment on lines
+90
to
+92
| onKeyDown={(e) => | ||
| (e.key === 'Enter' || e.key === ' ') && inputRef.current?.click() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/shared/ui/FileUpload.tsx | head -150Repository: 2025-snowCode/snowCode-Client
Length of output: 5767
🏁 Script executed:
# Verify that stopPropagation on click-only doesn't prevent keyboard event bubbling
rg -A 5 "type='button'" src/shared/ui/FileUpload.tsx | head -15Repository: 2025-snowCode/snowCode-Client
Length of output: 523
중첩된 버튼의 키보드 이벤트가 부모 핸들러로 버블링되어 파일 선택창이 예기치 않게 열립니다
선택된 파일 상태에서 삭제 버튼에 포커스하고 Enter/Space를 누르면, 버튼의 클릭 이벤트는 stopPropagation()으로 차단되지만 키보드 이벤트는 계속 부모 div로 버블링되어 90번 줄의 onKeyDown 핸들러가 실행됩니다. 그 결과 의도하지 않은 파일 선택창이 열립니다. 또한 Space 키 처리 시 preventDefault()가 없어 페이지 스크롤이 발생할 수 있습니다.
🔧 제안 diff
const handleRemoveFile = () => {
setSelectedFile(null);
onFileChange(null);
};
+
+ const handleTriggerKeyDown = (e: React.KeyboardEvent<HTMLDivElement>) => {
+ if (e.target !== e.currentTarget) return;
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ inputRef.current?.click();
+ }
+ };90번 줄과 118번 줄의 onKeyDown 핸들러를 아래와 같이 변경하세요:
- onKeyDown={(e) =>
- (e.key === 'Enter' || e.key === ' ') && inputRef.current?.click()
- }
+ onKeyDown={handleTriggerKeyDown}핵심 포인트:
e.target !== e.currentTarget: 자식 요소에서 발생한 이벤트를 필터링하여 부모 핸들러 실행 방지e.preventDefault(): Space 키의 기본 스크롤 동작 차단- 삭제 버튼에 포커스를 두고 Enter/Space 입력 시 파일 선택창이 열리지 않는지 확인하세요
참고 문서:
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onKeyDown={(e) => | |
| (e.key === 'Enter' || e.key === ' ') && inputRef.current?.click() | |
| } | |
| onKeyDown={handleTriggerKeyDown} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/ui/FileUpload.tsx` around lines 90 - 92, The parent onKeyDown
handler (the one that triggers inputRef.current?.click()) is firing for keyboard
events from nested controls; update those onKeyDown handlers to ignore events
not originating on the container and to prevent Space default: at the top of the
handler check if e.target !== e.currentTarget and return if true, then when
handling keys use if (e.key === 'Enter' || e.key === ' ') { e.preventDefault();
inputRef.current?.click(); } — apply the same change to both onKeyDown handlers
referenced (the container onKeyDowns that call inputRef.current?.click()) so
Enter/Space on the delete button no longer bubbles to the parent and Space
doesn't scroll the page.
JiiminHa
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚙️ Related ISSUE Number
close #115
📄 Work Description
1. 대량 작업 및 API 기능 강화
uploadTestcasesBulkAPI와 뮤테이션을 추가했습니다.addEnrollmentsBulkAPI와 일괄 삭제를 위한deleteEnrollmentsBulk뮤테이션을 도입했습니다.2. 강의 생성 및 수정 로직 개선
3. 스키마 및 데이터 처리 업데이트
studentId가 문자열(string)과 숫자(number) 타입을 모두 지원하도록 스키마를 개선하고, 향후 호환성을 위해.passthrough()를 추가했습니다.students배열을 포함하고 추가 필드 허용(passthrough) 설정을 더해, API 응답 처리가 더욱 유연해졌습니다.4. UI 및 상태 관리 개선
StudentTable컴포넌트가 행 선택 상태를 외부에서 관리하도록 리팩토링했습니다. 이를 통해 상위 컴포넌트에서 선택 상태를 제어하고 대량 작업을 원활하게 지원할 수 있습니다.📷 Screenshot
💬 To Reviewers
🔗 Reference