Merged
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/pages/unit-editor/mock/unitMock.ts (1)
28-31: 과제 5번 타이틀 복사-붙여넣기 오타 확인5번 과제(line 30)의 타이틀이 4번과 동일한
'날씨 데이터를 활용한 앱 만들기 4'로 되어 있습니다.'...5'가 의도된 것인지 확인해 주세요. 마찬가지로allUnitsResponse(lines 52–54)에서 단원 2, 3의 타이틀도 동일한'날씨 데이터 확인하기'를 사용하고 있어 구분이 어렵습니다.목 데이터는 API 연동 단계에서 교체될 예정이므로 지금 당장 수정이 필수는 아니지만, 개발 중 혼란을 줄이기 위해 고유한 값으로 맞춰두는 것을 추천합니다 😊
✏️ 수정 예시
{ id: 5, - title: '날씨 데이터를 활용한 앱 만들기 4', + title: '날씨 데이터를 활용한 앱 만들기 5', },{ id: 3, - title: '날씨 데이터 확인하기', + title: '함수와 반복문', assignmentCount: 3, },Based on learnings: mock data inconsistencies are not a priority to fix since the mock data will be replaced during API integration phase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/mock/unitMock.ts` around lines 28 - 31, In src/pages/unit-editor/mock/unitMock.ts update the mock entries so each unit has a unique title: fix the id:5 object currently titled '날씨 데이터를 활용한 앱 만들기 4' to the intended '날씨 데이터를 활용한 앱 만들기 5' (or another distinct label) and ensure the allUnitsResponse entries for units with id 2 and id 3 have distinct titles instead of both using '날씨 데이터 확인하기'; modify the title fields on the unit objects (e.g., the object with id 5 and the objects inside allUnitsResponse with ids 2 and 3) to unique, descriptive strings to avoid confusion during development.src/pages/unit-editor/UnitEditorPage.tsx (2)
96-96: 객체 단축 속성 사용 가능
courseId: courseId는 ES6 단축 속성으로 간결하게 표현할 수 있습니다.- addUnit({courseId: courseId, unit}); + addUnit({courseId, unit});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/UnitEditorPage.tsx` at line 96, addUnit is being called with an object that uses the verbose property form ({courseId: courseId, unit}); simplify it using ES6 shorthand by passing {courseId, unit} instead—update the addUnit call in UnitEditorPage (the addUnit({courseId: courseId, unit}) invocation) to use the shortened property syntax.
38-79:alert()기반 피드백 — 향후 Toast/Snackbar로 전환 권장
alert()는 블로킹 다이얼로그로 UX가 제한적입니다. 현 단계에서는 동작하지만, 향후 Toast 또는 Snackbar 컴포넌트로 교체하면 사용자 경험이 크게 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/UnitEditorPage.tsx` around lines 38 - 79, The code uses blocking alert() calls in the mutation handlers (in the useMutation blocks for addUnit, updateUnit, deleteUnit) which hurts UX; replace these alert(...) calls with a non-blocking toast/snackbar API (or the app's existing notification utility) in the onSuccess and onError handlers for addUnit, updateUnit, and deleteUnit, preserving the current messages and keeping console.error(error) for errors; if no notification utility exists, add a small toast helper (e.g., notify.success / notify.error) and call that instead, leaving state changes (invalidateUnitList, setMode, setSelectedUnitId) unchanged.src/entities/unit/api/unitQueries.ts (1)
13-18:unitId가null일 때enabled가드에 의존 — 타입 안전성 보완 권장
enabled: !!unitId로null시 쿼리 실행을 방지하고 있지만,queryFn자체는getUnitById(null)호출이 가능한 상태입니다. TanStack Query의skipToken을 사용하면 타입 레벨에서도 안전하게 방어할 수 있습니다.♻️ skipToken 활용 제안
+import {queryOptions, skipToken} from '@tanstack/react-query'; getUnitDetails: (unitId: number | null) => queryOptions({ queryKey: ['units', 'detail', unitId], - queryFn: () => getUnitById(unitId), - enabled: !!unitId, + queryFn: unitId ? () => getUnitById(unitId) : skipToken, }),참고: TanStack Query 공식 문서 — Disabling Queries
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/api/unitQueries.ts` around lines 13 - 18, The getUnitDetails query currently guards execution with enabled: !!unitId but still passes unitId into queryFn (getUnitById(unitId)), so getUnitById can be called with null; update getUnitDetails (the query factory in src/entities/unit/api/unitQueries.ts) to use TanStack Query's skipToken for type-safe disabling — return queryOptions with queryKey/queryFn only when unitId is non-null, otherwise return skipToken so getUnitById is never invoked with null; adjust references to getUnitDetails to accept skipToken-compatible return shape.src/entities/unit/model/types.ts (1)
11-11: 문자열 비교 기반 날짜 검증 — 현재 동작하지만 주의 필요
data.releaseDate <= data.dueDate는 문자열 lexicographic 비교입니다. HTML<input type="date">가YYYY-MM-DDISO 형식을 반환하므로 현재는 정상 동작하지만, 다른 형식의 날짜 문자열이 들어오면 비교가 깨질 수 있습니다. 현 단계에서는 문제 없으나, 향후 날짜 형식이 변경될 경우를 대비해new Date()변환 비교도 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/model/types.ts` at line 11, The current .refine((data) => data.releaseDate <= data.dueDate, { ... }) uses lexicographic string comparison on data.releaseDate and data.dueDate; change it to parse both into Date objects and compare their numeric time values (e.g., new Date(data.releaseDate).getTime() <= new Date(data.dueDate).getTime()), and ensure you guard against invalid dates by returning false if either parsed date is NaN so the refine fails for bad input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/entities/unit/api/unitApi.ts`:
- Around line 15-20: The current getUnitById function accepts unitId: number |
null and will call /units/null if null — fix by either (preferred) narrowing the
parameter to number and updating callers (e.g., unitQueries.ts) to guard the
query with enabled: unitId !== null and call getUnitById(unitId!) from the
queryFn, or (alternative) add an internal null guard in getUnitById that returns
an appropriate ApiResponse or throws before making privateAxios.get when unitId
is null; locate the function getUnitById in unitApi.ts and the query usage in
unitQueries.ts to implement one of these fixes.
In `@src/entities/unit/model/types.ts`:
- Line 1: The import of Zod uses a default import which fails for Zod v4; update
the import statement that currently reads "import z from 'zod'" to a supported
form such as "import * as z from 'zod'" or "import { z } from 'zod'" so the
module resolves correctly (locate the top-level import in the types file where z
is referenced).
In `@src/pages/unit-editor/mock/unitMock.ts`:
- Line 1: Rename the file unitMock.ts to kebab-case unit-mock.ts and update all
imports that reference unitMock (e.g., any import paths using "unitMock" or the
module exporting AllUnitsResponse/SingleUnitResponse) to use "unit-mock"
instead; ensure module exports/types in src/pages/unit-editor/mock/unit-mock.ts
keep the same names so no further code changes are needed and run a full project
search to update every import string to the new filename.
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx`:
- Line 99: The Tailwind class string in the UnitAssignmentList component
contains an invalid token "cursor grab"; update the className used in the
element (in UnitAssignmentList.tsx) to use the correct Tailwind utility
"cursor-grab" instead of "cursor grab" while keeping the rest of the classes
(e.g., active:cursor-grabbing, bg-white, shadow-box, isDragging conditional)
intact.
- Line 28: The component UnitAssignmentList initializes local state via const
[assignments, setAssignments] = useState<Assignment[]>(assignmentList) which
only uses assignmentList once at mount, so add synchronization when the prop
changes: inside UnitAssignmentList add a useEffect that watches assignmentList
and calls setAssignments(assignmentList) when it changes (or alternatively
accept a key prop from the parent to force remounts); reference the
assignments/setAssignments state and the assignmentList prop when implementing
the effect so the component updates whenever the parent provides a new
assignmentList.
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Around line 31-46: The current useForm call uses the values option which will
reset the form whenever those values change (causing user edits to be lost) and
also omits assignmentIds in editing mode; change to use defaultValues in
useForm({ resolver: zodResolver(unitFormSchema), defaultValues: { title: '',
releaseDate: '', dueDate: '', assignmentIds: [] } }) and then, in the UnitForm
component, watch for loaded unit data and call reset(loadedValues) inside a
useEffect (reset should include assignmentIds from unit for editing mode) so
form state is only updated intentionally; ensure references to useForm,
unitFormSchema, mode, unit, and reset are used to locate and implement this
change.
In `@src/pages/unit-editor/ui/UnitList.tsx`:
- Around line 43-48: Update the misleading comment above the Badge in
UnitList.tsx: replace the "단원 인덱스 배지" comment with a concise comment that
reflects the actual content (e.g., "과제 수 배지" or "assignment count badge") so it
matches the Badge usage showing {assignmentCount}; the affected JSX includes the
Badge component and the assignmentCount expression.
---
Duplicate comments:
In `@src/entities/unit/api/unitApi.ts`:
- Around line 30-36: The createUnit function uses a POST to /units/${courseId}
which is inconsistent with the GET pattern GET /courses/${courseId}/units;
verify the backend API contract and if creation should follow REST/other
endpoints, update the privateAxios.post call in createUnit to POST to
/courses/${courseId}/units (or adjust to the backend's intended path) and ensure
any related tests or callers use the same path.
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx`:
- Around line 35-48: handleDragEnd currently uses an any-typed event and
accesses over.id without null-checking; update the function signature to use
DragEndEvent from `@dnd-kit/core` (import it) and guard against a null/undefined
over before accessing its id (return early if !over). Keep usage of
getAssignmentIndex, setAssignments, and arrayMove as-is but ensure you reference
event.active and event.over only after the null check; remove the any type and
do not use any unless you add an inline justification comment per guidelines.
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Line 24: The state variables assignmentIds and setAssignmentIds declared in
UnitForm (const [assignmentIds, setAssignmentIds] = useState<number[]>([])) are
unused and causing ESLint build errors; either remove this unused useState
declaration entirely from UnitForm.tsx, or if you intend to use it later, keep
the declaration but add a clear TODO comment referencing the tracking issue/ID
(e.g., TODO: link issue `#1234`) explaining planned usage so ESLint warnings are
suppressed and intent is documented.
- Around line 73-74: Replace the hardcoded id 'unit-form' with a unique id using
unitIndex (e.g. `const formID = `unit-form-${unitIndex}``) and use that formID
wherever the form id is referenced (replace the literal id in the <form> element
and update the existing formID usage at the other occurrence), ensuring the
UnitForm component consistently generates and consumes this dynamic id to avoid
collisions when multiple instances render.
- Around line 93-118: The form only renders the dueDate error; add error display
spans for title and releaseDate by referencing the same pattern used for
dueDate: after the LabeledInput for register('title') render a span that shows
errors.title?.message, and after the LabeledInput for register('releaseDate')
render a span that shows errors.releaseDate?.message; ensure the spans use the
same styling (e.g., className='col-span-2 text-xs text-badge-red -mt-3 h-1' or
appropriate positioning) so users see validation messages for title and
releaseDate.
In `@src/pages/unit-editor/ui/UnitList.tsx`:
- Around line 38-56: The list item <li> in UnitList.tsx is not
keyboard-accessible; update the element rendering each unit (where unitList?.map
destructures id, title, assignmentCount and uses handleSelectUnit and
selectedUnitId) to add role="button", tabIndex={0}, and an onKeyDown handler
that triggers handleSelectUnit(id) when Enter or Space is pressed (and prevents
default for Space). Keep the existing onClick, key prop and visual selection
logic (selectedUnitId and ArrowrightIcon) unchanged.
In `@src/pages/unit-editor/UnitEditorPage.tsx`:
- Around line 14-20: The unit list query runs even when id is undefined because
courseId = Number(id) can be NaN; update the useQuery call for
unitQueries.getUnitList to include an enabled flag (e.g., enabled: !!id or
Number.isFinite(courseId)) so the query only fires when a valid id exists, and
likewise add enabled: selectedUnitId != null (or typeof selectedUnitId ===
'number') to the unitQueries.getUnitDetails useQuery to prevent requests for
null/undefined selectedUnitId; locate these changes around the
useParams/useQuery calls and variables courseId, unitQueries.getUnitList,
unitQueries.getUnitDetails, selectedUnitId and set the enabled conditions
accordingly.
---
Nitpick comments:
In `@src/entities/unit/api/unitQueries.ts`:
- Around line 13-18: The getUnitDetails query currently guards execution with
enabled: !!unitId but still passes unitId into queryFn (getUnitById(unitId)), so
getUnitById can be called with null; update getUnitDetails (the query factory in
src/entities/unit/api/unitQueries.ts) to use TanStack Query's skipToken for
type-safe disabling — return queryOptions with queryKey/queryFn only when unitId
is non-null, otherwise return skipToken so getUnitById is never invoked with
null; adjust references to getUnitDetails to accept skipToken-compatible return
shape.
In `@src/entities/unit/model/types.ts`:
- Line 11: The current .refine((data) => data.releaseDate <= data.dueDate, { ...
}) uses lexicographic string comparison on data.releaseDate and data.dueDate;
change it to parse both into Date objects and compare their numeric time values
(e.g., new Date(data.releaseDate).getTime() <= new
Date(data.dueDate).getTime()), and ensure you guard against invalid dates by
returning false if either parsed date is NaN so the refine fails for bad input.
In `@src/pages/unit-editor/mock/unitMock.ts`:
- Around line 28-31: In src/pages/unit-editor/mock/unitMock.ts update the mock
entries so each unit has a unique title: fix the id:5 object currently titled
'날씨 데이터를 활용한 앱 만들기 4' to the intended '날씨 데이터를 활용한 앱 만들기 5' (or another distinct
label) and ensure the allUnitsResponse entries for units with id 2 and id 3 have
distinct titles instead of both using '날씨 데이터 확인하기'; modify the title fields on
the unit objects (e.g., the object with id 5 and the objects inside
allUnitsResponse with ids 2 and 3) to unique, descriptive strings to avoid
confusion during development.
In `@src/pages/unit-editor/UnitEditorPage.tsx`:
- Line 96: addUnit is being called with an object that uses the verbose property
form ({courseId: courseId, unit}); simplify it using ES6 shorthand by passing
{courseId, unit} instead—update the addUnit call in UnitEditorPage (the
addUnit({courseId: courseId, unit}) invocation) to use the shortened property
syntax.
- Around line 38-79: The code uses blocking alert() calls in the mutation
handlers (in the useMutation blocks for addUnit, updateUnit, deleteUnit) which
hurts UX; replace these alert(...) calls with a non-blocking toast/snackbar API
(or the app's existing notification utility) in the onSuccess and onError
handlers for addUnit, updateUnit, and deleteUnit, preserving the current
messages and keeping console.error(error) for errors; if no notification utility
exists, add a small toast helper (e.g., notify.success / notify.error) and call
that instead, leaving state changes (invalidateUnitList, setMode,
setSelectedUnitId) unchanged.
Comment on lines
+15
to
+20
| export const getUnitById = async ( | ||
| unitId: number | null | ||
| ): Promise<ApiResponse<Unit>> => { | ||
| const response = await privateAxios.get(`/units/${unitId}`); | ||
| return response.data; | ||
| }; |
There was a problem hiding this comment.
unitId가 null일 때 /units/null 요청이 발생합니다
파라미터 타입이 number | null이지만, 함수 내부에 null 가드가 없어 실제로 null이 전달되면 /units/null이라는 잘못된 엔드포인트로 요청이 전송됩니다.
두 가지 개선 방향이 있습니다:
방향 1 (권장): 파라미터 타입을 number로 좁히고, 호출부(unitQueries.ts)에서 null 여부를 판단하여 쿼리를 비활성화합니다 (enabled: unitId !== null).
방향 2: 함수 내부에서 early return을 통해 방어합니다.
🛡️ 방향 1 — 파라미터 타입 축소 (권장)
-export const getUnitById = async (
- unitId: number | null
-): Promise<ApiResponse<Unit>> => {
+export const getUnitById = async (
+ unitId: number
+): Promise<ApiResponse<Unit>> => {
const response = await privateAxios.get(`/units/${unitId}`);
return response.data;
};unitQueries.ts 쪽에서는 아래와 같이 enabled 옵션으로 처리:
queryOptions({
queryKey: [...],
queryFn: () => getUnitById(unitId!),
enabled: unitId !== null,
})🛡️ 방향 2 — 함수 내부 null 가드
export const getUnitById = async (
unitId: number | null
): Promise<ApiResponse<Unit>> => {
+ if (unitId === null) {
+ throw new Error('unitId는 null일 수 없습니다.');
+ }
const response = await privateAxios.get(`/units/${unitId}`);
return response.data;
};📝 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
| export const getUnitById = async ( | |
| unitId: number | null | |
| ): Promise<ApiResponse<Unit>> => { | |
| const response = await privateAxios.get(`/units/${unitId}`); | |
| return response.data; | |
| }; | |
| export const getUnitById = async ( | |
| unitId: number | |
| ): Promise<ApiResponse<Unit>> => { | |
| const response = await privateAxios.get(`/units/${unitId}`); | |
| return response.data; | |
| }; |
Suggested change
| export const getUnitById = async ( | |
| unitId: number | null | |
| ): Promise<ApiResponse<Unit>> => { | |
| const response = await privateAxios.get(`/units/${unitId}`); | |
| return response.data; | |
| }; | |
| export const getUnitById = async ( | |
| unitId: number | null | |
| ): Promise<ApiResponse<Unit>> => { | |
| if (unitId === null) { | |
| throw new Error('unitId는 null일 수 없습니다.'); | |
| } | |
| const response = await privateAxios.get(`/units/${unitId}`); | |
| return response.data; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/entities/unit/api/unitApi.ts` around lines 15 - 20, The current
getUnitById function accepts unitId: number | null and will call /units/null if
null — fix by either (preferred) narrowing the parameter to number and updating
callers (e.g., unitQueries.ts) to guard the query with enabled: unitId !== null
and call getUnitById(unitId!) from the queryFn, or (alternative) add an internal
null guard in getUnitById that returns an appropriate ApiResponse or throws
before making privateAxios.get when unitId is null; locate the function
getUnitById in unitApi.ts and the query usage in unitQueries.ts to implement one
of these fixes.
| @@ -0,0 +1,58 @@ | |||
| import type {AllUnitsResponse, SingleUnitResponse} from '@/models/course'; | |||
There was a problem hiding this comment.
파일명을 kebab-case로 변경해 주세요 (unitMock.ts → unit-mock.ts)
프로젝트 코딩 가이드라인에 따르면 일반 파일명은 kebab-case 를 사용해야 합니다. unitMock.ts는 camelCase이므로 unit-mock.ts로 변경이 필요합니다. PR에서 파일명 개선 리팩토링이 포함되어 있는 만큼, 이 파일도 함께 맞춰주시면 좋겠습니다!
📂 변경 예시
- src/pages/unit-editor/mock/unitMock.ts
+ src/pages/unit-editor/mock/unit-mock.ts파일을 참조하는 모든 import 경로도 함께 업데이트해야 합니다.
As per coding guidelines: "일반 파일명: kebab-case"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/unit-editor/mock/unitMock.ts` at line 1, Rename the file
unitMock.ts to kebab-case unit-mock.ts and update all imports that reference
unitMock (e.g., any import paths using "unitMock" or the module exporting
AllUnitsResponse/SingleUnitResponse) to use "unit-mock" instead; ensure module
exports/types in src/pages/unit-editor/mock/unit-mock.ts keep the same names so
no further code changes are needed and run a full project search to update every
import string to the new filename.
| }: { | ||
| assignmentList: Assignment[]; | ||
| }) => { | ||
| const [assignments, setAssignments] = useState<Assignment[]>(assignmentList); |
There was a problem hiding this comment.
useState(assignmentList)는 초기값만 반영 — prop 변경 시 동기화 안 됨
useState의 초기값은 마운트 시 한 번만 사용됩니다. 부모에서 다른 단원을 선택해 assignmentList prop이 변경되어도 내부 assignments 상태는 갱신되지 않습니다.
useEffect로 prop 변경 시 상태를 동기화하거나, 부모에서 key prop을 전달하여 리마운트를 유도하는 방식을 권장합니다.
♻️ useEffect 동기화 예시
+ import {useEffect, useState} from 'react';
const [assignments, setAssignments] = useState<Assignment[]>(assignmentList);
+ useEffect(() => {
+ setAssignments(assignmentList);
+ }, [assignmentList]);참고: React 공식 문서 — You Might Not Need an Effect
📝 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 [assignments, setAssignments] = useState<Assignment[]>(assignmentList); | |
| const [assignments, setAssignments] = useState<Assignment[]>(assignmentList); | |
| useEffect(() => { | |
| setAssignments(assignmentList); | |
| }, [assignmentList]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx` at line 28, The component
UnitAssignmentList initializes local state via const [assignments,
setAssignments] = useState<Assignment[]>(assignmentList) which only uses
assignmentList once at mount, so add synchronization when the prop changes:
inside UnitAssignmentList add a useEffect that watches assignmentList and calls
setAssignments(assignmentList) when it changes (or alternatively accept a key
prop from the parent to force remounts); reference the
assignments/setAssignments state and the assignmentList prop when implementing
the effect so the component updates whenever the parent provides a new
assignmentList.
Comment on lines
+31
to
+46
| } = useForm<TUnitFormSchema>({ | ||
| resolver: zodResolver(unitFormSchema), | ||
| values: | ||
| mode === 'creating' | ||
| ? { | ||
| title: '', | ||
| releaseDate: '', | ||
| dueDate: '', | ||
| assignmentIds: [], | ||
| } | ||
| : { | ||
| title: unit?.title || '', | ||
| releaseDate: unit?.releaseDate || '', | ||
| dueDate: unit?.dueDate || '', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
values 옵션 사용 시 사용자 편집 내용이 덮어씌워질 수 있음
react-hook-form의 values 옵션은 값이 변경될 때마다 폼을 리셋합니다. unit 데이터가 React Query에 의해 리페치되면, 사용자가 편집 중인 내용이 유실될 수 있습니다.
또한, creating 모드에서는 assignmentIds: []를 포함하지만 editing 모드에서는 누락되어 있어 일관성이 부족합니다.
defaultValues + useEffect/reset() 패턴을 사용하거나, values 사용 시 리페치 시점을 제어하는 것을 권장합니다.
참고: react-hook-form 공식 문서 — values
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 31 - 46, The current
useForm call uses the values option which will reset the form whenever those
values change (causing user edits to be lost) and also omits assignmentIds in
editing mode; change to use defaultValues in useForm({ resolver:
zodResolver(unitFormSchema), defaultValues: { title: '', releaseDate: '',
dueDate: '', assignmentIds: [] } }) and then, in the UnitForm component, watch
for loaded unit data and call reset(loadedValues) inside a useEffect (reset
should include assignmentIds from unit for editing mode) so form state is only
updated intentionally; ensure references to useForm, unitFormSchema, mode, unit,
and reset are used to locate and implement this change.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/entities/assignment/model/types.ts (1)
6-10:src/shared/mocks/assignmentDummyTypes.ts의 중복Assignment인터페이스 정리를 권장합니다.
src/shared/mocks/assignmentDummyTypes.ts에 아래와 같이 동일한Assignment인터페이스가 별도로 정의되어 있습니다:// src/shared/mocks/assignmentDummyTypes.ts interface Assignment { id: number; title: string; submittedStatus?: SubmissionStatus; }현재는 두 정의가 동일하지만, 이후 어느 한쪽만 수정될 경우 타입 불일치가 발생할 수 있습니다. 모킹 파일에서는 별도 선언 대신 엔티티 타입을 직접 import해서 사용하는 것이 유지보수성 측면에서 좋습니다.
♻️ 개선 방향 예시
// src/shared/mocks/assignmentDummyTypes.ts -import type { SubmissionStatus } from '@/shared/model/common'; - -interface Assignment { - id: number; - title: string; - submittedStatus?: SubmissionStatus; -} +import type { Assignment } from '@/entities/assignment/model/types';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/assignment/model/types.ts` around lines 6 - 10, There is a duplicate Assignment interface declaration in the mock module; remove the local interface declaration in assignmentDummyTypes and instead import the canonical Assignment (and SubmissionStatus if needed) type from the shared entity type definition where Assignment is exported, then update the mock file to use the imported types everywhere the local interface was used so only the single authoritative Assignment interface remains.src/shared/ui/EmptyState.tsx (1)
1-13: 파일명 케밥-케이스 적용 권장:EmptyState.tsx→empty-state.tsx코딩 가이드라인에 따르면 일반 파일명은 kebab-case를 사용해야 합니다. 현재
EmptyState.tsx는 PascalCase로 되어 있습니다. PR 설명에도 파일 구조 및 파일명 개선이 예정되어 있으니 이번 기회에 함께 정리해 보시면 어떨까요? 😊- EmptyState.tsx + empty-state.tsx해당 파일을 임포트하는 곳에서도 경로를 업데이트해 주세요.
As per coding guidelines: "일반 파일명: kebab-case"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/EmptyState.tsx` around lines 1 - 13, Rename the component file from EmptyState.tsx to kebab-case empty-state.tsx and update any imports that reference EmptyState to import from './empty-state' instead of './EmptyState'; ensure the exported symbol remains Export const EmptyState and the props interface EmptyStateProps is left unchanged inside the file so type references still resolve. After renaming, run TypeScript build/IDE import fixer to catch and update remaining import paths across the project.src/entities/course/model/types.ts (1)
94-102: FSD 슬라이스 경계 위반 —unit관련 타입이course엔티티에 위치합니다.
SingleUnitResponse와AllUnitsResponse는 단원(Unit) 도메인의 API 응답 타입임에도course엔티티 모델에 선언되어 있습니다.FSD에서 엔티티는 슬라이스이고, 기본적으로 슬라이스는 서로 참조할 수 없습니다. 이런 엔티티 간 상호작용 비즈니스 로직은 Features나 Pages 같은 상위 레이어에 두는 것이 권장됩니다. 따라서 이 타입들은
src/entities/unit/model/types.ts에 위치하는 것이 맞습니다.PR 설명에서 FSD 구조 적용 예정이라고 명시했으므로, 머지 전 아래 이동을 확인해 주세요:
♻️ 권장 이동 방향
// src/entities/course/model/types.ts -// 단원 조회-생성 페이지 응답 타입 정의 - 단일 단원 -export type SingleUnitResponse = ApiResponse<Unit>; - -// 단원 조회-생성 페이지 응답 타입 정의 - 전체 단원 목록 -export type AllUnitsResponse = ApiResponse<{ - count: number; - units: Pick<Unit, 'id' | 'title' | 'assignmentCount'>[]; -}>;// src/entities/unit/model/types.ts (신규 또는 기존 파일) +import type { ApiResponse } from '@/shared/model/common'; +import type { Unit } from '@/entities/course/model/types'; // 또는 `@x` 표기법 활용 +export type SingleUnitResponse = ApiResponse<Unit>; +export type AllUnitsResponse = ApiResponse<{ + count: number; + units: Pick<Unit, 'id' | 'title' | 'assignmentCount'>[]; +}>;참고: FSD 엔티티 간 cross-import 시에는
@x표기법 사용을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/course/model/types.ts` around lines 94 - 102, The types SingleUnitResponse and AllUnitsResponse are defined in the course entity but belong to the unit domain; move their declarations out of the course model into the unit entity's model/types file (i.e., the unit slice's types module), update any imports/exports so consumers reference the unit slice API, and replace the removed exports in the course types with imports that re-export or use the unit types via the unit slice public API (using the project's cross-import convention) to avoid cross-slice boundary violations.src/shared/ui/Layout.tsx (1)
13-22: 넓은 뷰포트에서 헤더와 메인 콘텐츠 간 너비 불일치가 발생할 수 있습니다.내부 컨테이너는
min-w-289(최소 1156px)만 지정되어 있어 뷰포트가 더 넓을 경우 flex 기본 동작에 따라 늘어납니다. 반면 헤더 div는w-289 mx-auto로 1156px에 고정되지만,<main>(<Outlet />)은 너비 제약이 없어 헤더보다 넓게 렌더링될 수 있습니다.의도적으로 1156px 고정 레이아웃을 목표로 한다면
min-w-289대신w-289를 사용하거나,max-w-289를 추가하는 것을 고려해 주세요.♻️ 개선 제안
- <div className='min-w-289 px-4 py-12'> + <div className='w-289 px-4 py-12'>또는 헤더와 메인 콘텐츠를 동일한 너비로 맞추려면:
<main> + <div className='w-289 mx-auto'> <Outlet /> + </div> </main>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Layout.tsx` around lines 13 - 22, The outer container uses className 'min-w-289' which lets the main content grow wider than the header; change the layout so header and main share the same fixed width by replacing 'min-w-289' with 'w-289' (or add 'max-w-289') on the outer div, or alternatively wrap <Outlet /> in the same constrained div used for <Header /> (reference showHeader, <Header />, and <Outlet />) so both render at the same 1156px width.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Line 15: The import in App.tsx uses a relative path for UnitEditorPage; change
it to the project's absolute alias import (use
'@/pages/unit-editor/UnitEditorPage') so the UnitEditorPage import matches other
imports and follows the "src/** absolute path (`@/`...)" rule.
- Line 57: The Route for "units/:id" is declared outside the admin Route block
causing the URL to be "/units/:id" instead of the intended admin path; move the
Route path='units/:id' that renders UnitEditorPage into the existing Route
path='admin' block so the resolved route becomes "/admin/units/:id". Locate the
Route element with path='units/:id' and the parent Route element with
path='admin' and relocate the former as a child of the latter (keeping the
element prop referencing UnitEditorPage) so routing is consistent with other
admin pages.
In `@src/entities/course/model/types.ts`:
- Around line 99-102: Several files import types from the wrong path
'@/models/course' (which maps to non-existent src/models/course); update the
import statements in src/pages/unit-editor/mock/unitMock.ts,
src/entities/unit/api/unitApi.ts, src/pages/unit-editor/ui/UnitList.tsx, and
src/pages/unit-editor/model/types.ts to import from
'@/entities/course/model/types' instead so AllUnitsResponse and Unit-related
types resolve correctly; optionally consider extracting Pick<Unit, "id" |
"title" | "assignmentCount"> into a UnitSummary type near the Unit definition
(e.g., alongside Unit or in types.ts) for clarity, but this is low priority.
---
Duplicate comments:
In `@src/shared/ui/Button.tsx`:
- Line 4: The export of the non-component value buttonStyles must be moved out
of the component file to restore Vite Fast Refresh: create a new module (e.g.,
button-styles.ts or button-styles.tsx) that exports buttonStyles (keeping the
same exported symbol name), remove the buttonStyles export from Button.tsx, and
import { buttonStyles } into Button.tsx where it’s used; ensure any related
types or helpers used by buttonStyles are moved or re-exported so the Button
component references the relocated symbol without exporting non-component values
from the component file.
---
Nitpick comments:
In `@src/entities/assignment/model/types.ts`:
- Around line 6-10: There is a duplicate Assignment interface declaration in the
mock module; remove the local interface declaration in assignmentDummyTypes and
instead import the canonical Assignment (and SubmissionStatus if needed) type
from the shared entity type definition where Assignment is exported, then update
the mock file to use the imported types everywhere the local interface was used
so only the single authoritative Assignment interface remains.
In `@src/entities/course/model/types.ts`:
- Around line 94-102: The types SingleUnitResponse and AllUnitsResponse are
defined in the course entity but belong to the unit domain; move their
declarations out of the course model into the unit entity's model/types file
(i.e., the unit slice's types module), update any imports/exports so consumers
reference the unit slice API, and replace the removed exports in the course
types with imports that re-export or use the unit types via the unit slice
public API (using the project's cross-import convention) to avoid cross-slice
boundary violations.
In `@src/shared/ui/EmptyState.tsx`:
- Around line 1-13: Rename the component file from EmptyState.tsx to kebab-case
empty-state.tsx and update any imports that reference EmptyState to import from
'./empty-state' instead of './EmptyState'; ensure the exported symbol remains
Export const EmptyState and the props interface EmptyStateProps is left
unchanged inside the file so type references still resolve. After renaming, run
TypeScript build/IDE import fixer to catch and update remaining import paths
across the project.
In `@src/shared/ui/Layout.tsx`:
- Around line 13-22: The outer container uses className 'min-w-289' which lets
the main content grow wider than the header; change the layout so header and
main share the same fixed width by replacing 'min-w-289' with 'w-289' (or add
'max-w-289') on the outer div, or alternatively wrap <Outlet /> in the same
constrained div used for <Header /> (reference showHeader, <Header />, and
<Outlet />) so both render at the same 1156px width.
Comment on lines
+99
to
+102
| export type AllUnitsResponse = ApiResponse<{ | ||
| count: number; | ||
| units: Pick<Unit, 'id' | 'title' | 'assignmentCount'>[]; | ||
| }>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find tsconfig files and examine path aliases
find . -name "tsconfig*.json" -type f | head -10Repository: 2025-snowCode/snowCode-Client
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Check the tsconfig.json content for path aliases
cat tsconfig.json 2>/dev/null || echo "tsconfig.json not found"Repository: 2025-snowCode/snowCode-Client
Length of output: 191
🏁 Script executed:
#!/bin/bash
# Search for files using or referencing the Pick<Unit pattern
rg "Pick<Unit" -A 2 -B 2Repository: 2025-snowCode/snowCode-Client
Length of output: 337
🏁 Script executed:
#!/bin/bash
# Check the actual file mentioned in the review
cat -n src/entities/course/model/types.ts | sed -n '95,110p'Repository: 2025-snowCode/snowCode-Client
Length of output: 375
🏁 Script executed:
#!/bin/bash
# Find unitMock.ts and check its imports
fd "unitMock.ts" -type fRepository: 2025-snowCode/snowCode-Client
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Check tsconfig.app.json for path aliases
cat tsconfig.app.jsonRepository: 2025-snowCode/snowCode-Client
Length of output: 901
🏁 Script executed:
#!/bin/bash
# Find unitMock.ts using find command
find . -name "unitMock.ts"Repository: 2025-snowCode/snowCode-Client
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for any references to `@/models/course` or `@/entities/course`
rg "@/models/course|@/entities/course" -nRepository: 2025-snowCode/snowCode-Client
Length of output: 2459
🏁 Script executed:
#!/bin/bash
# Search for UnitSummary or similar unit summary types in codebase
rg "UnitSummary|Pick.*Unit.*id.*title.*assignmentCount" -nRepository: 2025-snowCode/snowCode-Client
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check if src/models directory exists and what it contains
find src/models -type f 2>/dev/null || echo "src/models directory does not exist"Repository: 2025-snowCode/snowCode-Client
Length of output: 108
🏁 Script executed:
#!/bin/bash
# Look for index.ts or course.ts in src/models or src/models/course
find . -path "./src/models*" -name "*.ts"Repository: 2025-snowCode/snowCode-Client
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Check the unitMock.ts file to see the full import context
cat -n src/pages/unit-editor/mock/unitMock.ts | head -20Repository: 2025-snowCode/snowCode-Client
Length of output: 661
🏁 Script executed:
#!/bin/bash
# Search for any barrel exports (index.ts) that might re-export from entities
rg "export.*from.*@/entities/course" -nRepository: 2025-snowCode/snowCode-Client
Length of output: 55
@/models/course 임포트 경로 문제 해결 필요
현재 코드에서 여러 파일이 @/models/course에서 타입을 임포트하고 있지만, tsconfig.app.json의 경로 별칭 설정에 따르면 @/models/course는 src/models/course로 매핑되며, 이 디렉토리는 존재하지 않습니다. 실제 타입 정의는 src/entities/course/model/types.ts에 있으므로, 다음 파일들의 임포트 경로를 수정해야 합니다:
src/pages/unit-editor/mock/unitMock.tssrc/entities/unit/api/unitApi.tssrc/pages/unit-editor/ui/UnitList.tsxsrc/pages/unit-editor/model/types.ts
이들 파일에서 @/models/course를 @/entities/course/model/types로 변경하세요.
부가 개선사항: Pick<Unit, 'id' | 'title' | 'assignmentCount'>을 UnitSummary 타입으로 추출하는 것도 가독성 향상에 도움이 되겠습니다. 다만 현재 코드베이스에서 이 패턴이 한 곳에서만 사용되므로, 재사용성 측면에서는 우선순위가 낮습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/entities/course/model/types.ts` around lines 99 - 102, Several files
import types from the wrong path '@/models/course' (which maps to non-existent
src/models/course); update the import statements in
src/pages/unit-editor/mock/unitMock.ts, src/entities/unit/api/unitApi.ts,
src/pages/unit-editor/ui/UnitList.tsx, and src/pages/unit-editor/model/types.ts
to import from '@/entities/course/model/types' instead so AllUnitsResponse and
Unit-related types resolve correctly; optionally consider extracting Pick<Unit,
"id" | "title" | "assignmentCount"> into a UnitSummary type near the Unit
definition (e.g., alongside Unit or in types.ts) for clarity, but this is low
priority.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/course-overview/ui/CourseActionsBar.tsx (1)
5-17:⚠️ Potential issue | 🟠 Major
courseId를 props로 전달하여 동적 라우팅을 적용해야 합니다.
/admin/units/3으로 하드코딩된 강의 ID는 실제 강의 ID와 맞지 않습니다.courseData에id필드가 존재하지만CourseActionsBar로 전달되지 않고 있어, 어떤 강의를 보더라도 항상 ID 3의 단원 편집 페이지로 이동하게 됩니다. 또한CourseContent의EmptyCourse경로는courseData에 접근할 수 없어 더욱 문제가 됩니다.
CourseOverviewPage에서courseData.id를 props로 전달하고, 각 부모 컴포넌트(CourseHero,CourseContent)가 이를CourseActionsBar에 넘기도록 수정해야 합니다.🔧 수정 방안
// CourseOverviewPage.tsx -<CourseHero - courseData={courseData} - assignmentCount={totalAssignmentCount} - isActiveCourse={hasUnits} -/> -<CourseContent units={courseData.units} isActiveCourse={hasUnits} /> +<CourseHero + courseData={courseData} + assignmentCount={totalAssignmentCount} + isActiveCourse={hasUnits} + courseId={courseData.id} +/> +<CourseContent units={courseData.units} isActiveCourse={hasUnits} courseId={courseData.id} />// CourseHero.tsx interface CourseHeroProps { courseData: Omit<CourseOverview, 'units'>; assignmentCount: number; isActiveCourse: boolean; + courseId: number; } const CourseHero = ({ courseData, assignmentCount, isActiveCourse, + courseId, }: CourseHeroProps) => { // ... <CourseActionsBar isActiveCourse={true} courseId={courseId} />// CourseActionsBar.tsx -const CourseActionsBar = ({isActiveCourse}: {isActiveCourse: boolean}) => { +const CourseActionsBar = ({isActiveCourse, courseId}: {isActiveCourse: boolean; courseId: number}) => { return ( <article className='flex gap-5'> <Link to=''> <Button color={isActiveCourse ? 'outlineWhite' : 'outlinePurple'}> 학생 목록 </Button> </Link> - <Link to={`/admin/units/3`}> + <Link to={`/admin/units/${courseId}`}> <Button color={isActiveCourse ? 'outlinePurple' : 'primary'}> 단원 추가 </Button> </Link> </article> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/course-overview/ui/CourseActionsBar.tsx` around lines 5 - 17, CourseActionsBar currently hardcodes `/admin/units/3`; change CourseActionsBar to accept a courseId prop (e.g., ({ courseId, isActiveCourse })) and use it to build the unit link (`/admin/units/${courseId}`), then update CourseOverviewPage to pass courseData.id down to its children and ensure CourseHero and CourseContent forward that prop into CourseActionsBar (also adjust EmptyCourse usages so they receive courseId from courseData.id instead of accessing courseData directly).
♻️ Duplicate comments (6)
src/shared/ui/button/button-styles.ts (1)
1-37: 이전 리뷰에서 제안된buttonStyles분리가 잘 적용되었습니다! 🎉
react-refresh/only-export-componentsESLint 경고를 해결하기 위해 스타일 정의를 별도 파일로 분리한 점이 좋습니다.type키워드를 사용한VariantPropsimport과ButtonVariantstype export도 코딩 가이드라인에 부합합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/button/button-styles.ts` around lines 1 - 37, Duplicate review comment detected for the approval of buttonStyles extraction; remove the redundant comment and keep a single approval. Locate the review thread referencing the exported symbols buttonStyles and ButtonVariants (and the import tv/VariantProps) and delete the duplicate comment entry so only one approval remains; ensure no code changes to the tv-based style definition are made and that the remaining comment reflects the original approval.src/entities/unit/api/unitApi.ts (1)
30-37:createUnit엔드포인트가 REST 컨벤션 및 기존 API 패턴과 불일치합니다이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
getAllUnitsByCourseId는 컬렉션 경로(GET /courses/${courseId}/units)를 사용하지만,createUnit은POST /units/${courseId}를 사용하고 있어 비일관적입니다. REST 컨벤션상 새 리소스 생성은 컬렉션 경로에 POST하는 것이 일반적입니다.백엔드 API 스펙을 확인하여 의도된 설계인지 검증 후 수정을 검토해 주세요.
🐛 수정 제안 (백엔드 스펙 확인 후 적용)
export const createUnit = async ( courseId: number, unit: TUnitFormSchema ): Promise<ApiResponse<Unit>> => { - const response = await privateAxios.post(`/units/${courseId}`, unit); + const response = await privateAxios.post(`/courses/${courseId}/units`, unit); return response.data; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/api/unitApi.ts` around lines 30 - 37, The createUnit function uses an inconsistent endpoint (POST /units/${courseId}) compared to getAllUnitsByCourseId (GET /courses/${courseId}/units); change createUnit to POST to the collection path POST /courses/${courseId}/units to match REST conventions and the existing API pattern, updating the call in createUnit accordingly, and if the backend spec intentionally uses the current route verify with the backend and document that exception in code/comments; reference the createUnit function and getAllUnitsByCourseId for locating the change.src/pages/unit-editor/ui/UnitForm.tsx (3)
24-24:⚠️ Potential issue | 🔴 Critical🚨 빌드 실패 중! 미사용
assignmentIds상태 변수 즉시 제거 필요이전 리뷰에서 제기되었으나 아직 수정되지 않았으며, 현재 CI 파이프라인이 이 라인으로 인해 빌드 실패 상태입니다.
assignmentIds와setAssignmentIds모두 선언 후 전혀 사용되지 않습니다.추후
UnitAssignmentList와 연동 시 다시 추가하면 되므로, 우선 제거하여 빌드를 복구하는 것을 권장합니다.🐛 즉시 수정 (빌드 복구)
- const [assignmentIds, setAssignmentIds] = useState<number[]>([]); + // TODO: UnitAssignmentList 연동 후 assignmentIds 관리 구현 예정🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` at line 24, Remove the unused React state declaration assignmentIds and its setter setAssignmentIds from UnitForm (the const [assignmentIds, setAssignmentIds] = useState<number[]>([]); line) to restore the build; after removing that line, also remove any now-unused React imports (e.g., useState) from the top of UnitForm.tsx and verify there are no remaining references to assignmentIds or setAssignmentIds elsewhere in the file so the code compiles cleanly.
31-46:values옵션 사용 시 React Query 리페치 시점에 사용자 편집 내용이 손실됩니다이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
react-hook-form의values옵션은 값이 변경될 때마다 폼을 리셋하므로, 백그라운드 리페치가 발생하면 사용자가 입력 중인 내용이 유실됩니다.또한
editing모드에서assignmentIds가 누락되어 있어creating모드와 일관성이 없습니다.
defaultValues+useEffect+reset()패턴으로 전환하는 것을 권장합니다. (react-hook-form 공식 문서 — values)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 31 - 46, The current useForm call uses the values option which resets the form on every change and causes user input loss on React Query refetch; change to using defaultValues in useForm (resolver: zodResolver(unitFormSchema), defaultValues: { title: '', releaseDate: '', dueDate: '', assignmentIds: [] } or populated from unit when mode !== 'creating') and add a useEffect that calls reset(...) (from useForm) whenever the fetched unit data changes so the form initializes/updates without clobbering in-progress edits; also ensure assignmentIds is included for the editing/mode !== 'creating' branch so both modes have consistent shape.
93-118:title,releaseDate필드의 유효성 검사 에러 메시지가 UI에 표시되지 않습니다이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다.
errors.dueDate?.message는 표시되지만,errors.title?.message와errors.releaseDate?.message는 렌더링되지 않아 제출 실패 시 사용자가 원인을 알 수 없습니다.🐛 수정 제안
<section className='grid grid-cols-2 gap-5.5'> <LabeledInput {...register('title')} label='제목' placeholder='단원 제목을 입력하세요' /> + {errors.title && ( + <span className='col-span-2 text-xs text-badge-red -mt-3 h-1'> + {errors.title.message} + </span> + )} </section> <section className='grid grid-cols-2 gap-5.5'> <LabeledInput {...register('releaseDate')} label='공개일' type='date' placeholder='날짜를 선택하세요' /> + {errors.releaseDate && ( + <span className='text-xs text-badge-red -mt-3 h-1'> + {errors.releaseDate.message} + </span> + )} <LabeledInput {...register('dueDate')} ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 93 - 118, The form currently only renders validation errors for dueDate; add error display lines for title and releaseDate so users see validation messages: beneath the LabeledInput for title render errors.title?.message in a similar <span> (use same classes as the dueDate span or appropriate positioning inside the same grid column), and beneath the releaseDate LabeledInput render errors.releaseDate?.message; locate the inputs registered via register('title') and register('releaseDate') in UnitForm (symbols: register, errors, LabeledInput, title, releaseDate, dueDate) and add corresponding error spans so each field shows its errors.src/pages/unit-editor/ui/UnitAssignmentList.tsx (1)
30-30:assignmentListprop 변경 시 내부 상태가 동기화되지 않습니다이전 리뷰에서 지적된 내용이 아직 반영되지 않았습니다. 부모 컴포넌트에서 다른 단원을 선택해
assignmentListprop이 변경되어도assignments상태는 초기값을 유지합니다.
useEffect로 동기화하거나, 부모에서keyprop을 전달해 리마운트를 유도하는 방식을 권장합니다.♻️ useEffect 동기화 제안
+import {useEffect, useState} from 'react'; const [assignments, setAssignments] = useState<Assignment[]>(assignmentList); +useEffect(() => { + setAssignments(assignmentList); +}, [assignmentList]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx` at line 30, The local state assignments initialized from the assignmentList prop in UnitAssignmentList doesn't update when assignmentList changes; add a useEffect in UnitAssignmentList that watches assignmentList and calls setAssignments(assignmentList) to resync (or alternatively accept a key prop from the parent to force remount); update references to assignmentList, assignments, and setAssignments to ensure the state tracks prop changes.
🧹 Nitpick comments (6)
src/shared/ui/list-row/ListRow.tsx (1)
20-22: 선택적 아이콘이 없을 때도 빈<div>가 렌더링됩니다.
leftIcon과rightIcon이undefined이면<div>{undefined}</div>형태로 빈 DOM 노드가 생성됩니다. 조건부 렌더링으로 불필요한 DOM을 제거할 수 있습니다.♻️ 개선 제안
- <div>{leftIcon}</div> + {leftIcon && <div>{leftIcon}</div>} <p className='text-base/6 font-normal'>{title}</p> - <div className='ml-auto'>{rightIcon}</div> + {rightIcon && <div className='ml-auto'>{rightIcon}</div>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/list-row/ListRow.tsx` around lines 20 - 22, The ListRow component currently always renders wrapper divs for leftIcon and rightIcon even when they are undefined; update the JSX in ListRow (the elements rendering leftIcon and rightIcon) to render those wrapper divs conditionally (render the leftIcon div only when leftIcon is truthy and the rightIcon div only when rightIcon is truthy) so no empty DOM nodes are produced; keep the title <p> as-is and ensure spacing/layout remains correct when one or both icons are absent.src/pages/common/UserIdInputPage.tsx (1)
124-134: 카카오 로그인 버튼에 공통Button컴포넌트 적용을 고려해보세요Lines 124~134의 카카오 로그인 버튼은 네이티브
<button>태그를 직접 사용하고 있어 프로젝트 전반의 버튼 스타일/동작 일관성에서 벗어나 있습니다. 이 부분은 현재 기능에는 영향 없는 기존 코드이지만, 추후 스타일/접근성 업데이트를 한 곳에서 관리하려면 공통Button컴포넌트를 활용하는 방향을 검토해볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/common/UserIdInputPage.tsx` around lines 124 - 134, Replace the native <button> in UserIdInputPage with the project's shared Button component: use the Button component (import if missing) and pass disabled={kakaoEnabled ? false : true} or disabled={ !kakaoEnabled }, the onClick handler handleKakaoLogin, and existing styling via the Button's className/variant props so the kakaoLogo <img> and "카카오 로그인" remain as children; ensure the Button preserves accessibility attributes and the same visual behavior (disabled appearance and cursor) as the original element.src/shared/ui/button/Button.tsx (1)
4-13:ButtonProps타입을 외부에 export하면 재사용성이 높아집니다현재
ButtonProps가 파일 내부에서만 사용되도록 정의되어 있어, 소비하는 컴포넌트에서 타입 확장이나 명시적 타입 지정이 불가능합니다. export하면 예를 들어Wrapper컴포넌트에서ButtonProps를 그대로 확장해 사용할 수 있습니다.♻️ 수정 제안
- interface ButtonProps extends ButtonVariants { + export interface ButtonProps extends ButtonVariants {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/button/Button.tsx` around lines 4 - 13, Export the ButtonProps interface so consumers can reuse and extend the button type: change the local interface declaration for ButtonProps to an exported declaration (export interface ButtonProps extends ButtonVariants { ... }) and ensure ButtonVariants is exported or re-exported if it isn't already; update any existing exports (e.g., export the Button component) so type consumers can import both ButtonProps and ButtonVariants from this module and adjust any internal references to use the new exported names.src/pages/course-overview/ui/CourseActionsBar.tsx (1)
8-8:학생 목록링크의to값이 빈 문자열입니다.기존 코드이지만, 빈 문자열
to=''는 현재 페이지로 리다이렉트되어 사실상 아무 동작도 하지 않습니다. 추후 학생 목록 페이지 라우트가 추가되면 함께 업데이트가 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/course-overview/ui/CourseActionsBar.tsx` at line 8, Link 컴포넌트의 to prop이 빈 문자열로 설정되어 있어(링크 텍스트 '학생 목록' in CourseActionsBar / <Link to=''>) 현재 페이지로 리다이렉트되는 무효한 링크입니다; to에 올바른 라우트 경로(예: 학생 목록 라우트의 경로 문자열 또는 route helper 상수)를 넣어 Link의 to 값을 수정하고, 아직 라우트가 미정이면 임시로 명확한 TODO 주석과 함께 적절한 경로 상수(예: STUDENT_LIST_ROUTE)로 교체하세요.src/shared/ui/badge/badge-styles.ts (1)
17-27:/lite빌드에서extend+ 새base사용 시 충돌 클래스 공존 주의
submissionBadgeStyles는badgeStyles를extend하면서 동시에 새base를 정의하고 있습니다./lite빌드는twMerge를 지원하지 않기 때문에 부모 base의text-base와 자식 base의text-sm이 모두 출력 클래스 문자열에 포함됩니다. CSS 캐스케이드 규칙(나중에 선언된 클래스 우선)에 의해 현재는text-sm이 적용되지만, 이는 암묵적인 의존이므로 명시적으로 처리하는 것이 더 안전합니다.문서 참고:
/lite빌드는twMerge및twMergeConfig옵션을 지원하지 않으며, 번들 크기가 중요하고 자동 충돌 해결이 필요 없을 때 선택합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/badge/badge-styles.ts` around lines 17 - 27, submissionBadgeStyles currently extends badgeStyles while also redefining base to include a conflicting text utility (text-sm) which causes both text-base (from badgeStyles) and text-sm to be emitted in the /lite build; remove the duplicate text utility from one side so only a single text-* class is produced — e.g., delete text-sm from submissionBadgeStyles.base (keeping 'bg-transparent flex-center gap-2') or alternatively remove text-base from badgeStyles.base so submissionBadgeStyles can safely declare text-sm; update whichever one you change (submissionBadgeStyles or badgeStyles) to ensure only one text-* utility remains.src/entities/unit/api/unitQueries.ts (1)
1-19: 쿼리 옵션 구조가 깔끔하게 정리되었네요! 🎉
getUnitDetails는enabled: !!unitId가드가 잘 적용되어 있습니다.getUnitList에도 일관성 차원에서 동일한 가드를 추가하는 것을 고려해볼 수 있습니다.useParams에서 파싱된courseId가0이나NaN일 경우 잘못된 엔드포인트로 요청이 전송될 수 있습니다.✨ 선택적 개선 제안
getUnitList: (courseId: number) => queryOptions({ queryKey: ['units', courseId], queryFn: () => getAllUnitsByCourseId(courseId), + enabled: !!courseId, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/api/unitQueries.ts` around lines 1 - 19, Add a guard to the getUnitList query so it won't fire with an invalid courseId: update the getUnitList factory (which calls queryOptions(... queryKey: ['units', courseId], queryFn: () => getAllUnitsByCourseId(courseId))) to include an enabled: !!courseId (and optionally Number.isFinite(courseId) or !Number.isNaN(courseId)) check so requests are only sent when courseId is a valid non-zero number.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx`:
- Around line 84-102: DraggableAssignmentItem currently renders DeleteIcon
without any click handler; add an onDelete prop to DraggableAssignmentItem and
invoke it when the delete control is clicked (attach to DeleteIcon's onClick and
stop event propagation to avoid drag interference), then update
UnitAssignmentList to pass a callback that calls setAssignments to remove the
item by id; refer to DraggableAssignmentItem, DeleteIcon, ListRow,
UnitAssignmentList and setAssignments to locate where to add the prop and
callback.
In `@src/shared/ui/badge/Badge.tsx`:
- Around line 18-20: SubmissionBadgeProps currently inherits
SubmissionBadgeVariants (from VariantProps<typeof submissionBadgeStyles>) which
makes status optional, forcing the unsafe non-null assertion props.status!
later; update SubmissionBadgeProps to explicitly require status (e.g., declare
status: 'CORRECT' | 'INCORRECT' | 'NOT_SUBMITTED') so callers and the component
(where props.status is used) are type-safe and you can remove the non-null
assertion; locate the type alias SubmissionBadgeProps and adjust its definition
to make status required while keeping the existing variant typing.
In `@src/shared/ui/button/button-styles.ts`:
- Line 20: The 'none' size variant in button-styles (symbol: none) removed
'w-fit', which breaks layout for components using size='none' in UserIdInputPage
and LandingPage (UnitForm is safe); either restore 'w-fit' to the none variant
in button-styles.ts or update the two components (UserIdInputPage.tsx and
LandingPage.tsx) to add an explicit width utility (e.g., 'w-fit' or 'w-auto' or
a fixed w- class) or switch them to an existing variant like size='compact'—pick
one approach and apply consistently so the flex parent gap/layout remains
correct.
In `@src/shared/ui/button/Button.tsx`:
- Around line 10-32: ButtonProps declares onMouseEnter/onMouseLeave but the
Button component never destructures them so they get swallowed into ...props and
are only passed into buttonStyles(props) (which ignores event handlers), leaving
the DOM <button> without those handlers; fix by destructuring onMouseEnter and
onMouseLeave from the Button component props (alongside children, onClick, type,
disabled, formID, className), pass the remaining sanitized props into
buttonStyles (e.g., exclude the event handlers when calling buttonStyles) and
explicitly attach onMouseEnter={onMouseEnter} and onMouseLeave={onMouseLeave} to
the <button> element so the handlers are actually bound at runtime.
In `@src/shared/ui/list-row/ListRow.tsx`:
- Around line 1-9: Import the React namespace as a type-only import and remove
the duplicate selected prop from ListRowProps: add "import type React from
'react'" (or the equivalent type-only React import) so React.ReactNode is in
scope for leftIcon/rightIcon, and delete the redundant "selected?: boolean"
since ListRowProps already extends ListRowVariants; update the ListRowProps
interface to reference React.ReactNode without introducing a runtime React
import.
---
Outside diff comments:
In `@src/pages/course-overview/ui/CourseActionsBar.tsx`:
- Around line 5-17: CourseActionsBar currently hardcodes `/admin/units/3`;
change CourseActionsBar to accept a courseId prop (e.g., ({ courseId,
isActiveCourse })) and use it to build the unit link
(`/admin/units/${courseId}`), then update CourseOverviewPage to pass
courseData.id down to its children and ensure CourseHero and CourseContent
forward that prop into CourseActionsBar (also adjust EmptyCourse usages so they
receive courseId from courseData.id instead of accessing courseData directly).
---
Duplicate comments:
In `@src/entities/unit/api/unitApi.ts`:
- Around line 30-37: The createUnit function uses an inconsistent endpoint (POST
/units/${courseId}) compared to getAllUnitsByCourseId (GET
/courses/${courseId}/units); change createUnit to POST to the collection path
POST /courses/${courseId}/units to match REST conventions and the existing API
pattern, updating the call in createUnit accordingly, and if the backend spec
intentionally uses the current route verify with the backend and document that
exception in code/comments; reference the createUnit function and
getAllUnitsByCourseId for locating the change.
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx`:
- Line 30: The local state assignments initialized from the assignmentList prop
in UnitAssignmentList doesn't update when assignmentList changes; add a
useEffect in UnitAssignmentList that watches assignmentList and calls
setAssignments(assignmentList) to resync (or alternatively accept a key prop
from the parent to force remount); update references to assignmentList,
assignments, and setAssignments to ensure the state tracks prop changes.
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Line 24: Remove the unused React state declaration assignmentIds and its
setter setAssignmentIds from UnitForm (the const [assignmentIds,
setAssignmentIds] = useState<number[]>([]); line) to restore the build; after
removing that line, also remove any now-unused React imports (e.g., useState)
from the top of UnitForm.tsx and verify there are no remaining references to
assignmentIds or setAssignmentIds elsewhere in the file so the code compiles
cleanly.
- Around line 31-46: The current useForm call uses the values option which
resets the form on every change and causes user input loss on React Query
refetch; change to using defaultValues in useForm (resolver:
zodResolver(unitFormSchema), defaultValues: { title: '', releaseDate: '',
dueDate: '', assignmentIds: [] } or populated from unit when mode !==
'creating') and add a useEffect that calls reset(...) (from useForm) whenever
the fetched unit data changes so the form initializes/updates without clobbering
in-progress edits; also ensure assignmentIds is included for the editing/mode
!== 'creating' branch so both modes have consistent shape.
- Around line 93-118: The form currently only renders validation errors for
dueDate; add error display lines for title and releaseDate so users see
validation messages: beneath the LabeledInput for title render
errors.title?.message in a similar <span> (use same classes as the dueDate span
or appropriate positioning inside the same grid column), and beneath the
releaseDate LabeledInput render errors.releaseDate?.message; locate the inputs
registered via register('title') and register('releaseDate') in UnitForm
(symbols: register, errors, LabeledInput, title, releaseDate, dueDate) and add
corresponding error spans so each field shows its errors.
In `@src/shared/ui/button/button-styles.ts`:
- Around line 1-37: Duplicate review comment detected for the approval of
buttonStyles extraction; remove the redundant comment and keep a single
approval. Locate the review thread referencing the exported symbols buttonStyles
and ButtonVariants (and the import tv/VariantProps) and delete the duplicate
comment entry so only one approval remains; ensure no code changes to the
tv-based style definition are made and that the remaining comment reflects the
original approval.
---
Nitpick comments:
In `@src/entities/unit/api/unitQueries.ts`:
- Around line 1-19: Add a guard to the getUnitList query so it won't fire with
an invalid courseId: update the getUnitList factory (which calls
queryOptions(... queryKey: ['units', courseId], queryFn: () =>
getAllUnitsByCourseId(courseId))) to include an enabled: !!courseId (and
optionally Number.isFinite(courseId) or !Number.isNaN(courseId)) check so
requests are only sent when courseId is a valid non-zero number.
In `@src/pages/common/UserIdInputPage.tsx`:
- Around line 124-134: Replace the native <button> in UserIdInputPage with the
project's shared Button component: use the Button component (import if missing)
and pass disabled={kakaoEnabled ? false : true} or disabled={ !kakaoEnabled },
the onClick handler handleKakaoLogin, and existing styling via the Button's
className/variant props so the kakaoLogo <img> and "카카오 로그인" remain as children;
ensure the Button preserves accessibility attributes and the same visual
behavior (disabled appearance and cursor) as the original element.
In `@src/pages/course-overview/ui/CourseActionsBar.tsx`:
- Line 8: Link 컴포넌트의 to prop이 빈 문자열로 설정되어 있어(링크 텍스트 '학생 목록' in CourseActionsBar
/ <Link to=''>) 현재 페이지로 리다이렉트되는 무효한 링크입니다; to에 올바른 라우트 경로(예: 학생 목록 라우트의 경로 문자열
또는 route helper 상수)를 넣어 Link의 to 값을 수정하고, 아직 라우트가 미정이면 임시로 명확한 TODO 주석과 함께 적절한
경로 상수(예: STUDENT_LIST_ROUTE)로 교체하세요.
In `@src/shared/ui/badge/badge-styles.ts`:
- Around line 17-27: submissionBadgeStyles currently extends badgeStyles while
also redefining base to include a conflicting text utility (text-sm) which
causes both text-base (from badgeStyles) and text-sm to be emitted in the /lite
build; remove the duplicate text utility from one side so only a single text-*
class is produced — e.g., delete text-sm from submissionBadgeStyles.base
(keeping 'bg-transparent flex-center gap-2') or alternatively remove text-base
from badgeStyles.base so submissionBadgeStyles can safely declare text-sm;
update whichever one you change (submissionBadgeStyles or badgeStyles) to ensure
only one text-* utility remains.
In `@src/shared/ui/button/Button.tsx`:
- Around line 4-13: Export the ButtonProps interface so consumers can reuse and
extend the button type: change the local interface declaration for ButtonProps
to an exported declaration (export interface ButtonProps extends ButtonVariants
{ ... }) and ensure ButtonVariants is exported or re-exported if it isn't
already; update any existing exports (e.g., export the Button component) so type
consumers can import both ButtonProps and ButtonVariants from this module and
adjust any internal references to use the new exported names.
In `@src/shared/ui/list-row/ListRow.tsx`:
- Around line 20-22: The ListRow component currently always renders wrapper divs
for leftIcon and rightIcon even when they are undefined; update the JSX in
ListRow (the elements rendering leftIcon and rightIcon) to render those wrapper
divs conditionally (render the leftIcon div only when leftIcon is truthy and the
rightIcon div only when rightIcon is truthy) so no empty DOM nodes are produced;
keep the title <p> as-is and ensure spacing/layout remains correct when one or
both icons are absent.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
src/App.tsxsrc/entities/student/ui/StudentProfile.tsxsrc/entities/student/ui/StudentTable.tsxsrc/entities/unit/api/unitApi.tssrc/entities/unit/api/unitQueries.tssrc/entities/unit/model/types.tssrc/pages/admin/assignments/AssignmentCreatePage.tsxsrc/pages/common/LandingPage.tsxsrc/pages/common/UserIdInputPage.tsxsrc/pages/course-overview/ui/AssignmentList.tsxsrc/pages/course-overview/ui/CourseActionsBar.tsxsrc/pages/course-overview/ui/UnitItem.tsxsrc/pages/dashboard/Dashboard.tsxsrc/pages/dashboard/ui/ScheduleCard.tsxsrc/pages/select-assignment/AssignmentSelectPage.tsxsrc/pages/unit-editor/UnitEditorPage.tsxsrc/pages/unit-editor/mock/unitMock.tssrc/pages/unit-editor/model/types.tssrc/pages/unit-editor/ui/UnitAssignmentList.tsxsrc/pages/unit-editor/ui/UnitForm.tsxsrc/pages/unit-editor/ui/UnitList.tsxsrc/shared/ui/ActionButton.tsxsrc/shared/ui/Header.tsxsrc/shared/ui/IconButton.tsxsrc/shared/ui/SelectableItem.tsxsrc/shared/ui/badge/Badge.tsxsrc/shared/ui/badge/badge-styles.tssrc/shared/ui/button/Button.tsxsrc/shared/ui/button/button-styles.tssrc/shared/ui/checkbox/Checkbox.module.csssrc/shared/ui/checkbox/Checkbox.tsxsrc/shared/ui/list-row/ListRow.tsxsrc/shared/ui/list-row/list-row-styles.tssrc/widgets/assignment-form-layout/ui/AssignmentFormLayout.tsxsrc/widgets/assignment-page-layout/ui/AssignmentPageLayout.tsx
💤 Files with no reviewable changes (3)
- src/shared/ui/ActionButton.tsx
- src/shared/ui/IconButton.tsx
- src/shared/ui/SelectableItem.tsx
✅ Files skipped from review due to trivial changes (1)
- src/pages/course-overview/ui/AssignmentList.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/pages/unit-editor/model/types.ts
- src/entities/unit/model/types.ts
- src/pages/unit-editor/mock/unitMock.ts
- src/pages/unit-editor/UnitEditorPage.tsx
- src/pages/unit-editor/ui/UnitList.tsx
- src/App.tsx
Comment on lines
+84
to
+102
| const DraggableAssignmentItem = ({id, title}: Assignment) => { | ||
| const {attributes, listeners, setNodeRef, transform, transition, isDragging} = | ||
| useSortable({id}); | ||
|
|
||
| const style = { | ||
| transform: CSS.Transform.toString(transform), | ||
| transition, | ||
| }; | ||
|
|
||
| return ( | ||
| <li ref={setNodeRef} {...attributes} {...listeners} style={style}> | ||
| <ListRow | ||
| title={title} | ||
| leftIcon={<DragAndDropIcon />} | ||
| rightIcon={<DeleteIcon className='w-3 h-3' />} | ||
| className={`cursor-grab touch-none bg-white shadow-box active:cursor-grabbing ${isDragging ? 'z-10 opacity-50' : ''}`} | ||
| /> | ||
| </li> | ||
| ); |
There was a problem hiding this comment.
삭제 버튼(DeleteIcon)에 onClick 핸들러가 없어 실제로 동작하지 않습니다
DraggableAssignmentItem은 DeleteIcon을 렌더링하지만 삭제 이벤트를 처리하는 콜백이 없습니다. 사용자가 삭제 아이콘을 클릭해도 아무 반응이 없어 UX 혼란을 줍니다.
DraggableAssignmentItem에 onDelete prop을 추가하고, UnitAssignmentList에서 setAssignments를 통해 목록을 갱신하는 콜백을 전달해야 합니다.
🐛 수정 제안
-const DraggableAssignmentItem = ({id, title}: Assignment) => {
+const DraggableAssignmentItem = ({
+ id,
+ title,
+ onDelete,
+}: Assignment & {onDelete: (id: number) => void}) => {
const {attributes, listeners, setNodeRef, transform, transition, isDragging} =
useSortable({id});
const style = {
transform: CSS.Transform.toString(transform),
transition,
};
return (
<li ref={setNodeRef} {...attributes} {...listeners} style={style}>
<ListRow
title={title}
leftIcon={<DragAndDropIcon />}
- rightIcon={<DeleteIcon className='w-3 h-3' />}
+ rightIcon={
+ <DeleteIcon
+ className='w-3 h-3'
+ onClick={(e) => {
+ e.stopPropagation();
+ onDelete(id);
+ }}
+ />
+ }
className={`cursor-grab touch-none bg-white shadow-box active:cursor-grabbing ${isDragging ? 'z-10 opacity-50' : ''}`}
/>
</li>
);
};UnitAssignmentList에서 호출:
-<DraggableAssignmentItem key={id} id={id} title={title} />
+<DraggableAssignmentItem
+ key={id}
+ id={id}
+ title={title}
+ onDelete={(deletedId) =>
+ setAssignments((prev) => prev.filter((a) => a.id !== deletedId))
+ }
+/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/unit-editor/ui/UnitAssignmentList.tsx` around lines 84 - 102,
DraggableAssignmentItem currently renders DeleteIcon without any click handler;
add an onDelete prop to DraggableAssignmentItem and invoke it when the delete
control is clicked (attach to DeleteIcon's onClick and stop event propagation to
avoid drag interference), then update UnitAssignmentList to pass a callback that
calls setAssignments to remove the item by id; refer to DraggableAssignmentItem,
DeleteIcon, ListRow, UnitAssignmentList and setAssignments to locate where to
add the prop and callback.
Comment on lines
18
to
+20
| type SubmissionBadgeProps = { | ||
| variant: 'submission'; | ||
| } & VariantProps<typeof submissionBadgeStyles>; | ||
| } & SubmissionBadgeVariants; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
SubmissionBadgeProps에서 status를 필수(required)로 선언해야 합니다
SubmissionBadgeVariants는 VariantProps<typeof submissionBadgeStyles>로, status가 optional('CORRECT' | 'INCORRECT' | 'NOT_SUBMITTED' | undefined)로 추론됩니다. 이로 인해 Line 70의 props.status! 비널 단언이 필요해집니다. status가 실제로는 항상 필요한 값이므로 SubmissionBadgeProps에서 명시적으로 required로 재선언하면 타입 안전성이 높아집니다.
🛡️ 제안 수정
type SubmissionBadgeProps = {
variant: 'submission';
-} & SubmissionBadgeVariants;
+ status: NonNullable<SubmissionBadgeVariants['status']>;
+};이렇게 하면 Line 70의 props.status! 단언도 props.status로 안전하게 변경할 수 있습니다:
- const {label, icon} = SubmissionMeta[props.status!];
+ const {label, icon} = SubmissionMeta[props.status];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/badge/Badge.tsx` around lines 18 - 20, SubmissionBadgeProps
currently inherits SubmissionBadgeVariants (from VariantProps<typeof
submissionBadgeStyles>) which makes status optional, forcing the unsafe non-null
assertion props.status! later; update SubmissionBadgeProps to explicitly require
status (e.g., declare status: 'CORRECT' | 'INCORRECT' | 'NOT_SUBMITTED') so
callers and the component (where props.status is used) are type-safe and you can
remove the non-null assertion; locate the type alias SubmissionBadgeProps and
adjust its definition to make status required while keeping the existing variant
typing.
| compact: 'w-fit px-3 py-1.5 leading-5', | ||
| wide: 'w-40 py-[15px]', | ||
| none: 'w-fit p-0', | ||
| none: 'p-0', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the button-styles.ts file to see the full context
cat -n src/shared/ui/button/button-styles.tsRepository: 2025-snowCode/snowCode-Client
Length of output: 1582
🏁 Script executed:
# Search for size='none' or size: 'none' usages in the codebase
echo "=== Searching for size='none' usages ==="
rg -n "size\s*[=:]\s*['\"]none['\"]" --type=ts --type=tsx -C 3Repository: 2025-snowCode/snowCode-Client
Length of output: 158
🏁 Script executed:
# Also search for button component usages with size prop
echo "=== Searching for Button components with size prop ==="
rg -n "Button[^a-zA-Z].*size" --type=ts --type=tsx -C 2 | head -50Repository: 2025-snowCode/snowCode-Client
Length of output: 172
🏁 Script executed:
# Search for size='none' without file type specification
echo "=== Searching for size='none' usages ==="
rg -n "size\s*[=:]\s*['\"]none['\"]" -C 3Repository: 2025-snowCode/snowCode-Client
Length of output: 1555
🏁 Script executed:
# Search for Button component usage in general
echo "=== Searching for Button imports ==="
rg -n "from.*button" -C 2Repository: 2025-snowCode/snowCode-Client
Length of output: 4691
🏁 Script executed:
# Look for any references to 'none' in the same context as Button or size
echo "=== Searching for 'none' in ui components ==="
rg -n "size.*none|none.*size" src/Repository: 2025-snowCode/snowCode-Client
Length of output: 303
🏁 Script executed:
# Check UserIdInputPage context around the button
sed -n '70,90p' src/pages/common/UserIdInputPage.tsxRepository: 2025-snowCode/snowCode-Client
Length of output: 812
🏁 Script executed:
# Check LandingPage context around the button
sed -n '35,55p' src/pages/common/LandingPage.tsxRepository: 2025-snowCode/snowCode-Client
Length of output: 734
🏁 Script executed:
# Check UnitForm context to confirm w-9 h-9 usage
sed -n '78,90p' src/pages/unit-editor/ui/UnitForm.tsxRepository: 2025-snowCode/snowCode-Client
Length of output: 550
none 사이즈 변형에서 w-fit 제거 — 2개 컴포넌트의 레이아웃 영향 확인 필요
size='none'은 현재 3개 컴포넌트에서 사용 중입니다:
- UnitForm.tsx (라인 84):
className='w-9 h-9 rounded-full'→ 명시적 너비/높이 지정으로 영향 없음 ✓ - UserIdInputPage.tsx (라인 81):
className='leading-7 text-lg'→ w-fit 제거로 버튼이 부모의 flex 컨테이너(gap-4)에서 의도대로 크기 조정되지 않을 수 있음 - LandingPage.tsx (라인 44):
className='leading-7 text-lg'→ 동일하게 영향받음
권장사항:
- UserIdInputPage와 LandingPage의 버튼은
w-fit복원이 필요하거나, 각 className에 명시적 너비 값 추가 (w-auto등) - 아니면
size='compact'같은 기존 변형 사용 고려
변경 의도가 불명확하므로, 이 2개 컴포넌트에서 레이아웃이 정상 작동하는지 확인 후 병합 부탁드립니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/button/button-styles.ts` at line 20, The 'none' size variant in
button-styles (symbol: none) removed 'w-fit', which breaks layout for components
using size='none' in UserIdInputPage and LandingPage (UnitForm is safe); either
restore 'w-fit' to the none variant in button-styles.ts or update the two
components (UserIdInputPage.tsx and LandingPage.tsx) to add an explicit width
utility (e.g., 'w-fit' or 'w-auto' or a fixed w- class) or switch them to an
existing variant like size='compact'—pick one approach and apply consistently so
the flex parent gap/layout remains correct.
Comment on lines
+10
to
+32
| onClick?: () => void; | ||
| onMouseEnter?: () => void; | ||
| onMouseLeave?: () => void; | ||
| } | ||
|
|
||
| const Button = ({ | ||
| children, | ||
| onClick, | ||
| type = 'button', | ||
| disabled = false, | ||
| formID, | ||
| className, | ||
| ...props | ||
| }: ButtonProps) => { | ||
| return ( | ||
| <button | ||
| type={type} | ||
| onClick={onClick} | ||
| disabled={disabled} | ||
| form={formID} | ||
| className={twMerge(buttonStyles(props), className)}> | ||
| {children} | ||
| </button> |
There was a problem hiding this comment.
onMouseEnter / onMouseLeave가 실제 <button>에 전달되지 않습니다 — 기능 버그
ButtonProps에 onMouseEnter, onMouseLeave를 선언했지만, 컴포넌트 내부에서 명시적으로 구조분해하지 않아 ...props에 섞인 채 buttonStyles(props)(스타일 함수)로만 전달됩니다. tailwind-variants의 tv()는 이 두 키를 무시하므로 DOM의 <button>에는 이벤트 핸들러가 전혀 바인딩되지 않습니다.
실제 피해: LandingPage.tsx에서 onMouseEnter/onMouseLeave로 호버 이미지 전환 로직을 구현하고 있어, 이 버그로 인해 그 기능이 완전히 동작하지 않습니다.
🐛 수정 제안
const Button = ({
children,
onClick,
type = 'button',
disabled = false,
formID,
className,
+ onMouseEnter,
+ onMouseLeave,
...props
}: ButtonProps) => {
return (
<button
type={type}
onClick={onClick}
disabled={disabled}
form={formID}
+ onMouseEnter={onMouseEnter}
+ onMouseLeave={onMouseLeave}
className={twMerge(buttonStyles(props), className)}>
{children}
</button>
);
};📝 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
| onClick?: () => void; | |
| onMouseEnter?: () => void; | |
| onMouseLeave?: () => void; | |
| } | |
| const Button = ({ | |
| children, | |
| onClick, | |
| type = 'button', | |
| disabled = false, | |
| formID, | |
| className, | |
| ...props | |
| }: ButtonProps) => { | |
| return ( | |
| <button | |
| type={type} | |
| onClick={onClick} | |
| disabled={disabled} | |
| form={formID} | |
| className={twMerge(buttonStyles(props), className)}> | |
| {children} | |
| </button> | |
| onClick?: () => void; | |
| onMouseEnter?: () => void; | |
| onMouseLeave?: () => void; | |
| } | |
| const Button = ({ | |
| children, | |
| onClick, | |
| type = 'button', | |
| disabled = false, | |
| formID, | |
| className, | |
| onMouseEnter, | |
| onMouseLeave, | |
| ...props | |
| }: ButtonProps) => { | |
| return ( | |
| <button | |
| type={type} | |
| onClick={onClick} | |
| disabled={disabled} | |
| form={formID} | |
| onMouseEnter={onMouseEnter} | |
| onMouseLeave={onMouseLeave} | |
| className={twMerge(buttonStyles(props), className)}> | |
| {children} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/button/Button.tsx` around lines 10 - 32, ButtonProps declares
onMouseEnter/onMouseLeave but the Button component never destructures them so
they get swallowed into ...props and are only passed into buttonStyles(props)
(which ignores event handlers), leaving the DOM <button> without those handlers;
fix by destructuring onMouseEnter and onMouseLeave from the Button component
props (alongside children, onClick, type, disabled, formID, className), pass the
remaining sanitized props into buttonStyles (e.g., exclude the event handlers
when calling buttonStyles) and explicitly attach onMouseEnter={onMouseEnter} and
onMouseLeave={onMouseLeave} to the <button> element so the handlers are actually
bound at runtime.
Comment on lines
+1
to
+9
| import {ListRowStyles, type ListRowVariants} from './list-row-styles'; | ||
|
|
||
| interface ListRowProps extends ListRowVariants { | ||
| selected?: boolean; | ||
| leftIcon?: React.ReactNode; | ||
| title: string; | ||
| rightIcon?: React.ReactNode; | ||
| className?: string; | ||
| } |
There was a problem hiding this comment.
React.ReactNode 사용 시 React 임포트가 누락되어 TypeScript 컴파일 오류가 발생합니다.
React.ReactNode를 타입으로 사용하기 위해서는 React가 스코프에 있어야 합니다. 새 JSX 변환(react-jsx)을 사용하더라도, React 네임스페이스 타입은 자동으로 주입되지 않습니다. 코딩 가이드라인의 "type-only import 적극 사용" 원칙에 따라 아래와 같이 수정해 주세요.
추가로, selected?: boolean은 extends ListRowVariants를 통해 이미 상속되므로 중복 선언입니다.
🐛 수정 제안
+import type {ReactNode} from 'react';
import {ListRowStyles, type ListRowVariants} from './list-row-styles';
interface ListRowProps extends ListRowVariants {
- selected?: boolean;
- leftIcon?: React.ReactNode;
+ leftIcon?: ReactNode;
title: string;
- rightIcon?: React.ReactNode;
+ rightIcon?: ReactNode;
className?: string;
}As per coding guidelines: "type-only import 적극 사용"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/list-row/ListRow.tsx` around lines 1 - 9, Import the React
namespace as a type-only import and remove the duplicate selected prop from
ListRowProps: add "import type React from 'react'" (or the equivalent type-only
React import) so React.ReactNode is in scope for leftIcon/rightIcon, and delete
the redundant "selected?: boolean" since ListRowProps already extends
ListRowVariants; update the ListRowProps interface to reference React.ReactNode
without introducing a runtime React import.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/entities/unit/api/unitApi.ts (2)
30-37:⚠️ Potential issue | 🟠 Major[미해결]
createUnit엔드포인트가 REST 컨벤션 및 나머지 Unit API와 불일치합니다이전 리뷰에서 이미 지적된 문제이나 아직 수정되지 않았습니다. 현재
POST /units/${courseId}는 컬렉션에 POST하는 REST 컨벤션과 맞지 않으며,getAllUnitsByCourseId의GET /courses/${courseId}/units패턴과도 구조적으로 불일치합니다.백엔드 스펙을 확인 후 의도된 설계가 아니라면
POST /courses/${courseId}/units로 맞춰주세요.🔧 제안 수정 (백엔드 스펙 확인 후 적용)
- const response = await privateAxios.post(`/units/${courseId}`, unit); + const response = await privateAxios.post(`/courses/${courseId}/units`, unit);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/api/unitApi.ts` around lines 30 - 37, The createUnit function currently posts to `/units/${courseId}` which breaks REST conventions and is inconsistent with getAllUnitsByCourseId (which uses `GET /courses/${courseId}/units`); update the endpoint used in createUnit to `POST /courses/${courseId}/units` so it matches the backend spec and the other Unit APIs (modify the URL string in the createUnit implementation accordingly).
15-20:⚠️ Potential issue | 🟠 Major[미해결]
unitId가null일 때/units/null요청이 발생합니다이전 리뷰에서 이미 지적된 문제이나 아직 수정되지 않았습니다.
number | null타입을 허용하면서 내부에null가드가 없어,null이 전달될 경우/units/null로 잘못된 요청이 전송됩니다.권장 수정 방향: 파라미터 타입을
number로 좁히고 호출부(unitQueries.ts)에서enabled: unitId !== null로 쿼리를 비활성화하세요.🛡️ 제안 수정
-export const getUnitById = async ( - unitId: number | null -): Promise<ApiResponse<Unit>> => { +export const getUnitById = async ( + unitId: number +): Promise<ApiResponse<Unit>> => { const response = await privateAxios.get(`/units/${unitId}`); return response.data; };
unitQueries.ts에서:queryFn: () => getUnitById(unitId!), enabled: unitId !== null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entities/unit/api/unitApi.ts` around lines 15 - 20, getUnitById currently accepts unitId: number | null and will issue a request to /units/null when called with null; change getUnitById to accept only number (narrow the parameter type to number) and update the caller in unitQueries.ts to guard the query by setting enabled: unitId !== null and call getUnitById with the non-null unitId (e.g., using the guaranteed number from the enabled check) so no request to /units/null is made.src/pages/unit-editor/ui/UnitForm.tsx (2)
93-118:⚠️ Potential issue | 🟠 Major[미해결]
title과releaseDate의 유효성 검사 에러 메시지가 표시되지 않습니다이전 리뷰에서 이미 지적된 문제이나 여전히 미해결 상태입니다.
errors.dueDate?.message는 Line 116에 표시되지만,errors.title?.message와errors.releaseDate?.message는 UI에 노출되지 않아 폼 제출 실패 시 사용자가 이유를 알 수 없습니다.🐛 제안 수정
<section className='grid grid-cols-2 gap-5.5'> <LabeledInput {...register('title')} label='제목' placeholder='단원 제목을 입력하세요' /> + {errors.title && ( + <span className='col-span-2 text-xs text-badge-red -mt-3 h-1'> + {errors.title.message} + </span> + )} </section> <section className='grid grid-cols-2 gap-5.5'> <LabeledInput {...register('releaseDate')} label='공개일' type='date' placeholder='날짜를 선택하세요' /> + {errors.releaseDate && ( + <span className='text-xs text-badge-red -mt-3 h-1'> + {errors.releaseDate.message} + </span> + )} <LabeledInput {...register('dueDate')} label='마감일' type='date' placeholder='마감일을 선택하세요' /> <span className='col-span-2 text-xs text-badge-red -mt-3 h-1'> {errors.dueDate?.message} </span> </section>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 93 - 118, The form currently only renders errors.dueDate?.message; add UI elements to display errors.title?.message and errors.releaseDate?.message next to their inputs so validation messages are visible. Specifically, in UnitForm's JSX around the LabeledInput for 'title' (registered via {...register('title')}) add a span (styled like the dueDate error: className='col-span-2 text-xs text-badge-red -mt-3 h-1' or appropriate positioning) that renders {errors.title?.message}; likewise, next to the LabeledInput for 'releaseDate' (registered via {...register('releaseDate')}) add a span that renders {errors.releaseDate?.message} so all three fields show validation errors consistently.
31-46:⚠️ Potential issue | 🟠 Major[미해결]
values옵션 사용 시 사용자 편집 내용 유실 +reset()오작동 위험이전 리뷰에서 이미 지적된 문제이나 여전히 미해결 상태입니다. 추가로
handleCancel의reset()호출 문제도 연관되어 있습니다.
values나defaultValues가 업데이트될 때 내부적으로 reset API가 호출되므로, React Query가 리페치하는 순간 사용자가 편집 중인 내용이 덮어씌워집니다.더불어
handleCancel에서reset()을 인수 없이 호출하는데,defaultValues가resetAPI에 제공되지 않으면 HTML 네이티브 reset API가 호출됩니다. 즉, 편집 모드에서 취소를 누르면 원래 단원 데이터로 복원되는 게 아니라 HTML 기본 reset이 발생합니다.
defaultValues+useEffect/reset(unitData)패턴 사용을 권장합니다:♻️ 제안 수정
const { register, handleSubmit, reset, formState: {errors, isSubmitting}, } = useForm<TUnitFormSchema>({ resolver: zodResolver(unitFormSchema), - values: - mode === 'creating' - ? { title: '', releaseDate: '', dueDate: '', assignmentIds: [] } - : { title: unit?.title || '', releaseDate: unit?.releaseDate || '', dueDate: unit?.dueDate || '' }, + defaultValues: { title: '', releaseDate: '', dueDate: '', assignmentIds: [] }, }); +// 편집 모드 진입 또는 unit 데이터 로드 시 폼 채우기 +useEffect(() => { + if (mode !== 'creating' && unit) { + reset({ + title: unit.title, + releaseDate: unit.releaseDate, + dueDate: unit.dueDate, + assignmentIds: [], + }); + } +}, [mode, unit?.id, reset]);react-hook-form values option resets form on every refetch editing mode🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 31 - 46, Remove the useForm "values" option (which causes automatic resets on refetch) and instead supply "defaultValues" for the creating case and call form.reset(unitData) when loaded; specifically, update useForm(...) (in UnitForm.tsx) to use defaultValues for the new-unit initial shape (title, releaseDate, dueDate, assignmentIds) and for edit-mode provide an empty/default placeholder, then add a useEffect that watches the fetched unit (unit) and calls reset(unit) (or a mapped unitData object matching unitFormSchema) once the data arrives; also change handleCancel to call reset(unit) or reset(mappedUnitData) rather than reset() so cancel restores the original unit values instead of invoking the native HTML reset.
🧹 Nitpick comments (1)
src/pages/unit-editor/ui/UnitForm.tsx (1)
10-10: 주석 처리된import제거를 권장합니다Line 10의
// import {useState} from 'react';는 구현 예정 코드의 흔적입니다. 프로덕션 코드에 주석 처리된 임포트를 남기기보다는 제거하고, 향후 구현 시 다시 추가하는 것이 권장됩니다. Line 24의 주석 처리된 상태 변수도 동일하게 적용됩니다.-// import {useState} from 'react';- // const [assignmentIds, setAssignmentIds] = useState<number[]>([]); + // TODO: SortableAssignmentList 연동 후 assignmentIds 상태 관리 구현 (`#이슈번호`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/unit-editor/ui/UnitForm.tsx` at line 10, Remove the leftover commented-out import and commented state variable in UnitForm.tsx: delete the line containing "// import {useState} from 'react';" and the commented state declaration (the commented useState variable around the top of the UnitForm component) so no commented implementation artifacts remain; re-add the import/state only when actually implementing stateful logic in functions like UnitForm or handlers that reference useState.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Around line 49-56: The onSubmit handler currently falls through to
onCreateUnit when mode === 'editing' but unit is undefined; update the onSubmit
logic to explicitly handle the invalid editing state: check mode first (if mode
=== 'editing') and if unit is missing, short-circuit (return or surface an
error) instead of calling onCreateUnit; otherwise call onUpdateUnit(unit.id,
data). For the create path, only call onCreateUnit when mode indicates creation.
Refer to the onSubmit function and the mode, unit, onUpdateUnit, and
onCreateUnit symbols to locate and change the branching.
---
Duplicate comments:
In `@src/entities/unit/api/unitApi.ts`:
- Around line 30-37: The createUnit function currently posts to
`/units/${courseId}` which breaks REST conventions and is inconsistent with
getAllUnitsByCourseId (which uses `GET /courses/${courseId}/units`); update the
endpoint used in createUnit to `POST /courses/${courseId}/units` so it matches
the backend spec and the other Unit APIs (modify the URL string in the
createUnit implementation accordingly).
- Around line 15-20: getUnitById currently accepts unitId: number | null and
will issue a request to /units/null when called with null; change getUnitById to
accept only number (narrow the parameter type to number) and update the caller
in unitQueries.ts to guard the query by setting enabled: unitId !== null and
call getUnitById with the non-null unitId (e.g., using the guaranteed number
from the enabled check) so no request to /units/null is made.
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Around line 93-118: The form currently only renders errors.dueDate?.message;
add UI elements to display errors.title?.message and errors.releaseDate?.message
next to their inputs so validation messages are visible. Specifically, in
UnitForm's JSX around the LabeledInput for 'title' (registered via
{...register('title')}) add a span (styled like the dueDate error:
className='col-span-2 text-xs text-badge-red -mt-3 h-1' or appropriate
positioning) that renders {errors.title?.message}; likewise, next to the
LabeledInput for 'releaseDate' (registered via {...register('releaseDate')}) add
a span that renders {errors.releaseDate?.message} so all three fields show
validation errors consistently.
- Around line 31-46: Remove the useForm "values" option (which causes automatic
resets on refetch) and instead supply "defaultValues" for the creating case and
call form.reset(unitData) when loaded; specifically, update useForm(...) (in
UnitForm.tsx) to use defaultValues for the new-unit initial shape (title,
releaseDate, dueDate, assignmentIds) and for edit-mode provide an empty/default
placeholder, then add a useEffect that watches the fetched unit (unit) and calls
reset(unit) (or a mapped unitData object matching unitFormSchema) once the data
arrives; also change handleCancel to call reset(unit) or reset(mappedUnitData)
rather than reset() so cancel restores the original unit values instead of
invoking the native HTML reset.
---
Nitpick comments:
In `@src/pages/unit-editor/ui/UnitForm.tsx`:
- Line 10: Remove the leftover commented-out import and commented state variable
in UnitForm.tsx: delete the line containing "// import {useState} from 'react';"
and the commented state declaration (the commented useState variable around the
top of the UnitForm component) so no commented implementation artifacts remain;
re-add the import/state only when actually implementing stateful logic in
functions like UnitForm or handlers that reference useState.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/entities/unit/api/unitApi.tssrc/entities/unit/api/unitQueries.tssrc/pages/unit-editor/ui/UnitForm.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/entities/unit/api/unitQueries.ts
Comment on lines
+49
to
+56
| const onSubmit = async (data: TUnitFormSchema) => { | ||
| if (mode === 'editing' && unit) { | ||
| onUpdateUnit(unit.id, data); | ||
| return; | ||
| } | ||
|
|
||
| onCreateUnit(data); | ||
| }; |
There was a problem hiding this comment.
mode === 'editing'이지만 unit이 없을 때 onCreateUnit이 의도치 않게 호출됩니다
편집 모드(mode === 'editing')인데 unit이 undefined이면 조건 mode === 'editing' && unit이 false가 되어 onCreateUnit(data)으로 폴스루됩니다. 즉, 예상치 못한 단원 생성 요청이 발생할 수 있습니다.
🛡️ 제안 수정
const onSubmit = async (data: TUnitFormSchema) => {
if (mode === 'editing' && unit) {
onUpdateUnit(unit.id, data);
return;
}
+
+ if (mode === 'editing' && !unit) {
+ console.error('편집 모드이지만 단원 데이터가 없습니다.');
+ return;
+ }
+
onCreateUnit(data);
};📝 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 onSubmit = async (data: TUnitFormSchema) => { | |
| if (mode === 'editing' && unit) { | |
| onUpdateUnit(unit.id, data); | |
| return; | |
| } | |
| onCreateUnit(data); | |
| }; | |
| const onSubmit = async (data: TUnitFormSchema) => { | |
| if (mode === 'editing' && unit) { | |
| onUpdateUnit(unit.id, data); | |
| return; | |
| } | |
| if (mode === 'editing' && !unit) { | |
| console.error('편집 모드이지만 단원 데이터가 없습니다.'); | |
| return; | |
| } | |
| onCreateUnit(data); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/unit-editor/ui/UnitForm.tsx` around lines 49 - 56, The onSubmit
handler currently falls through to onCreateUnit when mode === 'editing' but unit
is undefined; update the onSubmit logic to explicitly handle the invalid editing
state: check mode first (if mode === 'editing') and if unit is missing,
short-circuit (return or surface an error) instead of calling onCreateUnit;
otherwise call onUpdateUnit(unit.id, data). For the create path, only call
onCreateUnit when mode indicates creation. Refer to the onSubmit function and
the mode, unit, onUpdateUnit, and onCreateUnit symbols to locate and change the
branching.
JiiminHa
approved these changes
Feb 24, 2026
Contributor
JiiminHa
left a comment
JiiminHa
left a comment
There was a problem hiding this comment.
고생하셨습니다! 스키마는 수정해보면 좋을 것 같아요!!
Contributor
There was a problem hiding this comment.
프로젝트에 이미 zod가 있으니까, 이걸로 API 응답 전용 '스키마'를 만들어보면 좋을 것 같아요!
Contributor
Author
There was a problem hiding this comment.
프로젝트에 이미 zod가 있으니까, 이걸로 API 응답 전용 '스키마'를 만들어보면 좋을 것 같아요!
넵 좋아요!!
| // 단일 단원 조회 | ||
| export const getUnitById = async ( | ||
| unitId: number | null | ||
| ): Promise<ApiResponse<Unit>> => { |
Contributor
There was a problem hiding this comment.
앞에 언급한 zod 활용 응답용 스키마 정의하고
unitSchema.parse(response.data)처럼 한 번 걸러서 반환하면 좋을 것 같아요!
Contributor
There was a problem hiding this comment.
뭔가 이 UI 파일 안에 useMutation 로직이 길게 들어간 느낌이라 커스텀 훅으로 분리해버리는 거는 어떻게 생각하시나요...?
This was referenced Feb 26, 2026
Merged
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚙️ Related ISSUE Number
Related #44
📄 Work Description
API 레이어
unitApi.ts)unitQueries객체 패턴으로 쿼리 옵션 통합 및queryKey구조 정리unitMutations객체로 mutation 옵션 분리 (createUnit,updateUnit,deleteUnit)페이지 상태 관리
useParams)으로 강의 ID 연동isEditingboolean →Mode ('idle' | 'creating' | 'editing')타입으로 명시적 상태 관리useEffect)invalidateQueries로 목록 자동 갱신폼 연동
mode기반으로 생성/수정 분기 처리 (onSubmit)reset()으로 변경사항 폐기📷 Screenshot
💬 To Reviewers
PR 머지하기 전에 FSD 구조 적용하겠습니다!
🔗 Reference