From dc21016151389efc502b951e1f8a27405bf993c9 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Mon, 19 May 2025 23:20:40 +0100 Subject: [PATCH] fix: get presets working correctly with dynamic params (#17923) This adds a few fixes to get presets working correctly with dynamic params 1. Changes to preset params need to be rendered and displayed correctly 2. Changes to preset params need to be sent to the websocket 3. Changes to preset params need to be marked as touched so they won't be automatically changed later because of dynamic defaults. Dynamic defaults means any default parameter value can be changed by the websocket response unless edited by the user, set by autofill or set by a preset. --- .../DynamicParameter/DynamicParameter.tsx | 11 ++- .../CreateWorkspacePageExperimental.tsx | 2 +- .../CreateWorkspacePageViewExperimental.tsx | 73 ++++++++++++++++--- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index cbc7852bd1..94fa3bc383 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -222,6 +222,15 @@ const DebouncedParameterField: FC = ({ const onChangeEvent = useEffectEvent(onChange); // prevDebouncedValueRef is to prevent calling the onChangeEvent on the initial render const prevDebouncedValueRef = useRef(); + const prevValueRef = useRef(value); + + // This is necessary in the case of fields being set by preset parameters + useEffect(() => { + if (value !== undefined && value !== prevValueRef.current) { + setLocalValue(value); + prevValueRef.current = value; + } + }, [value]); useEffect(() => { if (prevDebouncedValueRef.current !== undefined) { @@ -458,7 +467,7 @@ const ParameterField: FC = ({ { onChange(value.toString()); }} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index 8268ded111..fbb35c61ee 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -101,7 +101,7 @@ const CreateWorkspacePageExperimental: FC = () => { } }, []); - // On sends all initial parameter values to the websocket + // On page load, sends all initial parameter values to the websocket // (including defaults and autofilled from the url) // This ensures the backend has the complete initial state of the form, // which is vital for correctly rendering dynamic UI elements where parameter visibility diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 434cd23fb9..630faf8e80 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -213,6 +213,15 @@ export const CreateWorkspacePageViewExperimental: FC< setPresetParameterNames(selectedPreset.Parameters.map((p) => p.Name)); + const currentValues = form.values.rich_parameter_values ?? []; + + const updates: Array<{ + field: string; + fieldValue: TypesGen.WorkspaceBuildParameter; + parameter: PreviewParameter; + presetValue: string; + }> = []; + for (const presetParameter of selectedPreset.Parameters) { const parameterIndex = parameters.findIndex( (p) => p.name === presetParameter.Name, @@ -220,32 +229,64 @@ export const CreateWorkspacePageViewExperimental: FC< if (parameterIndex === -1) continue; const parameterField = `rich_parameter_values.${parameterIndex}`; + const parameter = parameters[parameterIndex]; + const currentValue = currentValues.find( + (p) => p.name === presetParameter.Name, + )?.value; - form.setFieldValue(parameterField, { - name: presetParameter.Name, - value: presetParameter.Value, - }); + if (currentValue !== presetParameter.Value) { + updates.push({ + field: parameterField, + fieldValue: { + name: presetParameter.Name, + value: presetParameter.Value, + }, + parameter, + presetValue: presetParameter.Value, + }); + } + } + + if (updates.length > 0) { + for (const update of updates) { + form.setFieldValue(update.field, update.fieldValue); + form.setFieldTouched(update.parameter.name, true); + } + + sendDynamicParamsRequest( + updates.map((update) => ({ + parameter: update.parameter, + value: update.presetValue, + })), + ); } }, [ presetOptions, selectedPresetIndex, presets, form.setFieldValue, + form.setFieldTouched, parameters, + form.values.rich_parameter_values, ]); // send the last user modified parameter and all touched parameters to the websocket const sendDynamicParamsRequest = ( - parameter: PreviewParameter, - value: string, + parameters: Array<{ parameter: PreviewParameter; value: string }>, ) => { const formInputs: Record = {}; - formInputs[parameter.name] = value; - const parameters = form.values.rich_parameter_values ?? []; + const formParameters = form.values.rich_parameter_values ?? []; + + for (const { parameter, value } of parameters) { + formInputs[parameter.name] = value; + } for (const [fieldName, isTouched] of Object.entries(form.touched)) { - if (isTouched && fieldName !== parameter.name) { - const param = parameters.find((p) => p.name === fieldName); + if ( + isTouched && + !parameters.some((p) => p.parameter.name === fieldName) + ) { + const param = formParameters.find((p) => p.name === fieldName); if (param?.value) { formInputs[fieldName] = param.value; } @@ -260,12 +301,20 @@ export const CreateWorkspacePageViewExperimental: FC< parameterField: string, value: string, ) => { + const currentFormValue = form.values.rich_parameter_values?.find( + (p) => p.name === parameter.name, + )?.value; + await form.setFieldValue(parameterField, { name: parameter.name, value, }); - form.setFieldTouched(parameter.name, true); - sendDynamicParamsRequest(parameter, value); + + // Only send the request if the value has changed from the form value + if (currentFormValue !== value) { + form.setFieldTouched(parameter.name, true); + sendDynamicParamsRequest([{ parameter, value }]); + } }; useSyncFormParameters({