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

Feat 쿠폰 무한 스크롤 구현 #29

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kcsc2217
Copy link
Collaborator

@kcsc2217 kcsc2217 commented Oct 2, 2024

📌 과제 설명

  1. 쿠폰의 상태에 따른 동적 쿼리를 활용한 무한 스크롤 구현
  2. test 코드를 통한 무한 스크롤 검증
  3. 최종 무한 스크롤 api

👩‍💻 요구 사항과 구현 내용

✅ 피드백 반영사항

image

✅ PR 포인트 & 궁금한 점

@kcsc2217 kcsc2217 linked an issue Oct 2, 2024 that may be closed by this pull request
Copy link
Collaborator

@AnTaeho AnTaeho left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!
잘 작성 하신것 같아서 몇가지 의견 남겨봤습니다!



@GetMapping
public Slice<UserCouponResponse> getUserCoupons(@Valid UserCouponRequest userCouponRequest, HttpServletRequest request, Pageable pageable) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2
@RequestBody 필요할 것 같습니다!

Comment on lines +6 to +12
public record UserCouponRequest(
@NotNull
Boolean used
) {


}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4
만약에 Boolean에 null이 들어오면 사용/미사용 모두 보여주는 것은 어떤가요?

}

@Override
public Slice<UserCouponResponse> selectUserCoupons(UserCouponRequest userCouponRequest,Long memberId, Pageable pageable) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4
저희 엔티티 명이 user니까 memberId 말고 userId는 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 userId가 좋을 것 같습니다~~

Copy link
Collaborator

@tnals2384 tnals2384 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 ㅎㅎ~!!! 테스트코드에 given, then, when을 줄바꿈을 조곰만 신경써서 더 보기 좋게 하면 더더더 좋을 것 같아용!!!!

Comment on lines +28 to +29
public Slice<UserCouponResponse> getUserCoupons(@Valid UserCouponRequest userCouponRequest, HttpServletRequest request, Pageable pageable) {
Long userId = getUserId(request);
Copy link
Collaborator

Choose a reason for hiding this comment

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

궁금해서 여쭤봅니당!! 혹시 UserCouponRequest에서 used 하나만 받아오는데 파라미터나 pathvariable이 아닌 dto로 받아오는 특별한 이유가 있을까요?!?

}

@Override
public Slice<UserCouponResponse> selectUserCoupons(UserCouponRequest userCouponRequest,Long memberId, Pageable pageable) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 userId가 좋을 것 같습니다~~


import static org.assertj.core.api.Assertions.assertThat;

@SpringBootTest
Copy link
Collaborator

Choose a reason for hiding this comment

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

P4
여기도 관현님이 말씀하신것처럼 @SpringBootTest를 한번 더쓰는 것보다 IntegralTestSupport를 상속받는 게 어떨까욥??

//then

PageRequest pageRequest2 = PageRequest.of(1, 20); //10장 남음
Slice<UserCouponResponse> userCoupons2 = userCouponService.getUserCoupons(userCouponRequest, 1L, pageRequest2);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2
여기도 1L 대신 user.getId()로 사용하는게 더 안전할 것 같아요!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 쿠폰 무한 스크롤 구현
3 participants