From eee873195a3414ca6d37c61b0518d2e7f1ece4b3 Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Wed, 24 Jun 2026 10:55:39 -0700 Subject: [PATCH 1/4] fix(PaySchedule): clear server-side field errors on edit (SDK-923) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SDKFormProvider` mirrors server `fieldErrors` onto the form via `setError(name, { type: 'custom', ... })`. Without a clear-on-edit behavior in this form, that error stays set until a full refresh, blocking resubmission after the user corrects a date — the original SDK-923 repro. Subscribe to `formMethods.watch` inside `usePayScheduleForm` and clear any `type: 'custom'` error on a field as soon as the user edits it. RHF's values subject only fires for actual value changes (not blur), so the subscription is narrow. Client-side validation errors (`required`, `min`, zod-driven, etc.) are untouched — the type filter only matches the server-error path. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../usePayScheduleForm.test.tsx | 63 +++++++++++++++++++ .../usePayScheduleForm/usePayScheduleForm.tsx | 16 +++++ 2 files changed, 79 insertions(+) diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx index 171ec2ec7..c482109ff 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx @@ -351,6 +351,69 @@ describe('usePayScheduleForm', () => { expect(submitResult).toEqual(expect.objectContaining({ mode: 'update' })) }) }) + + describe('clearing server-side errors on edit (SDK-923)', () => { + it('clears a custom (server-side) error on a field when the user edits it', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('anchorPayDate', { + type: 'custom', + message: 'Pay date must be after start date', + }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error).toMatchObject({ + type: 'custom', + }) + }) + + act(() => { + formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error).toBeUndefined() + }) + }) + + it('does not clear non-custom (client-side) validation errors on edit', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('anchorPayDate', { type: 'required', message: 'Required' }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') + }) + + act(() => { + formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) + }) + + // Edit must NOT clear a client-side validation error — RHF's normal + // validation loop owns that. + expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') + }) + }) }) // ── Schema-level tests ────────────────────────────────────────────────── diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx index 05c8b4e57..f54ad74f8 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx @@ -309,6 +309,22 @@ export function usePayScheduleForm({ } }, [watchedFrequency, watchedCustomTwicePerMonth, formMethods.setValue]) + // Server-side validation errors (mirrored via SDKFormProvider as + // `type: 'custom'`) need to clear the moment the user edits a field; + // otherwise the stale error blocks resubmission until a refresh (SDK-923). + useEffect(() => { + const subscription = formMethods.watch((_, info) => { + const { name } = info + if (!name) return + if (formMethods.getFieldState(name).error?.type === 'custom') { + formMethods.clearErrors(name) + } + }) + return () => { + subscription.unsubscribe() + } + }, [formMethods]) + const formattedAnchorPayDate = formatWatchedDate(watchedAnchorPayDate) const formattedAnchorEndOfPayPeriod = formatWatchedDate(watchedAnchorEndOfPayPeriod) From 8192c5e57c932d78f22a97dacbca699f997cbad9 Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Fri, 26 Jun 2026 10:59:59 -0700 Subject: [PATCH 2/4] refactor(SDK-923): move clear-on-edit to useField + appliedRef in provider Address Steve's review: replace the per-render `formMethods.watch` subscription in `usePayScheduleForm` with an event-based clear inside `useField.handleChange`, so the fix lives in the shared form layer and only fires for the field actually being edited. Pair it with an `appliedRef` in `useSyncFieldErrors` so the effect no longer re-applies server errors on every render (which was overwriting the clear and trapping users in a stale error until refresh). Relocate the SDK-923 unit tests onto `useField` and add an 8-case permutation suite for `SDKFormProvider` error clearing. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Common/Fields/hooks/useField.test.tsx | 73 ++++++ .../Common/Fields/hooks/useField.ts | 6 + .../usePayScheduleForm.test.tsx | 63 ----- .../usePayScheduleForm/usePayScheduleForm.tsx | 16 -- .../SDKFormProvider.errorClearing.test.tsx | 219 ++++++++++++++++++ .../form/SDKFormProvider.tsx | 25 +- 6 files changed, 321 insertions(+), 81 deletions(-) create mode 100644 src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx diff --git a/src/components/Common/Fields/hooks/useField.test.tsx b/src/components/Common/Fields/hooks/useField.test.tsx index 240db9e70..8ca8617f1 100644 --- a/src/components/Common/Fields/hooks/useField.test.tsx +++ b/src/components/Common/Fields/hooks/useField.test.tsx @@ -145,6 +145,79 @@ describe('useField', () => { expect(result.current.value).toBe('some-test-value') }) + describe('clearing server-side errors on edit (SDK-923)', () => { + function renderClearOnEditHarness() { + let methods!: ReturnType + let field!: ReturnType> + function Probe({ onField }: { onField: (f: ReturnType>) => void }) { + const f = useField({ name: 'testField' }) + React.useEffect(() => { + onField(f) + }) + return null + } + const Wrapper = () => { + const m = useForm({ mode: 'onTouched' }) + methods = m + return ( + + (field = f)} /> + + ) + } + render() + return { + getMethods: () => methods, + getField: () => field, + } + } + + test('clears a custom (server-side) error on the field when the user edits it', async () => { + const { getMethods, getField } = renderClearOnEditHarness() + + act(() => { + getMethods().setError('testField', { + type: 'custom', + message: 'Server says no', + }) + }) + + await waitFor(() => { + expect(getMethods().getFieldState('testField').error).toMatchObject({ + type: 'custom', + }) + }) + + act(() => { + getField().onChange('user edit') + }) + + await waitFor(() => { + expect(getMethods().getFieldState('testField').error).toBeUndefined() + }) + }) + + test('does not clear non-custom (client-side) validation errors on edit', async () => { + const { getMethods, getField } = renderClearOnEditHarness() + + act(() => { + getMethods().setError('testField', { type: 'required', message: 'Required' }) + }) + + await waitFor(() => { + expect(getMethods().getFieldState('testField').error?.type).toBe('required') + }) + + act(() => { + getField().onChange('user edit') + }) + + // RHF's normal validation loop owns client-side errors — our clear-on-edit + // logic must be scoped to `type: 'custom'`. + expect(getMethods().getFieldState('testField').error?.type).toBe('required') + }) + }) + describe('description processing', () => { test('should return non-string descriptions as-is', () => { const jsxElement = JSX element diff --git a/src/components/Common/Fields/hooks/useField.ts b/src/components/Common/Fields/hooks/useField.ts index fa091bed6..00f81a81f 100644 --- a/src/components/Common/Fields/hooks/useField.ts +++ b/src/components/Common/Fields/hooks/useField.ts @@ -81,6 +81,12 @@ export function useField({ const handleChange = (updatedValue: TValue) => { const value = transform ? transform(updatedValue) : updatedValue + // Server-side validation errors (mirrored by SDKFormProvider as + // `type: 'custom'`) must clear when the user edits the field; otherwise + // the stale error blocks resubmission until refresh (SDK-923). + if (fieldState.error?.type === 'custom') { + formContext?.clearErrors(name) + } field.onChange(value) onChange?.(value) } diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx index c482109ff..171ec2ec7 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx @@ -351,69 +351,6 @@ describe('usePayScheduleForm', () => { expect(submitResult).toEqual(expect.objectContaining({ mode: 'update' })) }) }) - - describe('clearing server-side errors on edit (SDK-923)', () => { - it('clears a custom (server-side) error on a field when the user edits it', async () => { - const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { - wrapper: GustoTestProvider, - }) - - await waitFor(() => { - expect(result.current.isLoading).toBe(false) - }) - assertReady(result.current) - const { formMethods } = result.current.form.hookFormInternals - - act(() => { - formMethods.setError('anchorPayDate', { - type: 'custom', - message: 'Pay date must be after start date', - }) - }) - - await waitFor(() => { - expect(formMethods.getFieldState('anchorPayDate').error).toMatchObject({ - type: 'custom', - }) - }) - - act(() => { - formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) - }) - - await waitFor(() => { - expect(formMethods.getFieldState('anchorPayDate').error).toBeUndefined() - }) - }) - - it('does not clear non-custom (client-side) validation errors on edit', async () => { - const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { - wrapper: GustoTestProvider, - }) - - await waitFor(() => { - expect(result.current.isLoading).toBe(false) - }) - assertReady(result.current) - const { formMethods } = result.current.form.hookFormInternals - - act(() => { - formMethods.setError('anchorPayDate', { type: 'required', message: 'Required' }) - }) - - await waitFor(() => { - expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') - }) - - act(() => { - formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) - }) - - // Edit must NOT clear a client-side validation error — RHF's normal - // validation loop owns that. - expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') - }) - }) }) // ── Schema-level tests ────────────────────────────────────────────────── diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx index f54ad74f8..05c8b4e57 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx @@ -309,22 +309,6 @@ export function usePayScheduleForm({ } }, [watchedFrequency, watchedCustomTwicePerMonth, formMethods.setValue]) - // Server-side validation errors (mirrored via SDKFormProvider as - // `type: 'custom'`) need to clear the moment the user edits a field; - // otherwise the stale error blocks resubmission until a refresh (SDK-923). - useEffect(() => { - const subscription = formMethods.watch((_, info) => { - const { name } = info - if (!name) return - if (formMethods.getFieldState(name).error?.type === 'custom') { - formMethods.clearErrors(name) - } - }) - return () => { - subscription.unsubscribe() - } - }, [formMethods]) - const formattedAnchorPayDate = formatWatchedDate(watchedAnchorPayDate) const formattedAnchorEndOfPayPeriod = formatWatchedDate(watchedAnchorEndOfPayPeriod) diff --git a/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx b/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx new file mode 100644 index 000000000..62485908f --- /dev/null +++ b/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx @@ -0,0 +1,219 @@ +import { describe, expect, it, vi } from 'vitest' +import { screen, act } from '@testing-library/react' +import { userEvent } from '@testing-library/user-event' +import { useForm } from 'react-hook-form' +import { zodResolver } from '@hookform/resolvers/zod' +import { z } from 'zod' +import { useState } from 'react' +import type { FieldMetadata } from '../types' +import { SDKFormProvider } from './SDKFormProvider' +import { useHookFormInternals } from './useHookFormInternals' +import { TextInputField } from '@/components/Common/Fields/TextInputField/TextInputField' +import { renderWithProviders } from '@/test-utils/renderWithProviders' +import type { SDKError, SDKFieldError } from '@/types/sdkError' + +type Values = { name: string } + +const schema = z.object({ + name: z.string().refine(v => v.length > 0, { message: 'REQUIRED' }), +}) + +const metadata: Record = { + name: { isRequired: true, isDisabled: false, hasRedactedValue: false } as FieldMetadata, +} + +function Harness({ initialFieldErrors = [] }: { initialFieldErrors?: SDKFieldError[] }) { + const formMethods = useForm({ + mode: 'onSubmit', + resolver: zodResolver(schema), + defaultValues: { name: 'original' }, + }) + + const hookFormInternals = useHookFormInternals(formMethods) + const [fieldErrors] = useState(initialFieldErrors) + + const sdkErrors: SDKError[] = fieldErrors.length + ? [ + { + category: 'api_error', + message: 'fields have issues', + httpStatus: 422, + fieldErrors, + }, + ] + : [] + + const formHookResult = { + errorHandling: { errors: sdkErrors, retryQueries: vi.fn(), clearSubmitError: vi.fn() }, + form: { + fieldsMetadata: metadata, + hookFormInternals, + }, + } + + return ( + + + + + ) +} + +describe('SDKFormProvider error clearing — permutations', () => { + describe('client-side (zod) errors', () => { + it('shows required error after submit with empty value', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await user.clear(input) + }) + await act(async () => { + await user.click(screen.getByRole('button', { name: 'submit' })) + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + }) + + it('clears client error when user types a valid value', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await user.clear(input) + }) + await act(async () => { + await user.click(screen.getByRole('button', { name: 'submit' })) + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + await act(async () => { + await user.type(input, 'x') + }) + expect(input).toHaveAttribute('aria-invalid', 'false') + }) + + it('re-applies client error when user reverts to invalid empty', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await user.clear(input) + }) + await act(async () => { + await user.click(screen.getByRole('button', { name: 'submit' })) + }) + await act(async () => { + await user.type(input, 'x') + }) + await act(async () => { + await user.clear(input) + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + }) + }) + + describe('server-side (SDKError fieldErrors) errors', () => { + const fieldError: SDKFieldError = { + field: 'name', + category: 'invalid_attribute_value', + message: 'Name is taken', + } + + it('surfaces server fieldError onto the field', async () => { + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + // useSyncFieldErrors runs in an effect, so wait a tick + await act(async () => { + await Promise.resolve() + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + expect(screen.getByText('Name is taken')).toBeInTheDocument() + }) + + it('clears server error when user types a different value', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await Promise.resolve() + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + await act(async () => { + await user.type(input, 'X') + }) + expect(input).toHaveAttribute('aria-invalid', 'false') + expect(screen.queryByText('Name is taken')).not.toBeInTheDocument() + }) + + it('clears server error when user clears the field', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await Promise.resolve() + }) + await act(async () => { + await user.clear(input) + }) + expect(input).toHaveAttribute('aria-invalid', 'false') + }) + + it('clears server error when user types the same value back', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await Promise.resolve() + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + await act(async () => { + await user.clear(input) + await user.type(input, 'original') + }) + expect(input).toHaveAttribute('aria-invalid', 'false') + }) + }) + + describe('server error persistence across re-renders', () => { + const fieldError: SDKFieldError = { + field: 'name', + category: 'invalid_attribute_value', + message: 'Name is taken', + } + + function Outer() { + const [, setTick] = useState(0) + return ( + <> + + + + ) + } + + it('keeps server error cleared after outer re-render', async () => { + const user = userEvent.setup() + renderWithProviders() + const input = screen.getByLabelText(/Name/) as HTMLInputElement + await act(async () => { + await Promise.resolve() + }) + expect(input).toHaveAttribute('aria-invalid', 'true') + + await act(async () => { + await user.type(input, 'X') + }) + expect(input).toHaveAttribute('aria-invalid', 'false') + + await act(async () => { + await user.click(screen.getByRole('button', { name: /rerender outer/ })) + }) + // If useSyncFieldErrors re-applies the server error on every render + // (because fieldsMetadata reference changes), this will fail. + expect(input).toHaveAttribute('aria-invalid', 'false') + }) + }) +}) diff --git a/src/partner-hook-utils/form/SDKFormProvider.tsx b/src/partner-hook-utils/form/SDKFormProvider.tsx index 87d711483..d31bf407a 100644 --- a/src/partner-hook-utils/form/SDKFormProvider.tsx +++ b/src/partner-hook-utils/form/SDKFormProvider.tsx @@ -1,4 +1,4 @@ -import { type ReactNode, useEffect } from 'react' +import { type ReactNode, useEffect, useRef } from 'react' import type { FieldPath, FieldValues } from 'react-hook-form' import { FormProvider } from 'react-hook-form' import type { FieldMetadata, FieldMetadataWithOptions, HookFormInternals } from '../types' @@ -7,6 +7,10 @@ import { FieldElementRegistryProvider } from '@/components/Common/Fields/hooks/F import { normalizeErrorKeyForForm } from '@/helpers/formattedStrings' import type { SDKError, SDKFieldError } from '@/types/sdkError' +function fieldErrorKey(fe: SDKFieldError): string { + return `${fe.field}::${fe.message}` +} + function useSyncFieldErrors< TFormData extends FieldValues, TFieldsMetadata extends { @@ -21,17 +25,34 @@ function useSyncFieldErrors< ) { const { fieldsMetadata } = form const { setError } = form.hookFormInternals.formMethods + // Track which (field, message) tuples have already been pushed into RHF state. + // Without this, the effect re-applies every server error on every render — + // overwriting `useField.handleChange`'s clearErrors and trapping users in a + // stale error state until the next submit (SDK-923). + const appliedRef = useRef>(new Set()) useEffect(() => { - if (!fieldErrors.length) return + if (fieldErrors.length === 0) { + appliedRef.current.clear() + return + } const knownFields = new Set(Object.keys(fieldsMetadata)) + const currentKeys = new Set(fieldErrors.map(fieldErrorKey)) + // Drop tracking for tuples that disappeared from the latest response so + // they can be re-applied later if the same error comes back. + for (const key of appliedRef.current) { + if (!currentKeys.has(key)) appliedRef.current.delete(key) + } for (const fieldError of fieldErrors) { + const key = fieldErrorKey(fieldError) + if (appliedRef.current.has(key)) continue const normalizedField = normalizeErrorKeyForForm(fieldError.field) if (knownFields.has(normalizedField)) { setError(normalizedField as FieldPath, { type: 'custom', message: fieldError.message, }) + appliedRef.current.add(key) } } }, [fieldErrors, setError, fieldsMetadata]) From 12b117206152a9b38935d10fffc0fe5824944883 Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Fri, 26 Jun 2026 12:52:13 -0700 Subject: [PATCH 3/4] refactor(SDK-923): lift clear-on-edit out of useField into SDKFormProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useField is the generic SDK input hook and shouldn't carry form-error policy. Move the type:'custom' clear into useSyncFieldErrors using RHF's per-field `subscribe` API (RHF 7.55+), so we listen only to the fields that actually carry a server error rather than watching the whole form. Switch applied-tracking from a content-string Set to a WeakSet keyed by SDKFieldError identity so an identical-message re-submission still re-applies the error (new SDKError from composeSubmitHandler → new field-error refs). Drop the matching useField unit tests; the SDKFormProvider permutation suite covers it. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Common/Fields/hooks/useField.test.tsx | 73 ------------------- .../Common/Fields/hooks/useField.ts | 6 -- .../SDKFormProvider.errorClearing.test.tsx | 7 +- .../form/SDKFormProvider.tsx | 63 ++++++++++------ 4 files changed, 46 insertions(+), 103 deletions(-) diff --git a/src/components/Common/Fields/hooks/useField.test.tsx b/src/components/Common/Fields/hooks/useField.test.tsx index 8ca8617f1..240db9e70 100644 --- a/src/components/Common/Fields/hooks/useField.test.tsx +++ b/src/components/Common/Fields/hooks/useField.test.tsx @@ -145,79 +145,6 @@ describe('useField', () => { expect(result.current.value).toBe('some-test-value') }) - describe('clearing server-side errors on edit (SDK-923)', () => { - function renderClearOnEditHarness() { - let methods!: ReturnType - let field!: ReturnType> - function Probe({ onField }: { onField: (f: ReturnType>) => void }) { - const f = useField({ name: 'testField' }) - React.useEffect(() => { - onField(f) - }) - return null - } - const Wrapper = () => { - const m = useForm({ mode: 'onTouched' }) - methods = m - return ( - - (field = f)} /> - - ) - } - render() - return { - getMethods: () => methods, - getField: () => field, - } - } - - test('clears a custom (server-side) error on the field when the user edits it', async () => { - const { getMethods, getField } = renderClearOnEditHarness() - - act(() => { - getMethods().setError('testField', { - type: 'custom', - message: 'Server says no', - }) - }) - - await waitFor(() => { - expect(getMethods().getFieldState('testField').error).toMatchObject({ - type: 'custom', - }) - }) - - act(() => { - getField().onChange('user edit') - }) - - await waitFor(() => { - expect(getMethods().getFieldState('testField').error).toBeUndefined() - }) - }) - - test('does not clear non-custom (client-side) validation errors on edit', async () => { - const { getMethods, getField } = renderClearOnEditHarness() - - act(() => { - getMethods().setError('testField', { type: 'required', message: 'Required' }) - }) - - await waitFor(() => { - expect(getMethods().getFieldState('testField').error?.type).toBe('required') - }) - - act(() => { - getField().onChange('user edit') - }) - - // RHF's normal validation loop owns client-side errors — our clear-on-edit - // logic must be scoped to `type: 'custom'`. - expect(getMethods().getFieldState('testField').error?.type).toBe('required') - }) - }) - describe('description processing', () => { test('should return non-string descriptions as-is', () => { const jsxElement = JSX element diff --git a/src/components/Common/Fields/hooks/useField.ts b/src/components/Common/Fields/hooks/useField.ts index 00f81a81f..fa091bed6 100644 --- a/src/components/Common/Fields/hooks/useField.ts +++ b/src/components/Common/Fields/hooks/useField.ts @@ -81,12 +81,6 @@ export function useField({ const handleChange = (updatedValue: TValue) => { const value = transform ? transform(updatedValue) : updatedValue - // Server-side validation errors (mirrored by SDKFormProvider as - // `type: 'custom'`) must clear when the user edits the field; otherwise - // the stale error blocks resubmission until refresh (SDK-923). - if (fieldState.error?.type === 'custom') { - formContext?.clearErrors(name) - } field.onChange(value) onChange?.(value) } diff --git a/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx b/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx index 62485908f..9b8998a7d 100644 --- a/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx +++ b/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx @@ -187,7 +187,12 @@ describe('SDKFormProvider error clearing — permutations', () => { return ( <> - diff --git a/src/partner-hook-utils/form/SDKFormProvider.tsx b/src/partner-hook-utils/form/SDKFormProvider.tsx index d31bf407a..e2dd27f89 100644 --- a/src/partner-hook-utils/form/SDKFormProvider.tsx +++ b/src/partner-hook-utils/form/SDKFormProvider.tsx @@ -7,10 +7,6 @@ import { FieldElementRegistryProvider } from '@/components/Common/Fields/hooks/F import { normalizeErrorKeyForForm } from '@/helpers/formattedStrings' import type { SDKError, SDKFieldError } from '@/types/sdkError' -function fieldErrorKey(fe: SDKFieldError): string { - return `${fe.field}::${fe.message}` -} - function useSyncFieldErrors< TFormData extends FieldValues, TFieldsMetadata extends { @@ -24,38 +20,59 @@ function useSyncFieldErrors< }, ) { const { fieldsMetadata } = form - const { setError } = form.hookFormInternals.formMethods - // Track which (field, message) tuples have already been pushed into RHF state. - // Without this, the effect re-applies every server error on every render — - // overwriting `useField.handleChange`'s clearErrors and trapping users in a - // stale error state until the next submit (SDK-923). - const appliedRef = useRef>(new Set()) + const { setError, clearErrors, subscribe } = form.hookFormInternals.formMethods + // Track SDKFieldError objects we've already pushed into RHF state by reference + // identity. Without this, the effect re-applies on every render and undoes + // the clear-on-edit subscription below — trapping users in a stale error + // state until the next submit (SDK-923). A new submit produces new + // SDKFieldError refs (via composeSubmitHandler's setSubmitError), so even an + // identical-message re-submission re-applies correctly. + const consumedRef = useRef>(new WeakSet()) useEffect(() => { - if (fieldErrors.length === 0) { - appliedRef.current.clear() - return - } + if (fieldErrors.length === 0) return const knownFields = new Set(Object.keys(fieldsMetadata)) - const currentKeys = new Set(fieldErrors.map(fieldErrorKey)) - // Drop tracking for tuples that disappeared from the latest response so - // they can be re-applied later if the same error comes back. - for (const key of appliedRef.current) { - if (!currentKeys.has(key)) appliedRef.current.delete(key) - } for (const fieldError of fieldErrors) { - const key = fieldErrorKey(fieldError) - if (appliedRef.current.has(key)) continue + if (consumedRef.current.has(fieldError)) continue const normalizedField = normalizeErrorKeyForForm(fieldError.field) if (knownFields.has(normalizedField)) { setError(normalizedField as FieldPath, { type: 'custom', message: fieldError.message, }) - appliedRef.current.add(key) + consumedRef.current.add(fieldError) } } }, [fieldErrors, setError, fieldsMetadata]) + + // Subscribe ONLY to the fields that currently carry a server error and clear + // the custom error the moment the user edits one of them — otherwise the + // stale error keeps re-blocking handleSubmit (SDK-923). Per-field subscribe + // (RHF 7.55+) is genuinely event-based: no global watch over the form. + const knownFields = new Set(Object.keys(fieldsMetadata)) + const erroredFieldNames = Array.from( + new Set( + fieldErrors + .map(fe => normalizeErrorKeyForForm(fe.field)) + .filter(name => knownFields.has(name)), + ), + ) as Array> + const erroredKey = erroredFieldNames.join('|') + + useEffect(() => { + if (erroredFieldNames.length === 0) return + const unsubscribe = subscribe({ + name: erroredFieldNames, + formState: { values: true }, + callback: ({ name }) => { + if (!name) return + clearErrors(name as FieldPath) + }, + }) + return unsubscribe + // erroredKey re-runs the effect only when the set of errored fields changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [erroredKey, subscribe, clearErrors]) } interface SDKFormProviderProps< From 60b9113db32468d8a12b21020080855e5eb2e263 Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Fri, 26 Jun 2026 13:12:52 -0700 Subject: [PATCH 4/4] refactor(SDK-923): scope clear-on-edit to PaySchedule date fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the SDKFormProvider changes and the generic useField changes — the fix belongs in the PaySchedule form, not in shared infrastructure. Use RHF's per-field `subscribe` (RHF 7.55+) inside `usePayScheduleForm` against ONLY `anchorPayDate` and `anchorEndOfPayPeriod` so we listen exclusively to the fields that can carry a server validation error. On edit of one of those fields with a `type: 'custom'` error: clear the field error AND call `setSubmitError(null)` to drop the upstream source, which prevents `SDKFormProvider`'s sync effect from re-applying the error on the next render. Restore the PaySchedule-local SDK-923 tests (plus negative cases: client-side errors and non-date fields are unaffected). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../usePayScheduleForm.test.tsx | 124 ++++++++++ .../usePayScheduleForm/usePayScheduleForm.tsx | 21 ++ .../SDKFormProvider.errorClearing.test.tsx | 224 ------------------ .../form/SDKFormProvider.tsx | 44 +--- 4 files changed, 148 insertions(+), 265 deletions(-) delete mode 100644 src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx index 171ec2ec7..074b2a3f8 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.test.tsx @@ -351,6 +351,130 @@ describe('usePayScheduleForm', () => { expect(submitResult).toEqual(expect.objectContaining({ mode: 'update' })) }) }) + + describe('clearing server-side errors on edit (SDK-923)', () => { + it('clears a custom (server-side) error on a date field when the user edits it', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('anchorPayDate', { + type: 'custom', + message: 'Pay date must be after start date', + }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error).toMatchObject({ + type: 'custom', + }) + }) + + act(() => { + formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error).toBeUndefined() + }) + }) + + it('clears a custom error on the anchorEndOfPayPeriod date field when the user edits it', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('anchorEndOfPayPeriod', { + type: 'custom', + message: 'End of pay period invalid', + }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorEndOfPayPeriod').error).toMatchObject({ + type: 'custom', + }) + }) + + act(() => { + formMethods.setValue('anchorEndOfPayPeriod', '2026-07-22', { shouldDirty: true }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorEndOfPayPeriod').error).toBeUndefined() + }) + }) + + it('does not clear non-custom (client-side) validation errors on edit', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('anchorPayDate', { type: 'required', message: 'Required' }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') + }) + + act(() => { + formMethods.setValue('anchorPayDate', '2026-07-15', { shouldDirty: true }) + }) + + // Subscribe handler is scoped to type: 'custom' — RHF's normal validation + // loop owns client-side errors. + expect(formMethods.getFieldState('anchorPayDate').error?.type).toBe('required') + }) + + it('does not clear errors on non-date fields when they change', async () => { + const { result } = renderHook(() => usePayScheduleForm({ companyId: 'company-1' }), { + wrapper: GustoTestProvider, + }) + + await waitFor(() => { + expect(result.current.isLoading).toBe(false) + }) + assertReady(result.current) + const { formMethods } = result.current.form.hookFormInternals + + act(() => { + formMethods.setError('customName', { type: 'custom', message: 'Name taken' }) + }) + + await waitFor(() => { + expect(formMethods.getFieldState('customName').error?.type).toBe('custom') + }) + + act(() => { + formMethods.setValue('customName', 'Updated', { shouldDirty: true }) + }) + + // Subscribe is scoped to the two date fields; editing customName must + // not clear its custom error. + expect(formMethods.getFieldState('customName').error?.type).toBe('custom') + }) + }) }) // ── Schema-level tests ────────────────────────────────────────────────── diff --git a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx index 05c8b4e57..80d96ffb9 100644 --- a/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx +++ b/src/components/Company/PaySchedule/shared/usePayScheduleForm/usePayScheduleForm.tsx @@ -346,6 +346,27 @@ export function usePayScheduleForm({ const queries = payScheduleId ? [payScheduleQuery, paymentConfigsQuery] : [paymentConfigsQuery] const errorHandling = composeErrorHandler(queries, { submitError, setSubmitError }) + // SDK-923: server-side validation errors (mirrored by SDKFormProvider as + // `type: 'custom'`) on the date fields block resubmission until refresh. + // Subscribe ONLY to the two date fields that can receive server errors and + // clear them on edit. Clearing the upstream submitError too stops the + // provider's sync effect from re-applying the error on the next render. + useEffect(() => { + const dateFields = ['anchorPayDate', 'anchorEndOfPayPeriod'] as const + const unsubscribe = formMethods.subscribe({ + name: dateFields, + formState: { values: true }, + callback: ({ name }) => { + if (!name || !(dateFields as readonly string[]).includes(name)) return + const fieldName = name as (typeof dateFields)[number] + if (formMethods.getFieldState(fieldName).error?.type !== 'custom') return + formMethods.clearErrors(fieldName) + setSubmitError(null) + }, + }) + return unsubscribe + }, [formMethods, setSubmitError]) + const showCustomTwicePerMonth = watchedFrequency === 'Twice per month' const showDay1 = watchedFrequency === 'Monthly' || diff --git a/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx b/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx deleted file mode 100644 index 9b8998a7d..000000000 --- a/src/partner-hook-utils/form/SDKFormProvider.errorClearing.test.tsx +++ /dev/null @@ -1,224 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import { screen, act } from '@testing-library/react' -import { userEvent } from '@testing-library/user-event' -import { useForm } from 'react-hook-form' -import { zodResolver } from '@hookform/resolvers/zod' -import { z } from 'zod' -import { useState } from 'react' -import type { FieldMetadata } from '../types' -import { SDKFormProvider } from './SDKFormProvider' -import { useHookFormInternals } from './useHookFormInternals' -import { TextInputField } from '@/components/Common/Fields/TextInputField/TextInputField' -import { renderWithProviders } from '@/test-utils/renderWithProviders' -import type { SDKError, SDKFieldError } from '@/types/sdkError' - -type Values = { name: string } - -const schema = z.object({ - name: z.string().refine(v => v.length > 0, { message: 'REQUIRED' }), -}) - -const metadata: Record = { - name: { isRequired: true, isDisabled: false, hasRedactedValue: false } as FieldMetadata, -} - -function Harness({ initialFieldErrors = [] }: { initialFieldErrors?: SDKFieldError[] }) { - const formMethods = useForm({ - mode: 'onSubmit', - resolver: zodResolver(schema), - defaultValues: { name: 'original' }, - }) - - const hookFormInternals = useHookFormInternals(formMethods) - const [fieldErrors] = useState(initialFieldErrors) - - const sdkErrors: SDKError[] = fieldErrors.length - ? [ - { - category: 'api_error', - message: 'fields have issues', - httpStatus: 422, - fieldErrors, - }, - ] - : [] - - const formHookResult = { - errorHandling: { errors: sdkErrors, retryQueries: vi.fn(), clearSubmitError: vi.fn() }, - form: { - fieldsMetadata: metadata, - hookFormInternals, - }, - } - - return ( - - - - - ) -} - -describe('SDKFormProvider error clearing — permutations', () => { - describe('client-side (zod) errors', () => { - it('shows required error after submit with empty value', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await user.clear(input) - }) - await act(async () => { - await user.click(screen.getByRole('button', { name: 'submit' })) - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - }) - - it('clears client error when user types a valid value', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await user.clear(input) - }) - await act(async () => { - await user.click(screen.getByRole('button', { name: 'submit' })) - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - await act(async () => { - await user.type(input, 'x') - }) - expect(input).toHaveAttribute('aria-invalid', 'false') - }) - - it('re-applies client error when user reverts to invalid empty', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await user.clear(input) - }) - await act(async () => { - await user.click(screen.getByRole('button', { name: 'submit' })) - }) - await act(async () => { - await user.type(input, 'x') - }) - await act(async () => { - await user.clear(input) - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - }) - }) - - describe('server-side (SDKError fieldErrors) errors', () => { - const fieldError: SDKFieldError = { - field: 'name', - category: 'invalid_attribute_value', - message: 'Name is taken', - } - - it('surfaces server fieldError onto the field', async () => { - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - // useSyncFieldErrors runs in an effect, so wait a tick - await act(async () => { - await Promise.resolve() - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - expect(screen.getByText('Name is taken')).toBeInTheDocument() - }) - - it('clears server error when user types a different value', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await Promise.resolve() - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - await act(async () => { - await user.type(input, 'X') - }) - expect(input).toHaveAttribute('aria-invalid', 'false') - expect(screen.queryByText('Name is taken')).not.toBeInTheDocument() - }) - - it('clears server error when user clears the field', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await Promise.resolve() - }) - await act(async () => { - await user.clear(input) - }) - expect(input).toHaveAttribute('aria-invalid', 'false') - }) - - it('clears server error when user types the same value back', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await Promise.resolve() - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - await act(async () => { - await user.clear(input) - await user.type(input, 'original') - }) - expect(input).toHaveAttribute('aria-invalid', 'false') - }) - }) - - describe('server error persistence across re-renders', () => { - const fieldError: SDKFieldError = { - field: 'name', - category: 'invalid_attribute_value', - message: 'Name is taken', - } - - function Outer() { - const [, setTick] = useState(0) - return ( - <> - - - - ) - } - - it('keeps server error cleared after outer re-render', async () => { - const user = userEvent.setup() - renderWithProviders() - const input = screen.getByLabelText(/Name/) as HTMLInputElement - await act(async () => { - await Promise.resolve() - }) - expect(input).toHaveAttribute('aria-invalid', 'true') - - await act(async () => { - await user.type(input, 'X') - }) - expect(input).toHaveAttribute('aria-invalid', 'false') - - await act(async () => { - await user.click(screen.getByRole('button', { name: /rerender outer/ })) - }) - // If useSyncFieldErrors re-applies the server error on every render - // (because fieldsMetadata reference changes), this will fail. - expect(input).toHaveAttribute('aria-invalid', 'false') - }) - }) -}) diff --git a/src/partner-hook-utils/form/SDKFormProvider.tsx b/src/partner-hook-utils/form/SDKFormProvider.tsx index e2dd27f89..87d711483 100644 --- a/src/partner-hook-utils/form/SDKFormProvider.tsx +++ b/src/partner-hook-utils/form/SDKFormProvider.tsx @@ -1,4 +1,4 @@ -import { type ReactNode, useEffect, useRef } from 'react' +import { type ReactNode, useEffect } from 'react' import type { FieldPath, FieldValues } from 'react-hook-form' import { FormProvider } from 'react-hook-form' import type { FieldMetadata, FieldMetadataWithOptions, HookFormInternals } from '../types' @@ -20,59 +20,21 @@ function useSyncFieldErrors< }, ) { const { fieldsMetadata } = form - const { setError, clearErrors, subscribe } = form.hookFormInternals.formMethods - // Track SDKFieldError objects we've already pushed into RHF state by reference - // identity. Without this, the effect re-applies on every render and undoes - // the clear-on-edit subscription below — trapping users in a stale error - // state until the next submit (SDK-923). A new submit produces new - // SDKFieldError refs (via composeSubmitHandler's setSubmitError), so even an - // identical-message re-submission re-applies correctly. - const consumedRef = useRef>(new WeakSet()) + const { setError } = form.hookFormInternals.formMethods useEffect(() => { - if (fieldErrors.length === 0) return + if (!fieldErrors.length) return const knownFields = new Set(Object.keys(fieldsMetadata)) for (const fieldError of fieldErrors) { - if (consumedRef.current.has(fieldError)) continue const normalizedField = normalizeErrorKeyForForm(fieldError.field) if (knownFields.has(normalizedField)) { setError(normalizedField as FieldPath, { type: 'custom', message: fieldError.message, }) - consumedRef.current.add(fieldError) } } }, [fieldErrors, setError, fieldsMetadata]) - - // Subscribe ONLY to the fields that currently carry a server error and clear - // the custom error the moment the user edits one of them — otherwise the - // stale error keeps re-blocking handleSubmit (SDK-923). Per-field subscribe - // (RHF 7.55+) is genuinely event-based: no global watch over the form. - const knownFields = new Set(Object.keys(fieldsMetadata)) - const erroredFieldNames = Array.from( - new Set( - fieldErrors - .map(fe => normalizeErrorKeyForForm(fe.field)) - .filter(name => knownFields.has(name)), - ), - ) as Array> - const erroredKey = erroredFieldNames.join('|') - - useEffect(() => { - if (erroredFieldNames.length === 0) return - const unsubscribe = subscribe({ - name: erroredFieldNames, - formState: { values: true }, - callback: ({ name }) => { - if (!name) return - clearErrors(name as FieldPath) - }, - }) - return unsubscribe - // erroredKey re-runs the effect only when the set of errored fields changes. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [erroredKey, subscribe, clearErrors]) } interface SDKFormProviderProps<