-
Notifications
You must be signed in to change notification settings - Fork 3
[DDING-000] revert & feed 전체 조회 로직 수정 #357
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
Conversation
Walkthrough이 PR은 피드 관련 페이징 DTO와 쿼리 타입들을 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
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 (3)
src/main/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepository.java (1)
41-58:getAllFeedPage쿼리: 모든 피드 조회 로직이 올바르게 구현되었습니다이 메서드는 모든 동아리의 피드를 커서 기반으로 페이징하는 설계로, PR 목표(각 동아리의 모든 피드 조회)와 일치합니다. 쿼리 로직은 안정적이고 일관성이 있습니다.
제안: 향후 메서드 이름을
findAllFeedPage로 통일하면findPageByClubIdOrderById와의 네이밍 일관성이 더욱 개선될 것 같습니다.src/main/java/ddingdong/ddingdongBE/domain/form/infrastructure/SesFormResultEmailSender.java (1)
41-41: 설정값을 외부화하는 것을 권장합니다.구성 세트 이름을 하드코딩하면 환경별(개발, 스테이징, 프로덕션) 유연성이 떨어지고 테스트가 어려워집니다. 이전처럼
@Value를 통해 외부 설정으로 관리하는 것이 Spring Boot 모범 사례에 부합합니다.이 변경이 의도적인 것으로 보이는데, 특별한 이유가 있나요? 만약 모든 환경에서 동일한 구성 세트 이름을 사용해야 하는 요구사항이 있다면, 현재 구현도 괜찮지만, 그렇지 않다면 아래와 같이 되돌리는 것을 권장합니다:
🔎 외부 설정으로 되돌리는 제안
클래스 필드에 설정값 추가:
@Value("${cloud.aws.ses.sender-email}") private String senderEmail; + + @Value("${cloud.aws.ses.configuration-set-name}") + private String configurationSetName; private final SesEmailSender sesEmailSender;그리고 line 41을 다음과 같이 수정:
- .configurationSetName("ddingdong-form-application-result-set") + .configurationSetName(configurationSetName)src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/FeedPageResponse.java (1)
10-15: 필드명 일관성 검토 권장레코드 이름이
FeedPageResponse로 변경되었으나 필드명은 여전히newestFeeds입니다. PR 목적이 "동아리별 최신 피드 1개"에서 "모든 피드 조회"로 변경하는 것이므로, 필드명을feeds로 변경하면 의미가 더 명확해집니다.다만, API 응답 JSON 필드명 변경은 클라이언트에 영향을 줄 수 있으므로 의도적인 결정이라면 현재 상태를 유지해도 무방합니다.
🔎 필드명 변경 제안
public record FeedPageResponse( - @ArraySchema(schema = @Schema(name = "동아리 최신 피드 정보", implementation = FeedListResponse.class)) - List<FeedListResponse> newestFeeds, + @ArraySchema(schema = @Schema(name = "동아리 피드 정보", implementation = FeedListResponse.class)) + List<FeedListResponse> feeds, @Schema(name = "피드 페이지 정보", implementation = PagingResponse.class) PagingResponse pagingInfo ) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/ddingdong/ddingdongBE/domain/feed/api/FeedApi.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/controller/FeedController.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/FeedPageResponse.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepository.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedPageQuery.javasrc/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/NewestFeedPerClubPageQuery.javasrc/main/java/ddingdong/ddingdongBE/domain/form/infrastructure/SesFormResultEmailSender.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/FeedFixture.javasrc/test/java/ddingdong/ddingdongBE/common/fixture/VodProcessingJobFixture.javasrc/test/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepositoryTest.java
💤 Files with no reviewable changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/NewestFeedPerClubPageQuery.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedPageQuery.java (1)
src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/MyFeedPageQuery.java (1)
MyFeedPageQuery(6-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_push
🔇 Additional comments (10)
src/test/java/ddingdong/ddingdongBE/common/fixture/FeedFixture.java (1)
32-32: 파일 끝에 개행 추가 - 승인합니다.파일 끝에 개행을 추가한 부분입니다. 코드 스타일 정리 차원에서 적절한 변경입니다. 기존 팩토리 메서드들은 그대로 유지되며 기능적 변화가 없습니다.
src/test/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepositoryTest.java (1)
216-220: LGTM!클럽 변수 선언과 피드 변수 선언 사이에 빈 줄을 추가하여 가독성을 개선한 단순 포맷팅 변경입니다. 기능적 변경 사항은 없습니다.
src/main/java/ddingdong/ddingdongBE/domain/feed/repository/FeedRepository.java (2)
35-39: 파라미터 포맷팅 개선 승인코드 가독성을 위한 파라미터 포맷팅 변경이 잘 적용되었습니다.
Also applies to: 59-62
11-34:findPageByClubIdOrderById쿼리: 커서 기반 페이징 구현이 적절합니다Cursor-based pagination 구현이 올바릅니다.
id필드는 indexed 고유값이므로 cursor로 적합하며, DESC 정렬로 최신 데이터부터 조회합니다. 삭제된 피드 필터링 및 VOD 처리 상태 검증도 명확합니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/FacadeFeedService.java (1)
39-51: LGTM!
NewestFeedPerClubPageQuery에서FeedPageQuery로의 타입 리네이밍이 일관되게 적용되었습니다. 메서드 시그니처, empty 생성, 팩토리 메서드 호출 모두 올바르게 변경되었습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/service/dto/query/FeedPageQuery.java (1)
1-18: LGTM!
FeedPageQuery레코드가 기존MyFeedPageQuery와 동일한 패턴으로 구현되어 코드베이스 전반의 일관성을 유지합니다.of()팩토리 메서드와createEmpty()헬퍼가 적절하게 구현되었습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/api/FeedApi.java (1)
33-41: API 응답 타입 변경 확인
NewestFeedPerClubPageResponse에서FeedPageResponse로 API 응답 타입이 변경되었습니다. Swagger 스키마도 올바르게 업데이트되었습니다.API를 사용하는 클라이언트(프론트엔드 등)에서 응답 타입 변경에 대응했는지 확인이 필요합니다.
src/main/java/ddingdong/ddingdongBE/domain/feed/controller/FeedController.java (1)
30-37: LGTM!컨트롤러가 업데이트된
FeedApi인터페이스를 올바르게 구현합니다.FeedPageQuery와FeedPageResponse타입 사용이 서비스 레이어 및 API 레이어와 일관성 있게 적용되었습니다.src/main/java/ddingdong/ddingdongBE/domain/feed/controller/dto/response/FeedPageResponse.java (2)
17-25: LGTM!
FeedPageQuery를 받아FeedPageResponse로 변환하는 팩토리 메서드가 올바르게 구현되었습니다. 스트림 매핑과PagingResponse생성 로직이 적절합니다.
27-50: LGTM!
FeedListResponse내부 레코드가 빌더 패턴을 사용하여 깔끔하게 구현되었습니다.FeedListQuery에서 응답 객체로의 매핑이 정확합니다.
Gopistol
left a comment
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.
고생하셨습니다!
(cherry picked from commit 207b7f7)
🚀 작업 내용
전체 동아리 대상 피드 조회
before - 각 동아리당 1개 최신 피드만 조회
after - 동아리의 모든 피드를 조회
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.