-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DDING-80] 활동보고서 현재 회차 조회 API 로직 수정 및 페이지네이션 로직 수정 #220
Conversation
Warning Rate limit exceeded@5uhwann has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
워크스루이 풀 리퀘스트는 여러 클래스에 대한 변경 사항을 포함하고 있습니다. 변경 사항
관련 가능성 있는 풀 리퀘스트
제안된 라벨
제안된 리뷰어
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/entity/VodProcessingJob.java (1)
65-66
: 메서드 변경이 잘 이루어졌습니다!상태 확인 로직이 단순화되어 가독성이 향상되었고, 메서드 이름도 실제 동작을 더 정확하게 반영합니다.
JavaDoc 추가를 제안드립니다.
메서드의 목적과 반환값에 대한 설명을 JavaDoc으로 추가하면 좋을 것 같습니다.
+ /** + * VOD 처리 작업이 완료되었는지 확인합니다. + * + * @return 변환 작업 상태가 COMPLETE인 경우 true, 그렇지 않은 경우 false + */ public boolean isCompleted() { return this.convertJobStatus == ConvertJobStatus.COMPLETE; }src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java (3)
41-42
: 예외 메시지를 더 상세하게 개선하면 좋을 것 같습니다.예외 메시지에 피드의 ID 외에도 추가적인 컨텍스트 정보를 포함하면 디버깅에 도움이 될 것 같습니다.
- .orElseThrow(() -> new ResourceNotFound("Feed(id: " + feedId + ")를 찾을 수 없습니다.")); + .orElseThrow(() -> new ResourceNotFound(String.format("Feed를 찾을 수 없습니다. (feedId: %d, 요청 시각: %s)", + feedId, LocalDateTime.now())));
64-66
: 스트림 체이닝 최적화를 제안드립니다.현재 구현은 새로운 ArrayList를 생성한 후 다시 스트림으로 변환하고 있습니다. 불필요한 객체 생성을 줄일 수 있습니다.
- List<Feed> content = new ArrayList<>(originalSlice.getContent()).stream() - .filter(this::isComplete) - .collect(Collectors.toList()); + List<Feed> content = originalSlice.getContent() + .stream() + .filter(this::isComplete) + .collect(Collectors.toList());
80-90
: null 체크 로직을 Optional을 사용하여 개선하면 좋을 것 같습니다.현재 구현은 null 체크를 직접 수행하고 있는데, Optional을 활용하면 더 안전하고 가독성 있는 코드가 될 것 같습니다.
private boolean isComplete(Feed feed) { if (feed.isImage()) { return true; } - VodProcessingJob vodProcessingJob = vodProcessingJobService.findByVideoFeedId(feed.getId()); - if (vodProcessingJob == null) { - return false; - } - return vodProcessingJob.isCompleted(); + return Optional.ofNullable(vodProcessingJobService.findByVideoFeedId(feed.getId())) + .map(VodProcessingJob::isCompleted) + .orElse(false); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java
(5 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/entity/VodProcessingJob.java
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (7)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeClubFeedServiceImpl.java (2)
79-82
: 피드 필터링 로직 변경에 대한 검토 필요피드 완료 여부를 확인하는 로직이 제거되었습니다. 이로 인해 처리되지 않은 피드가 사용자에게 표시될 수 있습니다. 이 변경사항이 비즈니스 요구사항과 일치하는지 확인이 필요합니다.
다음 사항들을 확인해주세요:
- 미완료된 피드가 표시되어도 괜찮은지
- 프론트엔드에서 이러한 변경을 처리할 준비가 되어있는지
- 사용자 경험에 부정적인 영향은 없는지
92-96
: 코드 포맷팅 개선이 잘 되었습니다SseEvent 생성 부분의 들여쓰기가 개선되어 가독성이 향상되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/feed/service/GeneralFeedService.java (1)
6-7
: 의존성 주입이 올바르게 구현되었습니다!Spring의 생성자 주입 패턴을 잘 따르고 있으며, 필요한 import문도 적절히 추가되었습니다.
Also applies to: 25-25
src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java (4)
22-24
: 의존성 제거로 인한 책임 분리가 개선되었습니다.VodProcessingJobService 의존성이 제거되어 피드 서비스의 책임이 더 명확해졌습니다. 이는 코드의 응집도를 높이고 결합도를 낮추는 좋은 변경사항입니다.
39-50
: 코드 구조가 개선되었습니다.피드 처리 로직이 단순화되어 코드의 가독성과 유지보수성이 향상되었습니다. 페이지네이션 처리도 적절하게 구현되어 있습니다.
53-57
: 통합 테스트 검증이 권장됩니다.VodProcessingJobService 제거로 인한 간접적인 영향이 있을 수 있으므로, 다음 사항들의 테스트가 권장됩니다:
- 피드 상세 정보 조회 기능
- 클럽 정보 추출 기능
- 파일 정보 추출 기능
25-36
: 피드 상태 검증 로직 확인이 필요합니다.완료 상태 확인 로직이 제거되었는데, 이로 인해 미완료된 피드가 노출될 가능성이 있습니다. 다음 사항들을 확인해 주시기 바랍니다:
- feedService.getFeedPageByClubId에서 완료된 피드만 반환하는지
- 미완료 피드 노출 시 UI/UX 영향도
Quality Gate failedFailed conditions |
🚀 작업 내용
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
새로운 기능
개선 사항