From 3a72b128de26e966e445463af09aa2b3468400fd Mon Sep 17 00:00:00 2001 From: Danny White <3104761+dnywh@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:49:25 +1100 Subject: [PATCH] chore(studio): standardise key-value field array partial-row validation (#44411) ## What kind of change does this PR introduce? Design system and validation consistency update. ## What is the current behaviour? `KeyValueFieldArray` already renders per-cell form messages, but each consumer still decides its own validation rules. At the moment, some consumers allow partially filled rows to submit silently, while Log Drains now treats them as inline validation errors. ## What is the new behaviour? This PR standardises the recommended partial-row behaviour for the current `KeyValueFieldArray` consumers by introducing a shared validation helper and using it from each form schema. - adds `getKeyValueFieldArrayValidationIssues` alongside `KeyValueFieldArray` - keeps `KeyValueFieldArray` presentation-only and leaves validation in consumer schemas - shows inline errors when one side of a key/value row is filled and the other is empty - keeps fully empty rows as draft rows - keeps duplicate-key validation in Log Drains, where it already applies - updates the design-system docs and examples to describe the validation pattern explicitly ## Summary by CodeRabbit * **New Features** * Added reusable key/value validation utilities and public export; forms now trim header/key/value inputs, show inline errors for partially filled rows, and remove fully empty draft rows on submit. * **Documentation** * Clarified the field-array is rendering-only and added guidance for placing validation in form schemas and handling draft rows. * **Tests** * Added unit and integration tests covering validation rules, duplicate keys, trimming, draft-row stripping, and payload behavior. --- .../docs/fragments/key-value-field-array.mdx | 32 +++++ .../content/docs/ui-patterns/forms.mdx | 2 + .../example/form-patterns-pagelayout.tsx | 53 +++++--- .../example/form-patterns-sidepanel.tsx | 53 +++++--- .../example/key-value-field-array-demo.tsx | 31 ++++- .../Hooks/EditHookPanel.constants.test.ts | 42 ++++++ .../Database/Hooks/EditHookPanel.constants.ts | 41 +++++- .../CreateCronJobSheet.constants.test.ts | 46 +++++++ .../CreateCronJobSheet.constants.ts | 31 ++++- .../interfaces/LogDrains/LogDrains.utils.ts | 53 ++------ .../PlatformWebhooksEndpointSheet.test.tsx | 38 +++++- .../PlatformWebhooksEndpointSheet.tsx | 24 +++- packages/ui-patterns/package.json | 4 + .../KeyValueFieldArray/KeyValueFieldArray.tsx | 6 + .../KeyValueFieldArray/validation.test.ts | 121 ++++++++++++++++++ .../src/form/KeyValueFieldArray/validation.ts | 121 ++++++++++++++++++ 16 files changed, 609 insertions(+), 89 deletions(-) create mode 100644 packages/ui-patterns/src/form/KeyValueFieldArray/validation.test.ts create mode 100644 packages/ui-patterns/src/form/KeyValueFieldArray/validation.ts diff --git a/apps/design-system/content/docs/fragments/key-value-field-array.mdx b/apps/design-system/content/docs/fragments/key-value-field-array.mdx index 0bdd124791a..acfeea88d15 100644 --- a/apps/design-system/content/docs/fragments/key-value-field-array.mdx +++ b/apps/design-system/content/docs/fragments/key-value-field-array.mdx @@ -19,6 +19,7 @@ Use `KeyValueFieldArray` when each row is two text inputs backed by `react-hook- ```tsx import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValueFieldArray' +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' ``` ```tsx @@ -36,6 +37,37 @@ import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValue `KeyValueFieldArray` owns the row add/remove behavior and renders the per-input form messages for you. Compose it inside `FormItemLayout` when you want the standard label, description, and message treatment around the entire section. +## Validation + +`KeyValueFieldArray` is rendering-only. Keep validation in the consumer schema and use the shared validation helper when you want the standard draft-row behaviour: + +- fully empty rows may remain drafts +- partially filled rows should show inline errors on the missing cell + +If you persist the array rows directly, strip fully empty draft rows before saving them. + +```tsx +const formSchema = z + .object({ + headers: z.array(z.object({ name: z.string().trim(), value: z.string().trim() })), + }) + .superRefine((data, ctx) => { + getKeyValueFieldArrayValidationIssues({ + rows: data.headers, + keyFieldName: 'name', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: ['headers', ...issue.path], + }) + }) + }) +``` + ## When to use it - Use [Single Value Field Array](./single-value-field-array) for repeated single values such as redirect URIs. diff --git a/apps/design-system/content/docs/ui-patterns/forms.mdx b/apps/design-system/content/docs/ui-patterns/forms.mdx index 4dd556f82a8..33097def25b 100644 --- a/apps/design-system/content/docs/ui-patterns/forms.mdx +++ b/apps/design-system/content/docs/ui-patterns/forms.mdx @@ -38,6 +38,8 @@ Use the shared [Single Value Field Array](../fragments/single-value-field-array) Use the shared [Key/Value Field Array](../fragments/key-value-field-array) fragment when each row is two text inputs managed by `react-hook-form`. +Keep repeated-row validation in the form schema or shared validation helper, not in the fragment component itself. + Build a custom row when the cells are mixed controls, such as an input paired with a `Select`. ## Best Practices diff --git a/apps/design-system/registry/default/example/form-patterns-pagelayout.tsx b/apps/design-system/registry/default/example/form-patterns-pagelayout.tsx index 9cd8af4d7f9..a58732fb100 100644 --- a/apps/design-system/registry/default/example/form-patterns-pagelayout.tsx +++ b/apps/design-system/registry/default/example/form-patterns-pagelayout.tsx @@ -35,6 +35,7 @@ import { import { Input } from 'ui-patterns/DataInputs/Input' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValueFieldArray' +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import { SingleValueFieldArray } from 'ui-patterns/form/SingleValueFieldArray/SingleValueFieldArray' import { MultiSelector, @@ -52,24 +53,40 @@ import { } from 'ui-patterns/PageSection' import * as z from 'zod' -const formSchema = z.object({ - name: z.string().min(1, 'Name is required'), - description: z.string().optional(), - maxConnections: z.number().min(1).max(1000), - enableFeature: z.boolean(), - enableRls: z.boolean(), - enableNotifications: z.boolean(), - enableAnalytics: z.boolean(), - region: z.string().min(1, 'Region is required'), - schemas: z.array(z.string()).min(1, 'At least one schema is required'), - queueType: z.enum(['basic', 'partitioned']), - expiryDate: z.date().optional(), - password: z.string().min(8, 'Password must be at least 8 characters'), - duration: z.number().min(5).max(30), - redirectUris: z.array(z.object({ value: z.string().url('Must be a valid URL') })), - httpHeaders: z.array(z.object({ key: z.string(), value: z.string() })), - apiKey: z.string().optional(), -}) +const formSchema = z + .object({ + name: z.string().min(1, 'Name is required'), + description: z.string().optional(), + maxConnections: z.number().min(1).max(1000), + enableFeature: z.boolean(), + enableRls: z.boolean(), + enableNotifications: z.boolean(), + enableAnalytics: z.boolean(), + region: z.string().min(1, 'Region is required'), + schemas: z.array(z.string()).min(1, 'At least one schema is required'), + queueType: z.enum(['basic', 'partitioned']), + expiryDate: z.date().optional(), + password: z.string().min(8, 'Password must be at least 8 characters'), + duration: z.number().min(5).max(30), + redirectUris: z.array(z.object({ value: z.string().url('Must be a valid URL') })), + httpHeaders: z.array(z.object({ key: z.string().trim(), value: z.string().trim() })), + apiKey: z.string().optional(), + }) + .superRefine((data, ctx) => { + getKeyValueFieldArrayValidationIssues({ + rows: data.httpHeaders, + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: ['httpHeaders', ...issue.path], + }) + }) + }) const fakeApiKey = 'sk_live_51H3x4mpl3_4nd_53cur3_k3y_1234567890' diff --git a/apps/design-system/registry/default/example/form-patterns-sidepanel.tsx b/apps/design-system/registry/default/example/form-patterns-sidepanel.tsx index 6cfa7b74b82..42ff3887563 100644 --- a/apps/design-system/registry/default/example/form-patterns-sidepanel.tsx +++ b/apps/design-system/registry/default/example/form-patterns-sidepanel.tsx @@ -38,6 +38,7 @@ import { import { Input } from 'ui-patterns/DataInputs/Input' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValueFieldArray' +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import { SingleValueFieldArray } from 'ui-patterns/form/SingleValueFieldArray/SingleValueFieldArray' import { MultiSelector, @@ -48,24 +49,40 @@ import { } from 'ui-patterns/multi-select' import * as z from 'zod' -const formSchema = z.object({ - name: z.string().min(1, 'Name is required'), - description: z.string().optional(), - maxConnections: z.number().min(1).max(1000), - enableFeature: z.boolean(), - enableRls: z.boolean(), - enableNotifications: z.boolean(), - enableAnalytics: z.boolean(), - region: z.string().min(1, 'Region is required'), - schemas: z.array(z.string()).min(1, 'At least one schema is required'), - queueType: z.enum(['basic', 'partitioned']), - expiryDate: z.date().optional(), - password: z.string().min(8, 'Password must be at least 8 characters'), - duration: z.number().min(5).max(30), - redirectUris: z.array(z.object({ value: z.string().url('Must be a valid URL') })), - httpHeaders: z.array(z.object({ key: z.string(), value: z.string() })), - apiKey: z.string().optional(), -}) +const formSchema = z + .object({ + name: z.string().min(1, 'Name is required'), + description: z.string().optional(), + maxConnections: z.number().min(1).max(1000), + enableFeature: z.boolean(), + enableRls: z.boolean(), + enableNotifications: z.boolean(), + enableAnalytics: z.boolean(), + region: z.string().min(1, 'Region is required'), + schemas: z.array(z.string()).min(1, 'At least one schema is required'), + queueType: z.enum(['basic', 'partitioned']), + expiryDate: z.date().optional(), + password: z.string().min(8, 'Password must be at least 8 characters'), + duration: z.number().min(5).max(30), + redirectUris: z.array(z.object({ value: z.string().url('Must be a valid URL') })), + httpHeaders: z.array(z.object({ key: z.string().trim(), value: z.string().trim() })), + apiKey: z.string().optional(), + }) + .superRefine((data, ctx) => { + getKeyValueFieldArrayValidationIssues({ + rows: data.httpHeaders, + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: ['httpHeaders', ...issue.path], + }) + }) + }) const fakeApiKey = 'sk_live_51H3x4mpl3_4nd_53cur3_k3y_1234567890' diff --git a/apps/design-system/registry/default/example/key-value-field-array-demo.tsx b/apps/design-system/registry/default/example/key-value-field-array-demo.tsx index 52debebb64f..d8b6af42c6d 100644 --- a/apps/design-system/registry/default/example/key-value-field-array-demo.tsx +++ b/apps/design-system/registry/default/example/key-value-field-array-demo.tsx @@ -3,16 +3,33 @@ import { useForm } from 'react-hook-form' import { Button, Form_Shadcn_ } from 'ui' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValueFieldArray' +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import { z } from 'zod' -const formSchema = z.object({ - headers: z.array( - z.object({ - name: z.string(), - value: z.string(), +const formSchema = z + .object({ + headers: z.array( + z.object({ + name: z.string().trim(), + value: z.string().trim(), + }) + ), + }) + .superRefine((data, ctx) => { + getKeyValueFieldArrayValidationIssues({ + rows: data.headers, + keyFieldName: 'name', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: ['headers', ...issue.path], + }) }) - ), -}) + }) export default function KeyValueFieldArrayDemo() { const form = useForm>({ diff --git a/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.test.ts b/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.test.ts index 3a5508f00a0..2bdb310718d 100644 --- a/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.test.ts +++ b/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.test.ts @@ -46,4 +46,46 @@ describe('EditHookPanel FormSchema', () => { ).toBe(true) } }) + + it('rejects key-only webhook headers', () => { + const result = FormSchema.safeParse({ + name: 'Test hook', + table_id: 'public.messages', + http_method: 'POST' as const, + timeout_ms: 1000, + events: ['INSERT'], + httpHeaders: [{ id: 'header-1', name: 'X-Test', value: '' }], + httpParameters: [], + function_type: 'http_request' as const, + http_url: 'https://hooks.example.com/webhook', + }) + + expect(result.success).toBe(false) + if (!result.success) { + expect( + result.error.issues.some((issue) => issue.message === 'Header value is required') + ).toBe(true) + } + }) + + it('rejects value-only webhook parameters', () => { + const result = FormSchema.safeParse({ + name: 'Test hook', + table_id: 'public.messages', + http_method: 'POST' as const, + timeout_ms: 1000, + events: ['INSERT'], + httpHeaders: [], + httpParameters: [{ id: 'param-1', name: '', value: 'tenant' }], + function_type: 'http_request' as const, + http_url: 'https://hooks.example.com/webhook', + }) + + expect(result.success).toBe(false) + if (!result.success) { + expect( + result.error.issues.some((issue) => issue.message === 'Parameter name is required') + ).toBe(true) + } + }) }) diff --git a/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.ts b/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.ts index 87ece7fc9d0..8bd6940af1b 100644 --- a/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.ts +++ b/apps/studio/components/interfaces/Database/Hooks/EditHookPanel.constants.ts @@ -1,3 +1,4 @@ +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import { z } from 'zod' import { httpEndpointUrlSchema } from '@/lib/validation/http-url' @@ -19,6 +20,38 @@ const supabaseFunctionSchema = z.object({ .refine((val) => !val.includes('undefined'), 'No edge functions available for selection'), }) +const httpHeadersSchema = z.array( + z.object({ id: z.string(), name: z.string().trim(), value: z.string().trim() }) +) + +const httpParametersSchema = z.array( + z.object({ id: z.string(), name: z.string().trim(), value: z.string().trim() }) +) + +const addKeyValueIssues = ( + rows: z.infer | z.infer, + ctx: z.RefinementCtx, + pathPrefix: 'httpHeaders' | 'httpParameters' +) => { + const isHeaderField = pathPrefix === 'httpHeaders' + + getKeyValueFieldArrayValidationIssues({ + rows, + keyFieldName: 'name', + valueFieldName: 'value', + keyRequiredMessage: isHeaderField ? 'Header name is required' : 'Parameter name is required', + valueRequiredMessage: isHeaderField + ? 'Header value is required' + : 'Parameter value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: [pathPrefix, ...issue.path], + }) + }) +} + export const FormSchema = z .object({ name: z.string().min(1, 'Please provide a name for your webhook'), @@ -30,9 +63,13 @@ export const FormSchema = z .gte(1000, 'Timeout should be at least 1000ms') .lte(10000, 'Timeout should not exceed 10,000ms'), events: z.array(z.string()).min(1, 'Please select at least one event'), - httpHeaders: z.array(z.object({ id: z.string(), name: z.string(), value: z.string() })), - httpParameters: z.array(z.object({ id: z.string(), name: z.string(), value: z.string() })), + httpHeaders: httpHeadersSchema, + httpParameters: httpParametersSchema, }) .and(z.discriminatedUnion('function_type', [httpRequestSchema, supabaseFunctionSchema])) + .superRefine((data, ctx) => { + addKeyValueIssues(data.httpHeaders, ctx, 'httpHeaders') + addKeyValueIssues(data.httpParameters, ctx, 'httpParameters') + }) export type WebhookFormValues = z.infer diff --git a/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.test.ts b/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.test.ts index f048b52e4ab..10d6b4005d5 100644 --- a/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.test.ts +++ b/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.test.ts @@ -50,4 +50,50 @@ describe('CreateCronJobSheet FormSchema', () => { ).toBe(true) } }) + + it('rejects key-only http_request headers', () => { + const result = FormSchema.safeParse({ + name: 'Send webhook', + supportsSeconds: false, + schedule: '* * * * *', + values: { + type: 'http_request' as const, + method: 'POST' as const, + endpoint: 'https://hooks.example.com/webhook', + timeoutMs: 1000, + httpHeaders: [{ name: 'X-Test', value: '' }], + snippet: '', + }, + }) + + expect(result.success).toBe(false) + if (!result.success) { + expect( + result.error.issues.some((issue) => issue.message === 'Header value is required') + ).toBe(true) + } + }) + + it('rejects value-only edge function headers', () => { + const result = FormSchema.safeParse({ + name: 'Invoke edge function', + supportsSeconds: false, + schedule: '* * * * *', + values: { + type: 'edge_function' as const, + method: 'POST' as const, + edgeFunctionName: 'my-function', + timeoutMs: 1000, + httpHeaders: [{ name: '', value: 'test-value' }], + snippet: '', + }, + }) + + expect(result.success).toBe(false) + if (!result.success) { + expect(result.error.issues.some((issue) => issue.message === 'Header name is required')).toBe( + true + ) + } + }) }) diff --git a/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.ts b/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.ts index c187b76162e..29fce353d44 100644 --- a/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.ts +++ b/apps/studio/components/interfaces/Integrations/CronJobs/CreateCronJobSheet/CreateCronJobSheet.constants.ts @@ -1,4 +1,5 @@ import { toString as CronToString } from 'cronstrue' +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import z from 'zod' import { cronPattern, secondsPattern } from '../CronJobs.constants' @@ -15,12 +16,34 @@ const convertCronToString = (schedule: string) => { } } +const httpHeadersSchema = z.array(z.object({ name: z.string().trim(), value: z.string().trim() })) + +const addHttpHeaderIssues = ( + rows: z.infer, + ctx: z.RefinementCtx, + pathPrefix: string[] +) => { + getKeyValueFieldArrayValidationIssues({ + rows, + keyFieldName: 'name', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: [...pathPrefix, ...issue.path], + }) + }) +} + const edgeFunctionSchema = z.object({ type: z.literal('edge_function'), method: z.enum(['GET', 'POST']), edgeFunctionName: z.string().trim().min(1, 'Please select one of the listed Edge Functions'), timeoutMs: z.coerce.number().int().gte(1000).lte(5000).default(1000), - httpHeaders: z.array(z.object({ name: z.string(), value: z.string() })), + httpHeaders: httpHeadersSchema, httpBody: z .string() .trim() @@ -47,7 +70,7 @@ const httpRequestSchema = z.object({ prefixMessage: 'Please prefix your URL with http:// or https://', }), timeoutMs: z.coerce.number().int().gte(1000).lte(5000).default(1000), - httpHeaders: z.array(z.object({ name: z.string(), value: z.string() })), + httpHeaders: httpHeadersSchema, httpBody: z .string() .trim() @@ -116,6 +139,10 @@ export const FormSchema = z }) } } + + if (data.values.type === 'edge_function' || data.values.type === 'http_request') { + addHttpHeaderIssues(data.values.httpHeaders, ctx, ['values', 'httpHeaders']) + } }) export type CreateCronJobForm = z.infer diff --git a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts index c0f3fcd2fee..c085f29d344 100644 --- a/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts +++ b/apps/studio/components/interfaces/LogDrains/LogDrains.utils.ts @@ -3,6 +3,7 @@ * Extracted for testability */ +import { getKeyValueFieldArrayValidationIssues } from 'ui-patterns/form/KeyValueFieldArray/validation' import { z } from 'zod' import { LogDrainType } from './LogDrains.constants' @@ -74,46 +75,18 @@ export const logDrainHeaderEntriesSchema = z ) .max(20, HEADER_VALIDATION_ERRORS.MAX_LIMIT) .superRefine((rows, ctx) => { - const rowIndexesByKey = new Map() - - rows.forEach((row, index) => { - const key = row.key.trim() - const value = row.value.trim() - - if (!key && !value) return - - if (key && !value) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: HEADER_VALIDATION_ERRORS.VALUE_REQUIRED, - path: [index, 'value'], - }) - return - } - - if (!key && value) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: HEADER_VALIDATION_ERRORS.KEY_REQUIRED, - path: [index, 'key'], - }) - return - } - - const existingIndexes = rowIndexesByKey.get(key) ?? [] - existingIndexes.push(index) - rowIndexesByKey.set(key, existingIndexes) - }) - - rowIndexesByKey.forEach((indexes) => { - if (indexes.length < 2) return - - indexes.forEach((index) => { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: HEADER_VALIDATION_ERRORS.DUPLICATE, - path: [index, 'key'], - }) + getKeyValueFieldArrayValidationIssues({ + rows, + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: HEADER_VALIDATION_ERRORS.KEY_REQUIRED, + valueRequiredMessage: HEADER_VALIDATION_ERRORS.VALUE_REQUIRED, + duplicateKeyMessage: HEADER_VALIDATION_ERRORS.DUPLICATE, + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: issue.path, }) }) }) diff --git a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.test.tsx b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.test.tsx index 8feffdcfbfc..82dee7c35d3 100644 --- a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.test.tsx +++ b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.test.tsx @@ -4,7 +4,7 @@ import type { ComponentProps } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import type { WebhookEndpoint } from './PlatformWebhooks.types' -import { PlatformWebhooksEndpointSheet } from './PlatformWebhooksEndpointSheet' +import { PlatformWebhooksEndpointSheet, toEndpointPayload } from './PlatformWebhooksEndpointSheet' import { customRender } from '@/tests/lib/custom-render' const { generateWebhookEndpointNameMock } = vi.hoisted(() => ({ @@ -233,4 +233,40 @@ describe('PlatformWebhooksEndpointSheet', () => { expect.anything() ) }) + + it('blocks submit when a custom header is missing its value', async () => { + const user = userEvent.setup() + const { onSubmit } = renderEndpointSheet({ + mode: 'edit', + endpoint: createEndpoint(), + }) + + await user.click(screen.getByRole('button', { name: 'Add header' })) + await user.type(screen.getByPlaceholderText('Header name'), 'X-Webhook-Secret') + submitForm() + + expect(await screen.findByText('Header value is required')).toBeInTheDocument() + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('strips fully empty custom header rows from the payload', () => { + expect( + toEndpointPayload({ + name: 'Billing events', + url: 'https://hooks.example.com/billing', + description: '', + enabled: true, + subscribeAll: false, + eventTypes: ['project.updated'], + customHeaders: [ + { key: 'X-Webhook-Secret', value: 'super-secret' }, + { key: '', value: '' }, + ], + }) + ).toEqual( + expect.objectContaining({ + customHeaders: [{ key: 'X-Webhook-Secret', value: 'super-secret' }], + }) + ) + }) }) diff --git a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.tsx b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.tsx index 7f2a579e7f2..96ceeb6e245 100644 --- a/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.tsx +++ b/apps/studio/components/interfaces/Platform/Webhooks/PlatformWebhooksEndpointSheet.tsx @@ -28,6 +28,10 @@ import { } from 'ui' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' import { KeyValueFieldArray } from 'ui-patterns/form/KeyValueFieldArray/KeyValueFieldArray' +import { + getKeyValueFieldArrayValidationIssues, + stripEmptyKeyValueFieldArrayRows, +} from 'ui-patterns/form/KeyValueFieldArray/validation' import * as z from 'zod' import type { @@ -70,6 +74,20 @@ const endpointFormSchema = z path: ['eventTypes'], }) } + + getKeyValueFieldArrayValidationIssues({ + rows: data.customHeaders, + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }).forEach((issue) => { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: issue.message, + path: ['customHeaders', ...issue.path], + }) + }) }) export type EndpointFormValues = z.infer @@ -124,7 +142,11 @@ export const toEndpointPayload = (values: EndpointFormValues): UpsertWebhookEndp description: values.description, enabled: values.enabled, eventTypes: toEventTypes(values), - customHeaders: values.customHeaders, + customHeaders: stripEmptyKeyValueFieldArrayRows({ + rows: values.customHeaders, + keyFieldName: 'key', + valueFieldName: 'value', + }), }) interface EndpointSheetProps { diff --git a/packages/ui-patterns/package.json b/packages/ui-patterns/package.json index 3d0fc398dd0..45251e9adc5 100644 --- a/packages/ui-patterns/package.json +++ b/packages/ui-patterns/package.json @@ -730,6 +730,10 @@ "import": "./src/form/KeyValueFieldArray/KeyValueFieldArray.tsx", "types": "./src/form/KeyValueFieldArray/KeyValueFieldArray.tsx" }, + "./form/KeyValueFieldArray/validation": { + "import": "./src/form/KeyValueFieldArray/validation.ts", + "types": "./src/form/KeyValueFieldArray/validation.ts" + }, "./form/SingleValueFieldArray/SingleValueFieldArray.test": { "import": "./src/form/SingleValueFieldArray/SingleValueFieldArray.test.tsx", "types": "./src/form/SingleValueFieldArray/SingleValueFieldArray.test.tsx" diff --git a/packages/ui-patterns/src/form/KeyValueFieldArray/KeyValueFieldArray.tsx b/packages/ui-patterns/src/form/KeyValueFieldArray/KeyValueFieldArray.tsx index 194d83488ca..ae94f6ab66c 100644 --- a/packages/ui-patterns/src/form/KeyValueFieldArray/KeyValueFieldArray.tsx +++ b/packages/ui-patterns/src/form/KeyValueFieldArray/KeyValueFieldArray.tsx @@ -77,6 +77,12 @@ const appendRows = < append(Array.isArray(rows) && rows.length === 1 ? rows[0] : rows) } +/** + * Rendering-only field array for text/text pairs. + * + * Consumers own validation in their resolver schema and can rely on the nested + * `FormMessage_Shadcn_` instances here to display per-cell errors. + */ export const KeyValueFieldArray = < TFieldValues extends FieldValues, TFieldArrayName extends FieldArrayPath, diff --git a/packages/ui-patterns/src/form/KeyValueFieldArray/validation.test.ts b/packages/ui-patterns/src/form/KeyValueFieldArray/validation.test.ts new file mode 100644 index 00000000000..a82304e917b --- /dev/null +++ b/packages/ui-patterns/src/form/KeyValueFieldArray/validation.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' + +import { + getKeyValueFieldArrayValidationIssues, + stripEmptyKeyValueFieldArrayRows, +} from './validation' + +describe('getKeyValueFieldArrayValidationIssues', () => { + it('allows fully empty draft rows by default', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [{ key: '', value: '' }], + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }) + ).toEqual([]) + }) + + it('adds an issue on value for key-only rows', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [{ key: 'Authorization', value: '' }], + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }) + ).toEqual([{ path: [0, 'value'], message: 'Header value is required' }]) + }) + + it('adds an issue on key for value-only rows', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [{ key: '', value: 'Bearer token' }], + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }) + ).toEqual([{ path: [0, 'key'], message: 'Header name is required' }]) + }) + + it('adds duplicate key issues when duplicate validation is enabled', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [ + { key: 'Authorization', value: 'Bearer 1' }, + { key: 'Authorization', value: 'Bearer 2' }, + ], + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + duplicateKeyMessage: 'Header name already exists', + }) + ).toEqual([ + { path: [0, 'key'], message: 'Header name already exists' }, + { path: [1, 'key'], message: 'Header name already exists' }, + ]) + }) + + it('supports custom key/value field names', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [{ name: 'tenant', value: '' }], + keyFieldName: 'name', + valueFieldName: 'value', + keyRequiredMessage: 'Parameter name is required', + valueRequiredMessage: 'Parameter value is required', + }) + ).toEqual([{ path: [0, 'value'], message: 'Parameter value is required' }]) + }) + + it('skips duplicate checks when no duplicate message is provided', () => { + expect( + getKeyValueFieldArrayValidationIssues({ + rows: [ + { key: 'Authorization', value: 'Bearer 1' }, + { key: 'Authorization', value: 'Bearer 2' }, + ], + keyFieldName: 'key', + valueFieldName: 'value', + keyRequiredMessage: 'Header name is required', + valueRequiredMessage: 'Header value is required', + }) + ).toEqual([]) + }) +}) + +describe('stripEmptyKeyValueFieldArrayRows', () => { + it('removes fully empty draft rows', () => { + expect( + stripEmptyKeyValueFieldArrayRows({ + rows: [ + { key: 'Authorization', value: 'Bearer token' }, + { key: '', value: '' }, + ], + keyFieldName: 'key', + valueFieldName: 'value', + }) + ).toEqual([{ key: 'Authorization', value: 'Bearer token' }]) + }) + + it('keeps partially filled rows so schema validation can handle them', () => { + expect( + stripEmptyKeyValueFieldArrayRows({ + rows: [ + { name: 'Authorization', value: '' }, + { name: '', value: 'Bearer token' }, + ], + keyFieldName: 'name', + valueFieldName: 'value', + }) + ).toEqual([ + { name: 'Authorization', value: '' }, + { name: '', value: 'Bearer token' }, + ]) + }) +}) diff --git a/packages/ui-patterns/src/form/KeyValueFieldArray/validation.ts b/packages/ui-patterns/src/form/KeyValueFieldArray/validation.ts new file mode 100644 index 00000000000..3a65e3efdbd --- /dev/null +++ b/packages/ui-patterns/src/form/KeyValueFieldArray/validation.ts @@ -0,0 +1,121 @@ +type KeyValueFieldName = string + +export type KeyValueFieldArrayValidationIssue = { + path: [number, TFieldName] + message: string +} + +type GetKeyValueFieldArrayValidationIssuesParams< + TRow extends Record, + TKeyFieldName extends Extract, + TValueFieldName extends Extract, +> = { + rows: TRow[] + keyFieldName: TKeyFieldName + valueFieldName: TValueFieldName + keyRequiredMessage: string + valueRequiredMessage: string + duplicateKeyMessage?: string + allowEmptyRows?: boolean + normaliseKey?: (key: string) => string +} + +const getTrimmedString = (value: unknown) => (typeof value === 'string' ? value.trim() : '') + +export type StripEmptyKeyValueFieldArrayRowsParams< + TRow extends Record, + TKeyFieldName extends Extract, + TValueFieldName extends Extract, +> = { + rows: TRow[] + keyFieldName: TKeyFieldName + valueFieldName: TValueFieldName +} + +/** + * Removes fully empty draft rows before persisting field-array values. + */ +export const stripEmptyKeyValueFieldArrayRows = < + TRow extends Record, + TKeyFieldName extends Extract, + TValueFieldName extends Extract, +>({ + rows, + keyFieldName, + valueFieldName, +}: StripEmptyKeyValueFieldArrayRowsParams) => + rows.filter((row) => { + const key = getTrimmedString(row[keyFieldName]) + const value = getTrimmedString(row[valueFieldName]) + + return key.length > 0 || value.length > 0 + }) + +/** + * Returns per-cell validation issues for draft-friendly key/value rows. + * + * Consumers should feed these issues into their resolver schema, typically via + * `zod.superRefine(...)`, so validation stays declarative and local to the form. + */ +export const getKeyValueFieldArrayValidationIssues = < + TRow extends Record, + TKeyFieldName extends Extract, + TValueFieldName extends Extract, +>({ + rows, + keyFieldName, + valueFieldName, + keyRequiredMessage, + valueRequiredMessage, + duplicateKeyMessage, + allowEmptyRows = true, + normaliseKey = (key) => key, +}: GetKeyValueFieldArrayValidationIssuesParams) => { + const issues: KeyValueFieldArrayValidationIssue[] = [] + const rowIndexesByKey = duplicateKeyMessage ? new Map() : null + + rows.forEach((row, index) => { + const key = getTrimmedString(row[keyFieldName]) + const value = getTrimmedString(row[valueFieldName]) + + if (!key && !value) { + if (!allowEmptyRows) { + issues.push({ path: [index, keyFieldName], message: keyRequiredMessage }) + issues.push({ path: [index, valueFieldName], message: valueRequiredMessage }) + } + return + } + + if (!key) { + issues.push({ path: [index, keyFieldName], message: keyRequiredMessage }) + return + } + + if (!value) { + issues.push({ path: [index, valueFieldName], message: valueRequiredMessage }) + return + } + + if (!rowIndexesByKey) return + + const normalisedKey = normaliseKey(key) + if (!normalisedKey) return + + rowIndexesByKey.set(normalisedKey, [...(rowIndexesByKey.get(normalisedKey) ?? []), index]) + }) + + if (!rowIndexesByKey || !duplicateKeyMessage) return issues + + rowIndexesByKey.forEach((indexes) => { + if (indexes.length < 2) return + + indexes.forEach((index) => { + issues.push({ + path: [index, keyFieldName], + message: duplicateKeyMessage, + }) + }) + }) + + return issues +}