From 1fb7953a9536eda97a658f4097340352539e8c9b Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Thu, 16 Oct 2025 10:05:10 -0400 Subject: [PATCH] ConfigForm: Add field-specific error handling (#112397) * ConfigForm: Show api error to field * i18n * alert message only if error is not fetch error, consolidate getFormErros helpers --- .../provisioning/Config/ConfigForm.tsx | 30 ++++- .../provisioning/utils/getFormErrors.ts | 118 +++++++++++------- public/locales/en-US/grafana.json | 3 +- 3 files changed, 103 insertions(+), 48 deletions(-) diff --git a/public/app/features/provisioning/Config/ConfigForm.tsx b/public/app/features/provisioning/Config/ConfigForm.tsx index 0ba33a4e5db..e44a103c408 100644 --- a/public/app/features/provisioning/Config/ConfigForm.tsx +++ b/public/app/features/provisioning/Config/ConfigForm.tsx @@ -3,8 +3,9 @@ import { useEffect, useMemo, useState } from 'react'; import { Controller, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; +import { AppEvents } from '@grafana/data'; import { t } from '@grafana/i18n'; -import { reportInteraction } from '@grafana/runtime'; +import { getAppEvents, isFetchError, reportInteraction } from '@grafana/runtime'; import { Button, Checkbox, @@ -32,6 +33,7 @@ import { PROVISIONING_URL } from '../constants'; import { useCreateOrUpdateRepository } from '../hooks/useCreateOrUpdateRepository'; import { RepositoryFormData } from '../types'; import { dataToSpec } from '../utils/data'; +import { getConfigFormErrors } from '../utils/getFormErrors'; import { getHasTokenInstructions } from '../utils/git'; import { getRepositoryTypeConfig, isGitProvider } from '../utils/repositoryTypes'; @@ -138,6 +140,18 @@ export function ConfigForm({ data }: ConfigFormProps) { try { const spec = dataToSpec(form); await submitData(spec, form.token); + } catch (err) { + if (isFetchError(err)) { + const [field, errorMessage] = getConfigFormErrors(err.data?.errors); + + if (field && errorMessage) { + setError(field, errorMessage); + return; + } + } + + // fallback for non-fetch errors or unmapped fields + defaultAlert(); } finally { setIsLoading(false); } @@ -351,7 +365,12 @@ export function ConfigForm({ data }: ConfigFormProps) { }} /> - + ); } + +const defaultAlert = () => { + getAppEvents().publish({ + type: AppEvents.alertError.name, + payload: [t('provisioning.wizard-content.error-save-repository-setting', 'Failed to save repository setting')], + }); +}; diff --git a/public/app/features/provisioning/utils/getFormErrors.ts b/public/app/features/provisioning/utils/getFormErrors.ts index 3d94975a602..4b187c131d1 100644 --- a/public/app/features/provisioning/utils/getFormErrors.ts +++ b/public/app/features/provisioning/utils/getFormErrors.ts @@ -1,63 +1,91 @@ +import { Path } from 'react-hook-form'; + import { ErrorDetails } from 'app/api/clients/provisioning/v0alpha1'; import { WizardFormData } from '../Wizard/types'; +import { RepositoryFormData } from '../types'; export type RepositoryField = keyof WizardFormData['repository']; -export type RepositoryFormPath = `repository.${RepositoryField}` | `repository.sync.intervalSeconds`; -export type FormErrorTuple = [RepositoryFormPath | null, { message: string } | null]; +export type RepositoryFormPath = `repository.${RepositoryField}` | 'repository.sync.intervalSeconds'; + +type GenericFormPath = string; +type GenericFormErrorTuple = [T | null, { message: string } | null]; /** - * Maps API error details to form error fields for React Hook Form - * - * @param errors Array of error details from the API response - * @returns Tuple with form field path and error message + * Normalize API field name by removing "spec." prefix. */ -export const getFormErrors = (errors: ErrorDetails[]): FormErrorTuple => { - const fieldsToValidate = [ - 'local.path', - 'github.branch', - 'github.url', - 'github.path', - 'secure.token', - 'gitlab.branch', - 'gitlab.url', - 'bitbucket.branch', - 'bitbucket.url', - 'git.branch', - 'git.url', - 'sync.intervalSeconds', - ]; +const normalizeField = (field: string): string => field.replace(/^spec\./, ''); - const nestedFieldMap: Record = { - 'sync.intervalSeconds': 'repository.sync.intervalSeconds', - }; - - const fieldMap: Record = { - path: 'repository.path', - branch: 'repository.branch', - url: 'repository.url', - token: 'repository.token', - }; +/** + * Given a list of error details and a field mapping, + * returns the first matched form error tuple. + */ +function mapErrorsToField( + errors: ErrorDetails[] | undefined, + fieldMap: Record, + opts?: { allowPartial?: boolean } +): GenericFormErrorTuple { + if (!errors || errors.length === 0) { + return [null, null]; + } for (const error of errors) { - if (error.field) { - const cleanField = error.field.replace('spec.', ''); - if (fieldsToValidate.includes(cleanField)) { - // Check for direct nested field mapping first - if (cleanField in nestedFieldMap) { - return [nestedFieldMap[cleanField], { message: error.detail || `Invalid ${cleanField}` }]; - } + if (!error.field) { + continue; + } - // Fall back to simple field mapping for non-nested fields - const fieldParts = cleanField.split('.'); - const lastPart = fieldParts[fieldParts.length - 1]; + const normalized = normalizeField(error.field); + const segments = normalized.split('.'); + const lastPart = segments[segments.length - 1]; - if (lastPart in fieldMap) { - return [fieldMap[lastPart], { message: error.detail || `Invalid ${lastPart}` }]; - } - } + // Direct full match (e.g. "sync.intervalSeconds") + if (normalized in fieldMap) { + return [fieldMap[normalized], { message: error.detail || `Invalid ${normalized}` }]; + } + + // Partial match by last key (e.g. "url" -> "repository.url") + if (opts?.allowPartial && lastPart in fieldMap) { + return [fieldMap[lastPart], { message: error.detail || `Invalid ${lastPart}` }]; } } return [null, null]; +} + +// Wizard form errors +export type FormErrorTuple = GenericFormErrorTuple; +export const getFormErrors = (errors: ErrorDetails[]): FormErrorTuple => { + const fieldMap: Record = { + 'local.path': 'repository.path', + 'github.branch': 'repository.branch', + 'github.url': 'repository.url', + 'github.path': 'repository.path', + 'secure.token': 'repository.token', + 'gitlab.branch': 'repository.branch', + 'gitlab.url': 'repository.url', + 'bitbucket.branch': 'repository.branch', + 'bitbucket.url': 'repository.url', + 'git.branch': 'repository.branch', + 'git.url': 'repository.url', + 'sync.intervalSeconds': 'repository.sync.intervalSeconds', + }; + + return mapErrorsToField(errors, fieldMap, { allowPartial: true }); +}; + +// Config form errors +export type ConfigFormPath = Path; +export type ConfigFormErrorTuple = GenericFormErrorTuple; + +export const getConfigFormErrors = (errors?: ErrorDetails[]): ConfigFormErrorTuple => { + const fieldMap: Record = { + path: 'path', + branch: 'branch', + url: 'url', + token: 'token', + tokenUser: 'tokenUser', + 'sync.intervalSeconds': 'sync.intervalSeconds', + }; + + return mapErrorsToField(errors, fieldMap, { allowPartial: true }); }; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index c3eb0a2d8af..05f1d942d9a 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11828,7 +11828,8 @@ "button-cancelling": "Cancelling...", "button-previous": "Previous", "button-submitting": "Submitting...", - "error-instance-repository-exists": "Instance repository already exists" + "error-instance-repository-exists": "Instance repository already exists", + "error-save-repository-setting": "Failed to save repository setting" }, "workflow-options-label": { "push-to-a-new-branch": "Push to a new branch",