-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add support for opening trigger modal via URL pre-populated #55277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b5d3905
761f486
5d501cd
63ea8a6
43f9259
13e9ca0
599e395
77da4f2
deb6a41
ae96ea4
e655594
26d8a7b
969a418
463ddd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import dayjs from "dayjs"; | |
| import { useEffect, useState } from "react"; | ||
| import { useForm, Controller, useWatch } from "react-hook-form"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { useLocation } from "react-router-dom"; | ||
|
|
||
| import type { DAGResponse, DAGWithLatestDagRunsResponse, BackfillPostBody } from "openapi/requests/types.gen"; | ||
| import { Button } from "src/components/ui"; | ||
|
|
@@ -30,6 +31,7 @@ import { useCreateBackfillDryRun } from "src/queries/useCreateBackfillDryRun"; | |
| import { useDagParams } from "src/queries/useDagParams"; | ||
| import { useParamStore } from "src/queries/useParamStore"; | ||
| import { useTogglePause } from "src/queries/useTogglePause"; | ||
| import { getPreloadBackfillFormData } from "src/utils/trigger"; | ||
|
|
||
| import ConfigForm from "../ConfigForm"; | ||
| import { DateTimeInput } from "../DateTimeInput"; | ||
|
|
@@ -43,6 +45,7 @@ type RunBackfillFormProps = { | |
| readonly dag: DAGResponse | DAGWithLatestDagRunsResponse; | ||
| readonly onClose: () => void; | ||
| }; | ||
|
|
||
| const today = new Date().toISOString().slice(0, 16); | ||
|
|
||
| type BackfillFormProps = DagRunTriggerParams & Omit<BackfillPostBody, "dag_run_conf">; | ||
|
|
@@ -54,15 +57,26 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => { | |
| const [formError, setFormError] = useState(false); | ||
| const initialParamsDict = useDagParams(dag.dag_id, true); | ||
| const { conf } = useParamStore(); | ||
| const { control, handleSubmit, reset, watch } = useForm<BackfillFormProps>({ | ||
| const { search } = useLocation(); | ||
| const params = new URLSearchParams(search); | ||
| const { | ||
| conf: urlConf, | ||
| from_date: urlFromDate, | ||
| max_active_runs: urlMaxActiveRuns, | ||
| reprocess_behavior: urlReprocessBehavior, | ||
| run_backwards: urlRunBackwards, | ||
| to_date: urlToDate, | ||
| } = getPreloadBackfillFormData(params); | ||
|
Comment on lines
+61
to
+69
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use the |
||
|
|
||
| const { control, handleSubmit, reset } = useForm<BackfillFormProps>({ | ||
| defaultValues: { | ||
| conf, | ||
| conf: urlConf ?? (conf || "{}"), | ||
| dag_id: dag.dag_id, | ||
| from_date: "", | ||
| max_active_runs: 1, | ||
| reprocess_behavior: "none", | ||
| run_backwards: false, | ||
| to_date: "", | ||
| from_date: urlFromDate, | ||
| max_active_runs: urlMaxActiveRuns, | ||
| reprocess_behavior: urlReprocessBehavior, | ||
| run_backwards: urlRunBackwards, | ||
| to_date: urlToDate, | ||
| }, | ||
| mode: "onBlur", | ||
| }); | ||
|
|
@@ -90,22 +104,16 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => { | |
| useEffect(() => { | ||
| if (Boolean(dateValidationError)) { | ||
| setErrors((prev) => ({ ...prev, date: dateValidationError })); | ||
| } | ||
| }, [dateValidationError]); | ||
|
|
||
| useEffect(() => { | ||
| if (conf) { | ||
| } else if (urlConf === null && conf) { | ||
| reset((prevValues) => ({ | ||
| ...prevValues, | ||
| conf, | ||
| })); | ||
| } | ||
| }, [conf, reset]); | ||
| }, [dateValidationError, urlConf, conf, reset]); | ||
|
|
||
| const dataIntervalStart = watch("from_date"); | ||
| const dataIntervalEnd = watch("to_date"); | ||
| const noDataInterval = !Boolean(dataIntervalStart) || !Boolean(dataIntervalEnd); | ||
| const dataIntervalInvalid = dayjs(dataIntervalStart).isAfter(dayjs(dataIntervalEnd)); | ||
| const noDataInterval = !Boolean(values.from_date) || !Boolean(values.to_date); | ||
| const dataIntervalInvalid = dayjs(values.from_date).isAfter(dayjs(values.to_date)); | ||
|
|
||
| const onSubmit = (fdata: BackfillFormProps) => { | ||
| if (unpause && dag.is_paused) { | ||
|
|
@@ -248,6 +256,7 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => { | |
| control={control} | ||
| errors={errors} | ||
| initialParamsDict={initialParamsDict} | ||
| openAdvanced={Boolean(urlConf ?? conf)} | ||
| setErrors={setErrors} | ||
| setFormError={setFormError} | ||
| /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,12 +17,14 @@ | |
| * under the License. | ||
| */ | ||
| import { Heading, VStack, HStack, Spinner, Center, Text } from "@chakra-ui/react"; | ||
| import React, { useState } from "react"; | ||
| import React, { useEffect, useState } from "react"; | ||
| import { useTranslation } from "react-i18next"; | ||
| import { useLocation, useNavigate } from "react-router-dom"; | ||
|
|
||
| import { useDagServiceGetDag } from "openapi/queries"; | ||
| import { Dialog, Tooltip } from "src/components/ui"; | ||
| import { RadioCardItem, RadioCardRoot } from "src/components/ui/RadioCard"; | ||
| import { getRunModeFromPathname, normalizeTriggerPath } from "src/utils/trigger"; | ||
|
|
||
| import RunBackfillForm from "../DagActions/RunBackfillForm"; | ||
| import TriggerDAGForm from "./TriggerDAGForm"; | ||
|
|
@@ -48,7 +50,11 @@ const TriggerDAGModal: React.FC<TriggerDAGModalProps> = ({ | |
| open, | ||
| }) => { | ||
| const { t: translate } = useTranslation("components"); | ||
| const [runMode, setRunMode] = useState<RunMode>(RunMode.SINGLE); | ||
| const { pathname, search } = useLocation(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| const [runMode, setRunMode] = useState<RunMode>(getRunModeFromPathname(pathname)); | ||
|
|
||
| const { | ||
| data: dag, | ||
| isError, | ||
|
|
@@ -67,6 +73,19 @@ const TriggerDAGModal: React.FC<TriggerDAGModalProps> = ({ | |
| const maxDisplayLength = 59; // hard-coded length to prevent dag name overflowing the modal | ||
| const nameOverflowing = dagDisplayName.length > maxDisplayLength; | ||
|
|
||
| useEffect(() => { | ||
| // Only sync URL when already within trigger route to avoid interfering with close navigation | ||
| if (!open || !/\/trigger(\/(single|backfill))?$/u.test(pathname)) { | ||
| return; | ||
| } | ||
|
|
||
| const targetPath = normalizeTriggerPath(pathname, runMode); | ||
|
|
||
| if (pathname !== targetPath) { | ||
| navigate({ pathname: targetPath, search }, { replace: true }); | ||
| } | ||
| }, [runMode, pathname, search, navigate, open]); | ||
|
|
||
|
Comment on lines
+77
to
+88
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can probably add router entries on |
||
| return ( | ||
| <Dialog.Root lazyMount onOpenChange={onClose} open={open} size="xl" unmountOnExit> | ||
| <Dialog.Content backdrop> | ||
|
|
@@ -79,7 +98,7 @@ const TriggerDAGModal: React.FC<TriggerDAGModalProps> = ({ | |
| </VStack> | ||
| </Dialog.Header> | ||
|
|
||
| <Dialog.CloseTrigger /> | ||
| <Dialog.CloseTrigger onClick={onClose} /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need this change |
||
|
|
||
| <Dialog.Body> | ||
| {isLoading ? ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,15 @@ export const routerConfig = [ | |
| element: <Dag />, | ||
| path: "dags/:dagId", | ||
| }, | ||
| { | ||
| children: [ | ||
| { element: <Overview />, index: true }, | ||
| { element: <Overview />, path: "single" }, | ||
| { element: <Overview />, path: "backfill" }, | ||
|
Comment on lines
+179
to
+180
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should pass |
||
| ], | ||
| element: <Dag />, | ||
| path: "dags/:dagId/trigger", | ||
| }, | ||
| { | ||
| children: [ | ||
| { element: <TaskInstances />, index: true }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fields in the form would dynamically change based on user's config. Thus, we may not only need to handle these specific fields but we should handle all fields passed. The implementation is quite like #54783