feat: improve user experience for nested NodeExecutions (#77)

* feat: adds more identifying information for node executions (#70)

* feat: show workflow node name beneath node execution ids

* feat: updating DetailsPanel info for NodeExecutions

* fix lint errors

* adding tests for new formatter function

* test: adding new test for NodeExecutionDetails

* test: adding test for new code in NodeExecutionsTable

* refactor: fetch child node executions for NodeExecution rows (#72)

* refactor: fetch child node executions to determine expandability

* fix: handling detection of children for sub-workflows as well

* fix: poor performance with object-hash on some identifiers

* docs: cleanup and docs for newly exposed functions

* test: ensure request config is used for all levels

* chore: remove unused import

* feat: Remove intermediate NodeExecutionsTable row content (#75)

* refactor: removing specialized rows and rendering only nodes

* refactor: moving contexts up to common folder

* refactor: use a data cache for nested node mapping

* refactor: update loading of workflow data

* fix: update usage of NodeExecutions in graph tab

* fix: update TaskExecutionDetails to use data cache

* fix: getting tests and stories working again

* chore: docs and cleanup

* test: use a more robust element query

* refactor: use filter instead of reduce

* docs: adding some missing function docs

* 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/common/formatters.ts

@@ -144,3 +144,12 @@ export function ensureUrlWithProtocol(url: string): string {
     }
     return url;
 }
+
+/** Formats a number into a string with leading zeros to ensure a consistent
+ * width.
+ * Example: 1 will be '01'
+ *          10 will be '10'
+ */
+export function leftPaddedNumber(value: number, length: number): string {
+    return value.toString().padStart(length, '0');
+}

src/common/test/formatters.spec.ts

@@ -1,5 +1,4 @@
 import { millisecondsToDuration } from 'common/utils';
-import { Admin } from 'flyteidl';
 import {
     subSecondString,
     unknownValueString,
@@ -10,10 +9,10 @@ import {
     dateFromNow,
     dateWithFromNow,
     ensureUrlWithProtocol,
-    fixedRateToString,
     formatDate,
     formatDateLocalTimezone,
     formatDateUTC,
+    leftPaddedNumber,
     millisecondsToHMS,
     protobufDurationToHMS
 } from '../formatters';
@@ -177,3 +176,24 @@ describe('ensureUrlWithProtocol', () => {
         })
     );
 });
+
+describe('leftPaddedNumber', () => {
+    const cases: [number, number, string][] = [
+        [1, 0, '1'],
+        [10, 0, '10'],
+        [0, 1, '0'],
+        [0, 2, '00'],
+        [1, 1, '1'],
+        [1, 2, '01'],
+        [1, 3, '001'],
+        [10, 1, '10'],
+        [10, 2, '10'],
+        [10, 3, '010']
+    ];
+
+    cases.forEach(([value, width, expected]) =>
+        it(`should produce ${expected} with input (${value}, ${width})`, () => {
+            expect(leftPaddedNumber(value, width)).toEqual(expected);
+        })
+    );
+});

src/components/Cache/createCache.ts

@@ -14,9 +14,9 @@ function hasId(value: Object): value is HasIdObject {
  * strictly equal (===). The cache provides methods for getting/setting values
  * by key and for merging values or arrays of values with any existing items.
  */
-export interface ValueCache {
+export interface ValueCache<ValueType = object> {
     /** Retrieve an item by key */
-    get(key: EntityKey): object | undefined;
+    get(key: EntityKey): ValueType | undefined;
     /** Check existence of an item by key */
     has(key: EntityKey): boolean;
     /** Merge an array of values. If the items have an `id` property, its value
@@ -30,9 +30,9 @@ export interface ValueCache {
      * performed. For arrays, the value is _replaced_.
      * @returns The merged value
      */
-    mergeValue(key: EntityKey, value: object): object;
+    mergeValue(key: EntityKey, value: ValueType): ValueType;
     /** Set an item value by key. Replaces any existing value. */
-    set(key: EntityKey, value: object): object;
+    set(key: EntityKey, value: ValueType): ValueType;
 }
 
 type Cacheable = object | any[];

src/components/Cache/utils.ts

@@ -6,5 +6,6 @@ import * as objectHash from 'object-hash';
 export function getCacheKey(id: any[] | object | string) {
     return typeof id === 'string' || typeof id === 'symbol'
         ? id
-        : objectHash(id);
+        : // We only want to compare own properties, not .__proto__, .constructor, etc.
+          objectHash(id, { respectType: false });
 }

src/components/Executions/ExecutionDetails/ExecutionDetails.tsx

@@ -1,14 +1,12 @@
 import { WaitForData, withRouteParams } from 'components/common';
-import {
-    RefreshConfig,
-    useDataRefresher,
-    useWorkflowExecution
-} from 'components/hooks';
+import { RefreshConfig, useDataRefresher } from 'components/hooks';
 import { Execution } from 'models';
 import * as React from 'react';
 import { executionRefreshIntervalMs } from '../constants';
+import { ExecutionContext, ExecutionDataCacheContext } from '../contexts';
+import { useExecutionDataCache } from '../useExecutionDataCache';
+import { useWorkflowExecution } from '../useWorkflowExecution';
 import { executionIsTerminal } from '../utils';
-import { ExecutionContext } from './contexts';
 import { ExecutionDetailsAppBarContent } from './ExecutionDetailsAppBarContent';
 import { ExecutionNodeViews } from './ExecutionNodeViews';
 
@@ -35,15 +33,23 @@ export const ExecutionDetailsContainer: React.FC<ExecutionDetailsRouteParams> =
         domain: domainId,
         name: executionId
     };
-
-    const { fetchable, terminateExecution } = useWorkflowExecution(id);
+    const dataCache = useExecutionDataCache();
+    const { fetchable, terminateExecution } = useWorkflowExecution(
+        id,
+        dataCache
+    );
     useDataRefresher(id, fetchable, executionRefreshConfig);
-    const contextValue = { terminateExecution, execution: fetchable.value };
+    const contextValue = {
+        terminateExecution,
+        execution: fetchable.value
+    };
     return (
         <WaitForData {...fetchable}>
             <ExecutionContext.Provider value={contextValue}>
                 <ExecutionDetailsAppBarContent execution={fetchable.value} />
-                <ExecutionNodeViews execution={fetchable.value} />
+                <ExecutionDataCacheContext.Provider value={dataCache}>
+                    <ExecutionNodeViews execution={fetchable.value} />
+                </ExecutionDataCacheContext.Provider>
             </ExecutionContext.Provider>
         </WaitForData>
     );

src/components/Executions/ExecutionDetails/ExecutionNodeDetails/InputNodeDetails.tsx

@@ -6,7 +6,7 @@ import { NodeDetailsProps } from 'components/WorkflowGraph';
 import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles';
 
 import { LiteralMapViewer } from 'components/Literals';
-import { ExecutionContext } from '../contexts';
+import { ExecutionContext } from '../../contexts';
 
 /** Details panel renderer for the start/input node in a graph. Displays the
  * top level `WorkflowExecution` inputs.

src/components/Executions/ExecutionDetails/ExecutionNodeDetails/OutputNodeDetails.tsx

@@ -1,12 +1,12 @@
 import { SectionHeader, WaitForData } from 'components/common';
 import { useCommonStyles } from 'components/common/styles';
-import { useWorkflowExecutionData } from 'components/hooks';
 import { LiteralMapViewer, RemoteLiteralMapViewer } from 'components/Literals';
 import { NodeDetailsProps } from 'components/WorkflowGraph';
 import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles';
 import { emptyLiteralMapBlob, Execution } from 'models';
 import * as React from 'react';
-import { ExecutionContext } from '../contexts';
+import { ExecutionContext } from '../../contexts';
+import { useWorkflowExecutionData } from '../../useWorkflowExecution';
 
 const RemoteExecutionOutputs: React.FC<{ execution: Execution }> = ({
     execution

src/components/Executions/ExecutionDetails/ExecutionNodeDetails/TaskExecutionNodeDetails.tsx

@@ -14,7 +14,7 @@ import {
     TaskNodeDetails
 } from 'components/WorkflowGraph';
 import { useStyles as useBaseStyles } from 'components/WorkflowGraph/NodeDetails/styles';
-import { NodeExecutionsContext } from '../contexts';
+import { NodeExecutionsContext } from '../../contexts';
 import { NodeExecutionData } from '../NodeExecutionData';
 import { NodeExecutionInputs } from '../NodeExecutionInputs';
 import { NodeExecutionOutputs } from '../NodeExecutionOutputs';

src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx

@@ -4,6 +4,7 @@ import { WaitForData } from 'components/common';
 import { useTabState } from 'components/hooks/useTabState';
 import { Execution } from 'models';
 import * as React from 'react';
+import { NodeExecutionsRequestConfigContext } from '../contexts';
 import { ExecutionFilters } from '../ExecutionFilters';
 import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState';
 import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable';
@@ -43,10 +44,11 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
     const filterState = useNodeExecutionFiltersState();
     const tabState = useTabState(tabIds, tabIds.nodes);
 
-    const { workflow, nodeExecutions } = useWorkflowExecutionState(
-        execution,
-        filterState.appliedFilters
-    );
+    const {
+        workflow,
+        nodeExecutions,
+        nodeExecutionsRequestConfig
+    } = useWorkflowExecutionState(execution, filterState.appliedFilters);
 
     return (
         <WaitForData {...workflow}>
@@ -61,10 +63,14 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
                             <ExecutionFilters {...filterState} />
                         </div>
                         <WaitForData {...nodeExecutions}>
-                            <NodeExecutionsTable
-                                {...nodeExecutions}
-                                moreItemsAvailable={false}
-                            />
+                            <NodeExecutionsRequestConfigContext.Provider
+                                value={nodeExecutionsRequestConfig}
+                            >
+                                <NodeExecutionsTable
+                                    {...nodeExecutions}
+                                    moreItemsAvailable={false}
+                                />
+                            </NodeExecutionsRequestConfigContext.Provider>
                         </WaitForData>
                     </>
                 )}

src/components/Executions/ExecutionDetails/ExecutionWorkflowGraph.tsx

@@ -1,16 +1,16 @@
 import { DetailsPanel } from 'components/common';
 import { WorkflowGraph } from 'components/WorkflowGraph';
 import { keyBy } from 'lodash';
-import { endNodeId, startNodeId } from 'models';
+import { endNodeId, NodeExecution, startNodeId } from 'models';
 import { Workflow } from 'models/Workflow';
 import * as React from 'react';
-import { DetailedNodeExecution } from '../types';
-import { NodeExecutionsContext } from './contexts';
+import { NodeExecutionsContext } from '../contexts';
+import { useDetailedNodeExecutions } from '../useDetailedNodeExecutions';
 import { NodeExecutionDetails } from './NodeExecutionDetails';
 import { TaskExecutionNodeRenderer } from './TaskExecutionNodeRenderer/TaskExecutionNodeRenderer';
 
 export interface ExecutionWorkflowGraphProps {
-    nodeExecutions: DetailedNodeExecution[];
+    nodeExecutions: NodeExecution[];
     workflow: Workflow;
 }
 
@@ -19,9 +19,10 @@ export const ExecutionWorkflowGraph: React.FC<ExecutionWorkflowGraphProps> = ({
     nodeExecutions,
     workflow
 }) => {
+    const detailedNodeExecutions = useDetailedNodeExecutions(nodeExecutions);
     const nodeExecutionsById = React.useMemo(
-        () => keyBy(nodeExecutions, 'id.nodeId'),
-        [nodeExecutions]
+        () => keyBy(detailedNodeExecutions, 'id.nodeId'),
+        [detailedNodeExecutions]
     );
     const [selectedNodes, setSelectedNodes] = React.useState<string[]>([]);
     const onNodeSelectionChanged = (newSelection: string[]) => {

src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx

@@ -35,6 +35,9 @@ const useStyles = makeStyles((theme: Theme) => {
         content: {
             overflowY: 'auto'
         },
+        displayId: {
+            marginBottom: theme.spacing(1)
+        },
         header: {
             borderBottom: `${theme.spacing(1)}px solid ${theme.palette.divider}`
         },
@@ -60,8 +63,7 @@ const useStyles = makeStyles((theme: Theme) => {
         title: {
             alignItems: 'flex-start',
             display: 'flex',
-            justifyContent: 'space-between',
-            marginBottom: theme.spacing(1)
+            justifyContent: 'space-between'
         }
     };
 });
@@ -189,6 +191,16 @@ export const NodeExecutionDetails: React.FC<NodeExecutionDetailsProps> = ({
                             <Close />
                         </IconButton>
                     </Typography>
+                    <Typography
+                        className={classnames(
+                            commonStyles.textWrapped,
+                            styles.displayId
+                        )}
+                        variant="subtitle1"
+                        color="textSecondary"
+                    >
+                        {execution.displayId}
+                    </Typography>
                     {statusContent}
                     <ExecutionTypeDetails execution={execution} />
                 </div>

src/components/Executions/ExecutionDetails/TaskExecutionNodeRenderer/TaskExecutionNode.tsx

@@ -6,7 +6,7 @@ import { TaskNodeRenderer } from 'components/WorkflowGraph/TaskNodeRenderer';
 import { NodeExecutionPhase } from 'models/Execution/enums';
 import { DAGNode } from 'models/Graph';
 
-import { NodeExecutionsContext } from '../contexts';
+import { NodeExecutionsContext } from '../../contexts';
 import { StatusIndicator } from './StatusIndicator';
 
 /** Renders DAGNodes with colors based on their node type, as well as dots to

src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx

@@ -0,0 +1,45 @@
+import { render, waitFor } from '@testing-library/react';
+import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
+import { APIContext } from 'components/data/apiContext';
+import {
+    DetailedNodeExecution,
+    NodeExecutionDisplayType
+} from 'components/Executions/types';
+import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData';
+import { listTaskExecutions } from 'models/Execution/api';
+import * as React from 'react';
+import { NodeExecutionDetails } from '../NodeExecutionDetails';
+
+describe('NodeExecutionDetails', () => {
+    let execution: DetailedNodeExecution;
+    let mockListTaskExecutions: jest.Mock<ReturnType<
+        typeof listTaskExecutions
+    >>;
+    beforeEach(() => {
+        const { executions } = createMockNodeExecutions(1);
+        execution = {
+            ...executions[0],
+            displayType: NodeExecutionDisplayType.PythonTask,
+            displayId: 'com.flyte.testTask',
+            cacheKey: 'abcdefg'
+        };
+        mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] });
+    });
+
+    const renderComponent = () =>
+        render(
+            <APIContext.Provider
+                value={mockAPIContextValue({
+                    listTaskExecutions: mockListTaskExecutions
+                })}
+            >
+                <NodeExecutionDetails execution={execution} />
+            </APIContext.Provider>
+        );
+
+    it('renders displayId', async () => {
+        const { queryByText } = renderComponent();
+        await waitFor(() => {});
+        expect(queryByText(execution.displayId)).toBeInTheDocument();
+    });
+});

src/components/Executions/ExecutionInputsOutputsModal.tsx

@@ -2,10 +2,10 @@ import { Dialog, DialogContent, Tab, Tabs } from '@material-ui/core';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import { WaitForData } from 'components/common';
 import { ClosableDialogTitle } from 'components/common/ClosableDialogTitle';
-import { useWorkflowExecutionData } from 'components/hooks';
 import { LiteralMapViewer, RemoteLiteralMapViewer } from 'components/Literals';
 import { emptyLiteralMapBlob, Execution } from 'models';
 import * as React from 'react';
+import { useWorkflowExecutionData } from './useWorkflowExecution';
 
 const useStyles = makeStyles((theme: Theme) => ({
     content: {