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) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Tests for useValidationErrorHandler hook
|
||||
*/
|
||||
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { toast } from 'sonner';
|
||||
import { useValidationErrorHandler } from '@/lib/forms/hooks/useValidationErrorHandler';
|
||||
import type { FieldErrors } from 'react-hook-form';
|
||||
|
||||
// Mock sonner toast
|
||||
jest.mock('sonner', () => ({
|
||||
toast: {
|
||||
error: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
// Mock console.error to track debug logging
|
||||
const originalConsoleError = console.error;
|
||||
let consoleErrorMock: jest.SpyInstance;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
consoleErrorMock.mockRestore();
|
||||
console.error = originalConsoleError;
|
||||
});
|
||||
|
||||
describe('useValidationErrorHandler', () => {
|
||||
describe('basic functionality', () => {
|
||||
it('shows toast with first error message', () => {
|
||||
const { result } = renderHook(() => useValidationErrorHandler({ debug: false }));
|
||||
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Name is required', type: 'required' },
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(toast.error).toHaveBeenCalledWith('Please fix form errors', {
|
||||
description: 'name: Name is required',
|
||||
});
|
||||
});
|
||||
|
||||
it('uses custom toast title when provided', () => {
|
||||
const { result } = renderHook(() =>
|
||||
useValidationErrorHandler({
|
||||
toastTitle: 'Validation Failed',
|
||||
debug: false,
|
||||
})
|
||||
);
|
||||
|
||||
const errors: FieldErrors = {
|
||||
email: { message: 'Invalid email', type: 'pattern' },
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(toast.error).toHaveBeenCalledWith('Validation Failed', {
|
||||
description: 'email: Invalid email',
|
||||
});
|
||||
});
|
||||
|
||||
it('does nothing when no errors', () => {
|
||||
const { result } = renderHook(() => useValidationErrorHandler({ debug: false }));
|
||||
|
||||
result.current.onValidationError({});
|
||||
|
||||
expect(toast.error).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('nested errors', () => {
|
||||
it('handles nested field errors', () => {
|
||||
const { result } = renderHook(() => useValidationErrorHandler({ debug: false }));
|
||||
|
||||
const errors: FieldErrors = {
|
||||
model_params: {
|
||||
temperature: { message: 'Temperature must be between 0 and 2', type: 'max' },
|
||||
},
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(toast.error).toHaveBeenCalledWith('Please fix form errors', {
|
||||
description: 'model_params.temperature: Temperature must be between 0 and 2',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('tab navigation', () => {
|
||||
it('navigates to correct tab when mapping provided', () => {
|
||||
const setActiveTab = jest.fn();
|
||||
const tabMapping = {
|
||||
name: 'basic',
|
||||
model_params: 'model',
|
||||
};
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useValidationErrorHandler({
|
||||
tabMapping,
|
||||
setActiveTab,
|
||||
debug: false,
|
||||
})
|
||||
);
|
||||
|
||||
const errors: FieldErrors = {
|
||||
model_params: {
|
||||
temperature: { message: 'Invalid', type: 'type' },
|
||||
},
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(setActiveTab).toHaveBeenCalledWith('model');
|
||||
});
|
||||
|
||||
it('does not navigate if field not in mapping', () => {
|
||||
const setActiveTab = jest.fn();
|
||||
const tabMapping = {
|
||||
name: 'basic',
|
||||
};
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useValidationErrorHandler({
|
||||
tabMapping,
|
||||
setActiveTab,
|
||||
debug: false,
|
||||
})
|
||||
);
|
||||
|
||||
const errors: FieldErrors = {
|
||||
unknown_field: { message: 'Error', type: 'validation' },
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(setActiveTab).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not crash when setActiveTab not provided', () => {
|
||||
const tabMapping = { name: 'basic' };
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useValidationErrorHandler({
|
||||
tabMapping,
|
||||
// setActiveTab not provided
|
||||
debug: false,
|
||||
})
|
||||
);
|
||||
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Required', type: 'required' },
|
||||
};
|
||||
|
||||
expect(() => result.current.onValidationError(errors)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('debug logging', () => {
|
||||
it('logs errors when debug is true', () => {
|
||||
const { result } = renderHook(() => useValidationErrorHandler({ debug: true }));
|
||||
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Required', type: 'required' },
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(consoleErrorMock).toHaveBeenCalledWith('[Form Validation] Errors:', errors);
|
||||
});
|
||||
|
||||
it('does not log errors when debug is false', () => {
|
||||
const { result } = renderHook(() => useValidationErrorHandler({ debug: false }));
|
||||
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Required', type: 'required' },
|
||||
};
|
||||
|
||||
result.current.onValidationError(errors);
|
||||
|
||||
expect(consoleErrorMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('memoization', () => {
|
||||
it('returns stable callback reference', () => {
|
||||
const { result, rerender } = renderHook(() => useValidationErrorHandler({ debug: false }));
|
||||
|
||||
const firstCallback = result.current.onValidationError;
|
||||
rerender();
|
||||
const secondCallback = result.current.onValidationError;
|
||||
|
||||
expect(firstCallback).toBe(secondCallback);
|
||||
});
|
||||
|
||||
it('returns new callback when options change', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
({ title }) => useValidationErrorHandler({ toastTitle: title, debug: false }),
|
||||
{ initialProps: { title: 'Error A' } }
|
||||
);
|
||||
|
||||
const firstCallback = result.current.onValidationError;
|
||||
rerender({ title: 'Error B' });
|
||||
const secondCallback = result.current.onValidationError;
|
||||
|
||||
expect(firstCallback).not.toBe(secondCallback);
|
||||
});
|
||||
});
|
||||
});
|
||||
134
frontend/tests/lib/forms/utils/getFirstValidationError.test.ts
Normal file
134
frontend/tests/lib/forms/utils/getFirstValidationError.test.ts
Normal file
@@ -0,0 +1,134 @@
|
||||
/**
|
||||
* Tests for getFirstValidationError utility
|
||||
*/
|
||||
|
||||
import {
|
||||
getFirstValidationError,
|
||||
getAllValidationErrors,
|
||||
} from '@/lib/forms/utils/getFirstValidationError';
|
||||
import type { FieldErrors } from 'react-hook-form';
|
||||
|
||||
describe('getFirstValidationError', () => {
|
||||
it('returns null for empty errors object', () => {
|
||||
const result = getFirstValidationError({});
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it('extracts direct error message', () => {
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Name is required', type: 'required' },
|
||||
};
|
||||
const result = getFirstValidationError(errors);
|
||||
expect(result).toEqual({ field: 'name', message: 'Name is required' });
|
||||
});
|
||||
|
||||
it('extracts nested error message', () => {
|
||||
const errors: FieldErrors = {
|
||||
model_params: {
|
||||
temperature: { message: 'Temperature must be a number', type: 'type' },
|
||||
},
|
||||
};
|
||||
const result = getFirstValidationError(errors);
|
||||
expect(result).toEqual({
|
||||
field: 'model_params.temperature',
|
||||
message: 'Temperature must be a number',
|
||||
});
|
||||
});
|
||||
|
||||
it('returns first error when multiple fields have errors', () => {
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Name is required', type: 'required' },
|
||||
slug: { message: 'Slug is required', type: 'required' },
|
||||
};
|
||||
const result = getFirstValidationError(errors);
|
||||
// Object.keys order is insertion order, so 'name' comes first
|
||||
expect(result?.field).toBe('name');
|
||||
expect(result?.message).toBe('Name is required');
|
||||
});
|
||||
|
||||
it('handles deeply nested errors', () => {
|
||||
const errors: FieldErrors = {
|
||||
config: {
|
||||
nested: {
|
||||
deep: { message: 'Deep error', type: 'validation' },
|
||||
},
|
||||
},
|
||||
};
|
||||
const result = getFirstValidationError(errors);
|
||||
expect(result).toEqual({ field: 'config.nested.deep', message: 'Deep error' });
|
||||
});
|
||||
|
||||
it('skips null error entries', () => {
|
||||
const errors: FieldErrors = {
|
||||
name: null as unknown as undefined,
|
||||
slug: { message: 'Slug is required', type: 'required' },
|
||||
};
|
||||
const result = getFirstValidationError(errors);
|
||||
expect(result).toEqual({ field: 'slug', message: 'Slug is required' });
|
||||
});
|
||||
|
||||
it('handles error object with ref but no message', () => {
|
||||
// react-hook-form errors may have 'ref' property but no 'message'
|
||||
// We cast to FieldErrors to simulate edge cases
|
||||
const errors = {
|
||||
name: { type: 'required', ref: { current: null } },
|
||||
slug: { message: 'Slug is required', type: 'required' },
|
||||
} as unknown as FieldErrors;
|
||||
const result = getFirstValidationError(errors);
|
||||
// Should skip name (no message) and find slug
|
||||
expect(result).toEqual({ field: 'slug', message: 'Slug is required' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllValidationErrors', () => {
|
||||
it('returns empty array for empty errors object', () => {
|
||||
const result = getAllValidationErrors({});
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns all errors as flat array', () => {
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Name is required', type: 'required' },
|
||||
slug: { message: 'Slug is required', type: 'required' },
|
||||
};
|
||||
const result = getAllValidationErrors(errors);
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toContainEqual({ field: 'name', message: 'Name is required' });
|
||||
expect(result).toContainEqual({ field: 'slug', message: 'Slug is required' });
|
||||
});
|
||||
|
||||
it('flattens nested errors', () => {
|
||||
const errors: FieldErrors = {
|
||||
model_params: {
|
||||
temperature: { message: 'Invalid temperature', type: 'type' },
|
||||
max_tokens: { message: 'Invalid max tokens', type: 'type' },
|
||||
},
|
||||
};
|
||||
const result = getAllValidationErrors(errors);
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toContainEqual({
|
||||
field: 'model_params.temperature',
|
||||
message: 'Invalid temperature',
|
||||
});
|
||||
expect(result).toContainEqual({
|
||||
field: 'model_params.max_tokens',
|
||||
message: 'Invalid max tokens',
|
||||
});
|
||||
});
|
||||
|
||||
it('combines direct and nested errors', () => {
|
||||
const errors: FieldErrors = {
|
||||
name: { message: 'Name is required', type: 'required' },
|
||||
model_params: {
|
||||
temperature: { message: 'Invalid temperature', type: 'type' },
|
||||
},
|
||||
};
|
||||
const result = getAllValidationErrors(errors);
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result).toContainEqual({ field: 'name', message: 'Name is required' });
|
||||
expect(result).toContainEqual({
|
||||
field: 'model_params.temperature',
|
||||
message: 'Invalid temperature',
|
||||
});
|
||||
});
|
||||
});
|
||||
247
frontend/tests/lib/forms/utils/mergeWithDefaults.test.ts
Normal file
247
frontend/tests/lib/forms/utils/mergeWithDefaults.test.ts
Normal file
@@ -0,0 +1,247 @@
|
||||
/**
|
||||
* Tests for mergeWithDefaults utilities
|
||||
*/
|
||||
|
||||
import {
|
||||
safeValue,
|
||||
isNumber,
|
||||
isString,
|
||||
isBoolean,
|
||||
isArray,
|
||||
isObject,
|
||||
deepMergeWithDefaults,
|
||||
createFormInitializer,
|
||||
} from '@/lib/forms/utils/mergeWithDefaults';
|
||||
|
||||
describe('Type Guards', () => {
|
||||
describe('isNumber', () => {
|
||||
it('returns true for valid numbers', () => {
|
||||
expect(isNumber(0)).toBe(true);
|
||||
expect(isNumber(42)).toBe(true);
|
||||
expect(isNumber(-10)).toBe(true);
|
||||
expect(isNumber(3.14)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for NaN', () => {
|
||||
expect(isNumber(NaN)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for non-numbers', () => {
|
||||
expect(isNumber('42')).toBe(false);
|
||||
expect(isNumber(null)).toBe(false);
|
||||
expect(isNumber(undefined)).toBe(false);
|
||||
expect(isNumber({})).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isString', () => {
|
||||
it('returns true for strings', () => {
|
||||
expect(isString('')).toBe(true);
|
||||
expect(isString('hello')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for non-strings', () => {
|
||||
expect(isString(42)).toBe(false);
|
||||
expect(isString(null)).toBe(false);
|
||||
expect(isString(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isBoolean', () => {
|
||||
it('returns true for booleans', () => {
|
||||
expect(isBoolean(true)).toBe(true);
|
||||
expect(isBoolean(false)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for non-booleans', () => {
|
||||
expect(isBoolean(0)).toBe(false);
|
||||
expect(isBoolean(1)).toBe(false);
|
||||
expect(isBoolean('true')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isArray', () => {
|
||||
it('returns true for arrays', () => {
|
||||
expect(isArray([])).toBe(true);
|
||||
expect(isArray([1, 2, 3])).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for non-arrays', () => {
|
||||
expect(isArray({})).toBe(false);
|
||||
expect(isArray('array')).toBe(false);
|
||||
expect(isArray(null)).toBe(false);
|
||||
});
|
||||
|
||||
it('validates item types when itemCheck provided', () => {
|
||||
expect(isArray([1, 2, 3], isNumber)).toBe(true);
|
||||
expect(isArray(['a', 'b'], isString)).toBe(true);
|
||||
expect(isArray([1, 'two', 3], isNumber)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isObject', () => {
|
||||
it('returns true for plain objects', () => {
|
||||
expect(isObject({})).toBe(true);
|
||||
expect(isObject({ key: 'value' })).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false for null', () => {
|
||||
expect(isObject(null)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for arrays', () => {
|
||||
expect(isObject([])).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for primitives', () => {
|
||||
expect(isObject('string')).toBe(false);
|
||||
expect(isObject(42)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('safeValue', () => {
|
||||
it('returns value when type check passes', () => {
|
||||
expect(safeValue(42, 0, isNumber)).toBe(42);
|
||||
expect(safeValue('hello', '', isString)).toBe('hello');
|
||||
});
|
||||
|
||||
it('returns default when type check fails', () => {
|
||||
expect(safeValue('not a number', 0, isNumber)).toBe(0);
|
||||
expect(safeValue(42, '', isString)).toBe('');
|
||||
});
|
||||
|
||||
it('returns default for null/undefined', () => {
|
||||
expect(safeValue(null, 0, isNumber)).toBe(0);
|
||||
expect(safeValue(undefined, 'default', isString)).toBe('default');
|
||||
});
|
||||
});
|
||||
|
||||
describe('deepMergeWithDefaults', () => {
|
||||
it('returns defaults when source is null', () => {
|
||||
const defaults = { name: 'default', value: 10 };
|
||||
expect(deepMergeWithDefaults(defaults, null)).toEqual(defaults);
|
||||
});
|
||||
|
||||
it('returns defaults when source is undefined', () => {
|
||||
const defaults = { name: 'default', value: 10 };
|
||||
expect(deepMergeWithDefaults(defaults, undefined)).toEqual(defaults);
|
||||
});
|
||||
|
||||
it('merges source values over defaults', () => {
|
||||
const defaults = { name: 'default', value: 10 };
|
||||
const source = { name: 'custom' };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
name: 'custom',
|
||||
value: 10,
|
||||
});
|
||||
});
|
||||
|
||||
it('preserves default for missing source keys', () => {
|
||||
const defaults = { a: 1, b: 2, c: 3 };
|
||||
const source = { a: 10 };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
a: 10,
|
||||
b: 2,
|
||||
c: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it('recursively merges nested objects', () => {
|
||||
const defaults = {
|
||||
config: { temperature: 0.7, max_tokens: 8192 },
|
||||
};
|
||||
// Source has partial nested config - deepMerge fills in missing fields
|
||||
const source = {
|
||||
config: { temperature: 0.5 },
|
||||
} as unknown as Partial<typeof defaults>;
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
config: { temperature: 0.5, max_tokens: 8192 },
|
||||
});
|
||||
});
|
||||
|
||||
it('only uses source values if types match', () => {
|
||||
const defaults = { value: 10, name: 'default' };
|
||||
const source = { value: 'not a number' as unknown as number };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
value: 10,
|
||||
name: 'default',
|
||||
});
|
||||
});
|
||||
|
||||
it('handles arrays - uses source array if types match', () => {
|
||||
const defaults = { items: ['a', 'b'] };
|
||||
const source = { items: ['c', 'd', 'e'] };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
items: ['c', 'd', 'e'],
|
||||
});
|
||||
});
|
||||
|
||||
it('skips undefined source values', () => {
|
||||
const defaults = { name: 'default' };
|
||||
const source = { name: undefined };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({
|
||||
name: 'default',
|
||||
});
|
||||
});
|
||||
|
||||
it('handles null values when defaults are null', () => {
|
||||
const defaults = { value: null };
|
||||
const source = { value: null };
|
||||
expect(deepMergeWithDefaults(defaults, source)).toEqual({ value: null });
|
||||
});
|
||||
});
|
||||
|
||||
describe('createFormInitializer', () => {
|
||||
it('returns defaults when called with null', () => {
|
||||
const defaults = { name: '', age: 0 };
|
||||
const initializer = createFormInitializer(defaults);
|
||||
expect(initializer(null)).toEqual(defaults);
|
||||
});
|
||||
|
||||
it('returns defaults when called with undefined', () => {
|
||||
const defaults = { name: '', age: 0 };
|
||||
const initializer = createFormInitializer(defaults);
|
||||
expect(initializer(undefined)).toEqual(defaults);
|
||||
});
|
||||
|
||||
it('merges API data with defaults', () => {
|
||||
const defaults = { name: '', age: 0, active: false };
|
||||
const initializer = createFormInitializer(defaults);
|
||||
const result = initializer({ name: 'John', age: 25 });
|
||||
expect(result).toEqual({ name: 'John', age: 25, active: false });
|
||||
});
|
||||
|
||||
it('uses custom transform function when provided', () => {
|
||||
interface Form {
|
||||
fullName: string;
|
||||
isActive: boolean;
|
||||
}
|
||||
interface Api {
|
||||
first_name: string;
|
||||
last_name: string;
|
||||
active: boolean;
|
||||
}
|
||||
|
||||
const defaults: Form = { fullName: '', isActive: false };
|
||||
const initializer = createFormInitializer<Form, Api>(defaults, (apiData, defs) => ({
|
||||
fullName: apiData ? `${apiData.first_name} ${apiData.last_name}` : defs.fullName,
|
||||
isActive: apiData?.active ?? defs.isActive,
|
||||
}));
|
||||
|
||||
const result = initializer({
|
||||
first_name: 'John',
|
||||
last_name: 'Doe',
|
||||
active: true,
|
||||
});
|
||||
expect(result).toEqual({ fullName: 'John Doe', isActive: true });
|
||||
});
|
||||
|
||||
it('transform receives defaults when apiData is null', () => {
|
||||
const defaults = { name: 'default' };
|
||||
const initializer = createFormInitializer(defaults, (_, defs) => ({
|
||||
name: defs.name.toUpperCase(),
|
||||
}));
|
||||
expect(initializer(null)).toEqual({ name: 'DEFAULT' });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user