-
Notifications
You must be signed in to change notification settings - Fork 4
feat: 문의하기 기능 추가 #87
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: 문의하기 기능 추가 #87
Conversation
Zy0ung
left a comment
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.
고생하셨습니다!
net79736
left a comment
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.
수고하셨습니다^^
SangkiHan
left a comment
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.
고생하셨습니다! 몇가지 코멘트 남겨드렸습니다
| private Long id; | ||
|
|
||
| @NotNull(message = "문의 내용이 없으면 안됩니다.") | ||
| private String content; |
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.
Valid체크를 Entity에 하신이유가 있으실까요?
Valid 체크는 Controller Request단에서 하는게 맞는거 같습니다!
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.
넵 반영하겠습니다
| .password("teamPlayHive12#") | ||
| .build()).getPublicId(); | ||
| System.out.println("memberPublicId = " + memberPublicId); | ||
| } |
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.
AfterEach로 각 테스트가 끝났을 경우 inquiryRepository.deleteInBatch로 각 테이블에 저장된 내용들을 없애줘야 다른 테스트에 영향을 끼치지 않습니다!
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.
넵 반영하겠습니다!
| PageCustomResponse<Inquiry> response = inquiryReadService.getInquiriesByMember(testMember.getPublicId(), new PageInfoRequest(1, 10)); | ||
|
|
||
| // Then | ||
| assertThat("문의내역").isEqualTo(response.getContent().get(0).getContent()); |
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.
페이징에 대한 테스트인데 단건 데이터로는 테스트가 정확히 이루어 졌다고 보기 애매하네요ㅠ
그리고 list에 대한 테스트는 extracting과 contains, Tuple을 사용하여 테스트코드를 작성하면 좋습니다!
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.
4 5개정도 데이터를 밀어넣고 페이징 잘되는지 테스트하면 좋을 것 같습니다!
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.
넵 반영하겠습니다!
|
|
||
| public PageCustomResponse<Inquiry> getInquiriesByMember(UUID memberPublicId, PageInfoRequest pageInfoRequest) { | ||
| Member member = memberRepository.findByPublicId(memberPublicId) | ||
| .orElseThrow(() -> new PlayHiveException(ErrorCode.USER_NOT_FOUND)); |
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.
이 부분 MemberReadService를 재활용하면 좋을 것 같습니다!
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.
넵 반영하겠습니다!
| private final InquiryReadService inquiryReadService; | ||
| private final InquiryWriteService inquiryWriteService; | ||
|
|
||
| @PostMapping("/create") |
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.
Post Method는 이미 Rest에서 데이터를 저장한다는 의미가 있기 때문에 /create는 빼도 무방할 것 같아요!
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.
넵 반영하겠습니다!
| Member member = memberRepository.findByPublicId(memberPublicId) | ||
| .orElseThrow(() -> new PlayHiveException(ErrorCode.USER_NOT_FOUND)); | ||
| Pageable pageable = PageRequest.of(pageInfoRequest.getPage() - 1, pageInfoRequest.getSize()); | ||
| Page<Inquiry> inquiries = inquiryRepository.findByMember(member, pageable); |
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.
Entity를 바로 Response로 내려주는건 안좋습니다ㅠ InquiryResponse를 만들어 변환해줘서 내려주는게 좋습니다!
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.
죄송합니다 저번에 피드백 주신 부분인데 또 반영을 못했네요,, 수정하도록 하겠습니다.
SangkiHan
left a comment
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.
고생하셨습니다
| assertThat(response.getContent()).hasSize(5); | ||
| assertThat(response.getPageInfo().getCurrentPage()).isEqualTo(2); | ||
| assertThat(response.getPageInfo().getTotalPage()).isEqualTo(3); | ||
| assertThat(response.getPageInfo().getTotalElement()).isEqualTo(15); |
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.
assertThat(pageInfo)
.extracting("currentPage", "totalPage", "totalElement")
.containsExactlyInAnyOrder(
1, 2, 3L
)이런식으로 한번에 처리도 가능합니다! 참고해주세요!
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.
assertThat(pageInfo)
.extracting("currentPage", "totalPage", "totalElement")
.containsExactlyInAnyOrder(
1, 2, 3L
)
아 위에서 답변주신 부분이 이렇게 사용하는거군요.
많이 배웁니다 감사합니다 :)

기능 설명
작업 내용
수정 사항
추가 작업 예정
테스트