Skip to content

Commit 2432aa6

Browse files
avivturcursoragent
andauthored
Fix stale duplicate validation for automation scripts (#2402)
The duplicate script validation used useWatch values inside the validate closure, which are stale within the same event handler (React state hasn't re-rendered yet). Replaced with getValues() which reads directly from react-hook-form's internal store. Also added deps to re-validate sibling scripts when any name changes, and trigger all script names on guestType/scriptType change so stale errors clear when the conflict resolves. Resolves: MTV-4945 Signed-off-by: Aviv Turgeman <aturgema@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a71d06a commit 2432aa6

3 files changed

Lines changed: 70 additions & 42 deletions

File tree

src/plans/create/steps/customization-scripts/NewScriptsFields.tsx

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { useForkliftTranslation } from '@utils/i18n';
1010

1111
import { useCreatePlanFormContext } from '../../hooks/useCreatePlanFormContext';
1212

13+
import { useScriptFieldValidation } from './hooks/useScriptFieldValidation';
1314
import {
1415
CustomScriptsFieldId,
1516
DefaultScript,
@@ -20,11 +21,10 @@ import {
2021
ScriptTypeLabels,
2122
} from './constants';
2223
import ScriptContentField from './ScriptContentField';
23-
import { validateUniqueScriptKey } from './utils';
2424

2525
const NewScriptsFields: FC = () => {
2626
const { t } = useForkliftTranslation();
27-
const { control, setValue, trigger } = useCreatePlanFormContext();
27+
const { control, getValues, setValue, trigger } = useCreatePlanFormContext();
2828

2929
const {
3030
append,
@@ -36,22 +36,13 @@ const NewScriptsFields: FC = () => {
3636
});
3737

3838
const watchedScripts = useWatch({ control, name: CustomScriptsFieldId.Scripts });
39-
4039
const getScriptFieldId = (index: number, field: string): string =>
4140
`${CustomScriptsFieldId.Scripts}.${index}.${field}`;
42-
43-
const handleAddScript = (): void => {
44-
append({ ...DefaultScript });
45-
};
46-
47-
const handleRemoveScript = (index: number): void => {
48-
if (scripts.length > 1) {
49-
remove(index);
50-
return;
51-
}
52-
53-
setValue(CustomScriptsFieldId.Scripts, [DefaultScript]);
54-
};
41+
const { nameDeps, triggerAllNames, validateName } = useScriptFieldValidation(
42+
CustomScriptsFieldId.Scripts,
43+
trigger,
44+
() => getValues(CustomScriptsFieldId.Scripts),
45+
);
5546

5647
return (
5748
<div className="pf-v6-u-ml-lg">
@@ -64,7 +55,6 @@ const NewScriptsFields: FC = () => {
6455
fieldRows={scripts.map((fieldRow, index) => {
6556
const guestType = watchedScripts?.[index]?.guestType ?? GuestType.Linux;
6657
const isWindows = guestType === GuestType.Windows;
67-
6858
return {
6959
...fieldRow,
7060
additionalOptions: (
@@ -85,14 +75,7 @@ const NewScriptsFields: FC = () => {
8575
key="name"
8676
name={getScriptFieldId(index, 'name')}
8777
control={control}
88-
rules={{
89-
validate: (value) =>
90-
validateUniqueScriptKey(
91-
{ ...watchedScripts?.[index], name: value },
92-
index,
93-
watchedScripts ?? [],
94-
),
95-
}}
78+
rules={{ deps: nameDeps(scripts.length), validate: validateName(index) }}
9679
render={({ field, fieldState: { error } }) => (
9780
<>
9881
<TextInput
@@ -115,12 +98,10 @@ const NewScriptsFields: FC = () => {
11598
value={GuestTypeLabels[guestTypeField.value as GuestType]}
11699
onSelect={async (_event, value) => {
117100
guestTypeField.onChange(value);
118-
119101
if (value === GuestType.Windows) {
120102
setValue(getScriptFieldId(index, 'scriptType'), ScriptType.Firstboot);
121103
}
122-
123-
await trigger(getScriptFieldId(index, 'name'));
104+
await triggerAllNames(scripts.length);
124105
}}
125106
testId={`script-guest-type-${index}`}
126107
>
@@ -144,7 +125,7 @@ const NewScriptsFields: FC = () => {
144125
value={ScriptTypeLabels[scriptTypeField.value as ScriptType]}
145126
onSelect={async (_event, value) => {
146127
scriptTypeField.onChange(value);
147-
await trigger(getScriptFieldId(index, 'name'));
128+
await triggerAllNames(scripts.length);
148129
}}
149130
testId={`script-type-${index}`}
150131
>
@@ -172,10 +153,18 @@ const NewScriptsFields: FC = () => {
172153
})}
173154
addButton={{
174155
label: t('Add script'),
175-
onClick: handleAddScript,
156+
onClick: () => {
157+
append({ ...DefaultScript });
158+
},
176159
}}
177160
removeButton={{
178-
onClick: handleRemoveScript,
161+
onClick: (index) => {
162+
if (scripts.length > 1) {
163+
remove(index);
164+
} else {
165+
setValue(CustomScriptsFieldId.Scripts, [DefaultScript]);
166+
}
167+
},
179168
}}
180169
/>
181170
</div>
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { CustomScript } from '../types';
2+
import { validateUniqueScriptKey } from '../utils';
3+
4+
type TriggerFn = (name?: string | string[]) => Promise<boolean>;
5+
type GetScriptsFn = () => CustomScript[];
6+
7+
type ScriptFieldValidation = {
8+
nameDeps: (fieldCount: number) => string[];
9+
triggerAllNames: (fieldCount: number) => Promise<boolean>;
10+
validateName: (index: number) => (value: string) => string | undefined;
11+
};
12+
13+
export const useScriptFieldValidation = (
14+
prefix: string,
15+
trigger: TriggerFn,
16+
getScripts: GetScriptsFn,
17+
): ScriptFieldValidation => {
18+
const buildNameField = (index: number): string => `${prefix}.${index}.name`;
19+
20+
const nameDeps = (fieldCount: number): string[] =>
21+
Array.from({ length: fieldCount }, (_, i) => buildNameField(i));
22+
23+
const triggerAllNames = async (fieldCount: number): Promise<boolean> =>
24+
trigger(nameDeps(fieldCount));
25+
26+
const validateName =
27+
(index: number) =>
28+
(value: string): string | undefined => {
29+
const allScripts = getScripts();
30+
return validateUniqueScriptKey({ ...allScripts[index], name: value }, index, allScripts);
31+
};
32+
33+
return { nameDeps, triggerAllNames, validateName };
34+
};

src/plans/details/tabs/Automation/components/ScriptEdit/ScriptEditTable.tsx

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import {
88
ScriptType,
99
ScriptTypeLabels,
1010
} from 'src/plans/create/steps/customization-scripts/constants';
11+
import { useScriptFieldValidation } from 'src/plans/create/steps/customization-scripts/hooks/useScriptFieldValidation';
1112
import ScriptContentField from 'src/plans/create/steps/customization-scripts/ScriptContentField';
1213
import type { CustomScript } from 'src/plans/create/steps/customization-scripts/types';
13-
import { validateUniqueScriptKey } from 'src/plans/create/steps/customization-scripts/utils';
1414

1515
import Select from '@components/common/Select';
1616
import FieldBuilderTable from '@components/FieldBuilderTable/FieldBuilderTable';
@@ -27,11 +27,20 @@ type ScriptEditTableProps = {
2727
remove: (index: number) => void;
2828
};
2929

30+
const SCRIPTS_FIELD = 'scripts';
31+
3032
const ScriptEditTable: FC<ScriptEditTableProps> = ({ append, fields, remove }) => {
3133
const { t } = useForkliftTranslation();
32-
const { control, setValue, trigger } = useFormContext<{ scripts: CustomScript[] }>();
34+
const { control, getValues, setValue, trigger } = useFormContext<{ scripts: CustomScript[] }>();
3335

34-
const watchedScripts = useWatch({ control, name: 'scripts' });
36+
const watchedScripts = useWatch({ control, name: SCRIPTS_FIELD });
37+
const triggerByName = async (name?: string | string[]): Promise<boolean> =>
38+
trigger(name as Parameters<typeof trigger>[0]);
39+
const { nameDeps, triggerAllNames, validateName } = useScriptFieldValidation(
40+
SCRIPTS_FIELD,
41+
triggerByName,
42+
() => getValues(SCRIPTS_FIELD),
43+
);
3544

3645
return (
3746
<FieldBuilderTable
@@ -62,12 +71,8 @@ const ScriptEditTable: FC<ScriptEditTableProps> = ({ append, fields, remove }) =
6271
control={control}
6372
name={`scripts.${index}.name`}
6473
rules={{
65-
validate: (value) =>
66-
validateUniqueScriptKey(
67-
{ ...watchedScripts?.[index], name: value },
68-
index,
69-
watchedScripts ?? [],
70-
),
74+
deps: nameDeps(fields.length) as `scripts.${number}.name`[],
75+
validate: validateName(index),
7176
}}
7277
render={({ field, fieldState: { error } }) => (
7378
<>
@@ -96,7 +101,7 @@ const ScriptEditTable: FC<ScriptEditTableProps> = ({ append, fields, remove }) =
96101
setValue(`scripts.${index}.scriptType`, ScriptType.Firstboot);
97102
}
98103

99-
await trigger(`scripts.${index}.name`);
104+
await triggerAllNames(fields.length);
100105
}}
101106
testId={`script-guest-type-select-${index}`}
102107
>
@@ -120,7 +125,7 @@ const ScriptEditTable: FC<ScriptEditTableProps> = ({ append, fields, remove }) =
120125
value={ScriptTypeLabels[scriptTypeField.value]}
121126
onSelect={async (_event, value) => {
122127
scriptTypeField.onChange(value);
123-
await trigger(`scripts.${index}.name`);
128+
await triggerAllNames(fields.length);
124129
}}
125130
testId={`script-type-select-${index}`}
126131
>

0 commit comments

Comments
 (0)