From c6d310caf78f4fc02ab7b63ea3c1fbcd4f3064f1 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Thu, 18 Jul 2024 14:16:30 +1000
Subject: [PATCH] Fix loading sequence for PUI form fields (#7678)

- Ensure that "existing" values are honored
- Change hook execution ordering
- Delay field display until all fields are loaded
---
 src/frontend/src/components/forms/ApiForm.tsx | 53 ++++++++++++-------
 .../components/forms/fields/ApiFormField.tsx  |  4 +-
 2 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx
index 8cc11e915c..d7585d69f8 100644
--- a/src/frontend/src/components/forms/ApiForm.tsx
+++ b/src/frontend/src/components/forms/ApiForm.tsx
@@ -139,6 +139,7 @@ export function OptionsApiForm({
       if (!props.ignorePermissionCheck) {
         fields = extractAvailableFields(response, props.method);
       }
+
       return fields;
     },
     throwOnError: (error: any) => {
@@ -183,7 +184,7 @@ export function OptionsApiForm({
     <ApiForm
       id={id}
       props={formProps}
-      optionsLoading={optionsQuery.isFetching}
+      optionsLoading={optionsQuery.isFetching || !optionsQuery.data}
     />
   );
 }
@@ -206,9 +207,6 @@ export function ApiForm({
   const [fields, setFields] = useState<ApiFormFieldSet>(
     () => props.fields ?? {}
   );
-  useEffect(() => {
-    setFields(props.fields ?? {});
-  }, [props.fields]);
 
   const defaultValues: FieldValues = useMemo(() => {
     let defaultValuesMap = mapFields(fields ?? {}, (_path, field) => {
@@ -314,11 +312,24 @@ export function ApiForm({
       } catch (error) {
         console.error('ERR: Error fetching initial data:', error);
         // Re-throw error to allow react-query to handle error
-        throw error;
+        return {};
       }
     }
   });
 
+  useEffect(() => {
+    let _fields = props.fields ?? {};
+
+    // Ensure default values override initial field spec
+    for (const k of Object.keys(_fields)) {
+      if (defaultValues[k]) {
+        _fields[k].value = defaultValues[k];
+      }
+    }
+
+    setFields(_fields);
+  }, [props.fields, defaultValues, initialDataQuery.data]);
+
   // Fetch initial data on form load
   useEffect(() => {
     // Fetch initial data if the fetchInitialData property is set
@@ -559,21 +570,23 @@ export function ApiForm({
                 )}
               </Boundary>
               <Boundary label={`ApiForm-${id}-FormContent`}>
-                <FormProvider {...form}>
-                  <Stack gap="xs">
-                    {!optionsLoading &&
-                      Object.entries(fields).map(([fieldName, field]) => (
-                        <ApiFormField
-                          key={fieldName}
-                          fieldName={fieldName}
-                          definition={field}
-                          control={form.control}
-                          url={url}
-                          setFields={setFields}
-                        />
-                      ))}
-                  </Stack>
-                </FormProvider>
+                {!isLoading && (
+                  <FormProvider {...form}>
+                    <Stack gap="xs">
+                      {!optionsLoading &&
+                        Object.entries(fields).map(([fieldName, field]) => (
+                          <ApiFormField
+                            key={fieldName}
+                            fieldName={fieldName}
+                            definition={field}
+                            control={form.control}
+                            url={url}
+                            setFields={setFields}
+                          />
+                        ))}
+                    </Stack>
+                  </FormProvider>
+                )}
               </Boundary>
               <Boundary label={`ApiForm-${id}-PostFormContent`}>
                 {props.postFormContent}
diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx
index 5116790fc9..7a41f1fa74 100644
--- a/src/frontend/src/components/forms/fields/ApiFormField.tsx
+++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx
@@ -149,7 +149,7 @@ export function ApiFormField({
       label: hideLabels ? undefined : definition.label,
       description: hideLabels ? undefined : definition.description
     };
-  }, [definition]);
+  }, [hideLabels, definition]);
 
   // pull out onValueChange as this can cause strange errors when passing the
   // definition to the input components via spread syntax
@@ -202,7 +202,7 @@ export function ApiFormField({
     }
 
     return val;
-  }, [value]);
+  }, [definition.field_type, value]);
 
   // Coerce the value to a (stringified) boolean value
   const booleanValue: boolean = useMemo(() => {