Skip to content
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

#67,75 - Cursor Based Pagination을 도입하고, Sent Vote 스펙을 변경한다. #81

Merged
merged 13 commits into from
Aug 8, 2024

Conversation

kpeel5839
Copy link
Contributor

1. 🔗 관련 이슈

2. 📄 구현한 내용 또는 수정한 내용

간단하게 Cursor Based Pagination을 도입하고 Sent Vote 스펙을 변경했어요!!

3. 🙌 추가적으로 알리고 싶은 내용

4. 🙄 TODO / 고민하고 있는 것들

5. ✅ 배포 Checklist

  • 본인을 Assign 해주세요.
  • 본인을 제외한 백엔드 개발자를 리뷰어로 지정해주세요.
  • 변경된 DB 업데이트 해주세요. (꼭 해주세요 배포 안돼요!!!)

@kpeel5839 kpeel5839 added 🌱기능🌱 새로운 기능을 추가해요 ! 🔧리팩터링🔧 리팩터링일까요 리팩토링일까요? 🏋️매튜🏋️ 24기 김재연 labels Aug 8, 2024
@kpeel5839 kpeel5839 self-assigned this Aug 8, 2024
Copy link
Contributor

@sectionr0 sectionr0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커서 기반 페이징 적용하느라 고생하셨어용!

여전히 그대의 테스트코드.. 테스트 장인이네요

저도 여유되면 테스트 많이 작성해볼게요!

Comment on lines 27 to 25
val voteResults = votes.associateWith { getSentVotes(it, user) }
val votes = VoteServiceHelper.findVotesOrderByDateDesc(votePort, user, cursorId, limit + 1)
val classmates = VoteServiceHelper.findClassmatesByUser(userPort, user)
val voteResults = votes.take(limit.toInt()).associateWith { getSentVotes(it, user, classmates) }

return SentVotesResponses.from(voteResults)
return SentVotesResponses.from(voteResults, votes.size.toLong() == limit + 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에 limit + 1 은 어떤의미일까요?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리를 한번 더 날리는 것이 아니라, limit 기준에서 1개 더 가져와서, 만일 한개가 더 존재한다면 다음 페이지가 있는 것으로 처리하고 있어용!!

Comment on lines 120 to 136
if (cursorId == null) {
return voteJpaRepository.findAllBySchoolIdAndGradeAndClassNumberOrderByDateDesc(
schoolId,
grade,
classNumber,
Long.MAX_VALUE,
limit
)
}
return voteJpaRepository.findAllBySchoolIdAndGradeAndClassNumberOrderByDateDesc(
schoolId,
grade,
classNumber,
cursorId,
limit
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커서가 null 인 부분에 따라 repository 에서 분기처리를 하고 계시는군요..!

혹시 controller나 service 쪽에서 미리 확인 후 하시는건 어떠신가요??

이전에 제가 커서 기반 했을 때 common 모듈에다가

object CursorUtils {

    fun getEffectiveCursorId(cursorId: Long?): Long {
        return cursorId?.takeIf { it != 0L } ?: Long.MAX_VALUE
    }

}

해당 오브젝트를 만들어 놨어요!

val cursorValue = getEffectiveCursorId(cursorId)

이런 형식으로 앞 단에서 미리 값이 없는걸 확인하면 조금 더 코드가 깔끔해 질 것 같아서요!

재연짱이 한번 보고 판단해서 적용해주세요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오우 CursorUtil이 있군요 이 부분 적용할게용~!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 확실히 적용하기를 잘헀네요. 서비스 부분에서 이 부분을 판단해주는 것이 훨씬 맞는 선택인 것 같아요 🥰

@kpeel5839 kpeel5839 merged commit dda6919 into develop Aug 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱기능🌱 새로운 기능을 추가해요 ! 🏋️매튜🏋️ 24기 김재연 🔧리팩터링🔧 리팩터링일까요 리팩토링일까요?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants