[FEAT] 홈 화면 조회 API 구현 - #63
Conversation
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough홈 대시보드 조회 API와 응답 DTO를 추가하고, 스트릭·연습 시간·최근 플레이·학습 추천 데이터를 조합하도록 구현했습니다. 상대 날짜 포매터와 주 악기 조회 방식도 변경했으며 관련 서비스·컨트롤러 테스트를 추가했습니다. Changes홈 대시보드 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SecurityUtil
participant HomeController
participant HomeService
participant LearningService
participant PlayingRepository
Client->>SecurityUtil: 현재 사용자 ID 조회
SecurityUtil->>HomeController: userId 반환
HomeController->>HomeService: getHome(userId)
HomeService->>LearningService: 현재 학습 및 추천 학습 조회
HomeService->>PlayingRepository: 완료 플레이와 endedAt 조회
PlayingRepository-->>HomeService: 연습 데이터 반환
HomeService-->>HomeController: HomeResponseDTO 반환
HomeController-->>Client: ApiResponse<HomeResponseDTO> 반환
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 코드 리뷰 |
|
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (1)
src/main/java/com/mr/domain/home/service/HomeService.java (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
STREAK_LOOKBACK_DAYS이름이 실제 용도와 어긋납니다.이 상수는 스트릭(연속 출석) 계산에는 전혀 쓰이지 않습니다. 실제 스트릭은 60행
fetchPracticeDates가 상한 없이 전체 기록을 조회해서 계산하고,STREAK_LOOKBACK_DAYS는 오히려 주/월간 연습시간 집계(recentCompleted,buildPracticeSummary)의 조회 상한으로만 사용됩니다. 이름만 보면 스트릭 조회 기간이 60일로 제한된다고 오해하기 쉬워, 나중에 다른 개발자가 잘못된 가정으로 코드를 수정할 위험이 있습니다.
PRACTICE_SUMMARY_LOOKBACK_DAYS같은 이름으로 바꾸는 것을 제안합니다.✏️ 제안 diff
- private static final int STREAK_LOOKBACK_DAYS = 60; + private static final int PRACTICE_SUMMARY_LOOKBACK_DAYS = 60;- LocalDateTime since = LocalDateTime.now().minusDays(STREAK_LOOKBACK_DAYS); + LocalDateTime since = LocalDateTime.now().minusDays(PRACTICE_SUMMARY_LOOKBACK_DAYS);Also applies to: 60-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/home/service/HomeService.java` at line 47, Rename STREAK_LOOKBACK_DAYS to PRACTICE_SUMMARY_LOOKBACK_DAYS and update every reference in recentCompleted and buildPracticeSummary so the constant accurately represents the practice-summary query limit; leave fetchPracticeDates and its unbounded streak calculation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/mr/domain/learning/service/LearningService.java`:
- Around line 241-271: Update HomeService’s recommendation-combination flow, not
LearningService.getRecommendedLearnings, to exclude any recommendation whose
nextStepId matches currentLearning.nextStepId(). Preserve recommendations when
currentLearning or its next step is absent, and add coverage in the relevant
HomeService test for the duplicate-step case.
In `@src/main/java/com/mr/domain/playing/repository/PlayingRepository.java`:
- Around line 54-64: Update findEndedAtsByUserAndStatus to select distinct
endedAt dates, preserving the unlimited date range and existing user, status,
and deletion filters. Use the query’s date-level distinct behavior to prevent
duplicate daily timestamps from being returned to
HomeService.fetchPracticeDates.
In `@src/test/java/com/mr/domain/home/service/HomeServiceTest.java`:
- Around line 202-218: Update getHome_practiceSummary_sumsDurationInHours so its
date fixtures do not cross the current week or month boundaries. Use a fixed
mid-week, mid-month reference date when creating the Playing entries and ensure
the service under test uses that same reference, or calculate expectations from
the actual boundaries; preserve the hour-summing assertions without relying on
LocalDateTime.now().
---
Nitpick comments:
In `@src/main/java/com/mr/domain/home/service/HomeService.java`:
- Line 47: Rename STREAK_LOOKBACK_DAYS to PRACTICE_SUMMARY_LOOKBACK_DAYS and
update every reference in recentCompleted and buildPracticeSummary so the
constant accurately represents the practice-summary query limit; leave
fetchPracticeDates and its unbounded streak calculation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8556c35e-38d4-440a-8c5b-039bb9c860e0
📒 Files selected for processing (15)
src/main/java/com/mr/domain/history/service/HistoryService.javasrc/main/java/com/mr/domain/home/controller/HomeController.javasrc/main/java/com/mr/domain/home/dto/res/HomeResponseDTO.javasrc/main/java/com/mr/domain/home/service/HomeService.javasrc/main/java/com/mr/domain/learning/dto/res/LearningHomeResponseDTO.javasrc/main/java/com/mr/domain/learning/repository/UserLearningProgressRepository.javasrc/main/java/com/mr/domain/learning/service/LearningService.javasrc/main/java/com/mr/domain/playing/repository/PlayingRepository.javasrc/main/java/com/mr/domain/user/repository/StudentInstrumentRepository.javasrc/main/java/com/mr/domain/user/service/UserProfileService.javasrc/main/java/com/mr/global/util/RelativeDateFormatter.javasrc/test/java/com/mr/domain/home/controller/HomeControllerTest.javasrc/test/java/com/mr/domain/home/service/HomeServiceTest.javasrc/test/java/com/mr/domain/learning/service/LearningServiceTest.javasrc/test/java/com/mr/domain/user/service/UserProfileServiceTest.java
| buildStreak(practiceDates), | ||
| buildPracticeSummary(recentCompleted), | ||
| LearningSummary.from(currentLearning), | ||
| buildRecommendedLearnings(userId), |
There was a problem hiding this comment.
이어서 학습하기와 추천학습이 중복 노출될 가능성이 있는데, 어떤 생각이신지 궁금합니다! 사실 로직 추가 안해도 괜찮을 것 같은..
There was a problem hiding this comment.
추천학습 반환에 대한 정확한 로직을 전달 받은 것이 없어 임의로 로직을 짜둔 것이라, 중복은 감안하고 작성했습니다...ㅜ.^ 그냥 제 머리에서 나온 최선의? 추천!? 이었는데 예원님 생각도 궁금합니동.
|
|
||
| long daysBetween = ChronoUnit.DAYS.between(dateTime.toLocalDate(), LocalDate.now()); |
There was a problem hiding this comment.
안뇽하세요. 다른 부분 코드들도 확인해 본 결과 타임존 명시가 되어 있지 않은 곳들이 대부분이라 이쪽에만 타임존 지정을 하면 배포 서버에 이상이 생길 것 같아, 배포된 서버의 타임존에 의존하도록 통일한 다음 / 전체 코드 작성이 완료된 후 일괄적으로 타임존이 필요한 곳을 수정하는 방향을 생각하고 있습니다!
rkdehdrbs7885-oss
left a comment
rkdehdrbs7885-oss
left a comment
There was a problem hiding this comment.
홈 대시보드 화면에 다른 도메인의 데이터가 많아서 쿼리 짜고 DTO 조립하기 번거로우셨을 텐데 정말 고생 많으셨습니다!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/mr/domain/playing/entity/Playing.java (1)
40-47: 🚀 Performance & Scalability | 🔵 Trivial운영 DB에 인덱스 마이그레이션도 반영되는지 확인해 주세요.
@Table(indexes = ...)는 Hibernate가 스키마를 생성·갱신하는 환경에서만 실제 인덱스를 만듭니다. 운영에서 Flyway/Liquibase와ddl-auto=validate또는none을 사용한다면 별도의CREATE INDEX마이그레이션이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/playing/entity/Playing.java` around lines 40 - 47, Playing 엔티티의 idx_playing_user_status_ended_at 인덱스가 운영 DB에도 생성되도록 Flyway/Liquibase 마이그레이션에 user_id, status, ended_at 순서의 CREATE INDEX를 추가하거나 기존 마이그레이션 적용 여부를 확인하세요. 운영 스키마 자동 갱신이 비활성화된 경우에도 해당 마이그레이션이 배포 과정에서 실행되도록 검증하세요.src/main/java/com/mr/domain/playing/repository/PlayingRepository.java (1)
55-63: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift실제 저장소에서 JPQL을 실행하는 테스트를 추가해 주세요.
현재 서비스 테스트는
PlayingRepository를 mock하므로function('date', ...)의 JPQL 파싱, DB 반환 타입,distinct, 삭제·null 필터를 검증하지 못합니다. 운영과 동일한 DB 기반의@DataJpaTest또는 Testcontainers 테스트로 같은 날짜 중복 및 경계 조건을 확인하는 것이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/mr/domain/playing/repository/PlayingRepository.java` around lines 55 - 63, Add a real database-backed `@DataJpaTest` or Testcontainers test for PlayingRepository.findDistinctEndedDatesByUserAndStatus, executing the function('date', p.endedAt) JPQL against the production-equivalent database. Verify duplicate ended timestamps collapse to distinct LocalDate results and that deleted records, null endedAt values, different users, and different statuses are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/mr/domain/playing/repository/PlayingRepository.java`:
- Around line 55-63: The findDistinctEndedDatesByUserAndStatus query still loads
every historical practice date into the JVM. Keep the streak period cap while
limiting database results, preferably by aggregating consecutive days in the
query or fetching dates in descending paginated batches and stopping after the
first missing day; do not rely on the existing index alone to limit result
volume.
---
Nitpick comments:
In `@src/main/java/com/mr/domain/playing/entity/Playing.java`:
- Around line 40-47: Playing 엔티티의 idx_playing_user_status_ended_at 인덱스가 운영 DB에도
생성되도록 Flyway/Liquibase 마이그레이션에 user_id, status, ended_at 순서의 CREATE INDEX를 추가하거나
기존 마이그레이션 적용 여부를 확인하세요. 운영 스키마 자동 갱신이 비활성화된 경우에도 해당 마이그레이션이 배포 과정에서 실행되도록 검증하세요.
In `@src/main/java/com/mr/domain/playing/repository/PlayingRepository.java`:
- Around line 55-63: Add a real database-backed `@DataJpaTest` or Testcontainers
test for PlayingRepository.findDistinctEndedDatesByUserAndStatus, executing the
function('date', p.endedAt) JPQL against the production-equivalent database.
Verify duplicate ended timestamps collapse to distinct LocalDate results and
that deleted records, null endedAt values, different users, and different
statuses are excluded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9dde0608-5f8a-4ad2-a261-ecf06cee3cac
📒 Files selected for processing (4)
src/main/java/com/mr/domain/home/service/HomeService.javasrc/main/java/com/mr/domain/playing/entity/Playing.javasrc/main/java/com/mr/domain/playing/repository/PlayingRepository.javasrc/test/java/com/mr/domain/home/service/HomeServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/mr/domain/home/service/HomeService.java
- src/test/java/com/mr/domain/home/service/HomeServiceTest.java
| // 연속 출석일수는 상한이 없어 기간 제한 없이 날짜 단위 distinct 조회 | ||
| @Query(""" | ||
| select distinct function('date', p.endedAt) from Playing p | ||
| where p.user.userId = :userId | ||
| and p.status = :status | ||
| and p.deletedAt is null | ||
| and p.endedAt is not null | ||
| """) | ||
| List<LocalDate> findDistinctEndedDatesByUserAndStatus( |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
전체 연습 날짜를 매번 JVM으로 로드하는 문제는 남아 있습니다.
distinct로 하루 중복은 해결됐지만 endedAt 하한이 없어 사용자의 모든 연습 날짜를 조회합니다. 홈 조회가 반복될수록 결과 크기와 HashSet 메모리 사용량이 누적됩니다. 스트릭 기간 상한은 유지하되, DB에서 연속 일수를 집계하거나 최신 날짜부터 페이지 단위로 조회해 첫 결석 이후 중단하는 방식이 필요합니다. 인덱스는 스캔을 돕지만 결과 건수 자체는 제한하지 않습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/mr/domain/playing/repository/PlayingRepository.java` around
lines 55 - 63, The findDistinctEndedDatesByUserAndStatus query still loads every
historical practice date into the JVM. Keep the streak period cap while limiting
database results, preferably by aggregating consecutive days in the query or
fetching dates in descending paginated batches and stopping after the first
missing day; do not rely on the existing index alone to limit result volume.
3 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
🔥 리뷰 요청 사항
✅ 체크리스트
📎 참고 사항
리뷰 요청 사항에 적힌 부분을 꼼꼼히 리뷰해 주시면 좋겠습니다.
Summary by CodeRabbit
/api/home)와 응답 스키마를 추가했습니다.