Repository navigation
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
코드 리뷰: 출석체크 관리 기능
🔴 심각한 문제
1. 저장소를 localStorage로만 사용 — 데이터 무결성/공유 불가
client/src/utils/attendanceStorage.js에서 모든 출석 데이터를 브라우저 localStorage에 저장합니다. 이는 다음의 심각한 문제를 야기합니다.
- 관리자별 데이터 파편화: 다른 관리자가 같은 행사에 대해 서로 다른 출석 데이터를 갖게 됩니다.
- 기기 변경/브라우저 초기화 시 데이터 소실
- API 엔드포인트 미구현:
adminAttendanceApi에는getEvents()만 있고, 출석 세션 생성/조회/수정 API가 없습니다.
// 문제: 클라이언트에서만 상태를 관리
export const writeAttendanceSessions = (sessions) => { ... localStorage.setItem ... }권장: 백엔드에 POST /admin/attendance/sessions, PATCH /admin/attendance/sessions/:id/members/:userId 등의 엔드포인트를 두고 서버를 단일 진실 공급원(single source of truth)으로 사용해야 합니다.
2. DEFAULT_MEMBERS 데모 데이터가 프로덕션 코드에 포함
const DEFAULT_MEMBERS = [
{ id: "demo-1", name: "잔망루피", role: "ROLE_MEMBER" },
...
];createAttendanceSession(title, eventDate, members = DEFAULT_MEMBERS)의 기본값으로 하드코딩되어 있어, 호출부에서 members를 전달하지 않을 경우 데모 회원이 실제 세션에 포함됩니다. 또 ManageAttendancePage의 handleCreate는 members.length를 사전 체크하지만, 유틸 함수 자체의 기본값은 제거해야 안전합니다.
3. pageSize=100 고정 + 무한 루프 위험
while (hasNext) {
const response = await adminPermissonApi.getUserRoleList(page, pageSize);
members.push(...(response.content || []));
hasNext = Boolean(response.hasNext);
page += 1;
}- 서버가
hasNext: true를 반환하는 동안 무한 루프가 끝나지 않을 수 있습니다 (page상한 없음, 응답 형식이 예상과 다를 경우 위험). content가 비어 있어도hasNext가 true면 계속 요청을 보냅니다.
권장: 페이지 상한(예:page < 50)과content.length > 0조건을 함께 검사.
🟠 논리 오류 및 버그
4. useMemo 안에서 useState를 if 조건 없이 호출 — 정상이나 규칙 위반은 아님
const records = useMemo(() => selectedSession?.members || [], [selectedSession]);
const [sortBy, setSortBy] = useState("name");useState가 useMemo 뒤에 선언되어 있지만 훅 순서는 고정이므로 문제는 없습니다. 다만 records가 useMemo로 감싸져도 selectedSession이 매번 sessions.find(...)로 새로 만들어지므로 useMemo의 의미가 없습니다. useMemo를 제거하거나 selectedSessionId를 의존성으로 두고 sessions에서 파생시키세요.
5. refreshSessions의 useCallback 의존성이 실제 로직과 불일치
const refreshSessions = useCallback((preferredId, updatedSessions) => {
...
const id = preferredId ?? selectedSessionId; // selectedSessionId 참조
...
}, [selectedSessionId]);selectedSessionId가 바뀔 때마다 refreshSessions가 재생성되어 자식 리렌더/불필요한 파생 비용이 발생합니다. 함수형 업데이터(setSelectedSessionId(prev => ...))를 사용하면 의존성을 제거할 수 있습니다.
6. updateRecord가 record.userId로 비교 — 타입 불일치 위험
members: session.members.map((member) => member.userId !== record.userId ? member : { ...member, ...patch }),fetchAllMembers 시점에는 String() 변환으로 비교했지만, 여기서는 strict equality를 사용합니다. 로컬스토리지에서 읽어온 값은 항상 문자열이 아닐 수 있어(예: 최신 추가 회원의 id가 숫자) 매칭 실패 가능성이 있습니다. String(member.userId) === String(record.userId)로 통일하세요.
7. 종료된 세션에는 회원 재동기화 미적용 — 일관성 문제
...(session.status === "OPEN" ? loadedMembers.filter(...) : [])OPEN 세션만 새 회원을 추가합니다. 이는 의도적일 수 있으나, 회원의 이름/역할 변경은 종료 여부와 무관하게 반영되어야 합니다(위쪽에서 name/role 갱신은 하고 있음). 다만 CLOSED 세션의 role 갱신이 이력 보존 관점에서 옳은지는 정책 결정 필요. 주석으로 명시하세요.
8. 이벤트 선택 콤보박스의 defaultValue="" + onChange — 재선택 문제
<select className={styles.eventSelect} defaultValue="" onChange={...}>- 동일 이벤트를 두 번 선택할 수 없도록 uncontrolled로 되어 있으나, 사용자가 A 선택 → B 선택 → 다시 A 선택 시
defaultValue=""상태에서는 문제 없음. 다만title/eventDate를 초기화할 수단이 없어 "선택 또는 직접 입력"으로 되돌리려면 별도 처리 필요. value={event index}로 인덱스 기반이므로events배열이 바뀌면 오선택 위험이 있습니다.value={item.id}또는 이벤트의 안정적인 키를 사용하고selectedEventId를 상태로 관리하세요.
9. attendanceCounts가 session.members를 매번 순회 — 셀렉터 memoization 부재
세션 카드마다 attendanceCounts(session)를 호출하므로 세션 N개 × 회원 M명 순회가 매 렌더 발생합니다. 세션의 카운트를 캐시하거나 상위에서 한 번 계산해 내려주세요.
10. handleCreate에서 saving 플래그가 사실상 의미 없음
setSaving(true);
...
refreshSessions(created.id, [created, ...current]);
setTitle(""); setEventDate(""); setShowCreate(false);
setSaving(false);동기 처리이므로 saving 상태가 화면에 반영되지 않습니다(다음 렌더에 이미 false). 이후 서버 연동 시에는 await가 들어가야 하며, 현재 로직은 조용히 실패를 삼킬 수 있음. 서버 저장 실패 시 롤백/에러 표시가 없습니다.
🟡 스타일·유지보수
11. adminAttendanceApi에 필요한 API가 부족
export const adminAttendanceApi = {
getEvents: () => apiClient.get("/calendar/events"),
};adminPermissonApi.getUserRoleList에 의존해 회원을 가져오는 것은 계층적 결합을 높입니다. 출석 도메인 전용 API(회원 목록 포함)로 캡슐화하는 편이 낫습니다. 또한 함수명 오타 adminPermissonApi — adminPermissionApi로 리네이밍할 기회입니다(이 PR에서 건드리긴 부담이면 TODO라도).
12. 접근성(ARIA) 이슈
renderSession의<button>내부에<span>계층만 있어 스크린 리더가 "출석 : N명 미출석 : M명"을 매 카드마다 읽습니다. 세션별 요약 문구에aria-label부여를 권장.- 테이블
<th>안에<select>와<button>이 들어가 있어scope="col"없음. 스크린 리더 테이블 탐색 품질 저하. .statusButtons button이 두 개(출석/미출석)인데, 현재 상태 표시가 색상으로만 표현됨(presentActive,absentActive).aria-pressed사용 권장.
13. 인라인 스크립트 스타일 — CSS 모듈 사용
ManageAttendance.module.css는 잘 분리되어 있으나 대부분 한 줄에 몰아넣은 형태라 유지보수성이 매우 낮습니다. 포매팅(개행/그룹화)을 권장합니다.
14. 매직 넘버/스트링 다수
maxLength="255", maxLength="500", "OPEN", "CLOSED", "PRESENT", "ABSENT" 등을 상수로 정의하세요.
// 제안
export const AttendanceStatus = Object.freeze({ PRESENT: "PRESENT", ABSENT: "ABSENT" });
export const SessionStatus = Object.freeze({ OPEN: "OPEN", CLOSED: "CLOSED" });15. dateLabel의 날짜 파싱 취약
new Date(`${date}T00:00:00`)date가 "2024-01-01T00:00:00"처럼 시간이 포함된 경우 Invalid Date가 됩니다. String(date).slice(0, 10)으로 정규화 후 파싱하고, 실패 시 원문 반환하도록 하세요.
16. selectEvent의 날짜 검증 정규식
setEventDate(/^\d{4}-\d{2}-\d{2}/.test(rawDate) ? rawDate.slice(0,10) : "");타임존 표기가 있는 ISO 문자열을 slice(0,10)하면 UTC 기준 날짜가 되므로 KST와 하루 차이가 발생할 수 있습니다. 서버가 KST 로컬 날짜 문자열(YYYY-MM-DD)을 내려주는지 확인 필요.
🔵 보안/프라이버시
17. 회원 전체 명단을 클라이언트에 노출
관리자 페이지라고 해도 getUserRoleList로 모든 회원의 이름/역할을 클라이언트로 내려받아 로컬스토리지에 기록합니다. 브라우저 캐시/확장 프로그램을 통해 유출될 수 있습니다. 서버에서 출석 대상 회원만 전달하거나, 저장 시 개인정보 최소화(이름 마스킹 등)를 고려하세요.
18. note 필드에 maxLength=500만 있고 sanitize 없음
출력은 React가 이스케이프하므로 XSS는 방지되지만, 서버 연동 시 반드시 서버측 검증 필요. CSV 다운로드 등 향후 기능 추가 시 CSV 인젝션 방지도 검토.
✅ 잘한 점
useEffectcleanup으로 언마운트 시active = false처리 — 메모리 누수/setState경고 방지 양호.- 로딩/에러 상태 및
role="alert"aria 사용. - 파일/디렉터리 구조가 기존 admin 페이지 패턴을 따름.
📋 우선순위 요약
| 순위 | 항목 | 조치 |
|---|---|---|
| 1 | localStorage 기반 저장소 | 서버 API로 이전 (또는 명확한 제한사항/스코프 합의) |
| 2 | DEFAULT_MEMBERS 데모 데이터 |
프로덕션 코드에서 제거 |
| 3 | fetchAllMembers 무한 루프 |
페이지 상한/빈 응답 체크 |
| 4 | updateRecord 타입 비교 |
String() 통일 |
| 5 | 이벤트 select 인덱스 사용 | 안정적 id 기반으로 변경 |
| 6 | 접근성 (aria-pressed 등) |
버튼/테이블 속성 보강 |
| 7 | 상태/매직스트링 상수화 | 유지보수성 개선 |
특히 localStorage 의존은 이 기능의 근본적 설계 결함이며, 백엔드 API와 스키마 합의 없이는 프로덕션 배포가 부적절합니다.
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://feat-702-check-in-admin-ui.dev.waps.im, https://feat-702-check-in-admin-ui-waps-dev.leekus1001.workers.dev (commit 533d05b)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
…wap/WAPs into feat/702-check-in-admin-ui # Conflicts: # client/src/pages/adminPages/ManageAttendancePage.js
There was a problem hiding this comment.
코드 리뷰: 출석체크 관리 기능
🔴 심각한 문제 (Critical)
1. 잘못된 API 경로 사용
client/src/api/admin.js
getEvents: () => apiClient.get("/calendar/events"),adminPlanApi는/admin/calendar/event를 사용하는데, 여기서만/calendar/events(복수형, admin prefix 없음)로 호출하고 있습니다.- 서버 라우트와 일치하는지 반드시 확인해야 합니다. 오타일 가능성이 높습니다.
2. 데이터 영속화가 localStorage에만 의존
attendanceStorage.js
- 출석 데이터가 브라우저
localStorage에만 저장되어 있어 다음과 같은 문제가 있습니다:- 관리자별 데이터 불일치: 다른 PC/브라우저에서는 출석 데이터를 볼 수 없음
- 보안/감사 문제: 출석부는 공식 기록인데 서버 저장도 없고 감사 로그도 없음
- 브라우저 데이터 삭제 시 데이터 유실
- 서버 API로 저장하도록 설계를 변경해야 합니다. 최소한 이 임시 구현임을 명시하거나 TODO를 남기세요.
3. 모듈 최상위에서 window 접근
attendanceStorage.js
window.localStorage.getItem(STORAGE_KEY)- 함수 내부이긴 하나, SSR/테스트 환경(Node)에서 호출되면
window is not defined로 크래시합니다.typeof window !== "undefined"가드가 필요합니다.
4. fetchAllMembers 무한 루프 가능성
ManageAttendancePage.js
while (hasNext) {
const response = await adminPermissonApi.getUserRoleList(page, pageSize);
...
hasNext = Boolean(response.hasNext);
page += 1;
}- 서버가
hasNext: true를 계속 반환하거나content가 비어 있는데hasNext가 참이면 무한 루프가 됩니다. 안전장치가 전혀 없습니다. while (hasNext && page < MAX_PAGES)같은 상한을 두세요.
🟠 논리적 오류 (Logic Bugs)
5. useState가 useMemo 뒤에 선언됨
ManageAttendancePage.js
const records = useMemo(() => selectedSession?.members || EMPTY_RECORDS, [selectedSession]);
const visibleSourceMembers = editingClosed && draftMembers ? draftMembers : records;
const [sortBy, setSortBy] = useState("role"); // ← 훅 순서 위반- React Hook 규칙 위반은 아니지만(항상 같은 순서), 코드 가독성이 나쁘고 나중에 조건부 훅이 추가되면 버그가 됩니다. 모든 훅을 컴포넌트 상단에 모아 두세요.
6. refreshSessions에서 updatedSessions 없이 호출되면 저장이 안 됨
const refreshSessions = useCallback((preferredId, updatedSessions) => {
const next = updatedSessions || readAttendanceSessions();
if (updatedSessions && !writeAttendanceSessions(next)) { ... }
...
}, [selectedSessionId]);updatedSessions를 넘기지 않는 호출은 저장을 생략합니다. 지금은 모든 호출부가updatedSessions를 넘기지만, 실수로 생략하면 조용히 유실됩니다. 로딩 목적과 저장 목적을 분리하세요.
7. updateRecord에서 상태 비교 실패
member.userId !== record.userId- 한쪽은 문자열, 다른 쪽은 숫자일 수 있습니다(
createAttendanceSession에서member.id를 그대로 저장). 다른 곳에서는String(...)으로 비교하면서 여기서는 안 합니다. 반드시String(member.userId) !== String(record.userId)로 통일하세요.
8. 회원 목록 로딩 후 선택 세션이 stale
setSelectedSessionId(stored.find(...)?.id ?? null);- 이후
.then()안에서setSessions(refreshed)만 하고selectedSessionId는 그대로 둡니다. 만약 기존 세션의userIds가 갱신되면selectedSession은 여전히 이전 스냅샷을 볼 수 있습니다(다행히find가 매 렌더 재계산되므로 문제는 없으나,refreshSessions로직과 이중 관리로 혼란을 유발합니다).
9. handleCreate에서 saving 해제가 비동기 아님
setSaving(true);
...
setSaving(false);- 동기 함수라 의미가 거의 없고, 만약
createAttendanceSession이 실패하면saving이true로 남습니다. 최소한try/finally로 감싸세요.
10. draftMembers 갱신 시 원본 동기화 누락
setDraftMembers((current) => (current || selectedSession.members).map(...))draftMembers가null인 상태에서updateRecord가 호출되면selectedSession.members가 재사용됩니다.editingClosed를 켤 때 반드시setDraftMembers로 복사(deep copy) 해야 합니다. 현재는 객체 참조를 공유해status/note변경 시 원본도 함께 바뀔 위험이 있습니다.
11. attendanceCounts에서 visibleSourceMembers와 selectedSession.members 혼용
- 요약 카드에서
attendanceCounts({ ...selectedSession, members: visibleSourceMembers })를 사용하는데, 검색 필터가 적용된 상태에서는 필터된 인원만 카운트되어 통계가 틀려집니다. 요약은 전체 멤버로 계산해야 합니다.
12. 이름 검색의 로케일 처리 미흡
String(record.name || "").toLocaleLowerCase("ko").includes(...)toLocaleLowerCase는 한국어에서 대소문자 구분이 없어 무의미합니다(성능만 낭비). 그리고 초성/한글 정규화가 없다는 점은 별개지만, 최소한trim()결과를 변수로 뽑아 매 row마다 재계산하지 마세요.
13. event 타입 확인 미비
adminAttendanceApi.getEvents()
.then((response) => { if (active) setEvents(Array.isArray(response) ? response : []); })apiClient.get이response.data를 반환하는지,response가 곧 배열인지 확인해야 합니다.response?.data일 가능성도 있습니다. API 구현과 맞춰 검증하세요.
🟡 UX / 접근성 / 스타일
14. dateInputValue에서 시간대 문제
date.getFullYear(), date.getMonth() + 1, date.getDate()- 로컬 시간 기준의 YYYY-MM-DD는 의도대로 동작하지만,
toISOString().slice(0,10)을 쓰지 않도록 주의한 것은 좋습니다. 다만 다른 곳에서new Date().toISOString()(UTC)로 저장해 표시할 때 시간대가 하루 어긋나는 경우가 있습니다.endedAt/createdAt표시 로케일과 저장 UTC 기준을 문서화하세요.
15. aria-sort 위치 오류
<span className={...}>
...
<th aria-sort={...}>aria-sort는<th>에만 유효하지만,role="columnheader"가 없는div/span에는 의미가 없습니다. 위 코드는<th>에 붙어 있어 OK.- 다만
aria-sort값"none"은 스펙상 유효하지만, 정렬 상태가 아닐 때는 생략하는 편이 낫습니다.
16. 버튼에 type 미지정
<button className={styles.createSubmit} type="submit" ...> // OK
<button className={styles.mobileAdd} ...> // type 기본값 submit- 폼 안의 버튼이 아닌 것은 문제 없지만, 명시적으로
type="button"을 붙이는 편이 안전합니다. 특히openCreatePanel,closeSession,deleteClosedSession등은 명시하는 것이 좋습니다.
17. key로 index 사용
{events.map((item, index) => <option key={`${item.title}-${index}`} ...>title + index조합은title이 중복될 때 index 의존이 됩니다.item.id가 있다면 그걸 쓰세요.
18. CSS 모듈에서 !important 남용
.closedHeading{...color:#bbb!important}
.emptyRow{text-align:center!important; ...}- 우선순위 규칙을 재설계하면 제거 가능합니다.
19. 매우 긴 한 줄 JSX
renderSession,detailHeader등은 한 줄에 300자를 넘습니다. 유지보수성과 diff 가독성이 매우 나쁩니다. 컴포넌트로 분해하세요 (예:SessionRow,AttendanceRow,ClosedEditActions).
20. 중복 계산
const counts = attendanceCounts(session); // renderSession
...
attendanceCounts({ ...selectedSession, members: visibleSourceMembers }) // 3회 (present/late/absent)attendanceCounts한 번 호출로 결과를 변수에 담아 사용하세요.
🟢 보안 / 기타
21. 에러 메시지에 내부 정보 노출
"브라우저 저장 공간에 출석 데이터를 저장하지 못했습니다."등은 좋지만, 향후 서버 에러 메시지를 그대로 노출하면 정보 유출 위험. 항상 사용자용 메시지로 매핑하세요.
22. adminPermissonApi 오타
- 스펠링이
Permission이어야 하는데Permisson으로 되어 있습니다. 기존 코드의 문제지만, 이 PR에서 새로 import 하니 함께 수정하는 것을 고려하세요.
23. 로딩 상태 처리
loading이true일 때 우측 상세 패널은error만 표시합니다. 회원 로딩 실패 시 폼이 제출되지 않도록disabled={saving || loading}은 있으나, 스피너/스켈레톤이 없어 UX 불명확. 로딩 표시 추가 권장.
24. deadlineTime 검증 없음
if (!title.trim() || !eventDate || !deadlineTime || saving) return;deadlineTime < 현재시간이나eventDate가 과거인 경우 검증이 없습니다. 정책 확인 후 클라이언트+서버 양쪽 검증 필요.
25. deleteClosedSession 후 selectedSessionId 처리
refreshSessions(null, next);refreshSessions가next[0]등으로 자동 선택하는데, 삭제 직후 사용자에게 "어느 세션이 선택되었는지" 불명확합니다. UX 개선 여지.
26. 접근성
<th>안에<button>이 있어 정렬 버튼임을 알 수 있지만,aria-label이 없으면 스크린 리더가 "이름 ▲" 정도로만 읽습니다. "이름 열 정렬" 같은 라벨 권장..detailPanel의aria-label="회원 출석 현황"은 좋으나, 상태 변경 시 라이브 리전(aria-live)이 없어 결과 반영이 스크린 리더에 전달되지 않습니다.
✅ 잘한 점
- 상태(status)와 편집 모드(editingClosed)를 분리해 종료된 세션 수정 로직을 명확히 함
useCallback/useMemo로 재계산 최소화 노력EMPTY_RECORDS상수로 참조 안정성 확보- 로컬 스토리지 예외 처리 및 실패 시 사용자 알림
- 검색/정렬/필터 UX가 상세 패널에 잘 녹아 있음
📋 요약 권고사항
| 우선순위 | 항목 |
|---|---|
| 🔴 | /calendar/events 경로 오타 확인, 서버 저장으로 이관 |
| 🔴 | fetchAllMembers 무한 루프 방지 |
| 🔴 | updateRecord의 userId 타입 비교 통일 |
| 🔴 | draftMembers 초기화 시 deep copy |
| 🟠 | 요약 통계는 필터 이전 전체 멤버 기준으로 계산 |
| 🟠 | handleCreate의 saving try/finally |
| 🟡 | renderSession/detailHeader 컴포넌트 분해 |
| 🟡 | !important 제거, type="button" 명시 |
| 🟢 | API 스펠링(Permisson) 수정 검토, 과거 날짜/시간 검증 추가 |
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.
변경 내용
관리자가 회차별 출석 현황을 확인하고 수정할 수 있도록
/admin/attendance페이지와 사이드바 메뉴를 추가하고, 출석 관리 기능을 개선합니다.기본 기능
localStorage에 저장하고 재진입 시 복원합니다. 회원 조회 후 이름·등급을 갱신하고 진행 중인 회차에는 새 회원을 추가합니다.useMemo로 유지해 React Hook 의존성 경고를 해결했습니다.관리 기능 개선
LATE(지각)를 추가하고, 집계·정렬·버튼에 반영했습니다.createdAt·endedAt으로 기록해 회차 카드와 상세 헤더에 시작·종료 시각을 표시합니다.수정진입 후 출석 상태와 비고를 편집하고수정내용 저장/취소로 반영·되돌리기가 가능합니다.compareAttendanceRoles를 추가했습니다.관련 이슈
Closes #702
검증 결과
cd client && CI=true npm run build: 성공. 출석 페이지의 ESLint 경고 수정 후 CI 모드로 확인했습니다.git diff --check: 통과.리뷰 참고 사항
/admin/attendancesAPI와 QR 발급·스캔은 연결하지 않았습니다. 서버와 연결된 부분은 회원 목록 및 행사 일정 조회입니다.OPEN/CLOSED이며, 서버의SCHEDULED/ONGOING/ENDED상태와 별개입니다. 마감시간은 현재 표시와 입력 검증에만 사용되며 실제 판정 로직은 없습니다.localStorage데이터에 바로 반영되므로 되돌리려면 저장소를 직접 정리해야 합니다.체크리스트