Refactor the Dag trigger form (#45220)

* reset form * use field * use field error * place error alert at down * set width * error alert for date * show toaster after invalidating * readonly props * fetch conf
apache · Dec 26, 2024 · d4e0995 · d4e0995
1 parent e37110a
commit d4e0995
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 103 deletions.
diff --git a/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx b/airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx
@@ -16,11 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { Input, Button, Box, Text, Spacer, HStack } from "@chakra-ui/react";
+import { Input, Button, Box, Spacer, HStack, Field } from "@chakra-ui/react";
 import { json } from "@codemirror/lang-json";
 import { githubLight, githubDark } from "@uiw/codemirror-themes-all";
 import CodeMirror from "@uiw/react-codemirror";
-import { useEffect, useMemo, useState } from "react";
+import { useEffect, useState } from "react";
 import { useForm, Controller } from "react-hook-form";
 import { FiPlay } from "react-icons/fi";
 
@@ -32,9 +32,9 @@ import { ErrorAlert } from "../ErrorAlert";
 import { Accordion } from "../ui";
 
 type TriggerDAGFormProps = {
-  dagId: string;
-  onClose: () => void;
-  open: boolean;
+  readonly dagId: string;
+  readonly onClose: () => void;
+  readonly open: boolean;
 };
 
 export type DagRunTriggerParams = {
@@ -45,41 +45,59 @@ export type DagRunTriggerParams = {
   note: string;
 };
 
-const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
-  dagId,
-  onClose,
-  open,
-}) => {
-  const [errors, setErrors] = useState<{ conf?: string; date?: string }>({});
+const TriggerDAGForm = ({ dagId, onClose, open }: TriggerDAGFormProps) => {
+  const [errors, setErrors] = useState<{ conf?: string; date?: unknown }>({});
   const conf = useDagParams(dagId, open);
   const { error: errorTrigger, isPending, triggerDagRun } = useTrigger(onClose);
 
-  const dagRunRequestBody: DagRunTriggerParams = useMemo(
-    () => ({
-      conf,
-      dagRunId: "",
-      dataIntervalEnd: "",
-      dataIntervalStart: "",
-      note: "",
-    }),
-    [conf],
-  );
-
   const {
     control,
     formState: { isDirty },
     handleSubmit,
     reset,
     setValue,
     watch,
-  } = useForm<DagRunTriggerParams>({ defaultValues: dagRunRequestBody });
+  } = useForm<DagRunTriggerParams>({
+    defaultValues: {
+      conf,
+      dagRunId: "",
+      dataIntervalEnd: "",
+      dataIntervalStart: "",
+      note: "",
+    },
+  });
+
+  // Automatically reset form when conf is fetched
+  useEffect(() => {
+    if (conf) {
+      reset({ conf });
+    }
+  }, [conf, reset]);
 
   const dataIntervalStart = watch("dataIntervalStart");
   const dataIntervalEnd = watch("dataIntervalEnd");
 
-  useEffect(() => {
-    reset(dagRunRequestBody);
-  }, [dagRunRequestBody, reset]);
+  const handleReset = () => {
+    setErrors({ conf: undefined, date: undefined });
+    reset();
+  };
+
+  const onSubmit = (data: DagRunTriggerParams) => {
+    if (Boolean(data.dataIntervalStart) !== Boolean(data.dataIntervalEnd)) {
+      setErrors((prev) => ({
+        ...prev,
+        date: {
+          body: {
+            detail:
+              "Either both Data Interval Start and End must be provided, or both must be empty.",
+          },
+        },
+      }));
+
+      return;
+    }
+    triggerDagRun(dagId, data);
+  };
 
   const validateAndPrettifyJson = (value: string) => {
     try {
@@ -101,18 +119,6 @@ const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
     }
   };
 
-  const onSubmit = (data: DagRunTriggerParams) => {
-    if (Boolean(data.dataIntervalStart) !== Boolean(data.dataIntervalEnd)) {
-      setErrors((prev) => ({
-        ...prev,
-        date: "Either both Data Interval Start and End must be provided, or both must be empty.",
-      }));
-
-      return;
-    }
-    triggerDagRun(dagId, data);
-  };
-
   const validateDates = (
     fieldName: "dataIntervalEnd" | "dataIntervalStart",
   ) => {
@@ -136,73 +142,74 @@ const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
 
   return (
     <>
-      <ErrorAlert error={errorTrigger} />
-      <Accordion.Root collapsible mt={4} size="lg" variant="enclosed">
+      <Accordion.Root collapsible mb={4} mt={4} size="lg" variant="enclosed">
         <Accordion.Item key="advancedOptions" value="advancedOptions">
           <Accordion.ItemTrigger cursor="button">
             Advanced Options
           </Accordion.ItemTrigger>
           <Accordion.ItemContent>
             <Box p={5}>
-              <Text fontSize="md" mb={2}>
-                Data Interval Start Date
-              </Text>
               <Controller
                 control={control}
                 name="dataIntervalStart"
                 render={({ field }) => (
-                  <Input
-                    {...field}
-                    max={dataIntervalEnd || undefined}
-                    onBlur={() => validateDates("dataIntervalStart")}
-                    placeholder="yyyy-mm-ddThh:mm"
-                    size="sm"
-                    type="datetime-local"
-                  />
+                  <Field.Root invalid={Boolean(errors.date)}>
+                    <Field.Label fontSize="md">
+                      Data Interval Start Date
+                    </Field.Label>
+                    <Input
+                      {...field}
+                      max={dataIntervalEnd || undefined}
+                      onBlur={() => validateDates("dataIntervalStart")}
+                      placeholder="yyyy-mm-ddThh:mm"
+                      size="sm"
+                      type="datetime-local"
+                    />
+                  </Field.Root>
                 )}
               />
 
-              <Text fontSize="md" mb={2} mt={6}>
-                Data Interval End Date
-              </Text>
               <Controller
                 control={control}
                 name="dataIntervalEnd"
                 render={({ field }) => (
-                  <Input
-                    {...field}
-                    min={dataIntervalStart || undefined}
-                    onBlur={() => validateDates("dataIntervalEnd")}
-                    placeholder="yyyy-mm-ddThh:mm"
-                    size="sm"
-                    type="datetime-local"
-                  />
+                  <Field.Root invalid={Boolean(errors.date)} mt={6}>
+                    <Field.Label fontSize="md">
+                      Data Interval End Date
+                    </Field.Label>
+                    <Input
+                      {...field}
+                      min={dataIntervalStart || undefined}
+                      onBlur={() => validateDates("dataIntervalEnd")}
+                      placeholder="yyyy-mm-ddThh:mm"
+                      size="sm"
+                      type="datetime-local"
+                    />
+                  </Field.Root>
                 )}
               />
 
-              <Text fontSize="md" mb={2} mt={6}>
-                Run ID
-              </Text>
               <Controller
                 control={control}
                 name="dagRunId"
                 render={({ field }) => (
-                  <Input
-                    {...field}
-                    placeholder="Run Id, optional - will be generated if not provided"
-                    size="sm"
-                  />
+                  <Field.Root mt={6}>
+                    <Field.Label fontSize="md">Run ID</Field.Label>
+                    <Input
+                      {...field}
+                      placeholder="Run Id, optional - will be generated if not provided"
+                      size="sm"
+                    />
+                  </Field.Root>
                 )}
               />
 
-              <Text fontSize="md" mb={2} mt={6}>
-                Configuration JSON
-              </Text>
               <Controller
                 control={control}
                 name="conf"
                 render={({ field }) => (
-                  <Box mb={4}>
+                  <Field.Root invalid={Boolean(errors.conf)} mt={6}>
+                    <Field.Label fontSize="md">Configuration JSON</Field.Label>
                     <CodeMirror
                       {...field}
                       basicSetup={{
@@ -214,52 +221,43 @@ const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
                       extensions={[json()]}
                       height="200px"
                       onBlur={() => {
-                        const prettifiedJson = validateAndPrettifyJson(
-                          field.value,
-                        );
-
-                        field.onChange(prettifiedJson);
+                        field.onChange(validateAndPrettifyJson(field.value));
                       }}
                       style={{
                         border: "1px solid #CBD5E0",
                         borderRadius: "8px",
                         outline: "none",
                         padding: "2px",
+                        width: "100%",
                       }}
                       theme={colorMode === "dark" ? githubDark : githubLight}
                     />
-                    {Boolean(errors.conf) && (
-                      <Text color="red.500" fontSize="sm" mt={2}>
-                        {errors.conf}
-                      </Text>
-                    )}
-                  </Box>
+                    {Boolean(errors.conf) ? (
+                      <Field.ErrorText>{errors.conf}</Field.ErrorText>
+                    ) : undefined}
+                  </Field.Root>
                 )}
               />
 
-              <Text fontSize="md" mb={2} mt={6}>
-                Dag Run Notes
-              </Text>
               <Controller
                 control={control}
                 name="note"
                 render={({ field }) => (
-                  <Input {...field} placeholder="Optional" size="sm" />
+                  <Field.Root mt={6}>
+                    <Field.Label fontSize="md">Dag Run Notes</Field.Label>
+                    <Input {...field} placeholder="Optional" size="sm" />
+                  </Field.Root>
                 )}
               />
             </Box>
           </Accordion.ItemContent>
         </Accordion.Item>
       </Accordion.Root>
-      {Boolean(errors.date) && (
-        <Text color="red.500" fontSize="sm" mt={2}>
-          {errors.date}
-        </Text>
-      )}
+      <ErrorAlert error={errors.date ?? errorTrigger} />
       <Box as="footer" display="flex" justifyContent="flex-end" mt={4}>
         <HStack w="full">
           {isDirty ? (
-            <Button onClick={() => reset()} variant="outline">
+            <Button onClick={handleReset} variant="outline">
               Reset
             </Button>
           ) : undefined}

diff --git a/airflow/ui/src/queries/useTrigger.ts b/airflow/ui/src/queries/useTrigger.ts
@@ -43,15 +43,14 @@ export const useTrigger = (onClose: () => void) => {
       queryKeys.map((key) =>
         queryClient.invalidateQueries({ queryKey: [key] }),
       ),
-    );
-
-    toaster.create({
-      description: "DAG run has been successfully triggered.",
-      title: "DAG Run Request Submitted",
-      type: "success",
+    ).then(() => {
+      toaster.create({
+        description: "DAG run has been successfully triggered.",
+        title: "DAG Run Request Submitted",
+        type: "success",
+      });
+      onClose();
     });
-
-    onClose();
   };
 
   const onError = (_error: unknown) => {