[refacotor] Tanstack Query 리팩토링 및 API/Hooks 구조 개선#1058
[refacotor] Tanstack Query 리팩토링 및 API/Hooks 구조 개선#1058seongwon030 merged 19 commits intodevelop-fefrom
Conversation
- Mixpanel 관련 훅을 hooks/Mixpanel/ 폴더로 이동 - Scroll 관련 훅을 hooks/Scroll/ 폴더로 이동 - Application 관련 훅을 hooks/Application/ 폴더로 이동 - useValidateAnswers는 순수 함수이므로 utils로 이동 - 모든 import 경로 업데이트
…(applicants, application, auth, club, image) - apis/utils/apiHelpers.ts 추가하여 공통 에러 처리 로직 분리 - handleResponse와 withErrorHandling으로 중복 코드 제거 - 커스텀 에러 메시지 선택적 지원으로 기존 동작 유지 - 모든 API에 일관된 에러 처리 패턴 적용 - 43개 파일의 import 경로 업데이트 - 24개 API 파일을 5개 도메인별 파일로 통합 (applicants, application, auth, club, image) - apis/utils/apiHelpers.ts 추가하여 공통 에러 처리 로직 분리 - handleResponse와 withErrorHandling으로 중복 코드 제거 - 커스텀 에러 메시지 선택적 지원으로 기존 동작 유지 - 모든 API에 일관된 에러 처리 패턴 적용 - 43개 파일의 import 경로 업데이트
- API 함수에 사용자 친화적인 한국어 에러 메시지 추가 - queryKeys를 constants/queryKeys.ts로 분리 및 factory 패턴 적용 - hooks/queries → hooks/Queries 폴더 구조 정리
- apiHelpers/handleResponse: 빈 응답, JSON 파싱 실패, 잘못된 Content-Type에 대한 예외 처리 강화 - auth/getClubIdByToken: 응답 데이터 내 clubId 유효성 검사 로직 추가 - club/getClubDetail: 응답 데이터 내 club 객체 유효성 검사 로직 추가 - club/getClubList: 리스트 데이터 Null 체크 및 기본값(Fallback) 처리 추가
- App.tsx: QueryClient 전역 설정 추가 (staleTime: 60s, retry: 1) - Hooks: - 개별 훅 내 중복된 retry, staleTime 옵션 제거 - invalidateQueries 로직을 컴포넌트에서 훅 내부 onSuccess로 이동하여 응집도 향상 - 훅 내부 alert 제거 및 console.error로 로깅 전환 (SoC 준수) - Components: - mutate 호출 시 onError/onSuccess 콜백을 통해 사용자 알림(alert) 처리하도록 수정
- Hooks: useUpdateApplicationStatus 훅 추가 및 useDuplicateApplication 적용 - ApplicationListTab: 상태 변경 로직을 훅으로 위임하고 누락된 onDuplicate 핸들러 연결 - ApplicantDetailPage: 지원자 정보 수정 시 에러 핸들링(onError) 추가 - ApplicantsTab: 코드 정리 및 라우팅 관련 수정
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | 변경 요약 |
|---|---|
통합된 API 모듈 추가 frontend/src/apis/applicants.ts, frontend/src/apis/application.ts, frontend/src/apis/auth.ts, frontend/src/apis/club.ts, frontend/src/apis/image.ts |
여러 엔드포인트 헬퍼를 파일 단위로 통합(새 공개 함수 추가)하고 withErrorHandling/handleResponse 패턴 적용. 이미지 업로드(cover/feed/logo) 및 presigned upload flow 포함. |
삭제된 개별 API 파일들 frontend/src/apis/applicants/*, frontend/src/apis/application/*, frontend/src/apis/auth/*, frontend/src/apis/image/* |
통합으로 대체된 기존 per-endpoint 파일 다수 삭제(기존 default-export 유틸·함수 제거). |
API 유틸 추가 frontend/src/apis/utils/apiHelpers.ts |
handleResponse 및 withErrorHandling 추가로 응답 파싱과 에러 래핑 중앙화. |
쿼리 키 중앙화 frontend/src/constants/queryKeys.ts |
applicants/application/club용 재사용 가능한 query key 빌더 추가. |
React Query 훅 통합 / 신규 훅 frontend/src/hooks/Queries/* frontend/src/hooks/Queries/useApplicants.ts, .../useApplication.ts, .../useClub.ts, .../useClubCover.ts, .../useClubImages.ts |
개별 훅들을 영역별로 통합하고 신규 mutation 훅(예: useUploadLogo/useDeleteLogo, useDuplicateApplication 등) 추가. onSuccess에서 queryKeys 기반 invalidate 적용. |
삭제된 개별 훅 파일들 frontend/src/hooks/queries/..., frontend/src/hooks/PhotoList/*, frontend/src/hooks/InfoTabs/useAutoScroll.ts, frontend/src/hooks/PhotoModal/useModalNavigation.ts |
구형 개별 훅 파일 다수 제거(일부 기능이 통합 훅으로 이동). |
이미지 업로드 흐름 통합 frontend/src/apis/image.ts, 관련 훅 및 useClubImages/useClubCover 변경 |
presigned URL 획득 → S3 PUT(uploadToStorage) → 완료 API 호출로 통합; 실패/성공 경로와 cache invalidation(queryKeys.club.detail) 적용. |
App 설정 변경 frontend/src/App.tsx |
ScrollToTop 경로 수정 및 React Query QueryClient 기본 옵션 설정(staleTime: 60s, queries.retry: 1, mutations.retry: 0). |
| 컴포넌트·페이지 import 정리 다수 파일 (예: frontend/src/pages/**, frontend/src/components/**, frontend/src/hooks/**) |
Mixpanel 훅·Queries 훅 등 네임스페이스화(@/hooks/Mixpanel/*, @/hooks/Queries/*)로 import 경로 통일. |
| 스토리북·스타일·포맷 변경 여러 .stories.tsx, styles 파일들 |
Storybook meta/argTypes 확장, 포맷/임포트 정리, 일부 스타일·포맷 수정(무해한 변경). |
타입 이동/정리 frontend/src/types/club.ts, frontend/src/types/club.responses.ts |
ClubSearchResponse 타입 위치 변경(타입 재정의). |
기타 삭제 frontend/netlify.toml, frontend/src/constants/photoLayout.ts, frontend/src/constants/scrollSections.ts |
SPA 리다이렉트 설정 및 사진 레이아웃/스크롤 인덱스 관련 상수 삭제. |
Sequence Diagram(s)
sequenceDiagram
participant User as 사용자
participant UI as UI 컴포넌트
participant Hook as useUploadLogo 훅
participant API as 이미지 API (`frontend/src/apis/image.ts`)
participant S3 as S3 (프리사인드 URL)
participant QC as QueryClient
participant Backend as 서버
User->>UI: 이미지 선택 및 업로드 요청
UI->>Hook: mutate({ clubId, file })
Hook->>API: POST /logo/upload-url (getUploadUrl)
API-->>Hook: { presignedUrl, finalUrl }
Hook->>S3: PUT presignedUrl (uploadToStorage)
alt 업로드 성공
S3-->>Hook: 200 OK
Hook->>API: POST /logo/complete (completeUpload with fileUrl)
API->>Backend: 저장/처리
API-->>Hook: success
Hook->>QC: invalidateQueries(queryKeys.club.detail(clubId))
Hook-->>UI: onSuccess
UI-->>User: 업로드 완료 알림
else 업로드 실패
S3-->>Hook: error
Hook-->>UI: onError
UI-->>User: 에러 표시
end
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- [refactor] 이미지 업로드를 Presigned URL 직접 업로드 방식으로 개선 #906 — 이미지 업로드(presigned URL) 재구성 및 관련 훅/모듈 변경과 코드 영역이 직접적으로 중복됩니다.
- [release] FE v1.1.1 #733 — applicants API 및 훅(get/delete/update) 통합/재구성과 밀접한 연관성이 있습니다.
- [release] FE v1.1.6 릴리즈 #842 — 전반적인 API/훅 통합(apis/application, apis/auth, 쿼리 훅 재배치) 관련 코드 레벨 연관성이 높습니다.
Suggested reviewers
- lepitaaar
- oesnuj
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | 제목이 PR의 주요 변경사항(TanStack Query 리팩토링 및 API/Hooks 구조 개선)을 정확하게 요약하고 있습니다. |
| Linked Issues check | ✅ Passed | 코드 변경사항이 MOA-532 이슈의 주요 목표들(API 통합, QueryClient 설정, 에러 처리 분리, 책임 분리)을 충족합니다. |
| Out of Scope Changes check | ✅ Passed | 대부분의 변경사항이 리팩토링과 구조 개선에 집중하고 있으나, prettier/vite 설정 변경 등 부분적으로 범위 밖 변경이 포함되었습니다. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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: 2
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/ApplicantsTab.tsx (1)
77-80: delete 훅 인자에 clubId 전달은 오동작합니다.
useDeleteApplicants는 applicationFormId 기반 API를 호출하므로 clubId 전달 시 잘못된 엔드포인트로 요청됩니다.🛠 수정 제안
- const { mutate: deleteApplicants } = useDeleteApplicants(clubId!); + const { mutate: deleteApplicants } = useDeleteApplicants(applicationFormId!);
🤖 Fix all issues with AI agents
In `@frontend/src/hooks/Queries/useApplication.ts`:
- Around line 11-22: The useGetApplication hook currently falls back to
queryKeys.application.all when IDs are missing, causing cache collisions with
the application list; change it to always use the detail-specific key
(queryKeys.application.detail) even when clubId/applicationFormId are undefined
by passing deterministic placeholder values (or null-safe IDs) to that detail
key so it never equals queryKeys.application.all, keep the enabled flag as
!!clubId && !!applicationFormId, and continue to call getApplication(clubId!,
applicationFormId!) only when enabled; update references in this fix:
useGetApplication, queryKeys.application.detail, queryKeys.application.all, and
getApplication.
In
`@frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx`:
- Line 5: The file imports updateApplicationStatus but never uses it because the
component uses the useUpdateApplicationStatus hook's mutate function (aliased as
updateStatus); remove the unused import updateApplicationStatus from the top of
ApplicantsListTab.tsx, ensure there are no other references to
updateApplicationStatus in the file, and run a typecheck/lint to confirm no
unused-import errors remain; keep using useUpdateApplicationStatus and its
mutate/updateStatus as-is.
🧹 Nitpick comments (9)
frontend/src/apis/utils/apiHelpers.ts (1)
33-35: 응답이data래퍼가 아닐 때undefined반환 위험
일부 API가{ data: ... }형태가 아니거나null을 반환하면 현재 로직이undefined또는 런타임 오류를 만들 수 있습니다. 모든 응답이data래핑인지 확인하거나, 안전한 fallback을 고려해 주세요.♻️ 안전한 fallback 제안
- return result.data; + if (result && typeof result === 'object' && 'data' in result) { + return (result as { data: unknown }).data; + } + return result;frontend/src/pages/AdminPage/auth/LoginTab/LoginTab.tsx (1)
31-51:trackEvent호출 위치 검토 권장
trackEvent(ADMIN_EVENT.LOGIN_BUTTON_CLICKED)가try-catch-finally블록 이후에 호출되어, 로그인 성공/실패와 관계없이 항상 실행됩니다.성공/실패를 구분하여 트래킹하거나,
finally블록 내부로 이동하는 것이 더 명확할 수 있습니다. 현재 구현이 의도된 것이라면 무시해도 됩니다.♻️ 제안: 성공/실패 구분 트래킹
try { const { accessToken } = await login(userId, password); localStorage.setItem('accessToken', accessToken); + trackEvent(ADMIN_EVENT.LOGIN_BUTTON_CLICKED, { success: true }); alert('로그인 성공! 관리자 페이지로 이동합니다.'); navigate('/admin'); } catch (error: unknown) { console.error('로그인 실패:', error); + trackEvent(ADMIN_EVENT.LOGIN_BUTTON_CLICKED, { success: false }); let errorMessage = '로그인에 실패했습니다. 아이디 또는 비밀번호를 확인해주세요.'; if (error instanceof Error) { errorMessage = error.message; } alert(errorMessage); } finally { setLoading(false); } - trackEvent(ADMIN_EVENT.LOGIN_BUTTON_CLICKED);frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (1)
138-148: 컴포넌트 내부에서 styled-components 정의 지양
ActiveListBody와ActiveApplicationRow가 컴포넌트 함수 내부에서 정의되어 있어, 매 렌더링마다 새로 생성됩니다. 이는 불필요한 리렌더링을 유발하고 성능 저하 및 React 재조정(reconciliation) 문제를 일으킬 수 있습니다.♻️ 제안: styled-components를 컴포넌트 외부로 이동
+const ActiveListBody = styled(Styled.ApplicationList)` + border-top-left-radius: 0; +`; + +const ActiveApplicationRow = styled(ApplicationRowItem)` + &:hover { + background-color: `#f8f9fa`; + &:first-child { + border-top-right-radius: 20px; + } + } +`; + const ApplicationListTab = () => { // ... component logic - - const ActiveListBody = styled(Styled.ApplicationList)` - border-top-left-radius: 0; - `; - const ActiveApplicationRow = styled(ApplicationRowItem)` - &:hover { - background-color: `#f8f9fa`; - &:first-child { - border-top-right-radius: 20px; - } - } - `; return ( // ... ); };frontend/src/apis/club.ts (1)
47-75: 에러 메시지 언어 일관성 검토 필요
withErrorHandling의 두 번째 인자(로깅용 메시지)가 영어('Failed to update club description')이고,handleResponse의 메시지(사용자용)는 한국어입니다. 다른 API 파일들과 비교했을 때, 로깅 메시지도 한국어로 통일하는 것이 일관성 있어 보입니다.♻️ 로깅 메시지 한국어 통일 제안
export const updateClubDescription = async ( updatedData: ClubDescription, ): Promise<void> => { return withErrorHandling(async () => { const response = await secureFetch(`${API_BASE_URL}/api/club/description`, { method: 'PUT', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(updatedData), }); await handleResponse(response, '클럽 설명 수정에 실패했습니다.'); - }, 'Failed to update club description'); + }, '클럽 설명 수정 중 오류 발생:'); }; export const updateClubDetail = async ( updatedData: Partial<ClubDetail>, ): Promise<void> => { return withErrorHandling(async () => { const response = await secureFetch(`${API_BASE_URL}/api/club/info`, { method: 'PUT', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(updatedData), }); await handleResponse(response, '클럽 정보 수정에 실패했습니다.'); - }, 'Failed to update club detail'); + }, '클럽 정보 수정 중 오류 발생:'); };frontend/src/apis/application.ts (3)
7-27: 불필요한 spread 연산자Line 21의
[...answers]는 이미 배열인answers를 불필요하게 복사합니다.♻️ 간소화 제안
body: JSON.stringify({ - questions: [...answers], + questions: answers, }),
66-71: 동일 엔드포인트 중복 호출 함수
getActiveApplications와getApplicationOptions가 동일한 엔드포인트(/api/club/${clubId}/apply)를 호출합니다. 의도적인 설계라면 괜찮지만, 하나의 함수로 통합하거나 내부적으로 캐싱/재사용을 고려해볼 수 있습니다.Also applies to: 92-103
149-168: 상수 정의를 통한 타입 안정성 개선 필요현재 코드에서
currentStatus === 'ACTIVE'와 같이 하드코딩된 상태 문자열을 사용하고 있습니다.types/application.ts에는status: 'ACTIVE' | 'PUBLISHED' | 'UNPUBLISHED'타입이 정의되어 있음에도 불구하고, ApplicationListTab.tsx, ApplicantsListTab.tsx 등에서도 동일하게 하드코딩된 비교가 반복되고 있습니다.상태 값을 다음과 같이 상수로 정의하고 활용하는 것을 권장합니다:
const APPLICATION_STATUS = { ACTIVE: 'ACTIVE', PUBLISHED: 'PUBLISHED', UNPUBLISHED: 'UNPUBLISHED', } as const; type ApplicationStatus = typeof APPLICATION_STATUS[keyof typeof APPLICATION_STATUS];이를 통해 단일 정의 지점 원칙을 준수하고, 함수 매개변수 타입도
currentStatus: string대신currentStatus: ApplicationStatus로 개선하여 컴파일 타임 타입 체크를 강화할 수 있습니다.frontend/src/hooks/Queries/useClubCover.ts (1)
34-36: 에러 객체도 함께 로깅해 주세요.현재 문자열만 남아 원인 추적이 어렵습니다.
♻️ 제안 수정안
- onError: () => { - console.error('Error uploading cover'); - }, + onError: (error) => { + console.error('Error uploading cover', error); + }, ... - onError: () => { - console.error('Error deleting cover'); - }, + onError: (error) => { + console.error('Error deleting cover', error); + },Also applies to: 55-57
frontend/src/hooks/Queries/useClub.ts (1)
16-17: 동일 모듈을 두 번 기본 import 하는 부분은 정리하는 게 안전합니다.같은 파일을 다른 이름으로 가져오면 실제 동작이 동일해도 서로 다른 함수처럼 보일 수 있습니다. 하나로 통일하거나, 의도한 다른 유틸이 있다면 경로/네임드 export를 명확히 해주세요.
♻️ 정리 예시
-import convertToDriveUrl from '@/utils/convertGoogleDriveUrl'; -import convertGoogleDriveUrl from '@/utils/convertGoogleDriveUrl'; +import convertGoogleDriveUrl from '@/utils/convertGoogleDriveUrl'; ... - logo: convertToDriveUrl(club.logo), + logo: convertGoogleDriveUrl(club.logo),Also applies to: 61-64
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx
Show resolved
Hide resolved
- APPLICATION_FORM.ts → applicationForm.ts - CLUB_UNION_INFO.ts → clubUnionInfo.ts - INITIAL_FORM_DATA.ts → initialFormData.ts - 관련 import 경로 9개 파일 업데이트
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (1)
76-80: 오류 발생 시 사용자에게 잘못된 메시지가 표시될 수 있습니다.API 오류 발생 시 빈 옵션으로 모달을 열면 사용자에게 "지원 가능한 분야가 없습니다" 메시지가 표시됩니다. 이는 실제 상황(API 호출 실패)과 다른 정보를 전달하여 사용자 혼란을 야기할 수 있습니다.
🔧 오류 시 사용자에게 명확한 피드백 제공 제안
} catch (e) { - setApplicationOptions([]); - setIsApplicationModalOpen(true); console.error('지원서 옵션 조회 중 오류가 발생했습니다.', e); + alert('지원서 목록을 불러오는 중 오류가 발생했습니다. 다시 시도해주세요.'); }frontend/src/pages/ClubDetailPage/ClubDetailPage.tsx (1)
51-57: 에러 처리 조건문 순서 검토 필요현재
!clubDetail체크가error체크보다 먼저 실행됩니다. API 에러 발생 시clubDetail이undefined이면 에러 메시지 대신null이 반환되어 사용자에게 빈 화면만 표시됩니다.🔧 에러 체크 우선 처리 제안
+ if (error) { + return <div>에러가 발생했습니다.</div>; + } + if (!clubDetail) { return null; } - - if (error) { - return <div>에러가 발생했습니다.</div>; - }frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTab.tsx (1)
75-85:useUpdateClubDetail훅에서 이미 캐시 무효화를 처리하고 있으므로 중복 제거 필요.
useUpdateClubDetail훅(useClub.ts 86-104줄)의onSuccess콜백에서queryClient.invalidateQueries를 실행하고 있습니다. 이 컴포넌트에서도 같은 작업을 반복하고 있으므로, 78-80줄의invalidateQueries호출과 3줄, 33줄의useQueryClient관련 코드를 제거하세요.
🧹 Nitpick comments (10)
frontend/src/pages/AdminPage/auth/LoginTab/LoginTab.tsx (1)
57-129: 로딩 중 제출 버튼 비활성화로 UX 개선 권장
loading가드가 있어도 버튼은 계속 클릭 가능해서 불필요한 재클릭을 유도할 수 있습니다. 버튼 비활성화 +aria-busy로 상태를 명확히 표현하는 편이 좋습니다.♻️ 제안 수정
- <Button width='100%' type='submit'> + <Button width='100%' type='submit' disabled={loading} aria-busy={loading}> {loading ? '로그인 중...' : '로그인'} </Button>frontend/src/utils/getDeadLineText.ts (1)
25-25:formatStr는const로 선언하는 편이 명확합니다.재할당이 없어서 불변 선언이 더 적합합니다.
♻️ 제안 변경
- let formatStr = hour === 0 && minute === 0 ? 'M월 d일' : 'M월 d일 HH:mm'; + const formatStr = hour === 0 && minute === 0 ? 'M월 d일' : 'M월 d일 HH:mm';frontend/src/components/application/modals/ApplicationSelectModal.tsx (1)
51-51: 선택 사항: Boolean prop 간소화
closeOnBackdrop={true}는closeOnBackdrop로 간소화할 수 있습니다.♻️ 제안된 변경
- <PortalModal isOpen={isOpen} onClose={onClose} closeOnBackdrop={true}> + <PortalModal isOpen={isOpen} onClose={onClose} closeOnBackdrop>frontend/src/pages/ClubDetailPage/components/ClubApplyButton/ClubApplyButton.tsx (1)
109-112: async 이벤트 핸들러 처리 확인 필요
handleApplyButtonClick이 async 함수인데onClick에서 반환되는 Promise가 처리되지 않습니다. React에서는 일반적으로 문제가 되지 않지만, 명시적으로 void 처리하는 것이 좋습니다.♻️ 제안된 변경
<Styled.ApplyButton disabled={isRecruitmentUpcoming || isRecruitmentClosed} - onClick={handleApplyButtonClick} + onClick={() => void handleApplyButtonClick()} >frontend/src/components/common/CustomDropDown/CustomDropDown.stories.tsx (2)
100-104: 메뉴 아이템 렌더링에서 args.options 대신 OPTIONS를 사용하고 있습니다.
args.options가 Storybook 컨트롤에서 변경되더라도 메뉴 아이템은 항상 하드코딩된OPTIONS를 사용하여 렌더링됩니다. 컨트롤 변경이 반영되도록args.options를 사용하는 것이 좋습니다.♻️ 수정 제안
- {OPTIONS.map((option) => ( - <CustomDropDown.Item key={option.value} value={option.value}> - {option.label} - </CustomDropDown.Item> - ))} + {(args.options as readonly { label: string; value: string }[]).map((option) => ( + <CustomDropDown.Item key={option.value} value={option.value}> + {option.label} + </CustomDropDown.Item> + ))}
81-82: selectedLabel 계산도 args.options를 사용해야 일관성이 유지됩니다.위에서 언급한 것과 동일하게, 선택된 라벨을 찾을 때도
OPTIONS대신args.options를 사용해야 Storybook 컨트롤에서 옵션을 변경했을 때 정상적으로 동작합니다.♻️ 수정 제안
const selectedLabel = - OPTIONS.find((opt) => opt.value === selected)?.label || '선택하세요'; + (args.options as readonly { label: string; value: string }[]).find((opt) => opt.value === selected)?.label || '선택하세요';frontend/src/components/common/SearchField/SearchField.stories.tsx (1)
44-116: 세 스토리에서 동일한 render 함수가 중복되고 있습니다.Default, WithValue, CustomPlaceholder 스토리 모두 동일한 render 로직을 사용하고 있습니다. 공통 render 함수를 추출하면 코드 중복을 줄일 수 있습니다.
♻️ 공통 render 함수 추출 제안
// 공통 render 함수 정의 const renderSearchField = (args: typeof Default.args) => { const [value, setValue] = useState(args?.value ?? ''); return ( <SearchField {...args} value={value} onChange={(newValue) => { setValue(newValue); args?.onChange?.(newValue); }} /> ); }; // 각 스토리에서 재사용 export const Default: Story = { args: { /* ... */ }, render: renderSearchField, }; export const WithValue: Story = { args: { /* ... */ }, render: renderSearchField, };frontend/src/components/common/InputField/InputField.stories.tsx (1)
80-220: 다섯 개의 스토리에서 render 함수가 중복됩니다.Default, WithLabel, Password, ErrorState, WithMaxLength 스토리 모두 거의 동일한 render 로직을 사용하고 있습니다. SearchField.stories.tsx와 마찬가지로 공통 render 함수를 추출하여 유지보수성을 높일 수 있습니다.
♻️ 공통 render 함수 추출 제안
// 공통 render 함수 정의 const renderInputField: Story['render'] = (args) => { const [value, setValue] = useState(args.value || ''); return ( <InputField {...args} value={value} onChange={(e) => { setValue(e.target.value); args.onChange?.(e); }} onClear={() => { setValue(''); args.onClear?.(); }} /> ); }; // 각 스토리에서 재사용 export const Default: Story = { args: { /* ... */ }, render: renderInputField, };frontend/src/components/common/Modal/ModalLayout.stories.tsx (2)
4-32: children 컨트롤 타입을 JSX에 맞게 조정하세요
LongContent스토리에서children에 JSX<div>요소를 전달하고 있습니다.control: 'text'는 직렬화 불가능한 ReactNode/JSX 콘텐츠에 적합하지 않으므로 Storybook에서 제대로 작동하지 않습니다.control: false로 비활성화하는 것이 권장됩니다.♻️ 제안 변경
children: { - control: 'text', + control: false, description: '모달 내부에 렌더링될 컨텐츠입니다.', },
67-107: onClose 액션 로깅을 위해 args에서 함수 제거
onClose: () => {}를 args에 제공하면 Storybook의argTypes.onClose.action설정이 무시되어 액션 로그가 기록되지 않습니다. argTypes에서 설정한 액션 로깅이 정상 작동하려면 args에서onClose를 생략해야 합니다. 다른 스토리들과의 일관성을 위해서도 제거하는 것이 좋습니다.♻️ 제안 변경
args: { - onClose: () => {}, title: '내용이 긴 모달 레이아웃',
- clubId/applicationFormId가 없을 때 queryKeys.application.all 대신 고유한 키 사용 - useGetApplicationlist와 캐시 키 충돌로 인한 타입 불일치 문제 해결 - enabled가 false일 때도 명확한 캐시 분리 보장
- error 체크를 clubDetail 체크보다 먼저 수행 - API 에러 발생 시 에러 메시지가 정상적으로 표시되도록 개선
- club.responses.ts 파일 삭제 - ClubSearchResponse를 club.ts로 이동 - 단일 사용처만 있는 타입의 불필요한 파일 분리 제거
- ClubFeed 컴포넌트 내부로 모달 상태 관리 로직 이동 - hooks/PhotoList/usePhotoModal.ts 파일 삭제 - 단일 사용처만 있는 불필요한 훅 추상화 제거
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/pages/ClubDetailPage/components/ClubFeed/ClubFeed.tsx`:
- Around line 11-18: The modal index isn't clamped when the feed list changes,
causing out-of-range access in PhotoModal; update the open(i) handler to clamp
the incoming i (using current feed length) and add a useEffect that watches the
feed array/its length to either clamp setIndex to Math.min(currentIndex,
feeds.length - 1) or call setIsOpen(false) when feeds.length === 0; reference
state vars index, setIndex, isOpen, setIsOpen, the open and close functions, and
the PhotoModal usage to locate where to apply these fixes.
- feed 배열 변경 시 index가 범위를 벗어나지 않도록 useEffect 추가 - 빈 배열일 때 모달 자동 닫기 및 index 초기화 - PhotoModal에서 잘못된 배열 접근 방지
suhyun113
left a comment
suhyun113
left a comment
There was a problem hiding this comment.
변경된 작업이 정말 많네요. 수고 많으셨습니다ㅏ👍
변경 범위가 큰 리팩토링이다 보니, 다음부터는 PR을 조금 더 나눠서 올려주시면 리뷰하기 더 좋을 것 같습니다! 각 수정 사항마다 어떤 문제를 해결하기 위해 변경한건지 간단히 설명이 있으면 이해하는 데 도움이 될 것 같아요!
There was a problem hiding this comment.
useGetApplicationlist 는 네이밍 규칙을 생각했을때 useGetApplicationList가 어떨까요?
| export const getClubDetail = async (clubId: string): Promise<ClubDetail> => { | ||
| return withErrorHandling(async () => { | ||
| const response = await fetch(`${API_BASE_URL}/api/club/${clubId}`); | ||
| const data = await handleResponse( | ||
| response, | ||
| '클럽 정보를 불러오는데 실패했습니다.', | ||
| ); | ||
| if (!data?.club) { | ||
| throw new Error('클럽 정보를 가져올 수 없습니다.'); | ||
| } | ||
| return data.club; | ||
| }, 'Error fetching club details'); | ||
| }; | ||
|
|
||
| export const getClubList = async ( | ||
| keyword: string = '', | ||
| recruitmentStatus: string = 'all', | ||
| category: string = 'all', | ||
| division: string = 'all', | ||
| ) => { |
There was a problem hiding this comment.
API들이 분산되어있어 찾기 불편했는데 도메인별 API 파일로 통합되면서 관련 API를 한 곳에서 파악할 수 있게된거 좋습니다ㅏ
|
|
||
| export const withErrorHandling = async <T>( | ||
| apiCall: () => Promise<T>, | ||
| errorMessage: string, |
There was a problem hiding this comment.
호출부에서 errorMessage를 넣은 구조 덕분에 어떤 API 호출에서 발생한 에러인지 추적하기 쉬워진거 같습니다!
| const queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| staleTime: 60 * 1000, | ||
| retry: 1, | ||
| }, | ||
| mutations: { | ||
| retry: 0, | ||
| }, |
There was a problem hiding this comment.
수정이나 삭제 요청은 왜 재시도 하지 않도록 설정하는건가요?
There was a problem hiding this comment.
서버에서 성공했어도 네트워크 에러가 떠서 클라이언트가 응답을 못 받았다고 헀을 때 재시도한다면, 서버에서 이미 삭제되었으니 에러를 내뱉을 거에요.
여기서 만약 클라이언트가 에러처리를 했어도 재시도를 설정한다면 어떤 요청에서 에러가 떴는지 알기가 힘들 것 같아요.
| @@ -0,0 +1,22 @@ | |||
| export const queryKeys = { | |||
There was a problem hiding this comment.
queryKey 상수 객체로 관리하는거 좋아요~
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: queryKeys.applicants.detail(applicationFormId), |
There was a problem hiding this comment.
mutation의 onSuccess에서 invalidateQueries를 사용한 이유가 뭔가요?
There was a problem hiding this comment.
데이터 정합성 문제입니다. invalidateQueries는 mutation 요청을 날리고 나서 서버의 상태로 동기화하는 작업인데 지원자 삭제 요청을 하고 또다른 관리자가 지원자 수정/삭제 요청을 했다면 서버 상태가 바뀌었기 떄문에 동기화해야 합니다.
There was a problem hiding this comment.
여러 관리자가 동시에 작업하는 상황을 고려해 클라이언트가 중간 상태를 캐시하지 않도록 하기 위해 mutation 이후 캐시를 무효화하고 서버의 최종 상태를 기준으로 다시 동기화하는거네여! 이해했슴다ㅏ
There was a problem hiding this comment.
Actionable comments posted: 0
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/ApplicationListTab/ApplicationListTab.tsx (1)
67-79:useUpdateApplicationStatus훅을 사용하도록 변경 필요
handleDeleteApplication과handleDuplicateApplication은 mutation 훅을 사용하지만,handleToggleClick은 직접 API를 호출하고 수동으로queryClient.invalidateQueries를 수행합니다. 이는 PR 목표인 "invalidateQueries 호출을 훅 내부로 이전"과 일치하지 않습니다.또한 Line 73의 하드코딩된 쿼리 키
['applicationForm']대신queryKeys.application.all을 사용해야 합니다.🔧 훅 사용으로 변경 제안
import { useDeleteApplication, useDuplicateApplication, useGetApplicationList, + useUpdateApplicationStatus, } from '@/hooks/Queries/useApplication';const ApplicationListTab = () => { - const queryClient = useQueryClient(); const navigate = useNavigate(); const { data: allforms, isLoading, isError, error } = useGetApplicationList(); const { mutate: deleteApplication } = useDeleteApplication(); const { mutate: duplicateApplication } = useDuplicateApplication(); + const { mutate: updateStatus } = useUpdateApplicationStatus();- const handleToggleClick = async ( + const handleToggleClick = ( applicationFormId: string, currentStatus: string, ) => { - try { - await updateApplicationStatus(applicationFormId, currentStatus); - queryClient.invalidateQueries({ queryKey: ['applicationForm'] }); - setOpenMenuId(null); - } catch (error) { - console.error('지원서 상태 변경 실패:', error); - alert('상태 변경에 실패했습니다.'); - } + updateStatus( + { applicationFormId, currentStatus }, + { + onSuccess: () => { + setOpenMenuId(null); + }, + onError: () => { + alert('상태 변경에 실패했습니다.'); + }, + }, + ); };위 변경 적용 시, Line 3의
useQueryClientimport와 Line 5의updateApplicationStatusimport도 제거할 수 있습니다.
♻️ Duplicate comments (1)
frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (1)
5-5: 사용되지 않는 import 제거 필요
updateApplicationStatus가 import되었지만, 실제로는useUpdateApplicationStatus훅의mutate함수(updateStatus, Line 26)를 사용하고 있습니다.-import { updateApplicationStatus } from '@/apis/application';
🧹 Nitpick comments (2)
frontend/src/pages/AdminPage/tabs/ApplicationListTab/ApplicationListTab.tsx (1)
133-143: styled components를 컴포넌트 바깥으로 이동 권장
ActiveListBody와ActiveApplicationRow가 컴포넌트 내부에서 정의되어 매 렌더링마다 새로 생성됩니다. 이는 불필요한 리렌더링과 성능 저하를 유발할 수 있습니다.♻️ 컴포넌트 외부로 이동
+const ActiveListBody = styled(Styled.ApplicationList)` + border-top-left-radius: 0; +`; + +const ActiveApplicationRow = styled(ApplicationRowItem)` + &:hover { + background-color: `#f2f2f2`; + &:first-child { + border-top-right-radius: 20px; + } + } +`; + const ApplicationListTab = () => { // ... - const ActiveListBody = styled(Styled.ApplicationList)` - border-top-left-radius: 0; - `; - const ActiveApplicationRow = styled(ApplicationRowItem)` - &:hover { - background-color: `#f2f2f2`; - &:first-child { - border-top-right-radius: 20px; - } - } - `;frontend/src/pages/AdminPage/tabs/ApplicantsTab/ApplicantsListTab/ApplicantsListTab.tsx (1)
138-148: styled components를 컴포넌트 바깥으로 이동 권장
ActiveListBody와ActiveApplicationRow가 컴포넌트 내부에서 정의되어 매 렌더링마다 새로 생성됩니다.♻️ 컴포넌트 외부로 이동
+const ActiveListBody = styled(Styled.ApplicationList)` + border-top-left-radius: 0; +`; + +const ActiveApplicationRow = styled(ApplicationRowItem)` + &:hover { + background-color: `#f8f9fa`; + &:first-child { + border-top-right-radius: 20px; + } + } +`; + const ApplicationListTab = () => { // ... - const ActiveListBody = styled(Styled.ApplicationList)` - border-top-left-radius: 0; - `; - const ActiveApplicationRow = styled(ApplicationRowItem)` - &:hover { - background-color: `#f8f9fa`; - &:first-child { - border-top-right-radius: 20px; - } - } - `;
파일이 많아지면서 그만큼 읽고 찾아 들어가는 시간이 늘어나는 느낌을 받았습니다. 그래서 하는김에 전체적인 폴더 정리도 진행했습니다. 리팩토링 작업이 점진적으로 이뤄지면 좋겠지만 코드가 추가되다 보면 사실 점진적으로 리팩토링하기도 쉽지 않은 것 같습니다. 그래서 할거면 한 번에 헤치우자라는 마음으로 했는데 PR단위가 많이 크긴 하네요. 다음엔 좀 나누어서 작업해보겠습니다~ |
2 participants
#️⃣연관된 이슈
📝작업 내용
파일 구조
1. API 폴더 구조 개선 및 공통 에러 처리 로직 분리
handleResponse: 응답 처리 및 에러 핸들링withErrorHandling: try-catch 래퍼 함수2. Hooks 폴더 구조 도메인별 재구성
hooks/Queries/하위에 도메인별 커스텀 훅 정리3. React Query 설정 전역화
App.tsx에 QueryClient 전역 설정 추가4. 에러 핸들링 로직 UI 위임 (관심사 분리)
5. invalidateQueries 로직 훅 내부로 이동
useUpdateClubDescription, useUpdateClubDetail, useUpdateApplicationStatus6. 지원서 관리 로직 개선
useUpdateApplicationStatus훅 신규 추가ApplicantDetailPage.tsx에 지원자 정보 수정 시 에러 핸들링 추가7. API 응답 처리 안정성 강화
apiHelpers.ts의handleResponse에 빈 응답, JSON 파싱 실패, 잘못된 Content-Type 예외 처리 추가auth.ts,club.ts에 응답 데이터 유효성 검사 로직 추가중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
New Features
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.