From abecc837c8cb215e7416c657b632b170a85f03bb Mon Sep 17 00:00:00 2001 From: wjames111 Date: Thu, 21 May 2026 14:54:38 -0400 Subject: [PATCH] Scope PDS autosave disable to per-field saves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Form Type, Employment Status, and Pay Type selects briefly flickered disabled whenever any other field (e.g. Benefits) was being saved, because they were gated on the global isMutating flag. Track saving state per field via trackFieldMutation so a select only disables while its own save is in flight — preserving the same-field race protection without affecting unrelated fields. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PdsGoalCalculator/Setup/SetupStep.tsx | 4 +- .../Autosave/usePdsGoalAutoSave.test.tsx | 34 ++++++++++++ .../Shared/Autosave/usePdsGoalAutoSave.ts | 15 +++--- .../Shared/Autosave/useSaveField.ts | 13 +++-- .../Shared/PdsGoalCalculatorContext.tsx | 53 +++++++++++++++++++ 5 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/components/HrTools/PdsGoalCalculator/Setup/SetupStep.tsx b/src/components/HrTools/PdsGoalCalculator/Setup/SetupStep.tsx index a0ac1a2fba..c0c0cf4206 100644 --- a/src/components/HrTools/PdsGoalCalculator/Setup/SetupStep.tsx +++ b/src/components/HrTools/PdsGoalCalculator/Setup/SetupStep.tsx @@ -38,7 +38,7 @@ import { HoursPerWeekGrid } from './HoursPerWeekGrid/HoursPerWeekGrid'; export const SetupStep: React.FC = () => { const { t } = useTranslation(); const theme = useTheme(); - const { calculation, hcmUser, isMutating, setRightPanelContent } = + const { calculation, hcmUser, isFieldSaving, setRightPanelContent } = usePdsGoalCalculator(); const { data: userData } = useGetUserQuery(); const schema = useMemo( @@ -214,7 +214,7 @@ export const SetupStep: React.FC = () => { label={t('Pay Type')} helperText={t('Changing this clears Pay Rate.')} value={calculation?.salaryOrHourly ?? ''} - disabled={!calculation || isMutating} + disabled={!calculation || isFieldSaving('salaryOrHourly')} onChange={(event) => { const newValue = event.target .value as DesignationSupportSalaryType; diff --git a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.test.tsx b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.test.tsx index 7528c89522..6f42139aac 100644 --- a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.test.tsx +++ b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.test.tsx @@ -167,4 +167,38 @@ describe('usePdsGoalAutoSave', () => { expect(mutationSpy).toHaveGraphqlOperation('UpdatePdsGoalCalculation'), ); }); + + it('does not disable a saveOnChange field while an unrelated field is saving', async () => { + const { result } = renderHook( + () => ({ + formType: usePdsGoalAutoSave({ + fieldName: 'formType', + schema, + saveOnChange: true, + }), + name: usePdsGoalAutoSave({ fieldName: 'name', schema }), + }), + { wrapper: Wrapper }, + ); + + await waitFor(() => expect(result.current.name.value).toBe('Test Goal')); + expect(result.current.formType.disabled).toBe(false); + + // Kick off a save for the `name` field (blur-driven). + act(() => { + result.current.name.onChange({ + target: { value: 'Updated Goal' }, + } as React.ChangeEvent); + }); + act(() => { + result.current.name.onBlur(); + }); + + // The unrelated formType select must NOT flicker disabled while `name` + // is saving — only same-field saves should disable a select. + await waitFor(() => + expect(mutationSpy).toHaveGraphqlOperation('UpdatePdsGoalCalculation'), + ); + expect(result.current.formType.disabled).toBe(false); + }); }); diff --git a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.ts b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.ts index deaf5b0aae..9277016ef7 100644 --- a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.ts +++ b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/usePdsGoalAutoSave.ts @@ -15,17 +15,20 @@ export const usePdsGoalAutoSave = ({ ...options }: UsePdsAutoSaveOptions) => { const saveField = useSaveField(); - const { calculation, isMutating } = usePdsGoalCalculator(); + const { calculation, isFieldSaving } = usePdsGoalCalculator(); return useAutoSave({ value: calculation?.[fieldName] as string | number | null | undefined, saveValue: (value) => saveField({ [fieldName]: value }), fieldName, ...options, - // Block change-driven (select) autosaves while a save is in flight: rapid - // back-and-forth toggles can otherwise land out of order in the Apollo - // cache. formType is the load-bearing case — its value reshapes the goal - // calculation, so a stale final value silently understates the total. - disabled: !calculation || (options.saveOnChange === true && isMutating), + // Block change-driven (select) autosaves while a save for this same field + // is in flight: rapid back-and-forth toggles can otherwise land out of + // order in the Apollo cache. formType is the load-bearing case — its + // value reshapes the goal calculation, so a stale final value silently + // understates the total. Other fields' saves do not affect this one. + disabled: + !calculation || + (options.saveOnChange === true && isFieldSaving(fieldName)), }); }; diff --git a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/useSaveField.ts b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/useSaveField.ts index b8d839df84..5d1cfee90b 100644 --- a/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/useSaveField.ts +++ b/src/components/HrTools/PdsGoalCalculator/Shared/Autosave/useSaveField.ts @@ -6,7 +6,7 @@ import { useUpdatePdsGoalCalculationMutation } from '../../GoalsList/PdsGoalCalc import { usePdsGoalCalculator } from '../PdsGoalCalculatorContext'; export const useSaveField = () => { - const { calculation, trackMutation } = usePdsGoalCalculator(); + const { calculation, trackFieldMutation } = usePdsGoalCalculator(); const [updatePdsGoalCalculation] = useUpdatePdsGoalCalculationMutation(); const { enqueueSnackbar } = useSnackbar(); const { t } = useTranslation(); @@ -25,7 +25,7 @@ export const useSaveField = () => { } try { - return await trackMutation( + return await trackFieldMutation( updatePdsGoalCalculation({ variables: { attributes: { @@ -45,6 +45,7 @@ export const useSaveField = () => { }, }, }), + Object.keys(attributes), ); } catch { enqueueSnackbar(t('Failed to save changes. Please try again.'), { @@ -52,7 +53,13 @@ export const useSaveField = () => { }); } }, - [calculation, trackMutation, updatePdsGoalCalculation, enqueueSnackbar, t], + [ + calculation, + trackFieldMutation, + updatePdsGoalCalculation, + enqueueSnackbar, + t, + ], ); return saveField; diff --git a/src/components/HrTools/PdsGoalCalculator/Shared/PdsGoalCalculatorContext.tsx b/src/components/HrTools/PdsGoalCalculator/Shared/PdsGoalCalculatorContext.tsx index f2442c6947..0f995fa457 100644 --- a/src/components/HrTools/PdsGoalCalculator/Shared/PdsGoalCalculatorContext.tsx +++ b/src/components/HrTools/PdsGoalCalculator/Shared/PdsGoalCalculatorContext.tsx @@ -39,8 +39,19 @@ export type PdsGoalCalculatorType = { /** Whether any mutations are currently in progress */ isMutating: boolean; + /** Whether a save mutation tagged with the given field name is in flight */ + isFieldSaving: (fieldName: string) => boolean; /** Call with the mutation promise to track the start and end of mutations */ trackMutation: (mutation: Promise) => Promise; + /** + * Like trackMutation, but also marks the listed fields as saving while the + * mutation is in flight so per-field disable checks can scope to the field + * being saved instead of any save in the calculator. + */ + trackFieldMutation: ( + mutation: Promise, + fields: string[], + ) => Promise; rightPanelContent: React.ReactNode; setRightPanelContent: (content: React.ReactNode) => void; @@ -108,6 +119,44 @@ export const PdsGoalCalculatorProvider: React.FC = ({ children }) => { const [isDrawerOpen, setIsDrawerOpen] = useState(true); const { trackMutation, isMutating } = useTrackMutation(); + const [savingFieldCounts, setSavingFieldCounts] = useState< + Record + >({}); + + const isFieldSaving = useCallback( + (fieldName: string) => (savingFieldCounts[fieldName] ?? 0) > 0, + [savingFieldCounts], + ); + + const trackFieldMutation = useCallback( + (mutation: Promise, fields: string[]): Promise => { + setSavingFieldCounts((prev) => { + const next = { ...prev }; + for (const field of fields) { + next[field] = (next[field] ?? 0) + 1; + } + return next; + }); + return trackMutation( + mutation.finally(() => { + setSavingFieldCounts((prev) => { + const next = { ...prev }; + for (const field of fields) { + const remaining = (next[field] ?? 0) - 1; + if (remaining <= 0) { + delete next[field]; + } else { + next[field] = remaining; + } + } + return next; + }); + }), + ); + }, + [trackMutation], + ); + useEffect(() => { if (steps.some((s) => s.step === activeStep)) { return; @@ -173,7 +222,9 @@ export const PdsGoalCalculatorProvider: React.FC = ({ children }) => { summaryData, percentComplete, isMutating, + isFieldSaving, trackMutation, + trackFieldMutation, hcmUser, rightPanelContent, isDrawerOpen, @@ -194,7 +245,9 @@ export const PdsGoalCalculatorProvider: React.FC = ({ children }) => { summaryData, percentComplete, isMutating, + isFieldSaving, trackMutation, + trackFieldMutation, hcmUser, rightPanelContent, isDrawerOpen,