[BE-FEAT] 롱 폴링 응답 데이터 추가 #408
Conversation
변경 - 추가적인 정보를 위해 새로운 일정의 경우 디비에 저장한 후에 응답 - 수정의 경우 동일 - 삭제의 경우 추가적인 정보에 null 값을 저장해서 처리
## Walkthrough
The synchronization logic for personal events was refactored to return detailed results for each event processed. The service method now returns a list of synchronization results, and a new data structure with additional fields was introduced. API documentation was updated to reflect the new response schema, and handler logic was adjusted to utilize the returned data.
## Changes
| File(s) | Change Summary |
|------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `.../PersonalEventService.java` | Refactored `syncWithGoogleEvents` to return a `List<SyncPersonalEvent>`; added `syncPersonalEventFromGoogleEvent` method; removed `upsertPersonalEventByGoogleEvent`; changed `createWithRequest` to return entity; updated `deletePersonalEventByGoogleEvent` to return Optional. |
| `.../dto/SyncPersonalEvent.java` | Replaced `googleEventId` field with `id`, `isAdjustable`, `calendarId`; changed `status` to enum `Status`; replaced static factory method to create from `PersonalEvent` and `Status`. |
| `.../PersonalEventHandler.java` | Updated `sync` method to capture and use returned sync results from service; removed commented-out code; rearranged variable declarations. |
| `.../PersonalEventController.java` | Changed `createPersonalEvent` to convert entity to response DTO via static factory; enhanced OpenAPI doc for sync endpoint with explicit 200 response schema `SyncResponse`. |
| `.../dto/PersonalEventResponse.java` | Removed `googleEventId` field; updated `fromEntity` factory method accordingly. |
| `.../entity/PersonalEvent.java` | Removed static factory method `fromGoogleEvent`. |
| `.../PersonalEventControllerTest.java` | Modified test to mock and return `PersonalEvent` entity instead of response DTO for creation method. |
| `.../PersonalEventServiceTest.java` | Updated tests to reflect changed return types and usage of getters for `PersonalEvent`; added detailed stubbing for updated personal event in sync tests. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Client
participant Controller
participant Handler
participant Service
participant DB
Client->>Controller: POST /sync-personal-events
Controller->>Handler: handle sync request
Handler->>Service: syncWithGoogleEvents(events, user, calendarId)
loop For each GoogleEvent
Service->>Service: syncPersonalEventFromGoogleEvent(...)
alt CONFIRMED
Service->>DB: find existing PersonalEvent
alt exists
Service->>DB: update PersonalEvent
DB-->>Service: updated PersonalEvent
Service->>Service: create SyncPersonalEvent with UPDATED status
else not exists
Service->>DB: create PersonalEvent
DB-->>Service: created PersonalEvent
Service->>Service: create SyncPersonalEvent with CREATED status
end
else CANCELLED
Service->>DB: delete PersonalEvent
DB-->>Service: deleted PersonalEvent (optional)
Service->>Service: create SyncPersonalEvent with DELETED status if deleted
else TENTATIVE
Service->>Service: no action, returns empty
end
end
Service-->>Handler: List<SyncPersonalEvent>
Handler->>Controller: set result with List<SyncPersonalEvent>
Controller-->>Client: Response with SyncResponse schemaPossibly related PRs
Suggested reviewers
Poem
|
| LocalDateTime endDateTime, | ||
| String status | ||
| ) { | ||
|
|
There was a problem hiding this comment.
just ask; status를 enum 말고 String으로 했던 이유가 있었던가요?
There was a problem hiding this comment.
db를 거쳐도 googleEventId 응답에 포함해야 하나요?
There was a problem hiding this comment.
- 가독성을 위해 enum으로 수정하겠습니다!
- 이전 업데이트 때 일정들을 googleEventId로 구분하도록 프론트에서 반영했을거라 googleEventId를 응답에서 빼는 건 프론트와 협의가 필요할 거 같습니다!
There was a problem hiding this comment.
프론트와 협의한 결과, googleEventId를 응답에서 삭제하기로 결정하였습니다.!
| yield SyncPersonalEvent.from(googleEvent); | ||
| } | ||
| } | ||
| case TENTATIVE -> SyncPersonalEvent.from(googleEvent); |
There was a problem hiding this comment.
db 거치는데 confirmed를 그대로 주는 이유가 있을까요?
update와 create 분기하는 건 어떻게 생각하시나요
There was a problem hiding this comment.
- tentative는 제외하도록 하겠습니다!
- confirmed를 주는 건 cancelled와 구분하기 위해 필요할 거 같습니다!
- update와 create를 upsertPersonalEventByGoogleEvent에서 분기하고 있는데 혹시 어떤 걸 의미하는지 궁금합니다!
There was a problem hiding this comment.
프론트에서 confirmed를 받고 어떤 동작을 수행하고 있는지는 정확히 모르지만,
백에서 내려주는 응답을 생성, 변경, 삭제 세가지의 상태를 정의해서 주면 더 편한 부분이 있지 않을까 하는 제 생각이었습니다.
백엔드 로직에서도 구글로부터 confirmed를 받으면 해당 id의 일정이 존재하는지 확인 후 update, create로 분기를 하는데요, 프론트도 동일한 방법을 수행해야 한다면 효율성이 떨어지지 않을까 해서요!
There was a problem hiding this comment.
백에서 수행한 로직을 프론트에서도 수행하면 비효율적이긴 하겠네요!
어떤 것이 편한 지 프론트와 협의 후 수정하겠습니다!
There was a problem hiding this comment.
프론트와 협의한 결과, 백엔드에서 생성, 변경, 삭제를 구분해서 응답하는 걸로 수정하였습니다.!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
backend/src/main/java/endolphin/backend/domain/personal_event/dto/SyncPersonalEvent.java (1)
15-25: Great improvement using enum for status field!The change from
Stringto enum for thestatusfield improves type safety and addresses the previous review feedback. The enum valuesCREATED,UPDATED, andDELETEDclearly represent the synchronization states.backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (1)
225-241: Excellent implementation addressing previous feedback!The refactored method now returns detailed synchronization results as suggested in previous reviews. This allows the client to understand exactly what operations (create, update, delete) were performed for each event, improving the efficiency of client-side processing.
🧹 Nitpick comments (1)
backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java (1)
240-248: Consider using a test fixture or builder for cleaner test setup.While the mocking is correct, stubbing multiple getters creates brittle tests. Consider creating a test fixture method or using a builder pattern to create actual
PersonalEventinstances instead of mocks for cleaner and more maintainable tests.Example approach:
-PersonalEvent updatedPersonalEvent = createWithRequest("new Title"); -given(updatedPersonalEvent.getId()).willReturn(1L); -given(updatedPersonalEvent.getTitle()).willReturn("new Title"); -given(updatedPersonalEvent.getStartTime()).willReturn(LocalDateTime.of(2024, 3, 10, 10, 0)); -given(updatedPersonalEvent.getEndTime()).willReturn(LocalDateTime.of(2024, 3, 10, 12, 0)); -given(updatedPersonalEvent.getCalendarId()).willReturn("testCalendarId"); -given(updatedPersonalEvent.getIsAdjustable()).willReturn(false); +PersonalEvent updatedPersonalEvent = PersonalEvent.builder() + .id(1L) + .title("new Title") + .startTime(LocalDateTime.of(2024, 3, 10, 10, 0)) + .endTime(LocalDateTime.of(2024, 3, 10, 12, 0)) + .calendarId("testCalendarId") + .isAdjustable(false) + .user(user) + .build();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java(3 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java(7 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/dto/PersonalEventResponse.java(2 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/dto/SyncPersonalEvent.java(1 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java(0 hunks)backend/src/main/java/endolphin/backend/domain/personal_event/event/handler/PersonalEventHandler.java(1 hunks)backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventControllerTest.java(2 hunks)backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- backend/src/main/java/endolphin/backend/domain/personal_event/entity/PersonalEvent.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: backend
🔇 Additional comments (13)
backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventController.java (3)
6-6: Import added for entity handling refactor.The import of
PersonalEvententity is correctly added to support the refactoredcreatePersonalEventmethod.
85-86: Excellent separation of concerns improvement.The refactoring to have the service return a
PersonalEvententity and the controller handle DTO conversion follows good architectural patterns. This separates business logic (service) from presentation concerns (controller).
135-136: API documentation properly updated.The OpenAPI documentation correctly specifies the
SyncResponseschema for the 200 response, providing better API contract clarity for consumers.backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventControllerTest.java (3)
9-9: Necessary imports added for test refactoring.The imports for
PersonalEvententity andMockitoare correctly added to support the updated test logic.Also applies to: 17-17
52-53: Test properly aligned with controller refactoring.The test correctly mocks a
PersonalEvententity and updates the service mock to return this entity, properly reflecting the controller's new behavior of converting entities to DTOs.Also applies to: 58-58
57-57:Details
✅ Verification successful
Verify PersonalEventResponse constructor consistency.
The
PersonalEventResponseconstructor is being called with"testCalendarId"as the last parameter, but according to the DTO changes inPersonalEventResponse.java, thegoogleEventIdfield was removed. Please verify this constructor call is consistent with the updated DTO structure.
🏁 Script executed:
#!/bin/bash # Description: Verify PersonalEventResponse constructor parameters # Expected: Constructor should match the fields in the DTO after googleEventId removal ast-grep --pattern $'public record PersonalEventResponse( $$$ ) { $$$ }'Length of output: 2507
PersonalEventResponse constructor invocation matches updated DTO
I’ve confirmed that
PersonalEventResponseis defined with fields(Long id, String title, LocalDateTime startDateTime, LocalDateTime endDateTime, Boolean isAdjustable, String calendarId). The test’s call ending infalse, "testCalendarId")correctly maps toisAdjustableandcalendarId, and there is nogoogleEventIdfield. No changes are needed.backend/src/main/java/endolphin/backend/domain/personal_event/event/handler/PersonalEventHandler.java (2)
29-29: Good improvement in variable positioning.Moving the
googleCalendarIdretrieval earlier improves code readability and logical flow.
34-35: Excellent utilization of detailed sync results.The handler now properly captures and utilizes the
List<SyncPersonalEvent>returned by the service, enabling more detailed long polling responses with creation and modification event IDs as mentioned in the PR objectives.backend/src/main/java/endolphin/backend/domain/personal_event/dto/PersonalEventResponse.java (1)
22-22: fromEntity method correctly updated.The static factory method properly reflects the removal of the
googleEventIdfield, maintaining consistency between the record structure and the conversion logic.backend/src/test/java/endolphin/backend/domain/personal_event/PersonalEventServiceTest.java (1)
109-115: LGTM! Test correctly updated for the new return type.The test has been properly updated to handle the service method returning a
PersonalEvententity instead of aPersonalEventResponseDTO. The assertions correctly use getter methods to validate the entity's properties.backend/src/main/java/endolphin/backend/domain/personal_event/PersonalEventService.java (3)
69-100: Good change returning the entity directly.Returning the
PersonalEvententity from the service layer and letting the controller handle the DTO conversion is a cleaner separation of concerns.
243-274: Clean implementation with modern Java features!The use of switch expressions and Optional makes the code more readable and type-safe. The logic correctly implements the create/update/delete synchronization states as discussed in previous reviews.
However, there's a potential inconsistency: when creating a new event (lines 260-263), the
discussionsparameter is not used, while updates do process discussions. Consider whether newly created events should also be preprocessed for discussions.
282-291: Good use of Optional pattern for delete operations.The method correctly returns the deleted event wrapped in Optional, allowing the sync logic to include deletion details in the response. The implementation follows Optional best practices.
| Boolean isAdjustable, | ||
| String calendarId, | ||
| String googleEventId | ||
| String calendarId |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Appropriate removal of Google-specific field.
Removing the googleEventId field from the public API response is a good architectural decision that reduces coupling with Google Calendar implementation details.
Consider if this removal constitutes a breaking change for existing API consumers. Please verify if there are any client applications that depend on this field.
🏁 Script executed:
#!/bin/bash
# Description: Search for usage of googleEventId in client code or documentation
# Expected: Should identify any dependencies on this field
rg -i "googleeventid" --type-add 'docs:*.md,*.txt,*.rst' --type docs
rg -i "google.*event.*id" --type-add 'frontend:*.js,*.ts,*.jsx,*.tsx' --type frontendLength of output: 454
I’m not finding any occurrences because the previous ripgrep commands were overly restrictive. Let’s search the entire repo for both camelCase and snake_case variants:
🏁 Script executed:
#!/bin/bash
# Search for any references to googleEventId or google_event_id across all files
rg -i "googleEventId" -n .
rg -i "google_event_id" -n .Length of output: 6031
Breaking Change: Update Front-end Model After Removing googleEventId
The API DTO no longer includes googleEventId, but your front-end schema still expects it—this will break existing clients. Please remove or adapt all references to googleEventId in consumer code:
• frontend/src/features/my-calendar/model/index.ts
– Remove the googleEventId: z.string(), field (line 10)
– Remove googleEventId: true from the selected properties object (line 16)
After updating the front-end, verify end-to-end functionality (e.g., calendar listing, event details) to ensure no runtime errors occur when the field is absent.
🤖 Prompt for AI Agents
In
backend/src/main/java/endolphin/backend/domain/personal_event/dto/PersonalEventResponse.java
at line 12, the googleEventId field was removed, causing a breaking change for
front-end clients. To fix this, update the front-end model by removing the
googleEventId field declaration at line 10 and removing googleEventId: true from
the selected properties object at line 16 in
frontend/src/features/my-calendar/model/index.ts. After these changes, test the
full flow to ensure no runtime errors occur due to the missing field.
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
추가
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit