[Fix] WTH-348 : 출석 svg 아이콘 수정 및 QR 오류 수정#78
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough아이콘 두 개(TaskFinishedIcon, RushIcon)를 내보내고, 출석 관련 훅(SSE/QR)과 UI(모달·카드)를 변경해 QR 경로에 clubId를 포함시키고 SSE 상태를 상향 노출하여 모달/배너 렌더링과 오류 처리를 제어하도록 수정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Attendance UI
participant Hook as useAttendanceSSE
participant Server as SSE Server
participant QRGen as useAttendanceQR
UI->>Hook: subscribe SSE
Hook->>Server: open SSE connection
Server-->>Hook: event: qr-open (with expiredAt, url)
Hook-->>UI: status = qr-open, expiredAt
UI->>QRGen: build checkInUrl (includes clubId)
QRGen-->>UI: qr image / data
User->>UI: open code modal (openCodeModal)
UI->>Hook: observe status changes
Server-->>Hook: event: qr-close or qr-none
Hook-->>UI: status = qr-close/qr-none (expiredAt cleared)
UI->>User: show toast error and close modal (if qr-none/expired)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
개요아이콘 두 개(TaskFinishedIcon, RushIcon)를 추가로 내보내고, 출석 관련 컴포넌트들의 UI를 업데이트하여 완료 상태의 일러스트레이션을 변경하고 배너를 동적으로 렌더링하도록 수정했습니다. 또한 QR 코드 URL에 clubId를 포함시켰습니다. 변경사항
예상 코드 리뷰 시간🎯 3 (Moderate) | ⏱️ ~20 minutes 관련 가능성 있는 PR
제안 라벨
제안 리뷰어
시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 8 minutes and 35 seconds.Comment |
PR 테스트 결과✅ Jest: 통과 🎉 모든 테스트를 통과했습니다! |
PR 검증 결과✅ TypeScript: 통과 🎉 모든 검증을 통과했습니다! |
|
구현한 기능 Preview: https://weeth-j2ciohy54-weethsite-4975s-projects.vercel.app |
🤖 Claude 테스트 제안
변경된 컴포넌트에 대해 Claude가 생성한 테스트 코드입니다. 검토 후 적합한 부분만 사용하세요.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/attendance/useAttendanceQR.ts (1)
14-36:⚠️ Potential issue | 🟠 Major
clubIdnull 케이스에서 잘못된 QR URL이 생성될 수 있습니다.현재
clubId가null이면.../null/attendance가 QR에 들어갈 수 있습니다. URL 생성도 문자열 보간 대신URLAPI로 구성해 두 쿼리값 인코딩까지 같이 처리하는 편이 안전합니다.수정 제안
useEffect(() => { - if (!qrData || !qrRef.current) return; + if (!qrData || !qrRef.current || !clubId) return; - const checkInUrl = `${window.location.origin}/${clubId}/attendance?sessionId=${qrData.sessionId}&code=${qrData.code}`; + const checkInUrl = new URL(`/${clubId}/attendance`, window.location.origin); + checkInUrl.searchParams.set('sessionId', String(qrData.sessionId)); + checkInUrl.searchParams.set('code', qrData.code); + const checkInUrlString = checkInUrl.toString(); if (!qrCodeRef.current) { const dotColor = getComputedStyle(document.documentElement) .getPropertyValue('--icon-strong') .trim(); qrCodeRef.current = new QRCodeStyling({ width: 256, height: 256, - data: checkInUrl, + data: checkInUrlString, qrOptions: { errorCorrectionLevel: 'L' }, type: 'svg', dotsOptions: { type: 'dots', color: dotColor }, cornersSquareOptions: { type: 'extra-rounded', color: dotColor }, cornersDotOptions: { type: 'extra-rounded', color: dotColor }, backgroundOptions: { color: 'transparent' }, }); qrCodeRef.current.append(qrRef.current); } else { - qrCodeRef.current.update({ data: checkInUrl }); + qrCodeRef.current.update({ data: checkInUrlString }); } - }, [qrData]); + }, [qrData, clubId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/attendance/useAttendanceQR.ts` around lines 14 - 36, The QR URL construction can produce "/null/attendance" when clubId is null and doesn't encode query params; update the checkInUrl logic (used when creating/updating qrCodeRef in useAttendanceQR) to: early-return or skip QR generation if clubId is missing, and otherwise build the URL with the URL API—set the pathname using encodeURIComponent(clubId) (or safely compose pathname as `/${encodeURIComponent(clubId)}/attendance`) and add sessionId and code via url.searchParams.set to ensure proper encoding; then pass url.toString() to QRCodeStyling.update/constructor instead of the current string-interpolated checkInUrl.
🤖 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/attendance/AttendanceTodayCard.tsx`:
- Around line 32-35: The AttendanceBanner component uses a hardcoded gap class
(`gap-[10px]`) which violates spacing token rules; update the JSX in function
AttendanceBanner to replace `gap-[10px]` with a design token class (e.g.,
`gap-200` or another token in the `gap-100`~`gap-400` range) so spacing uses the
token system; keep existing tokenized padding (`p-300`) as-is and ensure no
other hardcoded spacing remains in AttendanceBanner.
---
Outside diff comments:
In `@src/hooks/attendance/useAttendanceQR.ts`:
- Around line 14-36: The QR URL construction can produce "/null/attendance" when
clubId is null and doesn't encode query params; update the checkInUrl logic
(used when creating/updating qrCodeRef in useAttendanceQR) to: early-return or
skip QR generation if clubId is missing, and otherwise build the URL with the
URL API—set the pathname using encodeURIComponent(clubId) (or safely compose
pathname as `/${encodeURIComponent(clubId)}/attendance`) and add sessionId and
code via url.searchParams.set to ensure proper encoding; then pass
url.toString() to QRCodeStyling.update/constructor instead of the current
string-interpolated checkInUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e08eaf38-fdb6-4606-be4d-e6b20a5792a1
⛔ Files ignored due to path filters (3)
src/assets/icons/complete.svgis excluded by!**/*.svgsrc/assets/icons/rush.svgis excluded by!**/*.svgsrc/assets/icons/task_finished.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
src/assets/icons/index.tssrc/components/attendance/AttendanceCompleteModal.tsxsrc/components/attendance/AttendanceTodayCard.tsxsrc/hooks/attendance/useAttendanceQR.ts
| function AttendanceBanner({ icon, alt, title, description }: AttendanceBannerProps) { | ||
| return ( | ||
| <div className="bg-background flex items-start gap-[10px] rounded-md p-300"> | ||
| <Image src={CompleteIcon} alt="출석 완료" width={40} height={40} /> | ||
| <Image src={icon} alt={alt} width={40} height={40} /> |
There was a problem hiding this comment.
하드코딩 간격(gap-[10px])을 토큰 간격으로 교체해 주세요.
Line 34는 spacing token 규칙을 벗어납니다. 디자인 시스템 간격 토큰(gap-100~400)으로 맞추는 게 안전합니다.
제안 diff
- <div className="bg-background flex items-start gap-[10px] rounded-md p-300">
+ <div className="bg-background flex items-start gap-100 rounded-md p-300">As per coding guidelines: Always use design token classes first; no hardcoded values. Use spacing token classes: p-100~500, gap-100~400.
📝 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.
| function AttendanceBanner({ icon, alt, title, description }: AttendanceBannerProps) { | |
| return ( | |
| <div className="bg-background flex items-start gap-[10px] rounded-md p-300"> | |
| <Image src={CompleteIcon} alt="출석 완료" width={40} height={40} /> | |
| <Image src={icon} alt={alt} width={40} height={40} /> | |
| function AttendanceBanner({ icon, alt, title, description }: AttendanceBannerProps) { | |
| return ( | |
| <div className="bg-background flex items-start gap-100 rounded-md p-300"> | |
| <Image src={icon} alt={alt} width={40} height={40} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/attendance/AttendanceTodayCard.tsx` around lines 32 - 35, The
AttendanceBanner component uses a hardcoded gap class (`gap-[10px]`) which
violates spacing token rules; update the JSX in function AttendanceBanner to
replace `gap-[10px]` with a design token class (e.g., `gap-200` or another token
in the `gap-100`~`gap-400` range) so spacing uses the token system; keep
existing tokenized padding (`p-300`) as-is and ensure no other hardcoded spacing
remains in AttendanceBanner.
There was a problem hiding this comment.
요거 CI warning 말대로 clubId를 의존성배열에 넣어줘도 될 것 같아욤
🤖 Claude 테스트 제안
변경된 컴포넌트에 대해 Claude가 생성한 테스트 코드입니다. 검토 후 적합한 부분만 사용하세요.
|
PR 테스트 결과✅ Jest: 통과 🎉 모든 테스트를 통과했습니다! |
PR 검증 결과✅ TypeScript: 통과 🎉 모든 검증을 통과했습니다! |
|
구현한 기능 Preview: https://weeth-5nk6w0hw2-weethsite-4975s-projects.vercel.app |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/attendance/useAttendanceSSE.ts (1)
128-137:⚠️ Potential issue | 🔴 Critical토큰 갱신 후 재연결 전에 구독자 존재 여부를 확인하세요.
현재 코드는
refreshRes.ok이후 즉시connect(clubId, conn)을 호출하는데, 토큰 갱신 중 마지막 구독자가 해제되었다면subscriberCount === 0인 상태에서 불필요한 SSE 연결이 생깁니다. 추가로refresh요청이 abort signal을 받지 않아 구독자 해제 시 취소될 수 없습니다.다음을 수정하세요:
- 재연결 직전에
conn.subscriberCount > 0인지 확인refresh요청에controller.signal전달connect()호출에void접두사 추가🔧 제안된 수정
if (refreshRes.ok) { + if (conn.subscriberCount === 0 || connections.get(clubId) !== conn) { + return; + } // 리프레시 성공 → 즉시 재연결 (retryCount 유지) - connect(clubId, conn); + void connect(clubId, conn); return; }const refreshRes = await fetch('/api/proxy/auth/refresh', { method: 'POST', + signal: controller.signal, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/attendance/useAttendanceSSE.ts` around lines 128 - 137, When handling a 401 and after a successful refresh response, ensure you only reconnect if there are active subscribers by checking conn.subscriberCount > 0 before calling connect; also make the refresh request cancellable by passing the existing AbortController.signal (controller.signal) into fetch('/api/proxy/auth/refresh', ...) and, when calling connect(clubId, conn) on success, prefix the call with void to explicitly ignore the returned promise (i.e., use void connect(clubId, conn)); update the logic around the refreshRes.ok branch accordingly.
🤖 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/attendance/AttendanceCodeModal.tsx`:
- Around line 52-67: The effect that reacts to modal open/status currently only
handles 'qr-none' and calls onOpenChange(false) but doesn't clear the input;
update that effect (the useEffect that reads open, status, hasShownRef) to also
handle the 'qr-close' status by setting hasShownRef.current = true (to avoid
repeated toasts), calling toastError with the expiration message, calling
onOpenChange(false) to close the modal, and clearing the attendance input state
(call the component's setCode('') or the existing code reset handler) so the OTP
is emptied for the next open; ensure hasShownRef is still reset in the other
useEffect when open becomes false.
In `@src/hooks/attendance/useAttendanceSSE.ts`:
- Around line 207-229: subscribeFn is recreated each render causing both
useSyncExternalStore calls to re-subscribe and flip subscriberCount; stabilize
its identity by memoizing it (e.g., replace subscribeFn with a single memoized
callback used by both useSyncExternalStore calls). Implement a stableSubscribe
via useCallback that captures clubId and calls getOrCreateConnection(clubId) and
subscribe(clubId, conn, listener), and use that stableSubscribe for the
expiredAt and status subscriptions so the same function instance is passed to
both useSyncExternalStore calls.
---
Outside diff comments:
In `@src/hooks/attendance/useAttendanceSSE.ts`:
- Around line 128-137: When handling a 401 and after a successful refresh
response, ensure you only reconnect if there are active subscribers by checking
conn.subscriberCount > 0 before calling connect; also make the refresh request
cancellable by passing the existing AbortController.signal (controller.signal)
into fetch('/api/proxy/auth/refresh', ...) and, when calling connect(clubId,
conn) on success, prefix the call with void to explicitly ignore the returned
promise (i.e., use void connect(clubId, conn)); update the logic around the
refreshRes.ok branch accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb4006be-76d6-44ef-8da3-612d11f9c086
📒 Files selected for processing (5)
src/components/attendance/AttendanceCodeModal.tsxsrc/constants/attendance/error.tssrc/hooks/attendance/useAttendanceQR.tssrc/hooks/attendance/useAttendanceSSE.tssrc/hooks/attendance/useCheckIn.ts
✅ Files skipped from review due to trivial changes (1)
- src/constants/attendance/error.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/attendance/useAttendanceQR.ts
| useEffect(() => { | ||
| if (!open) return; | ||
| if (status === null) return; | ||
|
|
||
| if (status === 'qr-none' && !hasShownRef.current) { | ||
| hasShownRef.current = true; | ||
| toastError('현재 출석이 진행 중이 아닙니다.'); | ||
| onOpenChange(false); | ||
| } | ||
| }, [open, status, onOpenChange]); | ||
|
|
||
| useEffect(() => { | ||
| if (!open) { | ||
| hasShownRef.current = false; | ||
| } | ||
| }, [open]); |
There was a problem hiding this comment.
자동 종료 경로에서 qr-close와 입력 초기화를 함께 처리하세요.
현재 이 effect는 qr-none만 닫고 onOpenChange(false)를 직접 호출해서 code를 비우지 못합니다. QR이 만료되는 흐름에서는 모달이 열린 채로 남거나, 다음 오픈 때 이전 OTP가 남을 수 있습니다.
🔧 Suggested fix
- if (status === 'qr-none' && !hasShownRef.current) {
+ if ((status === 'qr-none' || status === 'qr-close') && !hasShownRef.current) {
hasShownRef.current = true;
toastError('현재 출석이 진행 중이 아닙니다.');
- onOpenChange(false);
+ setCode('');
+ onOpenChange(false);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/attendance/AttendanceCodeModal.tsx` around lines 52 - 67, The
effect that reacts to modal open/status currently only handles 'qr-none' and
calls onOpenChange(false) but doesn't clear the input; update that effect (the
useEffect that reads open, status, hasShownRef) to also handle the 'qr-close'
status by setting hasShownRef.current = true (to avoid repeated toasts), calling
toastError with the expiration message, calling onOpenChange(false) to close the
modal, and clearing the attendance input state (call the component's setCode('')
or the existing code reset handler) so the OTP is emptied for the next open;
ensure hasShownRef is still reset in the other useEffect when open becomes
false.
🤖 Claude 테스트 제안
변경된 컴포넌트에 대해 Claude가 생성한 테스트 코드입니다. 검토 후 적합한 부분만 사용하세요.
|
PR 테스트 결과✅ Jest: 통과 🎉 모든 테스트를 통과했습니다! |
PR 검증 결과✅ TypeScript: 통과 🎉 모든 검증을 통과했습니다! |
|
구현한 기능 Preview: https://weeth-d1tf8oxul-weethsite-4975s-projects.vercel.app |
JIN921
left a comment
JIN921
left a comment
There was a problem hiding this comment.
아이콘 들어가니까 또 색다르네요 기엽다 다른 수정할 부분은 없어 보입니다 수고하셧어용!!!
4 participants
✅ PR 유형
어떤 변경 사항이 있었나요?
📌 관련 이슈번호
✅ Key Changes
useAttendanceQR에서 생성하는 체크인 URL에clubId경로 세그먼트가누락되어 있던 문제 수정
CompleteIcon→TaskFinishedIcon으로 교체하고, 제목 타이포그래피를typo-sub2→typo-sub1로 수정AttendanceCompleteBanner를 범용AttendanceBanner컴포넌트로 변경하여출석 완료/출석 진행 상태를 각각 다른 아이콘·문구로 표시
📸 스크린샷 or 실행영상
2026-04-29.113732.mp4
SSE 구독 이제 잘 됩니당!!!
🎸 기타 사항 or 추가 코멘트
어제 출석 페이지에서 QA 진행했던 내용들 일괄적으로 수정 완료했습니다!
SSE 관련 수정은 수요일 오전 안에 반영해둘게요!!
Summary by CodeRabbit
출시 노트
새 기능
개선 사항