bbovenzi commented on code in PR #44850:
URL: https://github.com/apache/airflow/pull/44850#discussion_r1884527269
##########
airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx:
##########
@@ -20,28 +20,49 @@ import { Input, Button, Box, Text, Spacer, HStack } 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, useState } from "react";
+import { useEffect, useMemo, useState } from "react";
import { useForm, Controller } from "react-hook-form";
import { FiPlay } from "react-icons/fi";
import { useColorMode } from "src/context/colorMode";
+import { useDagParams } from "src/queries/useDagParams";
+import { useTrigger } from "src/queries/useTrigger";
import { Accordion } from "../ui";
-import type { DagParams } from "./TriggerDag";
type TriggerDAGFormProps = {
- dagParams: DagParams;
+ dagId: string;
onClose: () => void;
- onTrigger: (updatedDagParams: DagParams) => void;
- setDagParams: React.Dispatch<React.SetStateAction<DagParams>>;
+ open: boolean;
+};
+
+export type DagRunTriggerParams = {
+ conf: string;
+ dagRunId: string;
+ dataIntervalEnd: string;
+ dataIntervalStart: string;
+ note: string;
};
const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
- dagParams,
- onTrigger,
- setDagParams,
+ dagId,
+ onClose,
+ open,
}) => {
const [jsonError, setJsonError] = useState<string | undefined>();
+ const conf = useDagParams(dagId, open);
+ const { isPending, triggerDagRun } = useTrigger();
Review Comment:
After we get the API error showing up we might want to add the data_interval
error as a client-side check before hitting submit
##########
airflow/ui/src/components/TriggerDag/TriggerDAGForm.tsx:
##########
@@ -20,28 +20,49 @@ import { Input, Button, Box, Text, Spacer, HStack } 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, useState } from "react";
+import { useEffect, useMemo, useState } from "react";
import { useForm, Controller } from "react-hook-form";
import { FiPlay } from "react-icons/fi";
import { useColorMode } from "src/context/colorMode";
+import { useDagParams } from "src/queries/useDagParams";
+import { useTrigger } from "src/queries/useTrigger";
import { Accordion } from "../ui";
-import type { DagParams } from "./TriggerDag";
type TriggerDAGFormProps = {
- dagParams: DagParams;
+ dagId: string;
onClose: () => void;
- onTrigger: (updatedDagParams: DagParams) => void;
- setDagParams: React.Dispatch<React.SetStateAction<DagParams>>;
+ open: boolean;
+};
+
+export type DagRunTriggerParams = {
+ conf: string;
+ dagRunId: string;
+ dataIntervalEnd: string;
+ dataIntervalStart: string;
+ note: string;
};
const TriggerDAGForm: React.FC<TriggerDAGFormProps> = ({
- dagParams,
- onTrigger,
- setDagParams,
+ dagId,
+ onClose,
+ open,
}) => {
const [jsonError, setJsonError] = useState<string | undefined>();
+ const conf = useDagParams(dagId, open);
+ const { isPending, triggerDagRun } = useTrigger();
Review Comment:
```suggestion
const { isPending, triggerDagRun, error } = useTrigger({ onClose );
```
Closing the modal and then showing an error toast is not an ideal UX.
Instead, the hook should return any error and it should be shown in the form.
For example, try passing just a data_interval_start but not the end.
Related, we should pass `onClose` as a callback onSuccess instead of when
the user hits submit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
