bbovenzi commented on code in PR #65835:
URL: https://github.com/apache/airflow/pull/65835#discussion_r3147883500
##########
airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearGroupTaskInstanceDialog.tsx:
##########
@@ -106,8 +107,28 @@ export const ClearGroupTaskInstanceDialog = ({ onClose,
open, taskInstance }: Pr
total_entries: 0,
};
- const shouldShowBundleVersionOption =
- dagDetails?.bundle_version !== null && dagDetails?.bundle_version !== "";
+ const latestDagVersionNumber =
dagDetails?.latest_dag_version?.version_number;
+ const groupDagVersionNumber = taskInstance.dag_version_number ?? undefined;
+ const dagVersionsDiffer =
+ latestDagVersionNumber !== undefined &&
+ groupDagVersionNumber !== undefined &&
+ latestDagVersionNumber !== groupDagVersionNumber;
+ // Fall back to legacy heuristic when grid summary has no version (older
API).
+ const shouldShowRunOnLatestOption =
+ dagVersionsDiffer || (dagDetails?.bundle_version !== null &&
dagDetails?.bundle_version !== "");
+
+ useEffect(() => {
+ if (!open) {
+ userToggledRunOnLatestRef.current = false;
+ }
+ }, [open]);
+
+ useEffect(() => {
+ if (!open || userToggledRunOnLatestRef.current) {
+ return;
+ }
+ setRunOnLatestVersion(dagVersionsDiffer);
+ }, [open, dagVersionsDiffer]);
Review Comment:
Let's use `onOpenChange` on <Dialog>` instead of needing so many useEffects
##########
airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearGroupTaskInstanceDialog.tsx:
##########
@@ -160,14 +181,17 @@ export const ClearGroupTaskInstanceDialog = ({ onClose,
open, taskInstance }: Pr
</Flex>
<ActionAccordion affectedTasks={affectedTasks} note={note}
setNote={setNote} />
<Flex
Review Comment:
While we're at it. Let's add a gap between the two different checkboxes
##########
airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearGroupTaskInstanceDialog.tsx:
##########
@@ -106,8 +107,28 @@ export const ClearGroupTaskInstanceDialog = ({ onClose,
open, taskInstance }: Pr
total_entries: 0,
};
- const shouldShowBundleVersionOption =
- dagDetails?.bundle_version !== null && dagDetails?.bundle_version !== "";
+ const latestDagVersionNumber =
dagDetails?.latest_dag_version?.version_number;
+ const groupDagVersionNumber = taskInstance.dag_version_number ?? undefined;
+ const dagVersionsDiffer =
+ latestDagVersionNumber !== undefined &&
+ groupDagVersionNumber !== undefined &&
+ latestDagVersionNumber !== groupDagVersionNumber;
+ // Fall back to legacy heuristic when grid summary has no version (older
API).
+ const shouldShowRunOnLatestOption =
+ dagVersionsDiffer || (dagDetails?.bundle_version !== null &&
dagDetails?.bundle_version !== "");
Review Comment:
And let's have a unit test for all these combinations.
##########
airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearGroupTaskInstanceDialog.tsx:
##########
@@ -106,8 +107,28 @@ export const ClearGroupTaskInstanceDialog = ({ onClose,
open, taskInstance }: Pr
total_entries: 0,
};
- const shouldShowBundleVersionOption =
- dagDetails?.bundle_version !== null && dagDetails?.bundle_version !== "";
+ const latestDagVersionNumber =
dagDetails?.latest_dag_version?.version_number;
+ const groupDagVersionNumber = taskInstance.dag_version_number ?? undefined;
+ const dagVersionsDiffer =
+ latestDagVersionNumber !== undefined &&
+ groupDagVersionNumber !== undefined &&
+ latestDagVersionNumber !== groupDagVersionNumber;
+ // Fall back to legacy heuristic when grid summary has no version (older
API).
+ const shouldShowRunOnLatestOption =
+ dagVersionsDiffer || (dagDetails?.bundle_version !== null &&
dagDetails?.bundle_version !== "");
Review Comment:
We're repeating a lot of code between the dialogs. Let's move them to a
shared util
--
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]
