-
Notifications
You must be signed in to change notification settings - Fork 8
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] 종료됨 탭에서 reviewDeadline 역순으로 정렬하는 기능 구현(#763) #765
Conversation
Test Results 66 files 66 suites 7s ⏱️ Results for commit 880ab53. ♻️ This comment has been updated with latest results. |
0c6e7d9
to
bd076ce
Compare
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.
PageRequest 에 Sort 도 동적으로 넣을 수 있었네용 👍👍
두개다 사소한 의견이라 꼭 반영해줄 필요는 없을꺼 같습니당
this.reviewDeadline = reviewDeadline; | ||
} | ||
|
||
private void validateDeadline(LocalDateTime recruitmentDeadline, LocalDateTime reviewDeadline) { |
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.
검증 로직을 밖에서 해도 될 거 같긴 한데 고민되네요.
( 절대, 해당 객체는 이전 시간으로는 생성하지 못할꺼 같다..? )
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.
( 절대, 해당 객체는 이전 시간으로는 생성하지 못할꺼 같다..? )
저는 오히려 그런 느낌이라 서비스에서 로직을 구현하지 않고, 원시값을 포장하여 구현한 느낌이에요!~
무조건 이전 시간으로는 생성할 수 없으니, 도메인 정책으로 정의할 수 있도록 내부로 구현하는 것이 좋다 생각 들었습니다~
@@ -32,7 +32,7 @@ public Room find(long roomId) { | |||
} | |||
|
|||
public List<Room> findAll(Specification<Room> spec) { | |||
return roomRepository.findAll(spec, Sort.by("recruitmentDeadline").descending()); | |||
return roomRepository.findAll(spec, Sort.by("roomDeadline.recruitmentDeadline").descending()); |
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.
해당 로직에서는 RoomSortStragety Enum 을 쓸 수 없나요?
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.
이 부분도 바꿀 수 있겠네요 👍
수정합니당~
📌 관련 이슈
✨ PR 세부 내용
종료된 방들을 종료된 최신순으로 조회하도록 수정했습니다.
구현 방식:
위의 코드는 스프링의 PageRequest 클래스입니다.
여기서 알 수 있는 것은 PageRequest가 매개변수로 Sort를 받을 수 있음을 알게 되었습니다.
이 점을 활용하여, 기존에 DB 메서드에서 직접 정렬하던 방식을 PageRequest의 매개변수로 Sort를 주입하여 정렬하도록 수정했습니다.
이를 통해 RoomStatus에 따라 다른 정렬 방식을 적용할 수 있었습니다.