[BE-FEAT] 사용자 캘린더 정보 조회 기능 구현 #395
Conversation
WalkthroughA new endpoint has been introduced in the Calendar module. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
Client->>Controller: GET /calendar/list
Controller->>Service: getCalendarList()
Service->>Repository: findAllByUserId(currentUser.id)
Repository-->>Service: List<Calendar>
Service->>Controller: ListResponse<CalendarResponse>
Controller-->>Client: ResponseEntity.ok(ListResponse)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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:
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/src/main/java/endolphin/backend/domain/calendar/dto/CalendarResponse.java (1)
3-7: The CalendarResponse record appears to be oversimplified.The current implementation only includes a
namefield, but calendars typically contain more information that users might find useful such as ID, description, color, or timezone. Consider expanding this record to include more relevant calendar properties to provide a richer response to clients.public record CalendarResponse( - String name + String name, + Long id, + String description ) { }backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java (1)
50-54: Consider adding API documentation and following RESTful conventions.While the endpoint implementation looks good, it would benefit from:
- Adding Swagger annotations for API documentation (@operation, @apiresponse)
- Using a more RESTful URL pattern (e.g.,
/calendarsinstead of/calendar/list)- Adding security annotations if necessary to specify access permissions
+@Operation(summary = "Get all calendars for the current user", + description = "Returns a list of calendars associated with the authenticated user") +@ApiResponses(value = { + @ApiResponse(responseCode = "200", description = "Successfully retrieved calendars"), + @ApiResponse(responseCode = "401", description = "Unauthorized, authentication required") +}) -@GetMapping("/calendar/list") +@GetMapping("/calendars") public ResponseEntity<ListResponse<CalendarResponse>> calendarList() { ListResponse<CalendarResponse> response = calendarService.getCalendarList(); return ResponseEntity.ok(response); }backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (1)
131-133: Consider including more properties in the calendar response.The current implementation only maps the calendar name to the response. If
Calendarentities contain more useful information like ID, description, or other metadata, consider including those in the response to provide a more comprehensive view for the client.List<CalendarResponse> response = calendars.stream() - .map(c -> new CalendarResponse(c.getName())).toList(); + .map(c -> new CalendarResponse(c.getName(), c.getId(), c.getDescription())).toList(); return new ListResponse<>(response);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/main/java/endolphin/backend/domain/calendar/CalendarController.java(2 hunks)backend/src/main/java/endolphin/backend/domain/calendar/CalendarRepository.java(1 hunks)backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java(4 hunks)backend/src/main/java/endolphin/backend/domain/calendar/dto/CalendarResponse.java(1 hunks)
🔇 Additional comments (2)
backend/src/main/java/endolphin/backend/domain/calendar/CalendarRepository.java (1)
24-25: LGTM! Method signature follows Spring Data JPA convention.The new repository method properly follows Spring Data JPA naming conventions and complements the existing methods for calendar retrieval.
backend/src/main/java/endolphin/backend/domain/calendar/CalendarService.java (1)
126-134:Details
❓ Verification inconclusive
Verify ListResponse constructor compatibility and error handling.
The current implementation looks good but has a couple of potential issues:
- From the provided code snippets,
ListResponseconstructor appears to require both a list and a totalCount parameter, but only the list is provided here.- There's no explicit error handling for the case when no calendars are found.
🏁 Script executed:
#!/bin/bash # Check ListResponse class definition to verify constructor parameters echo "Checking ListResponse class definition..." fd --type file --exec grep -l "ListResponse" {} \; | xargs grep -n "class ListResponse" -A 20Length of output: 189
I'm not seeing the expected output regarding the definition of the
ListResponseclass. To clarify the concerns about the constructor signature and error handling, let's run an additional shell script to search for theListResponseclass definition and its constructor details across the repository.Please execute this script so we can verify the constructor's parameters and then update the review comment accordingly.
🏁 Script executed:
#!/bin/bash # Search recursively for the ListResponse class definition rg -n "class ListResponse" . # Search for constructor definitions in the ListResponse class rg -n "public ListResponse(" .Length of output: 154
Action: Manually Verify ListResponse Constructor and Handle Empty Results
The implementation appears acceptable overall, but please confirm two points manually:
- Constructor Compatibility: It’s unclear whether the
ListResponseclass’s constructor requires both a list and a total count. The code currently passes only the list. Please verify that a single-argument constructor exists or, if not, update the code to pass the totalCount as well.- Error Handling for No Calendars: Although returning an empty list can be acceptable, consider whether the absence of explicit error handling when no calendars are found aligns with the intended business logic.
efdao
left a comment
efdao
left a comment
There was a problem hiding this comment.
고생하셨습니다. PR 작업내용 더 자세히 적어주세요
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit