[DP-447] 기술블로그 추천 API #133
Conversation
| private TechArticle techArticle; | ||
|
|
||
| @Builder | ||
| public TechArticleRecommend(boolean status, Member member, AnonymousMember anonymousMember, TechArticle techArticle) { |
| this.techArticle = techArticle; | ||
| } | ||
|
|
||
| public static TechArticleRecommend from(Member member, TechArticle techArticle) { |
There was a problem hiding this comment.
앗 요거 개발할 때 만들어두었다가 create를 새로 만들면서 사용안하게 되었네요. 제거하겠습니다 꼼꼼리뷰 감사합니다!
There was a problem hiding this comment.
TechArticleRecommend 클래스에 TechArticle 관련 연관관계 편의 메소드가 있어야 할 것 같아요~!
양방향인 경우에 연관관계 편의 메소드가 있는것이 편하고 실수를 방지합니당.
아래 처럼 서로 관계를 맺어주는 메소드가 필요 합니다~!
- TechArticleRecommend -> TechArticle
- TechArticle -> TechArticleRecommend
e.g)
public void(TechArticle techArticle) {
this.techArticle = techArticle;
techArticle.getRecommends().add(this);
}There was a problem hiding this comment.
changeTechArticle() 메서드에 techArticle.getRecommends().add(this); 이 코드 추가하겠습니다!
| if(!ObjectUtils.isEmpty(anonymousMemberId)) { | ||
| anonymousMember = anonymousMemberService.findOrCreateAnonymousMember(anonymousMemberId); | ||
| } | ||
|
|
There was a problem hiding this comment.
아래 조건문이 필요한가요?!
주석을 보면 익명회원을 조회하거나 생성이라고 작성되어 있는데, 그렇다는 것은 anonymousMembe가 null 이면 안된다는 의미 같아요.
// 익명 회원을 조회하거나 생성
AnonymousMember anonymousMember = null;
if(!ObjectUtils.isEmpty(anonymousMemberId)) {
anonymousMember = anonymousMemberService.findOrCreateAnonymousMember(anonymousMemberId);
}There was a problem hiding this comment.
익명회원의 추천기능을 지원하더라도 추천 API가 아닌 조회 API는 anonymousId가 없는 경우에도 열려있어야 한다고 생각했습니다~! GA 값이 없는 것이 조회 API의 로직에 영향을 줄만한 사유가 되지는 않는다고 생각하기 때문이에요!
주석 내용에 'anonymousId가 있다면' 을 추가하면 어떨까요?
There was a problem hiding this comment.
그렇군요~!!
소영님 말씀대로 추천기능으로 인해서 조회 기능에 영향을 주면 안될 것 같아요~!!
이런식으로 함수로 만들면 가독성 측면에서 더 좋을 것 같아요!
public AnonymousMember getAnonymousMemberOrNull(String anonymousMemberId) {
if(!ObjectUtils.isEmpty(anonymousMemberId)) {
return anonymousMemberService.findOrCreateAnonymousMember(anonymousMemberId);
}
return null;
}추가로 조회시 클라이언트에서 GA 값을 함께 요청하도록 하는 것도 필요해 보입니다!
There was a problem hiding this comment.
이런식으로 함수로 만들면 가독성 측면에서 더 좋을 것 같아요!
좋네요! 반영하겠습니다!
추가로 조회시 클라이언트에서 GA 값을 함께 요청하도록 하는 것도 필요해 보입니다!
현재 상세조회 API와 docs에 GA값을 받을 수 있도록 되어있는데, 혹시 추가로 어느부분의 추가작업이 필요할까요?
|
|
||
| @Override | ||
| @Transactional | ||
| public TechArticleRecommendResponse updateRecommend(Long techArticleId, String anonymousMemberId, Authentication authentication) { |
There was a problem hiding this comment.
비니지스 로직을 아주 쉽고 잘 작성하신 것 같아요! 👍
가독성이 매우 뛰어나요~!!!
술술술술술 읽혀욧ㅋㅋ
| @PostMapping("/articles/{techArticleId}/recommend") | ||
| public ResponseEntity<BasicResponse<TechArticleRecommendResponse>> updateRecommend( | ||
| @PathVariable Long techArticleId, | ||
| @RequestHeader(value = HEADER_ANONYMOUS_MEMBER_ID, required = false) String anonymousMemberId |
There was a problem hiding this comment.
추천시에 반드시 필요하니까 required = true로 변경해야 할 것 같아요!
There was a problem hiding this comment.
앗 회원이라면 anonymousId가 아닌 토큰으로 회원정보를 찾으니 nullable해야 하지 않을까요?!
|
|
||
| // when | ||
| TechArticleDetailResponse techArticleDetailResponse = guestTechArticleService.getTechArticle(id, | ||
| TechArticleDetailResponse techArticleDetailResponse = guestTechArticleService.getTechArticle(id, null, |
There was a problem hiding this comment.
guestTechArticleService.getTechArticle(id, null, ...) 에서 익명사용자를 테스트 하는 거라서 null 이면 안되는 것 아닌가요?
There was a problem hiding this comment.
위에서 답변드린 바와 같이 조회 API에서는 익명사용자가 null 이더라도 영향을 받지 않아야 한다고 생각해서 이렇게 변경했습니다! 익명사용자의 추천여부 응답을 확인하는 테스트코드에서만 GA 값이 넘어가도록 했는데, 혹시 어떻게 생각하시나요?
📝 작업 내용
🔗 참고할만한 자료(선택)
💬 리뷰 요구사항(선택)