fix: update node executions to display map tasks (#455)

* fix: update node executions to display map tasks * fix: update map task logs styles * test: add/update unit tests * fix: fix flickering and unnecessary re-renders Signed-off-by: Olga Nad <[email protected]>
flyteorg · May 13, 2022 · 7bed57e · 7bed57e
1 parent 686f3cc
commit 7bed57e
Show file tree

Hide file tree

Showing 20 changed files with 1,031 additions and 167 deletions.
diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx
@@ -4,12 +4,16 @@ import { DataError } from 'components/Errors/DataError';
 import { makeWorkflowQuery } from 'components/Workflow/workflowQueries';
 import { WorkflowGraph } from 'components/WorkflowGraph/WorkflowGraph';
 import { keyBy } from 'lodash';
-import { NodeExecution } from 'models/Execution/types';
+import { TaskExecutionPhase } from 'models/Execution/enums';
+import { ExternalResource, LogsByPhase, NodeExecution } from 'models/Execution/types';
 import { endNodeId, startNodeId } from 'models/Node/constants';
 import { Workflow, WorkflowId } from 'models/Workflow/types';
 import * as React from 'react';
+import { useMemo, useState } from 'react';
 import { useQuery, useQueryClient } from 'react-query';
 import { NodeExecutionsContext } from '../contexts';
+import { getGroupedLogs } from '../TaskExecutionsList/utils';
+import { useTaskExecutions, useTaskExecutionsRefresher } from '../useTaskExecutions';
 import { NodeExecutionDetailsPanelContent } from './NodeExecutionDetailsPanelContent';
 
 export interface ExecutionWorkflowGraphProps {
@@ -23,12 +27,30 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
   workflowId,
 }) => {
   const workflowQuery = useQuery<Workflow, Error>(makeWorkflowQuery(useQueryClient(), workflowId));
-  const nodeExecutionsById = React.useMemo(
-    () => keyBy(nodeExecutions, 'scopedId'),
-    [nodeExecutions],
+
+  const nodeExecutionsWithResources = nodeExecutions.map((nodeExecution) => {
+    const taskExecutions = useTaskExecutions(nodeExecution.id);
+    useTaskExecutionsRefresher(nodeExecution, taskExecutions);
+
+    const externalResources: ExternalResource[] = taskExecutions.value
+      .map((taskExecution) => taskExecution.closure.metadata?.externalResources)
+      .flat()
+      .filter((resource): resource is ExternalResource => !!resource);
+
+    const logsByPhase: LogsByPhase = getGroupedLogs(externalResources);
+
+    return {
+      ...nodeExecution,
+      ...(logsByPhase.size > 0 && { logsByPhase }),
+    };
+  });
+
+  const nodeExecutionsById = useMemo(
+    () => keyBy(nodeExecutionsWithResources, 'scopedId'),
+    [nodeExecutionsWithResources],
   );
 
-  const [selectedNodes, setSelectedNodes] = React.useState<string[]>([]);
+  const [selectedNodes, setSelectedNodes] = useState<string[]>([]);
   const onNodeSelectionChanged = (newSelection: string[]) => {
     const validSelection = newSelection.filter((nodeId) => {
       if (nodeId === startNodeId || nodeId === endNodeId) {
@@ -52,9 +74,12 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
 
   const onCloseDetailsPanel = () => setSelectedNodes([]);
 
+  const [selectedPhase, setSelectedPhase] = useState<TaskExecutionPhase | undefined>(undefined);
+
   const renderGraph = (workflow: Workflow) => (
     <WorkflowGraph
       onNodeSelectionChanged={onNodeSelectionChanged}
+      onPhaseSelectionChanged={setSelectedPhase}
       nodeExecutionsById={nodeExecutionsById}
       workflow={workflow}
     />
@@ -71,6 +96,7 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
         {selectedExecution && (
           <NodeExecutionDetailsPanelContent
             onClose={onCloseDetailsPanel}
+            phase={selectedPhase}
             nodeExecutionId={selectedExecution}
           />
         )}

diff --git a/...p/console/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx b/...p/console/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx
@@ -1,8 +1,9 @@
 import * as React from 'react';
-import { useEffect, useRef } from 'react';
+import { useEffect, useMemo, useRef, useState } from 'react';
 import { IconButton, Typography, Tab, Tabs } from '@material-ui/core';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import Close from '@material-ui/icons/Close';
+import ArrowBackIos from '@material-ui/icons/ArrowBackIos';
 import classnames from 'classnames';
 import { useCommonStyles } from 'components/common/styles';
 import { InfoIcon } from 'components/common/Icons/InfoIcon';
@@ -23,7 +24,7 @@ import { fetchWorkflow } from 'components/Workflow/workflowQueries';
 import { PanelSection } from 'components/common/PanelSection';
 import { DumpJSON } from 'components/common/DumpJSON';
 import { dNode } from 'models/Graph/types';
-import { NodeExecutionPhase } from 'models/Execution/enums';
+import { NodeExecutionPhase, TaskExecutionPhase } from 'models/Execution/enums';
 import { transformWorkflowToKeyedDag, getNodeNameFromDag } from 'components/WorkflowGraph/utils';
 import { NodeExecutionCacheStatus } from '../NodeExecutionCacheStatus';
 import { makeListTaskExecutionsQuery, makeNodeExecutionQuery } from '../nodeExecutionQueries';
@@ -126,6 +127,7 @@ const tabIds = {
 
 interface NodeExecutionDetailsProps {
   nodeExecutionId: NodeExecutionIdentifier;
+  phase?: TaskExecutionPhase;
   onClose?: () => void;
 }
 
@@ -218,8 +220,19 @@ const WorkflowTabs: React.FC<{
  */
 export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProps> = ({
   nodeExecutionId,
+  phase,
   onClose,
 }) => {
+  const commonStyles = useCommonStyles();
+  const styles = useStyles();
+  const queryClient = useQueryClient();
+  const detailsContext = useNodeExecutionContext();
+
+  const [isReasonsVisible, setReasonsVisible] = useState<boolean>(false);
+  const [dag, setDag] = useState<any>(null);
+  const [details, setDetails] = useState<NodeExecutionDetails | undefined>();
+  const [shouldShowTaskDetails, setShouldShowTaskDetails] = useState<boolean>(false); // TODO to be reused in https://github.com/flyteorg/flyteconsole/issues/312
+
   const isMounted = useRef(false);
   useEffect(() => {
     isMounted.current = true;
@@ -228,21 +241,14 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
     };
   }, []);
 
-  const queryClient = useQueryClient();
-  const detailsContext = useNodeExecutionContext();
-
-  const [isReasonsVisible, setReasonsVisible] = React.useState(false);
-  const [dag, setDag] = React.useState<any>(null);
-  const [details, setDetails] = React.useState<NodeExecutionDetails | undefined>();
-
   const nodeExecutionQuery = useQuery<NodeExecution, Error>({
     ...makeNodeExecutionQuery(nodeExecutionId),
     // The selected NodeExecution has been fetched at this point, we don't want to
     // issue an additional fetch.
     staleTime: Infinity,
   });
 
-  React.useEffect(() => {
+  useEffect(() => {
     let isCurrent = true;
     detailsContext.getNodeExecutionDetails(nodeExecution).then((res) => {
       if (isCurrent) {
@@ -255,7 +261,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
     };
   });
 
-  React.useEffect(() => {
+  useEffect(() => {
     setReasonsVisible(false);
   }, [nodeExecutionId]);
 
@@ -288,20 +294,48 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
 
   const reasons = getTaskExecutionDetailReasons(listTaskExecutionsQuery.data);
 
-  const commonStyles = useCommonStyles();
-  const styles = useStyles();
-  const displayName = details?.displayName ?? <Skeleton />;
+  const onBackClick = () => {
+    setShouldShowTaskDetails(false);
+  };
+
+  const headerTitle = useMemo(() => {
+    // TODO to be reused in https://github.com/flyteorg/flyteconsole/issues/312
+    // // eslint-disable-next-line no-useless-escape
+    // const regex = /\-([\w\s-]+)\-/; // extract string between first and last dash
+
+    // const mapTaskHeader = `${mapTask?.[0].externalId?.match(regex)?.[1]} of ${
+    //   nodeExecutionId.nodeId
+    // }`;
+    // const header = shouldShowTaskDetails ? mapTaskHeader : nodeExecutionId.nodeId;
+    const header = nodeExecutionId.nodeId;
 
-  const isRunningPhase = React.useMemo(() => {
+    return (
+      <Typography className={classnames(commonStyles.textWrapped, styles.title)} variant="h3">
+        <div>
+          {shouldShowTaskDetails && (
+            <IconButton onClick={onBackClick} size="small">
+              <ArrowBackIos />
+            </IconButton>
+          )}
+          {header}
+        </div>
+        <IconButton className={styles.closeButton} onClick={onClose} size="small">
+          <Close />
+        </IconButton>
+      </Typography>
+    );
+  }, [nodeExecutionId, shouldShowTaskDetails]);
+
+  const isRunningPhase = useMemo(() => {
     return (
       nodeExecution?.closure.phase === NodeExecutionPhase.QUEUED ||
       nodeExecution?.closure.phase === NodeExecutionPhase.RUNNING
     );
   }, [nodeExecution]);
 
-  const handleReasonsVisibility = React.useCallback(() => {
-    setReasonsVisible((prevVisibility) => !prevVisibility);
-  }, []);
+  const handleReasonsVisibility = () => {
+    setReasonsVisible(!isReasonsVisible);
+  };
 
   const statusContent = nodeExecution ? (
     <div className={styles.statusContainer}>
@@ -321,26 +355,32 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
     <div className={styles.notRunStatus}>NOT RUN</div>
   );
 
-  const detailsContent = nodeExecution ? (
-    <>
-      <NodeExecutionCacheStatus taskNodeMetadata={nodeExecution.closure.taskNodeMetadata} />
-      <ExecutionTypeDetails details={details} execution={nodeExecution} />
-    </>
-  ) : null;
+  let detailsContent: JSX.Element | null = null;
+  if (nodeExecution) {
+    detailsContent = (
+      <>
+        <NodeExecutionCacheStatus taskNodeMetadata={nodeExecution.closure.taskNodeMetadata} />
+        <ExecutionTypeDetails details={details} execution={nodeExecution} />
+      </>
+    );
+  }
 
-  const tabsContent = nodeExecution ? (
-    <NodeExecutionTabs nodeExecution={nodeExecution} taskTemplate={details?.taskTemplate} />
+  const tabsContent: JSX.Element | null = nodeExecution ? (
+    <NodeExecutionTabs
+      nodeExecution={nodeExecution}
+      shouldShowTaskDetails={shouldShowTaskDetails}
+      phase={phase}
+      taskTemplate={details?.taskTemplate}
+    />
   ) : null;
+
+  const displayName = details?.displayName ?? <Skeleton />;
+
   return (
     <section className={styles.container}>
       <header className={styles.header}>
         <div className={styles.headerContent}>
-          <Typography className={classnames(commonStyles.textWrapped, styles.title)} variant="h3">
-            {nodeExecutionId.nodeId}
-            <IconButton className={styles.closeButton} onClick={onClose} size="small">
-              <Close />
-            </IconButton>
-          </Typography>
+          {headerTitle}
           <Typography
             className={classnames(commonStyles.textWrapped, styles.displayId)}
             variant="subtitle1"

diff --git a/packages/zapp/console/src/components/Executions/ExecutionDetails/NodeExecutionTabs/index.tsx b/packages/zapp/console/src/components/Executions/ExecutionDetails/NodeExecutionTabs/index.tsx
@@ -7,6 +7,7 @@ import { useTabState } from 'components/hooks/useTabState';
 import { PanelSection } from 'components/common/PanelSection';
 import { DumpJSON } from 'components/common/DumpJSON';
 import { isMapTaskType } from 'models/Task/utils';
+import { TaskExecutionPhase } from 'models/Execution/enums';
 import { TaskExecutionsList } from '../../TaskExecutionsList/TaskExecutionsList';
 import { NodeExecutionInputs } from './NodeExecutionInputs';
 import { NodeExecutionOutputs } from './NodeExecutionOutputs';
@@ -39,8 +40,10 @@ const defaultTab = tabIds.executions;
 
 export const NodeExecutionTabs: React.FC<{
   nodeExecution: NodeExecution;
+  shouldShowTaskDetails: boolean;
+  phase?: TaskExecutionPhase;
   taskTemplate?: TaskTemplate | null;
-}> = ({ nodeExecution, taskTemplate }) => {
+}> = ({ nodeExecution, shouldShowTaskDetails, taskTemplate, phase }) => {
   const styles = useStyles();
   const tabState = useTabState(tabIds, defaultTab);
 
@@ -55,7 +58,7 @@ export const NodeExecutionTabs: React.FC<{
   let tabContent: JSX.Element | null = null;
   switch (tabState.value) {
     case tabIds.executions: {
-      tabContent = <TaskExecutionsList nodeExecution={nodeExecution} />;
+      tabContent = <TaskExecutionsList nodeExecution={nodeExecution} phase={phase} />;
       break;
     }
     case tabIds.inputs: {
@@ -76,7 +79,12 @@ export const NodeExecutionTabs: React.FC<{
     }
   }
 
-  const executionLabel = isMapTaskType(taskTemplate?.type) ? 'Map Execution' : 'Executions';
+  const executionLabel = isMapTaskType(taskTemplate?.type)
+    ? shouldShowTaskDetails
+      ? 'Execution'
+      : 'Map Execution'
+    : 'Executions';
+
   return (
     <>
       <Tabs {...tabState} className={styles.tabs}>

diff --git a/.../console/src/components/Executions/ExecutionDetails/NodeExecutionTabs/test/index.test.tsx b/.../console/src/components/Executions/ExecutionDetails/NodeExecutionTabs/test/index.test.tsx
@@ -0,0 +1,61 @@
+import { render } from '@testing-library/react';
+import { useTabState } from 'components/hooks/useTabState';
+import { extractTaskTemplates } from 'components/hooks/utils';
+import { TaskExecutionPhase } from 'models/Execution/enums';
+import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData';
+import { TaskType } from 'models/Task/constants';
+import { createMockWorkflow } from 'models/__mocks__/workflowData';
+import * as React from 'react';
+import { NodeExecutionTabs } from '../index';
+
+const getMockNodeExecution = () => createMockNodeExecutions(1).executions[0];
+const nodeExecution = getMockNodeExecution();
+const workflow = createMockWorkflow('SampleWorkflow');
+const taskTemplate = { ...extractTaskTemplates(workflow)[0], type: TaskType.ARRAY };
+const phase = TaskExecutionPhase.SUCCEEDED;
+
+jest.mock('components/hooks/useTabState');
+
+describe('NodeExecutionTabs', () => {
+  const mockUseTabState = useTabState as jest.Mock<any>;
+  mockUseTabState.mockReturnValue({ onChange: jest.fn(), value: 'executions' });
+  describe('with map tasks', () => {
+    it('should display proper tab name when it was provided and shouldShow is TRUE', async () => {
+      const { queryByText, queryAllByRole } = render(
+        <NodeExecutionTabs
+          nodeExecution={nodeExecution}
+          shouldShowTaskDetails={true}
+          phase={phase}
+          taskTemplate={taskTemplate}
+        />,
+      );
+      expect(queryAllByRole('tab')).toHaveLength(4);
+      expect(queryByText('Execution')).toBeInTheDocument();
+    });
+
+    it('should display proper tab name when it was provided and shouldShow is FALSE', async () => {
+      const { queryByText, queryAllByRole } = render(
+        <NodeExecutionTabs
+          nodeExecution={nodeExecution}
+          shouldShowTaskDetails={false}
+          phase={phase}
+          taskTemplate={taskTemplate}
+        />,
+      );
+
+      expect(queryAllByRole('tab')).toHaveLength(4);
+      expect(queryByText('Map Execution')).toBeInTheDocument();
+    });
+  });
+
+  describe('without map tasks', () => {
+    it('should display proper tab name when mapTask was not provided', async () => {
+      const { queryAllByRole, queryByText } = render(
+        <NodeExecutionTabs nodeExecution={nodeExecution} shouldShowTaskDetails={false} />,
+      );
+
+      expect(queryAllByRole('tab')).toHaveLength(3);
+      expect(queryByText('Executions')).toBeInTheDocument();
+    });
+  });
+});