Skip to content

Commit b43d79c

Browse files
[v3-4-test] UI: Fix clearing a task instance doing nothing after Confirm (#74725) (#74738)
Confirming a task instance clear opens a second dialog that re-runs the dry run and then sends the clear. Since the zag dialog upgrade that came with Chakra 3.37, closing the clear dialog underneath dismisses that second dialog twice over: the layer stack dismisses layers above a removed one, and the clear dialog hands focus back to its trigger. The dry run finished but the clear request was never sent, and the queued-task warning never appeared. The second dialog now ignores both of those dismissals. Outside clicks and Escape still close it. (cherry picked from commit ea805e3) closes: #74712 Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
1 parent 8854a75 commit b43d79c

3 files changed

Lines changed: 57 additions & 24 deletions

File tree

‎airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearTaskInstanceConfirmationDialog.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,18 @@ const ClearTaskInstanceConfirmationDialog = ({
8080
}
8181
}, [isFetching, data, open, taskCurrentState, preventRunningTask, onConfirm, onClose]);
8282

83+
// This dialog opens as the clear dialog closes beneath it, and that close would dismiss it
84+
// twice over: removing a dialog layer dismisses the layers stacked above it, and the clear
85+
// dialog hands focus back to its trigger outside this one. Either drops the clear before
86+
// the dry run settles. Outside clicks and Escape still close it.
8387
return (
84-
<Dialog.Root lazyMount onOpenChange={onClose} open={open}>
88+
<Dialog.Root
89+
lazyMount
90+
onFocusOutside={(event) => event.preventDefault()}
91+
onOpenChange={onClose}
92+
onRequestDismiss={(event) => event.preventDefault()}
93+
open={open}
94+
>
8595
<Dialog.Content backdrop>
8696
{isFetching ? (
8797
<VStack align="center" gap={3} justify="center" py={8}>

‎airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearTaskInstanceDialog.test.tsx‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import BulkClearTaskInstancesButton from "src/pages/TaskInstances/BulkClearTaskI
2828

2929
import {
3030
CLEAR_KEEP_TASK_STATE_KEY,
31+
CLEAR_PREVENT_RUNNING_TASK_KEY,
3132
CLEAR_TASK_INSTANCE_DEFAULT_OPTIONS_KEY,
3233
} from "src/constants/localStorage";
3334
import { handlers } from "src/mocks/handlers";
@@ -95,6 +96,7 @@ const loopMember: TaskInstanceResponse = {
9596

9697
const mutateMock = vi.fn();
9798
const dryRunsMock = vi.fn();
99+
const confirmationDryRun = { isFetching: false };
98100

99101
vi.mock("src/queries/useClearTaskInstances", () => ({
100102
useClearTaskInstances: () => ({ isPending: false, mutate: mutateMock }),
@@ -104,7 +106,11 @@ vi.mock("src/queries/useClearTaskInstances", () => ({
104106
// list in the main dialog and the confirmation dialog's running-task gate; a
105107
// same-tick response for both keeps the gate from blocking on a real fetch.
106108
vi.mock("src/queries/useClearTaskInstancesDryRun", () => ({
107-
useClearTaskInstancesDryRun: () => ({ data: affectedTasks, isFetching: false, isPending: false }),
109+
useClearTaskInstancesDryRun: () => ({
110+
data: affectedTasks,
111+
isFetching: confirmationDryRun.isFetching,
112+
isPending: false,
113+
}),
108114
useClearTaskInstancesDryRuns: (args: unknown) => {
109115
dryRunsMock(args);
110116

@@ -124,6 +130,7 @@ beforeAll(() => {
124130
});
125131
afterEach(() => {
126132
affectedTasks.task_instances = [taskInstance];
133+
confirmationDryRun.isFetching = false;
127134
mutateMock.mockClear();
128135
dryRunsMock.mockClear();
129136
server.resetHandlers();
@@ -154,6 +161,44 @@ describe("ClearTaskInstanceDialog", () => {
154161
expect(requestBody.keep_task_state).toBe(true);
155162
});
156163

164+
it("sends the clear once the running-task check resolves", async () => {
165+
const onClose = vi.fn();
166+
167+
confirmationDryRun.isFetching = true;
168+
const { rerender } = render(
169+
<ClearTaskInstanceDialog onClose={onClose} open taskInstance={taskInstance} />,
170+
{
171+
wrapper: Wrapper,
172+
},
173+
);
174+
175+
fireEvent.click(await screen.findByRole("button", { name: /modal\.confirm/iu }));
176+
// Let the clear dialog finish closing while the running-task check is still in flight.
177+
await new Promise((resolve) => {
178+
setTimeout(resolve, 0);
179+
});
180+
confirmationDryRun.isFetching = false;
181+
rerender(<ClearTaskInstanceDialog onClose={onClose} open taskInstance={taskInstance} />);
182+
183+
await waitFor(() => expect(mutateMock).toHaveBeenCalledTimes(1));
184+
const [{ requestBody }] = mutateMock.mock.calls[0] as [{ requestBody: { dry_run?: boolean } }];
185+
186+
expect(requestBody.dry_run).toBe(false);
187+
});
188+
189+
it("warns instead of clearing a queued task when running tasks are protected", async () => {
190+
localStorage.setItem(CLEAR_PREVENT_RUNNING_TASK_KEY, JSON.stringify(true));
191+
affectedTasks.task_instances = [{ ...taskInstance, state: "queued" }];
192+
render(<ClearTaskInstanceDialog onClose={vi.fn()} open taskInstance={taskInstance} />, {
193+
wrapper: Wrapper,
194+
});
195+
196+
fireEvent.click(await screen.findByRole("button", { name: /modal\.confirm/iu }));
197+
198+
expect(await screen.findByText(/confirmationDialog\.title/iu)).toBeVisible();
199+
expect(mutateMock).not.toHaveBeenCalled();
200+
});
201+
157202
it("clears the kept loop passes by execution id when another pass is unticked", async () => {
158203
affectedTasks.task_instances = [loopPass("pass-0", 0), loopPass("pass-2", 2)];
159204

‎airflow-core/src/airflow/ui/src/pages/Run/Execution.test.tsx‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,28 +42,6 @@ import { Execution } from "./Execution";
4242

4343
vi.mock("src/router", () => ({ taskInstanceRoutes: [] }));
4444

45-
// The running-task gate opens a second dialog that jsdom dismisses before its dry run settles.
46-
vi.mock("src/components/Clear/TaskInstance/ClearTaskInstanceConfirmationDialog", async () => {
47-
const { useEffect } = await import("react");
48-
49-
const ConfirmImmediately = ({
50-
onClose,
51-
onConfirm,
52-
}: {
53-
readonly onClose: () => void;
54-
readonly onConfirm?: () => void;
55-
}) => {
56-
useEffect(() => {
57-
onConfirm?.();
58-
onClose();
59-
}, [onClose, onConfirm]);
60-
61-
return undefined;
62-
};
63-
64-
return { default: ConfirmImmediately };
65-
});
66-
6745
beforeAll(() => {
6846
i18n.addResourceBundle("en", "dag", dagTranslations, true, true);
6947
i18n.addResourceBundle("en", "common", commonTranslations, true, true);

0 commit comments

Comments
 (0)