In 367 영상 상세정보 응답 수정#221
Merged
Hidden character warning
The head ref may contain hidden characters: "in-367-\uc601\uc0c1-\uc0c1\uc138\uc815\ubcf4-\uc751\ub2f5-\uc218\uc815"
Merged
Conversation
Walkthrough채널 상위/신규구독 영상 조회 API의 파라미터가 contentType에서 선택적 filter로 변경되고, 채널 서비스에 parseFilter 헬퍼가 추가되었다. 영상 통계 응답의 changeRate 계산이 채널 평균(VideoStatsChannelAverages) 기반으로 재구성되었고, retention 관련 예외가 RETENTION_NOT_FOUND에서 ANALYTICS_DATA_NOT_FOUND로 통합되었다. Changes채널 영상 filter 파라미터
Estimated code review effort: 3 (Moderate) | ~25 minutes 영상 통계 평균 및 retention 예외 통합
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ChannelController
participant ChannelService
Client->>ChannelController: GET /tops?filter=SHORT
ChannelController->>ChannelService: getTopVideos(channelId, filter)
ChannelService->>ChannelService: parseFilter(filter)
ChannelService-->>ChannelController: ChannelTopVideosResponse
ChannelController-->>Client: BaseResponse
sequenceDiagram
participant VideoService
participant VideoStatsRepository
participant VideoStatsResponse
VideoService->>VideoStatsRepository: findChannelAverages(channelId)
VideoStatsRepository-->>VideoService: VideoStatsChannelAverages
VideoService->>VideoStatsResponse: from(stats, analytics, averages)
VideoStatsResponse->>VideoStatsResponse: changeRate(value, average)
VideoStatsResponse-->>VideoService: VideoStatsResponse
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🧹 Nitpick comments (1)
src/main/java/com/example/inflace/domain/video/dto/VideoStatsResponse.java (1)
30-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winchangeRate 계산식은 DTO 밖으로 옮기는 게 좋겠습니다.
VideoStatsResponse가 응답 조립을 넘어 채널 평균 대비 증감률 계산 규칙까지 갖게 됐습니다. 최소한 계산식은AnalyticsCalculator로 이동하고, DTO는 값 포장만 담당하게 해주세요. Based on learnings, DTOs underdto/packages should remain pure data carriers and business/validation logic should live in services.🤖 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/example/inflace/domain/video/dto/VideoStatsResponse.java` around lines 30 - 69, `VideoStatsResponse` is doing business logic by calculating change rates in `changeRate`, so move that percentage calculation into `AnalyticsCalculator` and keep the DTO focused on assembling `StatValue` objects. Update `VideoStatsResponse.from(...)` to call the shared calculator method for each average-based change rate, and remove the local `changeRate` helper so the DTO remains a pure data carrier.Source: Learnings
🤖 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/example/inflace/domain/video/repository/VideoStatsRepository.java`:
- Around line 16-39: The aggregate query in VideoStatsRepository is treating
missing VideoStats and VideoAnalytics rows as zero by using coalesce(...) inside
the averages, which pulls incomplete videos into the channel metrics; update the
`@Query` in VideoStatsRepository so averages ignore null stats instead of
defaulting them to 0, and only apply default values when mapping the result into
VideoStatsChannelAverages or another response-layer DTO. Keep the ratio
expressions in the same query but ensure the Video and left-joined vs/va fields
used there are handled in a way that excludes absent stats from the averages.
---
Nitpick comments:
In `@src/main/java/com/example/inflace/domain/video/dto/VideoStatsResponse.java`:
- Around line 30-69: `VideoStatsResponse` is doing business logic by calculating
change rates in `changeRate`, so move that percentage calculation into
`AnalyticsCalculator` and keep the DTO focused on assembling `StatValue`
objects. Update `VideoStatsResponse.from(...)` to call the shared calculator
method for each average-based change rate, and remove the local `changeRate`
helper so the DTO remains a pure data carrier.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5eb99df-66df-4958-8dc1-40cbf79d04b8
📒 Files selected for processing (13)
src/main/java/com/example/inflace/domain/channel/controller/ChannelApi.javasrc/main/java/com/example/inflace/domain/channel/controller/ChannelController.javasrc/main/java/com/example/inflace/domain/channel/service/ChannelService.javasrc/main/java/com/example/inflace/domain/video/controller/VideoApi.javasrc/main/java/com/example/inflace/domain/video/dto/RetentionSummaryResponse.javasrc/main/java/com/example/inflace/domain/video/dto/VideoStatsChannelAverages.javasrc/main/java/com/example/inflace/domain/video/dto/VideoStatsResponse.javasrc/main/java/com/example/inflace/domain/video/repository/VideoRepository.javasrc/main/java/com/example/inflace/domain/video/repository/VideoStatsRepository.javasrc/main/java/com/example/inflace/domain/video/service/VideoService.javasrc/main/java/com/example/inflace/global/exception/ErrorDefine.javasrc/main/java/com/example/inflace/global/util/AnalyticsParser.javasrc/test/java/com/example/inflace/domain/video/service/VideoServiceTest.java
Comment on lines
+16
to
+39
| @Query(""" | ||
| select new com.example.inflace.domain.video.dto.VideoStatsChannelAverages( | ||
| avg(coalesce(vs.viewCount, 0)), | ||
| avg(coalesce(vs.likeCount, 0)), | ||
| avg(coalesce(vs.commentCount, 0)), | ||
| avg(coalesce(va.shareCount, 0)), | ||
| avg(coalesce(va.subscribersGained, 0)), | ||
| avg(coalesce(va.avgWatchDuration, 0.0)), | ||
| avg(case | ||
| when coalesce(vs.viewCount, 0) = 0 then 0.0 | ||
| else ((coalesce(vs.likeCount, 0) + coalesce(vs.commentCount, 0)) * 100.0 / vs.viewCount) | ||
| end), | ||
| avg(case | ||
| when coalesce(vs.viewCount, 0) = 0 then 0.0 | ||
| else (coalesce(va.unsubscribedViewCount, 0) * 100.0 / vs.viewCount) | ||
| end), | ||
| avg(coalesce(vs.outlierScore, 0.0)), | ||
| avg(coalesce(vs.vph, 0.0)) | ||
| ) | ||
| from Video v | ||
| left join VideoStats vs on vs.video = v | ||
| left join VideoAnalytics va on va.video = v | ||
| where v.channel.id = :channelId | ||
| """) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 결측 VideoStats/VideoAnalytics가 있는 채널 평균 테스트가 있는지 확인합니다.
rg -n "findChannelAverages|VideoStatsChannelAverages|avg\\(coalesce|missing.*analytics|missing.*stats" src/main/java src/test/javaRepository: JECT-Study/inflace-4th-server
Length of output: 2298
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/main/java/com/example/inflace/domain/video/repository/VideoStatsRepository.java
printf '\n---\n'
sed -n '1,220p' src/main/java/com/example/inflace/domain/video/dto/VideoStatsChannelAverages.java
printf '\n---\n'
sed -n '1,220p' src/main/java/com/example/inflace/domain/video/dto/VideoStatsResponse.java
printf '\n---\n'
sed -n '1,220p' src/main/java/com/example/inflace/domain/video/service/VideoService.java
printf '\n---\n'
rg -n "findChannelAverages|VideoStatsChannelAverages|changeRate|avg\\(coalesce|coalesce\\(avg" src/test/java src/main/javaRepository: JECT-Study/inflace-4th-server
Length of output: 16663
결측 통계는 평균에서 제외하세요. left join된 영상에 coalesce(..., 0)를 적용하면 VideoStats/VideoAnalytics가 아직 없는 영상도 0으로 채널 평균에 섞여 changeRate가 실제보다 낮아집니다. 평균은 null을 무시하도록 계산하고, 0 기본값은 응답 단계에서만 채우는 편이 안전합니다.
🤖 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/example/inflace/domain/video/repository/VideoStatsRepository.java`
around lines 16 - 39, The aggregate query in VideoStatsRepository is treating
missing VideoStats and VideoAnalytics rows as zero by using coalesce(...) inside
the averages, which pulls incomplete videos into the channel metrics; update the
`@Query` in VideoStatsRepository so averages ignore null stats instead of
defaulting them to 0, and only apply default values when mapping the result into
VideoStatsChannelAverages or another response-layer DTO. Keep the ratio
expressions in the same query but ensure the Video and left-joined vs/va fields
used there are handled in a way that excludes absent stats from the averages.
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
2 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.
Issue
closed #219
comment
Summary by CodeRabbit
New Features
Bug Fixes