fix(calendar): preserve eventType when updating Focus Time, OOO, and Working Location events#319
Conversation
![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
This pull request modifies the CalendarService to fetch existing event data before performing an update, ensuring that eventType and type-specific properties are preserved to prevent API errors. Feedback suggests using the patch method instead of update to improve efficiency by removing the extra fetch and to prevent accidental data loss for fields not included in the request body.
| const existing = await calendar.events.get({ | ||
| calendarId: finalCalendarId, | ||
| eventId, | ||
| }); |
There was a problem hiding this comment.
Using calendar.events.patch instead of calendar.events.update is a more efficient and safer approach for partial updates.
- Efficiency:
patchonly updates the fields provided in the request body, which eliminates the need for this extragetrequest to fetch and preserve theeventTypeand other properties. - Safety: The current use of
update(PUT semantics) replaces the entire resource. Any event fields not explicitly handled inUpdateEventInput(such aslocation,reminders,colorId,visibility,transparency, etc.) will be wiped out from the event on the server because they are missing from therequestBody.
Switching to patch solves the eventType issue while also preventing accidental data loss for all other fields. Note that the comment on line 735 already mentions "patch semantics," which further suggests that patch is the intended method.
…all event fields The updateEvent method used events.update (PUT), which replaces the entire event resource. Any field not explicitly included in the request body gets wiped -- including summary, description, reminders, colorId, visibility, recurrence, attachments, and conferenceData. Critically, it also rejects non-default eventTypes (Focus Time, Out of Office, Working Location) with Event type cannot be changed because omitting eventType is interpreted as changing it to default. Switch to events.patch (PATCH), which only modifies the fields present in the request body and preserves everything else. This is the correct HTTP semantics for partial updates and matches the methods documented intent (patch semantics comment was already in the code). Fixes updating Focus Time, Out of Office, and Working Location events. Also fixes data loss when updating any event with only a subset of fields (e.g., changing only the time without re-specifying the title). Ref: https://developers.google.com/calendar/api/v3/reference/events/patch
RussellZager
force-pushed
the
fix/preserve-event-type-on-update
branch
from
3679be0 to
570520f
Compare
…all event fields The updateEvent method used events.update (PUT), which replaces the entire event resource. Any field not explicitly included in the request body gets wiped -- including summary, description, reminders, colorId, visibility, recurrence, attachments, and conferenceData. Critically, it also rejects non-default eventTypes (Focus Time, Out of Office, Working Location) with Event type cannot be changed because omitting eventType is interpreted as changing it to default. Switch to events.patch (PATCH), which only modifies the fields present in the request body and preserves everything else. This is the correct HTTP semantics for partial updates and matches the methods documented intent (patch semantics comment was already in the code). Changes: - Replace events.update with events.patch in CalendarService.ts - Remove unnecessary type cast on applyMeetAndAttachments call - Update all updateEvent test mocks from events.update to events.patch - Add test verifying Focus Time events are preserved on partial update Ref: https://developers.google.com/calendar/api/v3/reference/events/patch
|
I did address also this in #316 |
|
Thanks @galuszkak — just saw #316 covers this as part of the larger refactor. Closing as duplicate. The core fix is the same: |
3 participants
Summary
calendar.updateEventusesevents.update(PUT), which replaces the entire event resource. This causes two bugs:"Event type cannot be changed"— omittingeventTypefrom the PUT body is interpreted as changing it todefaultFix
Switch from
events.update(PUT) toevents.patch(PATCH). PATCH only modifies the fields present in the request body, preserving everything else. This matches the existing code comment (// patch semantics) and is the correct HTTP method for partial updates.1 file changed, 9 insertions, 4 deletions.
Repro (bug 1)
{"error": "Event type cannot be changed."}Repro (bug 2)
Ref
Test plan
node esbuild.config.js)