diff --git a/.changeset/webhook-null-leaves.md b/.changeset/webhook-null-leaves.md new file mode 100644 index 0000000..a2c6732 --- /dev/null +++ b/.changeset/webhook-null-leaves.md @@ -0,0 +1,5 @@ +--- +'@spicygolf/ghin': patch +--- + +Fix webhook settings GET to accept `null` leaves. GHIN returns every event key on every top-level field with `null` as the "unregistered" sentinel rather than omitting the key, which previously caused `schemaWebhookSettings` parsing to fail and `ensureRegistered` to misreport state. The response schema now allows `string | null | undefined` per leaf while PATCH bodies retain the stricter "optional, no null" shape (use `''` to clear a URL). diff --git a/src/client/ghin/index.test.ts b/src/client/ghin/index.test.ts index d31d55e..cbc90d2 100644 --- a/src/client/ghin/index.test.ts +++ b/src/client/ghin/index.test.ts @@ -1254,6 +1254,30 @@ describe('GhinClient', () => { expect(result.changed).toBe(true) }) + it('should PATCH when GHIN returns null leaves (unregistered sentinel)', async () => { + // GHIN's GET response always includes every event key with `null` for + // unset slots, not an empty object. Verifies the response schema + // accepts null and ensureRegistered treats it as "not set". + mockFetchCustomPath + .mockResolvedValueOnce( + ok({ + webhook_url: { golfer: null, score: null, revision: null, club: null, course: null, gpa: null }, + webhook_data_type: { golfer: null, score: null, revision: null, club: null, course: null, gpa: null }, + webhook_enabled: { golfer: null, score: null, revision: null, club: null, course: null, gpa: null }, + }), + ) + .mockResolvedValueOnce(ok(matchingSettings)) + + const result = await ghinClient.webhooks.ensureRegistered({ + event: 'revision', + url: 'https://example.com/hooks', + }) + + expect(result.changed).toBe(true) + expect(result.reason).toMatch(/url differs.*\(not set\)/) + expect(mockFetchCustomPath).toHaveBeenCalledTimes(2) + }) + it('should honor non-default dataType and enabled', async () => { mockFetchCustomPath .mockResolvedValueOnce(ok({ webhook_url: {}, webhook_data_type: {}, webhook_enabled: {} })) diff --git a/src/client/ghin/index.ts b/src/client/ghin/index.ts index 691e4a7..c4cbd9c 100644 --- a/src/client/ghin/index.ts +++ b/src/client/ghin/index.ts @@ -1149,11 +1149,12 @@ const ITERATE_UNDELIVERED_MAX_PAGES = 10_000 // Strip trailing slashes so e.g. `https://x/y/` and `https://x/y` compare // equal — avoids a PATCH every boot when GHIN normalizes the registered URL -// differently than the caller. -const normalizeWebhookUrl = (url: string | undefined): string | undefined => - url === undefined ? undefined : url.replace(/\/+$/, '') +// differently than the caller. Treats null (GHIN's "unregistered" sentinel +// in GET responses) the same as undefined. +const normalizeWebhookUrl = (url: string | null | undefined): string | undefined => + url == null ? undefined : url.replace(/\/+$/, '') -const describeLeaf = (value: string | boolean | undefined): string => - value === undefined ? '(not set)' : String(value) +const describeLeaf = (value: string | boolean | null | undefined): string => + value == null ? '(not set)' : String(value) export * from './models' diff --git a/src/client/ghin/models/webhooks/settings.ts b/src/client/ghin/models/webhooks/settings.ts index afd4a0a..231e114 100644 --- a/src/client/ghin/models/webhooks/settings.ts +++ b/src/client/ghin/models/webhooks/settings.ts @@ -8,7 +8,21 @@ export type WebhookEventType = z.infer export const schemaWebhookDataType = z.enum(['all', 'changes_only']) export type WebhookDataType = z.infer -const partialEventMap = (value: T) => +// GHIN's GET response always includes every event key under each top-level +// field, with `null` as the "unregistered" sentinel — fields aren't omitted, +// they're nulled. So response inner schemas must accept both undefined +// (defensive — older deployments may omit) and null (observed shape). +const partialEventMapResponse = (value: T) => + z.object( + Object.fromEntries(WEBHOOK_EVENT_TYPES.map((event) => [event, value.nullable().optional()])) as { + [K in WebhookEventType]: z.ZodOptional> + }, + ) + +// PATCH bodies we send keep the original "optional, no null" shape — we +// don't want to invite callers to send `null`, which GHIN's docs don't +// cover. The empty-string sentinel below handles the "clear a URL" case. +const partialEventMapRequest = (value: T) => z.object( Object.fromEntries(WEBHOOK_EVENT_TYPES.map((event) => [event, value.optional()])) as { [K in WebhookEventType]: z.ZodOptional @@ -22,18 +36,18 @@ const partialEventMap = (value: T) => const webhookPatchUrl = z.union([z.literal(''), z.string().url()]) export const schemaWebhookSettings = z.object({ - webhook_url: partialEventMap(z.string()).optional().default({}), - webhook_data_type: partialEventMap(schemaWebhookDataType).optional().default({}), - webhook_enabled: partialEventMap(z.boolean()).optional().default({}), + webhook_url: partialEventMapResponse(z.string()).optional().default({}), + webhook_data_type: partialEventMapResponse(schemaWebhookDataType).optional().default({}), + webhook_enabled: partialEventMapResponse(z.boolean()).optional().default({}), }) export type WebhookSettings = z.infer export const schemaWebhookSettingsPatch = z .object({ - webhook_url: partialEventMap(webhookPatchUrl).optional(), - webhook_data_type: partialEventMap(schemaWebhookDataType).optional(), - webhook_enabled: partialEventMap(z.boolean()).optional(), + webhook_url: partialEventMapRequest(webhookPatchUrl).optional(), + webhook_data_type: partialEventMapRequest(schemaWebhookDataType).optional(), + webhook_enabled: partialEventMapRequest(z.boolean()).optional(), }) .refine( (value) => {