-
Notifications
You must be signed in to change notification settings - Fork 2
public인 즐겨찾기 리스트 가져오기 #109
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
The head ref may contain hidden characters: "20250602_#105_\uAE30\uB2A5\uCD94\uAC00_\uC990\uACA8\uCC3E\uAE30_public\uC778_\uC990\uACA8\uCC3E\uAE30_\uB9AC\uC2A4\uD2B8_\uAC00\uC838\uC624\uAE30"
Conversation
|
""" Walkthrough북마크의 공개/비공개(visibility) 필터와 페이지네이션을 지원하는 새로운 API 엔드포인트가 추가되었습니다. 기존 사용자의 북마크 조회 메서드 및 서비스 계층 메서드의 명칭이 명확하게 변경되었으며, 저장소 계층에는 visibility 기반 조회와 페이지네이션을 지원하는 메서드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BookmarkController
participant BookmarkService
participant BookmarkRepository
Client->>BookmarkController: GET /api/bookmark/list?page=&size=&visibility=
BookmarkController->>BookmarkService: getBookmarks(page, size, visibility)
BookmarkService->>BookmarkRepository: findByVisibility(visibility, pageable)
BookmarkRepository-->>BookmarkService: Page<Bookmark>
BookmarkService-->>BookmarkController: Page<BookmarkResponse>
BookmarkController-->>Client: ResponseEntity<Page<BookmarkResponse>>
Possibly related PRs
Suggested reviewers
Poem
""" 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/jombi/freemates/controller/BookmarkController.java (2)
15-16: 불필요한 임포트가 포함되어 있습니다.
Bookmark,Place,Default임포트가 실제로 사용되지 않는 것으로 보입니다. 사용하지 않는 임포트는 제거하는 것이 좋습니다.Also applies to: 20-20, 23-23
164-173: 새로운 API 엔드포인트가 잘 설계되었습니다.페이지네이션과 가시성 필터링을 지원하는 새로운 엔드포인트가 적절히 구현되었습니다. API 문서도 상세하고 이해하기 쉽게 작성되었습니다.
한 가지 개선 제안:
visibility매개변수가null인 경우 모든 가시성의 북마크를 반환할지, 아니면 기본값(예: PUBLIC)을 사용할지 명확히 하는 것이 좋겠습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/jombi/freemates/controller/BookmarkController.java(2 hunks)src/main/java/jombi/freemates/repository/BookmarkRepository.java(2 hunks)src/main/java/jombi/freemates/service/BookmarkService.java(5 hunks)
🔇 Additional comments (4)
src/main/java/jombi/freemates/repository/BookmarkRepository.java (1)
6-6: 저장소 계층 변경사항이 올바르게 구현되었습니다.새로운
findByVisibility메서드가 적절한 Spring Data JPA 네이밍 규칙을 따르고 있으며, 페이지네이션과 가시성 필터링을 위한 올바른 매개변수 타입을 사용하고 있습니다.Also applies to: 9-10, 20-20
src/main/java/jombi/freemates/service/BookmarkService.java (2)
105-119: 새로운 페이지네이션 메서드가 올바르게 구현되었습니다.
getBookmarks메서드는 적절한 페이지네이션과 가시성 필터링을 제공하며, 응답 DTO 매핑도 정확합니다.bookmarkId가 응답에 포함된 것은 좋은 개선사항입니다.
86-86:Details
✅ Verification successful
메서드명 변경이 기존 호출부에 미치는 영향을 확인해주세요.
listByMember에서getBookmarksByMember로 메서드명이 변경되었습니다. 모든 호출부가 올바르게 업데이트되었는지 확인이 필요합니다.다음 스크립트를 실행하여 메서드 사용 현황을 확인하세요:
🏁 Script executed:
#!/bin/bash # 기존 메서드명이 여전히 사용되고 있는지 확인 rg -n "listByMember" --type java # 새 메서드명 사용 현황 확인 rg -n "getBookmarksByMember" --type javaLength of output: 364
메서드명 변경 검증 완료
기존
listByMember호출은 더 이상 존재하지 않으며, 모든 호출부가getBookmarksByMember로 정상 업데이트된 것을 확인했습니다. 추가 작업이 필요 없습니다.• src/main/java/jombi/freemates/controller/BookmarkController.java:
getBookmarksByMember호출
•listByMember사용 내역 없음src/main/java/jombi/freemates/controller/BookmarkController.java (1)
127-131: 메서드명 변경이 일관성 있게 적용되었습니다.컨트롤러의 메서드명이
list에서getMyBookmarks로 변경되고, 서비스 호출도getBookmarksByMember로 업데이트되어 일관성을 유지하고 있습니다.
src/main/java/jombi/freemates/controller/BookmarkController.java
Outdated
Show resolved
Hide resolved
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
Cassiiopeia
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.
문제없습니다
근데 이름은 좀 거슬리긴함
Page 는 마음에 안드는 명명 Page
코드 읽다가 헷갈렸네요
전 반환값 Dto 자체면 BookmarkResponse 쓰는데
이건 페이지안에있는 dto니까...
| */ | ||
| @Transactional(Transactional.TxType.SUPPORTS) | ||
| public List<BookmarkResponse> listByMember(CustomUserDetails customUserDetails) { | ||
| public List<BookmarkResponse> getBookmarksByMember(CustomUserDetails customUserDetails) { |
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를 넣으라니까~
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를 넣어도 동작을 하나요...? 찾아보니까 사용하고 싶으면 @AuthenticationPrincipal(expression = "member") 이런식의 어노테이션을 쓰라던디 그게 맞는가 싶어서 물어봅니다
| /** | ||
| * 즐겨찾기 목록 조회 (페이징) | ||
| */ | ||
| @Transactional(Transactional.TxType.SUPPORTS) |
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.
이게 그 SUPPORTS 는 “현재 트랜잭션이 존재하면 그 트랜잭션에 참여하고, 없으면 그냥 트랜잭션 없이(non-transactional) 실행”한다는 의미입니다.
| * 즐겨찾기 목록 조회 (페이징) | ||
| */ | ||
| @Transactional(Transactional.TxType.SUPPORTS) | ||
| public Page<BookmarkResponse> getBookmarks(int page, int size, Visibility visibility) { |
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.
저같음 이거 BookmarkDto 라고 명명할듯?
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.
request가 있어서 그럼 헷갈릴꺼 같아가지고요...
| ) | ||
| }) | ||
| @Operation( | ||
| summary = "즐겨찾기 목록 가져오기", |
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이랑 PRIVATE 별도로만 가져와요? ALL 은없어요?
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만 가져오게 하고-> 홈화면에서 보여줄때
all로 가져오는 경우는 내 즐겨찾기 볼때만 all로 가져옵니다! ->내꺼 볼때
| .map(b -> BookmarkResponse.builder() | ||
| .bookmarkId(b.getBookmarkId()) | ||
| .memberId(b.getMember().getMemberId()) | ||
| .nickname(b.getMember().getNickname()) | ||
| .imageUrl(b.getImageUrl()) | ||
| .title(b.getTitle()) | ||
| .description(b.getDescription()) | ||
| .pinColor(b.getPinColor()) | ||
| .visibility(b.getVisibility()) | ||
| .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.
이렇게 해도되고
BookmarkResponse -> BookmarkDto 라고 생각할께요
BookmarkDto 내부에 컨버팅 메소드 Page fromBookmarksPage(Page bookmarksPage){} 이렇게 만들어서 처리하기도합니다
스트림식 써서 각각 builder만드는 로직을 저기안에 넣고
해당코드도 나쁜코드는 아님
한번더 리펙토링이 가능하다 이정도요
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
#105
Summary by CodeRabbit
신규 기능
버그 수정
문서화