-
Notifications
You must be signed in to change notification settings - Fork 0
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
[REFACTOR] 플레이그라운드 게시물 반환 기능 + 채용 정보 조회 사용자 정보 추가 - #434 #453
Conversation
|
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.
고생하셨습니다! 몇가지 코멘트 남겨뒀으니 확인부탁드려요!
+ response에서 isHotPost 필드가 아직 hotPost로 보이는 것 같아요
if (postDetail.member() != null) { | ||
post.setProfileImage(postDetail.member().profileImage()); | ||
post.setName(postDetail.member().name()); | ||
} else if (postDetail.anonymousProfile() != null) { | ||
post.setProfileImage(postDetail.anonymousProfile().profileImgUrl()); | ||
post.setName(postDetail.anonymousProfile().nickname()); | ||
} |
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.
P3. set을 사용하지 않고, getPlayGroundPostDetail 값을 받고, getPlaygroundEmploymentPost 받아서 새로운 DTO를 만들어서 반환하는 것이 불변성을 보장하기에 더 좋은 방법일 것 같아요
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.
넵 변경해두겠습니다!
@@ -0,0 +1,7 @@ | |||
package org.sopt.app.application.playground.dto; | |||
|
|||
public interface PostWithMemberInfo { |
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.
P3. PostWithMemberInfo interface를 override하는 방법이 저로서는 현재 이점이 명확하지 않은 것 같아요!
어떤 이점이 있다 판단하여 PostWithMemberInfo라는 interface를 도입한 것인지 궁금해요
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.
최근 게시물 반환, 채용탭 10개 반환에서 똑같은 로직으로 멤버의 이름과 프로필 이미지를 받아와 추가하는데
같은 메소드를 사용하고 명시적으로 다른 타입의 같은 메소드를 사용할때 어떤 것을 추가해야 할지 명시적으로 결정해주기 위해 다음과 같이 정했습니다!
) { | ||
} | ||
|
||
public record AnonymousProfile( |
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.
Q. 이름이 AnonymousProfile인 이유가 있나요?
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.
확인해보니 포스트의 프로필을 설정하지 않고 null인 유저가 있었습니다.
유저가 null인경우 플그쪽에서 해당이름으로 유저를 보여주지않는 방식으로 내려주어 해당이름으로 정했습니다!
final Map<String, String> accessToken = createAuthorizationHeaderByUserPlaygroundToken(playgroundToken); | ||
for (T post : posts) { | ||
Long postId = post.getId(); | ||
PlayGroundPostDetailResponse postDetail = playgroundClient.getPlayGroundPostDetail(accessToken, postId); |
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.
P2. 플그에게서 채용탭 post를 모두 받고, 그 포스트를 이용해서 하나씩 다시 postDetail을 받는다는 로직이 너무 비용이 크다고 생각해요. 한 번에 채용탭과 함께 받아오기는 어려운걸까요?
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.
기존의 로직대로 하면 한번에 받아올수있는데, username과 프로필 이미지가 추가되어서 다시받아오는걸로 결정했습니다! 필드 변경에 띠라서 API 재요청을 드리는것보다 후에 이러한 작업을 하나로 묶는것이 일단 마감 Date때문에 이렇게 결정했습니다!
📝 PR Summary
🌴 Works
🌱 Related Issue
closed #434
🌵 PR 참고사항
플레이그라운드 게시물 반환 기능
채용 정보 조회