[feat] 감상 전체 조회 기능 추가 (#363) #367
Conversation
| package dev.tripdraw.common.domain; | ||
|
|
||
| public record Paging(Long lastViewedId, Integer limit) { | ||
| } |
There was a problem hiding this comment.
해당 클래스가 domain 패키지에 위치하는건 조금 고민해볼 수 있을 것 같아요.
계층을 나누기 위해 Paging과 SearchPaging을 분리하신 것 같습니다!
VO 하나로 통일해보는건 어떠신가요?
There was a problem hiding this comment.
VO 하나로 통일하고, 패키지 분리했습니다!
| SearchConditions searchConditions = postSearchRequest.condition().toSearchConditions(); | ||
| Paging paging = postSearchRequest.paging().toPaging(); |
There was a problem hiding this comment.
만약 지금과 같이 SearchConditions의 계층 분리를 하신다면 getter를 사용한 후 메서드를 호출하는 것 보다 postSearchRequest에서 바로 postSearchRequest.toSearchConditions()이 더 가독성이 좋아보이는데 어떻게 생각하시나요?
아래의 paging도 동일한 의견입니다.
There was a problem hiding this comment.
VO를 하나로 통일하면서 해결됐습니다!
| package dev.tripdraw.post.domain; | ||
|
|
||
| import dev.tripdraw.common.domain.Paging; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public interface PostCustomRepository { | ||
|
|
||
| List<Post> findAllByConditions(SearchConditions conditions, Paging paging); | ||
| } |
There was a problem hiding this comment.
QueryDsl 관련 클래스(repositioy, repositoryImpl)들이 domain 패키지에 있는 부분에 대해 어떻게 생각하시나요? query용 패키지 및 queryService를 따로 분리하는 것도 좋을 것 같습니다.
|
|
||
| @Override | ||
| public List<Post> findAllByConditions(SearchConditions conditions, Paging paging) { | ||
| QPost post = QPost.post; |
There was a problem hiding this comment.
QPost를 변수 추출하지 않고 static import 해보는건 어떠신가요?
| if (lastViewedId == null) { | ||
| return null; | ||
| } | ||
|
|
||
| return post.id.lt(lastViewedId); |
|
|
||
| public record PostsSearchResponse(List<PostSearchResponse> posts, boolean hasNextPage) { | ||
| public static PostsSearchResponse of(List<PostSearchResponse> postSearchResponses, boolean hasNextPage) { | ||
| return new PostsSearchResponse(postSearchResponses, hasNextPage); | ||
| } |
There was a problem hiding this comment.
클라이언트와 hasNextPage에 대한 정보만 제공하기로 합의하셨군요 👍
| PostsSearchResponse postsSearchJejuResponse = postService.readAll(postSearchRequestJeju); | ||
| PostsSearchResponse postsSearchJulyResponse = postService.readAll(postSearchRequestJuly); |
There was a problem hiding this comment.
하나의 메서드에서 여러 개 검증을 하는 것보다 메서드를 분리해서 2개의 테스트 메서드를 작성하는 건 어떨까요?
Repository도 같은 의견입니다!
There was a problem hiding this comment.
같은 GIVEN 조건이라면 하나의 메서드 안에 있는 것을 선호하는 편입니다! 허브는 어떻게 생각하실까요?
| assertThat(postsSearchResponse.posts().get(0).postId()).isEqualTo(jejuSeptember17hourPostResponse.postId()); | ||
| assertThat(postsSearchResponse.posts().get(1).postId()).isEqualTo(jejuAugust17hourPostResponse.postId()); |
There was a problem hiding this comment.
controller나 service 검증부에 id만 검증하게 되는 경우 개인적인 경험으로 휴먼에러로 인해 실제로 데이터가 제공되지 않는 경우가 종종 있었어요. 모든 필드에 대해 검증해보는건 어떨까요?
There was a problem hiding this comment.
일단 ID로 비교하고 추후에 조금 더 추가하겠습니다!
| return value.in(values); | ||
| } | ||
|
|
||
| private BooleanExpression eqAdress(StringPath address, String targetAddress) { |
📌 관련 이슈
📁 작업 설명
💻 미완료 태스크(선택)