Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/webhook-null-leaves.md
Original file line number Diff line number Diff line change
@@ -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).
24 changes: 24 additions & 0 deletions src/client/ghin/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} }))
Expand Down
11 changes: 6 additions & 5 deletions src/client/ghin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
28 changes: 21 additions & 7 deletions src/client/ghin/models/webhooks/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,21 @@ export type WebhookEventType = z.infer<typeof schemaWebhookEventType>
export const schemaWebhookDataType = z.enum(['all', 'changes_only'])
export type WebhookDataType = z.infer<typeof schemaWebhookDataType>

const partialEventMap = <T extends z.ZodTypeAny>(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 = <T extends z.ZodTypeAny>(value: T) =>
z.object(
Object.fromEntries(WEBHOOK_EVENT_TYPES.map((event) => [event, value.nullable().optional()])) as {
[K in WebhookEventType]: z.ZodOptional<z.ZodNullable<T>>
},
)

// 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 = <T extends z.ZodTypeAny>(value: T) =>
z.object(
Object.fromEntries(WEBHOOK_EVENT_TYPES.map((event) => [event, value.optional()])) as {
[K in WebhookEventType]: z.ZodOptional<T>
Expand All @@ -22,18 +36,18 @@ const partialEventMap = <T extends z.ZodTypeAny>(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<typeof schemaWebhookSettings>

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) => {
Expand Down
Loading