feat: 코스 북마크 연관 API 구현#315
Conversation
- 코스 북마크 추가/삭제 PUT {courseId}/bookmarks
- 북마크한 코스 조회 GET /{courseId}/bookmarks
Summary of ChangesHello @lovelyAlien, 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
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 코스 북마크 추가/삭제 및 조회 API를 구현합니다. 전반적으로 코드는 잘 작성되었으며, 기능 구현도 명확합니다. 다만, API 응답 설계와 JPQL 쿼리의 가독성 측면에서 몇 가지 개선점을 제안합니다. toggleBookmark API가 현재 상태를 나타내는 불리언 값을 반환하도록 변경하면 클라이언트에서 더 유용하게 사용할 수 있습니다. 또한, JPQL에서 엔티티 별칭을 일관성 있게 사용하여 코드의 명확성을 높이는 것이 좋겠습니다.
| public ApiResponseDto<String> toggleBookmark(@PathVariable Long courseId, | ||
| @AuthenticationPrincipal User user) { | ||
| return ApiResponseDto.success(HttpStatus.OK.value(), | ||
| routeFacade.toggleBookmark(courseId, user)); | ||
| } |
There was a problem hiding this comment.
toggleBookmark API가 현재 문자열 메시지를 반환하고 있습니다. 이는 로깅에는 유용할 수 있으나, API를 사용하는 클라이언트 입장에서는 북마크의 최종 상태를 알기 위해 문자열을 파싱해야 하는 번거로움이 있습니다. API 응답으로 북마크의 새로운 상태(예: true 또는 false)를 나타내는 불리언 값을 반환하는 것이 더 RESTful하고 실용적입니다. 이렇게 변경하면 RouteService의 toggleBookmark 메서드에서 중복된 문자열 반환 로직도 함께 개선할 수 있습니다. 이 변경을 위해서는 RouteFacade와 RouteService의 관련 메서드들도 boolean을 반환하도록 수정해야 합니다.
| public interface RouteBookmarkRepository extends JpaRepository<RouteBookmark, Long> { | ||
|
|
||
| @Query("SELECT cb FROM RouteBookmark cb JOIN FETCH cb.route WHERE cb.user = :user") | ||
| @Query("SELECT cb FROM RouteBookmark cb JOIN FETCH cb.route WHERE cb.user = :user AND cb.deletedAt IS NULL") |
There was a problem hiding this comment.
JPQL 쿼리에서 RouteBookmark 엔티티의 별칭으로 cb를 사용하고 있습니다. 이는 'CourseBookmark'의 약어로 오해될 수 있어 가독성을 해칠 수 있습니다. 엔티티 이름(RouteBookmark)에 맞게 rb와 같이 더 명확한 별칭을 사용하는 것이 좋겠습니다.
| @Query("SELECT cb FROM RouteBookmark cb JOIN FETCH cb.route WHERE cb.user = :user AND cb.deletedAt IS NULL") | |
| @Query("SELECT rb FROM RouteBookmark rb JOIN FETCH rb.route WHERE rb.user = :user AND rb.deletedAt IS NULL") |
| @Query("SELECT COUNT(cb) FROM RouteBookmark cb " + | ||
| "WHERE cb.route = :route AND cb.deletedAt IS NULL") |
There was a problem hiding this comment.
이 쿼리에서도 RouteBookmark의 별칭으로 cb를 사용하고 있습니다. 코드 전체의 일관성과 명확성을 위해 rb로 변경하는 것을 권장합니다.
| @Query("SELECT COUNT(cb) FROM RouteBookmark cb " + | |
| "WHERE cb.route = :route AND cb.deletedAt IS NULL") | |
| @Query("SELECT COUNT(rb) FROM RouteBookmark rb " + | |
| "WHERE rb.route = :route AND rb.deletedAt IS NULL") |
There was a problem hiding this comment.
Pull request overview
This PR implements course bookmark-related APIs to allow users to bookmark courses and check bookmark status, resolving issue #314.
Changes:
- Added PUT
/api/courses/{courseId}/bookmarksendpoint to toggle course bookmarks (add/delete) - Added GET
/api/courses/{courseId}/bookmarksendpoint to check if a course is bookmarked - Implemented soft delete pattern for course bookmarks with restore functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| RouteController.java | Added two new authenticated endpoints for bookmark toggle and bookmark status check |
| RouteFacade.java | Added facade methods to coordinate bookmark operations with proper transaction boundaries |
| RouteService.java | Implemented core bookmark toggle logic with soft delete/restore, bookmark status check, and route retrieval by ID |
| RouteBookmarkRepository.java | Added repository methods for finding bookmarks by route and user, and checking bookmark existence |
| RouteBookmark.java | Added factory method and soft delete/restore utility methods to support bookmark lifecycle management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public Route getRouteById(Long routeId) { | ||
| return routeRepository.findById(routeId) | ||
| .orElseThrow(() -> new BusinessException(RouteErrorCode.NOT_FOUND)); | ||
| } |
There was a problem hiding this comment.
The getRouteById method should be annotated with @transactional(readOnly = true) for consistency with other read-only query methods in this service class and to ensure proper transaction management.
| @Transactional | ||
| public String toggleBookmark(Route route, User user) { | ||
| Optional<RouteBookmark> existingBookmark = routeBookmarkRepository.findByRouteAndUser(route, | ||
| user); | ||
|
|
||
| if (existingBookmark.isPresent()) { | ||
| RouteBookmark bookmark = existingBookmark.get(); | ||
| if (bookmark.isDeleted()) { | ||
| bookmark.restore(); | ||
| return "코스가 북마크에 추가되었습니다."; | ||
| } else { | ||
| bookmark.softDelete(); | ||
| return "코스가 북마크에서 삭제되었습니다."; | ||
| } | ||
| } else { | ||
| RouteBookmark newBookmark = RouteBookmark.of(route, user); | ||
| routeBookmarkRepository.save(newBookmark); | ||
| return "코스가 북마크에 추가되었습니다."; | ||
| } | ||
| } |
There was a problem hiding this comment.
The new toggleBookmark and isBookmarked methods lack test coverage. Since this repository has comprehensive automated testing for other services (e.g., BadgeServiceTest), consider adding unit tests to verify the bookmark toggle logic, soft delete/restore behavior, and the bookmark status check functionality.
API
PUT {courseId}/bookmarksGET /{courseId}/bookmarksIssue
resolves #314