refactor: shadcn 도입 및 리팩토링 - #6
Merged
Merged
Conversation
WalkthroughShadCN 기반 UI 컴포넌트 집합과 중앙집중식 HTTP 클라이언트(ApiResult/ requestApi)를 추가하고, 여러 페이지와 컴포넌트를 훅/섹션 기반으로 대규모 리팩토링했습니다. 경로 별칭( Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces shadcn UI component library and performs a comprehensive refactoring of the admin dashboard application. The changes modernize the UI, consolidate API request handling, improve error management, and establish consistent design patterns across the codebase.
Changes:
- Introduced shadcn UI component library with complete setup and configuration
- Consolidated duplicate API request handlers into a centralized HTTP client
- Refactored all pages and components to use new UI components and patterns
- Replaced react-toastify with sonner for notifications
- Implemented new layout system with AppShell, protected routes, and responsive navigation
Reviewed changes
Copilot reviewed 100 out of 103 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Added path alias configuration for @/ imports |
| tsconfig.*.json | Configured TypeScript path mappings for alias support |
| components.json | Added shadcn UI configuration |
| src/lib/http/client.ts | New centralized HTTP client with consistent error handling |
| src/lib/errors/admin-error.ts | New error message resolution utility |
| src/lib/utils.ts | New utility for className merging (shadcn standard) |
| src/components/ui/* | Added shadcn UI components (Button, Card, Dialog, etc.) |
| src/components/layout/* | New layout system with AppShell, Header, Sidebar |
| src/components/admin/* | New reusable admin UI components |
| src/utils/alert.tsx | Simplified to use sonner instead of react-toastify |
| src/api/*/request.ts | Consolidated to use centralized HTTP client |
| src/api/*/types.ts | Unified ApiResult type definition |
| src/page/* | All pages refactored to use new UI components |
| src/context/AuthContext.tsx | Enhanced with welcome message and error handling |
| src/App.tsx | Restructured with ProtectedLayout and sonner Toaster |
| package.json | Updated dependencies (added shadcn, removed react-icons) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (15)
src/lib/errors/admin-error.ts-25-27 (1)
25-27:⚠️ Potential issue | 🟡 Minor
overrides존재 확인을 진리값 대신in연산자로 변경 권장현재
options?.overrides?.[errorName]의 진리값(truthiness) 체크는, 누군가overrides에 빈 문자열("")을 의도적으로 지정하면 falsy로 처리되어DEFAULT_ADMIN_ERROR_MESSAGES또는fallback으로 폴백합니다. 명시적 키 존재 확인이 의도를 더 명확히 표현합니다.♻️ 제안하는 수정
- if (options?.overrides?.[errorName]) { - return options.overrides[errorName] + if (options?.overrides && errorName in options.overrides) { + return options.overrides[errorName] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/errors/admin-error.ts` around lines 25 - 27, Change the truthiness check for overrides to test key existence instead of truthy value: instead of if (options?.overrides?.[errorName]) return ..., check whether errorName exists on the overrides object (e.g. if (options?.overrides && errorName in options.overrides) or if (options?.overrides && Object.prototype.hasOwnProperty.call(options.overrides, errorName)) return options.overrides[errorName]; this ensures intentionally falsy values like "" are returned rather than falling back to DEFAULT_ADMIN_ERROR_MESSAGES or fallback.src/page/login.tsx-5-9 (1)
5-9:⚠️ Potential issue | 🟡 Minor
VITE_API_BASE_URL이 미설정일 경우 잘못된 URL로 리다이렉트됩니다.
import.meta.env.VITE_API_BASE_URL이 설정되지 않으면"undefined/oauth2/authorization/google"로 이동합니다. 런타임 가드 또는 빌드 타임 검증을 추가하는 것이 좋습니다.🛡️ 환경변수 검증 예시
const API_BASE_URL = import.meta.env.VITE_API_BASE_URL +if (!API_BASE_URL) { + throw new Error("VITE_API_BASE_URL 환경변수가 설정되지 않았습니다.") +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/login.tsx` around lines 5 - 9, API_BASE_URL가 없을 때 "undefined/..."로 리다이렉트되는 문제를 고치려면 import.meta.env.VITE_API_BASE_URL 값을 사용하기 전에 검증 로직을 추가하세요: Login 컴포넌트(또는 모듈 초기화) 내에서 API_BASE_URL을 확인하고 비어있거나 undefined면 예외를 던지거나 안전한 기본값(예: 빈 문자열이나 환경에 맞는 기본 도메인)을 사용하도록 변경하고, handleLogin 함수에서 `${API_BASE_URL}/oauth2/authorization/google`를 만들기 전에 유효한 값인지 검사하여 유효하지 않으면 사용자에게 에러를 표시하거나 리다이렉트를 수행하지 않도록 하세요; 참조 심볼: API_BASE_URL, Login, handleLogin.tsconfig.json-2-6 (1)
2-6:⚠️ Potential issue | 🟡 Minor경로 별칭이 중복 정의되어 있습니다.
@/*별칭이tsconfig.app.json에 이미 명시적으로 정의되어 있으므로 IDE/빌드에서는 문제없이 인식됩니다. 다만tsconfig.app.json이 루트tsconfig.json을extends하지 않기 때문에, 루트에서 정의한baseUrl과paths는 실제로 상속되지 않습니다. 설정 중복을 제거하기 위해 루트tsconfig.json에서 해당 설정을 제거하거나,tsconfig.app.json과tsconfig.node.json에서 루트를extends하도록 리팩터링하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.json` around lines 2 - 6, The root tsconfig.json currently defines compilerOptions.baseUrl and compilerOptions.paths with the "@/*" alias which is duplicated in tsconfig.app.json; either remove the baseUrl/paths block from the root tsconfig.json (so aliases live only in tsconfig.app.json/tsconfig.node.json) or modify tsconfig.app.json and tsconfig.node.json to extend the root tsconfig.json (add "extends": "./tsconfig.json") so they inherit baseUrl and paths; pick one approach and apply it consistently, ensuring the "@/*" alias is defined in exactly one place and that consumers extend that config.src/page/coupon/sections/CouponPageHeader.tsx-20-24 (1)
20-24:⚠️ Potential issue | 🟡 Minor검색 입력에 접근성 라벨을 추가해주세요.
placeholder만으로는 보조기기에서 의미가 부족할 수 있습니다.
♿ 제안 수정
<Input className="max-w-sm" value={searchText} onChange={(event) => onSearchTextChange(event.target.value)} placeholder="ID, 쿠폰명, 회원명, 학번, 이메일, 코드, 설명 검색" + aria-label="쿠폰 검색" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/coupon/sections/CouponPageHeader.tsx` around lines 20 - 24, The search Input currently relies only on placeholder which is not sufficient for assistive tech; update the Input in CouponPageHeader to include an accessible label by adding an aria-label (e.g., aria-label="쿠폰 검색" or more specific like "ID, 쿠폰명, 회원명 등 검색") or associate it with a visible/visually-hidden <label> using id and aria-labelledby; keep existing props (value={searchText}, onChange={event => onSearchTextChange(event.target.value)}, placeholder) and ensure the chosen label text matches the placeholder semantics for clarity.src/api/payment/get-admin-payments.ts-12-26 (1)
12-26:⚠️ Potential issue | 🟡 Minorpage/size 값 범위 보정이 있으면 더 안전합니다.
현재 음수나 비정상 값이 그대로 전달될 수 있어요. 최소값 보정을 추가해 주세요.
🛠️ 제안 수정
export async function getAdminPayments(query: AdminPaymentsQuery): Promise<ApiResult<AdminPaymentPage>> { const params = new URLSearchParams(); - params.set('page', String(query.page ?? 0)); - params.set('size', String(query.size ?? 50)); + const page = Math.max(0, Math.floor(query.page ?? 0)); + const size = Math.max(1, Math.floor(query.size ?? 50)); + + params.set('page', String(page)); + params.set('size', String(size));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/payment/get-admin-payments.ts` around lines 12 - 26, The getAdminPayments function currently passes page and size directly into URLSearchParams allowing negative or invalid numbers; clamp both values before setting params (e.g., compute page = Math.max(0, Number(query.page ?? 0)) and size = Math.max(1, Number(query.size ?? 50))) and use those sanitized values when calling params.set('page', ...) and params.set('size', ...)); update any references to query.page/query.size in this function to use the sanitized variables so only valid ranges are sent.src/components/layout/AppShell.tsx-26-33 (1)
26-33:⚠️ Potential issue | 🟡 Minor
SheetDescription누락으로 인한 Radix UI 접근성 경고
SheetTitle은sr-only로 제공되었지만SheetDescription이 없어, Radix UI가 개발 환경에서aria-describedby관련 콘솔 경고를 출력할 수 있습니다.🛡️ 제안하는 수정 (두 가지 중 하나 선택)
방법 1:
SheetDescription을 sr-only로 추가-import { Sheet, SheetContent, SheetTitle } from "@/components/ui/sheet" +import { Sheet, SheetContent, SheetDescription, SheetTitle } from "@/components/ui/sheet" ... <SheetTitle className="sr-only">네비게이션</SheetTitle> +<SheetDescription className="sr-only">사이드바 네비게이션 메뉴</SheetDescription>방법 2:
SheetContent에aria-describedby를 명시적으로 제거-<SheetContent side="left" className="w-[280px] p-0"> +<SheetContent side="left" className="w-[280px] p-0" aria-describedby={undefined}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/AppShell.tsx` around lines 26 - 33, Radix Sheet is emitting an accessibility warning because SheetTitle (used as sr-only) lacks a corresponding SheetDescription; update the Sheet markup around SheetContent/SheetTitle to either (A) add a Screen Reader only SheetDescription element (e.g., place a <SheetDescription className="sr-only"> with a short description for the navigation) so aria-describedby resolves, or (B) explicitly remove/override the aria-describedby on SheetContent (e.g., ensure SheetContent does not render aria-describedby) — locate the Sheet, SheetContent, and SheetTitle usage (props mobileNavOpen, setMobileNavOpen, and AppSidebar) and apply one of these two fixes.src/components/layout/AppHeader.tsx-28-29 (1)
28-29:⚠️ Potential issue | 🟡 Minor
showSuccess토스트가window.location.href리다이렉트로 인해 표시되지 않습니다.
window.location.href = "/login"은 전체 페이지를 새로 로드하므로 React 상태(Sonner 토스터 포함)가 즉시 파괴됩니다.showSuccess호출 직후 네비게이션이 발생하기 때문에 사용자는 "로그아웃 되었습니다." 메시지를 볼 수 없습니다.로그아웃 후 React 상태를 완전히 초기화하려면
window.location.href유지가 적절하지만, 그 경우showSuccess호출은 제거하는 것이 적합합니다. 또는 이미 import된navigate를 사용하면 SPA 전환 중에 토스트가 표시됩니다.🛠️ 제안: 불필요한 showSuccess 제거
- showSuccess("로그아웃 되었습니다.") window.location.href = "/login"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/AppHeader.tsx` around lines 28 - 29, The toast call showSuccess("로그아웃 되었습니다.") is being wiped out by the full-page redirect via window.location.href = "/login"; either remove the showSuccess call if you intend to fully reload the app, or keep the toast and replace the full reload with SPA navigation by calling navigate("/login") (ensure the existing imported navigate is used) so the Sonner toast can render during the transition; update the logout handler containing showSuccess and window.location.href accordingly to implement one of these two options.src/page/payment/sections/TransactionManagementSection.tsx-37-43 (1)
37-43:⚠️ Potential issue | 🟡 Minor
formatDateTime에서 null 처리 누락 가능
transactionTime가 null/빈 값이면new Date(null)로 1970-01-01이 표시될 수 있습니다. 기존 유틸 패턴처럼 falsy 체크를 추가하는 편이 안전합니다.🛠 제안 변경
-function formatDateTime(value: string): string { - const parsed = new Date(value) +function formatDateTime(value: string | null): string { + if (!value) { + return "-" + } + const parsed = new Date(value) if (Number.isNaN(parsed.getTime())) { return "-" } return parsed.toLocaleString("ko-KR", { hour12: false }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/sections/TransactionManagementSection.tsx` around lines 37 - 43, formatDateTime currently constructs new Date(value) without guarding falsy inputs, so null/empty strings will produce the 1970-01-01 epoch; update formatDateTime to first check for falsy value (e.g., null, undefined, empty string) and return "-" immediately, then parse the date and keep the existing Number.isNaN(parsed.getTime()) guard to return "-" on invalid dates; modify the function implementation around the formatDateTime symbol to follow the existing utility pattern used elsewhere.src/page/member-management/sections/MemberRecordsSection.tsx-111-118 (1)
111-118:⚠️ Potential issue | 🟡 Minor첫 검색 전에도 "조회 결과가 없습니다" 표시됨
recordPage가null일 때(null?.content.length ?? 0) === 0은true이므로, 페이지 진입 직후(첫 검색 전)에도 빈 결과 메시지가 표시됩니다.recordPage === null인 초기 상태와 실제 빈 검색 결과를 구분하는 것이 UX상 더 자연스럽습니다.💡 개선 제안
-{!isRecordLoading && (recordPage?.content.length ?? 0) === 0 && ( +{!isRecordLoading && recordPage !== null && recordPage.content.length === 0 && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-management/sections/MemberRecordsSection.tsx` around lines 111 - 118, The empty-results message is shown even before the first search because (recordPage?.content.length ?? 0) === 0 evaluates true when recordPage is null; change the condition to only show the "조회 결과가 없습니다." row when recordPage is non-null and has zero items (e.g., isRecordLoading is false && recordPage !== null && (recordPage.content.length === 0)), updating the conditional around TableBody/TableRow/TableCell that currently references isRecordLoading and recordPage so initial null state is not treated as an empty result.src/page/feature-flags.tsx-41-54 (1)
41-54:⚠️ Potential issue | 🟡 Minor잘못된 날짜 문자열 처리 시
"-"대신 원본 값 반환 — 다른 구현체와 불일치이 파일의
formatDateTime은 파싱 불가 날짜에value(원본 문자열)를 반환하지만,useMemberManagementPageState.ts·useCouponPageState.ts·PaymentManagementSection.tsx의 동일 함수는 모두"-"를 반환합니다. 의도적인 차이라면 주석으로 명시하고, 아니라면"-"반환으로 통일하거나 공유 유틸로 추출하세요.if (Number.isNaN(parsed.getTime())) { - return value + return "-" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/feature-flags.tsx` around lines 41 - 54, The formatDateTime function currently returns the original value when parsing fails, which is inconsistent with the implementations in useMemberManagementPageState.ts, useCouponPageState.ts, and PaymentManagementSection.tsx; change formatDateTime to return "-" for null/invalid inputs (instead of returning value) and consolidate it into a shared util function (e.g., export a common formatDateTime from a new util module) and update the callers in useMemberManagementPageState, useCouponPageState, and PaymentManagementSection to use the shared util for consistent behavior.src/components/QRScanner.tsx-26-34 (1)
26-34:⚠️ Potential issue | 🟡 Minor
setTimeoutID가 보관되지 않아 컴포넌트 언마운트 시 미정리컴포넌트가
delayMs내에 언마운트되면 타이머를 취소할 수 없어setIsProcessing(false)및 ref 뮤테이션이 불필요하게 실행됩니다.🛠️ 개선 제안
- const resetProcessingState = useCallback((delayMs: number) => { - window.setTimeout(() => { + const resetTimerRef = useRef<ReturnType<typeof window.setTimeout> | null>(null) + + const resetProcessingState = useCallback((delayMs: number) => { + if (resetTimerRef.current !== null) window.clearTimeout(resetTimerRef.current) + resetTimerRef.current = window.setTimeout(() => { + resetTimerRef.current = null isProcessingRef.current = false ... }, delayMs) }, [])
useEffectcleanup에서도clearTimeout(resetTimerRef.current)호출.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QRScanner.tsx` around lines 26 - 34, The resetProcessingState callback currently calls window.setTimeout without saving the timer id, so add a resetTimerRef (e.g. resetTimerRef.current) assignment when creating the timeout in resetProcessingState, and clear any existing timer before setting a new one (clearTimeout(resetTimerRef.current)); also add a useEffect cleanup that calls clearTimeout(resetTimerRef.current) on unmount to prevent setIsProcessing(false) and ref mutations after the component unmounts. Ensure you keep the same refs (isProcessingRef, isThrottled, lastProcessedQR, lastScanTime) and setIsProcessing usage while only changing how the timeout id is stored and cleared.src/page/coupon/sections/CouponTabSection.tsx-44-59 (1)
44-59:⚠️ Potential issue | 🟡 Minor
Label과Input이 연결되지 않아 접근성 문제 발생
htmlFor없이<Label>만 있으면 스크린 리더가 입력 필드와 레이블을 연결하지 못합니다.♿ 접근성 개선 제안
<div className="min-w-[220px] space-y-2"> - <Label>쿠폰 이름</Label> + <Label htmlFor="new-coupon-name">쿠폰 이름</Label> <Input + id="new-coupon-name" value={newCouponName} onChange={(event) => onNewCouponNameChange(event.target.value)} placeholder="예) 2026 신입생 환영 쿠폰" /> </div> <div className="w-[200px] space-y-2"> - <Label>할인 금액</Label> + <Label htmlFor="new-discount-amount">할인 금액</Label> <Input + id="new-discount-amount" type="number" min={1}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/coupon/sections/CouponTabSection.tsx` around lines 44 - 59, The Label components in CouponTabSection are not associated with their Inputs which hurts accessibility; update the coupon name and discount amount pairs by assigning a unique id to each Input (e.g., couponNameInputId, discountAmountInputId) and pass that id into the corresponding Label via htmlFor so screen readers can link label→input; ensure the onNewCouponNameChange and onNewDiscountAmountChange handlers keep working with the same value props (newCouponName, newDiscountAmount) after adding the id attributes.src/page/feature-flags.tsx-179-217 (1)
179-217:⚠️ Potential issue | 🟡 Minor
flags가null인 초기 로드 중 모든 배지가 "차단"으로 잘못 표시됨
AdminSectionCard섹션들은flags값과 무관하게 항상 렌더링됩니다. 데이터 로드 전(flags === null) 배지 표현식flags?.memberSignup.signupAllowed ? "허용" : "차단"은undefined를 평가해 "차단"을 표시합니다. 실제 값이 "허용"인 경우 사용자가 순간적으로 잘못된 상태를 보게 됩니다.💡 개선 제안
-{!flags && isDataLoading && ( - <Card>...</Card> -)} - -<AdminSectionCard title="회원가입 허용" ...> +{!flags && isDataLoading ? ( + <Card>...</Card> +) : flags && ( + <> + <AdminSectionCard title="회원가입 허용" ...> ... -</AdminSectionCard> + </AdminSectionCard> ... + </> +)}또는 각 배지를
flags ? (flags.memberSignup.signupAllowed ? "허용" : "차단") : "로딩 중..."으로 처리하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/feature-flags.tsx` around lines 179 - 217, The badges inside AdminSectionCard are showing "차단" when flags is null because expressions like flags?.memberSignup.signupAllowed ? "허용" : "차단" evaluate to false for undefined; update the rendering to account for the loading/null state: inside the AdminSectionCard update the Badge text expressions (references: flags, memberSignup, signupAllowed, valid, rawValue, and Badge) to use a conditional that checks flags first (e.g. flags ? (flags.memberSignup.signupAllowed ? "허용" : "차단") : "로딩 중..." or render a placeholder) or hide the badges until flags is non-null so the UI doesn’t show incorrect "차단" during initial load.src/page/coupon/hooks/useCouponPageState.ts-269-307 (1)
269-307:⚠️ Potential issue | 🟡 Minor쿠폰 이름은 trim 된 값으로 저장/전송해주세요.
현재는 공백 포함 원본이 API로 전달되어 중복 판단/표시가 어긋날 수 있습니다.🛠️ 제안 변경
const handleCreateCoupon = async (): Promise<void> => { - if (!newCouponName.trim()) { + const normalizedName = newCouponName.trim() + if (!normalizedName) { showError("쿠폰 이름을 입력해주세요.") return } const parsedDiscountAmount = Number(newDiscountAmount) if (!Number.isFinite(parsedDiscountAmount) || parsedDiscountAmount <= 0) { showError("할인 금액은 0보다 커야 합니다.") return } - const response = await createCoupon(newCouponName, parsedDiscountAmount) + const response = await createCoupon(normalizedName, parsedDiscountAmount) if (!response.ok) { showError(resolveCouponErrorMessage(response.errorName)) return } showSuccess("쿠폰을 생성했습니다.") setNewCouponName("") await loadAll() } const handleUpdateCouponName = async (couponId: number): Promise<void> => { - const draftName = couponNameDrafts[couponId] ?? "" - if (!draftName.trim()) { + const normalizedName = (couponNameDrafts[couponId] ?? "").trim() + if (!normalizedName) { showError("쿠폰 이름을 입력해주세요.") return } - const response = await updateCouponName(couponId, draftName) + const response = await updateCouponName(couponId, normalizedName) if (!response.ok) { showError(resolveCouponErrorMessage(response.errorName)) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/coupon/hooks/useCouponPageState.ts` around lines 269 - 307, Trim coupon names before sending or persisting: in handleCreateCoupon use const name = newCouponName.trim() (validate against that) and pass name to createCoupon instead of newCouponName, and in handleUpdateCouponName compute const draftName = (couponNameDrafts[couponId] ?? "").trim() (validate against that) and pass the trimmed value to updateCouponName; also ensure any state updates or cleared values (e.g., setNewCouponName) use the trimmed/normalized string so API/duplicate checks and UI display stay consistent.src/page/payment/hooks/usePaymentPageState.ts-177-183 (1)
177-183:⚠️ Potential issue | 🟡 Minor페이지 상한 검증이 누락되어 있습니다.
nextPage < 0만 체크하고totalPages이상의 값은 차단하지 않습니다.moveTransactionPage(Line 238-251)도 동일합니다. 서버가 빈 결과를 반환하겠지만, 불필요한 API 호출이 발생합니다.💡 상한 검증 추가 제안
const movePaymentPage = async (nextPage: number): Promise<void> => { - if (nextPage < 0) { + if (nextPage < 0 || (paymentData && nextPage >= paymentData.totalPages)) { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/hooks/usePaymentPageState.ts` around lines 177 - 183, The page upper-bound is not validated causing needless API calls; update movePaymentPage to also return early when nextPage is >= totalPages (use the same totalPages state/variable used for pagination) before calling setPaymentPage and fetchPayments, and apply the identical guard to moveTransactionPage so both functions validate 0 <= nextPage < totalPages and skip setting state or fetching if outside that range.
🧹 Nitpick comments (25)
.gitignore (1)
29-33: 마크다운 규칙이# Environment variables섹션에 잘못 배치되어 있습니다.내용상 환경 변수와 무관한 마크다운 규칙이므로, 별도 섹션으로 분리하는 것이 가독성에 좋습니다.
♻️ 섹션 분리 제안
# Environment variables .env .env.local + +# Markdown files +*.md +!README.md +!AGENTS.md +!CLAUDE.md그리고 기존 위치(29~33번 줄)의 내용은 제거합니다.
추가로,
*.md는 저장소 전체의 모든 마크다운 파일을 무시하므로, 향후CHANGELOG.md,CONTRIBUTING.md,docs/*.md등의 문서 파일이 추가될 경우 의도치 않게 Git 추적에서 제외될 수 있습니다. 이러한 파일을 버전 관리할 계획이 있다면 해당 파일명도!예외 규칙으로 추가해 두는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 29 - 33, The .gitignore block containing *.md and its exceptions (!README.md, !AGENTS.md, !CLAUDE.md) is placed under the Environment variables section and should be moved to its own "Markdown files" section; remove the current lines (*.md, !README.md, !AGENTS.md, !CLAUDE.md) from the Environment variables area, create a new header like "# Markdown files" and paste those patterns there, and instead of a blanket *.md consider narrowing the ignore (or add additional explicit exceptions such as !CHANGELOG.md, !CONTRIBUTING.md, !docs/*.md) so you don't unintentionally ignore other repo markdown files.src/components/ui/label.tsx (1)
11-22: React 19에서는forwardRef없이ref를 일반 prop으로 사용할 수 있습니다.프로젝트가 React
^19.2.3을 사용하고 있으므로,forwardRef대신ref를 일반 prop으로 직접 받는 패턴으로 간소화할 수 있습니다. shadcn CLI가 생성한 코드이므로 당장 변경할 필요는 없지만, 향후 컴포넌트 수정 시 고려해 주세요.♻️ React 19 ref-as-prop 패턴 제안
-const Label = React.forwardRef< - React.ElementRef<typeof LabelPrimitive.Root>, - React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root> & - VariantProps<typeof labelVariants> ->(({ className, ...props }, ref) => ( - <LabelPrimitive.Root - ref={ref} - className={cn(labelVariants(), className)} - {...props} - /> -)) -Label.displayName = LabelPrimitive.Root.displayName +function Label({ + className, + ref, + ...props +}: React.ComponentPropsWithRef<typeof LabelPrimitive.Root> & + VariantProps<typeof labelVariants>) { + return ( + <LabelPrimitive.Root + ref={ref} + className={cn(labelVariants(), className)} + {...props} + /> + ) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/label.tsx` around lines 11 - 22, The component uses React.forwardRef unnecessarily for React 19 where refs can be received as a normal prop; replace the forwardRef wrapper for the Label component by accepting a plain ref prop in the function signature (e.g., ({ className, ref, ...props })) and pass that ref directly into LabelPrimitive.Root, keep className composed via cn(labelVariants(), className) and spread {...props}, and retain the displayName assignment to LabelPrimitive.Root.displayName; update any types to remove React.forwardRef generics and instead include ref in the component props (related symbols: Label, LabelPrimitive.Root, labelVariants, cn).package.json (1)
33-33: Tailwind v4 네이티브 애니메이션 유틸리티로 마이그레이션을 권장합니다.
tailwindcss-animatev1.0.7은 현재@plugin "tailwindcss-animate";지시어를 통해 Tailwind v4에서 정상 작동하지만, v4 생태계에서는 더 이상 권장되지 않습니다. v4의 CSS 우선 접근 방식에 맞춰tw-animate-css,tailwind-animate, 또는tailwindcss-animated같은 CSS 기반 대안으로 마이그레이션을 검토하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 33, The package.json currently depends on "tailwindcss-animate": "^1.0.7" which is discouraged for Tailwind v4; remove that dependency and replace it with a CSS-based animation package (e.g., "tw-animate-css", "tailwind-animate", or "tailwindcss-animated") in package.json, remove any Tailwind plugin directive referencing tailwindcss-animate (e.g., the `@plugin` "tailwindcss-animate"; entry in your Tailwind config), and instead import the chosen CSS-based library into your global stylesheet (e.g., main CSS entry) so animations are applied via CSS; finally run your package manager to install the new dependency and verify animations and build pipeline work.src/components/ui/sonner.tsx (1)
6-25: 다크 모드 미지원 — 시스템 테마 감지 고려
theme="light"이 하드코딩되어 있어, 다크 모드 환경에서도 Toast가 항상 라이트 테마로 표시됩니다.{...props}가 뒤에 spread되므로 호출자가themeprop을 직접 넘겨 재정의할 수는 있습니다.ShadCN 공식 구현은
next-themes의useTheme()으로 현재 테마를 동적으로 읽지만, 이 프로젝트가 Vite 기반이므로window.matchMedia('(prefers-color-scheme: dark)')를 이용하거나, 별도 테마 컨텍스트에서 테마 값을 주입하는 방식도 고려해볼 수 있습니다.♻️ 예시: matchMedia를 사용한 기본 구현
+import { useEffect, useState } from "react" +import type { ComponentProps } from "react" import { Toaster as Sonner } from "sonner" -import type { ComponentProps } from "react" type ToasterProps = ComponentProps<typeof Sonner> const Toaster = ({ ...props }: ToasterProps) => { + const [theme, setTheme] = useState<"light" | "dark">("light") + useEffect(() => { + const mq = window.matchMedia("(prefers-color-scheme: dark)") + setTheme(mq.matches ? "dark" : "light") + const handler = (e: MediaQueryListEvent) => setTheme(e.matches ? "dark" : "light") + mq.addEventListener("change", handler) + return () => mq.removeEventListener("change", handler) + }, []) return ( <Sonner - theme="light" + theme={theme}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sonner.tsx` around lines 6 - 25, The Toaster currently hardcodes theme="light" in the Toaster component causing no dark-mode support; change Toaster to determine the current theme (e.g., via window.matchMedia('(prefers-color-scheme: dark)') or from your app theme context) and pass theme={themeValue ? "dark" : "light"} to the Sonner component (while still spreading {...props} so callers can override), and ensure you set initial theme and subscribe to matchMedia change events (and clean up) so the Sonner theme updates when the system theme changes.src/components/ui/scroll-area.tsx (1)
1-1:"use client"지시어 불필요 — 제거 권장이 프로젝트는 Vite 기반으로,
"use client"는 Next.js RSC(React Server Components) 전용 지시어입니다. Vite 번들러는 이 지시어를 단순 문자열 표현식으로 처리하여 무해하지만, 동일 PR 내 다른 UI 파일(sonner.tsx,checkbox.tsx,separator.tsx등)에는 존재하지 않아 일관성이 깨집니다.♻️ 제안하는 변경
-"use client" - import * as React from "react"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/scroll-area.tsx` at line 1, Remove the unnecessary "use client" directive from src/components/ui/scroll-area.tsx to match the other UI files (e.g., sonner.tsx, checkbox.tsx, separator.tsx); locate the top of the file where the string literal "use client" appears and delete that line, then run the build/linters to verify no client-only behavior is required by any exported component (e.g., ScrollArea) before committing.src/lib/errors/admin-error.ts (1)
1-4:ResolveAdminErrorMessageOptions인터페이스를 export 권장현재 미export 상태로, 호출 측에서 옵션 객체에 명시적 타입 어노테이션이 필요한 경우 이 타입을 import할 수 없습니다. 함수 인자 타입 추론으로 대부분 동작하지만, 공개 API의 일부로 export하는 것이 유지보수성과 DX에 유리합니다.
♻️ 제안하는 수정
-interface ResolveAdminErrorMessageOptions { +export interface ResolveAdminErrorMessageOptions { overrides?: Record<string, string> fallback?: string }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/errors/admin-error.ts` around lines 1 - 4, Export the ResolveAdminErrorMessageOptions interface so callers can import and use it in type annotations: change the declaration to be exported (export interface ResolveAdminErrorMessageOptions) and update any files that should import this type (e.g., places that pass overrides or fallback to resolveAdminErrorMessage) to import the symbol rather than relying on implicit inference; keep the name and shape unchanged to preserve backwards compatibility.src/page/member-management/hooks/useMemberManagementPageState.ts (2)
21-25: 하드코딩된 폴백 학기 옵션은 시간이 지나면 유효하지 않게 됩니다.
FALLBACK_SEMESTER_OPTIONS가 특정 연도/학기로 고정되어 있어 시간이 지나면 현재 학기와 맞지 않게 됩니다. 현재 날짜 기반으로 동적 생성하는 것을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-management/hooks/useMemberManagementPageState.ts` around lines 21 - 25, FALLBACK_SEMESTER_OPTIONS is hardcoded and will become stale; replace the constant with a small helper that builds MemberRecordSemesterOption[] from the current date (e.g., a function like buildFallbackSemesterOptions or getFallbackSemesterOptions) and returns a few recent semesters (current and prior 2–3) with correct yearSemester strings, labels and the current boolean set based on today; locate and update usages of FALLBACK_SEMESTER_OPTIONS to call the new function so the page state always reflects the actual current semester.
58-69:formatDateTime함수를 공통 유틸리티로 추출하세요.코드베이스에서 동일한 날짜 포맷팅 로직이 최소 6개 위치에서 중복되어 있습니다:
src/page/member-management/hooks/useMemberManagementPageState.ts,src/page/point/hooks/usePointPageState.ts,src/page/coupon/hooks/useCouponPageState.ts,src/page/payment/sections/TransactionManagementSection.tsx,src/page/feature-flags.tsx등.src/utils/format.ts같은 공통 유틸리티 모듈에 추출하면 중복을 제거하고 유지보수성을 향상시킬 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-management/hooks/useMemberManagementPageState.ts` around lines 58 - 69, Extract the formatDateTime implementation into a shared utility (e.g., add and export a function named formatDateTime in src/utils/format.ts) that accepts value: string | null and returns the same formatted string (handle null/invalid dates by returning "-"); then replace the duplicate implementations by importing and calling this exported formatDateTime in the places that currently define it (functions or components like useMemberManagementPageState, usePointPageState, useCouponPageState, TransactionManagementSection, feature-flags, etc.), ensuring the import/export names match and tests/usage still compile.src/components/ui/select.tsx (1)
1-1:"use client"지시어는 Vite SPA 프로젝트에서 불필요합니다.이 프로젝트는 Vite 기반(Next.js가 아닌) SPA로 보이므로
"use client"지시어는 효과가 없습니다. ShadCN CLI가 기본으로 추가하는 것이니 혼동을 줄이려면 제거를 고려해보세요.sheet.tsx에도 동일하게 적용됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/select.tsx` at line 1, Remove the unnecessary "use client" directive string from the top of the component files (e.g., the "use client" line in select.tsx and the same in sheet.tsx) since this is a Vite SPA and the directive has no effect; simply delete the directive line in those modules so they no longer include "use client".src/components/admin/AdminSectionCard.tsx (1)
15-34:AdminPageHeader와 헤더 렌더링 로직이 중복됩니다.
AdminPageHeader.tsx(Lines 12-24)와 이 컴포넌트의CardHeader부분이 거의 동일합니다 (cn(actions && "flex flex-row..."), title/description 래핑, actions 래핑). 공통 헤더 렌더링을 내부 헬퍼로 추출하거나,AdminSectionCard가AdminPageHeader를 합성하는 방식을 고려해볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/AdminSectionCard.tsx` around lines 15 - 34, AdminSectionCard duplicates the header rendering found in AdminPageHeader (same cn(actions && "flex flex-row..."), title/description wrapper, and actions wrapper); refactor by extracting that header into a shared component (e.g., AdminHeader) or have AdminSectionCard compose AdminPageHeader: move the CardHeader JSX (the cn logic, CardTitle/CardDescription rendering of title/description, and actions container) into the shared AdminHeader component that accepts title, description, actions and any header class overrides, then replace the CardHeader block in AdminSectionCard with the new AdminHeader (ensuring AdminSectionCard still forwards className/contentClassName and children) and update imports/usages accordingly.src/components/admin/AdminTableEmptyRow.tsx (1)
17-20: 로딩 메시지 접근성 알림을 추가하는 것을 고려해주세요.
isLoading 전환 시 스크린리더가 변경을 알기 어려울 수 있어, 로딩 상태일 때role="status"/aria-live="polite"를 부여하는 방안을 검토해 주세요.♿ 제안 수정
- <TableCell colSpan={colSpan} className="h-16 text-center text-muted-foreground"> + <TableCell + colSpan={colSpan} + className="h-16 text-center text-muted-foreground" + role={isLoading ? "status" : undefined} + aria-live={isLoading ? "polite" : undefined} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/AdminTableEmptyRow.tsx` around lines 17 - 20, The TableCell in AdminTableEmptyRow currently swaps between loadingMessage and emptyMessage but doesn't announce changes to assistive tech; update the TableCell (in the AdminTableEmptyRow component) to include accessibility attributes when isLoading is true—specifically add role="status" and aria-live="polite" (optionally aria-atomic="true") to the TableCell element so screen readers are notified of the loadingMessage changes while preserving the existing colSpan and classes.src/api/auth/types.ts (1)
1-5:role필드에MemberRole유니온 타입 사용 권장
role: string은useSessionKeepAlive에서'ADMIN'과 비교하는 등의 사용처가 있는데, 이미src/api/member-management/types.ts에MemberRole = 'ADMIN' | 'USER' | 'GUEST'가 정의되어 있습니다. 해당 타입을 재사용하면 오타나 잘못된 역할 값을 컴파일 시점에 잡을 수 있습니다.♻️ 제안 수정
+import type { MemberRole } from '@/api/member-management/types'; export interface AdminMember { memberId?: number; name: string; - role: string; + role: MemberRole; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/auth/types.ts` around lines 1 - 5, AdminMember의 role이 현재 string으로 되어 있어 컴파일 시점 검사가 불가능하므로 src/api/member-management/types.ts에 정의된 MemberRole 유니온 타입을 재사용하도록 변경하세요: AdminMember 인터페이스의 role 타입을 MemberRole로 바꾸고 필요한 경우 해당 파일에서 MemberRole을 import 하여 사용하도록 수정(인터페이스 이름: AdminMember, 외부 타입: MemberRole).src/hooks/useSessionKeepAlive.ts (1)
36-40:catch블록이 도달 불가능한 코드(dead code)입니다
Members()는 내부적으로src/lib/http/client.ts의requestApi를 통해 호출되며, 해당 함수는fetch오류를 내부try-catch로 잡아{ ok: false, status: 0 }을 반환합니다. 결국Members()는 절대 예외를 던지지 않으므로 이catch블록은 실행되지 않습니다.♻️ 제안 수정 (catch 블록 제거)
const ping = async () => { if (inFlight || !active) return; inFlight = true; - try { - const response = await Members(); - const unauthorized = ...; - if (active && unauthorized) { - onUnauthorized?.(); - } - } catch { - // 네트워크 오류 등은 조용히 무시하고 다음 틱에 재시도 - } finally { - inFlight = false; - } + const response = await Members(); + const isAuthFailure = response.status === 401 || response.status === 403; + const isRoleMismatch = response.ok && (!response.data || response.data.role !== 'ADMIN'); + if (active && (isAuthFailure || isRoleMismatch)) { + onUnauthorized?.(); + } + inFlight = false; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useSessionKeepAlive.ts` around lines 36 - 40, The catch block in useSessionKeepAlive.ts is dead because Members() (which calls requestApi in src/lib/http/client.ts) never throws — requestApi already catches fetch errors and returns { ok: false, status: 0 }; remove the unused catch block and keep the finally to ensure inFlight is reset, i.e., delete the empty catch branch around the Members() call in the function that toggles inFlight while leaving the surrounding try/finally intact.src/api/auth/members.ts (1)
4-7: 관용적이지 않은 함수 명명 및 불필요한 메서드 지정두 가지 개선 사항입니다:
Members()는 파스칼케이스를 사용하는데, TypeScript/JavaScript에서 함수명은 일반적으로 캐멀케이스를 따릅니다 (getMembers등).method: 'GET'은fetch의 기본값이므로 생략 가능합니다.♻️ 제안하는 수정
-export async function Members(): Promise<ApiResult<AdminMember>> { - return requestApi<AdminMember>('/members', { - method: 'GET', - }); +export async function getMembers(): Promise<ApiResult<AdminMember>> { + return requestApi<AdminMember>('/members'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/auth/members.ts` around lines 4 - 7, Rename the exported function Members to a camelCase name like getMembers and update all references/imports accordingly, and simplify the request by removing the redundant method: 'GET' option from the requestApi call (leave requestApi<AdminMember>('/members') as the call site). Ensure the function signature remains async and returns Promise<ApiResult<AdminMember>> and update any tests or consumers that call Members to use getMembers.src/page/event/sections/EventTableSection.tsx (2)
65-68: 불필요한div래퍼
flex items-center gap-2클래스를 가진div안에<span>하나만 존재합니다. 아이콘 등 추가 요소가 없다면 래퍼를 제거해도 됩니다.♻️ 제안하는 수정
<TableCell> - <div className="flex items-center gap-2"> - <span>{row.name}</span> - </div> + {row.name} </TableCell>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/event/sections/EventTableSection.tsx` around lines 65 - 68, Remove the unnecessary wrapper div inside the TableCell in EventTableSection: replace the <div className="flex items-center gap-2"> that only contains a single <span>{row.name}</span> with the span itself (or the span plus the classes if layout is needed on the span), so keep TableCell and the span showing row.name but eliminate the redundant div wrapper to simplify the markup and DOM.
74-85: 비동기 작업 중 버튼 비활성화 처리 누락
수정/삭제버튼은 비동기 핸들러를 호출하지만, 작업이 진행 중인 동안에도 버튼이 활성 상태를 유지합니다. 중복 클릭 시 동일 행에 대해 동시 요청이 발생할 수 있습니다.
PaymentManagementSection의 "강제 완료" 버튼은forceCompletingPaymentId를 추적하여 해당 행의 버튼을 비활성화합니다. 이와 동일한 패턴을 적용해야 합니다.
useEventPageState훅과EventTableSectionProps에processingRowId: number | null상태를 추가하고, 해당 행의 버튼을disabled={processingRowId === row.id}로 제어하는 방식을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/event/sections/EventTableSection.tsx` around lines 74 - 85, Add a processingRowId state and wire it through EventTableSection props/state so row-level async actions disable buttons while in-flight: update EventTableSectionProps and the useEventPageState hook to include processingRowId: number | null; in the handlers onOpenEditDialog and onDelete set processingRowId = row.id before starting the async operation and clear it after completion/failure; then change the row buttons to disabled={processingRowId === row.id} (apply same for any action that should lock the row, e.g., onOpenQR if needed) to prevent duplicate concurrent requests.src/page/point/sections/PointLedgerSection.tsx (1)
49-75:md:grid-cols-6그리드에 자식 요소가 5개로, 마지막 열이 항상 비어있습니다.필터 컨트롤(키워드 입력, 유형 선택, 시작일, 종료일, 조회 버튼) 5개가 6열 그리드 안에 배치되어 있어 오른쪽 끝 열이 비어있습니다.
md:grid-cols-5로 변경하거나, 버튼에col-span-2를 추가하는 방향이 의도에 맞을 수 있습니다.♻️ 제안: 열 수 수정
- <div className="grid grid-cols-1 gap-3 md:grid-cols-6"> + <div className="grid grid-cols-1 gap-3 md:grid-cols-5">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/point/sections/PointLedgerSection.tsx` around lines 49 - 75, The grid wrapper currently uses "md:grid-cols-6" causing an empty column because there are five child controls; change the layout to "md:grid-cols-5" on the div with className="grid grid-cols-1 gap-3 md:grid-cols-6" OR keep the 6-column layout and make the Button span two columns by adding a "col-span-2" class to the Button component so the controls fill the grid correctly; update the div's className or the Button's className accordingly (references: the div with className containing md:grid-cols-6, the Button element with onClick={() => void onLedgerSearch()}).src/page/coupon/sections/CouponCodeTabSection.tsx (1)
50-76: 쿠폰 미선택 상태에서 코드 생성 요청 가능선택되지 않은 상태에서도 생성 버튼이 활성화됩니다. 선택이 완료된 경우에만 활성화하면 오류 요청을 줄일 수 있습니다.
✅ 제안 변경
- <Button onClick={() => void onCreateCouponCode()}>코드 생성</Button> + <Button onClick={() => void onCreateCouponCode()} disabled={!selectedCouponIdForCode}> + 코드 생성 + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/coupon/sections/CouponCodeTabSection.tsx` around lines 50 - 76, The Create button is active even when no coupon is selected; update the UI to prevent submissions by disabling the Button unless a coupon is chosen. In CouponCodeTabSection.tsx use the selectedCouponIdForCode state to control the Button's disabled prop (e.g., disabled when selectedCouponIdForCode is null/undefined/0 or empty string) and also guard the onCreateCouponCode invocation inside its handler to no-op when no valid selectedCouponIdForCode exists. Reference the Select, selectedCouponIdForCode, and onCreateCouponCode symbols to locate and apply these changes.src/page/point/sections/PointGrantSection.tsx (1)
61-123: 입력값 검증 없이 지급 실행 가능단건/일괄 지급 버튼이 대상/금액 미입력 상태에서도 활성화됩니다. UI에서 기본 검증을 추가하면 불필요한 요청을 줄일 수 있습니다.
✅ 제안 변경
export const PointGrantSection: React.FC<PointGrantSectionProps> = ({ memberSearchResults, singleMemberId, singleAmount, singleReason, isSingleSubmitting, selectedBatchMemberIds, selectedBatchMembers, batchAmount, batchReason, isBatchSubmitting, batchResult, onSingleMemberIdChange, onSingleAmountChange, onSingleReasonChange, onSingleGrant, onBatchAmountChange, onBatchReasonChange, onBatchGrant, }) => { + const isSingleValid = !!singleMemberId && Number(singleAmount) > 0 + const isBatchValid = selectedBatchMemberIds.length > 0 && Number(batchAmount) > 0 + return ( <Card> @@ - <Button className="w-full" onClick={() => void onSingleGrant()} disabled={isSingleSubmitting}> + <Button + className="w-full" + onClick={() => void onSingleGrant()} + disabled={isSingleSubmitting || !isSingleValid} + > {isSingleSubmitting ? "지급 중..." : "단건 지급"} </Button> @@ - <Button className="w-full" onClick={() => void onBatchGrant()} disabled={isBatchSubmitting}> + <Button + className="w-full" + onClick={() => void onBatchGrant()} + disabled={isBatchSubmitting || !isBatchValid} + > {isBatchSubmitting ? "일괄 지급 중..." : "일괄 지급"} </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/point/sections/PointGrantSection.tsx` around lines 61 - 123, The grant buttons (onSingleGrant, onBatchGrant) are enabled even when required inputs are missing; update their disabled conditions to also check input validity: for the single grant Button disable when isSingleSubmitting is true OR singleMemberId is null/undefined OR singleAmount is empty/<=0 (and optionally require singleReason if needed); for the batch grant Button disable when isBatchSubmitting is true OR selectedBatchMemberIds.length === 0 OR batchAmount is empty/<=0 (and optionally require batchReason); implement these predicates where the Button components render so the UI prevents submitting invalid requests before calling onSingleGrant/onBatchGrant.src/page/payment/sections/PaymentManagementSection.tsx (1)
37-43:formatDateTime코드 중복 — 공유 유틸리티로 추출 권장동일한
formatDateTime함수가useMemberManagementPageState.ts,useCouponPageState.ts,feature-flags.tsx, 그리고 이 파일에 걸쳐 최소 4곳에 중복 정의되어 있습니다. 또한 이 파일의 함수는string만 받아 null을 처리하지 않아 다른 구현체와 시그니처가 불일치합니다.공유 유틸 파일(예:
src/utils/format.ts)로 추출하여 일관성을 유지하세요.♻️ 개선 방향 예시
-function formatDateTime(value: string): string { - const parsed = new Date(value) - if (Number.isNaN(parsed.getTime())) { - return "-" - } - return parsed.toLocaleString("ko-KR", { hour12: false }) -}
src/utils/format.ts에string | null시그니처로 통합:+// src/utils/format.ts +export function formatDateTime(value: string | null): string { + if (!value) return "-" + const parsed = new Date(value) + if (Number.isNaN(parsed.getTime())) return "-" + return parsed.toLocaleString("ko-KR", { hour12: false }) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/sections/PaymentManagementSection.tsx` around lines 37 - 43, The duplicated formatDateTime implementation should be extracted to a shared utility (e.g. create a function formatDateTime in src/utils/format.ts) with the unified signature formatDateTime(value: string | null): string that returns "-" for null/invalid dates and otherwise returns parsed.toLocaleString("ko-KR", { hour12: false }); replace the local definitions in PaymentManagementSection (and other places such as useMemberManagementPageState, useCouponPageState, feature-flags) to import this shared formatDateTime and remove the duplicated functions so all callers use the single implementation.src/page/coupon/sections/CouponTabSection.tsx (1)
61-103: 비동기 작업 중 버튼 비활성화 및 삭제 확인 없음
- "쿠폰 생성", "저장" 버튼에 처리 중
disabled상태가 없어 중복 요청이 가능합니다.- "삭제" 버튼에 확인(confirm) 절차가 없어 실수로 쿠폰이 삭제될 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/coupon/sections/CouponTabSection.tsx` around lines 61 - 103, The "쿠폰 생성" and "저장" buttons can send duplicate requests because they lack disabled state during async operations, and the "삭제" button has no confirmation; add and use loading state(s) to disable UI during async work and prompt for confirmation before deleting: introduce a createLoading state used by onCreateCoupon to disable the "쿠폰 생성" Button, introduce per-item updateLoading map keyed by couponId (or a single updatingId) used by onUpdateCouponName to disable the "저장" Button and Input for that coupon (referencing couponNameDrafts and onCouponNameDraftChange), and modify onDeleteCoupon to show a confirmation (e.g., window.confirm or a modal) and set a deleteLoading state for that coupon to disable the "삭제" Button while the deletion runs (use filteredCoupons and coupon.couponId to identify items).src/page/event/sections/EventDialogsSection.tsx (1)
17-22:Dispatch<SetStateAction<T>>를 단순 콜백 타입으로 교체 권장컴포넌트 prop 타입으로 React 내부 타입인
Dispatch<SetStateAction<T>>를 사용하면 부모가useState의 setter를 직접 전달해야 합니다.useReducer나 다른 상태 관리 방식 사용 시 호환성이 저하됩니다.♻️ 개선 제안
-import type { Dispatch, FormEvent, SetStateAction } from "react" +import type { FormEvent } from "react" interface EventDialogsSectionProps { showEditDialog: boolean - onShowEditDialogChange: Dispatch<SetStateAction<boolean>> + onShowEditDialogChange: (value: boolean) => void editingActivityId: number | null eventName: string - onEventNameChange: Dispatch<SetStateAction<string>> + onEventNameChange: (value: string) => void pointAmount: string - onPointAmountChange: Dispatch<SetStateAction<string>> + onPointAmountChange: (value: string) => void🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/event/sections/EventDialogsSection.tsx` around lines 17 - 22, Replace the React-specific Dispatch<SetStateAction<T>> prop types with plain callback types to decouple the component from useState internals: change onShowEditDialogChange from Dispatch<SetStateAction<boolean>> to (next: boolean) => void (or (updater: boolean | ((prev:boolean)=>boolean))=>void if you need functional updates), change onEventNameChange from Dispatch<SetStateAction<string>> to (next: string) => void, and change onPointAmountChange from Dispatch<SetStateAction<string>> to (next: string) => void; then update any internal calls in EventDialogsSection that currently pass the setter directly to call these callbacks (e.g., where onShowEditDialogChange(...), onEventNameChange(...), onPointAmountChange(...)) so they accept plain values or functional updaters as you chose.src/page/event/hooks/useEventPageState.ts (1)
53-65: 비동기 로딩 플래그를 try/finally로 보호해주세요.
요청 함수가 예외를 던지면isLoading이 true로 고정될 수 있습니다.try/finally로 해제를 보장하면 안정성이 좋아집니다.♻️ 제안 변경
const loadActivities = useCallback(async () => { setIsLoading(true) - const response = await GetActivities() - if (!response.ok || !response.data) { - setActivities([]) - showError(resolveAdminErrorMessage(response.errorName, { fallback: "활동 목록을 불러올 수 없습니다." })) - setIsLoading(false) - return - } - - setActivities(response.data) - setIsLoading(false) + try { + const response = await GetActivities() + if (!response.ok || !response.data) { + setActivities([]) + showError(resolveAdminErrorMessage(response.errorName, { fallback: "활동 목록을 불러올 수 없습니다." })) + return + } + + setActivities(response.data) + } finally { + setIsLoading(false) + } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/event/hooks/useEventPageState.ts` around lines 53 - 65, The loadActivities async callback sets isLoading=true but doesn't guarantee resetting it if GetActivities throws; wrap the await call and subsequent logic in a try/finally so setIsLoading(false) always runs. Specifically, in the loadActivities function (where GetActivities() is called and setActivities/showError are used), move the async call and response handling into a try block and call setIsLoading(false) in a finally block to ensure the loading flag is cleared on success, error response, or thrown exception.src/page/point/hooks/usePointPageState.ts (1)
78-118: 로딩 플래그는 예외 발생 시에도 해제되도록 보호하세요.
요청 함수가 throw 되면isLedgerLoading/isMemberPointLoading이 true로 남을 수 있습니다.try/finally로 안전하게 정리하는 편이 좋습니다.♻️ 제안 변경
const fetchLedger = async ( page: number, memberKeyword: string, transactionType: PointTransactionFilter, from: string, to: string, ): Promise<void> => { setIsLedgerLoading(true) - const response = await getPointLedger({ - page, - size: 50, - memberKeyword, - transactionType: transactionType === "ALL" ? undefined : transactionType, - from: from || undefined, - to: to || undefined, - }) - - if (!response.ok || !response.data) { - showError(resolvePointErrorMessage(response.errorName)) - setIsLedgerLoading(false) - return - } - - setLedgerData(response.data) - setIsLedgerLoading(false) + try { + const response = await getPointLedger({ + page, + size: 50, + memberKeyword, + transactionType: transactionType === "ALL" ? undefined : transactionType, + from: from || undefined, + to: to || undefined, + }) + + if (!response.ok || !response.data) { + showError(resolvePointErrorMessage(response.errorName)) + return + } + + setLedgerData(response.data) + } finally { + setIsLedgerLoading(false) + } } const fetchMemberPoint = async (memberId: number): Promise<void> => { setIsMemberPointLoading(true) - const response = await getPointMember(memberId) - if (!response.ok || !response.data) { - showError(resolvePointErrorMessage(response.errorName)) - setIsMemberPointLoading(false) - return - } - - setSelectedDetailMemberId(memberId) - setMemberPoint(response.data) - setSingleMemberId(memberId) - setIsMemberPointLoading(false) + try { + const response = await getPointMember(memberId) + if (!response.ok || !response.data) { + showError(resolvePointErrorMessage(response.errorName)) + return + } + + setSelectedDetailMemberId(memberId) + setMemberPoint(response.data) + setSingleMemberId(memberId) + } finally { + setIsMemberPointLoading(false) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/point/hooks/usePointPageState.ts` around lines 78 - 118, The loading flags can remain true if getPointLedger or getPointMember throws; wrap the bodies of fetchLedger and fetchMemberPoint in try/finally so setIsLedgerLoading(false) and setIsMemberPointLoading(false) always run, and keep existing early-return error handling inside the try; locate functions fetchLedger and fetchMemberPoint and ensure any await calls to getPointLedger/getPointMember are inside the try block and the corresponding setIs...Loading(false) calls are moved to the finally block.src/page/payment/hooks/usePaymentPageState.ts (1)
72-76: 하드코딩된 학기 옵션은 유지보수 부담이 됩니다.현재 2026년 2월 기준으로 2026-2 학기가 곧 필요해질 것이며, 매 학기마다 코드를 수정해야 합니다. 서버에서 옵션 목록을 가져오거나, 현재 날짜 기준으로 동적 생성하는 방안을 고려해 보세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/hooks/usePaymentPageState.ts` around lines 72 - 76, Replace the hardcoded YEAR_SEMESTER_OPTIONS array by generating the options dynamically (or fetching them from the server) inside usePaymentPageState: compute year/semester pairs based on the current date (e.g., determine current year and whether it's semester 1 or 2 and produce a range of past/future semesters) or call an API endpoint to retrieve semester options, then map those results to the YearSemesterOption shape; update any consumer of YEAR_SEMESTER_OPTIONS to use the new generator/function so the list auto-updates without code changes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (102)
.gitignorecomponents.jsonpackage.jsonsrc/App.tsxsrc/api/activity/delete-acitivites.tssrc/api/activity/get-activities.tssrc/api/activity/get-qrcode.tssrc/api/activity/post-activities.tssrc/api/activity/post-memebr-activities.tssrc/api/activity/put-activities.tssrc/api/activity/request.tssrc/api/activity/types.tssrc/api/auth/members.tssrc/api/auth/request.tssrc/api/auth/types.tssrc/api/coupon/request.tssrc/api/coupon/types.tssrc/api/member-management/request.tssrc/api/member-management/types.tssrc/api/payment/get-admin-payments.tssrc/api/payment/get-admin-transactions.tssrc/api/payment/patch-force-complete-payment.tssrc/api/payment/request.tssrc/api/payment/types.tssrc/api/point/request.tssrc/api/point/types.tssrc/components/EventTable.tsxsrc/components/Header.tsxsrc/components/QRScanner.tsxsrc/components/Search.tsxsrc/components/SideBar.tsxsrc/components/admin/AdminFilterBar.tsxsrc/components/admin/AdminPageHeader.tsxsrc/components/admin/AdminSectionCard.tsxsrc/components/admin/AdminTableEmptyRow.tsxsrc/components/admin/index.tssrc/components/layout/AppHeader.tsxsrc/components/layout/AppShell.tsxsrc/components/layout/AppSidebar.tsxsrc/components/layout/ProtectedLayout.tsxsrc/components/layout/nav-items.tsxsrc/components/ui/alert-dialog.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/checkbox.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/scroll-area.tsxsrc/components/ui/select.tsxsrc/components/ui/separator.tsxsrc/components/ui/sheet.tsxsrc/components/ui/skeleton.tsxsrc/components/ui/sonner.tsxsrc/components/ui/table.tsxsrc/components/ui/tabs.tsxsrc/components/ui/textarea.tsxsrc/context/AuthContext.tsxsrc/hooks/useSessionKeepAlive.tssrc/index.csssrc/lib/errors/admin-error.tssrc/lib/http/client.tssrc/lib/http/types.tssrc/lib/utils.tssrc/page/coupon.tsxsrc/page/coupon/hooks/useCouponPageState.tssrc/page/coupon/sections/CouponCodeTabSection.tsxsrc/page/coupon/sections/CouponIssuedTabSection.tsxsrc/page/coupon/sections/CouponPageHeader.tsxsrc/page/coupon/sections/CouponTabSection.tsxsrc/page/event.tsxsrc/page/event/hooks/useEventPageState.tssrc/page/event/sections/EventDialogsSection.tsxsrc/page/event/sections/EventTableSection.tsxsrc/page/event/sections/EventToolbarSection.tsxsrc/page/feature-flags.tsxsrc/page/home.tsxsrc/page/login.tsxsrc/page/member-demotion.tsxsrc/page/member-management.tsxsrc/page/member-management/hooks/useMemberManagementPageState.tssrc/page/member-management/sections/MemberDetailSection.tsxsrc/page/member-management/sections/MemberManagementHeaderSection.tsxsrc/page/member-management/sections/MemberRecordsSection.tsxsrc/page/notfound.tsxsrc/page/payment.tsxsrc/page/payment/hooks/usePaymentPageState.tssrc/page/payment/sections/PaymentManagementSection.tsxsrc/page/payment/sections/TransactionManagementSection.tsxsrc/page/point.tsxsrc/page/point/hooks/usePointPageState.tssrc/page/point/sections/PointGrantSection.tsxsrc/page/point/sections/PointLedgerSection.tsxsrc/page/point/sections/PointMemberSection.tsxsrc/page/point/sections/PointPageHeaderSection.tsxsrc/utils/alert.tsxtsconfig.app.jsontsconfig.jsontsconfig.node.jsonvite.config.ts
💤 Files with no reviewable changes (4)
- src/components/SideBar.tsx
- src/components/EventTable.tsx
- src/components/Search.tsx
- src/components/Header.tsx
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
src/page/member-management/hooks/useMemberManagementPageState.ts (1)
129-218: 이전 리뷰에서 지적된 두 가지 문제 모두 올바르게 수정되었습니다.
loadSemesters및loadRecords모두try/catch/finally패턴으로 감싸져 있어 네트워크 오류 시에도 로딩 상태가 영구적으로 유지되지 않습니다.- 각
useEffect내부에stale플래그와 cleanup 함수(return () => { stale = true })가 적용되어 의존성이 빠르게 변경될 때 오래된 응답이 상태를 덮어쓰는 문제도 해결되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-management/hooks/useMemberManagementPageState.ts` around lines 129 - 218, Two concurrency/cleanup bugs were identified: loading spinners could remain true on network errors and stale async responses could overwrite newer state; both are now fixed by wrapping async calls in try/catch/finally and using a stale flag with a cleanup function. Ensure the try/catch/finally pattern in loadSemesters and loadRecords always clears loading via setIsSemesterLoading(false) and setIsRecordLoading(false) in the finally block, and keep the stale boolean + return () => { stale = true } cleanup inside both useEffect hooks so outdated responses (inside loadSemesters and loadRecords) return early and never call setState; verify the dependencies array for the records effect includes [selectedYearSemester, recordPageIndex, appliedKeyword, roleFilter, sortFilter] so effects run correctly.src/page/member-demotion.tsx (1)
38-57:try/finally패턴이 올바르게 적용되었습니다.이전 리뷰에서 지적된
isSubmitting이 예외 발생 시 영구적으로true로 남는 문제가finally블록으로 해결되었습니다. Line 44의return이 실행될 때도finally가 동작하므로 모든 경로에서setIsSubmitting(false)가 보장됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-demotion.tsx` around lines 38 - 57, The try/finally correctly ensures setIsSubmitting(false) always runs (even when the early return after showError executes), so no change is needed; leave the demoteMembersForCurrentSemester call and its surrounding try { ... } finally { setIsSubmitting(false) } intact and keep the existing usage of showError, showSuccess, setLastDemotedStudentIds and handling of response.data.
🧹 Nitpick comments (6)
src/components/QRScanner.tsx (2)
27-35:setTimeout반환값을 저장하지 않아 언마운트 시 정리되지 않음성공 경로에서
resetProcessingState(1000)을 호출하는데, 1초 내에 컴포넌트가 언마운트되면 타이머가 해제된 ref와 state를 갱신합니다. React 18+에서 경고는 발생하지 않지만, 클린업 누락은 잠재적 버그의 원인이 됩니다.🛡️ 타이머 정리를 위한 개선안
+ const resetTimerRef = useRef<ReturnType<typeof setTimeout>>(null) + const resetProcessingState = useCallback((delayMs: number) => { + if (resetTimerRef.current) { + clearTimeout(resetTimerRef.current) + } - window.setTimeout(() => { + resetTimerRef.current = window.setTimeout(() => { isProcessingRef.current = false isThrottled.current = false lastProcessedQR.current = "" lastScanTime.current = 0 setIsProcessing(false) }, delayMs) }, [])그리고 cleanup effect 추가:
useEffect(() => { return () => { if (resetTimerRef.current) { clearTimeout(resetTimerRef.current) } } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QRScanner.tsx` around lines 27 - 35, The resetProcessingState function starts a timer via window.setTimeout but doesn't save the timeout id, so it isn't cleared on unmount; update resetProcessingState to store the returned id in a ref (e.g., resetTimerRef.current) and clear any existing timer before setting a new one, and add a cleanup useEffect that calls clearTimeout(resetTimerRef.current) on unmount and resets the ref; reference the resetProcessingState function and resetTimerRef so the change is easy to locate.
42-48:isThrottled와isProcessingRef는 항상 동시에 설정/해제되어 사실상 중복두 ref가 라인 50-51에서 동시에
true로 설정되고,resetProcessingState에서 동시에false로 초기화됩니다. 현재로는 하나로 통합 가능합니다. 향후 분리할 계획이 있다면 의도를 주석으로 남겨두면 좋겠습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QRScanner.tsx` around lines 42 - 48, The two refs isThrottled and isProcessingRef are redundant because they are set and cleared together; consolidate them into a single ref (e.g., isProcessing) by replacing uses of isThrottled.current and isProcessingRef.current with isProcessing.current, updating the early-return check in the scanner (the block referencing isProcessingRef.current || isThrottled.current), the places that set them true on start (lines where both are set), and resetProcessingState to clear only isProcessing.current; if you intend to keep them separate for future behavior, add a clear comment above their declaration explaining that intent instead of keeping two refs with identical semantics.src/page/payment/hooks/usePaymentPageState.ts (2)
134-165: 연속 요청 시 응답 역전 방지를 고려해 주세요.빠른 검색/페이지 전환 시 이전 응답이 최신 상태를 덮어쓸 수 있으니 requestId/AbortController 등으로 최신 요청만 반영하는 방식을 검토해 주세요. (fetchPayments에도 동일 적용 가능)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/hooks/usePaymentPageState.ts` around lines 134 - 165, fetchTransactions can suffer from response reordering when users make rapid requests; update it to track and apply only the latest request by adding a per-hook request identifier or AbortController (e.g., a requestIdRef or controllerRef) that you increment/create before each call, abort the previous controller if using AbortController, capture the current id/controller inside fetchTransactions, and only call setTransactionData / setIsTransactionsLoading (and showError) when the response matches the current id or is not aborted; apply the same pattern to fetchPayments to prevent stale responses overwriting newer state.
72-76: 학기 옵션 하드코딩은 금방 노후화될 수 있습니다.학기 옵션을 설정/백엔드에서 주입하거나 상수 파일로 분리해 매 학기 코드 수정을 줄이는 방안을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/payment/hooks/usePaymentPageState.ts` around lines 72 - 76, The YEAR_SEMESTER_OPTIONS constant in usePaymentPageState.ts is hardcoded and will become outdated; extract these options into a shared constant or backend-config source and make the hook accept injected options with a fallback. Concretely: move the array into a new export (e.g., export const YEAR_SEMESTER_OPTIONS_DEFAULT) in a constants module, import and use that default in usePaymentPageState, and update the hook signature (usePaymentPageState) to accept an optional yearSemesterOptions parameter so callers (or a backend loader) can provide up-to-date values while still falling back to the shared default.src/page/member-management/hooks/useMemberManagementPageState.ts (1)
125-127:resolveSemesterLabel을useCallback으로 감싸 참조 안정성을 확보하세요.
semesterLabelMap은useMemo로 안정적이지만,resolveSemesterLabel은 매 렌더마다 새 참조로 생성됩니다. 이를 반환받는 소비 컴포넌트에서 해당 함수를 의존성 배열에 포함하면 불필요한 재실행이 발생할 수 있습니다.♻️ useCallback 적용 제안
- const resolveSemesterLabel = (yearSemester: string): string => { - return semesterLabelMap.get(yearSemester) ?? yearSemester - } + const resolveSemesterLabel = useCallback( + (yearSemester: string): string => semesterLabelMap.get(yearSemester) ?? yearSemester, + [semesterLabelMap], + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-management/hooks/useMemberManagementPageState.ts` around lines 125 - 127, The function resolveSemesterLabel is recreated on every render which breaks reference stability; wrap it with React's useCallback and return the memoized callback instead (i.e., replace the plain const resolveSemesterLabel = (...) => ... with a useCallback that depends on semesterLabelMap) so consumers can safely include resolveSemesterLabel in dependency arrays; ensure the dependency array includes semesterLabelMap (or any other values it reads).src/page/member-demotion.tsx (1)
82-85: 초기 로드 시 "0명" 표시가 의미론적으로 어색합니다.강등 작업을 한 번도 실행하지 않은 상태에서도
0명이 표시되어, 실제로 0명이 강등된 결과인지 아직 아무 작업도 수행하지 않은 상태인지 구분이 되지 않습니다. "-" 등의 초기값 처리를 고려할 수 있습니다.✨ 개선 제안
- <div className="flex items-center gap-2"> - <span className="text-muted-foreground">마지막 강등 인원</span> - <Badge variant="secondary">{lastDemotedStudentIds.length}명</Badge> - </div> + <div className="flex items-center gap-2"> + <span className="text-muted-foreground">마지막 강등 인원</span> + <Badge variant="secondary"> + {lastDemotedStudentIds.length > 0 || hasRunOnce ? `${lastDemotedStudentIds.length}명` : "-"} + </Badge> + </div>또는 더 단순하게
null상태(lastDemotedStudentIds를string[] | null로 초기화)를 두어 강등 결과 섹션 자체를 실행 전에는 숨기는 방법도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/page/member-demotion.tsx` around lines 82 - 85, The UI always shows "0명" because lastDemotedStudentIds is initialized as an empty array; change the state and rendering so the initial/unexecuted state is distinct: update the state type of lastDemotedStudentIds to string[] | null (or add an isLoaded flag) and initialize it to null, then modify the render around the Badge (the JSX that currently uses lastDemotedStudentIds.length and Badge) to show a placeholder like "–" (or hide the whole section) when lastDemotedStudentIds is null, and only display `${lastDemotedStudentIds.length}명` after it becomes a non-null array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/QRScanner.tsx`:
- Around line 109-118: The inline onUnauthorized arrow passed to
useSessionKeepAlive in QRScanner creates a new function each render and causes
the hook to reset its interval; wrap the callback in useCallback (e.g., create a
memoized onUnauthorized via React.useCallback that calls showError, onClose, and
sets window.location.href) and pass that memoized function to
useSessionKeepAlive, or alternatively update useSessionKeepAlive to hold the
latest onUnauthorized in a ref so it doesn't include the callback in its effect
dependencies (modify useSessionKeepAlive to useRef for onUnauthorized and call
ref.current inside the interval).
In `@src/page/event/hooks/useEventPageState.ts`:
- Around line 53-65: The loadActivities function can leave isLoading true if
GetActivities throws; wrap the async work in a try/catch/finally: call
setIsLoading(true) before the try, await GetActivities() inside try, in catch
handle the thrown error by calling setActivities([]) and
showError(resolveAdminErrorMessage(...)) (same message currently used), and in
finally always call setIsLoading(false) so the loading state is cleared; update
the useCallback implementation for loadActivities to use try/catch/finally and
reference GetActivities, setIsLoading, setActivities, and showError.
- Around line 165-202: The branch in handleSubmitEvent uses a truthy check on
editingActivityId so an ID of 0 falls through to the "create" path; change the
condition to an explicit null check (e.g., if (editingActivityId !== null) or if
(editingActivityId != null) ) so updateActivity(editingActivityId, ...) is used
for valid numeric IDs including 0; update the condition only (and keep
references to updateActivity/createActivity and editingActivityId intact).
In `@src/page/member-demotion.tsx`:
- Line 19: The file references the React namespace via the type React.FC on the
component declaration (MemberDemotionPage) but only imports useState, causing a
TS 'React refers to a UMD global' error; fix by either importing the React
namespace (add React to the import, e.g. import React, { useState } from
"react") or remove the namespace usage by switching the component to a plain
function type (change const MemberDemotionPage: React.FC = () => { to const
MemberDemotionPage = () => {) and ensure the import line that currently reads
import { useState } from "react" is updated accordingly.
In `@src/page/member-management/hooks/useMemberManagementPageState.ts`:
- Around line 21-25: FALLBACK_SEMESTER_OPTIONS currently hardcodes semesters and
sets the wrong current flag; update the fallback so the "current" field is
computed at runtime instead of hardcoded: implement a small helper in
useMemberManagementPageState (or nearby) that derives the current year+semester
from today's date and maps MemberRecordSemesterOption entries to set
current=true for the computed semester, or load the fallback array from a
configurable source (env/config) and then mark the computed current one; update
the FALLBACK_SEMESTER_OPTIONS usage to call that helper so the active semester
is always determined dynamically.
- Around line 287-293: The semester-change handler handleChangeSemester
currently resets recordPageIndex, selectedMember, timelineItems, and
activityDetail but does not clear keyword filters; update handleChangeSemester
to also reset keyword-related state by calling setKeywordInput('') and
setAppliedKeyword(null or '') so prior keyword filters do not persist across
semesters and cause unexpected filtering of the new semester's member list.
---
Duplicate comments:
In `@src/page/member-demotion.tsx`:
- Around line 38-57: The try/finally correctly ensures setIsSubmitting(false)
always runs (even when the early return after showError executes), so no change
is needed; leave the demoteMembersForCurrentSemester call and its surrounding
try { ... } finally { setIsSubmitting(false) } intact and keep the existing
usage of showError, showSuccess, setLastDemotedStudentIds and handling of
response.data.
In `@src/page/member-management/hooks/useMemberManagementPageState.ts`:
- Around line 129-218: Two concurrency/cleanup bugs were identified: loading
spinners could remain true on network errors and stale async responses could
overwrite newer state; both are now fixed by wrapping async calls in
try/catch/finally and using a stale flag with a cleanup function. Ensure the
try/catch/finally pattern in loadSemesters and loadRecords always clears loading
via setIsSemesterLoading(false) and setIsRecordLoading(false) in the finally
block, and keep the stale boolean + return () => { stale = true } cleanup inside
both useEffect hooks so outdated responses (inside loadSemesters and
loadRecords) return early and never call setState; verify the dependencies array
for the records effect includes [selectedYearSemester, recordPageIndex,
appliedKeyword, roleFilter, sortFilter] so effects run correctly.
---
Nitpick comments:
In `@src/components/QRScanner.tsx`:
- Around line 27-35: The resetProcessingState function starts a timer via
window.setTimeout but doesn't save the timeout id, so it isn't cleared on
unmount; update resetProcessingState to store the returned id in a ref (e.g.,
resetTimerRef.current) and clear any existing timer before setting a new one,
and add a cleanup useEffect that calls clearTimeout(resetTimerRef.current) on
unmount and resets the ref; reference the resetProcessingState function and
resetTimerRef so the change is easy to locate.
- Around line 42-48: The two refs isThrottled and isProcessingRef are redundant
because they are set and cleared together; consolidate them into a single ref
(e.g., isProcessing) by replacing uses of isThrottled.current and
isProcessingRef.current with isProcessing.current, updating the early-return
check in the scanner (the block referencing isProcessingRef.current ||
isThrottled.current), the places that set them true on start (lines where both
are set), and resetProcessingState to clear only isProcessing.current; if you
intend to keep them separate for future behavior, add a clear comment above
their declaration explaining that intent instead of keeping two refs with
identical semantics.
In `@src/page/member-demotion.tsx`:
- Around line 82-85: The UI always shows "0명" because lastDemotedStudentIds is
initialized as an empty array; change the state and rendering so the
initial/unexecuted state is distinct: update the state type of
lastDemotedStudentIds to string[] | null (or add an isLoaded flag) and
initialize it to null, then modify the render around the Badge (the JSX that
currently uses lastDemotedStudentIds.length and Badge) to show a placeholder
like "–" (or hide the whole section) when lastDemotedStudentIds is null, and
only display `${lastDemotedStudentIds.length}명` after it becomes a non-null
array.
In `@src/page/member-management/hooks/useMemberManagementPageState.ts`:
- Around line 125-127: The function resolveSemesterLabel is recreated on every
render which breaks reference stability; wrap it with React's useCallback and
return the memoized callback instead (i.e., replace the plain const
resolveSemesterLabel = (...) => ... with a useCallback that depends on
semesterLabelMap) so consumers can safely include resolveSemesterLabel in
dependency arrays; ensure the dependency array includes semesterLabelMap (or any
other values it reads).
In `@src/page/payment/hooks/usePaymentPageState.ts`:
- Around line 134-165: fetchTransactions can suffer from response reordering
when users make rapid requests; update it to track and apply only the latest
request by adding a per-hook request identifier or AbortController (e.g., a
requestIdRef or controllerRef) that you increment/create before each call, abort
the previous controller if using AbortController, capture the current
id/controller inside fetchTransactions, and only call setTransactionData /
setIsTransactionsLoading (and showError) when the response matches the current
id or is not aborted; apply the same pattern to fetchPayments to prevent stale
responses overwriting newer state.
- Around line 72-76: The YEAR_SEMESTER_OPTIONS constant in
usePaymentPageState.ts is hardcoded and will become outdated; extract these
options into a shared constant or backend-config source and make the hook accept
injected options with a fallback. Concretely: move the array into a new export
(e.g., export const YEAR_SEMESTER_OPTIONS_DEFAULT) in a constants module, import
and use that default in usePaymentPageState, and update the hook signature
(usePaymentPageState) to accept an optional yearSemesterOptions parameter so
callers (or a backend loader) can provide up-to-date values while still falling
back to the shared default.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (9)
src/api/activity/post-activities.tssrc/api/activity/post-member-activities.tssrc/components/QRScanner.tsxsrc/hooks/useSessionKeepAlive.tssrc/page/event/hooks/useEventPageState.tssrc/page/event/sections/EventDialogsSection.tsxsrc/page/member-demotion.tsxsrc/page/member-management/hooks/useMemberManagementPageState.tssrc/page/payment/hooks/usePaymentPageState.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/hooks/useSessionKeepAlive.ts
- src/api/activity/post-activities.ts
- src/page/event/sections/EventDialogsSection.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
기타