-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/#658 자신이 작성한 피드 목록 조회 #675
The head ref may contain hidden characters: "Feature/#658-\uC790\uC2E0\uC774_\uC791\uC131\uD55C_\uD53C\uB4DC_\uBAA9\uB85D_\uC870\uD68C"
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.
고생하셨습니다 !!
제가 남긴 의문은 바로 말씀해주셔도 좋고 코멘트 남겨주시고 말씀해주시면 좋을 것 같아요
@@ -43,6 +44,11 @@ public FeedDetailResponse findFeed(final Member member, @PathVariable final Long | |||
return feedQueryService.findFeed(member, id); | |||
} | |||
|
|||
@GetMapping("/my") |
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.
의견을 낼 때, 다른 제안을 같이 드려야하는데 뭔가 my 보다 좋은게 없는 것 같기도 하고,,
다른 분들은 어떻게 생각하시나요?
/feeds/my
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.
저도 이 부분은 고민이 많았었는데 feeds/my
와 feeds?member-id={member-id}
중 feeds/my
를 선택했습니다.
이유는 이미 Member
를 파라미터로 받고 있는데 사용자의 id를 추가로 받을 경우 불필요한 파라미터가 추가되고 유효성 검사를 해주어야 하는데 이 또한 member-id
를 받지 않으면 유효성 검사를 되므로 불필요하다고 생각했기 때문입니다.
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-id를 따로 받는 것이 조금 더 정석적이긴 하지만, 굳이 해야하나? 라는 생각이 들기도 하더라고요.
그리고 , /feeds?member-id = 형식으로 짜면
위에 /feeds?event-id = 랑 겹쳐서 문제가 생길거에요.
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.
전 확장성을 고려했을 때 /feeds/member-id={member_id}
도 괜찮은 것 같아요. 특정 사용자가 작성한 feed 목록을 조회하는 요구사항이 생긴다면 유연하게 사용할 수 있을 것 같아서요~!
.collect(Collectors.toList()); | ||
|
||
final Map<Long, Long> feedCommentCounts = getFeedIdCommentCountMap(feedIds); | ||
final Map<Long, List<Image>> feedImages = getFeedImagesMap(feedIds); |
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.
내가 쓴 피드 목록을 볼 때 모든 이미지가 필요한가요??
화면이 제대로 없어서 잘 모르겠지만, 썸네일만 보여주지 않을까라는 생각이 듭니다
이 부분이 안드 분과 논의된 내용인지 말씀해주시면 좋을 것 같습니다
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.
기존 피드 목록과 api를 완전히 통일해 달라는 요청이 있었기 때문에 이처럼 구현했습니다.
저도 추후 시간이 난다면 api에 대해 다시 이야기 하여 images와 thumbnail를 분리하고 목록에서는 images를 생략할 필요가 있다고 생각합니다.
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.
ㅇㅈ 추후 생략해야 할 것 같아요
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.
잘 봤습니다. approve할게요
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.
수고하셨습니다~~!! 크게 수정할 점은 없는 것 같아요.
approve하겠습니다!
@@ -43,6 +44,11 @@ public FeedDetailResponse findFeed(final Member member, @PathVariable final Long | |||
return feedQueryService.findFeed(member, id); | |||
} | |||
|
|||
@GetMapping("/my") |
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.
전 확장성을 고려했을 때 /feeds/member-id={member_id}
도 괜찮은 것 같아요. 특정 사용자가 작성한 feed 목록을 조회하는 요구사항이 생긴다면 유연하게 사용할 수 있을 것 같아서요~!
#️⃣연관된 이슈
close #658
📝작업 내용
자신이 작성한 피드 목록을 조회합니다.
예상 소요 시간 및 실제 소요 시간
2시간 / 2시간