[FEAT] 학습 조회 API 구현 (3/3) - 학습 홈 · 커리큘럼 조회 · 단계별 상세 조회 · 실전 반주법 전체보기 - #55
Merged
Conversation
p1001q
requested review from
kimyw1018,
on1yoneprivate,
ownue and
rkdehdrbs7885-oss
July 26, 2026 01:06
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough학습 홈·커리큘럼·단계 상세·반주법 목록 조회 API를 추가하고, 응답 DTO·저장소 조회·진행률 계산·Swagger 문서·테스트를 확장했습니다. Changes학습 조회 API
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant LearningController
participant LearningService
participant LearningRepository
participant LearningStepRepository
participant UserLearningProgressRepository
Client->>LearningController: 학습 조회 GET 요청
LearningController->>LearningService: 사용자 ID와 학습 식별자 전달
LearningService->>LearningRepository: 활성 학습 및 패키지 조회
LearningService->>LearningStepRepository: 단계 목록 및 총 단계 수 조회
LearningService->>UserLearningProgressRepository: 진행 기록과 완료 단계 집계
LearningService-->>LearningController: 응답 DTO 반환
LearningController-->>Client: ApiResponse 성공 응답
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
p1001q
marked this pull request as draft
July 26, 2026 01:07
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/dto/res/LearningHomeResponseDTO.java`:
- Around line 15-16: Update the `@Schema` description on
LearningHomeResponseDTO.theoryPackages to state that the list contains up to
three items and excludes difficulty levels without available data; do not
describe it as always containing three items.
In `@src/main/java/com/mr/domain/learning/service/LearningService.java`:
- Around line 186-222: Update buildCurrentLearning and its latest-progress
repository query to select only learning records whose associated package is
active, so an inactive most-recent record is skipped in favor of the next active
record or null when none exists. Add tests covering both fallback to the next
active learning and returning null when all recent records are inactive.
🪄 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: 88de6f68-31c9-45d1-bdce-880672272f1d
📒 Files selected for processing (18)
src/main/java/com/mr/domain/learning/controller/LearningController.javasrc/main/java/com/mr/domain/learning/dto/req/LearningResultSaveRequestDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningAccompanimentListResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningCurriculumResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningHomeResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningPracticeDataResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningProgressResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningResultResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningStepDetailResponseDTO.javasrc/main/java/com/mr/domain/learning/dto/res/LearningTheoryListResponseDTO.javasrc/main/java/com/mr/domain/learning/repository/ChordExampleRepository.javasrc/main/java/com/mr/domain/learning/repository/LearningRepository.javasrc/main/java/com/mr/domain/learning/repository/LearningStepRepository.javasrc/main/java/com/mr/domain/learning/repository/UserLearningProgressRepository.javasrc/main/java/com/mr/domain/learning/service/LearningService.javasrc/main/java/com/mr/global/apipayload/ApiResponse.javasrc/test/java/com/mr/domain/learning/controller/LearningControllerTest.javasrc/test/java/com/mr/domain/learning/service/LearningServiceTest.java
p1001q
marked this pull request as ready for review
July 26, 2026 06:18
ownue
reviewed
Jul 26, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/controller/LearningController.java`:
- Around line 121-126: LearningController.getTheoryList의 difficulty 요청 파라미터를 실제
필수 값으로 검증하세요. `@RequestParam의` required 설정을 true로 변경하거나 기본값을 사용해 파라미터 누락 시 서비스 호출
전에 컨트롤러가 400을 반환하도록 Swagger 계약과 구현을 일치시키세요.
🪄 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: 87e87a6e-817d-4c8e-b1dc-3cd888570859
📒 Files selected for processing (5)
src/main/java/com/mr/domain/learning/controller/LearningController.javasrc/main/java/com/mr/domain/learning/dto/res/LearningCurriculumResponseDTO.javasrc/main/java/com/mr/domain/learning/service/LearningService.javasrc/test/java/com/mr/domain/learning/controller/LearningControllerTest.javasrc/test/java/com/mr/domain/learning/service/LearningServiceTest.java
💤 Files with no reviewable changes (1)
- src/test/java/com/mr/domain/learning/controller/LearningControllerTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/mr/domain/learning/dto/res/LearningCurriculumResponseDTO.java
- src/test/java/com/mr/domain/learning/service/LearningServiceTest.java
- src/main/java/com/mr/domain/learning/service/LearningService.java
Comment on lines
121
to
+126
| public ApiResponse<LearningTheoryListResponseDTO.TheoryListResultDTO> getTheoryList( | ||
| @AuthenticationPrincipal CustomUserDetails userDetails, | ||
| @Parameter(description = "BEGINNER | INTERMEDIATE | ADVANCED", required = true) | ||
| @RequestParam(required = false) String difficulty | ||
| ) { | ||
| LearningTheoryListResponseDTO.TheoryListResultDTO result = | ||
| learningService.getTheoryList(userDetails.getUserId(), difficulty); | ||
| learningService.getTheoryList(SecurityUtil.getCurrentUserId(), difficulty); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
difficulty를 실제 요청에서도 필수로 검증해 주세요.
Swagger와 설명은 필수 파라미터로 선언했지만 @RequestParam(required = false)라서 누락 요청이 null로 서비스에 전달됩니다. GET /api/learnings/theory에서 파라미터가 없으면 컨트롤러가 즉시 400을 반환하도록 required = true(기본값)로 맞춰 API 계약과 구현을 일치시키는 편이 안전합니다.
- `@RequestParam`(required = false) String difficulty
+ `@RequestParam` String difficultyPR 목표의 difficulty 필수 계약과 현재 컨트롤러 선언을 대조한 결과입니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public ApiResponse<LearningTheoryListResponseDTO.TheoryListResultDTO> getTheoryList( | |
| @AuthenticationPrincipal CustomUserDetails userDetails, | |
| @Parameter(description = "BEGINNER | INTERMEDIATE | ADVANCED", required = true) | |
| @RequestParam(required = false) String difficulty | |
| ) { | |
| LearningTheoryListResponseDTO.TheoryListResultDTO result = | |
| learningService.getTheoryList(userDetails.getUserId(), difficulty); | |
| learningService.getTheoryList(SecurityUtil.getCurrentUserId(), difficulty); | |
| public ApiResponse<LearningTheoryListResponseDTO.TheoryListResultDTO> getTheoryList( | |
| `@Parameter`(description = "BEGINNER | INTERMEDIATE | ADVANCED", required = true) | |
| `@RequestParam` String difficulty | |
| ) { | |
| LearningTheoryListResponseDTO.TheoryListResultDTO result = | |
| learningService.getTheoryList(SecurityUtil.getCurrentUserId(), difficulty); |
🤖 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/learning/controller/LearningController.java`
around lines 121 - 126, LearningController.getTheoryList의 difficulty 요청 파라미터를 실제
필수 값으로 검증하세요. `@RequestParam의` required 설정을 true로 변경하거나 기본값을 사용해 파라미터 누락 시 서비스 호출
전에 컨트롤러가 400을 반환하도록 Swagger 계약과 구현을 일치시키세요.
Contributor
Author
There was a problem hiding this comment.
확인 감사합니다. 다만 이 부분은 의도적으로 required=false로 둔 것이라 그대로 유지하려 합니다.
GlobalExceptionHandler에 MissingServletRequestParameterException 전용 핸들러가 없는 상태라, required=true로 바꾸면 파라미터 누락 시 @ExceptionHandler(Exception.class) catch-all로 빠져서 400이 아니라 500이 나가게 됩니다.
지금은 required=false로 받은 뒤 LearningService.parseDifficulty()에서 검증해 LEARNING_400_02(400)로 응답하도록 처리하고 있고, 이 계약은 LearningControllerTest.getTheoryList_missingDifficulty_throws400으로 테스트도 돼 있습니다.
private LearningDifficulty parseDifficulty(String difficulty) {
if (difficulty == null) {
throw new GeneralException(LearningErrorStatus.INVALID_DIFFICULTY);
}
...
}There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
# Conflicts: # src/main/java/com/mr/domain/learning/repository/UserLearningProgressRepository.java
kimyw1018
approved these changes
Jul 27, 2026
Comment on lines
+64
to
+74
| public static ProgressInfo of(long completedStepCount, long totalStepCount) { | ||
| int progressRate; | ||
| if (totalStepCount == 0) { | ||
| progressRate = 0; | ||
| } else { | ||
| int rawProgressRate = (int) Math.round((double) completedStepCount / totalStepCount * 100); | ||
| progressRate = Math.min(100, Math.max(0, rawProgressRate)); | ||
| } | ||
| return new ProgressInfo((int) completedStepCount, (int) totalStepCount, progressRate); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
비지니스 로직 일부가 dto에 있는데 이부분 서비스 단으로 빼는건 어떨까요??
사실 상관없을것 같긴 합니다
This was referenced Aug 1, 2026
This was referenced Aug 10, 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.
📍 개요
이슈 #40 작업의 마지막 PR입니다.
PR #50(1/3)에서 학습 주제 전체보기·실습 데이터 조회 2개를 먼저 병합했고
원래 계획했던 후속 PR 2개(2/3, 3/3)는 진행률 계산 로직을 공유하는 API들이라 하나로 묶어 이번 PR(3/3)로 한 번에 올립니다.
⛓️💥 관련 이슈
🛠️ 작업 내용
GET /api/learnings/home(학습 홈 조회): 최근 학습 이어서 하기(currentLearning), 학습 주제 대표 3개(난이도별), 실전 반주법 대표 3개 조회
GET /api/learnings/{learningId}(학습 커리큘럼 조회): 패키지 상세 정보, 전체 진행률(progress), 단계별 목록(상태·점수 포함) 조회
GET /api/learnings/{learningId}/steps/{learningStepId}(학습 단계별 상세 조회): 단계별 이론 설명, 연습 팁, 모범 연주 예시(있으면), 코드 예시 조회
GET /api/learnings/accompaniment(실전 반주법 전체보기): 난이도 구분 없이 제목순으로 전체 목록 조회,
progressRate는 진행률(%)만 반환LearningStepRepository/UserLearningProgressRepository에 배치 집계 쿼리 추가ensureUserExists) 추가 — 홈/주제/반주법 조회와 동작 일치currentLearning에nextStepId추가 —[이어서 학습하기]가 커리큘럼 페이지가 아니라 단계 상세로 바로 이동하도록 확정, 진행률 0%/100%면 카드 자체를 숨기도록 변경is_active=false)된 패키지는 건너뛰고 그다음으로 최근인 활성 패키지를 조회하도록 수정ApiResponse) Swagger@Schema응답/요청 예시값 추가🔥 리뷰 요청 사항
GROUP BY learning.id)로 처리한 방식이, 개별 조회 대비 적절한 트레이드오프인지학습 단계별 상세 조회/단계별 연습 실행 정보 조회는userId를 안 받아서 유저 존재 검증이 구조적으로 불가능한데(로그인은 여전히 필요), 이 판단이 맞는지nextStepId계산 시, 정방향(마지막 학습 단계 뒤)에 미완료가 없으면 패키지 전체에서 가장 이른 미완료로 폴백하는 방식이 타당한지✅ 체크리스트
📎 참고 사항
학습 홈 화면 api를 추가하거나 수정해야돼서 잠깐 막아놨습니다.> 풀었어용Summary by CodeRabbit