Repository navigation
[BE] SISC1-228 [FIX] session user 관련 로직 개선 - #152
Hidden character warning
Conversation
Walkthrough세션 기반 출석 엔드포인트 두 개를 deprecated로 표시하고, 출석 체크인 DTO에서 username을 제거했으며 AttendanceService에서 익명 사용자 제거와 날짜/시간/위치 검증을 중앙화하는 등 출석 처리 흐름을 단일화·엄격화했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50분
Possibly related PRs
Suggested reviewers
시
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceController.java(2 hunks)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceCheckInRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java(1 hunks)backend/src/main/java/org/sejongisc/backend/attendance/repository/SessionUserRepository.java(1 hunks)backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java(4 hunks)
🔇 Additional comments (9)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceCheckInRequest.java (1)
35-44: 조건부 유효성 검사 로직이 적절히 구현되었습니다.
@NotNull을 제거하고nullable = true로 변경한 것은 세션에 위치 정보가 있을 때만 필수로 처리하는 조건부 검증 로직과 일치합니다.@DecimalMin/@DecimalMax제약 조건은 값이 null이 아닐 때만 적용되므로 올바르게 동작합니다.서비스 레이어(
AttendanceService.checkInByRound, lines 129-137)에서 세션에 위치 정보가 있을 경우 null 체크를 수행하고 있어 일관성이 유지됩니다.backend/src/main/java/org/sejongisc/backend/attendance/repository/SessionUserRepository.java (1)
39-40: JPQL 쿼리 간소화가 적절합니다.
CASE WHEN COUNT(...) > 0 THEN true ELSE false END대신COUNT(su) > 0을 사용하여 쿼리가 더 간결해졌습니다. Spring Data JPA는 이 표현식을 boolean으로 올바르게 매핑합니다.backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (5)
65-70: 시간 검증 로직이 개선되었습니다. 다만, 자정을 넘는 세션에 대한 엣지 케이스를 확인해주세요.
LocalDate와LocalTime을 분리하여 검증하는 방식이 명확합니다. 그러나LocalTime비교는 자정을 넘는 세션(예: 23:00~01:00)을 올바르게 처리하지 못합니다.예시:
startTime = 23:00,endTime = 01:00인 경우checkTime = 23:30이면checkTime.isBefore(endTime)→23:30 < 01:00→false로 평가되어 유효한 시간임에도 실패합니다.현재 시스템에서 자정을 넘는 세션이 없다면 문제없지만, 향후 지원이 필요하다면
LocalDateTime기반 비교로 변경을 고려해주세요.
73-81: 날짜 검증 로직이 명확하고 적절합니다.현재 날짜와 라운드 날짜가 일치하지 않으면 조기 반환하며, 로그와 응답 메시지에 충분한 디버깅 정보가 포함되어 있습니다.
83-96: 시간 범위 검증이 올바르게 구현되었습니다.
[startTime, endTime)범위 검증이 정확하며, 성공/실패 시 로깅에 필요한 정보가 모두 포함되어 있습니다.
156-161: 지각 판별 로직이 명확합니다.시작 시간 + 5분 이내(
checkTime <= lateThreshold)는 정상 출석, 그 이후는 지각으로 처리됩니다. 로그에 판별 기준이 모두 표시되어 디버깅에 용이합니다.
187-190: 남은 시간 계산이 검증된 시간과 일관되게 개선되었습니다.
checkTime(검증에 사용된 시간)을 재사용하여LocalTime.now()를 다시 호출하는 것보다 일관성이 높습니다. 검증을 통과했다면checkTime < endTime이 보장되므로 양수 값이 반환됩니다.backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceController.java (2)
37-47: Deprecation 전략이 잘 구현되었습니다.Javadoc
@deprecated, Java@Deprecated(since = "2.0", forRemoval = true), OpenAPIdeprecated = true를 모두 사용하여 포괄적으로 문서화했습니다. 설명에 마이그레이션 경로(GET /api/attendance/rounds/{roundId}/attendances)가 명확히 제시되어 있어 API 소비자가 전환하기 쉽습니다.
83-94: 두 번째 deprecated 엔드포인트도 일관되게 처리되었습니다.첫 번째 엔드포인트와 동일한 패턴으로 deprecation이 적용되어 있으며, 새로운 라운드 기반 엔드포인트(
PUT /api/attendance/rounds/{roundId}/attendances/{userId})로의 마이그레이션 경로가 명확합니다.
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)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (1)
332-352:convertToResponse에서user가 null일 경우 NPE가 발생할 수 있습니다.
attendance.getUser()를 직접 호출하는데, 레거시 데이터나 다른 흐름에서 생성된Attendance레코드에user가 null일 수 있습니다. Line 256에서는 null 체크가 있지만 여기서는 누락되어 일관성이 없습니다.private AttendanceResponse convertToResponse(Attendance attendance) { + User user = attendance.getUser(); return AttendanceResponse.builder() .attendanceId(attendance.getAttendanceId()) - .userId(attendance.getUser().getUserId()) - .userName(attendance.getUser().getName()) + .userId(user != null ? user.getUserId() : null) + .userName(user != null ? user.getName() : "익명") .attendanceSessionId(attendance.getAttendanceSession().getAttendanceSessionId())
🧹 Nitpick comments (1)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (1)
57-57: 지각 기준 시간을 상수 또는 설정으로 분리하는 것을 권장합니다.
startTime.plusMinutes(5)로 하드코딩된 지각 기준을 상수나 설정으로 분리하면 향후 정책 변경 시 유지보수가 용이해집니다.private static final int LATE_THRESHOLD_MINUTES = 5; // ... LocalTime lateThreshold = startTime.plusMinutes(LATE_THRESHOLD_MINUTES);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceCheckInRequest.java(1 hunks)backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java(5 hunks)
🔇 Additional comments (6)
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceCheckInRequest.java (1)
31-36: 조건부 필수 필드 설명이 적절합니다.세션에 위치 정보가 있을 때만 필수라는 조건이
@Schema설명에 명확히 기술되어 있고, 실제 검증은AttendanceService에서 수행되므로 DTO에서@NotNull제거는 적절합니다.backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java (5)
40-42: 사용자 조회 로직이 적절합니다.익명 사용자 경로를 제거하고 명시적인 사용자 조회로 통일된 것은 올바른 개선입니다. 예외 메시지도 명확합니다.
52-68: 날짜 검증 로직이 잘 구현되었습니다.라운드 날짜와 현재 날짜를 비교하는 검증이 명확하게 추가되었습니다. 실패 시 적절한 응답을 반환합니다.
70-83: 시간 범위 검증 로직이 명확합니다.
startTime <= now < endTime조건이 올바르게 구현되었고, 로깅도 적절합니다.
85-95: 중복 출석 검사가 사용자 기반으로 단순화되었습니다.익명 사용자 경로 제거에 따라 중복 검사 로직이 간결해졌습니다.
156-159: 남은 시간 계산이checkTime기준으로 일관성 있게 변경되었습니다.시스템 시간 대신 검증에 사용된
checkTime을 활용하여 계산의 일관성이 보장됩니다.
2 participants
Summary by CodeRabbit
Deprecated
Refactor
API 변경
✏️ Tip: You can customize this high-level summary in your review settings.