From 3c6b14d2bfa407402122c7abbce4820da71215b6 Mon Sep 17 00:00:00 2001 From: Felipe Cardoso Date: Tue, 6 Jan 2026 13:50:36 +0100 Subject: [PATCH] refactor(forms): extract reusable form utilities and components MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add getFirstValidationError utility for nested FieldErrors extraction - Add mergeWithDefaults utilities (deepMergeWithDefaults, type guards) - Add useValidationErrorHandler hook for toast + tab navigation - Add FormSelect component with Controller integration - Add FormTextarea component with register integration - Refactor AgentTypeForm to use new utilities - Remove verbose debug logging (now handled by hook) - Add comprehensive tests (53 new tests, 100 total) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../src/components/agents/AgentTypeForm.tsx | 189 ++++++-------- frontend/src/components/forms/FormSelect.tsx | 133 ++++++++++ .../src/components/forms/FormTextarea.tsx | 101 +++++++ frontend/src/components/forms/index.ts | 4 + .../forms/hooks/useValidationErrorHandler.ts | 118 +++++++++ frontend/src/lib/forms/index.ts | 30 +++ .../forms/utils/getFirstValidationError.ts | 84 ++++++ .../src/lib/forms/utils/mergeWithDefaults.ts | 165 ++++++++++++ .../hooks/useValidationErrorHandler.test.tsx | 212 +++++++++++++++ .../utils/getFirstValidationError.test.ts | 134 ++++++++++ .../lib/forms/utils/mergeWithDefaults.test.ts | 247 ++++++++++++++++++ 11 files changed, 1303 insertions(+), 114 deletions(-) create mode 100644 frontend/src/components/forms/FormSelect.tsx create mode 100644 frontend/src/components/forms/FormTextarea.tsx create mode 100644 frontend/src/lib/forms/hooks/useValidationErrorHandler.ts create mode 100644 frontend/src/lib/forms/index.ts create mode 100644 frontend/src/lib/forms/utils/getFirstValidationError.ts create mode 100644 frontend/src/lib/forms/utils/mergeWithDefaults.ts create mode 100644 frontend/tests/lib/forms/hooks/useValidationErrorHandler.test.tsx create mode 100644 frontend/tests/lib/forms/utils/getFirstValidationError.test.ts create mode 100644 frontend/tests/lib/forms/utils/mergeWithDefaults.test.ts diff --git a/frontend/src/components/agents/AgentTypeForm.tsx b/frontend/src/components/agents/AgentTypeForm.tsx index 962949e..cd4fd00 100644 --- a/frontend/src/components/agents/AgentTypeForm.tsx +++ b/frontend/src/components/agents/AgentTypeForm.tsx @@ -3,14 +3,17 @@ * * React Hook Form-based form for creating and editing agent types. * Features tabbed interface for organizing form sections. + * + * Uses reusable form utilities for: + * - Validation error handling with toast notifications + * - Safe API-to-form data transformation with defaults */ 'use client'; -import { useEffect, useState, useCallback } from 'react'; -import { useForm, Controller, type FieldErrors } from 'react-hook-form'; +import { useEffect, useState, useCallback, useMemo } from 'react'; +import { useForm, Controller } from 'react-hook-form'; import { zodResolver } from '@hookform/resolvers/zod'; -import { toast } from 'sonner'; import { Button } from '@/components/ui/button'; import { Input } from '@/components/ui/input'; import { Textarea } from '@/components/ui/textarea'; @@ -37,6 +40,7 @@ import { generateSlug, } from '@/lib/validations/agentType'; import type { AgentTypeResponse } from '@/lib/api/types/agentTypes'; +import { useValidationErrorHandler, deepMergeWithDefaults, isNumber } from '@/lib/forms'; interface AgentTypeFormProps { agentType?: AgentTypeResponse; @@ -46,6 +50,60 @@ interface AgentTypeFormProps { className?: string; } +// Tab navigation mapping for validation errors +const TAB_FIELD_MAPPING = { + name: 'basic', + slug: 'basic', + description: 'basic', + expertise: 'basic', + is_active: 'basic', + primary_model: 'model', + fallback_models: 'model', + model_params: 'model', + mcp_servers: 'permissions', + tool_permissions: 'permissions', + personality_prompt: 'personality', +} as const; + +/** + * Transform API response to form values with safe defaults + * + * Uses deepMergeWithDefaults for most fields, with special handling + * for model_params which needs numeric type validation. + */ +function transformAgentTypeToFormValues( + agentType: AgentTypeResponse | undefined +): AgentTypeCreateFormValues { + if (!agentType) return defaultAgentTypeValues; + + // model_params needs special handling for numeric validation + const modelParams = agentType.model_params ?? {}; + const safeModelParams = { + temperature: isNumber(modelParams.temperature) ? modelParams.temperature : 0.7, + max_tokens: isNumber(modelParams.max_tokens) ? modelParams.max_tokens : 8192, + top_p: isNumber(modelParams.top_p) ? modelParams.top_p : 0.95, + }; + + // Merge with defaults, then override model_params with safe version + const merged = deepMergeWithDefaults(defaultAgentTypeValues, { + name: agentType.name, + slug: agentType.slug, + description: agentType.description, + expertise: agentType.expertise, + personality_prompt: agentType.personality_prompt, + primary_model: agentType.primary_model, + fallback_models: agentType.fallback_models, + mcp_servers: agentType.mcp_servers, + tool_permissions: agentType.tool_permissions, + is_active: agentType.is_active, + }); + + return { + ...merged, + model_params: safeModelParams, + }; +} + export function AgentTypeForm({ agentType, onSubmit, @@ -57,37 +115,13 @@ export function AgentTypeForm({ const [activeTab, setActiveTab] = useState('basic'); const [expertiseInput, setExpertiseInput] = useState(''); - // Build initial values with proper defaults for missing fields - const getInitialValues = useCallback((): AgentTypeCreateFormValues => { - if (!agentType) return defaultAgentTypeValues; - - // Ensure model_params has all required fields with proper defaults - const modelParams = agentType.model_params ?? {}; - const safeModelParams = { - temperature: typeof modelParams.temperature === 'number' ? modelParams.temperature : 0.7, - max_tokens: typeof modelParams.max_tokens === 'number' ? modelParams.max_tokens : 8192, - top_p: typeof modelParams.top_p === 'number' ? modelParams.top_p : 0.95, - }; - - return { - name: agentType.name, - slug: agentType.slug, - description: agentType.description, - expertise: agentType.expertise ?? [], - personality_prompt: agentType.personality_prompt ?? '', - primary_model: agentType.primary_model ?? 'claude-opus-4-5-20251101', - fallback_models: agentType.fallback_models ?? [], - model_params: safeModelParams, - mcp_servers: agentType.mcp_servers ?? [], - tool_permissions: agentType.tool_permissions ?? {}, - is_active: agentType.is_active ?? false, - }; - }, [agentType]); + // Memoize initial values transformation + const initialValues = useMemo(() => transformAgentTypeToFormValues(agentType), [agentType]); // Always use create schema for validation - editing requires all fields too const form = useForm({ resolver: zodResolver(agentTypeCreateSchema), - defaultValues: getInitialValues(), + defaultValues: initialValues, }); const { @@ -99,58 +133,11 @@ export function AgentTypeForm({ formState: { errors }, } = form; - // Helper to extract first error message from nested errors - const getFirstErrorMessage = useCallback( - (formErrors: FieldErrors): { field: string; message: string } => { - for (const key of Object.keys(formErrors)) { - const error = formErrors[key as keyof typeof formErrors]; - if (!error) continue; - - // Handle nested errors (like model_params.temperature) - if (typeof error === 'object' && !('message' in error)) { - for (const nestedKey of Object.keys(error)) { - const nestedError = (error as Record)[nestedKey]; - if (nestedError?.message) { - return { field: `${key}.${nestedKey}`, message: nestedError.message }; - } - } - } - - // Handle direct error - if ('message' in error && error.message) { - return { field: key, message: error.message as string }; - } - } - return { field: 'unknown', message: 'Validation error' }; - }, - [] - ); - - // Handle form validation errors - show toast with first error - const handleFormError = useCallback( - (formErrors: FieldErrors) => { - // Log for debugging - console.error('[AgentTypeForm] Validation errors:', formErrors); - - const { field, message } = getFirstErrorMessage(formErrors); - toast.error('Please fix form errors', { - description: `${field}: ${message}`, - }); - - // Navigate to the tab containing the error - const topLevelField = field.split('.')[0]; - if (['name', 'slug', 'description', 'expertise', 'is_active'].includes(topLevelField)) { - setActiveTab('basic'); - } else if (['primary_model', 'fallback_models', 'model_params'].includes(topLevelField)) { - setActiveTab('model'); - } else if (['mcp_servers', 'tool_permissions'].includes(topLevelField)) { - setActiveTab('permissions'); - } else if (topLevelField === 'personality_prompt') { - setActiveTab('personality'); - } - }, - [getFirstErrorMessage] - ); + // Use the reusable validation error handler hook + const { onValidationError } = useValidationErrorHandler({ + tabMapping: TAB_FIELD_MAPPING, + setActiveTab, + }); const watchName = watch('name'); /* istanbul ignore next -- defensive fallback, expertise always has default */ @@ -161,10 +148,9 @@ export function AgentTypeForm({ // Reset form when agentType changes (e.g., switching to edit mode) useEffect(() => { if (agentType) { - const values = getInitialValues(); - form.reset(values); + form.reset(initialValues); } - }, [agentType?.id, form, getInitialValues]); + }, [agentType?.id, form, initialValues]); // Auto-generate slug from name for new agent types useEffect(() => { @@ -203,37 +189,12 @@ export function AgentTypeForm({ } }; - // Wrap the form submission with logging + // Handle form submission with validation const onFormSubmit = useCallback( - async (e: React.FormEvent) => { - console.log('[AgentTypeForm] Form submit triggered'); - console.log('[AgentTypeForm] Current form values:', form.getValues()); - console.log('[AgentTypeForm] Form state:', { - isDirty: form.formState.isDirty, - isValid: form.formState.isValid, - isSubmitting: form.formState.isSubmitting, - errors: form.formState.errors, - }); - - // Let react-hook-form handle the actual submission - return handleSubmit( - async (data) => { - console.log('[AgentTypeForm] Validation passed, calling onSubmit with:', data); - try { - await onSubmit(data); - console.log('[AgentTypeForm] onSubmit completed successfully'); - } catch (error) { - console.error('[AgentTypeForm] onSubmit threw error:', error); - throw error; - } - }, - (errors) => { - console.error('[AgentTypeForm] Validation failed:', errors); - handleFormError(errors); - } - )(e); + (e: React.FormEvent) => { + return handleSubmit(onSubmit, onValidationError)(e); }, - [form, handleSubmit, onSubmit, handleFormError] + [handleSubmit, onSubmit, onValidationError] ); return ( diff --git a/frontend/src/components/forms/FormSelect.tsx b/frontend/src/components/forms/FormSelect.tsx new file mode 100644 index 0000000..1c6363e --- /dev/null +++ b/frontend/src/components/forms/FormSelect.tsx @@ -0,0 +1,133 @@ +/** + * FormSelect Component + * + * Reusable Select field with Controller integration for react-hook-form. + * Handles label, error display, and description automatically. + * + * @module components/forms/FormSelect + */ + +'use client'; + +import { Controller, type Control, type FieldValues, type Path } from 'react-hook-form'; +import { Label } from '@/components/ui/label'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; + +export interface SelectOption { + value: string; + label: string; +} + +export interface FormSelectProps { + /** Field name (must be a valid path in the form) */ + name: Path; + /** Form control from useForm */ + control: Control; + /** Field label */ + label: string; + /** Available options */ + options: SelectOption[]; + /** Is field required? Shows asterisk if true */ + required?: boolean; + /** Placeholder text when no value selected */ + placeholder?: string; + /** Helper text below the field */ + description?: string; + /** Disable the select */ + disabled?: boolean; + /** Additional class name */ + className?: string; +} + +/** + * FormSelect - Controlled Select field for react-hook-form + * + * Automatically handles: + * - Controller wrapper for react-hook-form + * - Label with required indicator + * - Error message display + * - Description/helper text + * - Accessibility attributes + * + * @example + * ```tsx + * + * ``` + */ +export function FormSelect({ + name, + control, + label, + options, + required = false, + placeholder, + description, + disabled = false, + className, +}: FormSelectProps) { + const selectId = String(name); + const errorId = `${selectId}-error`; + const descriptionId = description ? `${selectId}-description` : undefined; + + return ( + ( +
+
+ + + {fieldState.error && ( + + )} + {description && ( +

+ {description} +

+ )} +
+
+ )} + /> + ); +} diff --git a/frontend/src/components/forms/FormTextarea.tsx b/frontend/src/components/forms/FormTextarea.tsx new file mode 100644 index 0000000..b4a3165 --- /dev/null +++ b/frontend/src/components/forms/FormTextarea.tsx @@ -0,0 +1,101 @@ +/** + * FormTextarea Component + * + * Reusable Textarea field for react-hook-form with register integration. + * Handles label, error display, and description automatically. + * + * @module components/forms/FormTextarea + */ + +'use client'; + +import { ComponentProps } from 'react'; +import type { FieldError, UseFormRegisterReturn } from 'react-hook-form'; +import { Label } from '@/components/ui/label'; +import { Textarea } from '@/components/ui/textarea'; + +export interface FormTextareaProps extends Omit, 'children'> { + /** Field label */ + label: string; + /** Field name (optional if provided via register) */ + name?: string; + /** Is field required? Shows asterisk if true */ + required?: boolean; + /** Form error from react-hook-form */ + error?: FieldError; + /** Helper text below the field */ + description?: string; + /** Register return object from useForm */ + registration?: UseFormRegisterReturn; +} + +/** + * FormTextarea - Textarea field for react-hook-form + * + * Automatically handles: + * - Label with required indicator + * - Error message display + * - Description/helper text + * - Accessibility attributes + * + * @example + * ```tsx + * + * ``` + */ +export function FormTextarea({ + label, + name: explicitName, + required = false, + error, + description, + registration, + ...textareaProps +}: FormTextareaProps) { + // Extract name from props or registration + const registerName = + 'name' in textareaProps ? (textareaProps as { name: string }).name : undefined; + const name = explicitName || registerName || registration?.name; + + if (!name) { + throw new Error('FormTextarea: name must be provided either explicitly or via register()'); + } + + const errorId = error ? `${name}-error` : undefined; + const descriptionId = description ? `${name}-description` : undefined; + const ariaDescribedBy = [errorId, descriptionId].filter(Boolean).join(' ') || undefined; + + // Merge registration props with other props + const mergedProps = registration ? { ...registration, ...textareaProps } : textareaProps; + + return ( +
+ + {description && ( +

+ {description} +

+ )} +