fix: cleanup for dynamic tasks refactoring (#76)

* test: creating dynamic task cases for NodeExecutionsTable stories

* fix: styling for child group labels

* fix: mock api context for NodeExecutionsTable stories

* test: mock nodeExecutionData endpoint

* chore: remove unused imports

* fix: extract nodes from subworkflows as well

* fix: adjust borders to make child groups more obvious

* refactor: checkpoint for getting the nesting styles correct

* refactor: adding logic for borders/spacing based on nesting/index

* fix: correct workflow execution table row styles

src/components/Executions/Tables/NodeExecutionChildren.tsx

@@ -1,45 +1,62 @@
 import { Typography } from '@material-ui/core';
 import * as classnames from 'classnames';
-import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText';
+import { useTheme } from 'components/Theme/useTheme';
 import * as React from 'react';
 import { DetailedNodeExecutionGroup } from '../types';
 import { NodeExecutionRow } from './NodeExecutionRow';
 import { useExecutionTableStyles } from './styles';
+import { calculateNodeExecutionRowLeftSpacing } from './utils';
 
 export interface NodeExecutionChildrenProps {
     childGroups: DetailedNodeExecutionGroup[];
+    level: number;
 }
 
 /** Renders a nested list of row items for children of a NodeExecution */
 export const NodeExecutionChildren: React.FC<NodeExecutionChildrenProps> = ({
-    childGroups
+    childGroups,
+    level
 }) => {
     const showNames = childGroups.length > 1;
     const tableStyles = useExecutionTableStyles();
-    const monospaceTextStyles = useExpandableMonospaceTextStyles();
+    const theme = useTheme();
+    const childGroupLabelStyle = {
+        // The label is aligned with the parent above, so remove one level of spacing
+        marginLeft: `${calculateNodeExecutionRowLeftSpacing(
+            level - 1,
+            theme.spacing
+        )}px`
+    };
     return (
         <>
-            {childGroups.map(({ name, nodeExecutions }) => {
-                const rows = nodeExecutions.map(nodeExecution => (
+            {childGroups.map(({ name, nodeExecutions }, groupIndex) => {
+                const rows = nodeExecutions.map((nodeExecution, index) => (
                     <NodeExecutionRow
                         key={nodeExecution.cacheKey}
+                        index={index}
                         execution={nodeExecution}
+                        level={level}
                     />
                 ));
                 const key = `group-${name}`;
                 return showNames ? (
                     <div key={key}>
-                        <div className={tableStyles.row}>
-                            <Typography variant="overline">{name}</Typography>
-                        </div>
                         <div
                             className={classnames(
-                                tableStyles.childrenContainer,
-                                monospaceTextStyles.nestedParent
+                                { [tableStyles.borderTop]: groupIndex > 0 },
+                                tableStyles.borderBottom,
+                                tableStyles.childGroupLabel
                             )}
+                            style={childGroupLabelStyle}
                         >
-                            {rows}
+                            <Typography
+                                variant="overline"
+                                color="textSecondary"
+                            >
+                                {name}
+                            </Typography>
                         </div>
+                        <div>{rows}</div>
                     </div>
                 ) : (
                     <div key={key}>{rows}</div>

src/components/Executions/Tables/NodeExecutionRow.tsx

@@ -1,5 +1,5 @@
 import * as classnames from 'classnames';
-import { useExpandableMonospaceTextStyles } from 'components/common/ExpandableMonospaceText';
+import { useTheme } from 'components/Theme/useTheme';
 import * as React from 'react';
 import {
     ExecutionContext,
@@ -13,17 +13,23 @@ import { ExpandableExecutionError } from './ExpandableExecutionError';
 import { NodeExecutionChildren } from './NodeExecutionChildren';
 import { RowExpander } from './RowExpander';
 import { selectedClassName, useExecutionTableStyles } from './styles';
+import { calculateNodeExecutionRowLeftSpacing } from './utils';
 
 interface NodeExecutionRowProps {
+    index: number;
     execution: DetailedNodeExecution;
+    level?: number;
     style?: React.CSSProperties;
 }
 
 /** Renders a NodeExecution as a row inside a `NodeExecutionsTable` */
 export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
     execution: nodeExecution,
+    index,
+    level = 0,
     style
 }) => {
+    const theme = useTheme();
     const { columns, state } = React.useContext(NodeExecutionsTableContext);
     const requestConfig = React.useContext(NodeExecutionsRequestConfigContext);
     const { execution: workflowExecution } = React.useContext(ExecutionContext);
@@ -33,6 +39,17 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
         setExpanded(!expanded);
     };
 
+    // For the first level, we want the borders to span the entire table,
+    // so we'll use padding to space the content. For nested rows, we want the
+    // border to start where the content does, so we'll use margin.
+    const spacingProp = level === 0 ? 'paddingLeft' : 'marginLeft';
+    const rowContentStyle = {
+        [spacingProp]: `${calculateNodeExecutionRowLeftSpacing(
+            level,
+            theme.spacing
+        )}px`
+    };
+
     // TODO: Handle error case for loading children.
     // Maybe show an expander in that case and make the content the error?
     const childNodeExecutions = useChildNodeExecutions({
@@ -47,7 +64,6 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
 
     const isExpandable = detailedChildNodeExecutions.length > 0;
     const tableStyles = useExecutionTableStyles();
-    const monospaceTextStyles = useExpandableMonospaceTextStyles();
 
     const selected = state.selectedExecution
         ? state.selectedExecution === nodeExecution
@@ -64,17 +80,16 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
 
     const extraContent = expanded ? (
         <div
-            className={classnames(
-                tableStyles.childrenContainer,
-                monospaceTextStyles.nestedParent
-            )}
+            className={classnames(tableStyles.childrenContainer, {
+                [tableStyles.borderBottom]: level === 0
+            })}
         >
-            {errorContent}
-            <NodeExecutionChildren childGroups={detailedChildNodeExecutions} />
+            <NodeExecutionChildren
+                childGroups={detailedChildNodeExecutions}
+                level={level + 1}
+            />
         </div>
-    ) : (
-        errorContent
-    );
+    ) : null;
 
     return (
         <div
@@ -83,19 +98,41 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
             })}
             style={style}
         >
-            <div className={tableStyles.rowColumns}>
-                <div className={tableStyles.expander}>{expanderContent}</div>
-                {columns.map(({ className, key: columnKey, cellRenderer }) => (
+            <div
+                className={classnames(tableStyles.rowContent, {
+                    [tableStyles.borderBottom]:
+                        level === 0 || (level > 0 && expanded),
+                    [tableStyles.borderTop]: level > 0 && index > 0
+                })}
+                style={rowContentStyle}
+            >
+                <div className={tableStyles.rowColumns}>
                     <div
-                        key={columnKey}
-                        className={classnames(tableStyles.rowColumn, className)}
+                        className={classnames(
+                            tableStyles.rowColumn,
+                            tableStyles.expander
+                        )}
                     >
-                        {cellRenderer({
-                            state,
-                            execution: nodeExecution
-                        })}
+                        {expanderContent}
                     </div>
-                ))}
+                    {columns.map(
+                        ({ className, key: columnKey, cellRenderer }) => (
+                            <div
+                                key={columnKey}
+                                className={classnames(
+                                    tableStyles.rowColumn,
+                                    className
+                                )}
+                            >
+                                {cellRenderer({
+                                    state,
+                                    execution: nodeExecution
+                                })}
+                            </div>
+                        )
+                    )}
+                </div>
+                {errorContent}
             </div>
             {extraContent}
         </div>

src/components/Executions/Tables/NodeExecutionsTable.tsx

@@ -38,10 +38,11 @@ export const NodeExecutionsTable: React.FC<NodeExecutionsTableProps> = props =>
     const rowProps = { state, onHeightChange: () => {} };
     const content =
         state.executions.length > 0 ? (
-            state.executions.map(execution => {
+            state.executions.map((execution, index) => {
                 return (
                     <NodeExecutionRow
                         {...rowProps}
+                        index={index}
                         key={execution.cacheKey}
                         execution={execution}
                     />

src/components/Executions/Tables/WorkflowExecutionsTable.tsx

@@ -53,6 +53,9 @@ const useStyles = makeStyles((theme: Theme) => ({
         marginLeft: theme.spacing(2),
         marginRight: theme.spacing(2),
         textAlign: 'left'
+    },
+    row: {
+        paddingLeft: theme.spacing(2)
     }
 }));
 
@@ -199,7 +202,14 @@ export const WorkflowExecutionsTable: React.FC<WorkflowExecutionsTableProps> = p
         const { error } = execution.closure;
 
         return (
-            <div className={classnames(tableStyles.row)} style={style}>
+            <div
+                className={classnames(
+                    tableStyles.row,
+                    styles.row,
+                    tableStyles.borderBottom
+                )}
+                style={style}
+            >
                 <div className={tableStyles.rowColumns}>
                     {columns.map(
                         ({ className, key: columnKey, cellRenderer }) => (

src/components/Executions/Tables/__stories__/NodeExecutionsTable.stories.tsx

@@ -2,15 +2,12 @@ import { makeStyles, Theme } from '@material-ui/core/styles';
 import { action } from '@storybook/addon-actions';
 import { storiesOf } from '@storybook/react';
 import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
+import { APIContext } from 'components/data/apiContext';
+import { createMockExecutionEntities } from 'components/Executions/__mocks__/createMockExecutionEntities';
 import { ExecutionDataCacheContext } from 'components/Executions/contexts';
 import { createExecutionDataCache } from 'components/Executions/useExecutionDataCache';
-import {
-    createMockWorkflow,
-    createMockWorkflowClosure
-} from 'models/__mocks__/workflowData';
-import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData';
-import { Execution } from 'models/Execution/types';
-import { mockTasks } from 'models/Task/__mocks__/mockTaskData';
+import { keyBy } from 'lodash';
+import { createMockTaskExecutionForNodeExecution } from 'models/Execution/__mocks__/mockTaskExecutionsData';
 import * as React from 'react';
 import {
     NodeExecutionsTable,
@@ -28,33 +25,59 @@ const useStyles = makeStyles((theme: Theme) => ({
 }));
 
 const {
-    executions: mockExecutions,
-    nodes: mockNodes
-} = createMockNodeExecutions(10);
+    nodes,
+    nodeExecutions,
+    workflow,
+    workflowExecution
+} = createMockExecutionEntities({
+    workflowName: 'SampleWorkflow',
+    nodeExecutionCount: 10
+});
 
-const mockWorkflow = createMockWorkflow('SampleWorkflow');
-const mockWorkflowClosure = createMockWorkflowClosure();
-const compiledWorkflow = mockWorkflowClosure.compiledWorkflow!;
-const {
-    primary: { template },
-    tasks
-} = compiledWorkflow;
-template.nodes = template.nodes.concat(mockNodes);
-compiledWorkflow.tasks = tasks.concat(mockTasks);
-mockWorkflow.closure = mockWorkflowClosure;
+const nodesById = keyBy(nodes, n => n.id);
+const nodesWithChildren = {
+    [nodes[0].id]: true,
+    [nodes[1].id]: true
+};
+const nodeRetryAttempts = {
+    [nodes[1].id]: 2
+};
 
-const apiContext = mockAPIContextValue({});
+const apiContext = mockAPIContextValue({
+    getExecution: () => Promise.resolve(workflowExecution),
+    getNodeExecutionData: () => Promise.resolve({ inputs: {}, outputs: {} }),
+    listTaskExecutions: nodeExecutionId => {
+        const length = nodeRetryAttempts[nodeExecutionId.nodeId] || 1;
+        const entities = Array.from({ length }, (_, retryAttempt) =>
+            createMockTaskExecutionForNodeExecution(
+                nodeExecutionId,
+                nodesById[nodeExecutionId.nodeId],
+                retryAttempt,
+                { isParent: !!nodesWithChildren[nodeExecutionId.nodeId] }
+            )
+        );
+        return Promise.resolve({ entities });
+    },
+    listTaskExecutionChildren: ({ retryAttempt }) =>
+        Promise.resolve({
+            entities: nodeExecutions.slice(0, 2).map(ne => ({
+                ...ne,
+                id: {
+                    ...ne.id,
+                    nodeId: `${ne.id.nodeId}_${retryAttempt}`
+                }
+            }))
+        }),
+    getWorkflow: () => Promise.resolve(workflow)
+});
 const dataCache = createExecutionDataCache(apiContext);
-dataCache.insertWorkflow(mockWorkflow);
-dataCache.insertWorkflowExecutionReference(
-    mockExecutions[0].id.executionId,
-    mockWorkflow.id
-);
+dataCache.insertWorkflow(workflow);
+dataCache.insertWorkflowExecutionReference(workflowExecution.id, workflow.id);
 
 const fetchAction = action('fetch');
 
 const props: NodeExecutionsTableProps = {
-    value: mockExecutions,
+    value: nodeExecutions,
     lastError: null,
     loading: false,
     moreItemsAvailable: false,
@@ -64,9 +87,11 @@ const props: NodeExecutionsTableProps = {
 const stories = storiesOf('Tables/NodeExecutionsTable', module);
 stories.addDecorator(story => {
     return (
-        <ExecutionDataCacheContext.Provider value={dataCache}>
-            <div className={useStyles().container}>{story()}</div>
-        </ExecutionDataCacheContext.Provider>
+        <APIContext.Provider value={apiContext}>
+            <ExecutionDataCacheContext.Provider value={dataCache}>
+                <div className={useStyles().container}>{story()}</div>
+            </ExecutionDataCacheContext.Provider>
+        </APIContext.Provider>
     );
 });
 stories.add('Basic', () => <NodeExecutionsTable {...props} />);