Conversation
Contributor
Walkthrough스터디 세션의 최대 집중력 시간 단위를 분(minutes)에서 시간(hours)으로 변경했습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java (1)
107-111:⚠️ Potential issue | 🟠 Major정확히 3시간 경과한 세션이 누락될 수 있습니다.
Line 109의
findAllByStatusAndStartTimeBefore는<비교라 cutoff와 동일한 시작 시각은 제외됩니다. 요구사항이 “3시간 이상”이면<=비교 메서드로 바꾸는 게 정확합니다.제안 코드
- List<StudySession> expiredSessions = studySessionRepository.findAllByStatusAndStartTimeBefore( + List<StudySession> expiredSessions = studySessionRepository.findAllByStatusAndStartTimeLessThanEqual( StudyStatus.STARTED, cutoffTime );// src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java List<StudySession> findAllByStatusAndStartTimeLessThanEqual(StudyStatus status, LocalDateTime cutoffTime);Based on learnings: When a user studies for 3 hours or more, the system must automatically terminate the session.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java` around lines 107 - 111, The current query in StudySessionService uses studySessionRepository.findAllByStatusAndStartTimeBefore which excludes sessions that started exactly at the cutoff (e.g., exactly 3 hours) — change the repository method to use a less-than-or-equal variant (e.g., findAllByStatusAndStartTimeLessThanEqual) and update the call in StudySessionService to use that method so sessions with startTime == cutoffTime are included; modify the StudySessionRepository interface to declare List<StudySession> findAllByStatusAndStartTimeLessThanEqual(StudyStatus status, LocalDateTime cutoffTime) and replace invocations of findAllByStatusAndStartTimeBefore in StudySessionService with the new method.
🧹 Nitpick comments (2)
src/main/java/com/gpt/geumpumtabackend/study/domain/StudySession.java (1)
50-53: maxFocusHours 입력값 검증을 추가해 종료 시점 불변식을 보호해주세요.Line 51에서
maxFocusHours가 0 이하로 들어오면endTime/totalMillis가 비정상 값이 될 수 있습니다. 최소 1 이상 검증을 넣는 편이 안전합니다.제안 코드
public void endMaxFocusStudySession(int maxFocusHours) { + if (maxFocusHours <= 0) { + throw new BusinessException(ExceptionType.INVALID_END_TIME); + } this.endTime = this.startTime.plusHours(maxFocusHours); status = StudyStatus.FINISHED; this.totalMillis = Duration.between(this.startTime, this.endTime).toMillis(); }Based on learnings: Validate that endTime is not earlier than startTime when ending a study session.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/study/domain/StudySession.java` around lines 50 - 53, The endMaxFocusStudySession method currently sets endTime and totalMillis without validating maxFocusHours; add input validation at the start of endMaxFocusStudySession to ensure maxFocusHours >= 1 (throw IllegalArgumentException with a clear message if not), then compute this.endTime = this.startTime.plusHours(maxFocusHours) and set status = StudyStatus.FINISHED; after computing totalMillis = Duration.between(this.startTime, this.endTime).toMillis(), also guard that totalMillis >= 0 (throw or clamp and log if it would be negative) to ensure endTime is not earlier than startTime and preserve invariants for endTime, startTime, and totalMillis.src/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java (1)
317-346: 경계값(정확히 3시간) 테스트를 추가해 회귀를 막아주세요.현재는 4시간 경과 케이스 중심이라, “정확히 3시간” 포함 여부가 깨져도 놓칠 수 있습니다. Line 318 부근에 3시간 경계 시나리오를 별도 추가하는 것을 권장합니다.
As per coding guidelines: Test coverage should include normal cases, exception cases, and boundary value cases per TESTING.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java` around lines 317 - 346, Add a new unit test in StudySessionServiceTest that covers the 3-hour boundary: create a User (createTestUser) and a StudySession whose startStudySession time is LocalDateTime.now().minusHours(3), mock studyProperties.getMaxFocusHours() to return 3 and stub studySessionRepository.findAllByStatusAndStartTimeBefore(StudyStatus.STARTED, ...) to return that session, call studySessionService.endExpiredMaxFocusSessions(), and assert that the repository was called with a cutoff ~ now.minusHours(3) (capture with ArgumentCaptor), the returned usersToNotify contains the test user, and the session’s endTime == start.plusHours(3), totalMillis == 10_800_000L, and status == StudyStatus.FINISHED; reference the existing test pattern used in expired_cutoff_uses_three_hours and the methods endExpiredMaxFocusSessions, studyProperties.getMaxFocusHours, and studySessionRepository.findAllByStatusAndStartTimeBefore to implement it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java`:
- Around line 107-111: The current query in StudySessionService uses
studySessionRepository.findAllByStatusAndStartTimeBefore which excludes sessions
that started exactly at the cutoff (e.g., exactly 3 hours) — change the
repository method to use a less-than-or-equal variant (e.g.,
findAllByStatusAndStartTimeLessThanEqual) and update the call in
StudySessionService to use that method so sessions with startTime == cutoffTime
are included; modify the StudySessionRepository interface to declare
List<StudySession> findAllByStatusAndStartTimeLessThanEqual(StudyStatus status,
LocalDateTime cutoffTime) and replace invocations of
findAllByStatusAndStartTimeBefore in StudySessionService with the new method.
---
Nitpick comments:
In `@src/main/java/com/gpt/geumpumtabackend/study/domain/StudySession.java`:
- Around line 50-53: The endMaxFocusStudySession method currently sets endTime
and totalMillis without validating maxFocusHours; add input validation at the
start of endMaxFocusStudySession to ensure maxFocusHours >= 1 (throw
IllegalArgumentException with a clear message if not), then compute this.endTime
= this.startTime.plusHours(maxFocusHours) and set status = StudyStatus.FINISHED;
after computing totalMillis = Duration.between(this.startTime,
this.endTime).toMillis(), also guard that totalMillis >= 0 (throw or clamp and
log if it would be negative) to ensure endTime is not earlier than startTime and
preserve invariants for endTime, startTime, and totalMillis.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java`:
- Around line 317-346: Add a new unit test in StudySessionServiceTest that
covers the 3-hour boundary: create a User (createTestUser) and a StudySession
whose startStudySession time is LocalDateTime.now().minusHours(3), mock
studyProperties.getMaxFocusHours() to return 3 and stub
studySessionRepository.findAllByStatusAndStartTimeBefore(StudyStatus.STARTED,
...) to return that session, call
studySessionService.endExpiredMaxFocusSessions(), and assert that the repository
was called with a cutoff ~ now.minusHours(3) (capture with ArgumentCaptor), the
returned usersToNotify contains the test user, and the session’s endTime ==
start.plusHours(3), totalMillis == 10_800_000L, and status ==
StudyStatus.FINISHED; reference the existing test pattern used in
expired_cutoff_uses_three_hours and the methods endExpiredMaxFocusSessions,
studyProperties.getMaxFocusHours, and
studySessionRepository.findAllByStatusAndStartTimeBefore to implement it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57051e11-308b-487e-84d8-f7e0d244580f
📒 Files selected for processing (3)
src/main/java/com/gpt/geumpumtabackend/study/domain/StudySession.javasrc/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.javasrc/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java
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.
🚀 1. 개요
Summary by CodeRabbit
버그 수정
테스트