[REFACTOR] 대시보드 지원자 목록 실시간 동기화 및 필터링 개선 (#140)#161
Conversation
- 지원자 데이터 캐싱(staleTime) 및 자동 갱신(refetchInterval) 적용 - 상태별 카운트 계산 로직 추가 - 선택된 필터 상태(frontend) 반영
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughStatus types were refactored from localized strings to typed tokens and propagated across components, hooks, and mocks. Dashboard summary fetching and hook were added with polling. Applicant filtering moved client-side with memoized counts and conditional empty states. API endpoints were adjusted to relative club routes. Minor UI updates reflect new status labels and empty states. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin User
participant UI as Dashboard Page
participant Hook as useApplicants
participant API as /api/clubs/{id}/applicants
Note over UI,Hook: Client-side filtering with polling
UI->>Hook: useApplicants(clubId, status)
Hook->>API: fetch applicants (poll every 30s)
API-->>Hook: applicants[]
Hook->>Hook: memoize counts and filteredData
Hook-->>UI: { data: filteredData, counts, isLoading, error }
UI->>UI: on filter change, re-render using memoized filteredData
sequenceDiagram
autonumber
actor Admin as Admin User
participant UI as Dashboard Summary
participant Hook as useDashboardSummary
participant API as /api/clubs/{id}/dashboard
Note over UI,Hook: Data-driven summary with polling
UI->>Hook: useDashboardSummary(clubId)
Hook->>API: fetch summary (every 30s)
API-->>Hook: { applicants, documents, startDay, endDay }
Hook-->>UI: data | isLoading | error
alt dates missing
UI->>UI: SummaryCard(isEmpty = true)
else dates present
UI->>UI: SummaryCard(recruitmentPeriod)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
Summary of ChangesHello @kanghaeun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 대시보드 지원자 목록의 실시간 동기화 및 필터링 기능을 전반적으로 개선합니다. React Query를 활용하여 지원자 데이터를 주기적으로 업데이트하고, 클라이언트 측에서 효율적으로 필터링하도록 변경했습니다. 또한, 대시보드 요약 정보를 표시하는 기능을 추가하고, 지원자 상태 관리 및 UI를 개선하여 사용자 경험을 향상시켰습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 대시보드 지원자 목록의 실시간 동기화 및 필터링 기능을 개선하는 리팩터링을 포함하고 있습니다. react-query를 사용하여 30초마다 데이터를 새로고침하고, 클라이언트 사이드에서 필터링을 수행하도록 변경되었습니다. 전반적으로 코드의 효율성과 유지보수성이 향상되었습니다. 몇 가지 개선점을 제안합니다. 하드코딩된 clubId를 동적으로 처리하고, 가독성을 높이기 위한 리팩터링, 그리고 데이터 처리 효율을 높이는 방안을 검토해 주시기 바랍니다.
| }; | ||
|
|
||
| export const ApplicationStatusFilter = ({ option, onOptionChange }: Props) => { | ||
| const { counts } = useApplicants(1); |
There was a problem hiding this comment.
clubId가 1로 하드코딩되어 있습니다. 이 컴포넌트가 다른 클럽에서도 재사용될 수 있도록 clubId를 props로 받거나 URL 파라미터에서 동적으로 가져오는 방식으로 수정해야 합니다.
| import type { DashboardCard } from '@/pages/admin/Dashboard/types/dashboard'; | ||
|
|
||
| export const DashboardSummarySection = () => { | ||
| const clubId = '1'; |
There was a problem hiding this comment.
clubId가 '1'로 하드코딩되어 있습니다. 이 값은 동적으로 결정되어야 합니다. 예를 들어, react-router-dom의 useParams 훅을 사용하여 URL에서 clubId를 가져오는 것을 고려해 보세요.
| {filterOption === 'ALL' | ||
| ? '아직 지원자가 없습니다.' | ||
| : `${filterOption === 'PENDING' ? '심사중' : filterOption === 'APPROVED' ? '합격' : '불합격'} 지원자가 없습니다.`} |
There was a problem hiding this comment.
중첩된 삼항 연산자를 사용하여 빈 목록 메시지를 생성하는 것은 가독성을 저해하고 유지보수를 어렵게 만듭니다. 상태 값에 따른 표시 문자열을 객체로 매핑하여 사용하면 코드가 더 명확해지고 관리하기 쉬워집니다.
예를 들어, 컴포넌트 상단에 다음과 같은 맵을 정의하고 사용할 수 있습니다.
const statusLabels = {
PENDING: '심사중',
APPROVED: '합격',
REJECTED: '불합격',
};
// JSX 내부
{
filterOption === 'ALL'
? '아직 지원자가 없습니다.'
: `${statusLabels[filterOption]} 지원자가 없습니다.`
}| if (error) { | ||
| return <Wrapper>Error: {error.message}</Wrapper>; | ||
| } |
There was a problem hiding this comment.
API 호출 실패 시 간단한 텍스트로 에러를 표시하고 있습니다. 사용자 경험을 개선하기 위해, 에러 상태를 명확하게 보여주는 별도의 재사용 가능한 에러 컴포넌트를 만들어 사용하는 것을 고려해 보세요.
| const counts = useMemo(() => { | ||
| if (!data) return { ALL: 0, PENDING: 0, APPROVED: 0, REJECTED: 0 }; | ||
|
|
||
| return { | ||
| ALL: data.length, | ||
| PENDING: data.filter((a) => a.status === 'PENDING').length, | ||
| APPROVED: data.filter((a) => a.status === 'APPROVED').length, | ||
| REJECTED: data.filter((a) => a.status === 'REJECTED').length, | ||
| }; | ||
| }, [data]); |
There was a problem hiding this comment.
지원자 상태별 수를 계산하기 위해 filter를 여러 번 호출하고 있습니다. 지원자 수가 많아질 경우 성능에 영향을 줄 수 있습니다. 배열을 한 번만 순회하는 reduce 메소드를 사용하면 더 효율적으로 계산할 수 있습니다.
const counts = useMemo(() => {
if (!data) return { ALL: 0, PENDING: 0, APPROVED: 0, REJECTED: 0 };
return data.reduce(
(acc, applicant) => {
acc[applicant.status] += 1;
return acc;
},
{
ALL: data.length,
PENDING: 0,
APPROVED: 0,
REJECTED: 0,
},
);
}, [data]);| import type { DashboardSummary } from '@/pages/admin/Dashboard/types/dashboard'; | ||
| import type { UseApiQueryResult } from '@/types/useApiQueryResult'; | ||
|
|
||
| export const useDashboardSummary = (clubId: string): UseApiQueryResult<DashboardSummary | null> => { |
There was a problem hiding this comment.
clubId의 타입이 string으로 정의되어 있습니다. 반면 useApplicants 훅에서는 number 타입을 사용하고 있어 일관성이 부족합니다. ID와 같이 여러 곳에서 사용되는 값의 타입은 하나로 통일하는 것이 좋습니다. URL 경로에 사용되므로 string으로 통일하는 것을 고려해 보세요.
![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)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantList.tsx (1)
16-16: Remove hardcoded clubId values.Hardcoded
clubId = 1appears on lines 16 and 22, breaking modularity and preventing this component from working with different clubs.Pass
clubIdas a prop:type Props = { + clubId: number; filterOption: ApplicationFilterOption; }; -export const ApplicantList = ({ filterOption }: Props) => { +export const ApplicantList = ({ clubId, filterOption }: Props) => { const navigate = useNavigate(); - const { data: applicants, isLoading, error } = useApplicants(1, filterOption); + const { data: applicants, isLoading, error } = useApplicants(clubId, filterOption); if (isLoading) return <LoadingSpinner />; if (error) return <div>에러발생 : {error.message}</div>; const handleItemClick = (applicantId: number) => { - const clubId = 1; navigate(`/admin/clubs/${clubId}/applicants/${applicantId}`); };Also applies to: 22-22
🧹 Nitpick comments (4)
src/pages/admin/Dashboard/api/dashboard.ts (1)
3-11: Consider adding runtime validation for API response.The function types the response as
DashboardSummarybutresponse.json()returnsPromise<any>. If the API contract changes or returns unexpected data, this could cause runtime errors downstream.Consider adding runtime validation using a schema validator like Zod:
+import { z } from 'zod'; + +const DashboardSummarySchema = z.object({ + totalApplicantCount: z.number(), + pendingApplicationCount: z.number(), + startDay: z.string(), + endDay: z.string(), +}); + export const fetchDashboardSummary = async (clubId: string): Promise<DashboardSummary> => { const response = await fetch(`/api/clubs/${clubId}/dashboard`); if (!response.ok) { throw new Error('대시보드 요약 정보를 불러오는 데 실패했습니다.'); } - return response.json(); + const data = await response.json(); + return DashboardSummarySchema.parse(data); };src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (1)
6-12: Consider adding anenabledoption to prevent unnecessary fetches.The query will execute even if
clubIdis empty or invalid. Consider adding validation:export const useDashboardSummary = (clubId: string): UseApiQueryResult<DashboardSummary | null> => { const { data, isLoading, error } = useQuery<DashboardSummary | undefined>({ queryKey: ['dashboardSummary', clubId], queryFn: () => fetchDashboardSummary(clubId), + enabled: Boolean(clubId), staleTime: 1000 * 60 * 5, refetchInterval: 30000, });src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantList.tsx (1)
50-52: Simplify complex nested ternary for better readability.The triple-nested ternary is hard to read and maintain.
Extract to a helper function or object lookup:
+const EMPTY_MESSAGES: Record<ApplicationFilterOption, string> = { + ALL: '아직 지원자가 없습니다.', + PENDING: '심사중 지원자가 없습니다.', + APPROVED: '합격 지원자가 없습니다.', + REJECTED: '불합격 지원자가 없습니다.', +}; + return ( <Container> {/* ... */} {applicants.length > 0 ? ( {/* ... */} ) : ( - <EmptyMessage> - {filterOption === 'ALL' - ? '아직 지원자가 없습니다.' - : `${filterOption === 'PENDING' ? '심사중' : filterOption === 'APPROVED' ? '합격' : '불합격'} 지원자가 없습니다.`} - </EmptyMessage> + <EmptyMessage>{EMPTY_MESSAGES[filterOption]}</EmptyMessage> )} </Container> );src/pages/admin/Dashboard/hooks/useApplicants.ts (1)
33-41: Compute counts in one pass to avoid repeated array scans
data.filter(...)is executed three times on every render/refetch. That creates temporary arrays and walks the list multiple times, which is unnecessary as the data set grows. A singlereducecan accumulate all four buckets in one traversal.- return { - ALL: data.length, - PENDING: data.filter((a) => a.status === 'PENDING').length, - APPROVED: data.filter((a) => a.status === 'APPROVED').length, - REJECTED: data.filter((a) => a.status === 'REJECTED').length, - }; + return data.reduce( + (acc, applicant) => { + acc.ALL += 1; + acc[applicant.status] += 1; + return acc; + }, + { ALL: 0, PENDING: 0, APPROVED: 0, REJECTED: 0 }, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusButton.tsx(1 hunks)src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusToggle.tsx(2 hunks)src/pages/admin/Dashboard/api/dashboard.ts(1 hunks)src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantList.tsx(2 hunks)src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantListItem.tsx(3 hunks)src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx(2 hunks)src/pages/admin/Dashboard/components/DashboardSummarySection/SummaryCard.tsx(3 hunks)src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx(1 hunks)src/pages/admin/Dashboard/hooks/useApplicants.ts(1 hunks)src/pages/admin/Dashboard/hooks/useDashboardSummary.ts(1 hunks)src/pages/admin/Dashboard/types/dashboard.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusButton.tsx (2)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx (1)
Props(6-9)src/pages/admin/Dashboard/types/dashboard.ts (2)
StatusLabel(10-10)ApplicationFilterOption(12-12)
src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx (4)
src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (1)
useDashboardSummary(6-15)src/shared/components/LoadingSpinner.tsx (1)
LoadingSpinner(19-19)src/utils/dateUtils.ts (1)
formatDate(1-4)src/pages/admin/Dashboard/components/DashboardSummarySection/SummaryCard.tsx (1)
SummaryCard(8-19)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantListItem.tsx (1)
src/pages/admin/Dashboard/types/dashboard.ts (1)
ApplicantData(14-22)
src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (3)
src/types/useApiQueryResult.ts (1)
UseApiQueryResult(1-6)src/pages/admin/Dashboard/types/dashboard.ts (1)
DashboardSummary(24-29)src/pages/admin/Dashboard/api/dashboard.ts (1)
fetchDashboardSummary(3-11)
src/pages/admin/Dashboard/api/dashboard.ts (1)
src/pages/admin/Dashboard/types/dashboard.ts (1)
DashboardSummary(24-29)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantList.tsx (3)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantListItem.tsx (1)
ApplicantListItem(14-34)src/shared/components/Button/index.tsx (1)
Button(17-64)src/styles/theme.ts (1)
theme(3-10)
src/pages/admin/Dashboard/hooks/useApplicants.ts (3)
src/types/useApiQueryResult.ts (1)
UseApiQueryResult(1-6)src/pages/admin/Dashboard/types/dashboard.ts (3)
ApplicantCounts(31-36)ApplicationFilterOption(12-12)ApplicantData(14-22)src/pages/admin/Dashboard/api/applicant.ts (1)
fetchApplicants(3-13)
src/pages/admin/Dashboard/components/DashboardSummarySection/SummaryCard.tsx (1)
src/pages/admin/Dashboard/types/dashboard.ts (1)
DashboardCard(3-8)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx (2)
src/pages/admin/Dashboard/hooks/useApplicants.ts (1)
useApplicants(15-50)src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilterButton.tsx (1)
ApplicantFilterButton(11-17)
src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusToggle.tsx (2)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx (1)
Props(6-9)src/pages/admin/Dashboard/types/dashboard.ts (1)
ApplicationFilterOption(12-12)
🔇 Additional comments (7)
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantListItem.tsx (1)
8-12: LGTM! Good separation of concerns.The introduction of
STATUS_LABELmapping improves maintainability by separating data representation from display labels. UsingApplicationStatusenum keys in styles instead of Korean strings enhances type safety.Also applies to: 31-31, 56-67
src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (1)
6-12: Verify polling interval against requirements.The code sets
refetchInterval: 30000(30 seconds), but linked issue #140 specifies 15 seconds for polling. Please confirm whether 30 seconds is the intended interval or if this should be updated to match the original requirement.src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx (1)
18-18: LGTM! Dynamic counts improve user experience.Displaying real-time counts for each filter option provides immediate feedback and helps users understand data distribution.
Also applies to: 24-24, 30-30, 36-36
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicantList.tsx (1)
34-60: LGTM! Good UX improvement with conditional rendering.The empty state messages and conditional button rendering improve user experience by providing clear feedback when no applicants match the filter.
src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx (1)
14-22: LGTM! Proper loading and error state handling.The component correctly handles loading and error states before rendering data, and computes the recruitment period with appropriate fallbacks.
Also applies to: 31-46
src/pages/admin/Dashboard/types/dashboard.ts (1)
10-12: LGTM! Well-structured type definitions.The new types (
StatusLabel,ApplicationStatus,ApplicationFilterOption,DashboardSummary,ApplicantCounts) provide clear contracts and improve type safety across the dashboard. The change fromACCEPTEDtoAPPROVEDis consistently applied.Also applies to: 21-21, 24-36
src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusButton.tsx (1)
2-2: LGTM! Type alignment with refactored dashboard types.The component correctly adopts the new
StatusLabelandApplicationFilterOptiontypes, maintaining consistency with the dashboard's refactored type system.Also applies to: 5-8
| import type { ApplicationFilterOption } from '@/pages/admin/Dashboard/types/dashboard'; | ||
|
|
||
| type Props = { | ||
| status?: ApplicantStatus; | ||
| updateStatus: (status: ApplicantStatus) => void; | ||
| status?: ApplicationFilterOption; | ||
| updateStatus: (status: ApplicationFilterOption) => void; | ||
| }; | ||
|
|
||
| export const ApplicantStatusToggle = ({ status, updateStatus }: Props) => { | ||
| const [statusOption, setStatusOption] = useState(status); | ||
|
|
||
| const handleClick = (newStatus: ApplicantStatus) => { | ||
| const handleClick = (newStatus: ApplicationFilterOption) => { | ||
| setStatusOption(newStatus); | ||
| updateStatus(newStatus); | ||
| }; |
There was a problem hiding this comment.
Keep detail-status types constrained to ApplicationStatus
ApplicationFilterOption widens the contract to allow 'ALL', but this toggle only drives real applicant status updates. Passing 'ALL' downstream would violate the API expectation and TypeScript will no longer catch that mistake.
Please revert these props (and the handler parameter) to ApplicationStatus so only valid statuses can flow through.
-import type { ApplicationFilterOption } from '@/pages/admin/Dashboard/types/dashboard';
+import type { ApplicationStatus } from '@/pages/admin/Dashboard/types/dashboard';
type Props = {
- status?: ApplicationFilterOption;
- updateStatus: (status: ApplicationFilterOption) => void;
+ status?: ApplicationStatus;
+ updateStatus: (status: ApplicationStatus) => void;
};
export const ApplicantStatusToggle = ({ status, updateStatus }: Props) => {
const [statusOption, setStatusOption] = useState(status);
- const handleClick = (newStatus: ApplicationFilterOption) => {
+ const handleClick = (newStatus: ApplicationStatus) => {📝 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.
| import type { ApplicationFilterOption } from '@/pages/admin/Dashboard/types/dashboard'; | |
| type Props = { | |
| status?: ApplicantStatus; | |
| updateStatus: (status: ApplicantStatus) => void; | |
| status?: ApplicationFilterOption; | |
| updateStatus: (status: ApplicationFilterOption) => void; | |
| }; | |
| export const ApplicantStatusToggle = ({ status, updateStatus }: Props) => { | |
| const [statusOption, setStatusOption] = useState(status); | |
| const handleClick = (newStatus: ApplicantStatus) => { | |
| const handleClick = (newStatus: ApplicationFilterOption) => { | |
| setStatusOption(newStatus); | |
| updateStatus(newStatus); | |
| }; | |
| import type { ApplicationStatus } from '@/pages/admin/Dashboard/types/dashboard'; | |
| type Props = { | |
| status?: ApplicationStatus; | |
| updateStatus: (status: ApplicationStatus) => void; | |
| }; | |
| export const ApplicantStatusToggle = ({ status, updateStatus }: Props) => { | |
| const [statusOption, setStatusOption] = useState(status); | |
| const handleClick = (newStatus: ApplicationStatus) => { | |
| setStatusOption(newStatus); | |
| updateStatus(newStatus); | |
| }; |
🤖 Prompt for AI Agents
In
src/pages/admin/ApplicationDetail/components/ApplicantProfileSection/ApplicantStatusToggle.tsx
around lines 4 to 17, the component props and handler use
ApplicationFilterOption (which includes 'ALL') but this toggle only sets
concrete applicant statuses; change the types to ApplicationStatus everywhere:
update the import to bring in ApplicationStatus, change the Props.status and
updateStatus parameter type to ApplicationStatus, change the
handleClick(newStatus) parameter to ApplicationStatus, and ensure the
useState(initial) is typed/initialized as ApplicationStatus so only valid
statuses are passed downstream.
| }; | ||
|
|
||
| export const ApplicationStatusFilter = ({ option, onOptionChange }: Props) => { | ||
| const { counts } = useApplicants(1); |
There was a problem hiding this comment.
Remove hardcoded clubId.
The hardcoded clubId = 1 is a magic number that breaks modularity and will cause issues when viewing different clubs' dashboards.
Pass clubId as a prop:
export type Props = {
+ clubId: number;
option: ApplicationFilterOption;
onOptionChange: (option: ApplicationFilterOption) => void;
};
-export const ApplicationStatusFilter = ({ option, onOptionChange }: Props) => {
- const { counts } = useApplicants(1);
+export const ApplicationStatusFilter = ({ clubId, option, onOptionChange }: Props) => {
+ const { counts } = useApplicants(clubId);📝 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.
| const { counts } = useApplicants(1); | |
| export type Props = { | |
| clubId: number; | |
| option: ApplicationFilterOption; | |
| onOptionChange: (option: ApplicationFilterOption) => void; | |
| }; | |
| export const ApplicationStatusFilter = ({ clubId, option, onOptionChange }: Props) => { | |
| const { counts } = useApplicants(clubId); |
🤖 Prompt for AI Agents
In
src/pages/admin/Dashboard/components/ApplicantListSection/ApplicationFilter.tsx
around line 12 the hook is called with a hardcoded clubId (useApplicants(1));
replace the magic number by accepting a clubId prop on ApplicationFilter (with
appropriate typing), use that prop in the useApplicants call (e.g.,
useApplicants(clubId)), and update the parent(s) that render ApplicationFilter
to pass the correct clubId down (ensure any tests/types are updated
accordingly).
| import type { DashboardCard } from '@/pages/admin/Dashboard/types/dashboard'; | ||
|
|
||
| export const DashboardSummarySection = () => { | ||
| const clubId = '1'; |
There was a problem hiding this comment.
Remove hardcoded clubId and fix type inconsistency.
The hardcoded clubId = '1' has two issues:
- It's a magic number preventing reusability
- It's typed as
stringhere butnumberin other files (e.g.,useApplicants(clubId: number))
Pass clubId as a prop with consistent typing:
-export const DashboardSummarySection = () => {
- const clubId = '1';
+type Props = {
+ clubId: string;
+};
+
+export const DashboardSummarySection = ({ clubId }: Props) => {Then ensure all hooks and components use consistent types across the codebase.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx around
line 10, remove the hardcoded const clubId = '1' and instead accept clubId as a
prop (with type number) on the component, update the component's props interface
to include clubId: number, and pass that prop into any hooks or child components
that require clubId; then update call sites of this component to provide the
numeric clubId and audit related hooks/components (e.g., useApplicants) to
ensure they all use number for clubId to keep types consistent across the
codebase.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/pages/admin/Dashboard/hooks/useApplicants.ts (1)
33-48: Excellent refactor—addresses past review feedback!The use of
reduceto compute counts in a single pass is efficient and addresses the past review comment about multiplefiltercalls. This is a significant performance improvement when dealing with large applicant lists.Optional enhancement: While TypeScript's
ApplicationStatustype provides compile-time safety, consider adding runtime validation for unexpected status values from the backend:const counts = useMemo(() => { if (!data) return { ALL: 0, PENDING: 0, APPROVED: 0, REJECTED: 0 }; return data.reduce( (acc, applicant) => { if (applicant.status in acc) { acc[applicant.status] += 1; } return acc; }, { ALL: data.length, PENDING: 0, APPROVED: 0, REJECTED: 0, }, ); }, [data]);src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (2)
7-12: Consider adding background polling control.The
refetchIntervalwill continue polling every 30 seconds even when the browser tab is not visible, which may waste resources and API calls. Consider addingrefetchIntervalInBackground: falseto only poll when the user is actively viewing the dashboard.Apply this diff to optimize polling behavior:
const { data, isLoading, error } = useQuery<DashboardSummary | undefined>({ queryKey: ['dashboardSummary', clubId], queryFn: () => fetchDashboardSummary(clubId), staleTime: 1000 * 60 * 5, refetchInterval: 30000, + refetchIntervalInBackground: false, });
14-14: IncludeisErrorin the return value.The
UseApiQueryResultinterface includes an optionalisErrorproperty, anduseQueryprovides this field. Including it in the return value ensures consistency with the interface and gives consumers better error state information.Apply this diff to include
isError:-const { data, isLoading, error } = useQuery<DashboardSummary | undefined>({ +const { data, isLoading, error, isError } = useQuery<DashboardSummary | undefined>({ queryKey: ['dashboardSummary', clubId], queryFn: () => fetchDashboardSummary(clubId), staleTime: 1000 * 60 * 5, refetchInterval: 30000, }); -return { data: data || null, isLoading, error }; +return { data: data || null, isLoading, error, isError };src/pages/admin/Dashboard/api/dashboard.ts (1)
10-10: Add error handling for JSON parsing.If the server returns malformed JSON,
response.json()will throw an uncaught error. Wrapping this in a try-catch block would provide better error messaging and prevent unhandled promise rejections.Apply this diff to add JSON parsing error handling:
-return response.json(); +try { + return await response.json(); +} catch (error) { + throw new Error('대시보드 요약 데이터 파싱에 실패했습니다.'); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pages/admin/Dashboard/api/dashboard.ts(1 hunks)src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx(1 hunks)src/pages/admin/Dashboard/hooks/useApplicants.ts(1 hunks)src/pages/admin/Dashboard/hooks/useDashboardSummary.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/admin/Dashboard/components/DashboardSummarySection/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/pages/admin/Dashboard/hooks/useApplicants.ts (3)
src/types/useApiQueryResult.ts (1)
UseApiQueryResult(1-6)src/pages/admin/Dashboard/types/dashboard.ts (3)
ApplicantCounts(31-36)ApplicationFilterOption(12-12)ApplicantData(14-22)src/pages/admin/Dashboard/api/applicant.ts (1)
fetchApplicants(3-13)
src/pages/admin/Dashboard/hooks/useDashboardSummary.ts (3)
src/types/useApiQueryResult.ts (1)
UseApiQueryResult(1-6)src/pages/admin/Dashboard/types/dashboard.ts (1)
DashboardSummary(24-29)src/pages/admin/Dashboard/api/dashboard.ts (1)
fetchDashboardSummary(3-11)
src/pages/admin/Dashboard/api/dashboard.ts (1)
src/pages/admin/Dashboard/types/dashboard.ts (1)
DashboardSummary(24-29)
🔇 Additional comments (5)
src/pages/admin/Dashboard/hooks/useApplicants.ts (4)
1-9: LGTM!The imports are correctly updated to support the new client-side filtering and counts computation.
11-13: LGTM!The interface extension cleanly adds the counts field while maintaining compatibility with the base UseApiQueryResult type.
26-31: LGTM!The client-side filtering logic is correct and efficiently handles all cases (no data, 'ALL' filter, and specific status filtering).
50-55: LGTM!The return structure correctly provides filtered data and counts, aligning with the ExtendedUseApiQueryResult interface.
src/pages/admin/Dashboard/api/dashboard.ts (1)
3-4: Verify API request configuration: include auth, base URL, and timeout as needed.
- Authentication: add required tokens (e.g. JWT in Authorization header) if your API enforces auth.
- Base URL: although
/apiis proxied in vite.config.ts, consider using import.meta.env.VITE_API_BASE_URL or a central client for consistency.- Timeout: native fetch lacks timeouts—use AbortController or a fetch wrapper that supports request cancellation/timeouts.
| if (!response.ok) { | ||
| throw new Error('대시보드 요약 정보를 불러오는 데 실패했습니다.'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve error handling with HTTP status information.
The generic error message provides no debugging context. Including the HTTP status code and response details would significantly improve error diagnostics and help differentiate between client errors (4xx) and server errors (5xx).
Apply this diff to enhance error reporting:
if (!response.ok) {
- throw new Error('대시보드 요약 정보를 불러오는 데 실패했습니다.');
+ const errorMessage = `대시보드 요약 정보를 불러오는 데 실패했습니다. (상태 코드: ${response.status})`;
+ throw new Error(errorMessage);
}Alternatively, for more comprehensive error handling:
if (!response.ok) {
- throw new Error('대시보드 요약 정보를 불러오는 데 실패했습니다.');
+ let errorMessage = `대시보드 요약 정보를 불러오는 데 실패했습니다. (상태 코드: ${response.status})`;
+ try {
+ const errorBody = await response.json();
+ if (errorBody?.message) {
+ errorMessage += ` - ${errorBody.message}`;
+ }
+ } catch {
+ // Ignore JSON parsing errors for error responses
+ }
+ throw new Error(errorMessage);
}📝 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.
| if (!response.ok) { | |
| throw new Error('대시보드 요약 정보를 불러오는 데 실패했습니다.'); | |
| } | |
| if (!response.ok) { | |
| let errorMessage = `대시보드 요약 정보를 불러오는 데 실패했습니다. (상태 코드: ${response.status})`; | |
| try { | |
| const errorBody = await response.json(); | |
| if (errorBody?.message) { | |
| errorMessage += ` - ${errorBody.message}`; | |
| } | |
| } catch { | |
| // Ignore JSON parsing errors for error responses | |
| } | |
| throw new Error(errorMessage); | |
| } |
🤖 Prompt for AI Agents
In src/pages/admin/Dashboard/api/dashboard.ts around lines 6 to 8, the current
error throw uses a generic message; change it to include HTTP status and
response details by reading response.status and response.statusText (and
optionally await response.text() or response.json() safely) and include those
values in the thrown Error (or processLogger.error) so the message contains
status code, status text and a short response body snippet for debugging.
| const { data, isLoading, error } = useQuery({ | ||
| queryKey: ['applicants', clubId, status], | ||
| queryKey: ['applicants', clubId], | ||
| queryFn: () => fetchApplicants(clubId), | ||
| staleTime: 1000 * 60 * 2, | ||
| staleTime: 1000 * 60 * 5, | ||
| refetchInterval: 30000, | ||
| }); |
There was a problem hiding this comment.
Verify refetchInterval and add query validation.
-
Discrepancy: The
refetchIntervalis set to 30 seconds here, but linked issue [REFACTOR] 대시보드 지원자 필터링 방식 개선 및 API 연동 #140 specifies 15 seconds. Please confirm the intended polling interval. -
Missing query guard: Consider adding an
enabledoption to prevent fetching with invalidclubIdvalues (e.g., 0 or negative). -
staleTime consideration: With
staleTimeset to 5 minutes andrefetchIntervalat 30 seconds, React Query may serve cached data marked as "fresh" for up to 5 minutes even after background polling updates it. If real-time accuracy is critical, consider reducingstaleTimeto match or be shorter thanrefetchInterval.
Apply this diff to add validation and align intervals:
const { data, isLoading, error } = useQuery({
queryKey: ['applicants', clubId],
queryFn: () => fetchApplicants(clubId),
- staleTime: 1000 * 60 * 5,
- refetchInterval: 30000,
+ staleTime: 1000 * 30, // Match refetchInterval for real-time accuracy
+ refetchInterval: 30000, // Or change to 15000 per issue #140
+ enabled: clubId > 0,
});📝 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.
| const { data, isLoading, error } = useQuery({ | |
| queryKey: ['applicants', clubId, status], | |
| queryKey: ['applicants', clubId], | |
| queryFn: () => fetchApplicants(clubId), | |
| staleTime: 1000 * 60 * 2, | |
| staleTime: 1000 * 60 * 5, | |
| refetchInterval: 30000, | |
| }); | |
| const { data, isLoading, error } = useQuery({ | |
| queryKey: ['applicants', clubId], | |
| queryFn: () => fetchApplicants(clubId), | |
| staleTime: 1000 * 30, // Match refetchInterval for real-time accuracy | |
| refetchInterval: 30000, // Or change to 15000 per issue #140 | |
| enabled: clubId > 0, | |
| }); |
🤖 Prompt for AI Agents
In src/pages/admin/Dashboard/hooks/useApplicants.ts around lines 19 to 24,
update the query options to match issue #140 by changing refetchInterval from
30000 to 15000, add an enabled guard to prevent fetching when clubId is invalid
(e.g., enabled: Boolean(clubId) && clubId > 0), and reduce staleTime to be equal
to or less than the refetch interval (for example staleTime: 15000) so cached
data isn't considered fresh longer than the polling frequency; keep other
options the same.
| import type { DashboardCard } from '@/pages/admin/Dashboard/types/dashboard'; | ||
|
|
||
| export const DashboardSummarySection = () => { | ||
| const clubId = 1; |
There was a problem hiding this comment.
이후에 동아리 관리페이지의 clubId는 동아리를 메인페이지에서 선택하면 해당 동아리의 id 값을 어디에 받아서 갖고있나요?!
There was a problem hiding this comment.
추후 로그인 기능이 도입되면, 서버에서 해당 clubId에 대한 권한을 확인해야 할 것 같습니다.
url도 지금은 /clubs/dashboard 형태인데 /clubs/:clubId/dashboard로 수정해서 clubId를 가져오도록 해야 될 것 같네요!
| queryFn: () => fetchApplicants(clubId), | ||
| staleTime: 1000 * 60 * 2, | ||
| staleTime: 1000 * 60 * 5, | ||
| refetchInterval: 30000, |
There was a problem hiding this comment.
statleTime이랑 reftechInterval 명시적으로 설정하셨네요 나중에 리팩토링할 부분 생긴다면 참고하겠습니다!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/user/ClubDetail/api/clubDetail.ts (1)
7-9: Consider including response details in the error.The error message is generic and doesn't include response status or error details, which can make debugging difficult.
Apply this diff to include response details:
if (!res.ok) { - throw new Error('동아리 상세 정보를 가져오는데 실패했습니다.'); + const errorBody = await res.text().catch(() => ''); + throw new Error( + `동아리 상세 정보를 가져오는데 실패했습니다. (Status: ${res.status}${errorBody ? `, ${errorBody}` : ''})` + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/user/ClubDetail/api/clubDetail.ts(1 hunks)
🔇 Additional comments (1)
src/pages/user/ClubDetail/api/clubDetail.ts (1)
4-5: Approve relative/api/...fetch usage — all API calls uniformly use relative/api/...endpoints, Vite proxy (vite.config.ts) and MSW mocks handle the underlying base URL, and default fetch credentials cover auth.
…licant-filter-api#140 [REFACTOR] 대시보드 지원자 목록 실시간 동기화 및 필터링 개선 (#140)
…licant-filter-api#140 [REFACTOR] 대시보드 지원자 목록 실시간 동기화 및 필터링 개선 (#140)
3 participants
#️⃣연관된 이슈
📝작업 내용
실시간 데이터 동기화 도입
클라이언트 사이드 필터링으로 전환
Summary by CodeRabbit
New Features
Refactor