[FEAT] 히스토리 목록/상세 조회 API 구현 - #52
Conversation
|
Warning Review limit reached
Next review available in: 45 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 (1)
📝 WalkthroughWalkthrough사용자의 완료된 연습 기록을 기간별·페이지별로 조회하는 목록 API와 특정 기록의 분석 결과를 조회하는 상세 API가 추가되었습니다. 저장소 쿼리, 응답 DTO, 검증·예외 처리 및 서비스·컨트롤러 테스트가 함께 구현되었습니다. Changes히스토리 조회 기능
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HistoryController
participant HistoryService
participant Repositories
Client->>HistoryController: GET /api/histories
HistoryController->>HistoryService: 사용자 ID와 조회 조건 전달
HistoryService->>Repositories: 완료 Playing 및 Analysis 조회
Repositories-->>HistoryService: 페이징 재생 기록과 최신 분석
HistoryService-->>HistoryController: HistoryListResponseDTO
HistoryController-->>Client: 성공 응답
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
p1001q
left a comment
There was a problem hiding this comment.
은우님 리뷰 요청 답변
1. 에러코드 체계(HISTORY_400_01/02, 403_01, 404_01, 409_01) 적절한지
→ 문제없어요. 5개 다 실제로 throw되고 있고(제가 지난번 리뷰한 PR들은 정의만 해두고 안 쓰는 경우가 많았는데 이건 전부 연결돼있음), HTTP 상태도 상황에 맞게 잘 골랐어요(권한 없음=403, 완료 안 됨=409로 구분한 것도 적절).
2. Analysis 소유권 쿼리 조건 방어가 충분한지
→ 충분해 보여요. Playing.user는 진짜 FK 관계라 validateOwner()에서 먼저 확실하게 걸러지고, 그 다음 Analysis 쿼리의 userId 조건은 "혹시 모를 데이터 불일치에 대한 추가 방어"라 최악의 경우에도 "있어야 할 분석이 안 보이는" 방향이지 "남의 데이터가 새는" 방향은 아니에요. FK 연결 전까지는 이 정도면 안전한 선택이라고 봐요.
3. scoreChange 같은 페이지 내 인접 비교만 하는 게 괜찮은지
→ 기능적으로는 문제없지만 UX적으로 좀 애매한 지점이 있어요: 페이지의 마지막 항목은 실제로는 이전 기록이 있어도(다음 페이지에) 항상 scoreChange: null로 나가요. 페이지 경계가 실제 데이터랑 무관한 곳에서 정보 누락을 만드는 셈이라, 사용자 입장에선 "왜 이 항목만 변화량이 안 보이지" 싶을 수 있어요. 당장 문제는 아니니 이대로 가도 되지만, 나중에 필요하면 size+1개를 조회해서 마지막 항목 비교용으로만 쓰고 응답에선 빼는 방식으로 개선할 수 있어요.
4. AUTO_STOPPED 히스토리 노출 방식
→ 이건 순수 프로덕트 결정이라 디스코드 스레드에서 피엠님과 논의해보는 거 어떤가용
승인해드렸습니다! 수고 많으셨어용
리뷰한 것중에 엉터리 리뷰가 있다면 편하게 무시해주세요 ㄱ ㅡ...
- HistoryPeriod.RECENT 케이스에 정보성 주석 추가 - relativeDate를 피그마 명세대로 수정 - durationSec을 목록/상세 응답에 추가
|
수연님 리뷰 중 3 / 4 관련 -> 3은 UX적 문제가 있는 것은 확인했으나 디벨롭 기간에 진행하는 게 나을 듯해 (시간 이슈...^^) 서브 이슈로 관리하겠습니다. -> 4는 별도 스레드 파서 논의 후 수정할 점이 생기면 반영하겠습니다. |
- develop 브랜치 반영이 늦어 생긴 오류라 merge 후 필드명 수정
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
GET /api/histories,GET /api/histories/{playingId})PlayingRepository신설 및AnalysisRepository배치 조회 메서드 추가🔥 리뷰 요청 사항
HISTORY_400_01/400_02/403_01/404_01/409_01) 부여가 적절한지Analysis가 unlinked 컬럼(FK 아님)이라 소유권을 쿼리 조건으로 방어한 방식이 충분한지 (추후 별도 이슈에서 FK로 연결 후 변경 예정)scoreChange를 같은 페이지 내 인접 항목끼리만 비교하도록 단순화한 게 괜찮은지AUTO_STOPPED상태 연주는 지금 히스토리에서 계속 제외되는데, 히스토리에 정확히 어떤 상태값을 남기면 될지/지금처럼 유지해도 되는지✅ 체크리스트
📎 참고 사항
totalBars,estimatedSeconds는 현재 저장된 소스가 없어null로 반환하고 있습니다!totalBars는playtimeSec/bpm으로 추정 계산도 가능하다고 하는데, 정확도 이슈가 있어 넣을지는 논의 후 결정할 예정입니다. 디스코드에 스레드를 파두었으니 확인 후 답변 부탁드립니다...!Analysis.userId/playingId를 실제 FK로 전환하는 작업은 이번 PR 범위 밖이라 별도 이슈로 분리할 예정입니다. 대신 이번 PR에서는 조회 쿼리에userId조건을 직접 걸어 방어 처리를 했습니다.Summary by CodeRabbit