fix(calendar): tighten createEvent validation for event types#308
fix(calendar): tighten createEvent validation for event types#308allenhutchison merged 3 commits intomainfrom
Conversation
- Reject start/end with both dateTime and date (must be exactly one) - Require summary for regular events (default/unset eventType) - Validate officeLocation/customLocation sub-properties are present for their respective workingLocationProperties.type values - Fix updateEvent datetime validation for all-day events
![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 strengthens the validation logic for event creation in the CalendarService by ensuring exactly one of dateTime or date is provided, requiring summaries for regular events, and mandating location details for working location types. Corresponding unit tests were added to verify these constraints. The review feedback recommends splitting combined validation checks for more granular error messages, moving location validation earlier to fail fast, and replacing truthiness checks with undefined checks to ensure empty strings are correctly validated.
- Split combined start/end validation into separate checks with field-specific error paths - Move working location sub-property validation earlier to fail fast before calendar ID resolution and logging - Use !== undefined instead of truthiness for dateTime checks in updateEvent to catch empty strings
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances validation logic for calendar events in CalendarService.ts, specifically ensuring that event start and end times contain exactly one of dateTime or date. It also adds requirements for summaries on regular events and validates sub-properties for workingLocation event types. Corresponding unit tests have been added to verify these new constraints. One piece of feedback suggests extending the ambiguity check for dateTime and date to the updateEvent method to maintain consistency with the new createEvent validation.
Extend the same exactly-one-of check from createEvent to updateEvent, so providing both dateTime and date (or neither) in an update is rejected early with a clear error.
|
|
||
| // Validate start/end: at least one of dateTime or date must be provided | ||
| if ((!start.dateTime && !start.date) || (!end.dateTime && !end.date)) { | ||
| // Validate start: exactly one of dateTime or date must be provided |
There was a problem hiding this comment.
The validations here make sense. At some point it may make sense to have a validation layer to separate all this logic to make it easier to read.
There was a problem hiding this comment.
Good point filed #313 to cover this work.
2 participants
Summary
Follow-up to #290 — tightens validation in
createEventto catch edge cases that could produce confusing API behavior:dateTimeanddateare provided, reject instead of letting the Google API pick onesummarywas made optional in feat(calendar): introduce eventType (Out of Office, Focus Time, Working Location) in Calendar Service #290 to support status event defaults, but regular events (noeventTypeoreventType: "default") should still require itofficeLocationdetails are now required whentypeis"officeLocation", andcustomLocationdetails whentypeis"customLocation"updateEventwas unconditionally validatingstart.dateTimeeven for date-only updatesTest plan