-
Notifications
You must be signed in to change notification settings - Fork 1
[Feature/#199] 나의 푸드트럭 등록 / 수정 api / 리뷰 XXXX #214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough음식 트럭 폼의 편집 모드를 API 기반으로 구현하는 변경입니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
빌드 및 린트 결과모든 검사 통과 🎉
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/pages/food-truck-detail/hooks/use-food-truck-detail.ts (1)
13-13: ID 검증 로직을 더 명확하게 개선하세요.
enabled: !!foodTruckId는 0을 허용합니다. 푸드트럭 ID가 1부터 시작한다면enabled: foodTruckId > 0이 더 명확합니다.🔎 제안하는 수정
- enabled: !!foodTruckId, + enabled: foodTruckId > 0,src/pages/@owner/food-truck-form/hooks/use-food-truck-form.ts (1)
56-80: 여러 번의 리렌더링 가능성을 고려해주세요.
foodTruckDetailData와menuData가 서로 다른 시점에 로드되면 폼이 여러 번 리셋될 수 있습니다. 또한 API 호출 실패 시 에러 처리가 없습니다.💡 한 번만 리셋하도록 개선하는 제안
두 데이터가 모두 준비된 후 한 번만 리셋하도록 수정할 수 있습니다:
useEffect(() => { - if (foodTruckDetailData) { + if (foodTruckDetailData && menuData !== undefined) { setIsEdit(true); reset({ // ... 기존 코드 }); } }, [foodTruckDetailData, menuData, reset]);또는 두 API 호출이 모두 완료될 때까지 대기하는 로딩 상태를 추가할 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/pages/@owner/estimate/hooks/use-estimate-form.tssrc/pages/@owner/food-truck-form/FoodTruckForm.tsxsrc/pages/@owner/food-truck-form/constants/food-truck.tssrc/pages/@owner/food-truck-form/hooks/use-food-truck-form.tssrc/pages/@owner/food-truck-form/schemas/food-truck-form.schema.tssrc/pages/food-truck-detail/hooks/use-food-truck-detail.tssrc/shared/constant/need-electricity.tssrc/shared/constant/payment-method.tssrc/shared/utils/date/date-formatter.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/pages/@owner/estimate/hooks/use-estimate-form.ts (1)
src/shared/utils/date/date-formatter.ts (1)
formatStringDatesToAvailableDates(73-102)
src/pages/@owner/food-truck-form/hooks/use-food-truck-form.ts (4)
src/pages/@owner/food-truck-form/schemas/food-truck-form.schema.ts (1)
foodTruckSchema(12-65)src/pages/food-truck-detail/hooks/use-food-truck-detail.ts (1)
useFoodTruckDetail(16-46)src/pages/@owner/menu/hooks/use-menus-query.ts (1)
useMenusQuery(6-25)src/shared/utils/date/date-formatter.ts (1)
formatStringDatesToAvailableDates(73-102)
src/pages/food-truck-detail/hooks/use-food-truck-detail.ts (1)
apis/data-contracts.ts (1)
FoodTruckDetailResponse(1427-1517)
src/pages/@owner/food-truck-form/FoodTruckForm.tsx (3)
src/pages/@owner/food-truck-form/hooks/use-food-truck-form.ts (1)
useFoodTruckForm(33-103)src/router/constant/routes.ts (1)
ROUTES(1-33)src/pages/@owner/food-truck-form/@section/region-section/RegionSection.tsx (1)
RegionSection(15-46)
🔇 Additional comments (10)
src/pages/@owner/estimate/hooks/use-estimate-form.ts (1)
9-9: LGTM!함수 이름 변경에 맞춰 import와 사용처가 올바르게 업데이트되었습니다.
Also applies to: 70-72
src/pages/@owner/food-truck-form/FoodTruckForm.tsx (2)
82-82: LGTM!
isEdit플래그에 따라 제목을 동적으로 표시하는 로직이 적절합니다.
97-97: LGTM!
RegionSection에foodTruckId를 전달하여 지역 데이터를 로드할 수 있도록 개선되었습니다.src/pages/@owner/food-truck-form/constants/food-truck.ts (1)
26-26: 백엔드 API와의 호환성 확인됨 - 추가 검증 불필요변경사항이 다음 항목들과 호환되는 것으로 확인되었습니다:
- 백엔드 API:
apis/data-contracts.ts라인 269에서 이미@maxItems 4로 정의되어 있음- UI 처리:
ActiveDate.tsx,ScheduleCalendar.tsx등의 컴포넌트가 날짜 범위를 동적으로 처리하므로 4개 제한 수용 가능- 폼 검증:
food-truck-form.schema.ts에서FOOD_TRUCK_MAX_LENGTH.availableDates.max를 참조하므로 자동으로 새로운 값 적용됨- 에러 메시지: 템플릿 기반이므로 상수값 변경 시 자동 반영됨
src/shared/utils/date/date-formatter.ts (1)
68-75: 함수 이름 변경이 적절하게 완료되었습니다.
formatEstimateDatesToAvailableDates를formatStringDatesToAvailableDates로 변경하여 함수의 범용성을 더 잘 표현하고 있으며, JSDoc도 적절히 업데이트되었습니다. 모든 호출처(use-food-truck-form.ts,use-estimate-form.ts)가 정상적으로 업데이트되었고, 이전 함수명의 남은 참조는 없습니다.src/pages/@owner/food-truck-form/hooks/use-food-truck-form.ts (5)
1-12: LGTM!새로운 기능에 필요한 import들이 올바르게 추가되었습니다.
95-103: LGTM!
isEdit상태를 반환값에 추가하여 컴포넌트에서 편집 모드를 판단할 수 있도록 한 것이 적절합니다.
50-54: 두 훅 모두 이미 유효하지 않은 ID로부터 보호되어 있습니다.
useFoodTruckDetail과useMenusQuery모두enabled: !!foodTruckId조건부 실행 로직을 포함하고 있으므로, 무효한 ID가 전달될 경우 API 호출이 실행되지 않습니다. 현재 구현이 이미 적절한 보호를 제공하고 있어 추가 수정이 필요하지 않습니다.Likely an incorrect or invalid review comment.
33-48: 문제 없습니다.useFoodTruckDetail과useMenusQuery훅 모두enabled: !!foodTruckId조건을 통해 유효하지 않은 ID로 인한 불필요한 API 호출을 이미 방지하고 있습니다. 새로운 푸드트럭 등록 시foodTruckIdNumber가 NaN이 되면 쿼리가 실행되지 않으므로 추가 검증이 필요하지 않습니다.
69-69: 이 코드는 타입 오류가 없습니다.FoodTruckDetailResponse.photoUrl은 이미string[]배열로 정의되어 있으며, 폼 필드photoUrls도string[]을 기대합니다.
| if (!foodTruckId || isNaN(foodTruckIdNumber)) { | ||
| toast.error('잘못된 접근입니다.'); | ||
| navigate(ROUTES.FOOD_TRUCK_MANAGEMENT); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hooks 규칙 위반 및 로직 흐름 문제를 수정해주세요.
현재 구조는 다음 문제들이 있습니다:
-
Hooks 규칙 위반: 유효성 검증(line 69-73)이 hooks 호출(line 43-61) 이후에 위치하여, 조건부로 hooks를 건너뛰게 됩니다. Hooks는 항상 동일한 순서로 호출되어야 합니다.
-
로직 흐름 문제:
navigate()는 비동기적으로 작동하므로return null전에 컴포넌트가 잠깐 렌더링될 수 있으며, 토스트 메시지가 표시되지 않을 수 있습니다. -
사용자 경험: 잘못된 접근 시 빈 화면이 깜빡일 수 있습니다.
🔎 제안하는 수정
방법 1: Early return을 hooks 전에 배치
export default function FoodTruckForm() {
const { foodTruckId } = useParams();
- const foodTruckIdNumber = Number(foodTruckId);
-
const navigate = useNavigate();
const location = useLocation();
const toast = useToast();
+ // Hooks 호출 전에 early return
+ if (!foodTruckId || isNaN(Number(foodTruckId))) {
+ useEffect(() => {
+ toast.error('잘못된 접근입니다.');
+ navigate(ROUTES.FOOD_TRUCK_MANAGEMENT);
+ }, [navigate, toast]);
+ return null;
+ }
+
+ const foodTruckIdNumber = Number(foodTruckId);
+
const { isEdit, methods, reset, isFormValid, handleSubmit } =
useFoodTruckForm(foodTruckIdNumber);
// ... rest of hooks
- if (!foodTruckId || isNaN(foodTruckIdNumber)) {
- toast.error('잘못된 접근입니다.');
- navigate(ROUTES.FOOD_TRUCK_MANAGEMENT);
- return null;
- }방법 2: useEffect로 검증 처리 (더 권장)
export default function FoodTruckForm() {
const { foodTruckId } = useParams();
const foodTruckIdNumber = Number(foodTruckId);
const navigate = useNavigate();
const location = useLocation();
const toast = useToast();
+ // 유효성 검증을 useEffect로 처리
+ useEffect(() => {
+ if (!foodTruckId || isNaN(foodTruckIdNumber)) {
+ toast.error('잘못된 접근입니다.');
+ navigate(ROUTES.FOOD_TRUCK_MANAGEMENT);
+ }
+ }, [foodTruckId, foodTruckIdNumber, navigate, toast]);
+
+ // 잘못된 접근인 경우 로딩 상태 표시
+ if (!foodTruckId || isNaN(foodTruckIdNumber)) {
+ return null; // 또는 <LoadingSpinner /> 등
+ }
const { isEdit, methods, reset, isFormValid, handleSubmit } =
useFoodTruckForm(foodTruckIdNumber);
// ... rest of the component
- if (!foodTruckId || isNaN(foodTruckIdNumber)) {
- toast.error('잘못된 접근입니다.');
- navigate(ROUTES.FOOD_TRUCK_MANAGEMENT);
- return null;
- }| ), | ||
| activeTime: foodTruckDetailData.activeTime, | ||
| timeDiscussRequired: foodTruckDetailData.timeDiscussRequired, | ||
| menus: menuData !== undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메뉴 존재 여부 체크 로직을 개선해주세요.
menuData !== undefined는 useMenusQuery의 반환값이 존재하는지만 체크합니다. useInfiniteQuery는 data.pages 배열을 반환하므로, 실제 메뉴 항목이 있는지 확인하려면 페이지 내용을 체크해야 합니다.
🔎 메뉴 존재 여부를 정확히 체크하는 수정 제안
- menus: menuData !== undefined,
+ menus: menuData?.pages?.[0]?.content?.length > 0,또는 더 안전하게:
- menus: menuData?.pages.some(page => page.content?.length > 0) ?? false,📝 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.
| menus: menuData !== undefined, | |
| menus: menuData?.pages?.[0]?.content?.length > 0, |
🤖 Prompt for AI Agents
In src/pages/@owner/food-truck-form/hooks/use-food-truck-form.ts around line 77,
the current menus check uses `menuData !== undefined`, which only verifies the
query returned something but not whether any menu items exist; update it to
inspect `menuData.pages` (e.g., check `menuData?.pages?.some(page => page.items
&& page.items.length > 0)` or `menuData?.pages?.flatMap(p => p.items ??
[]).length > 0`) so the flag truly reflects presence of menu entries and safely
handles undefined/null pages/items.
| availableQuantity: z.string(), | ||
| needElectricity: z.string(), | ||
| paymentMethod: z.string(), | ||
| menuCategories: z.array(z.string()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입 안정성 손실을 해결해주세요.
availableQuantity, needElectricity, paymentMethod, menuCategories가 z.nativeEnum()에서 z.string()으로 변경되어 다음 문제가 발생합니다:
- 컴파일 타임 타입 체크 손실: 잘못된 값이 런타임까지 발견되지 않음
- 허용값 검증 없음: 오타나 잘못된 문자열도 통과
- API 계약 불일치 가능성: 백엔드가 예상하지 못한 값 수신 가능
의도적인 변경이라면 다음 중 하나를 권장합니다:
z.enum([...])사용하여 허용값 명시.refine()추가로 런타임 검증- 백엔드 스키마와 동기화된 공유 타입 정의
🔎 제안하는 개선안 (예시: needElectricity)
+import { NEED_ELECTRICITY } from '@shared/constant/need-electricity';
+
+const needElectricityValues = Object.values(NEED_ELECTRICITY) as [string, ...string[]];
+
export const foodTruckSchema = z.object({
// ... other fields
- needElectricity: z.string(),
+ needElectricity: z.enum(needElectricityValues, {
+ error: () => ({ code: 'custom', message: '올바른 전기 사용 옵션을 선택해주세요.' }),
+ }),다른 필드들도 유사하게 처리할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
| export const NEED_ELECTRICITY = { | ||
| REQUIRED: '필요', | ||
| NOT_REQUIRED: '필요 없음', | ||
| NOT_REQUIRED: '불필요', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for needElectricity usage across the codebase
rg -nP --type=ts --type=tsx -C3 'needElectricity|NEED_ELECTRICITY|need-electricity' -g '!node_modules'Repository: CHA-CALL/ChaCall-FE
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for needElectricity usage across the codebase
# Using glob pattern to include .ts and .tsx files
rg 'needElectricity|NEED_ELECTRICITY|need-electricity' -g '*.ts' -g '*.tsx' -C3Repository: CHA-CALL/ChaCall-FE
Length of output: 23723
Zod 스키마에서 needElectricity 유효성 검증 누락
food-truck-form.schema.ts에서 needElectricity가 z.string()으로 정의되어 있어 허용되는 값에 대한 검증이 없습니다. 백엔드 API는 "필요" | "불필요" | "논의 필요" 중 하나만 받아들이므로, 다음과 같이 수정이 필요합니다:
needElectricity: z.enum(['필요', '불필요', '논의 필요'])또한 예상과 다르게 src/pages/@owner/estimate/@section/basic-info-section/NeedElectricity.tsx에서는 같은 필드가 boolean 타입으로 처리되고 있습니다. 데이터 타입 일관성을 확인해주세요.
🤖 Prompt for AI Agents
src/shared/constant/need-electricity.ts (around line 3) and related files: the
current constant and form/schema usage are inconsistent with the backend enum;
update food-truck-form.schema.ts to validate needElectricity as
z.enum(['필요','불필요','논의 필요']) instead of z.string(), ensure
src/pages/@owner/estimate/@section/basic-info-section/NeedElectricity.tsx stops
treating the field as boolean and instead reads/writes one of the three string
values, and sync the constant file to export exactly those three string values
so all places use the same type/labels.
| BANK_TRANSFER: '계좌이체', | ||
| ANY: '무관', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find usages of paymentMethod and PAYMENT_METHOD across the codebase
rg -nP --type=ts --type=tsx -C3 'paymentMethod|PAYMENT_METHOD' -g '!node_modules'Repository: CHA-CALL/ChaCall-FE
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for paymentMethod and PAYMENT_METHOD usage without tsx type
rg -nP 'paymentMethod|PAYMENT_METHOD' -C3 -g '!node_modules'Repository: CHA-CALL/ChaCall-FE
Length of output: 16300
🏁 Script executed:
#!/bin/bash
# Search for old payment method values that may have changed
rg '계좌 이체|아무거나' -g '!node_modules'Repository: CHA-CALL/ChaCall-FE
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Read the food-truck-form.schema.ts file
head -100 src/pages/@owner/food-truck-form/schemas/food-truck-form.schema.tsRepository: CHA-CALL/ChaCall-FE
Length of output: 2146
🏁 Script executed:
#!/bin/bash
# Search for any hardcoded payment method comparisons or mappings
rg 'BANK_TRANSFER|CARD|ANY' -C2 src/shared/constant/payment-method.tsRepository: CHA-CALL/ChaCall-FE
Length of output: 159
기존 데이터와의 호환성을 확인해주세요.
BANK_TRANSFER와 ANY의 문자열 값이 변경되었습니다:
- '계좌 이체' → '계좌이체' (공백 제거)
- '아무거나' → '무관' (더 공식적인 표현)
백엔드 API 계약서에는 이미 새로운 값('무관', '계좌이체', '카드')이 정의되어 있어 새 값을 수신하는 데는 문제가 없습니다. 그러나 데이터베이스에 존재하는 기존 푸드트럭 데이터가 여전히 이전 문자열 값을 가지고 있을 수 있으므로 다음을 확인해주세요:
- 기존 데이터를 조회할 때 이전 값이 반환되는 경우 UI에서 제대로 처리되는지
- 기존 푸드트럭 수정 시 백엔드에서 반환된 이전 값이 폼에 올바르게 로드되는지
- 필요한 경우 데이터 마이그레이션 계획
📌 Related Issues
✅ 체크 리스트
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
🔔 ETC
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
기타
✏️ Tip: You can customize this high-level summary in your review settings.