diff --git a/airflow/www/static/js/dag/details/gantt/Row.tsx b/airflow/www/static/js/dag/details/gantt/Row.tsx index 304fb83e22559..bf1ce7ac60a25 100644 --- a/airflow/www/static/js/dag/details/gantt/Row.tsx +++ b/airflow/www/static/js/dag/details/gantt/Row.tsx @@ -53,7 +53,7 @@ const Row = ({ const instance = task.instances.find((ti) => ti.runId === runId); const isSelected = taskId === instance?.taskId; const hasQueuedDttm = !!instance?.queuedDttm; - const isOpen = openGroupIds.includes(task.label || ""); + const isOpen = openGroupIds.includes(task.id || ""); // Calculate durations in ms const taskDuration = getDuration(instance?.startDate, instance?.endDate); diff --git a/airflow/www/static/js/dag/details/graph/utils.ts b/airflow/www/static/js/dag/details/graph/utils.ts index 3d85053619258..ac940d46c7847 100644 --- a/airflow/www/static/js/dag/details/graph/utils.ts +++ b/airflow/www/static/js/dag/details/graph/utils.ts @@ -79,9 +79,9 @@ export const flattenNodes = ({ onToggleCollapse: () => { let newGroupIds = []; if (!node.value.isOpen) { - newGroupIds = [...openGroupIds, node.value.label]; + newGroupIds = [...openGroupIds, node.id]; } else { - newGroupIds = openGroupIds.filter((g) => g !== node.value.label); + newGroupIds = openGroupIds.filter((g) => g !== node.id); } onToggleGroups(newGroupIds); }, diff --git a/airflow/www/static/js/dag/grid/TaskName.test.tsx b/airflow/www/static/js/dag/grid/TaskName.test.tsx index f7a4c7267d1f2..8002ab78ca038 100644 --- a/airflow/www/static/js/dag/grid/TaskName.test.tsx +++ b/airflow/www/static/js/dag/grid/TaskName.test.tsx @@ -29,7 +29,7 @@ import TaskName from "./TaskName"; describe("Test TaskName", () => { test("Displays a normal task name", () => { const { getByText } = render( - {}} />, + {}} />, { wrapper: ChakraWrapper } ); @@ -38,7 +38,13 @@ describe("Test TaskName", () => { test("Displays a mapped task name", () => { const { getByText } = render( - {}} />, + {}} + />, { wrapper: ChakraWrapper } ); @@ -47,7 +53,7 @@ describe("Test TaskName", () => { test("Displays a group task name", () => { const { getByText, getByTestId } = render( - {}} />, + {}} />, { wrapper: ChakraWrapper } ); diff --git a/airflow/www/static/js/dag/grid/TaskName.tsx b/airflow/www/static/js/dag/grid/TaskName.tsx index 030767212fd4d..ce2de10bf69b8 100644 --- a/airflow/www/static/js/dag/grid/TaskName.tsx +++ b/airflow/www/static/js/dag/grid/TaskName.tsx @@ -28,6 +28,7 @@ interface Props { isOpen?: boolean; level?: number; label: string; + id: string; } const TaskName = ({ @@ -37,11 +38,13 @@ const TaskName = ({ isOpen = false, level = 0, label, + id, }: Props) => ( { const checkTasks = (tasks: Task[]) => tasks.forEach((task) => { if (task.children) { - groupIds.push(task.label!); + groupIds.push(task.id!); checkTasks(task.children); } }); diff --git a/airflow/www/static/js/dag/grid/index.test.tsx b/airflow/www/static/js/dag/grid/index.test.tsx index 66e1cafaeb401..5cc6e7178885d 100644 --- a/airflow/www/static/js/dag/grid/index.test.tsx +++ b/airflow/www/static/js/dag/grid/index.test.tsx @@ -86,6 +86,55 @@ const mockGridData = { }, ], }, + { + id: "group_2", + label: "group_2", + instances: [ + { + endDate: "2021-10-26T15:42:03.391939+00:00", + runId: "run1", + startDate: "2021-10-26T15:42:03.391917+00:00", + state: "success", + taskId: "group_1", + tryNumber: 1, + }, + ], + children: [ + { + id: "group_2.task_1", + operator: "DummyOperator", + label: "task_1", + instances: [ + { + endDate: "2021-10-26T15:42:03.391939+00:00", + runId: "run1", + startDate: "2021-10-26T15:42:03.391917+00:00", + state: "success", + taskId: "group_1.task_1", + tryNumber: 1, + }, + ], + children: [ + { + id: "group_2.task_1.sub_task_1", + label: "sub_task_1", + extraLinks: [], + operator: "DummyOperator", + instances: [ + { + endDate: "2021-10-26T15:42:03.391939+00:00", + runId: "run1", + startDate: "2021-10-26T15:42:03.391917+00:00", + state: "success", + taskId: "group_2.task_1.sub_task_1", + tryNumber: 1, + }, + ], + }, + ], + }, + ], + }, ], instances: [], }, @@ -144,16 +193,18 @@ describe("Test ToggleGroups", () => { }); test("Group defaults to closed", () => { - const { getByTestId, getByText, getAllByTestId } = render( + const { queryAllByTestId, getByText, getAllByTestId } = render( {}} />, { wrapper: Wrapper } ); - const groupName = getByText("group_1"); + const groupName1 = getByText("group_1"); + const groupName2 = getByText("group_2"); - expect(getAllByTestId("task-instance")).toHaveLength(1); - expect(groupName).toBeInTheDocument(); - expect(getByTestId("open-group")).toBeInTheDocument(); + expect(getAllByTestId("task-instance")).toHaveLength(2); + expect(groupName1).toBeInTheDocument(); + expect(groupName2).toBeInTheDocument(); + expect(queryAllByTestId("open-group")).toHaveLength(2); }); test("Buttons are disabled if all groups are expanded or collapsed", () => { @@ -168,12 +219,12 @@ describe("Test ToggleGroups", () => { expect(collapseButton).toBeDisabled(); const taskElements = queryAllByTestId("task-instance"); - expect(taskElements).toHaveLength(1); + expect(taskElements).toHaveLength(2); fireEvent.click(expandButton); const newTaskElements = queryAllByTestId("task-instance"); - expect(newTaskElements).toHaveLength(3); + expect(newTaskElements).toHaveLength(6); expect(collapseButton).toBeEnabled(); expect(expandButton).toBeDisabled(); @@ -188,41 +239,71 @@ describe("Test ToggleGroups", () => { const expandButton = getByTitle(EXPAND); const collapseButton = getByTitle(COLLAPSE); - const groupName = getByText("group_1"); + const groupName1 = getByText("group_1"); + const groupName2 = getByText("group_2"); - expect(queryAllByTestId("task-instance")).toHaveLength(3); - expect(groupName).toBeInTheDocument(); + expect(queryAllByTestId("task-instance")).toHaveLength(6); + expect(groupName1).toBeInTheDocument(); + expect(groupName2).toBeInTheDocument(); - expect(queryAllByTestId("close-group")).toHaveLength(2); + expect(queryAllByTestId("close-group")).toHaveLength(4); expect(queryAllByTestId("open-group")).toHaveLength(0); fireEvent.click(collapseButton); await waitFor(() => - expect(queryAllByTestId("task-instance")).toHaveLength(1) + expect(queryAllByTestId("task-instance")).toHaveLength(2) ); expect(queryAllByTestId("close-group")).toHaveLength(0); - // Since the groups are nested, only the parent row is rendered - expect(queryAllByTestId("open-group")).toHaveLength(1); + // Since the groups are nested, only the parent rows is rendered + expect(queryAllByTestId("open-group")).toHaveLength(2); + + fireEvent.click(expandButton); + + await waitFor(() => + expect(queryAllByTestId("task-instance")).toHaveLength(6) + ); + expect(queryAllByTestId("close-group")).toHaveLength(4); + expect(queryAllByTestId("open-group")).toHaveLength(0); + }); + + test("Toggling a group does not affect groups with the same label", async () => { + const { getByTitle, getByTestId, queryAllByTestId } = render( + , + { wrapper: Wrapper } + ); + + const expandButton = getByTitle(EXPAND); fireEvent.click(expandButton); await waitFor(() => - expect(queryAllByTestId("task-instance")).toHaveLength(3) + expect(queryAllByTestId("task-instance")).toHaveLength(6) ); - expect(queryAllByTestId("close-group")).toHaveLength(2); + expect(queryAllByTestId("close-group")).toHaveLength(4); expect(queryAllByTestId("open-group")).toHaveLength(0); + + const group2Task1 = getByTestId("group_2.task_1"); + + fireEvent.click(group2Task1); + + // We only expect group_2.task_1 to be affected, not group_1.task_1 + await waitFor(() => + expect(queryAllByTestId("task-instance")).toHaveLength(5) + ); + expect(queryAllByTestId("close-group")).toHaveLength(3); + expect(queryAllByTestId("open-group")).toHaveLength(1); }); test("Hovered effect on task state", async () => { - const openGroupIds = ["group_1", "task_1"]; + const openGroupIds = ["group_1", "group_1.task_1"]; const { rerender, queryAllByTestId } = render( {}} />, { wrapper: Wrapper } ); const taskElements = queryAllByTestId("task-instance"); - expect(taskElements).toHaveLength(3); + expect(taskElements).toHaveLength(4); taskElements.forEach((taskElement) => { expect(taskElement).toHaveStyle("opacity: 1"); diff --git a/airflow/www/static/js/dag/grid/renderTaskRows.tsx b/airflow/www/static/js/dag/grid/renderTaskRows.tsx index c661eba6775ce..181186b172db9 100644 --- a/airflow/www/static/js/dag/grid/renderTaskRows.tsx +++ b/airflow/www/static/js/dag/grid/renderTaskRows.tsx @@ -122,20 +122,20 @@ const Row = (props: RowProps) => { const isGroup = !!task.children; const isSelected = selected.taskId === task.id; - const isOpen = openGroupIds.some((g) => g === task.label); + const isOpen = openGroupIds.some((g) => g === task.id); // assure the function is the same across renders const memoizedToggle = useCallback(() => { - if (isGroup && task.label) { + if (isGroup && task.id) { let newGroupIds = []; if (!isOpen) { - newGroupIds = [...openGroupIds, task.label]; + newGroupIds = [...openGroupIds, task.id]; } else { - newGroupIds = openGroupIds.filter((g) => g !== task.label); + newGroupIds = openGroupIds.filter((g) => g !== task.id); } onToggleGroups(newGroupIds); } - }, [isGroup, isOpen, task.label, openGroupIds, onToggleGroups]); + }, [isGroup, isOpen, task.id, openGroupIds, onToggleGroups]); // check if the group's parents are all open, if not, return null if (level !== openParentCount) return null; @@ -168,6 +168,7 @@ const Row = (props: RowProps) => { isGroup={isGroup} isMapped={task.isMapped && !isParentMapped} label={task.label || task.id || ""} + id={task.id || ""} isOpen={isOpen} level={level} /> diff --git a/airflow/www/static/js/utils/graph.ts b/airflow/www/static/js/utils/graph.ts index 08d541583331b..1f77990930356 100644 --- a/airflow/www/static/js/utils/graph.ts +++ b/airflow/www/static/js/utils/graph.ts @@ -117,7 +117,7 @@ const generateGraph = ({ height?: number; } => { const { id, value, children } = node; - const isOpen = openGroupIds?.includes(value.label); + const isOpen = openGroupIds?.includes(id); const childCount = children?.filter((c: DepNode) => !c.id.includes("join_id")).length || 0; const childIds = children?.length ? getNestedChildIds(children) : []; @@ -170,7 +170,7 @@ const generateGraph = ({ sourceId: childIds.indexOf(e.sourceId) > -1 ? node.id : e.sourceId, targetId: childIds.indexOf(e.targetId) > -1 ? node.id : e.targetId, })); - closedGroupIds.push(value.label); + closedGroupIds.push(id); } return { id,