-
Notifications
You must be signed in to change notification settings - Fork 2
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
[BE] feat: 내가 받은 리뷰 목록에 페이지네이션 적용 #683
Changes from 31 commits
b85e6d2
b8b3666
8c238a2
2b3bb9a
34077d6
79b4bda
5c30f34
787e707
fc28d73
f0fa9c6
8443181
ecb444b
d2064d4
34702c7
a530a67
1ac7f16
88eea36
1dceecb
b865dce
89cdb58
b825f1b
2b6f268
622ea23
840f337
c793331
d40ded6
9364818
ea61130
cb1c797
d4d546c
3dcaeaf
1548304
5076911
b83f55f
708afd2
8bc24d2
cb64bb0
7588097
e86f4df
fc5f345
7954cc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,5 +17,16 @@ public interface ReviewRepository extends JpaRepository<Review, Long> { | |
""", nativeQuery = true) | ||
List<Review> findAllByGroupId(long reviewGroupId); | ||
|
||
@Query(value = """ | ||
SELECT r.* FROM review r | ||
WHERE r.review_group_id = :reviewGroupId | ||
AND r.id < :lastReviewId | ||
ORDER BY r.created_at DESC | ||
LIMIT :limit | ||
""", nativeQuery = true) | ||
List<Review> findByReviewGroupIdWithLimit(long reviewGroupId, long lastReviewId, int limit); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AND (:lastReviewId IS NULL OR r.id < :lastReviewId) 이렇게 하면 LastReviewId 클래스 필요 없을 것 같아요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오! 적용해볼게요! |
||
|
||
Optional<Review> findByIdAndReviewGroupId(long reviewId, long reviewGroupId); | ||
|
||
int countByReviewGroupId(long id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용하지 않습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 삭제했습니다! |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package reviewme.review.service; | ||
|
||
import lombok.Getter; | ||
|
||
@Getter | ||
public class LastReviewId { | ||
|
||
private static final long DEFAULT_LAST_REVIEW_ID = Long.MAX_VALUE; | ||
|
||
private final long id; | ||
|
||
LastReviewId(Long id) { | ||
if (id == null) { | ||
this.id = DEFAULT_LAST_REVIEW_ID; | ||
return; | ||
} | ||
this.id = id; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package reviewme.review.service; | ||
|
||
import lombok.Getter; | ||
|
||
@Getter | ||
public class PageSize { | ||
|
||
private static final int DEFAULT_SIZE = 5; | ||
private static final int MAX_SIZE = 50; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 두가지가 도메인의 특성보다는 정책적인 특성이라고 생각해서(=변경 가능성 있음) 서비스로 올리고 주입받는 건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 설득완! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 디폴트가 5인 이유는 무엇인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가로 ReviewListLookupService 에서 리뷰 목록 조회만의 기능을 가지고 있는데 따로 클래스로 분리한 이유가 있을까요?? 안에서 해도 될 것 같다는 생각.. |
||
|
||
private final int size; | ||
|
||
PageSize(Integer size) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 default로 해도 되겠곤요👍 |
||
if (size == null || size < 1 || size > MAX_SIZE) { | ||
this.size = DEFAULT_SIZE; | ||
return; | ||
} | ||
this.size = size; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
import reviewme.review.repository.ReviewRepository; | ||
import reviewme.review.service.dto.response.list.ReceivedReviewsResponse; | ||
import reviewme.review.service.dto.response.list.ReviewListElementResponse; | ||
import reviewme.review.service.exception.ReviewGroupNotFoundByReviewRequestCodeException; | ||
|
@@ -17,15 +18,28 @@ public class ReviewListLookupService { | |
|
||
private final ReviewGroupRepository reviewGroupRepository; | ||
private final ReviewListMapper reviewListMapper; | ||
private final ReviewRepository reviewRepository; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 사용하지 않는 것 같슴다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영했습니다🫡 |
||
|
||
@Transactional(readOnly = true) | ||
public ReceivedReviewsResponse getReceivedReviews(String reviewRequestCode) { | ||
public ReceivedReviewsResponse getReceivedReviews(Long lastReviewId, Integer size, String reviewRequestCode) { | ||
ReviewGroup reviewGroup = reviewGroupRepository.findByReviewRequestCode(reviewRequestCode) | ||
.orElseThrow(() -> new ReviewGroupNotFoundByReviewRequestCodeException(reviewRequestCode)); | ||
|
||
List<ReviewListElementResponse> reviewGroupResponse = reviewListMapper.mapToReviewList(reviewGroup); | ||
LastReviewId lastId = new LastReviewId(lastReviewId); | ||
PageSize pageSize = new PageSize(size); | ||
List<ReviewListElementResponse> reviewListResponse | ||
= reviewListMapper.mapToReviewList(reviewGroup, lastId.getId(), pageSize.getSize()); | ||
int totalSize = reviewRepository.countByReviewGroupId(reviewGroup.getId()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. totalSize를 먼저 확인해서 0일경우 별다른 로직을 타지 않고 반환하는건 어때요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
long newLastReviewId = calculateLastReviewId(reviewListResponse); | ||
return new ReceivedReviewsResponse( | ||
reviewGroup.getReviewee(), reviewGroup.getProjectName(), reviewGroupResponse | ||
reviewGroup.getReviewee(), reviewGroup.getProjectName(), totalSize, newLastReviewId, reviewListResponse | ||
); | ||
} | ||
|
||
private long calculateLastReviewId(List<ReviewListElementResponse> elements) { | ||
if (elements.isEmpty()) { | ||
return 0; | ||
} | ||
return elements.get(elements.size() - 1).reviewId(); | ||
} | ||
Comment on lines
+35
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 책임 가질 객체 나올 것 같은데 나중에 고민해볼게요 |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ | |
public record ReceivedReviewsResponse( | ||
String revieweeName, | ||
String projectName, | ||
int totalSize, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이거 반영하려면 프론트와 상의해야 해요 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요청-응답간에 크기를 나타내는 용어로 'size'가 사용되고 있어서 이것도 size 로 끝나면 좋겠어요🥹 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. size도 count도 지금 보면 괜찮은 듯 하기도 하고... |
||
long lastReviewId, | ||
List<ReviewListElementResponse> reviews | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,10 @@ public class ReviewListMapper { | |
|
||
private final ReviewPreviewGenerator reviewPreviewGenerator = new ReviewPreviewGenerator(); | ||
|
||
public List<ReviewListElementResponse> mapToReviewList(ReviewGroup reviewGroup) { | ||
public List<ReviewListElementResponse> mapToReviewList(ReviewGroup reviewGroup, | ||
long lastReviewId, int size) { | ||
List<OptionItem> categoryOptionIds = optionItemRepository.findAllByOptionType(OptionType.CATEGORY); | ||
return reviewRepository.findAllByGroupId(reviewGroup.getId()) | ||
return reviewRepository.findByReviewGroupIdWithLimit(reviewGroup.getId(), lastReviewId, size) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ~WithLimit 보다는 페이지네이션을 명시해서 확실하게 사용처를 알 수 있게 해도 좋을 것 같습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 pagination을 사용하는 경우가 있고 없고, 이렇다면 pagination이 명시되는게 맞다고 생각하지만! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이하동문입니다~.~ |
||
.stream() | ||
.map(review -> mapToReviewListElementResponse(review, categoryOptionIds)) | ||
.toList(); | ||
|
@@ -43,7 +44,7 @@ private ReviewListElementResponse mapToReviewListElementResponse(Review review, | |
} | ||
|
||
private List<ReviewCategoryResponse> mapToCategoryOptionResponse(Review review, | ||
List<OptionItem> categoryOptionItems) { | ||
List<OptionItem> categoryOptionItems) { | ||
Set<Long> checkBoxOptionIds = review.getAllCheckBoxOptionIds(); | ||
return categoryOptionItems.stream() | ||
.filter(optionItem -> checkBoxOptionIds.contains(optionItem.getId())) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
package reviewme.review.repository; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.junit.jupiter.api.Assertions.*; | ||
import static reviewme.fixture.QuestionFixture.서술형_필수_질문; | ||
import static reviewme.fixture.ReviewGroupFixture.리뷰_그룹; | ||
import static reviewme.fixture.SectionFixture.항상_보이는_섹션; | ||
import static reviewme.fixture.TemplateFixture.템플릿; | ||
|
||
import java.util.List; | ||
import org.junit.jupiter.api.Nested; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
|
@@ -59,4 +59,83 @@ class ReviewRepositoryTest { | |
// then | ||
assertThat(actual).containsExactly(review2, review1); | ||
} | ||
|
||
@Nested | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복 데이터 삽입을 안해도 되는 것 좋네요👍👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 하하~ 신경 좀 썼습니다 Nested 아름답죵? ㅎㅎ |
||
class 리뷰그룹_아이디에_해당하는_리뷰를_생성일_기준_내림차순으로_페이징하여_불러온다 { | ||
|
||
private final Question question = questionRepository.save(서술형_필수_질문()); | ||
private final Section section = sectionRepository.save(항상_보이는_섹션(List.of(question.getId()))); | ||
private final Template template = templateRepository.save(템플릿(List.of(section.getId()))); | ||
private final ReviewGroup reviewGroup = reviewGroupRepository.save(리뷰_그룹()); | ||
|
||
private final Review review1 = reviewRepository.save( | ||
new Review(template.getId(), reviewGroup.getId(), null, null)); | ||
private final Review review2 = reviewRepository.save( | ||
new Review(template.getId(), reviewGroup.getId(), null, null)); | ||
private final Review review3 = reviewRepository.save( | ||
new Review(template.getId(), reviewGroup.getId(), null, null)); | ||
|
||
|
||
@Test | ||
void 페이징_크기보다_적은_수의_리뷰가_등록되었으면_그_크기만큼의_리뷰만_반환한다() { | ||
// given | ||
int limit = 5; | ||
long lastReviewId = Long.MAX_VALUE; | ||
|
||
// when | ||
List<Review> actual = reviewRepository.findByReviewGroupIdWithLimit( | ||
reviewGroup.getId(), lastReviewId, limit); | ||
|
||
// then | ||
assertThat(actual) | ||
.hasSize(3) | ||
.containsExactly(review3, review2, review1); | ||
} | ||
|
||
@Test | ||
void 페이징_크기보다_큰_수의_리뷰가_등록되었으면_페이징_크기만큼의_리뷰를_반환한다() { | ||
// given | ||
int limit = 2; | ||
long lastReviewId = Long.MAX_VALUE; | ||
|
||
// when | ||
List<Review> actual = reviewRepository.findByReviewGroupIdWithLimit( | ||
reviewGroup.getId(), lastReviewId, limit); | ||
|
||
// then | ||
assertThat(actual) | ||
.hasSize(2) | ||
.containsExactly(review3, review2); | ||
} | ||
|
||
@Test | ||
void 마지막_리뷰_아이디를_기준으로_그보다_전에_적힌_리뷰를_반환한다() { | ||
// given | ||
int limit = 5; | ||
long lastReviewId = review3.getId(); | ||
|
||
// when | ||
List<Review> actual = reviewRepository.findByReviewGroupIdWithLimit( | ||
reviewGroup.getId(), lastReviewId, limit); | ||
|
||
// then | ||
assertThat(actual) | ||
.hasSize(2) | ||
.containsExactly(review2, review1); | ||
} | ||
|
||
@Test | ||
void 마지막으로_온_리뷰_전에_작성된_리뷰가_없으면_빈_리스트를_반환한다() { | ||
// given | ||
int limit = 5; | ||
long lastReviewId = review1.getId(); | ||
|
||
// when | ||
List<Review> actual = reviewRepository.findByReviewGroupIdWithLimit( | ||
reviewGroup.getId(), lastReviewId, limit); | ||
|
||
// then | ||
assertThat(actual).isEmpty(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5초컷 given,when,then 추가 요청😋 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영완! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package reviewme.review.service; | ||
|
||
import static org.junit.jupiter.api.Assertions.*; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
||
class LastReviewIdTest { | ||
|
||
@Test | ||
void null이_들어오면_기본값이_설정된다() { | ||
LastReviewId lastReviewId = new LastReviewId(null); | ||
assertEquals(Long.MAX_VALUE, lastReviewId.getId()); | ||
} | ||
|
||
@Test | ||
void 유효한_값이_들어오면_그_값이_설정된다() { | ||
long id = 10L; | ||
LastReviewId lastReviewId = new LastReviewId(id); | ||
assertEquals(id, lastReviewId.getId()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package reviewme.review.service; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.ValueSource; | ||
|
||
class PageSizeTest { | ||
|
||
@Test | ||
void 유효한_값이_들어오면_그_값을_설정한다() { | ||
int size = 10; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아주 쬐~~~그만 수정 요청) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영완! |
||
|
||
PageSize pageSize = new PageSize(size); | ||
|
||
assertEquals(size, pageSize.getSize()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given, when, then 추가를 소소하게 요청합니다~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영완! |
||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(ints = {0, -1, 51}) | ||
void 유효한_범위_외의_값이_들어오면_기본값으로_설정한다(Integer size) { | ||
// given | ||
int defaultSize = 5; | ||
|
||
// when | ||
PageSize pageSize = new PageSize(size); | ||
|
||
// then | ||
assertEquals(defaultSize, pageSize.getSize()); | ||
} | ||
|
||
@Test | ||
void null이_들어오면_기본값으로_설정한다() { | ||
// given | ||
int defaultSize = 5; | ||
|
||
// when | ||
PageSize pageSize = new PageSize(null); | ||
|
||
// then | ||
assertEquals(defaultSize, pageSize.getSize()); | ||
} | ||
} |
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.
index.adoc의 리뷰 목록, 단건 조회 문서명이 서로 바뀌었는데 아루 세션 pr에서 수정되었네요!
저는 보면서 헷갈렸어서 여기도 남겨는 놓을게요~