20260310 #292 출석 세션 관리자 추가 삭제 api 구현#293
Merged
discipline24 merged 5 commits intomainfrom Mar 10, 2026
Hidden character warning
The head ref may contain hidden characters: "20260310-#292-\ucd9c\uc11d-\uc138\uc158-\uad00\ub9ac\uc790-\ucd94\uac00-\uc0ad\uc81c-API-\uad6c\ud604"
Merged
Conversation
docs 수정 권한 검증 확인 선배 관련 리팩토링 (삭제) grade 수정 API 추가 사용하지않는 메서드 삭제
|
Warning Rate limit exceeded
⌛ 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 (1)
Walkthrough관리자 등급 API가 promoteToSenior → updateUserGrade(UUID, Grade)로 일반화되고 권한이 PRESIDENT 또는 SYSTEM_ADMIN으로 확대되었습니다. 출석 세션/라운드 권한이 OWNER 중심으로 재조정되고 세션 관리자 추가/제거 엔드포인트 및 세션 단위 bulk 삭제 리포지토리 메서드가 추가되었습니다. 일부 enum 상수(SENIOR, GRADUATED)가 주석 처리되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionUserController
participant Auth as Security
participant SessionUserService
participant SessionUserRepository
participant DB
Client->>SessionUserController: POST /{sessionId}/admins/{userId} (토큰)
SessionUserController->>Auth: 토큰 검증
Auth-->>SessionUserController: principal (actorUserId)
SessionUserController->>SessionUserService: addAdmin(sessionId, targetUserId, actorUserId)
SessionUserService->>SessionUserService: ensureOwner(actorUserId, sessionId)
SessionUserService->>SessionUserRepository: update role -> MANAGER for targetUserId
SessionUserRepository->>DB: execute update
DB-->>SessionUserRepository: OK
SessionUserRepository-->>SessionUserService: success
SessionUserService-->>SessionUserController: success
SessionUserController-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50분 Possibly related PRs
시
🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java (1)
97-120:⚠️ Potential issue | 🟡 MinorAPI 문서의 권한 및 에러 코드 설명이 실제 구현과 불일치합니다.
Line 104에서 권한을 "세션 OWNER"로 올바르게 변경했지만:
- Line 114: "요청 유저가 해당 세션의 관리 권한(MANAGER/OWNER)을 가졌는지 검증"은 이제 OWNER만 가능하므로 수정이 필요합니다.
- Line 118: 에러 코드
NOT_SESSION_ADMIN은NOT_SESSION_OWNER로 변경해야 합니다 (서비스에서ensureOwner호출 시NOT_SESSION_OWNER발생).📝 문서 수정 제안
## 동작 설명 - 특정 출석 회차(Round)를 삭제 - - 요청 유저가 해당 세션의 관리 권한(MANAGER/OWNER)을 가졌는지 검증 + - 요청 유저가 해당 세션의 소유자(OWNER) 권한을 가졌는지 검증 ## 에러 코드 - **`ROUND_NOT_FOUND`**: 해당 출석 라운드가 존재하지 않습니다. - - **`NOT_SESSION_ADMIN`**: 세션 관리자 권한이 없습니다. + - **`NOT_SESSION_OWNER`**: 세션 소유자 권한이 없습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java` around lines 97 - 120, Update the API doc block in AttendanceRoundController for the DeleteMapping("/rounds/{roundId}") to match the implementation: change the permission description that currently says "요청 유저가 해당 세션의 관리 권한(MANAGER/OWNER)을 가졌는지 검증" to indicate only the session OWNER is allowed (e.g., "요청 유저가 세션 OWNER인지 검증"), and replace the listed error code `NOT_SESSION_ADMIN` with `NOT_SESSION_OWNER` so the documented error matches the service's ensureOwner behavior.backend/src/main/java/org/sejongisc/backend/user/entity/Grade.java (1)
3-18:⚠️ Potential issue | 🟠 Major데이터 마이그레이션 필요: SENIOR 값 제거 시 런타임 오류 발생 가능
SENIORenum 상수를 주석 처리하면, 데이터베이스에grade='SENIOR'로 저장된 기존 사용자 레코드를 로드할 때 JPA가IllegalArgumentException을 발생시킵니다.AdminUserService.java의 주석(line 113)에서 사용자를 SENIOR 등급으로 변경하던 기능이 있었던 것으로 보아, 기존 데이터베이스에 SENIOR 값이 존재할 가능성이 있습니다.코드 주석의 "선배/OB는 정회원으로 대체"에 따라, 배포 전에 기존
SENIOR데이터를REGULAR_MEMBER로 마이그레이션하는 SQL 스크립트를 작성하고 실행해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/user/entity/Grade.java` around lines 3 - 18, Existing DB rows with grade='SENIOR' will cause JPA to throw IllegalArgumentException because the SENIOR enum constant was removed from Grade; before deploying, run a migration that updates all legacy values to REGULAR_MEMBER (for example: UPDATE user SET grade='REGULAR_MEMBER' WHERE grade='SENIOR') and verify AdminUserService logic that previously set SENIOR is updated to use REGULAR_MEMBER; also consider adding a temporary tolerant import in Grade.fromString to map "선배"/"OB" strings to REGULAR_MEMBER during migration rollout if needed.
🧹 Nitpick comments (8)
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java (1)
102-103: 세션 조회 결과가 사용되지 않습니다.
attendanceSessionRepository.findById(sessionId)결과가 변수에 할당되지 않고 존재 여부만 확인합니다. 이 패턴이 의도된 것이라면, 가독성을 위해existsById를 사용하거나 명시적으로 무시하는 것이 좋습니다.♻️ 개선 제안
- attendanceSessionRepository.findById(sessionId) - .orElseThrow(() -> new CustomException(ErrorCode.SESSION_NOT_FOUND)); + if (!attendanceSessionRepository.existsById(sessionId)) { + throw new CustomException(ErrorCode.SESSION_NOT_FOUND); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java` around lines 102 - 103, The call attendanceSessionRepository.findById(sessionId) in SessionUserService currently only checks existence and discards the result; change it to capture the Optional result and assign the resolved entity to a variable (e.g., attendanceSession = attendanceSessionRepository.findById(sessionId).orElseThrow(() -> new CustomException(ErrorCode.SESSION_NOT_FOUND))) so the retrieved session can be used, or if the intent is only to check existence replace the call with attendanceSessionRepository.existsById(sessionId) or explicitly ignore the value with a clear comment; update any subsequent code to use the new attendanceSession variable if needed.backend/src/main/java/org/sejongisc/backend/user/entity/User.java (1)
87-94: 주석이 새로운 구현과 일치하지 않습니다.Line 89의 주석 "직위에 '팀장', '대표', '부대표' 등의 키워드가 있으면 운영진 권한 부여 후보"는 이전
positionName기반 구현에 대한 설명입니다. 새 구현은role기반이므로 주석을 업데이트해야 합니다.구현 자체는 명확하고 적절합니다:
Roleenum을 기반으로 한 권한 검증이 문자열 매칭보다 안전합니다.📝 주석 수정 제안
public boolean isManagerPosition() { if (this.role == Role.TEAM_MEMBER || this.role == Role.PENDING_MEMBER ) return false; - // 직위에 '팀장', '대표', '부대표' 등의 키워드가 있으면 운영진 권한 부여 후보 + // SYSTEM_ADMIN, PRESIDENT, VICE_PRESIDENT, TEAM_LEADER 역할은 운영진 권한 보유 return this.role == Role.SYSTEM_ADMIN || this.role == Role.PRESIDENT || this.role == Role.VICE_PRESIDENT || this.role == Role.TEAM_LEADER; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/user/entity/User.java` around lines 87 - 94, Update the outdated comment in the isManagerPosition method to reflect that the check is now role-based rather than string-matching on positionName: replace the current note about keywords like '팀장', '대표', '부대표' with a brief description that the method returns true for users whose Role enum is one of SYSTEM_ADMIN, PRESIDENT, VICE_PRESIDENT, or TEAM_LEADER (and false for TEAM_MEMBER and PENDING_MEMBER), referencing the isManagerPosition method and the Role enum so future readers understand the enum-based permission check.backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java (1)
164-174: Javadoc과 실제 구현이 일치하지 않습니다.Line 164의 주석에는 "라운드 삭제(관리자/OWNER)"라고 되어 있지만, 실제 구현(line 170)은
ensureOwner를 호출하여 OWNER만 삭제할 수 있습니다. 주석을 업데이트하거나 의도된 권한 정책을 확인해 주세요.또한
closeRound와openRound는 여전히ensureAdmin을 사용하는데,deleteRound만ensureOwner를 사용하는 것이 의도된 설계인지 확인이 필요합니다.📝 Javadoc 수정 제안
- /** 라운드 삭제(관리자/OWNER) */ + /** 라운드 삭제(OWNER 전용) */ public void deleteRound(UUID roundId, UUID userId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java` around lines 164 - 174, The Javadoc "라운드 삭제(관리자/OWNER)" for deleteRound is inconsistent with the implementation which calls authorizationService.ensureOwner (so only OWNER can delete); decide which policy you want and make them consistent: either update the Javadoc on deleteRound(UUID, UUID) to state "OWNER only" or change the authorization call to use the same admin-check as closeRound/openRound (e.g., replace ensureOwner with the admin check used in closeRound/openRound) and then run a quick pass to ensure closeRound and openRound (methods closeRound(...) and openRound(...)) use the intended authority uniformly across the class.backend/src/main/java/org/sejongisc/backend/attendance/controller/SessionUserController.java (2)
195-214: removeAdminFromSession에도 동일하게 문서 보완이 필요합니다.
addAdminToSession과 마찬가지로## 인증(JWT),## 경로 파라미터,## 에러 코드섹션을 추가하여 API 문서의 일관성을 유지해주세요.📝 제안된 문서 개선
`@Operation`( summary = "세션 관리자 제거", description = """ + ## 인증(JWT) + - **필요** + ## 권한 - **세션 OWNER** + ## 경로 파라미터 + - **`sessionId`**: 세션 ID (`UUID`) + - **`userId`**: 관리자에서 제거할 사용자 ID (`UUID`) + ## 동작 설명 - 특정 사용자의 역할을 `PARTICIPANT`로 강등시킵니다. - `OWNER`는 강등될 수 없습니다. + + ## 에러 코드 + - **`SESSION_NOT_FOUND`**: 해당 출석 세션이 존재하지 않습니다. + - **`NOT_SESSION_OWNER`**: 세션 소유자 권한이 없습니다. """)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/controller/SessionUserController.java` around lines 195 - 214, The Operation description for removeAdminFromSession in SessionUserController lacks the same documentation sections as addAdminToSession; update the `@Operation`(description=...) block for the method removeAdminFromSession to include ## 인증(JWT) with info about required JWT and scope, ## 경로 파라미터 describing sessionId and userId, and ## 에러 코드 listing possible HTTP error responses (e.g., 400/401/403/404) and their conditions so the API docs match addAdminToSession's format.
171-189: OpenAPI 문서가 다른 엔드포인트와 일관성이 부족합니다.이 파일의 다른 엔드포인트들과 달리
## 인증(JWT),## 경로 파라미터,## 에러 코드섹션이 누락되어 있습니다. 또한 Line 187의 주석은 서비스 레이어에서 검증이 구현되어 있다면 제거하는 것이 좋습니다.📝 제안된 문서 개선
`@Operation`( summary = "세션 관리자 추가", description = """ + ## 인증(JWT) + - **필요** + ## 권한 - **세션 OWNER** + ## 경로 파라미터 + - **`sessionId`**: 세션 ID (`UUID`) + - **`userId`**: 관리자로 추가할 사용자 ID (`UUID`) + ## 동작 설명 - 특정 사용자의 역할을 `MANAGER`로 격상시킵니다. + + ## 에러 코드 + - **`SESSION_NOT_FOUND`**: 해당 출석 세션이 존재하지 않습니다. + - **`USER_NOT_FOUND`**: 유저를 찾을 수 없습니다. + - **`NOT_SESSION_OWNER`**: 세션 소유자 권한이 없습니다. """) `@PostMapping`("/{sessionId}/admins/{userId}") public ResponseEntity<Void> addAdminToSession( `@PathVariable` UUID sessionId, `@PathVariable` UUID userId, `@AuthenticationPrincipal` CustomUserDetails userDetails) { UUID adminUserId = requireUserId(userDetails); - // 서비스단에서 세션 소유자(OWNER)인지 검증하는 로직이 포함되어야 함 sessionUserService.addAdmin(sessionId, userId, adminUserId); return ResponseEntity.ok().build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/attendance/controller/SessionUserController.java` around lines 171 - 189, Update the OpenAPI Operation for addAdminToSession to match other endpoints by adding "## 인증(JWT)" with JWT auth note, "## 경로 파라미터" documenting sessionId and userId, and "## 에러 코드" listing possible HTTP errors; remove the inline comment about service-layer validation on the line above sessionUserService.addAdmin (since validation belongs in the service), and ensure you reference requireUserId(CustomUserDetails) and sessionUserService.addAdmin(sessionId, userId, adminUserId) as the call that performs the OWNER check in the service layer.backend/src/main/java/org/sejongisc/backend/admin/service/AdminUserService.java (1)
112-118: Javadoc 주석이 실제 메서드 동작과 불일치합니다.메서드가 이제 모든 등급 변경을 처리하지만, Javadoc은 여전히 "선배(SENIOR) 등급으로 변경"이라고 되어 있습니다.
📝 제안된 수정
/** - * 특정 사용자를 선배(SENIOR) 등급으로 변경 + * 특정 사용자의 등급(Grade) 변경 */ `@Transactional` public void updateUserGrade(UUID userId, Grade grade) { userService.updateUserGrade(userId, grade); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/admin/service/AdminUserService.java` around lines 112 - 118, The Javadoc for AdminUserService.updateUserGrade is inaccurate (says it changes a user to SENIOR) while the method accepts a Grade and updates any grade; update the method comment to describe that updateUserGrade(UUID userId, Grade grade) updates the specified user's grade to the provided Grade value, mention parameters (userId, grade) and transactional behavior if desired, and remove the SENIOR-specific wording so the doc matches the method signature and behavior.backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java (1)
99-101: 메서드명이 실제 동작과 불일치합니다.메서드명
promoteToSenior는 이제 SENIOR만이 아닌 모든 등급 변경을 처리하므로updateUserGrade로 변경하는 것이 적절합니다.♻️ 제안된 수정
- public ResponseEntity<Void> promoteToSenior(`@PathVariable` UUID userId, + public ResponseEntity<Void> updateUserGrade(`@PathVariable` UUID userId, `@RequestParam` Grade grade) { adminUserService.updateUserGrade(userId, grade); return ResponseEntity.noContent().build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java` around lines 99 - 101, The controller method name promoteToSenior no longer matches its behavior (it updates any Grade), so rename the method in AdminUserController from promoteToSenior to updateUserGrade; update the method signature and any internal references or usages (e.g., in tests or other callers) to the new name while keeping the existing `@PathVariable` UUID userId and `@RequestParam` Grade grade parameters and request mapping intact so behavior and routing don't change.backend/src/main/java/org/sejongisc/backend/user/service/UserService.java (1)
100-105: 로그 메시지가 실제 동작과 불일치합니다.
updateUserGrade메서드는 이제 모든 등급 변경을 처리하지만, 로그 메시지는 여전히 "선배 등급 전환 완료"라고 되어 있습니다. 정확한 로깅을 위해 실제 변경된 등급을 포함하도록 수정해주세요.🔧 제안된 수정
`@Transactional` public void updateUserGrade(UUID userId, Grade grade) { User user = findUser(userId); user.setGrade(grade); - log.info("선배 등급 전환 완료: userId={}, 학번={}", userId, user.getStudentId()); + log.info("사용자 등급 변경 완료: userId={}, 학번={}, 등급={}", userId, user.getStudentId(), grade); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/user/service/UserService.java` around lines 100 - 105, updateUserGrade in UserService currently logs a hardcoded "선배 등급 전환 완료" message which is incorrect; change the log.info call in updateUserGrade(UUID userId, Grade grade) to include the actual new grade value (e.g., include grade or user.getGrade()) and adjust the message text to reflect the dynamic grade change (for example "유저 등급 전환 완료: userId={}, 학번={}, 이전등급={}, 변경등급={}"), so the log accurately reports the changed grade.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java`:
- Around line 86-103: The `@Operation` annotation on the promoteToSenior method is
missing its closing parenthesis, causing a compile error; fix it by adding the
closing ')' after the description block in the `@Operation` declaration (the
annotation that precedes the promoteToSenior(...) method) so the annotation is
properly terminated and the method signature compiles.
- Around line 97-101: Frontend callers that currently hit
/api/admin/users/{userId}/senior must be updated because the backend expose the
promoteToSenior(PATCH) handler which now lives at
/api/admin/users/{userId}/grade and requires a grade query parameter used by
adminUserService.updateUserGrade; change the API calls to PATCH
/api/admin/users/{userId}/grade and append ?grade=REGULAR_MEMBER (or the desired
Grade enum value), ensuring the HTTP method remains PATCH and the grade query
string is included in the request URL wherever the old /senior endpoint was
used.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java`:
- Line 116: The admin manual attendance update (in
updateAttendanceStatusByRound) currently only has a TODO for activityLog and
does not publish an ActivityEvent like checkIn does; modify
updateAttendanceStatusByRound to create and publish an ActivityEvent (or call
the same helper used by checkIn) whenever an attendance is created, its status
changes, or deleted, including relevant metadata (actor=admin, target attendance
id, previousStatus, newStatus, reason), and ensure you reuse the existing
ActivityEvent class and publisher/service used by checkIn so logs are consistent
and tests still pass.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java`:
- Around line 131-141: deleteSession currently deletes the AttendanceSession
without removing related rows, which will violate FK constraints on SessionUser,
AttendanceRound, and Attendance; either add cascade deletes on the entity
relationships or implement explicit deletion order in the service. Option A: in
the AttendanceSession entity, add/adjust `@OneToMany` mappings for sessionUsers
and attendanceRounds to include cascade = CascadeType.REMOVE (or
CascadeType.ALL) and orphanRemoval = true, and in AttendanceRound add `@OneToMany`
for attendances with cascade remove so removing AttendanceSession deletes all
children. Option B: in AttendanceSessionService.deleteSession, before calling
attendanceSessionRepository.delete(session), delete dependent records explicitly
(call the repository methods such as attendanceRepository.deleteByRoundIds(...),
attendanceRoundRepository.deleteBySessionId(sessionId),
sessionUserRepository.deleteBySessionId(sessionId)) in the correct order, then
delete the session; update/introduce repository deleteBy... methods as needed.
---
Outside diff comments:
In
`@backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java`:
- Around line 97-120: Update the API doc block in AttendanceRoundController for
the DeleteMapping("/rounds/{roundId}") to match the implementation: change the
permission description that currently says "요청 유저가 해당 세션의 관리 권한(MANAGER/OWNER)을
가졌는지 검증" to indicate only the session OWNER is allowed (e.g., "요청 유저가 세션 OWNER인지
검증"), and replace the listed error code `NOT_SESSION_ADMIN` with
`NOT_SESSION_OWNER` so the documented error matches the service's ensureOwner
behavior.
In `@backend/src/main/java/org/sejongisc/backend/user/entity/Grade.java`:
- Around line 3-18: Existing DB rows with grade='SENIOR' will cause JPA to throw
IllegalArgumentException because the SENIOR enum constant was removed from
Grade; before deploying, run a migration that updates all legacy values to
REGULAR_MEMBER (for example: UPDATE user SET grade='REGULAR_MEMBER' WHERE
grade='SENIOR') and verify AdminUserService logic that previously set SENIOR is
updated to use REGULAR_MEMBER; also consider adding a temporary tolerant import
in Grade.fromString to map "선배"/"OB" strings to REGULAR_MEMBER during migration
rollout if needed.
---
Nitpick comments:
In
`@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java`:
- Around line 99-101: The controller method name promoteToSenior no longer
matches its behavior (it updates any Grade), so rename the method in
AdminUserController from promoteToSenior to updateUserGrade; update the method
signature and any internal references or usages (e.g., in tests or other
callers) to the new name while keeping the existing `@PathVariable` UUID userId
and `@RequestParam` Grade grade parameters and request mapping intact so behavior
and routing don't change.
In
`@backend/src/main/java/org/sejongisc/backend/admin/service/AdminUserService.java`:
- Around line 112-118: The Javadoc for AdminUserService.updateUserGrade is
inaccurate (says it changes a user to SENIOR) while the method accepts a Grade
and updates any grade; update the method comment to describe that
updateUserGrade(UUID userId, Grade grade) updates the specified user's grade to
the provided Grade value, mention parameters (userId, grade) and transactional
behavior if desired, and remove the SENIOR-specific wording so the doc matches
the method signature and behavior.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/controller/SessionUserController.java`:
- Around line 195-214: The Operation description for removeAdminFromSession in
SessionUserController lacks the same documentation sections as
addAdminToSession; update the `@Operation`(description=...) block for the method
removeAdminFromSession to include ## 인증(JWT) with info about required JWT and
scope, ## 경로 파라미터 describing sessionId and userId, and ## 에러 코드 listing possible
HTTP error responses (e.g., 400/401/403/404) and their conditions so the API
docs match addAdminToSession's format.
- Around line 171-189: Update the OpenAPI Operation for addAdminToSession to
match other endpoints by adding "## 인증(JWT)" with JWT auth note, "## 경로 파라미터"
documenting sessionId and userId, and "## 에러 코드" listing possible HTTP errors;
remove the inline comment about service-layer validation on the line above
sessionUserService.addAdmin (since validation belongs in the service), and
ensure you reference requireUserId(CustomUserDetails) and
sessionUserService.addAdmin(sessionId, userId, adminUserId) as the call that
performs the OWNER check in the service layer.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java`:
- Around line 164-174: The Javadoc "라운드 삭제(관리자/OWNER)" for deleteRound is
inconsistent with the implementation which calls
authorizationService.ensureOwner (so only OWNER can delete); decide which policy
you want and make them consistent: either update the Javadoc on
deleteRound(UUID, UUID) to state "OWNER only" or change the authorization call
to use the same admin-check as closeRound/openRound (e.g., replace ensureOwner
with the admin check used in closeRound/openRound) and then run a quick pass to
ensure closeRound and openRound (methods closeRound(...) and openRound(...)) use
the intended authority uniformly across the class.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java`:
- Around line 102-103: The call attendanceSessionRepository.findById(sessionId)
in SessionUserService currently only checks existence and discards the result;
change it to capture the Optional result and assign the resolved entity to a
variable (e.g., attendanceSession =
attendanceSessionRepository.findById(sessionId).orElseThrow(() -> new
CustomException(ErrorCode.SESSION_NOT_FOUND))) so the retrieved session can be
used, or if the intent is only to check existence replace the call with
attendanceSessionRepository.existsById(sessionId) or explicitly ignore the value
with a clear comment; update any subsequent code to use the new
attendanceSession variable if needed.
In `@backend/src/main/java/org/sejongisc/backend/user/entity/User.java`:
- Around line 87-94: Update the outdated comment in the isManagerPosition method
to reflect that the check is now role-based rather than string-matching on
positionName: replace the current note about keywords like '팀장', '대표', '부대표'
with a brief description that the method returns true for users whose Role enum
is one of SYSTEM_ADMIN, PRESIDENT, VICE_PRESIDENT, or TEAM_LEADER (and false for
TEAM_MEMBER and PENDING_MEMBER), referencing the isManagerPosition method and
the Role enum so future readers understand the enum-based permission check.
In `@backend/src/main/java/org/sejongisc/backend/user/service/UserService.java`:
- Around line 100-105: updateUserGrade in UserService currently logs a hardcoded
"선배 등급 전환 완료" message which is incorrect; change the log.info call in
updateUserGrade(UUID userId, Grade grade) to include the actual new grade value
(e.g., include grade or user.getGrade()) and adjust the message text to reflect
the dynamic grade change (for example "유저 등급 전환 완료: userId={}, 학번={}, 이전등급={},
변경등급={}"), so the log accurately reports the changed grade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c233d833-5166-45f4-99d2-1b4fd8163119
📒 Files selected for processing (14)
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.javabackend/src/main/java/org/sejongisc/backend/admin/service/AdminUserService.javabackend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.javabackend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceSessionController.javabackend/src/main/java/org/sejongisc/backend/attendance/controller/SessionUserController.javabackend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.javabackend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.javabackend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.javabackend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.javabackend/src/main/java/org/sejongisc/backend/common/exception/ErrorCode.javabackend/src/main/java/org/sejongisc/backend/user/entity/Grade.javabackend/src/main/java/org/sejongisc/backend/user/entity/User.javabackend/src/main/java/org/sejongisc/backend/user/entity/UserStatus.javabackend/src/main/java/org/sejongisc/backend/user/service/UserService.java
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java (1)
99-101: 메서드명은 이제 실제 동작과 맞추는 편이 좋겠습니다.엔드포인트와 서비스 호출은 범용
Grade변경인데, 컨트롤러 메서드명만promoteToSenior로 남아 있어 코드 탐색과 로그 해석이 헷갈립니다.updateUserGrade처럼 맞춰 두는 편이 낫습니다.♻️ 제안
- public ResponseEntity<Void> promoteToSenior(`@PathVariable` UUID userId, - `@RequestParam` Grade grade) { + public ResponseEntity<Void> updateUserGrade(`@PathVariable` UUID userId, + `@RequestParam` Grade grade) { adminUserService.updateUserGrade(userId, grade); return ResponseEntity.noContent().build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java` around lines 99 - 101, The controller method name promoteToSenior no longer matches its behavior—it's calling adminUserService.updateUserGrade and accepts a Grade param—so rename the method to updateUserGrade (or another name matching the service) in AdminUserController and update any references (handlers, tests, logs) so the mapping and behavior remain unchanged; ensure the `@PathVariable` UUID userId and `@RequestParam` Grade grade signature and the ResponseEntity<Void> return are preserved while only renaming the method to align with adminUserService.updateUserGrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java`:
- Around line 64-66: The OpenAPI `@Operation` text and the actual access control
(`@PreAuthorize`) are inconsistent: either update the `@Operation`
summaries/descriptions to list both allowed roles (PRESIDENT and SYSTEM_ADMIN)
for the three annotated endpoints (the blocks using
`@PatchMapping`("/{userId}/status"), the other two similar annotation blocks
around the file) or change the `@PreAuthorize` expressions to match the
documentation (e.g., remove PRESIDENT so only SYSTEM_ADMIN can call them).
Locate the three annotation groups in AdminUserController.java (the
`@Operation/`@PatchMapping/@PreAuthorize sets) and apply the chosen fix
consistently across all three places so the Swagger docs and runtime security
match.
- Line 66: Controller must pass the caller's ID into the service and the service
must enforce self-modify and superior-role protection: update the controller
calls to supply requesterId to updateUserStatus, updateUserRole,
updateUserGrade, deleteUser, then implement checks inside the corresponding
UserService methods (e.g., updateUserStatus, updateUserRole, updateUserGrade,
deleteUser) to throw a CustomException (use ErrorCode.CANNOT_MODIFY_SELF) if
userId.equals(requesterId) and to load the target user (e.g., via findUser) and
forbid modifications to Role.SYSTEM_ADMIN unless the requester has SYSTEM_ADMIN
or higher (throw ErrorCode.CANNOT_MODIFY_SUPERIOR); ensure these checks run
before applying any entity setters/changes.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java`:
- Around line 142-152: The current manual child-deletes then parent delete
(attendanceRepository.deleteBySessionId,
attendanceRoundRepository.deleteBySessionId,
sessionUserRepository.deleteBySessionId,
attendanceSessionRepository.delete(session)) risks a race where new child rows
are inserted between the child deletes and parent delete causing FK failures;
fix by either (A) protecting the parent row with a pessimistic lock before
deleting children—add a repository method like findByIdForUpdate(...) that uses
LockModeType.PESSIMISTIC_WRITE and load the AttendanceSession with that lock
inside the `@Transactional` service method before performing deletes—or (B) make
deletes atomic via JPA cascade/remove on AttendanceSession (e.g., `@OneToMany`
mappedBy = "attendanceSession" with cascade = CascadeType.REMOVE and
orphanRemoval = true) and then simply delete the parent entity; choose one
approach and update the service to use the locked parent retrieval or the
cascade-enabled entity accordingly.
---
Nitpick comments:
In
`@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java`:
- Around line 99-101: The controller method name promoteToSenior no longer
matches its behavior—it's calling adminUserService.updateUserGrade and accepts a
Grade param—so rename the method to updateUserGrade (or another name matching
the service) in AdminUserController and update any references (handlers, tests,
logs) so the mapping and behavior remain unchanged; ensure the `@PathVariable`
UUID userId and `@RequestParam` Grade grade signature and the ResponseEntity<Void>
return are preserved while only renaming the method to align with
adminUserService.updateUserGrade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 123ab75f-6c73-469f-89d8-6f1344e1df30
📒 Files selected for processing (5)
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.javabackend/src/main/java/org/sejongisc/backend/attendance/repository/AttendanceRepository.javabackend/src/main/java/org/sejongisc/backend/attendance/repository/AttendanceRoundRepository.javabackend/src/main/java/org/sejongisc/backend/attendance/repository/SessionUserRepository.javabackend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java
Show resolved
Hide resolved
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
1 participant
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
새로운 기능
개선사항
제거/변경