add: 마을/다락방 출석 편집 기능 및 출석 API 보안 강화#73
Conversation
- /admin/soon/attendance 에 [조회] / [입력] 탭 분리 - [입력] 탭: 마을/다락방/순원 3열 리스트 + 모바일 드릴다운 - 상태 필터 5개 (전체/기록안됨/출석/결석/기타) + 이름 검색 - 그룹별 tri-state 체크박스 (마을/다락방/전체 선택) - 하단 sticky bulk action bar (출석/결석/기타) - 공통 사유 다이얼로그 + Undo 스낵바 (10초) - 서버: /admin/soon/update-attendance 엔드포인트 + 권한 검증 (Admin / VillageLeader / Leader 각자 스코프 내만 편집 가능) - 부수: /leader/all-attendance 의 authUserData race condition 수정
- 필터 chip 가로 스크롤 (한 줄, 페이드 힌트) - iPhone safe-area inset 적용 (bulk bar, snackbar, 스크롤 여백) - 예배 select를 OS 네이티브 picker로 (iOS 휠 / Android 다이얼로그) - 예배/필터/검색 헤더 sticky (리스트 스크롤해도 상단 고정) - 상태 chip 말줄임 + hover tooltip (긴 memo 레이아웃 보호) - bulk 버튼 아이콘화 (600px 미만에선 아이콘만, 이상은 텍스트+아이콘) - AttendanceTable hydration mismatch 수정 (global.innerWidth → useMediaQuery)
statusLabel을 파일 하단에서 하위 컴포넌트 영역 위로 이동하여 사용처와의 거리를 줄였습니다. 자명한 주석 1줄만 제거하고 WHY를 설명하는 주석은 유지했습니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
403 응답 본문을 한국어 상세 설명에서 "Forbidden"으로 단축하여 엔드포인트의 권한 체크 로직이 외부로 추정되지 않도록 했습니다. 동일 엔드포인트의 401/400 응답 스타일과도 일관됩니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
isAttend가 AttendStatus enum 값인지 확인하고, memo의 타입과 길이(500자 이내)를 검증하여 데이터 무결성 훼손과 과대 입력을 방어합니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an admin attendance editing experience (village → darak → user) and introduces a new attendance update API with role-based authorization, while reorganizing existing attendance “overview” into a separate tab and improving mobile UX for attendance management.
Changes:
- Server: add
POST /admin/soon/update-attendancewith JWT auth + role/subtree-based authorization and input validation. - Client (admin): split attendance page into
OverviewTaband newEditTab(bulk edit, memo dialog, undo snackbar), with tab switching shell. - Client (tables): make attendance cells editable (popover editor) and wire save handlers into existing tables (including leader view).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/routes/admin/soonRouter.ts | Adds secured attendance update endpoint + subtree authorization helpers. |
| client/src/app/leader/all-attendance/page.tsx | Wires editable attendance table + save handler on leader attendance page. |
| client/src/app/admin/soon/attendance/page.tsx | Converts the admin attendance page into a tabbed shell for overview/edit. |
| client/src/app/admin/soon/attendance/OverviewTab.tsx | Extracts existing overview logic into a dedicated tab and adds cell-save wiring. |
| client/src/app/admin/soon/attendance/EditTab.tsx | New bulk-edit UI with filters, selection, memo dialog, and undo flow. |
| client/src/app/admin/soon/attendance/AttendanceTable.tsx | Adds editable mode + cell save callback support; switches to useMediaQuery. |
| client/src/app/admin/soon/attendance/AttendCell.tsx | Adds editable popover editor for per-cell attendance updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { | ||
| Box, | ||
| Stack, | ||
| Typography, | ||
| Paper, | ||
| Chip, | ||
| useMediaQuery, | ||
| useTheme, | ||
| } from "@mui/material" |
There was a problem hiding this comment.
This component now uses React hooks (useTheme/useMediaQuery) but the file is missing the "use client" directive, which will cause a Next.js App Router build error (hooks in a Server Component / Client Component import boundary violation). Add "use client" at the top of this file.
| // 출석 관리 권한이 있는 역할: Leader (자기 다락방) 또는 VillageLeader (마을 트리 하위 전체) | ||
| if (!requester.role.Leader && !requester.role.VillageLeader) return false | ||
|
|
There was a problem hiding this comment.
PR description says Leader can edit only within their own 다락방, while VillageLeader can edit the whole subtree. Current logic treats Leader and VillageLeader the same (subtree check), so a Leader would also be able to edit descendants if any exist. Consider enforcing Leader => target.community.id === requester.community.id, and VillageLeader => isInSubtree.
| const all = await communityDatabase.find({ relations: { children: true } }) | ||
| const visited = new Set<number>() | ||
|
|
||
| function walk(id: number): boolean { | ||
| if (visited.has(id)) return false | ||
| visited.add(id) | ||
| if (id === targetId) return true | ||
| const node = all.find((c) => c.id === id) | ||
| if (!node) return false |
There was a problem hiding this comment.
isInSubtree() does a full community table read on every attendance update and repeatedly scans the array via all.find() during traversal. This can become a noticeable bottleneck under frequent edits. Consider precomputing an id->node map (or caching the community tree for a short TTL) to make lookups O(1) and avoid repeated DB reads per request.
| Select, | ||
| MenuItem, | ||
| FormControl, | ||
| InputLabel, |
There was a problem hiding this comment.
These MUI imports (Select/MenuItem/FormControl/InputLabel) appear unused in this file (schedule selection uses TextField with SelectProps.native). Removing them will reduce bundle size and avoid lint noise.
| Select, | |
| MenuItem, | |
| FormControl, | |
| InputLabel, |
| const results = await Promise.allSettled( | ||
| ids.map((userId) => | ||
| axios.post("/admin/soon/update-attendance", { | ||
| userId, | ||
| worshipScheduleId: selectedScheduleId, | ||
| isAttend: status, | ||
| memo, | ||
| }), | ||
| ), | ||
| ) | ||
| const successfulIds: string[] = [] | ||
| ids.forEach((id, i) => { | ||
| if (results[i].status === "fulfilled") successfulIds.push(id) | ||
| }) | ||
|
|
||
| setAttendData((prev) => { | ||
| const map = new Map(prev.map((d) => [d.user.id, d])) | ||
| successfulIds.forEach((userId) => { | ||
| const existing = map.get(userId) | ||
| if (existing) { | ||
| map.set(userId, { ...existing, isAttend: status, memo }) | ||
| } else { | ||
| map.set(userId, { | ||
| id: "local-" + userId, | ||
| user: { id: userId } as User, | ||
| worshipSchedule: { | ||
| id: Number(selectedScheduleId), | ||
| } as WorshipSchedule, | ||
| isAttend: status, | ||
| memo, | ||
| } as AttendData) | ||
| } | ||
| }) | ||
| return Array.from(map.values()) | ||
| }) | ||
|
|
||
| setSaving(false) | ||
| const failed = ids.length - successfulIds.length | ||
| if (failed > 0) { | ||
| error(`${failed}건 저장 실패`) | ||
| } | ||
|
|
||
| // Phase 3: Undo 액션 준비 (성공한 것만) | ||
| if (successfulIds.length > 0) { | ||
| const snapshot = new Map< | ||
| string, | ||
| { status: StatusFilter; memo: string } | ||
| >() | ||
| successfulIds.forEach((id) => { | ||
| const prev = previousStates.get(id) | ||
| if (prev) snapshot.set(id, prev) | ||
| }) | ||
| setUndoAction({ | ||
| userIds: successfulIds, | ||
| previousStates: snapshot, | ||
| newStatus: status, | ||
| scheduleId: Number(selectedScheduleId), | ||
| }) | ||
| } | ||
| setCheckedIds(new Set()) |
There was a problem hiding this comment.
Bulk edit currently fires one HTTP request per selected user via Promise.allSettled(ids.map(...axios.post...)). With large selections this can overload the server and hit browser connection limits. Consider adding a server-side bulk update endpoint (accepting userIds[]) or batching/limiting concurrency on the client.
| const results = await Promise.allSettled( | |
| ids.map((userId) => | |
| axios.post("/admin/soon/update-attendance", { | |
| userId, | |
| worshipScheduleId: selectedScheduleId, | |
| isAttend: status, | |
| memo, | |
| }), | |
| ), | |
| ) | |
| const successfulIds: string[] = [] | |
| ids.forEach((id, i) => { | |
| if (results[i].status === "fulfilled") successfulIds.push(id) | |
| }) | |
| setAttendData((prev) => { | |
| const map = new Map(prev.map((d) => [d.user.id, d])) | |
| successfulIds.forEach((userId) => { | |
| const existing = map.get(userId) | |
| if (existing) { | |
| map.set(userId, { ...existing, isAttend: status, memo }) | |
| } else { | |
| map.set(userId, { | |
| id: "local-" + userId, | |
| user: { id: userId } as User, | |
| worshipSchedule: { | |
| id: Number(selectedScheduleId), | |
| } as WorshipSchedule, | |
| isAttend: status, | |
| memo, | |
| } as AttendData) | |
| } | |
| }) | |
| return Array.from(map.values()) | |
| }) | |
| setSaving(false) | |
| const failed = ids.length - successfulIds.length | |
| if (failed > 0) { | |
| error(`${failed}건 저장 실패`) | |
| } | |
| // Phase 3: Undo 액션 준비 (성공한 것만) | |
| if (successfulIds.length > 0) { | |
| const snapshot = new Map< | |
| string, | |
| { status: StatusFilter; memo: string } | |
| >() | |
| successfulIds.forEach((id) => { | |
| const prev = previousStates.get(id) | |
| if (prev) snapshot.set(id, prev) | |
| }) | |
| setUndoAction({ | |
| userIds: successfulIds, | |
| previousStates: snapshot, | |
| newStatus: status, | |
| scheduleId: Number(selectedScheduleId), | |
| }) | |
| } | |
| setCheckedIds(new Set()) | |
| try { | |
| const BULK_SAVE_BATCH_SIZE = 10 | |
| const results: PromiseSettledResult<unknown>[] = [] | |
| for (let i = 0; i < ids.length; i += BULK_SAVE_BATCH_SIZE) { | |
| const batchIds = ids.slice(i, i + BULK_SAVE_BATCH_SIZE) | |
| const batchResults = await Promise.allSettled( | |
| batchIds.map((userId) => | |
| axios.post("/admin/soon/update-attendance", { | |
| userId, | |
| worshipScheduleId: selectedScheduleId, | |
| isAttend: status, | |
| memo, | |
| }), | |
| ), | |
| ) | |
| results.push(...batchResults) | |
| } | |
| const successfulIds: string[] = [] | |
| ids.forEach((id, i) => { | |
| if (results[i].status === "fulfilled") successfulIds.push(id) | |
| }) | |
| setAttendData((prev) => { | |
| const map = new Map(prev.map((d) => [d.user.id, d])) | |
| successfulIds.forEach((userId) => { | |
| const existing = map.get(userId) | |
| if (existing) { | |
| map.set(userId, { ...existing, isAttend: status, memo }) | |
| } else { | |
| map.set(userId, { | |
| id: "local-" + userId, | |
| user: { id: userId } as User, | |
| worshipSchedule: { | |
| id: Number(selectedScheduleId), | |
| } as WorshipSchedule, | |
| isAttend: status, | |
| memo, | |
| } as AttendData) | |
| } | |
| }) | |
| return Array.from(map.values()) | |
| }) | |
| const failed = ids.length - successfulIds.length | |
| if (failed > 0) { | |
| error(`${failed}건 저장 실패`) | |
| } | |
| // Phase 3: Undo 액션 준비 (성공한 것만) | |
| if (successfulIds.length > 0) { | |
| const snapshot = new Map< | |
| string, | |
| { status: StatusFilter; memo: string } | |
| >() | |
| successfulIds.forEach((id) => { | |
| const prev = previousStates.get(id) | |
| if (prev) snapshot.set(id, prev) | |
| }) | |
| setUndoAction({ | |
| userIds: successfulIds, | |
| previousStates: snapshot, | |
| newStatus: status, | |
| scheduleId: Number(selectedScheduleId), | |
| }) | |
| } | |
| setCheckedIds(new Set()) | |
| } finally { | |
| setSaving(false) | |
| } |
| import { useState } from "react" | ||
| import { | ||
| Box, | ||
| Popover, | ||
| Select, | ||
| MenuItem, | ||
| TextField, | ||
| Button, | ||
| Stack, | ||
| Typography, | ||
| } from "@mui/material" |
There was a problem hiding this comment.
This component uses useState and renders a Popover, but the file is missing the "use client" directive. In Next.js App Router this will fail to compile because hooks can’t run in Server Components. Add "use client" at the top of this file.
| // 대상 유저의 community가 내 community의 하위(또는 동일)인지 | ||
| return await isInSubtree(requester.community?.id, target.community.id) | ||
| } |
There was a problem hiding this comment.
Authorization check currently delegates both Leader and VillageLeader to isInSubtree(). If you need the stricter Leader-only-own-community rule (per PR description), this call site needs branching by role before applying the subtree check.
| if (memo !== undefined && memo !== null) { | ||
| if (typeof memo !== "string") { | ||
| res.status(400).send({ error: "Invalid memo type" }) | ||
| return | ||
| } | ||
| if (memo.length > 500) { | ||
| res.status(400).send({ error: "Memo too long" }) | ||
| return |
There was a problem hiding this comment.
memo is treated as optional in validation, but later memo || "" is always written to the DB. If the caller omits memo, this will clear any existing memo unexpectedly. Consider either requiring memo explicitly, or only updating memo when it is provided (leave existing.memo unchanged when memo is undefined/null).
| useEffect(() => { | ||
| // authUserData가 비동기로 로드되므로 준비될 때까지 판정 보류 | ||
| if (!authUserData) return | ||
| fetchCommunities() | ||
| if (!authUserData?.role.VillageLeader) { | ||
| if (!authUserData.role.VillageLeader) { | ||
| error("접근 권한이 없습니다.") | ||
| push("/leader") | ||
| } |
There was a problem hiding this comment.
In this effect, fetchCommunities() runs before the role check/redirect. For non-VillageLeader users this triggers an unnecessary /admin/community request (and may flash errors). Consider checking authUserData.role first, redirecting early, and only fetching when authorized.
페이지 접근 가드가 VillageLeader만 허용하므로 editable에 포함된 role.Leader는 실행에 도달하지 않는 죽은 조건입니다. 의도와 실제를 일치시키기 위해 제거했습니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
출석 편집은 /admin/soon/attendance 에서만 가능하도록 하기 위해 leader 페이지에 추가했던 editable 변수, handleSaveCell 핸들러, AttendanceTable의 편집 props를 걷어냈습니다. authUserData 로딩 타이밍 수정은 원래 있던 버그 수정이므로 유지합니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| async function isInSubtree( | ||
| ancestorId: number | undefined, | ||
| targetId: number | undefined, | ||
| ): Promise<boolean> { | ||
| if (!ancestorId || !targetId) return false | ||
| if (ancestorId === targetId) return true | ||
|
|
||
| const all = await communityDatabase.find({ relations: { children: true } }) | ||
| const visited = new Set<number>() | ||
|
|
||
| function walk(id: number): boolean { | ||
| if (visited.has(id)) return false | ||
| visited.add(id) | ||
| if (id === targetId) return true | ||
| const node = all.find((c) => c.id === id) | ||
| if (!node) return false | ||
| return node.children.some((child) => walk(child.id)) | ||
| } | ||
| return walk(ancestorId) | ||
| } | ||
|
|
||
| async function canEditUserAttendance( | ||
| requester: jwtPayload, | ||
| targetUserId: string, | ||
| ): Promise<boolean> { | ||
| // Admin은 어느 유저든 편집 가능 | ||
| if (requester.role.Admin) return true | ||
|
|
||
| // 출석 관리 권한이 있는 역할: Leader (자기 다락방) 또는 VillageLeader (마을 트리 하위 전체) | ||
| if (!requester.role.Leader && !requester.role.VillageLeader) return false | ||
|
|
||
| const target = await userDatabase.findOne({ | ||
| where: { id: targetUserId }, | ||
| relations: { community: true }, | ||
| }) | ||
| if (!target?.community) return false | ||
|
|
||
| // 대상 유저의 community가 내 community의 하위(또는 동일)인지 | ||
| return await isInSubtree(requester.community?.id, target.community.id) | ||
| } |
There was a problem hiding this comment.
여긴 라우터라서 로직이 필요하다면 로직은 model 쪽으로 옮기면 좋을거 같아요.
| if (!(Object.values(AttendStatus) as string[]).includes(isAttend)) { | ||
| res.status(400).send({ error: "Invalid isAttend value" }) | ||
| return | ||
| } | ||
|
|
||
| if (memo !== undefined && memo !== null) { | ||
| if (typeof memo !== "string") { | ||
| res.status(400).send({ error: "Invalid memo type" }) | ||
| return | ||
| } | ||
| if (memo.length > 500) { | ||
| res.status(400).send({ error: "Memo too long" }) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| const allowed = await canEditUserAttendance(jwt, userId) | ||
| if (!allowed) { | ||
| res.status(403).send({ error: "Forbidden" }) | ||
| return | ||
| } |
There was a problem hiding this comment.
에러를 프론트에 그대로 보여주기에, 사용자가 알기 쉬운 문구로 내려주거나, 프론트에서 에러 코드에 대한 처리를 따로 해주거나 하면 좋을거 같아요
- canEditUserAttendance, isInSubtree를 server/src/model/attendance.ts로 이동 (라우터 슬림화) - Leader는 자기 community만, VillageLeader는 subtree 전체로 권한 범위를 명시적으로 분리 (기존엔 둘 다 subtree 체크) - isInSubtree에 id→node Map을 추가해 탐색을 O(1)로 개선 - memo 미전송 시 기존 memo가 빈 문자열로 덮어써지던 문제를 조건부 업데이트로 수정 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- AttendCell(useState/Popover), AttendanceTable(useTheme/useMediaQuery)에 "use client" 지시어를 명시해 Next.js App Router 경계 규약을 충족 - /leader/all-attendance 에서 fetchCommunities 호출을 VillageLeader 체크 이후로 이동해 비권한 사용자가 불필요한 /admin/community 요청을 내지 않도록 함 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- client/src/util/attendanceError.ts 추가: 서버 에러 코드를 한국어 메시지로 매핑하는 유틸 - OverviewTab, EditTab의 에러 처리에서 이 유틸을 사용해 "Forbidden" 같은 서버 원문 대신 사용자 친화적 문구를 노출 - EditTab의 runBulkSave를 10개씩 배치로 Promise.allSettled 처리해 대량 선택 시 서버 과부하 및 브라우저 연결 한계를 완화 - 실패 발생 시 실패 건수뿐 아니라 첫 실패의 원인을 함께 표시 - EditTab에서 미사용 MUI 컴포넌트(Select/MenuItem/FormControl/InputLabel) import 제거 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| }: AttendanceTableProps) { | ||
| const isMobile = global.innerWidth < 600 | ||
| const theme = useTheme() | ||
| // SSR-safe: 서버 렌더 시엔 false, 마운트 후 실제 window 크기로 재계산 |
| onSave={ | ||
| onSaveCell | ||
| ? (status, memo) => | ||
| onSaveCell(user.id, worshipSchedule.id, status, memo) | ||
| : undefined | ||
| } |
출석 편집 요청마다 전체 community 테이블을 다시 로드하던 것을 모듈 레벨 Map에 TTL 30초로 캐시하여 반복 DB 조회를 제거합니다. 트리 구조 변경은 드물어 30초 지연이 허용됩니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- AttendanceTable의 "SSR-safe" 주석을 "SSG"로 교정 (output:"export" 환경) - AttendanceTable/AttendCell에 추가됐던 "use client"는 부모에서 상속되는 프로젝트 관례와 어긋나므로 제거 - worshipScheduleMap 루프에서 onSave prop 삼항식을 handleSave 변수로 추출해 가독성 개선 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
클라에서 10개씩 병렬 HTTP 요청을 나눠 보내던 runBulkSave와 handleUndo를 신설한 POST /update-attendance-bulk 한 번의 요청으로 통합합니다. 서버는 최대 100건까지 받아 각 아이템별로 검증·권한 체크·저장을 수행하고, 부분 실패 시 HTTP 207 Multi-Status와 per-item 결과를 반환합니다. 클라는 결과의 status 기반으로 성공 userId를 계산하고 첫 실패 사유를 토스트에 표시합니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EditTab의 runBulkSave/handleUndo 두 군데에 인라인으로 정의돼 있던 BulkAttendanceResponse 타입을 util/attendanceError.ts로 옮기고, 인라인 status 매핑(forbidden/invalid/error → 한국어)도 toBulkResultMessage 함수로 추출했습니다. 새 status 추가 시 매핑 테이블 한 곳만 수정하면 됩니다. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@iubns 리뷰 코멘트 4건 모두 반영했고, Copilot 9건 중 7건 반영 + 2건은 SSG/부모 상속 관례상 false positive로 정리했습니다. 추가로 bulk 처리는 클라 배치 대신 서버 bulk endpoint(207 Multi-Status)로 승격, |
Summary
/update-attendance단건 +/update-attendance-bulk일괄)하고 인가·입력 검증·응답 최소화·207 Multi-Status를 적용해 보안과 일관성을 강화했습니다주요 변경
🆕 기능
EditTab(신규, ~1160줄): 마을 → 다락방 → 순원 3컬럼 뷰에서 개별/일괄 출석 편집, 사유 다이얼로그, Undo 스낵바 지원OverviewTab(신규, ~251줄): 기존page.tsx의 현황 보기 로직을 독립 탭으로 분리page.tsx: 두 탭을 스위칭하는 얇은 셸로 축소 (204줄 → 32줄)🔒 보안 / 권한
server/src/model/attendance.ts로 분리isAttend/memo입력 검증 (단건·일괄 모두 적용)⚡ 성능
isInSubtree: id→nodeMap으로 O(1) 조회 + 30초 모듈 캐시로 중복 DB 조회 제거ok/forbidden/invalid/error)로 부분 실패 명확화📱 UX
AttendCell모바일 렌더링 개선leader/all-attendance페이지 정보 밀도 조정 + 권한 체크 후 fetch (불필요 트래픽 제거)util/attendanceError.ts)🔄 Review 반영 (Round 2)
canEditUserAttendance에서 role별 분기isInSubtree성능Select/MenuItem/FormControl/InputLabel제거if (typeof memo === "string")조건부 업데이트fetchCommunities순서server/src/model/attendance.ts신설util/attendanceError.ts매핑 유틸 (단건·bulk 통합)AttendanceTable주석SSR-safe → SSG로 교정handleSave변수 추출"use client"누락 경고 (Copilot)Test plan
cd client && npm run build)cd server && npx tsc --noEmit)cd client && npx tsc --noEmit)비고
후속 검토 (별도 PR 권장)
pLimit등으로 DB connection pool 보호)🤖 Generated with Claude Code