[refactor] 선택한 지원서 양식 id 새로고침해도 저장된다#902
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| 코호트 / 파일 | 요약 |
|---|---|
컨텍스트 상태 제거 frontend/src/context/AdminClubContext.tsx |
AdminClubContextType 인터페이스에서 applicationFormId 및 setApplicationFormId 속성 제거; AdminClubProvider에서 해당 상태 관리 및 context value에서 제거 |
라우팅 업데이트 frontend/src/pages/AdminPage/AdminRoutes.tsx |
application-list/:applicationFormId/edit 동적 라우트 추가; ApplicationEditTab으로 렌더링하며 기존 정적 라우트 유지 |
라우트 매개변수 기반 데이터 페칭 frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx |
useParams로 applicationFormId 추출; 컨텍스트 대신 라우트 매개변수로 useGetApplication 호출; formData.questions에 옵셔널 체이닝 추가 |
컨텍스트 사용 제거 frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsx |
useParams로 applicationFormId 추출; AdminClubContext 의존성 제거; useGetApplicants 및 네비게이션에 라우트 기반 ID 사용 |
렌더링 구조 및 메뉴 개선 frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx |
AdminClubContext 제거; 학기별 블록 구조로 리팩토링; 각 행별로 MoreButton 및 ApplicationMenu 포함; 문서 mousedown 리스너로 메뉴 외부 클릭 처리 추가 |
라우트 기반 formId 추출 frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx |
useParams 임포트 및 applicationFormId를 라우트 매개변수로부터 추출; useGetApplication 호출에 라우트 기반 formId 사용 |
편집 네비게이션 및 상태 관리 frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx |
handleGoToEditForm을 /admin/application-list/{id}/edit로 직접 네비게이션하도록 변경; 항목별 메뉴 및 MoreButton 추가; openMenuId 상태로 외부 클릭 처리; 항목별 updateApplicationStatus 호출 |
예상 코드 리뷰 소요시간
🎯 4 (복잡함) | ⏱️ ~60분
추가 검토 필요 영역:
- ApplicantsListTab.tsx 및 ApplicationListTab.tsx: 렌더링 구조 대규모 리팩토링으로 인해 UI 흐름, 메뉴 토글 로직, 외부 클릭 처리의 정확성 검증 필요
- 라우트 매개변수 전파: 모든 컴포넌트에서 applicationFormId 추출 및 사용이 올바르게 연쇄되는지 확인 필요 (undefined 폴백 처리 포함)
- 컨텍스트 의존성 제거: 다른 컴포넌트에서 applicationFormId가 여전히 컨텍스트에 의존하지 않는지 확인 필요
- 선택적 체이닝 및 가드 클로즈: formData.questions의 선택적 체이닝 및 applicationFormId/applicantsData 존재 검증이 모든 렌더링 경로에서 일관적인지 검토 필요
관련 이슈
- MOA-395: 선택한 지원서 양식 ID를 컨텍스트 기반 관리에서 라우트 매개변수 기반 관리로 변경하는 단계이며, 향후 Zustand 스토어로의 마이그레이션을 위한 준비 작업으로 보임
관련 PR
- [feature] 지원서 목록 페이지 생성 #772: application-list/edit 라우팅 및 ApplicationListTab 컴포넌트 수정 (기존 라우트 도입과 본 PR의 매개변수화 단계)
- [feature] 사용자가 지원하기를 눌렀을때 지원가능한 지원서들이 모달로 뜬다 #787: AdminClubContext에 applicationFormId/setApplicationFormId 추가했던 PR과 반대 방향의 변경 (context 제거)
- [feature & refactor] 객관식 항목 1개 허용 · 필수 입력 UX 개선 · Application 구조 리팩토링 #558: 관리자 지원서 편집 흐름 수정 (ApplicationEditTab 도입 및 applicationFormId 제공 방식 변경)
검토자 제안
- seongwon030
- oesnuj
- Zepelown
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | PR에서 AdminContext의 applicationFormId를 제거하고 URL 경로 매개변수로 변경하여 새로고침 시에도 applicationFormId가 유지되도록 구현했습니다. 그러나 연결된 이슈 MOA-395에서 요구하는 Zustand store 사용은 구현되지 않았습니다. | Zustand store를 구현하여 applicationFormId를 저장 및 관리하거나, 현재 경로 기반 접근 방식이 의도적인 변경임을 이슈에서 명확히 문서화해야 합니다. | |
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목이 변경 사항의 주요 목표와 일치합니다. 새로고침 시 선택된 지원서 양식 ID가 유지되도록 하는 기능이 코드 변경에 반영되어 있습니다. |
| Out of Scope Changes check | ✅ Passed | 대부분의 변경사항은 applicationFormId 관리 방식을 AdminContext에서 URL 경로 매개변수로 변경하는 것과 관련되어 있으며, 이는 목표 범위 내입니다. 라우팅 구조 및 컴포넌트 렌더링 업데이트는 이 리팩토링을 지원하는 필요한 변경입니다. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
refactor/#901-store-application-form-id-MOA-395
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx (1)
125-137: Navigation routes in ApplicantDetailPage are broken—mismatched base paths, missing parameters, and a typo prevent navigation from working correctly.Current code (lines 125-137, 153-155):
previousApplicant()navigates to/admin/applicants/{id}nextApplicant()navigates to/admin/applicants/{id}- Select dropdown navigates to
/admin/applicants-lsit/{id}(typo: "lsit")The only defined route in AdminRoutes.tsx is
applicants-list/:applicationFormId/:questionId(line 34-35), but ApplicantsTab.tsx correctly navigates using/admin/applicants-list/${applicationFormId}/${item.id}(line 492). All three navigation methods in ApplicantDetailPage must match this pattern:const previousApplicant = () => { const previousData = applicantsData.applicants[applicantIndex - 1]; - if (applicantIndex < 0 || !previousData) return; + if (applicantIndex < 0 || !previousData || !applicationFormId) return; - navigate(`/admin/applicants/${previousData.id}`); + navigate( + `/admin/applicants-list/${applicationFormId}/${previousData.id}`, + ); }; const nextApplicant = () => { const nextData = applicantsData.applicants[applicantIndex + 1]; - if (applicantIndex < 0 || !nextData) return; + if (applicantIndex < 0 || !nextData || !applicationFormId) return; - navigate(`/admin/applicants/${nextData.id}`); + navigate(`/admin/applicants-list/${applicationFormId}/${nextData.id}`); }; - onChange={(e) => - navigate(`/admin/applicants-lsit/${e.target.value}`) - } + onChange={(e) => + applicationFormId && + navigate( + `/admin/applicants-list/${applicationFormId}/${e.target.value}`, + ) + }
🧹 Nitpick comments (1)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsx (1)
18-20: 라우트 기반applicationFormId연동과 상세 페이지 네비게이션이 잘 맞습니다.
useParams로 받은applicationFormId를useGetApplicants,useUpdateApplicant에 넘기고, 상세 페이지로는
/admin/applicants-list/${applicationFormId}/${item.id}로 이동하는 구조가AdminRoutes의
applicants-list/:applicationFormId/:questionId정의와 정확히 일치합니다.- 현재 라우트 구조상
applicationFormId는 항상 존재하므로 동작에는 문제가 없을 것으로 보입니다. 다만, 훅 시그니처 차원에서applicationFormId ?? ''대신 그대로 넘기고 훅 내부에서 enabled 조건을 처리하면(다른 파일들과도) 더 일관된 패턴이 될 수 있습니다.Also applies to: 40-47, 76-79, 487-495
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
frontend/src/context/AdminClubContext.tsx(2 hunks)frontend/src/pages/AdminPage/AdminRoutes.tsx(1 hunks)frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx(4 hunks)frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx(3 hunks)frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsx(6 hunks)frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx(1 hunks)frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx(4 hunks)frontend/src/store/useApplicationFormStore.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.{ts,tsx,js,jsx}: Replace magic numbers with named constants for clarity
Replace complex/nested ternaries withif/elseor IIFEs for readability
Assign complex boolean conditions to named variables for explicit meaning
Avoid hidden side effects; functions should only perform actions implied by their signature (Single Responsibility Principle)
Use unique and descriptive names for custom wrappers/functions to avoid ambiguity
Define constants near related logic or ensure names link them clearly to avoid silent failures
Break down broad state management into smaller, focused hooks/contexts to reduce coupling
Files:
frontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsxfrontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsxfrontend/src/context/AdminClubContext.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsxfrontend/src/store/useApplicationFormStore.ts
frontend/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.{tsx,jsx}: Abstract complex logic/interactions into dedicated components/HOCs
Separate significantly different conditional UI/logic into distinct components
Colocate simple, localized logic or use inline definitions to reduce context switching
Choose field-level or form-level cohesion based on form requirements when using form libraries like react-hook-form
Use Component Composition instead of Props Drilling to reduce coupling
Files:
frontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsxfrontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsxfrontend/src/context/AdminClubContext.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
Use consistent return types for similar functions/hooks
Files:
frontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsxfrontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsxfrontend/src/context/AdminClubContext.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsxfrontend/src/store/useApplicationFormStore.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: seongwon030
Repo: Moadong/moadong PR: 195
File: frontend/src/pages/AdminPage/AdminPage.tsx:7-7
Timestamp: 2025-03-19T05:18:07.818Z
Learning: AdminPage.tsx에서 현재 하드코딩된 클럽 ID('67d2e3b9b15c136c6acbf20b')는 로그인 기능 구현 후 동적으로 가져오는 방식으로 수정될 예정입니다.
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx
📚 Learning: 2025-11-25T14:08:23.234Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-11-25T14:08:23.234Z
Learning: Applies to frontend/**/*.{ts,tsx,js,jsx} : Break down broad state management into smaller, focused hooks/contexts to reduce coupling
Applied to files:
frontend/src/context/AdminClubContext.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsTab.tsxfrontend/src/store/useApplicationFormStore.ts
📚 Learning: 2025-11-25T14:08:23.234Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-11-25T14:08:23.234Z
Learning: Applies to frontend/**/*.{tsx,jsx} : Abstract complex logic/interactions into dedicated components/HOCs
Applied to files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx
📚 Learning: 2025-11-25T14:08:23.234Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-11-25T14:08:23.234Z
Learning: Applies to frontend/**/*.{tsx,jsx} : Colocate simple, localized logic or use inline definitions to reduce context switching
Applied to files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx
📚 Learning: 2025-11-25T14:08:23.234Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-11-25T14:08:23.234Z
Learning: Applies to frontend/**/*.{tsx,jsx} : Separate significantly different conditional UI/logic into distinct components
Applied to files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx
🧬 Code graph analysis (3)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx (4)
frontend/src/context/AdminClubContext.tsx (1)
useAdminClubContext(39-46)frontend/src/hooks/queries/applicants/useUpdateApplicant.ts (1)
useUpdateApplicant(5-24)frontend/src/types/application.ts (1)
Question(5-14)frontend/src/mocks/data/mockData.ts (1)
Question(21-28)
frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx (1)
frontend/src/context/AdminClubContext.tsx (1)
useAdminClubContext(39-46)
frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx (3)
frontend/src/hooks/queries/application/useGetApplicationlist.ts (1)
useGetApplicationlist(4-10)frontend/src/apis/application/updateApplication.ts (1)
updateApplicationStatus(33-58)frontend/src/types/application.ts (1)
ApplicationFormItem(70-75)
🔇 Additional comments (4)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (1)
13-23: 라우트 파라미터 기반 네비게이션/메뉴 토글 로직이 자연스럽게 정리되었습니다.
handleGoToDetailForm에서/admin/applicants-list/${applicationFormId}로 이동하는 부분이AdminRoutes의applicants-list/:applicationFormId라우트와 잘 맞습니다.- 학기별 그룹 렌더링과 per-row
ApplicationMenu토글(openMenuId + outside click 처리)도 결합도 낮게 잘 구성되어 있어 유지보수에 좋아 보입니다.isToday계산을 여러 줄로 풀어 가독성 높인 것도 👍 입니다.Also applies to: 74-81, 106-156
frontend/src/context/AdminClubContext.tsx (1)
11-33: AdminClubContext에서 역할을 깔끔하게 분리하신 것 같습니다.
applicationFormId를 제거하고clubId,applicantsData에만 집중하도록 정리된 구조가 명확하고, Provider의 value도 타입과 정확히 일치합니다. 이 변경으로 컨텍스트 책임이 줄어들어 이후 상태 분리에도 유리해 보입니다.frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx (1)
15-25: 개별 양식 수정 라우트 및 상태 토글 로직이 새 구조에 잘 맞게 정리되었습니다.
handleGoToEditForm에서/admin/application-list/${applicationFormId}/edit로 이동하는 부분이AdminRoutes의application-list/:applicationFormId/edit라우트와 정확히 매칭됩니다.handleToggleClick에서updateApplicationStatus호출 후['applicationForm']쿼리를 무효화하고 메뉴를 닫는 처리로 리스트/상태 동기화 흐름이 명확합니다.- 학기별 그룹/행 렌더링과 per-row 메뉴 구조도 ApplicantsListTab과 패턴이 맞아 재사용성/일관성이 좋아 보입니다. (새 양식 생성용 라우트는
AdminRoutes.tsx코멘트 참고 부탁드립니다.)Also applies to: 40-52, 91-97, 124-177
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx (1)
35-39: 라우트 기반applicationFormId연동과 조회/수정 훅 연결은 올바르게 보입니다.
useParams에서questionId,applicationFormId를 함께 받아useGetApplication(clubId!, applicationFormId ?? undefined)와useUpdateApplicant(applicationFormId ?? undefined)에 넘기는 구조가 다른 파일들과 일관적입니다.!applicationFormId,!applicantsData,isLoading,isError에 대한 가드가 명시적으로 들어가 있어 에러 케이스 처리도 비교적 안전해 보입니다.- 질문 렌더링 시
formData.questions?.map으로 optional chaining을 추가한 것도 런타임 안전성 측면에서 좋은 변경입니다.Also applies to: 45-59, 91-99, 195-203
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import Button from '@/components/common/Button/Button'; | ||
|
|
||
| const ApplicationEditTab = () => { | ||
| const navigate = useNavigate(); | ||
| const queryClient = useQueryClient(); | ||
| const { clubId, applicationFormId: formId } = useAdminClubContext(); | ||
| const { applicationFormId: formId } = useParams<{ | ||
| applicationFormId: string; | ||
| }>(); | ||
| const { clubId } = useAdminClubContext(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine ApplicationEditTab.tsx around the mentioned lines
fd -e tsx -e ts "ApplicationEditTab.tsx" --type f | head -5Repository: Moadong/moadong
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Find and examine the ApplicationEditTab file with line numbers
find . -name "ApplicationEditTab.tsx" -type f 2>/dev/nullRepository: Moadong/moadong
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Search more broadly for the file
rg "ApplicationEditTab" -l --type typescriptRepository: Moadong/moadong
Length of output: 93
🏁 Script executed:
#!/bin/bash
# List files in frontend pages
ls -la frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ 2>/dev/null || echo "Path not found, searching..."
find frontend -name "*ApplicationEditTab*" -type f 2>/dev/null || echo "Not in frontend, searching project root"
find . -name "*ApplicationEditTab*" -type f 2>/dev/null | head -10Repository: Moadong/moadong
Length of output: 644
🏁 Script executed:
#!/bin/bash
# Read the ApplicationEditTab.tsx file with line numbers
cat -n frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsxRepository: Moadong/moadong
Length of output: 14342
🏁 Script executed:
#!/bin/bash
# Find the useGetApplication hook
find . -name "*useGetApplication*" -type f 2>/dev/nullRepository: Moadong/moadong
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Read the useGetApplication hook
cat -n frontend/src/hooks/queries/application/useGetApplication.tsRepository: Moadong/moadong
Length of output: 603
🏁 Script executed:
#!/bin/bash
# Search for ApplicantDetailPage
find . -name "*ApplicantDetailPage*" -type f 2>/dev/nullRepository: Moadong/moadong
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Read ApplicantDetailPage.tsx and search for useGetApplication usage
rg -n "useGetApplication" frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx -A 3 -B 3Repository: Moadong/moadong
Length of output: 762
🏁 Script executed:
#!/bin/bash
# Search for all useGetApplication calls in the codebase
rg "useGetApplication\(" -A 1 frontend/src --type ts --type tsxRepository: Moadong/moadong
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Search for all useGetApplication calls in the codebase
rg "useGetApplication\(" -A 1 frontend/srcRepository: Moadong/moadong
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Check ApplicationFormPage for context
rg -n "useGetApplication" frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx -B 5 -A 2Repository: Moadong/moadong
Length of output: 797
🏁 Script executed:
#!/bin/bash
# Search for AdminRoutes to see if there's a create route without applicationFormId
find . -name "*AdminRoute*" -o -name "*admin*route*" | grep -i routeRepository: Moadong/moadong
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Read AdminRoutes to check route definitions
cat -n frontend/src/pages/AdminPage/AdminRoutes.tsxRepository: Moadong/moadong
Length of output: 2257
🏁 Script executed:
#!/bin/bash
# Search for mentions of application creation routes or formId defaults
rg "application.*create" frontend/src/pages/AdminPage -i -B 2 -A 2 | head -30Repository: Moadong/moadong
Length of output: 1448
Based on my verification, I can now provide the rewritten review comment. The original review is correct and well-founded:
Pass formId directly or use undefined consistently with other files for semantic clarity
ApplicationEditTab currently uses formId ?? '' when calling useGetApplication, while ApplicantDetailPage uses applicationFormId ?? undefined. The codebase shows this pattern is inconsistent—ApplicantDetailPage (and the general pattern) prefers undefined to represent absence of a value.
Though the current enabled: !!clubId && !!applicationFormId condition in the hook treats both empty string and undefined identically (both prevent the query), using undefined is semantically clearer and safer for future changes. If the enabled condition or hook logic is modified, an empty string could unintentionally trigger API calls.
- } = useGetApplication(clubId ?? undefined, formId ?? '');
+ } = useGetApplication(clubId ?? undefined, formId);When a create route (without applicationFormId in the URL) is added to AdminRoutes, this pattern will already be in place. The hook's enabled condition will then properly gate the query.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx
around lines 17 to 27, the component currently passes formId ?? '' to
useGetApplication which uses an empty string to represent "no id" but the
codebase standard is to use undefined; change the call to pass formId directly
or use formId ?? undefined so absence is represented by undefined, and keep the
hook's enabled condition as !!clubId && !!applicationFormId (or adjust to check
undefined if necessary) to ensure consistent, semantically correct gating of the
query.
| interface ApplicationFormStore { | ||
| applicationFormId: string | null; | ||
| setApplicationFormId: (id: string | null) => void; | ||
| } | ||
|
|
||
| export const useApplicationFormStore = create<ApplicationFormStore>()( | ||
| subscribeWithSelector( | ||
| persist( | ||
| (set) => ({ | ||
| applicationFormId: '', | ||
| setApplicationFormId: (id: string | null) => | ||
| set({ applicationFormId: id }), | ||
| }), |
There was a problem hiding this comment.
applicationFormId 초기값을 null로 두는 편이 타입/의도에 더 잘 맞아 보입니다.
ApplicationFormStore에서 applicationFormId를 string | null로 선언해 두셨는데, 실제 초기값은 ''(빈 문자열)입니다. 호출부에서 applicationFormId === null 여부로 분기하거나, falsy 체크와 혼용하면 미묘한 버그가 생길 수 있습니다.
선택되지 않음 상태를 일관되게 null로 두는 쪽으로 맞추는 것을 권장드립니다.
- (set) => ({
- applicationFormId: '',
+ (set) => ({
+ applicationFormId: null,
setApplicationFormId: (id: string | null) =>
set({ applicationFormId: id }),📝 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.
| interface ApplicationFormStore { | |
| applicationFormId: string | null; | |
| setApplicationFormId: (id: string | null) => void; | |
| } | |
| export const useApplicationFormStore = create<ApplicationFormStore>()( | |
| subscribeWithSelector( | |
| persist( | |
| (set) => ({ | |
| applicationFormId: '', | |
| setApplicationFormId: (id: string | null) => | |
| set({ applicationFormId: id }), | |
| }), | |
| interface ApplicationFormStore { | |
| applicationFormId: string | null; | |
| setApplicationFormId: (id: string | null) => void; | |
| } | |
| export const useApplicationFormStore = create<ApplicationFormStore>()( | |
| subscribeWithSelector( | |
| persist( | |
| (set) => ({ | |
| applicationFormId: null, | |
| setApplicationFormId: (id: string | null) => | |
| set({ applicationFormId: id }), | |
| }), |
🤖 Prompt for AI Agents
In frontend/src/store/useApplicationFormStore.ts around lines 8 to 20, the store
declares applicationFormId as string | null but initializes it to an empty
string; change the initial value from '' to null so the runtime value matches
the declared type and intended "not selected" state, and ensure
setApplicationFormId continues to accept and set null values (no other API
changes required).
seongwon030
left a comment
seongwon030
left a comment
There was a problem hiding this comment.
param 방식으로 수정한 것 확인했습니다 수정 필요할 부분들 리뷰했어요~
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx (5)
15-25: 쿼리 결과 디스트럭처링과 편집 라우트 경로가 일관적으로 잘 연결되어 있습니다
useGetApplicationlist에서error까지 구조 분해하고, 아래에서isError분기 후error.message를 사용하는 패턴은 자연스럽고Spinner/에러 UI 흐름도 명확합니다. 다만 React Query 기본 제네릭에서는error타입이unknown이기 때문에, 훅 정의 쪽에서useQuery<..., Error>처럼 에러 타입을Error로 고정해 두었는지 한 번 확인해 주세요. 또한handleGoToEditForm의navigate(\/admin/application-list/${applicationFormId}/edit`)가 AdminRoutes의application-list/:applicationFormId/edit` 라우트와 정확히 매칭되어 라우팅 구조도 잘 맞습니다.
27-30: 삭제 관련 TODO 주석이 명확해 이후 구현 시 찾기 좋습니다지원서 삭제 API가 준비되면 바로 아래 주석 처리된
handleDeleteApplication를 살리면 되도록 TODO를 남겨둔 점이 좋습니다. 동일한 패턴이 ApplicantsListTab에도 있어서, 실제 API 구현 시 두 탭에서 함께 사용할 공통 삭제 핸들러(예: 커스텀 훅 또는 유틸 함수)로 추출하는 것도 고려해 볼 만합니다. 필요하시면 해당 공통 로직 스케치도 도와드릴 수 있습니다.
44-56: 상태 토글 핸들러 로직은 적절하지만 타입과 변수명은 조금 더 명확히 할 수 있습니다
updateApplicationStatus호출 후invalidateQueries({ queryKey: ['applicationForm'] })로 리스트를 재조회하고, 메뉴를 닫는 흐름이 깔끔합니다. 다만:
currentStatus를 단순string이 아니라ApplicationFormItem['status']로 좁히면 잘못된 상태 값 전달을 컴파일 타임에 막을 수 있고,catch (error)의error이름이 상단의 쿼리 결과error와 같은 스코프에서 중첩되어 약간 헷갈릴 수 있어 다른 이름으로 바꾸면 가독성이 좋아집니다.예시:
- const handleToggleClick = async ( - applicationFormId: string, - currentStatus: string, - ) => { + const handleToggleClick = async ( + applicationFormId: string, + currentStatus: ApplicationFormItem['status'], + ) => { try { await updateApplicationStatus(applicationFormId, currentStatus); queryClient.invalidateQueries({ queryKey: ['applicationForm'] }); setOpenMenuId(null); - } catch (error) { - console.error('지원서 상태 변경 실패:', error); + } catch (caughtError) { + console.error('지원서 상태 변경 실패:', caughtError); alert('상태 변경에 실패했습니다.'); } };
98-101: isToday 플래그로 날짜 비교를 분리한 점이 가독성 측면에서 좋습니다
isToday변수를 한 번 계산해 두고 오늘/그 외 날짜 포맷을 분기하는 구조가 명확해서, 복잡한 조건식이 JSX 안으로 흘러들어가지 않는 점이 좋습니다. 같은 로직이 ApplicantsListTab에도 있으니 나중에 중복을 줄이고 싶다면formatDateTime유틸 함수로 분리하는 것도 고려해 볼 수 있습니다.
128-181: 학기별 그룹 렌더링과 per-row 메뉴 동작이 의도에 잘 부합합니다
semesterTermLabel/semesterTitle로 학기 헤더를 구성하고, 그 아래에group.forms를 매핑하는 구조가 읽기 쉽고 ApplicantsListTab과도 UI 일관성이 잘 맞습니다.- 각 행에서 제목 클릭은 편집 화면으로, More 버튼 클릭은
ApplicationMenu를 열도록 분리해 둔 점도 UX 측면에서 명확합니다.openMenuId와menuRef를 이용해 한 번에 하나의 행 메뉴만 열리게 하고, 외부 클릭 시 닫히도록 한 구현도 문제 없어 보입니다.ApplicationListTab와 ApplicantsListTab 양쪽에서 거의 동일한 리스트/메뉴 렌더링 패턴을 사용하고 있으니, 나중에 디자인이 더 고정되면 공통 리스트 컴포넌트나 훅으로 추출해 중복을 줄이는 것도 좋겠습니다.
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (2)
13-27: 쿼리 결과 사용과 상세 화면 라우트가 새 라우팅 구조와 잘 맞습니다
useGetApplicationlist에서error까지 구조 분해하고isError분기에서error.message를 사용하는 부분이 ApplicationListTab과 동일하게 맞춰져 있어 일관성이 좋습니다. 마찬가지로 훅 정의에서 React Query의error제네릭을Error로 고정해 두었는지 한 번 확인해 주시면 타입 안정성이 더 좋아집니다.handleGoToDetailForm의navigate(\/admin/applicants-list/${applicationFormId}`)는 AdminRoutes의applicants-list/:applicationFormId` 라우트와 정확히 일치해서, 폼 목록 → 해당 폼의 지원자 목록으로 자연스럽게 이동할 수 있을 것 같습니다.- 삭제 관련 TODO 주석도 ApplicationListTab과 동일한 위치에 있어, 실제 삭제 API 구현 시 두 탭을 함께 리팩터링하기 쉬운 구조입니다.
81-160: 날짜 포맷과 학기/폼 리스트 렌더링이 ApplicationListTab과 잘 맞춰져 있으며, 약간의 공통화 여지가 있습니다
isToday플래그로 오늘/그 외 날짜를 나누어 포맷하는 로직이 ApplicationListTab과 완전히 동일해 두 탭의 표시가 일관적입니다. 두 곳에서 모두 사용하는 만큼, 나중에formatDateTime유틸 함수로 분리하면 중복을 줄이고 유지보수에 도움이 되겠습니다.- 학기 헤더(
semesterTermLabel,semesterTitle)와 그 아래ApplicationRow/More 버튼/ApplicationMenu구조 역시 ApplicationListTab과 거의 동일해, 관리자가 보는 화면 전체의 UX가 통일되어 있다는 점이 좋습니다.- 이 탭에서는
ApplicationMenu에isActive만 전달하고 상태 토글 핸들러는 넘기지 않고 있는데,ApplicationMenu컴포넌트의 props에서onToggleStatus가 선택적(optional)으로 정의되어 있는지만 한 번 확인해 주세요. 필수 prop라면 이쪽에서도 명시적으로 넘겨주어야 합니다.- 마지막으로, 이 파일의 컴포넌트 이름이
ApplicationListTab인 반면 AdminRoutes에서는ApplicantsListTab이름으로 import하고 있어 살짝 혼란을 줄 수 있습니다. 여유가 생기면 아래와 같이 이름을 맞춰 두면 추적이 더 쉬울 것 같습니다.-const ApplicationListTab = () => { +const ApplicantsListTab = () => { ... -}; - -export default ApplicationListTab; +}; + +export default ApplicantsListTab;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/src/pages/AdminPage/AdminRoutes.tsx(1 hunks)frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx(4 hunks)frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx(3 hunks)frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx(1 hunks)frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx
- frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantDetailPage/ApplicantDetailPage.tsx
🧰 Additional context used
📓 Path-based instructions (3)
frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.{ts,tsx,js,jsx}: Replace magic numbers with named constants for clarity
Replace complex/nested ternaries withif/elseor IIFEs for readability
Assign complex boolean conditions to named variables for explicit meaning
Avoid hidden side effects; functions should only perform actions implied by their signature (Single Responsibility Principle)
Use unique and descriptive names for custom wrappers/functions to avoid ambiguity
Define constants near related logic or ensure names link them clearly to avoid silent failures
Break down broad state management into smaller, focused hooks/contexts to reduce coupling
Files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx
frontend/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
frontend/**/*.{tsx,jsx}: Abstract complex logic/interactions into dedicated components/HOCs
Separate significantly different conditional UI/logic into distinct components
Colocate simple, localized logic or use inline definitions to reduce context switching
Choose field-level or form-level cohesion based on form requirements when using form libraries like react-hook-form
Use Component Composition instead of Props Drilling to reduce coupling
Files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (frontend/.cursorrules)
Use consistent return types for similar functions/hooks
Files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsxfrontend/src/pages/AdminPage/AdminRoutes.tsxfrontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-25T14:08:23.234Z
Learnt from: CR
Repo: Moadong/moadong PR: 0
File: frontend/.cursorrules:0-0
Timestamp: 2025-11-25T14:08:23.234Z
Learning: Applies to frontend/**/*.{ts,tsx,js,jsx} : Break down broad state management into smaller, focused hooks/contexts to reduce coupling
Applied to files:
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx
🧬 Code graph analysis (2)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (2)
frontend/src/hooks/queries/application/useGetApplicationlist.ts (1)
useGetApplicationlist(4-10)frontend/src/types/application.ts (1)
ApplicationFormItem(70-75)
frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx (3)
frontend/src/hooks/queries/application/useGetApplicationlist.ts (1)
useGetApplicationlist(4-10)frontend/src/apis/application/updateApplication.ts (1)
updateApplicationStatus(33-58)frontend/src/types/application.ts (1)
ApplicationFormItem(70-75)
🔇 Additional comments (1)
frontend/src/pages/AdminPage/AdminRoutes.tsx (1)
24-27: applicationFormId 편집용 동적 라우트 추가가 기존 네비게이션과 잘 맞습니다
path='application-list/:applicationFormId/edit'가ApplicationListTab에서 사용하는navigate(\/admin/application-list/${applicationFormId}/edit`)와 정확히 일치해서, 목록 → 개별 양식 편집 플로우가 자연스럽게 이어질 것 같습니다. 바로 아래 정적 라우트application-list/edit도 유지되고 있어 새 양식 생성(/admin/application-list/edit`)과 충돌 없이 공존할 수 있을 것으로 보입니다. 실제로 두 URL을 눌러 각각 올바른 화면이 뜨는지만 한 번씩만 수동 확인 부탁드립니다.
2 participants
#️⃣연관된 이슈
#901
📝작업 내용
새로고침시 선택했던 applicationFormId가 사라지는 문제
zustand store로 문제해결을 하려했지만 기존에 pathparam이 존재해 zustand에서 path로 변경
2025-11-28.16.00.37.mov
<문제>
2025-11-28.16.00.46.mov
<해결>
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.