feat(providers): tighten sdks types, add pagination, and typed errors - #468
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2ca4c70 to
aff161b
Compare
There was a problem hiding this comment.
15 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/components/calendar/flows/update-event/update-event-attendee-dialog.tsx">
<violation number="1" location="apps/web/src/components/calendar/flows/update-event/update-event-attendee-dialog.tsx:25">
P3: The Microsoft notification check and disabled Save behavior are now repeated verbatim across all attendee dialogs, so future provider-policy changes can leave one flow inconsistent with the others. A shared notification-choice component or helper for the provider rule would keep create, update, and delete behavior aligned.</violation>
</file>
<file name="packages/providers/src/interfaces/calendars.ts">
<violation number="1" location="packages/providers/src/interfaces/calendars.ts:26">
P2: The new typed-error variant is not produced by either free/busy adapter: Microsoft still rejects on `info.error`, while Google ignores `calendar.errors` and returns an empty `busy` result. This leaves the public typed-error path dead and causes per-calendar failures to be rejected or misreported as no availability; the adapters should normalize and return this shape, or the variant should be removed.</violation>
</file>
<file name="packages/providers/src/calendars/google-calendar/events/index.ts">
<violation number="1" location="packages/providers/src/calendars/google-calendar/events/index.ts:296">
P2: Updating events with completed or partial Google conference data can fail or drop the existing conference: `toGoogleCalendarEventInput(existingEvent)` does not preserve completed `createRequest` data and assumes `entryPoints` exists. Align the conversion with `parseConferenceData` (only treat non-success requests as pending and handle missing entry points) before sending the full update.</violation>
</file>
<file name="packages/providers/src/conferencing/google-meet.ts">
<violation number="1" location="packages/providers/src/conferencing/google-meet.ts:32">
P3: Conference creation now consumes more Google Calendar quota per call: `events.patch` costs 3 quota units, while the previous `get` plus `update` flow costs 2. Retaining the existing get/update sequence, or documenting and accepting this quota tradeoff, avoids a measurable API-quota regression.</violation>
</file>
<file name="packages/google-people/src/error.ts">
<violation number="1" location="packages/google-people/src/error.ts:28">
P2: Consumers handling batch contact failures cannot receive the specialized detail types through `APIError.error`: `details` is restricted to `GooglePeopleErrorDetail[]`, while `BatchCreateContactsErrorDetails` and `BatchUpdateContactsErrorDetails` are excluded. Including these variants in the details union would make the typed error definitions usable without casts.</violation>
</file>
<file name="packages/microsoft-calendar/src/error.ts">
<violation number="1" location="packages/microsoft-calendar/src/error.ts:22">
P2: The public `ODataInnerError` type rejects the string fields Microsoft Graph actually returns and gives consumers the wrong type when reading them. Using `Record<string, unknown>` (or a recursive model with string fields) keeps valid Graph errors assignable and accurately represents the payload.</violation>
</file>
<file name="packages/google-calendar/src/interfaces.ts">
<violation number="1" location="packages/google-calendar/src/interfaces.ts:8">
P3: Consumers cannot pass standard Fetch `HeadersInit` values such as a `Headers` object or tuple list through `GoogleCalendarRequestOptions.headers`. Typing this alias as `HeadersInit` would match the implementation and the Fetch API accepted inputs.</violation>
</file>
<file name="packages/api/src/routers/events.ts">
<violation number="1" location="packages/api/src/routers/events.ts:241">
P2: Microsoft event updates silently ignore the top-level `sendUpdate` value, so a `false` notification choice is not honored for ordinary edits. The provider should either propagate an equivalent control or reject unsupported values instead of accepting the flag as if it worked.</violation>
</file>
<file name="packages/google-calendar/src/freebusy/interfaces.ts">
<violation number="1" location="packages/google-calendar/src/freebusy/interfaces.ts:34">
P2: A valid `fields` partial-response request can omit `calendars` (or nested `busy`/`start`/`end`), but this type now guarantees those properties exist, so consumers can dereference `undefined` at runtime. These response properties should remain optional, or the SDK should model the response from the requested field mask.</violation>
</file>
<file name="packages/providers/src/calendars/google-calendar/events/utils.ts">
<violation number="1" location="packages/providers/src/calendars/google-calendar/events/utils.ts:136">
P2: Existing file attachments can be dropped when an unrelated event update is sent. Since this mapper now includes attachments in the full PUT body, the update request should set `supportsAttachments: true` whenever attachments are preserved.</violation>
<violation number="2" location="packages/providers/src/calendars/google-calendar/events/utils.ts:148">
P2: Events using a Google event label lose that label on any update. The full-replacement input should preserve `eventLabelId` and send `eventLabelVersion: 1` when that field is present.</violation>
</file>
<file name="packages/google-calendar/src/client.ts">
<violation number="1" location="packages/google-calendar/src/client.ts:63">
P2: Per-request `Authorization` and `Content-Type` headers are silently discarded because the SDK defaults overwrite them after construction. Installing each default only when `requestHeaders` does not already contain that key would make the new headers API behave consistently with its other values.</violation>
</file>
<file name="packages/providers/src/interfaces/providers/calendar.ts">
<violation number="1" location="packages/providers/src/interfaces/providers/calendar.ts:77">
P2: Microsoft callers cannot honor `sendUpdate: false`: `MicrosoftCalendarEvents.create` ignores the field and always posts `toMicrosoftEvent(event)`, so Graph sends invitations when attendees are present. The provider should reject unsupported `false` (as `delete` does) or implement an equivalent suppression path before exposing this option in the shared interface.</violation>
<violation number="2" location="packages/providers/src/interfaces/providers/calendar.ts:92">
P1: Deleting a Microsoft event with an `etag` still issues an unconditional DELETE because `MicrosoftCalendarEvents.delete` drops it, even though the SDK accepts `ifMatch` and emits `If-Match`. Passing the value through would make stale reads fail instead of deleting a concurrently changed event.</violation>
</file>
<file name="packages/google-people/src/interfaces.ts">
<violation number="1" location="packages/google-people/src/interfaces.ts:9">
P3: The raw and JSON query contracts now duplicate the same exclusion list, allowing the two definitions to drift. Reusing `GooglePeopleRawQueryParams` from `client.ts` would keep these SDK query types synchronized.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export interface CalendarProviderEventsDeleteOptions { | ||
| calendarId: string; | ||
| eventId: string; | ||
| etag?: string; |
There was a problem hiding this comment.
P1: Deleting a Microsoft event with an etag still issues an unconditional DELETE because MicrosoftCalendarEvents.delete drops it, even though the SDK accepts ifMatch and emits If-Match. Passing the value through would make stale reads fail instead of deleting a concurrently changed event.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/providers/src/interfaces/providers/calendar.ts, line 92:
<comment>Deleting a Microsoft event with an `etag` still issues an unconditional DELETE because `MicrosoftCalendarEvents.delete` drops it, even though the SDK accepts `ifMatch` and emits `If-Match`. Passing the value through would make stale reads fail instead of deleting a concurrently changed event.</comment>
<file context>
@@ -82,11 +83,13 @@ export interface CalendarProviderEventsUpdateOptions {
export interface CalendarProviderEventsDeleteOptions {
calendarId: string;
eventId: string;
+ etag?: string;
sendUpdate: boolean;
}
</file context>
| | undefined; | ||
| export type QueryParams = Record<string, QueryParamValue>; | ||
|
|
||
| export type GooglePeopleRawQueryParams = QueryParams & { |
There was a problem hiding this comment.
P3: The raw and JSON query contracts now duplicate the same exclusion list, allowing the two definitions to drift. Reusing GooglePeopleRawQueryParams from client.ts would keep these SDK query types synchronized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/google-people/src/interfaces.ts, line 9:
<comment>The raw and JSON query contracts now duplicate the same exclusion list, allowing the two definitions to drift. Reusing `GooglePeopleRawQueryParams` from `client.ts` would keep these SDK query types synchronized.</comment>
<file context>
@@ -6,10 +6,25 @@ export type QueryParamValue =
| undefined;
export type QueryParams = Record<string, QueryParamValue>;
+export type GooglePeopleRawQueryParams = QueryParams & {
+ alt?: never;
+ $alt?: never;
</file context>
| snapshot.matches("askNotifyAttendee"), | ||
| ); | ||
| // Microsoft Calendar always notifies attendees, so saving without notifying is not supported. | ||
| const notifyRequired = UpdateQueueContext.useSelector( |
There was a problem hiding this comment.
P3: The Microsoft notification check and disabled Save behavior are now repeated verbatim across all attendee dialogs, so future provider-policy changes can leave one flow inconsistent with the others. A shared notification-choice component or helper for the provider rule would keep create, update, and delete behavior aligned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/calendar/flows/update-event/update-event-attendee-dialog.tsx, line 25:
<comment>The Microsoft notification check and disabled Save behavior are now repeated verbatim across all attendee dialogs, so future provider-policy changes can leave one flow inconsistent with the others. A shared notification-choice component or helper for the provider rule would keep create, update, and delete behavior aligned.</comment>
<file context>
@@ -21,6 +21,11 @@ export function UpdateEventAttendeeDialog() {
snapshot.matches("askNotifyAttendee"),
);
+ // Microsoft Calendar always notifies attendees, so saving without notifying is not supported.
+ const notifyRequired = UpdateQueueContext.useSelector(
+ (snapshot) =>
+ snapshot.context.item?.event.calendar.provider.id === "microsoft",
</file context>
| } | ||
|
|
||
| const existingEvent = await this.client.events.get({ | ||
| const updatedEvent = await this.client.events.patch({ |
There was a problem hiding this comment.
P3: Conference creation now consumes more Google Calendar quota per call: events.patch costs 3 quota units, while the previous get plus update flow costs 2. Retaining the existing get/update sequence, or documenting and accepting this quota tradeoff, avoids a measurable API-quota regression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/providers/src/conferencing/google-meet.ts, line 32:
<comment>Conference creation now consumes more Google Calendar quota per call: `events.patch` costs 3 quota units, while the previous `get` plus `update` flow costs 2. Retaining the existing get/update sequence, or documenting and accepting this quota tradeoff, avoids a measurable API-quota regression.</comment>
<file context>
@@ -29,15 +29,9 @@ export class GoogleMeetProvider implements ConferencingProvider {
}
- const existingEvent = await this.client.events.get({
+ const updatedEvent = await this.client.events.patch({
calendarId,
eventId,
</file context>
| | null | ||
| | undefined; | ||
| export type QueryParams = Record<string, QueryParamValue>; | ||
| export type RequestHeaders = Record<string, string>; |
There was a problem hiding this comment.
P3: Consumers cannot pass standard Fetch HeadersInit values such as a Headers object or tuple list through GoogleCalendarRequestOptions.headers. Typing this alias as HeadersInit would match the implementation and the Fetch API accepted inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/google-calendar/src/interfaces.ts, line 8:
<comment>Consumers cannot pass standard Fetch `HeadersInit` values such as a `Headers` object or tuple list through `GoogleCalendarRequestOptions.headers`. Typing this alias as `HeadersInit` would match the implementation and the Fetch API accepted inputs.</comment>
<file context>
@@ -5,9 +5,11 @@ export type QueryParamValue =
| null
| undefined;
export type QueryParams = Record<string, QueryParamValue>;
+export type RequestHeaders = Record<string, string>;
export interface GoogleCalendarRequestOptions {
</file context>
| export type RequestHeaders = Record<string, string>; | |
| export type RequestHeaders = HeadersInit; |
aff161b to
c0d1302
Compare
Description
Briefly describe what you did and why.
Screenshots / Recordings
Add screenshots or recordings here to help reviewers understand your changes.
Type of Change
Related Areas
Testing
Checklist
Notes
(Optional) Add anything else you'd like to share.
By submitting, I confirm I understand and stand behind this code. If AI was used, I’ve reviewed and verified everything myself.
Summary by cubic
Tightened provider SDK types, added pagination helpers, and introduced typed errors across
@analog/google-calendar,@analog/microsoft-calendar, and@analog/google-people. Wired attendee notifications end-to-end with a sendUpdate flag; Microsoft calendars always notify.New Features
labelProperties, stricter ACL role/scope enums, typed channel stop, requiredbusyarrays in free/busy, header injection support.personFieldsand"people/me"resource name), ContactGroups.get, batch delete returns void, refined sources.@odata.nextLinkhandling, requiredscheduleIdin free/busy results.sendUpdateon create/update (default true); dialogs and menu disable “save without notifying” for Microsoft using provider-aware gating.sendUpdate.isAllDaytoallDay, preservedresponsein form state, and added deferred field patching so drags/RSVPs apply cleanly in dirty forms.events.patchwithconferenceDataVersion=1.Migration
events.createandevents.updateaccept optionalsendUpdate(default true); pass false to suppress notifications where allowed.nullclears a field;undefinedleaves it unchanged.notifications.unsubscribenow requiresidandresourceId.replacerequests can include aprevioussnapshot; diffs and If-Match use it to 412 on true conflicts.onSuccessreturns the canonical event so callers can advance their baseline.isAllDaytoallDay; update schema and usages.response(SELF attendee) for accurate RSVP diffs.id/resourceId, calendar list items always an array, free/busybusyalways present; PeoplepersonFieldsrequired andresourceNameconstrained).scheduleIdrequired in free/busy responses; adopt new headers parameters where applicable.Written for commit c0d1302. Summary will update on new commits.