From 9c5803f6b2ba8319ea5d751e1d26c304b05b6be4 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Sun, 1 Nov 2026 10:09:44 -0500 Subject: [PATCH] FIX: show per-field API validation errors in the finding dialog --- .../Operations/FindingDialog.test.tsx | 29 ++++++++++++++ .../components/Operations/FindingDialog.tsx | 17 ++++++-- frontend/src/services/errors.test.ts | 23 +++++++++++ frontend/src/services/errors.ts | 39 ++++++++++++++++--- 4 files changed, 100 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/Operations/FindingDialog.test.tsx b/frontend/src/components/Operations/FindingDialog.test.tsx index 0915b4eaad..eba0af80fa 100644 --- a/frontend/src/components/Operations/FindingDialog.test.tsx +++ b/frontend/src/components/Operations/FindingDialog.test.tsx @@ -194,4 +194,33 @@ describe('FindingDialog', () => { id: expect.any(String), attackResultId: 'attack-1', }) }) + + it('shows the API field message on the Evidence attack ID and keeps it invalid until edited', async () => { + const user = userEvent.setup() + const rejection = Object.assign(new Error('Request failed'), { + isAxiosError: true, + response: { + status: 422, + data: { + detail: 'Request validation failed', + errors: [{ field: 'body.attack_result_id', message: 'Input should be a valid UUID', code: 'uuid_parsing' }], + }, + }, + }) + const onSave = jest.fn().mockRejectedValueOnce(rejection).mockResolvedValue(undefined) + render( + ) + await user.type(screen.getByRole('textbox', { name: 'Title' }), 'Bad evidence') + await user.type(screen.getByRole('textbox', { name: 'Evidence attack ID' }), 'not-a-uuid') + await waitFor(() => expect(screen.getByRole('button', { name: 'Save finding' })).toBeEnabled()) + await user.click(screen.getByRole('button', { name: 'Save finding' })) + + expect(await screen.findByText(/Input should be a valid UUID/)).toBeInTheDocument() + expect(screen.getByRole('textbox', { name: 'Evidence attack ID' })).toHaveAttribute('aria-invalid', 'true') + expect(screen.getByRole('button', { name: 'Save finding' })).toBeDisabled() + + await user.type(screen.getByRole('textbox', { name: 'Evidence attack ID' }), 'x') + expect(screen.queryByText(/Input should be a valid UUID/)).not.toBeInTheDocument() + await waitFor(() => expect(screen.getByRole('button', { name: 'Save finding' })).toBeEnabled()) + }) }) diff --git a/frontend/src/components/Operations/FindingDialog.tsx b/frontend/src/components/Operations/FindingDialog.tsx index 04e7e47bcb..4d19ff4612 100644 --- a/frontend/src/components/Operations/FindingDialog.tsx +++ b/frontend/src/components/Operations/FindingDialog.tsx @@ -41,6 +41,7 @@ export default function FindingDialog({ const [draft, setDraft] = useState({ ...EMPTY_DRAFT, ...initialValues }) const [attackResultId, setAttackResultId] = useState('') const [error, setError] = useState('') + const [attackResultIdError, setAttackResultIdError] = useState('') const [saving, setSaving] = useState(false) const submitting = useRef(false) const attempt = useRef<{ signature: string; id: string } | null>(null) @@ -51,7 +52,7 @@ export default function FindingDialog({ const [harmSearch, setHarmSearch] = useState('') const classificationsValid = (draft.severity !== 'other' || Boolean(draft.severity_other?.trim())) && (draft.harm_type !== 'Other' || Boolean(draft.harm_type_other?.trim())) - const canSave = !saveDisabled && Boolean(draft.title.trim()) && classificationsValid && harmTypes !== null + const canSave = !saveDisabled && !attackResultIdError && Boolean(draft.title.trim()) && classificationsValid && harmTypes !== null && !optionsError useEffect(() => { @@ -75,13 +76,22 @@ export default function FindingDialog({ submitting.current = true setSaving(true) setError('') + setAttackResultIdError('') const trimmedAttackResultId = attackResultId.trim() const signature = JSON.stringify([draft, trimmedAttackResultId]) if (attempt.current?.signature !== signature) attempt.current = { signature, id: generateClientId() } try { await onSave(draft, { id: attempt.current.id, attackResultId: trimmedAttackResultId }) } catch (cause: unknown) { - setError(toApiError(cause).detail) + const apiError = toApiError(cause) + const evidenceMessages = apiError.fieldErrors.filter(item => item.field.split('.').pop() === 'attack_result_id') + .map(item => item.message) + const otherMessages = apiError.fieldErrors.filter(item => item.field.split('.').pop() !== 'attack_result_id') + .map(item => `${item.field.replace(/^body\./, '')}: ${item.message}`) + setAttackResultIdError(evidenceMessages.join(' ')) + if (evidenceMessages.length === 0 || otherMessages.length > 0) { + setError([otherMessages.length > 0 ? '' : apiError.detail, ...otherMessages].filter(Boolean).join(' ')) + } } finally { submitting.current = false setSaving(false) @@ -158,9 +168,10 @@ export default function FindingDialog({ onChange={(_, data) => { setDraft(value => ({ ...value, description: data.value })) }} /> {evidenceField && { setAttackResultId(data.value) }} /> + onChange={(_, data) => { setAttackResultId(data.value); setAttackResultIdError('') }} /> }
diff --git a/frontend/src/services/errors.test.ts b/frontend/src/services/errors.test.ts index a926cf5451..c3e953e19f 100644 --- a/frontend/src/services/errors.test.ts +++ b/frontend/src/services/errors.test.ts @@ -207,6 +207,29 @@ describe('toApiError', () => { expect(result.detail).toBe('Field "name" is required.') expect(result.type).toBe('validation_error') }) + + it('preserves per-field validation errors from the RFC 7807 body', () => { + const err = makeAxiosError({ + status: 422, + data: { + detail: 'Request validation failed', + errors: [ + { field: 'body.attack_result_id', message: 'Input should be a valid UUID', code: 'uuid_parsing' }, + { field: 'body.title', message: 42 }, + 'not-an-object', + ], + }, + }) + + expect(toApiError(err).fieldErrors).toEqual([ + { field: 'body.attack_result_id', message: 'Input should be a valid UUID', code: 'uuid_parsing' }, + ]) + }) + + it('leaves fieldErrors empty when the body has no errors array', () => { + const err = makeAxiosError({ status: 500, data: { detail: 'boom' } }) + expect(toApiError(err).fieldErrors).toEqual([]) + }) }) describe('decodeBlobErrorAsync', () => { diff --git a/frontend/src/services/errors.ts b/frontend/src/services/errors.ts index 492d34a5dd..bd7355c96a 100644 --- a/frontend/src/services/errors.ts +++ b/frontend/src/services/errors.ts @@ -16,6 +16,13 @@ export function conflictingOperation(cause: unknown): Operation | null { return { id: operation.id, name: operation.name, created_at: operation.created_at } } +export interface FieldError { + /** Location of the invalid input, e.g. `body.attack_result_id`. */ + field: string + message: string + code?: string +} + /** * Normalized error from any API call. * @@ -30,6 +37,8 @@ export interface ApiError { detail: string /** RFC 7807 `type` URI, if the backend included one. */ type?: string + /** Per-field validation errors from the RFC 7807 `errors` array; empty when the body has none. */ + fieldErrors: FieldError[] /** True when no HTTP response was received (DNS failure, CORS block, etc.). */ isNetworkError: boolean /** True when the request exceeded the configured timeout. */ @@ -58,6 +67,7 @@ export function toApiError(err: unknown): ApiError { return { status: null, detail: 'Request timed out. The server may be busy — please try again.', + fieldErrors: [], isNetworkError: false, isTimeout: true, raw: err, @@ -69,6 +79,7 @@ export function toApiError(err: unknown): ApiError { return { status: null, detail: 'Network error — check that the backend is running and reachable.', + fieldErrors: [], isNetworkError: true, isTimeout: false, raw: err, @@ -77,12 +88,13 @@ export function toApiError(err: unknown): ApiError { // We have an HTTP response — try to extract RFC 7807 detail const { status, data } = err.response - const { detail, type } = extractDetail(data) + const { detail, type, fieldErrors } = extractDetail(data) return { status, detail: detail || `Server error (${status})`, type, + fieldErrors, isNetworkError: false, isTimeout: false, raw: err, @@ -94,6 +106,7 @@ export function toApiError(err: unknown): ApiError { return { status: null, detail: err.message || 'An unexpected error occurred.', + fieldErrors: [], isNetworkError: false, isTimeout: false, raw: err, @@ -105,6 +118,7 @@ export function toApiError(err: unknown): ApiError { return { status: null, detail: err, + fieldErrors: [], isNetworkError: false, isTimeout: false, raw: err, @@ -115,6 +129,7 @@ export function toApiError(err: unknown): ApiError { return { status: null, detail: 'An unexpected error occurred.', + fieldErrors: [], isNetworkError: false, isTimeout: false, raw: err, @@ -180,15 +195,29 @@ function readBlobText(blob: Blob): Promise { * - A plain string (e.g. nginx HTML error page) * - Something else entirely (null, number, etc.) */ -function extractDetail(data: unknown): { detail: string | undefined; type: string | undefined } { +function extractDetail(data: unknown): { + detail: string | undefined; type: string | undefined; fieldErrors: FieldError[] +} { if (typeof data === 'string') { - return { detail: data, type: undefined } + return { detail: data, type: undefined, fieldErrors: [] } } if (typeof data === 'object' && data !== null) { const obj = data as Record const detail = typeof obj.detail === 'string' ? obj.detail : undefined const type = typeof obj.type === 'string' ? obj.type : undefined - return { detail, type } + return { detail, type, fieldErrors: extractFieldErrors(obj.errors) } + } + return { detail: undefined, type: undefined, fieldErrors: [] } +} + +function extractFieldErrors(errors: unknown): FieldError[] { + if (!Array.isArray(errors)) return [] + const result: FieldError[] = [] + for (const item of errors) { + if (typeof item !== 'object' || item === null) continue + const { field, message, code } = item as Record + if (typeof field !== 'string' || typeof message !== 'string') continue + result.push(typeof code === 'string' ? { field, message, code } : { field, message }) } - return { detail: undefined, type: undefined } + return result }