-
Notifications
You must be signed in to change notification settings - Fork 2
Feat: 프로필 화면 상단정보 / 피드 이미지 미리보기 api #177
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
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.
Summary of Changes
Hello @kjhh2605, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 사용자 프로필 화면에 필요한 정보를 제공하기 위한 새로운 API 엔드포인트와 관련 로직을 추가합니다. 구체적으로, 사용자 프로필 상단에 표시될 상세 정보(총 카드 수, 팔로워/팔로잉 수 등)와 해당 사용자가 공유한 카드 이미지 목록(피드 미리보기)을 조회하는 기능을 구현합니다.
Highlights
- 프로필 피드 미리보기 API 추가: CardController에 특정 사용자의 공유된 카드 목록을 조회하는 새 GET 엔드포인트 /profile/{userId}가 추가되었습니다. 이 엔드포인트는 커서 기반 페이지네이션을 지원합니다.
- 사용자 프로필 상세 정보 API 추가: UserRestController에 사용자 프로필의 상세 정보(총 카드 수, 팔로워 수, 팔로잉 수 포함)를 가져오는 새 POST 엔드포인트 /profile/{userId}가 추가되었습니다.
- 카드 관련 DTO 및 컨버터 개선: CardResponse에 프로필 피드 미리보기 응답을 위한 새로운 DTO(profileCardListDTO, ProfileCardDTO)가 정의되었으며, CardConverter에 해당 DTO로 변환하는 메서드(toProfileCardDto, toProfileCardList)가 추가되었습니다.
- 사용자 관련 DTO 및 컨버터 개선: UserResponseDTO에 상세 사용자 프로필 정보를 위한 새로운 DTO(DetailProfileDto)가 추가되었고, UserConverter에 사용자 데이터, 팔로우 상태, 각종 카운트를 이 DTO로 매핑하는 toDetailProfileDto 메서드가 추가되었습니다.
- 리포지토리 기능 확장: CardRepository에 사용자 ID로 공유된 카드를 페이지네이션하여 조회하고(findByUserIdAndIsSharedTrueOrderByIdDesc, findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc), 공유된 카드 수를 세는(countByUserIdAndIsDeletedFalseAndIsSharedTrue) 새로운 메서드가 추가되었습니다. UserFollowRepository에는 팔로워 및 팔로잉 수를 세는 메서드(countUserFollowByTargetUser, countUserFollowByUser)가 추가되었습니다.
- 서비스 계층 로직 구현: CardService와 CardServiceImpl에 getProfileCardList 로직이 구현되었으며, UserService와 UserServiceImpl에 getProfile 로직이 구현되어 사용자 상세 정보와 관련 카운트를 조회합니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
이번 PR은 프로필 화면과 피드 미리보기 API를 추가하는 기능 구현이군요. 전반적으로 잘 작성되었지만, 몇 가지 개선점을 제안합니다. REST API 설계 원칙을 준수하기 위해 조회 API의 HTTP 메서드를 POST에서 GET으로 변경하는 것이 좋습니다. 또한, DTO 클래스 이름의 네이밍 컨벤션을 일관성 있게 수정하고, 서비스 로직에서 발생할 수 있는 NullPointerException을 방지하기 위한 예외 처리를 추가하는 것을 권장합니다. 자세한 내용은 각 파일에 대한 코멘트를 참고해주세요.
src/main/java/EatPic/spring/domain/user/controller/UserRestController.java
Outdated
Show resolved
Hide resolved
src/main/java/EatPic/spring/domain/user/service/UserServiceImpl.java
Outdated
Show resolved
Hide resolved
|
|
||
| @GetMapping("/profile/{userId}") | ||
| @Operation(summary = "프로필 화면 피드 미리보기", description = "공유한 카드의 번호와 이미지 url 조회 API") | ||
| public ApiResponse<CardResponse.profileCardListDTO> getProfileCardsList(@PathVariable Long userId, |
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 static CardResponse.profileCardListDTO toProfileCardList(Long userId, Slice<Card> cardList) { | ||
| return CardResponse.profileCardListDTO.builder() |
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.
컨버터 메서드의 반환 타입과 빌더 호출 부분도 DTO 클래스 이름에 맞춰 ProfileCardListDTO로 변경해야 합니다. 일관성을 위해 수정해주세요.
| public static CardResponse.profileCardListDTO toProfileCardList(Long userId, Slice<Card> cardList) { | |
| return CardResponse.profileCardListDTO.builder() | |
| public static CardResponse.ProfileCardListDTO toProfileCardList(Long userId, Slice<Card> cardList) { | |
| return CardResponse.ProfileCardListDTO.builder() |
| @Getter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public static class profileCardListDTO{ |
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.
Code Review
이 PR은 프로필 화면에 필요한 상단 정보(카드, 팔로워, 팔로잉 수)와 피드 이미지 미리보기를 위한 API를 추가하는 변경 사항을 담고 있습니다. 전반적으로 기능 구현은 잘 되었지만, 몇 가지 개선점을 발견했습니다. REST API의 HTTP 메서드 사용 관례를 따르고, 삭제된 데이터가 노출될 수 있는 잠재적인 버그를 수정하는 것이 좋겠습니다. 자세한 내용은 아래 주석을 참고해주세요.
| Slice<Card> findByUserIdAndIsSharedTrueOrderByIdDesc(Long userId, Pageable pageable); | ||
| Slice<Card> findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(Long userId, Long cursor, Pageable 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.
새로 추가된 프로필 피드 조회용 쿼리 메서드에 isDeleted = false 조건이 누락되어 있습니다. 이로 인해 논리적으로 삭제된 카드가 프로필에 노출될 수 있습니다. 다른 쿼리 메서드들처럼 삭제되지 않은 카드만 조회하도록 IsDeletedFalse 조건을 추가하는 것이 좋습니다. 이 메서드 시그니처를 변경하면 CardServiceImpl에서 호출하는 부분도 함께 수정해야 합니다.
| Slice<Card> findByUserIdAndIsSharedTrueOrderByIdDesc(Long userId, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(Long userId, Long cursor, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsDeletedFalseAndIsSharedTrueOrderByIdDesc(Long userId, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsDeletedFalseAndIsSharedTrueAndIdLessThanOrderByIdDesc(Long userId, Long cursor, Pageable pageable); |
src/main/java/EatPic/spring/domain/user/controller/UserRestController.java
Outdated
Show resolved
Hide resolved
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.
Code Review
프로필 화면 상단 정보와 피드 이미지 미리보기 API 추가에 대한 PR 잘 보았습니다. 몇 가지 개선점을 제안합니다. UserRestController에서 프로필 조회 API는 GET 메서드를 사용하는 것이 REST 원칙에 더 부합합니다. 또한, CardRepository와 CardServiceImpl에서 삭제된 카드를 필터링하는 로직이 누락되어 있어 프로필에 삭제된 카드가 표시될 수 있는 문제를 발견했습니다. 이 부분을 수정하여 일관성과 정확성을 높이는 것이 중요해 보입니다. 마지막으로 CardConverter에서 코드 가독성과 안정성을 높일 수 있는 작은 리팩토링을 제안합니다. 자세한 내용은 각 파일에 남긴 코멘트를 확인해주세요.
| Slice<Card> findByUserIdAndIsSharedTrueOrderByIdDesc(Long userId, Pageable pageable); | ||
| Slice<Card> findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(Long userId, Long cursor, Pageable 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.
프로필 카드 목록을 조회하는 쿼리 메서드에 삭제된 카드를 필터링하는 IsDeletedFalse 조건이 누락되었습니다. countByUserIdAndIsDeletedFalseAndIsSharedTrue 메서드에서는 해당 조건이 사용되고 있어 로직의 일관성이 부족하며, 삭제된 카드가 프로필에 노출될 수 있는 버그를 유발할 수 있습니다. 메서드 이름과 쿼리에 IsDeletedFalse 조건을 추가하여 수정하는 것을 권장합니다.
| Slice<Card> findByUserIdAndIsSharedTrueOrderByIdDesc(Long userId, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(Long userId, Long cursor, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsSharedTrueAndIsDeletedFalseOrderByIdDesc(Long userId, Pageable pageable); | |
| Slice<Card> findByUserIdAndIsSharedTrueAndIdLessThanAndIsDeletedFalseOrderByIdDesc(Long userId, Long cursor, Pageable pageable); |
| if (cursor == null) { | ||
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueOrderByIdDesc(userId, pageable); | ||
| } else { | ||
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(userId, cursor, 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.
이 메서드에서 호출하는 cardRepository의 쿼리 메서드들은 삭제된 카드를 필터링하지 않습니다. CardRepository의 관련 메서드에 IsDeletedFalse 조건을 추가하고, 여기서 수정된 메서드를 호출하도록 변경해야 합니다. 이렇게 해야 삭제된 카드가 프로필 피드에 나타나지 않습니다.
| if (cursor == null) { | |
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueOrderByIdDesc(userId, pageable); | |
| } else { | |
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueAndIdLessThanOrderByIdDesc(userId, cursor, pageable); | |
| } | |
| if (cursor == null) { | |
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueAndIsDeletedFalseOrderByIdDesc(userId, pageable); | |
| } else { | |
| cardSlice = cardRepository.findByUserIdAndIsSharedTrueAndIdLessThanAndIsDeletedFalseOrderByIdDesc(userId, cursor, pageable); | |
| } |
src/main/java/EatPic/spring/domain/user/controller/UserRestController.java
Outdated
Show resolved
Hide resolved
| public static CardResponse.ProfileCardListDTO toProfileCardList(Long userId, Slice<Card> cardList) { | ||
| return CardResponse.ProfileCardListDTO.builder() | ||
| .hasNext(cardList.hasNext()) | ||
| .nextCursor(cardList.hasNext() ? cardList.getContent().get(cardList.getContent().size() - 1).getId() : null) | ||
| .userId(userId) | ||
| .cardsList(cardList.getContent().stream() | ||
| .map(CardConverter::toProfileCardDto) | ||
| .toList()) | ||
| .build(); | ||
| } |
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.
cardList.getContent()가 두 번 호출되고 있습니다. 결과를 지역 변수에 저장하여 재사용하면 가독성이 향상되고 불필요한 메서드 호출을 줄일 수 있습니다. 또한 nextCursor를 설정하는 로직에서 getContent()의 결과가 비어있는 경우를 명시적으로 확인하여 코드의 안정성을 높일 수 있습니다.
public static CardResponse.ProfileCardListDTO toProfileCardList(Long userId, Slice<Card> cardList) {
List<Card> cards = cardList.getContent();
return CardResponse.ProfileCardListDTO.builder()
.hasNext(cardList.hasNext())
.nextCursor(cardList.hasNext() && !cards.isEmpty() ? cards.get(cards.size() - 1).getId() : null)
.userId(userId)
.cardsList(cards.stream()
.map(CardConverter::toProfileCardDto)
.toList())
.build();
}
src/main/java/EatPic/spring/domain/user/controller/UserRestController.java
Outdated
Show resolved
Hide resolved
2anizirong
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.
수수수수고했습니다..
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.
Code Review
이번 PR은 프로필 화면에 필요한 상단 정보(게시물, 팔로워, 팔로잉 수)와 피드 이미지 미리보기 API를 추가하는 변경사항을 담고 있습니다. 전반적으로 기능 구현은 잘 이루어졌습니다. 코드 리뷰에서는 몇 가지 개선점을 제안했습니다. 중복 코드 제거, 트랜잭션 어노테이션 추가를 통한 성능 및 일관성 개선, 그리고 가독성 향상을 위한 리포지토리 메서드 이름 변경 등을 포함합니다. 이 피드백들이 코드 품질을 높이는 데 도움이 되기를 바랍니다.
| .cardsList(cardList.getContent().stream() | ||
| .map(CardConverter::toProfileCardDto) | ||
| .toList()) |
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.
cardList.getContent()가 중복으로 호출되고 있습니다. 169번째 줄에서 이미 list 변수에 저장했으므로, list.stream()을 사용하여 중복 호출을 피하는 것이 좋습니다. 이렇게 하면 코드가 더 효율적이고 깔끔해집니다.
| .cardsList(cardList.getContent().stream() | |
| .map(CardConverter::toProfileCardDto) | |
| .toList()) | |
| .cardsList(list.stream() | |
| .map(CardConverter::toProfileCardDto) | |
| .toList()) |
| ); | ||
| } | ||
|
|
||
| @Override |
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.
| Long countUserFollowByTargetUser(User targetUser); | ||
| Long countUserFollowByUser(User user); |
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.
메서드 이름이 약간 혼동을 줄 수 있습니다. Spring Data JPA의 명명 규칙에 따라 더 명확하고 간결한 이름으로 변경하는 것을 제안합니다. 예를 들어, countByTargetUser는 팔로워 수를, countByUser는 팔로잉 수를 계산하는 것으로 명확하게 이해될 수 있습니다. 이 변경 사항을 적용할 경우 UserServiceImpl에서의 메서드 호출도 함께 수정해야 합니다.
| Long countUserFollowByTargetUser(User targetUser); | |
| Long countUserFollowByUser(User user); | |
| Long countByTargetUser(User targetUser); | |
| Long countByUser(User user); |
| .orElseThrow(() -> new ExceptionHandler(ErrorStatus.MEMBER_NOT_FOUND)); | ||
| } | ||
|
|
||
| @Override |
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.
#️⃣연관된 이슈
📝작업 내용
스크린샷 (선택)
💬리뷰 요구사항(선택)