kaxil opened a new pull request, #74725: URL: https://github.com/apache/airflow/pull/74725
Clearing a task instance from the UI on 3.4.0b1 does nothing after Confirm. The browser sends the `dry_run: true` request to `clearTaskInstances` and never the real one, so the task is not cleared. It happens from the task instance page and from the run's Execution tab. With "Prevent running tasks" checked on a queued task, the "Cannot Clear Task Instance" warning never shows either. 3.3.2 is not affected. closes: #74712 **Cause.** Confirm hides the clear dialog and opens `ClearTaskInstanceConfirmationDialog`, which re-runs the dry run, then either sends the clear or shows the queued-task warning. A `git bisect` over the UI puts the break at #72524, the Chakra 3.36.1 to 3.37.0 bump, which moved zag's dialog, dismissable and focus-trap packages from 1.41.2 to 1.43.3 (3.3.2 still ships 1.41.2). With 1.43.3, the clear dialog closing underneath dismisses the new dialog before its dry run returns, through two separate paths: - zag's layer stack dismisses every layer stacked above a layer being removed (`layerStack.remove()` calls `dismiss()` on the nested layers). - The closing dialog returns focus to its trigger button, and the new dialog treats that as focus moving outside it. **Fix.** The confirmation dialog cancels both: `onRequestDismiss`, which zag only fires for the layer-stack case, and `onFocusOutside`. Outside clicks and Escape still close it. Blocking only the layer-stack dismissal, or only outside interaction, still lost the clear in Chrome. **Why guard the dismissals instead of restructuring the flow.** Opening the second dialog only once there is a warning to show also works, but then nothing is on screen during the check. The page stays clickable, so on the Task Instances list a selection change during the check could widen the clear. Keeping the clear dialog open under the second one leaves two modals stacked; in the unit tests the lower one then closed itself and reset the note. The guards keep the 3.3 behaviour as it was. **Tests.** `Execution.test.tsx` mocked the confirmation dialog out because "jsdom dismisses [it] before its dry run settles"; that was this bug. The mock is removed. Two Execution tests fail against the old dialog without it and pass with the fix. The two new tests in `ClearTaskInstanceDialog.test.tsx` (the clear goes out once a slow check resolves; the queued-task warning shows) also fail against the old dialog. happy-dom reproduces the layer-stack path but not the focus one, so I checked the fix in Chrome against a local Airflow. Confirm sends `dry_run: false` from the task instance page and the Execution tab. A queued task with "Prevent running tasks" shows the warning, sends nothing, and closes with Escape or an outside click. There is no visual change, so no screenshots. --- * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged. * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- 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]
