forked from cardosofelipe/pragma-stack
refactor(forms): extract reusable form utilities and components
- 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)
This commit is contained in:
@@ -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<AgentTypeCreateFormValues>({
|
||||
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<AgentTypeCreateFormValues>): { 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<string, { message?: string }>)[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<AgentTypeCreateFormValues>) => {
|
||||
// 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<AgentTypeCreateFormValues>({
|
||||
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<HTMLFormElement>) => {
|
||||
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<HTMLFormElement>) => {
|
||||
return handleSubmit(onSubmit, onValidationError)(e);
|
||||
},
|
||||
[form, handleSubmit, onSubmit, handleFormError]
|
||||
[handleSubmit, onSubmit, onValidationError]
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
133
frontend/src/components/forms/FormSelect.tsx
Normal file
133
frontend/src/components/forms/FormSelect.tsx
Normal file
@@ -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<T extends FieldValues> {
|
||||
/** Field name (must be a valid path in the form) */
|
||||
name: Path<T>;
|
||||
/** Form control from useForm */
|
||||
control: Control<T>;
|
||||
/** 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
|
||||
* <FormSelect
|
||||
* name="primary_model"
|
||||
* control={form.control}
|
||||
* label="Primary Model"
|
||||
* required
|
||||
* options={[
|
||||
* { value: 'claude-opus', label: 'Claude Opus' },
|
||||
* { value: 'claude-sonnet', label: 'Claude Sonnet' },
|
||||
* ]}
|
||||
* description="Main model used for this agent"
|
||||
* />
|
||||
* ```
|
||||
*/
|
||||
export function FormSelect<T extends FieldValues>({
|
||||
name,
|
||||
control,
|
||||
label,
|
||||
options,
|
||||
required = false,
|
||||
placeholder,
|
||||
description,
|
||||
disabled = false,
|
||||
className,
|
||||
}: FormSelectProps<T>) {
|
||||
const selectId = String(name);
|
||||
const errorId = `${selectId}-error`;
|
||||
const descriptionId = description ? `${selectId}-description` : undefined;
|
||||
|
||||
return (
|
||||
<Controller
|
||||
name={name}
|
||||
control={control}
|
||||
render={({ field, fieldState }) => (
|
||||
<div className={className}>
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor={selectId}>
|
||||
{label}
|
||||
{required && <span className="text-destructive"> *</span>}
|
||||
</Label>
|
||||
<Select value={field.value ?? ''} onValueChange={field.onChange} disabled={disabled}>
|
||||
<SelectTrigger
|
||||
id={selectId}
|
||||
aria-invalid={!!fieldState.error}
|
||||
aria-describedby={
|
||||
[fieldState.error ? errorId : null, descriptionId].filter(Boolean).join(' ') ||
|
||||
undefined
|
||||
}
|
||||
>
|
||||
<SelectValue placeholder={placeholder ?? `Select ${label.toLowerCase()}`} />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{options.map((option) => (
|
||||
<SelectItem key={option.value} value={option.value}>
|
||||
{option.label}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
{fieldState.error && (
|
||||
<p id={errorId} className="text-sm text-destructive" role="alert">
|
||||
{fieldState.error.message}
|
||||
</p>
|
||||
)}
|
||||
{description && (
|
||||
<p id={descriptionId} className="text-xs text-muted-foreground">
|
||||
{description}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
101
frontend/src/components/forms/FormTextarea.tsx
Normal file
101
frontend/src/components/forms/FormTextarea.tsx
Normal file
@@ -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<ComponentProps<typeof Textarea>, '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
|
||||
* <FormTextarea
|
||||
* label="Personality Prompt"
|
||||
* required
|
||||
* error={errors.personality_prompt}
|
||||
* rows={10}
|
||||
* {...register('personality_prompt')}
|
||||
* />
|
||||
* ```
|
||||
*/
|
||||
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 (
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor={name}>
|
||||
{label}
|
||||
{required && <span className="text-destructive"> *</span>}
|
||||
</Label>
|
||||
{description && (
|
||||
<p id={descriptionId} className="text-sm text-muted-foreground">
|
||||
{description}
|
||||
</p>
|
||||
)}
|
||||
<Textarea
|
||||
id={name}
|
||||
aria-invalid={!!error}
|
||||
aria-describedby={ariaDescribedBy}
|
||||
{...mergedProps}
|
||||
/>
|
||||
{error && (
|
||||
<p id={errorId} className="text-sm text-destructive" role="alert">
|
||||
{error.message}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -1,5 +1,9 @@
|
||||
// Shared form components and utilities
|
||||
export { FormField } from './FormField';
|
||||
export type { FormFieldProps } from './FormField';
|
||||
export { FormSelect } from './FormSelect';
|
||||
export type { FormSelectProps, SelectOption } from './FormSelect';
|
||||
export { FormTextarea } from './FormTextarea';
|
||||
export type { FormTextareaProps } from './FormTextarea';
|
||||
export { useFormError } from './useFormError';
|
||||
export type { UseFormErrorReturn } from './useFormError';
|
||||
|
||||
Reference in New Issue
Block a user