Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cleanup for dynamic tasks refactoring #76

Merged
merged 10 commits into from
Jun 29, 2020
39 changes: 28 additions & 11 deletions src/components/Executions/Tables/NodeExecutionChildren.tsx
Original file line number Diff line number Diff line change
@@ -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>
Expand Down
79 changes: 58 additions & 21 deletions src/components/Executions/Tables/NodeExecutionRow.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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);
Expand All @@ -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({
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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>
Expand Down
3 changes: 2 additions & 1 deletion src/components/Executions/Tables/NodeExecutionsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
Expand Down
12 changes: 11 additions & 1 deletion src/components/Executions/Tables/WorkflowExecutionsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const useStyles = makeStyles((theme: Theme) => ({
marginLeft: theme.spacing(2),
marginRight: theme.spacing(2),
textAlign: 'left'
},
row: {
paddingLeft: theme.spacing(2)
}
}));

Expand Down Expand Up @@ -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 }) => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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} />);
Expand Down
Loading