Conversation
Walkthrough출석 체크가 QR 토큰 + deviceId 요청으로 전환되고 Attendance 엔티티에 deviceId 및 (round_id, device_id) 유니크 제약이 추가되었으며, 서비스 예외가 CustomException/ErrorCode로 대체되고 다수 엔티티에 Lombok이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as AttendanceController
participant Service as AttendanceService
participant RoundService as AttendanceRoundService
participant Repo as AttendanceRepository
participant DB as Database
Client->>Controller: POST /api/attendance/check-in { qrToken, deviceId }
Controller->>Service: checkIn(userId, AttendanceRoundQrTokenRequest)
Service->>RoundService: verifyQrTokenAndGetRound(qrToken)
RoundService->>DB: SELECT round by token
DB-->>RoundService: round / not found
alt token valid
RoundService-->>Service: round
Service->>Repo: existsByAttendanceRound_RoundIdAndUserId(roundId, userId)
Repo-->>Service: boolean
Service->>Repo: existsByAttendanceRound_RoundIdAndDeviceId(roundId, deviceId)
Repo-->>Service: boolean
alt not duplicated
Service->>DB: INSERT Attendance(userId, roundId, deviceId, status)
DB-->>Service: created attendance
Service-->>Controller: 200/201 OK
else duplicated
Service-->>Controller: throw CustomException(ALREADY_CHECKED_IN / DEVICE_ALREADY_USED)
end
else token invalid
RoundService-->>Service: throw CustomException(QR_TOKEN_MALFORMED / ROUND_NOT_FOUND)
end
Controller-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceController.java (1)
33-40: 요청 DTO 검증(@Valid) 누락 가능성 확인 필요.
Line 35-38에서@Valid가 빠져 있으면qrToken/deviceId누락 시 500으로 떨어질 수 있습니다.✅ 수정 제안
public ResponseEntity<Void> checkIn( `@AuthenticationPrincipal` CustomUserDetails userDetails, - `@RequestBody` AttendanceRoundQrTokenRequest request + `@Valid` `@RequestBody` AttendanceRoundQrTokenRequest request ) {
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceSessionController.java`:
- Around line 85-93: The controller currently forces authentication by calling
requireUserId(userDetails) in getSession, which throws when userDetails is null;
change getSession to handle unauthenticated requests by checking if userDetails
is null and passing a null (or Optional.empty()) adminUserId to
attendanceSessionService.getSessionById instead of calling requireUserId, and
update the service method signature/implementation (getSessionById) to accept a
nullable/optional adminUserId and behave correctly for unauthenticated callers;
keep requireUserId for endpoints that still require auth but do not call it from
getSession.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java`:
- Around line 109-118: In verifyQrTokenAndGetRound, guard against null or blank
qrToken before calling split to avoid NPE: at the start of the method check if
qrToken is null or qrToken.trim().isEmpty() and if so throw new
CustomException(ErrorCode.QR_TOKEN_MALFORMED); then proceed with the existing
parsing (String[] parts = qrToken.split(":")) and subsequent
UUID.fromString(parts[0]) handling.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java`:
- Around line 51-76: Add a DB-level unique constraint on
(attendanceRound.roundId, deviceId) in the Attendance entity/migration to
prevent race-condition duplicate device check-ins, and improve the
DataIntegrityViolationException handling around attendanceRepository.save(att)
to distinguish causes: when a save fails, re-query
attendanceRepository.existsByAttendanceRound_RoundIdAndDeviceId(round.getRoundId(),
request.deviceId()) (or inspect the constraint name from the exception) and
throw ErrorCode.DEVICE_ALREADY_USED if true, otherwise throw
ErrorCode.ALREADY_CHECKED_IN; update the Attendance entity/migration, the save
block catch for DataIntegrityViolationException, and keep existing pre-checks
(attendanceRepository.existsByUserAndAttendanceRound and the device pre-check)
intact.
🧹 Nitpick comments (8)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundQrTokenRequest.java (1)
3-6: 입력 유효성 검사 어노테이션 추가 권장
qrToken과deviceId는 대리출석 방지를 위한 핵심 필드입니다.@NotBlank또는@NotNull어노테이션을 추가하여 컨트롤러 레벨에서 유효성 검사를 수행하는 것이 좋습니다.♻️ 제안된 수정
+import jakarta.validation.constraints.NotBlank; + public record AttendanceRoundQrTokenRequest( + `@NotBlank`(message = "QR 토큰은 필수입니다.") String qrToken, + `@NotBlank`(message = "기기 ID는 필수입니다.") String deviceId ) { }컨트롤러에서
@Valid어노테이션도 함께 사용해야 합니다.backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceRound.java (1)
78-94: 상태 변경 메서드 개선 권장
changeStatus메서드에서 잘못된 상태 전환 시 조용히 무시됩니다. 디버깅 및 유지보수를 위해 로깅을 추가하거나, 잘못된 전환 시 예외를 던지는 것이 좋습니다. 또한 Line 84에 포맷팅 이슈가 있습니다.♻️ 제안된 수정
public void changeStatus(RoundStatus newStatus) { // 종료된 라운드는 상태 변경 불가 if (this.roundStatus == RoundStatus.CLOSED) { return; } - if(this.roundStatus == RoundStatus.ACTIVE &&newStatus == RoundStatus.UPCOMING) { + if (this.roundStatus == RoundStatus.ACTIVE && newStatus == RoundStatus.UPCOMING) { // ACTIVE -> UPCOMING 불가 return; }backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceSession.java (1)
18-24:@Setter사용에 대한 고려엔티티에
@Setter를 사용하면 모든 필드가 외부에서 변경 가능해집니다.AttendanceRound처럼 특정 비즈니스 로직을 통해서만 상태를 변경하도록 하거나,toBuilder()패턴만 사용하는 것이 데이터 무결성 측면에서 더 안전할 수 있습니다.backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java (2)
96-103: 메모리 내 필터링 대신 Repository 쿼리 사용 권장
findAll()후 메모리에서 필터링하는 것은 데이터가 많아지면 비효율적입니다. Repository에findByStatus(SessionStatus status)메서드를 추가하여 DB 레벨에서 필터링하는 것이 좋습니다.♻️ 제안된 수정
Repository에 메서드 추가:
List<AttendanceSession> findByStatus(SessionStatus status);Service 메서드 수정:
`@Transactional`(readOnly = true) public List<AttendanceSessionResponse> getActiveSessions() { - List<AttendanceSession> allSessions = attendanceSessionRepository.findAll(); - - return allSessions.stream() - .filter(session -> session.getStatus() == SessionStatus.OPEN) - .map(AttendanceSessionResponse::from) - .toList(); + return attendanceSessionRepository.findByStatus(SessionStatus.OPEN).stream() + .map(AttendanceSessionResponse::from) + .toList(); }
38-62: 세션 생성 순서 개선 권장현재 세션을 먼저 저장한 후 사용자를 조회합니다. 사용자 조회를 먼저 수행하면, 유효하지 않은 사용자에 대한 불필요한 DB 작업을 방지할 수 있습니다. 트랜잭션으로 롤백되지만, 순서를 변경하면 더 효율적입니다.
♻️ 제안된 수정
`@Transactional` public void createSession(UUID creatorId, AttendanceSessionRequest request) { + // 사용자 먼저 검증 + User creator = userRepository.findById(creatorId) + .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); // 출석 세션 엔티티 생성 AttendanceSession attendanceSession = AttendanceSession.builder() .title(request.title()) .description(request.description()) .allowedMinutes(request.allowedMinutes()) .status(SessionStatus.OPEN) .build(); AttendanceSession saved = attendanceSessionRepository.save(attendanceSession); - User creator = userRepository.findById(creatorId) - .orElseThrow(() -> new CustomException(ErrorCode.USER_NOT_FOUND)); // 세션 생성자를 OWNER로 세션 사용자에 추가 SessionUser su = SessionUser.builder()backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceSessionController.java (1)
209-215: 삭제 응답 상태 코드 고려RESTful 관례상 삭제 작업은 일반적으로
204 No Content를 반환합니다. 현재200 OK도 동작하지만, 일관성을 위해 고려해볼 수 있습니다.♻️ 제안된 수정
`@DeleteMapping`("/{sessionId}") public ResponseEntity<Void> deleteSession(`@PathVariable` UUID sessionId, `@AuthenticationPrincipal` CustomUserDetails userDetails) { UUID adminUserId = requireUserId(userDetails); attendanceSessionService.deleteSession(sessionId, adminUserId); - return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build(); }backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java (2)
56-57:deviceId컬럼에 길이 제한 추가를 권장합니다.
length속성이 지정되지 않아 데이터베이스에서 무제한 VARCHAR 또는 TEXT로 생성될 수 있습니다. 디바이스 ID의 예상 형식(UUID, 브라우저 핑거프린트 등)에 맞는 적절한 길이를 명시하면 스토리지 효율성과 데이터 일관성이 향상됩니다.♻️ 제안된 수정
- `@Column`(name = "device_id", nullable = false) + `@Column`(name = "device_id", nullable = false, length = 255) private String deviceId;
63-65: TODO 주석: 노트 필드 접근 제어 기능 미구현.지각 사유나 특이사항을 개인이 작성하고 관리자만 볼 수 있도록 하는 기능이 아직 구현되지 않은 것으로 보입니다. 이 요구사항을 별도 이슈로 추적하는 것을 권장합니다.
이 기능의 구현을 도와드리거나 추적을 위한 새 이슈를 생성해 드릴까요?
...d/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceSessionController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceRoundService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java`:
- Around line 102-116: Add two controller endpoints in AttendanceRoundController
to expose the service methods: create methods that accept `@PathVariable` UUID
roundId and `@AuthenticationPrincipal` CustomUserDetails userDetails, call
requireUserId(userDetails) to get userId, log the action (e.g., "라운드 닫기" / "라운드
열기"), invoke attendanceRoundService.closeRound(roundId, userId) and
attendanceRoundService.openRound(roundId, userId) respectively, and return an
appropriate ResponseEntity (e.g., ResponseEntity.noContent().build()). Use
distinct mappings such as `@PostMapping`("/rounds/{roundId}/close") and
`@PostMapping`("/rounds/{roundId}/open") (or `@PatchMapping` if preferred) and
ensure method names reflect the actions (e.g., closeRound and openRound in the
controller) to match the service methods.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java`:
- Around line 46-74: Add non-blank validation for deviceId to prevent
empty-device check-ins: annotate the deviceId field in
AttendanceRoundQrTokenRequest with `@NotBlank` (javax.validation) and ensure the
controller endpoint that consumes that DTO has `@Valid` on the `@RequestBody`
parameter (e.g., the controller method that handles QR check-ins). Additionally
add a defensive service-level check in the AttendanceService method that
performs the check-in (e.g., the method that processes QR tokens / marks
attendance) to explicitly validate StringUtils.hasText(deviceId) and throw a
BadRequest/ValidationException if blank, so the DEVICE_ALREADY_USED logic cannot
be bypassed; this touches Attendance.deviceId,
AttendanceRoundQrTokenRequest.deviceId, the controller QR-checkin method, and
the AttendanceService check-in method.
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java`:
- Around line 38-67: The addUserToSession method currently checks exists then
saves which allows a race to create duplicate SessionUser rows; add a unique
constraint on SessionUser for (attendance_session_id, user_id) at the
database/entity level and remove reliance on exists as the sole guard, and
replace the IllegalArgumentException("ALREADY_JOINED") with throwing a
CustomException using a new or existing ErrorCode (e.g.
ErrorCode.ALREADY_JOINED); additionally wrap the sessionUserRepository.save(...)
call in a try/catch that converts DataIntegrityViolationException (or the JPA
constraint violation exception) into the same CustomException so concurrent
inserts return the consistent error response.
- Around line 72-86: The method removeUserFromSession currently throws
IllegalArgumentException("SESSION_NOT_FOUND"); replace these raw
IllegalArgumentException/IllegalState usages with your application's
CustomException/ErrorCode-based exceptions so the global handler maps them
correctly — e.g., throw new CustomException(ErrorCode.SESSION_NOT_FOUND) when
attendanceSessionRepository.findById(...) is empty, and ensure any
authorizationService.ensureOwner or other places that currently raise
NOT_SESSION_MEMBER/SESSION_NOT_FOUND use the same CustomException(ErrorCode.*)
pattern (also update the analogous cases referenced elsewhere in this class) so
the global exception handler handles them consistently.
- Around line 167-185: Add an actorUserId parameter to
SessionUserService.addAdmin and removeAdmin, and before loading the target
SessionUser call authorizationService.ensureOwner(actorUserId, sessionId) to
enforce owner-only actions; replace thrown IllegalStateException cases
("TARGET_NOT_SESSION_MEMBER", "CANNOT_DEMOTE_OWNER") with the unified
CustomException (use the existing domain error codes/messages pattern) and keep
existing logic (prevent demoting OWNER and set roles via
SessionUser.changeRole). Ensure method signatures and any callers are updated
accordingly.
- Around line 127-149: The joinSession method is missing the
createAbsentForPastRounds call and is vulnerable to a race between the exists
check and save; after loading AttendanceSession
(attendanceSessionRepository.findById(...)) and User
(userRepository.findById(...)), call createAbsentForPastRounds(sessionId, user)
before saving the new SessionUser (SessionUser.builder...), and wrap the
sessionUserRepository.save(...) in a try/catch for
DataIntegrityViolationException (or appropriate JPA constraint exception) to
translate a unique-constraint violation into the same "ALREADY_JOINED" error
(throw IllegalStateException("ALREADY_JOINED") or the existing error handling)
to handle concurrent inserts.
- Around line 91-96: The Javadoc/inline comment for
SessionUserService.getSessionUsers claims "members or public" access but the
implementation only calls authorizationService.ensureMember(sessionId,
viewerUserId) which throws for non-members; either remove "or public" from the
comment to match current behavior, or implement public-access support by adding
a visibility check: fetch the AttendanceSession (via AttendanceSessionRepository
or SessionUserService helper), expose a boolean like isPublic on
AttendanceSession (or a dedicated DAO call), and replace ensureMember with a new
guard (e.g., ensureMemberOrPublic or combine ensureMember with a visibility
check) so non-members can view when the session is public—update the comment to
reflect whichever approach you choose.
🧹 Nitpick comments (2)
backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java (1)
87-100: QR 토큰 발급에GET대신POST사용 고려토큰 발급은 서버에서 새로운 토큰을 생성하는 작업으로, 멱등성이 없는 부수 효과가 있습니다. REST 관점에서 리소스 생성/발급에는
POST가 더 적합할 수 있습니다. 다만, 토큰이 DB에 저장되지 않고 매번 새로 계산되는 구조라면GET도 허용될 수 있습니다.backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java (1)
106-123: 과거 라운드 결석 처리에서 N+1 쿼리 가능성라운드마다 attendance 조회가 발생해 라운드 수가 많으면 성능이 떨어질 수 있습니다. 사용자 기존 출석을 한 번에 조회해 Set으로 필터링하거나 batch 저장으로 줄이는 리팩토링을 권장합니다.
...end/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceRoundController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java`:
- Around line 126-133: The Attendance creation branch (when attendance == null)
is missing the required deviceId causing NOT NULL constraint failures; update
the Attendance.builder() call in the attendance == null path inside
AttendanceService.create/update logic to set deviceId (e.g., .deviceId(deviceId)
using an input param or fallback to a constant like "ADMIN_MANUAL") before
build(), so the Attendance entity’s deviceId (`@Column`(name = "device_id",
nullable = false)) is always populated on admin manual creation.
♻️ Duplicate comments (5)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (1)
70-82: 저장 실패 시 에러코드 분기가 필요합니다.
현재DataIntegrityViolationException을 항상ALREADY_CHECKED_IN으로 매핑해 deviceId 유니크 충돌도 동일 응답이 됩니다. 레이스 상황에서 잘못된 에러코드가 내려갈 수 있습니다.🛠️ 제안 수정
try { attendanceRepository.save(att); } catch (DataIntegrityViolationException e) { - throw new CustomException(ErrorCode.ALREADY_CHECKED_IN); + if (attendanceRepository.existsByAttendanceRound_RoundIdAndDeviceId( + round.getRoundId(), deviceId)) { + throw new CustomException(ErrorCode.DEVICE_ALREADY_USED); + } + throw new CustomException(ErrorCode.ALREADY_CHECKED_IN); }backend/src/main/java/org/sejongisc/backend/attendance/service/SessionUserService.java (4)
35-67: 동시 가입 경쟁 조건을 제약/예외 매핑으로 보강하세요.
exists확인 후save사이에 동시 요청이 들어오면 중복 삽입/제약 위반이 발생할 수 있습니다. (sessionId, userId) 유니크 제약을 보장하고save예외를ALREADY_JOINED로 변환해 주세요.🔧 개선 제안
- SessionUser saved = sessionUserRepository.save(sessionUser); + SessionUser saved; + try { + saved = sessionUserRepository.save(sessionUser); + } catch (DataIntegrityViolationException e) { + throw new CustomException(ErrorCode.ALREADY_JOINED); + }import org.springframework.dao.DataIntegrityViolationException;
88-99: 주석과 동작이 불일치합니다.
현재는ensureMember만 호출되어 비멤버는 조회 불가인데, 주석에는 “또는 공개”가 남아 있습니다. 공개 기능이 없다면 주석을 정정하고, 필요하다면 공개 세션 권한 로직을 추가해 주세요.
127-149: 자가 가입 시 과거 라운드 결석 처리와 경쟁 조건 보강이 필요합니다.
addUserToSession과 정책을 맞추기 위해createAbsentForPastRounds호출을 추가하고, 동시 가입 시 제약 위반을ALREADY_JOINED로 매핑해 주세요.🔧 개선 제안
- sessionUserRepository.save(SessionUser.builder() - .attendanceSession(session) - .user(user) - .sessionRole(SessionRole.PARTICIPANT) - .build()); + try { + sessionUserRepository.save(SessionUser.builder() + .attendanceSession(session) + .user(user) + .sessionRole(SessionRole.PARTICIPANT) + .build()); + } catch (DataIntegrityViolationException e) { + throw new CustomException(ErrorCode.ALREADY_JOINED); + } + + createAbsentForPastRounds(sessionId, user);import org.springframework.dao.DataIntegrityViolationException;
163-185: 관리자 승격/강등에 권한 검증이 누락되어 있습니다.
현재는 호출자 검증 없이 역할 변경이 가능해 보입니다.actorUserId를 받아ensureOwner(또는 정책에 맞는 권한 검증)를 추가하고 호출부를 함께 갱신하세요.🔧 개선 방향 예시
- public void addAdmin(UUID sessionId, UUID targetUserId) { + public void addAdmin(UUID sessionId, UUID targetUserId, UUID actorUserId) { + authorizationService.ensureOwner(sessionId, actorUserId); SessionUser su = sessionUserRepository .findByAttendanceSession_AttendanceSessionIdAndUser_UserId(sessionId, targetUserId) .orElseThrow(() -> new CustomException(ErrorCode.TARGET_NOT_SESSION_MEMBER)); su.changeRole(SessionRole.MANAGER); }- public void removeAdmin(UUID sessionId, UUID targetUserId) { + public void removeAdmin(UUID sessionId, UUID targetUserId, UUID actorUserId) { + authorizationService.ensureOwner(sessionId, actorUserId); SessionUser su = sessionUserRepository .findByAttendanceSession_AttendanceSessionIdAndUser_UserId(sessionId, targetUserId) .orElseThrow(() -> new CustomException(ErrorCode.TARGET_NOT_SESSION_MEMBER));
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundQrTokenRequest.java
Show resolved
Hide resolved
discipline24
approved these changes
Jan 28, 2026
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
3 participants
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.
#193
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.