Repository navigation
[BE] SISC1-224 [FIX] 출석 라운드 관련 수정 - #145
Hidden character warning
Conversation
Walkthrough세션 기반 출석 확인 API 엔드포인트와 관련 서비스 메서드가 제거되었습니다. 동시에 시간 필드 표현이 변경되었습니다: Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as AttendanceController
participant Service as AttendanceSessionService
participant Repo as Repository
Note over Client,Controller: POST /sessions/{sessionId}/check-in<br/>(REMOVED)
rect rgb(200, 100, 100)
Client-XController: POST /check-in (deprecated)
Note over Controller: Endpoint removed<br/>No longer available
Controller--XClient: 404 Not Found
end
Note over Service: Round-based check-in<br/>(Active flow)
Client->>Service: checkInByRound(roundId, request, userId)
Service->>Service: Validate attendance round
Service->>Repo: Query & Update
Repo-->>Service: Attendance record
Service-->>Client: AttendanceResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분 특별히 주의가 필요한 부분:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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: 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/AttendanceSessionService.java (1)
82-94: 주석과 구현의 불일치주석에는 "생성 순으로 정렬"이라고 되어 있지만,
findAll()은 정렬 순서를 보장하지 않습니다. 정렬이 필요하다면 명시적인 정렬을 추가하거나, 정렬이 불필요하다면 주석을 수정해주세요.정렬이 필요한 경우:
@Transactional(readOnly = true) public List<AttendanceSessionResponse> getAllSessions() { - List<AttendanceSession> sessions = attendanceSessionRepository.findAll(); + List<AttendanceSession> sessions = attendanceSessionRepository.findAll(Sort.by(Sort.Direction.DESC, "createdAt"));정렬이 불필요한 경우 주석을 수정:
- * - 생성 순으로 정렬 + * - 정렬 순서 보장 없음
🧹 Nitpick comments (3)
backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceSession.java (1)
61-62:defaultStartTimenull 체크 누락
allowedMinutes에 대해 null 처리가 되어 있지만,defaultStartTime이 null인 경우 NPE가 발생할 수 있습니다. 컬럼에nullable = false가 설정되어 있지만 빌더 패턴이나 직렬화 과정에서 null이 설정될 수 있습니다.public LocalTime getEndTime() { + if (defaultStartTime == null) { + return null; + } return defaultStartTime.plusMinutes(allowedMinutes != null ? allowedMinutes : 30); }backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java (2)
16-16: 사용되지 않는 import 가능성
startsAt(LocalDateTime)이defaultStartTime(LocalTime)으로 변경되면서LocalDateTimeimport가 더 이상 사용되지 않는 것으로 보입니다.-import java.time.LocalDateTime;
96-108:getAllSessions와 중복된 구현
getPublicSessions가getAllSessions와 완전히 동일한 구현입니다. 의도적인 것인지 확인이 필요합니다:
- 공개 세션만 필터링하는 로직이 필요한 것이 아닌지?
- 중복이 의도적이라면 한 메서드가 다른 메서드를 호출하도록 리팩터링 고려
또한 Line 99의 주석도
getAllSessions와 동일한 정렬 불일치 문제가 있습니다.@Transactional(readOnly = true) public List<AttendanceSessionResponse> getPublicSessions() { - List<AttendanceSession> sessions = attendanceSessionRepository.findAll(); - - return sessions.stream() - .map(this::convertToResponse) - .collect(Collectors.toList()); + return getAllSessions(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceController.java(0 hunks)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java(3 hunks)backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionRequest.java(3 hunks)backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java(1 hunks)backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceSession.java(3 hunks)backend/src/main/java/org/sejongisc/backend/attendance/repository/AttendanceSessionRepository.java(0 hunks)backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java(0 hunks)backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java(9 hunks)backend/src/test/java/org/sejongisc/backend/attendance/controller/AttendanceRoundControllerTest.java(8 hunks)
💤 Files with no reviewable changes (3)
- backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceService.java
- backend/src/main/java/org/sejongisc/backend/attendance/controller/AttendanceController.java
- backend/src/main/java/org/sejongisc/backend/attendance/repository/AttendanceSessionRepository.java
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/test/java/org/sejongisc/backend/attendance/controller/AttendanceRoundControllerTest.java (1)
frontend/src/contexts/AttendanceContext.jsx (1)
value(202-229)
backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java (2)
frontend/src/contexts/AttendanceContext.jsx (1)
sessions(52-52)frontend/src/components/attendancemanage/SessionModifyModal.jsx (1)
defaultAvailableMinutes(29-31)
🔇 Additional comments (16)
backend/src/main/java/org/sejongisc/backend/attendance/entity/Attendance.java (1)
71-92: 지각 판단 로직 개선 - LGTM with minor concern라운드 기반 및 세션 기반 출석에 대해 각각 적절한 기준 시간을 사용하고 있으며, null 처리도 잘 되어 있습니다.
다만
AttendanceSession.isCheckInAvailableForRound()와 마찬가지로 자정을 넘는 경우LocalTime비교가 의도대로 동작하지 않을 수 있습니다. 예를 들어 23:55에 시작한 라운드에서 00:02에 체크인하면00:02.isAfter(00:00)(23:55 + 5분)이 true가 되어 지각으로 처리되지만, 실제로는 시작 후 7분이므로 지각이 맞습니다. 그러나 시작 시간이 00:02이고 체크인이 23:58인 경우 잘못된 결과가 나올 수 있습니다.backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceRoundResponse.java (2)
65-77: fromEntity 매핑 변경 - LGTM엔티티에서 DTO로의 변환 로직이 새로운 필드명에 맞게 올바르게 업데이트되었습니다.
29-38: Unable to verify frontend compatibility - repository access requiredI was unable to access the repository to verify whether the frontend code actually references the old field names (
roundId,roundDate). The repository appears to be private or inaccessible.To complete this verification, you would need to:
- Manually search the frontend codebase for usage of
roundIdandroundDatefields- Check
AttendanceContext.jsxand related components that consume theAttendanceRoundResponseAPI- Confirm whether these field names are hardcoded in API response handling or if they use dynamic property access that would adapt to the new field names
Without this information, I cannot definitively confirm whether this breaking change requires frontend updates.
backend/src/test/java/org/sejongisc/backend/attendance/controller/AttendanceRoundControllerTest.java (5)
78-98: 테스트 코드 업데이트 - LGTM
AttendanceRoundResponseDTO의 필드명 변경(roundId→id,roundDate→date)에 맞게 테스트 빌더와 JSON 경로 검증이 올바르게 업데이트되었습니다.
127-142: 조회 테스트 업데이트 - LGTM새로운 필드명으로 응답 객체 생성 및 검증이 일관되게 수정되었습니다.
153-177: 목록 조회 테스트 - LGTM여러 라운드를 조회하는 테스트에서 각 응답 객체의 필드명이 올바르게 업데이트되었습니다.
194-212: 수정 테스트 업데이트 - LGTM라운드 수정 성공 테스트가 새로운 필드명을 올바르게 사용하고 있습니다.
321-336: 날짜별 조회 테스트 - LGTM특정 날짜의 라운드 조회 테스트가 새로운 필드명을 사용하여 올바르게 업데이트되었습니다.
backend/src/main/java/org/sejongisc/backend/attendance/dto/AttendanceSessionRequest.java (2)
29-36: 시간 표현 변경 - Breaking API 변경
startsAt(LocalDateTime) →defaultStartTime(LocalTime)로 변경되었습니다. API 클라이언트에서 요청 형식을 변경해야 합니다. 스키마 설명과 예시가 올바르게 업데이트되었습니다.
38-47: 유효성 검사 범위 변경 - LGTM
windowSeconds→allowedMinutes로 변경되고, 유효 범위가 5분 ~ 240분(4시간)으로 설정되었습니다. 검증 어노테이션과 메시지가 적절히 업데이트되었습니다.backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceSession.java (2)
30-34: Database migration scripts verification inconclusiveThe repository could not be accessed to verify whether database migration scripts have been prepared for the field changes (
startsAt→defaultStartTime,windowSeconds→allowedMinutes) and column renames (default_start_time,allowed_minutes).Please manually verify that the following are in place:
- Migration scripts (SQL or XML) exist in your migration tool's directory (typically
db/migration/,sql/, or similar)- If using Flyway, verify versioned SQL files (e.g.,
V*.sql) exist- If using Liquibase, verify changelog XML/YAML files exist
- The migration covers table schema changes and data type conversions
75-77: Unable to verifycalculateCurrentStatus()method behavior due to repository access limitationsThe verification process encountered persistent repository access failures, preventing examination of:
- All call sites of
calculateCurrentStatus()- The
SessionStatusenum definition- Evidence of whether state calculation was migrated to a round level
- The impact on dependent code
The original review comment warrants manual investigation to confirm whether this unconditional return of
SessionStatus.OPENis intentional or represents incomplete refactoring.backend/src/main/java/org/sejongisc/backend/attendance/service/AttendanceSessionService.java (4)
37-68: LGTM!로깅과 세션 빌더가 새로운 필드명(
defaultStartTime,allowedMinutes)으로 올바르게 업데이트되었습니다.
110-123: LGTM!라운드 기반 로직으로의 변경이 올바르게 구현되었습니다.
OPEN상태의 세션만 필터링하는 방식이 명확합니다.
125-160: LGTM!세션 수정 로직이 새로운 필드명(
defaultStartTime,allowedMinutes)으로 올바르게 업데이트되었습니다.
273-297: LGTM!
convertToResponse메서드가 새로운 필드를 올바르게 매핑하고 있습니다. 엔티티의allowedMinutes가 응답 DTO의defaultAvailableMinutes로 매핑되는 것이 프론트엔드 컨텍스트(SessionModifyModal.jsx)와 일치합니다.
| public boolean isCheckInAvailableForRound(java.time.LocalTime currentTime) { | ||
| return !currentTime.isBefore(defaultStartTime) && currentTime.isBefore(getEndTime()); |
There was a problem hiding this comment.
자정을 넘는 세션 시간 처리 문제
LocalTime 비교는 자정을 넘는 경우를 올바르게 처리하지 못합니다. 예를 들어 세션이 23:00에 시작하고 60분간 진행되면 종료 시간이 00:00이 되는데, 23:30의 currentTime은 isBefore(00:00)에서 false를 반환하여 체크인이 불가능하게 됩니다.
자정을 넘는 세션이 예상되는 경우 다음과 같이 수정을 고려해 주세요:
public boolean isCheckInAvailableForRound(java.time.LocalTime currentTime) {
- return !currentTime.isBefore(defaultStartTime) && currentTime.isBefore(getEndTime());
+ LocalTime endTime = getEndTime();
+ if (endTime.isBefore(defaultStartTime)) {
+ // 자정을 넘는 경우
+ return !currentTime.isBefore(defaultStartTime) || currentTime.isBefore(endTime);
+ }
+ return !currentTime.isBefore(defaultStartTime) && currentTime.isBefore(endTime);
}🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/attendance/entity/AttendanceSession.java
around lines 68-69, the current LocalTime comparison fails for sessions that
span midnight; change the logic to detect when getEndTime() is before or equal
to defaultStartTime (meaning the interval wraps past midnight) and in that case
allow check-in when currentTime is >= defaultStartTime OR currentTime is <
getEndTime(); otherwise use the normal non-wrapping check (currentTime >=
defaultStartTime AND currentTime < getEndTime()). Ensure you use LocalTime
comparisons (isBefore/isAfter/equals) consistently to include equality at start
time.
2 participants
Summary by CodeRabbit
릴리스 노트
Breaking Changes
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.