Skip to content

Commit

Permalink
fix(#278): wrong nodeExecutions details are showed in node executions…
Browse files Browse the repository at this point in the history
… list (#277)

* fix: wrong nodeExecutions names are shown in the table
* chore: tests updates
* chore: storybook update

Signed-off-by: Nastya Rusina <nastya@union.ai>
  • Loading branch information
anrusina authored Feb 11, 2022
1 parent b438626 commit 15878ab
Show file tree
Hide file tree
Showing 20 changed files with 690 additions and 529 deletions.
41 changes: 23 additions & 18 deletions src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { DataError } from 'components/Errors/DataError';
import { useTabState } from 'components/hooks/useTabState';
import { secondaryBackgroundColor } from 'components/Theme/constants';
import { Execution, NodeExecution } from 'models/Execution/types';
import { NodeExecutionDetailsContextProvider } from '../contextProvider/NodeExecutionDetails';
import { NodeExecutionsRequestConfigContext } from '../contexts';
import { ExecutionFilters } from '../ExecutionFilters';
import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState';
Expand Down Expand Up @@ -85,29 +86,33 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
<Tab value={tabs.nodes.id} label={tabs.nodes.label} />
<Tab value={tabs.graph.id} label={tabs.graph.label} />
</Tabs>
<div className={styles.nodesContainer}>
{tabState.value === tabs.nodes.id && (
<>
<div className={styles.filters}>
<ExecutionFilters {...filterState} />
</div>
<NodeExecutionDetailsContextProvider
workflowId={execution.closure.workflowId}
>
<div className={styles.nodesContainer}>
{tabState.value === tabs.nodes.id && (
<>
<div className={styles.filters}>
<ExecutionFilters {...filterState} />
</div>
<WaitForQuery
errorComponent={DataError}
query={nodeExecutionsQuery}
>
{renderNodeExecutionsTable}
</WaitForQuery>
</>
)}
{tabState.value === tabs.graph.id && (
<WaitForQuery
errorComponent={DataError}
query={nodeExecutionsQuery}
>
{renderNodeExecutionsTable}
{renderExecutionLoader}
</WaitForQuery>
</>
)}
{tabState.value === tabs.graph.id && (
<WaitForQuery
errorComponent={DataError}
query={nodeExecutionsQuery}
>
{renderExecutionLoader}
</WaitForQuery>
)}
</div>
)}
</div>
</NodeExecutionDetailsContextProvider>
</>
);
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react';
import { useEffect, useRef, useState } from 'react';
import { IconButton, Typography } from '@material-ui/core';
import { makeStyles, Theme } from '@material-ui/core/styles';
import Tab from '@material-ui/core/Tab';
Expand Down Expand Up @@ -32,7 +32,7 @@ import {
} from '../nodeExecutionQueries';
import { TaskExecutionsList } from '../TaskExecutionsList/TaskExecutionsList';
import { NodeExecutionDetails } from '../types';
import { useNodeExecutionDetails } from '../useNodeExecutionDetails';
import { useNodeExecutionContext } from '../contextProvider/NodeExecutionDetails';
import { NodeExecutionInputs } from './NodeExecutionInputs';
import { NodeExecutionOutputs } from './NodeExecutionOutputs';
import { NodeExecutionTaskDetails } from './NodeExecutionTaskDetails';
Expand Down Expand Up @@ -210,6 +210,14 @@ const NodeExecutionTabs: React.FC<{
const styles = useStyles();
const tabState = useTabState(tabIds, defaultTab);

if (tabState.value === tabIds.task && !taskTemplate) {
// Reset tab value, if task tab is selected, while no taskTemplate is avaible
// can happen when user switches between nodeExecutions without closing the drawer
tabState.onChange(() => {
/* */
}, defaultTab);
}

let tabContent: JSX.Element | null = null;
switch (tabState.value) {
case tabIds.executions: {
Expand Down Expand Up @@ -293,22 +301,43 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
nodeExecutionId,
onClose
}) => {
const [mounted, setMounted] = useState(true);
const isMounted = useRef(false);
useEffect(() => {
isMounted.current = true;
return () => {
setMounted(false);
isMounted.current = false;
};
}, []);

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(() => {
let isCurrent = true;
detailsContext.getNodeExecutionDetails(nodeExecution).then(res => {
if (isCurrent) {
setDetails(res);
}
});

return () => {
isCurrent = false;
};
});

React.useEffect(() => {
setReasonsVisible(false);
}, [nodeExecutionId]);
Expand All @@ -326,7 +355,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
);
if (workflowData) {
const keyedDag = transformWorkflowToKeyedDag(workflowData);
if (mounted) setDag(keyedDag);
if (isMounted.current) setDag(keyedDag);
}
};

Expand All @@ -347,15 +376,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp

const commonStyles = useCommonStyles();
const styles = useStyles();
const detailsQuery = useNodeExecutionDetails(nodeExecution);
const displayName = detailsQuery.data ? (
detailsQuery.data.displayName
) : (
<Skeleton />
);
const taskTemplate = detailsQuery.data
? detailsQuery.data.taskTemplate
: null;
const displayName = details?.displayName ?? <Skeleton />;

const isRunningPhase = React.useMemo(() => {
return (
Expand Down Expand Up @@ -400,17 +421,14 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
<NodeExecutionCacheStatus
taskNodeMetadata={nodeExecution.closure.taskNodeMetadata}
/>
<ExecutionTypeDetails
details={detailsQuery.data}
execution={nodeExecution}
/>
<ExecutionTypeDetails details={details} execution={nodeExecution} />
</>
) : null;

const tabsContent = nodeExecution ? (
<NodeExecutionTabs
nodeExecution={nodeExecution}
taskTemplate={taskTemplate}
taskTemplate={details?.taskTemplate}
/>
) : null;
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import {
cacheStatusMessages,
viewSourceExecutionString
} from 'components/Executions/constants';
import { NodeExecutionDetailsContextProvider } from 'components/Executions/contextProvider/NodeExecutionDetails';
import { Core } from 'flyteidl';
import { basicPythonWorkflow } from 'mocks/data/fixtures/basicPythonWorkflow';
import { mockWorkflowId } from 'mocks/data/fixtures/types';
import { insertFixture } from 'mocks/data/insertFixture';
import { mockServer } from 'mocks/server';
import { NodeExecution, TaskNodeMetadata } from 'models/Execution/types';
Expand All @@ -17,6 +19,9 @@ import { makeIdentifier } from 'test/modelUtils';
import { createTestQueryClient } from 'test/utils';
import { NodeExecutionDetailsPanelContent } from '../NodeExecutionDetailsPanelContent';

jest.mock('components/Workflow/workflowQueries');
const { fetchWorkflow } = require('components/Workflow/workflowQueries');

describe('NodeExecutionDetails', () => {
let fixture: ReturnType<typeof basicPythonWorkflow.generate>;
let execution: NodeExecution;
Expand All @@ -27,16 +32,23 @@ describe('NodeExecutionDetails', () => {
execution =
fixture.workflowExecutions.top.nodeExecutions.pythonNode.data;
insertFixture(mockServer, fixture);
fetchWorkflow.mockImplementation(() =>
Promise.resolve(fixture.workflows.top)
);
queryClient = createTestQueryClient();
});

const renderComponent = () =>
render(
<MemoryRouter>
<QueryClientProvider client={queryClient}>
<NodeExecutionDetailsPanelContent
nodeExecutionId={execution.id}
/>
<NodeExecutionDetailsContextProvider
workflowId={mockWorkflowId}
>
<NodeExecutionDetailsPanelContent
nodeExecutionId={execution.id}
/>
</NodeExecutionDetailsContextProvider>
</QueryClientProvider>
</MemoryRouter>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { makeStyles, Theme } from '@material-ui/core/styles';
import { storiesOf } from '@storybook/react';
import { NodeExecutionDetailsContext } from 'components/Executions/contextProvider/NodeExecutionDetails';
import { makeNodeExecutionListQuery } from 'components/Executions/nodeExecutionQueries';
import { basicPythonWorkflow } from 'mocks/data/fixtures/basicPythonWorkflow';
import * as React from 'react';
Expand All @@ -19,6 +20,14 @@ const useStyles = makeStyles((theme: Theme) => ({
const fixture = basicPythonWorkflow.generate();
const workflowExecution = fixture.workflowExecutions.top.data;

const getNodeExecutionDetails = async () => {
return {
displayId: 'node0',
displayName: 'basic.byton.workflow.unique.task_name',
displayType: 'Python-Task'
};
};

const stories = storiesOf('Tables/NodeExecutionsTable', module);
stories.addDecorator(story => {
return <div className={useStyles().container}>{story()}</div>;
Expand All @@ -28,9 +37,21 @@ stories.add('Basic', () => {
makeNodeExecutionListQuery(useQueryClient(), workflowExecution.id)
);
return query.data ? (
<NodeExecutionsTable nodeExecutions={query.data} />
<NodeExecutionDetailsContext.Provider
value={{ getNodeExecutionDetails }}
>
<NodeExecutionsTable nodeExecutions={query.data} />
</NodeExecutionDetailsContext.Provider>
) : (
<div />
);
});
stories.add('With no items', () => <NodeExecutionsTable nodeExecutions={[]} />);
stories.add('With no items', () => {
return (
<NodeExecutionDetailsContext.Provider
value={{ getNodeExecutionDetails }}
>
<NodeExecutionsTable nodeExecutions={[]} />
</NodeExecutionDetailsContext.Provider>
);
});
2 changes: 1 addition & 1 deletion src/components/Executions/Tables/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const workflowExecutionsTableColumnWidths = {

export const nodeExecutionsTableColumnWidths = {
duration: 100,
logs: 225,
logs: 100,
type: 144,
nodeId: 144,
name: 380,
Expand Down
Loading

0 comments on commit 15878ab

Please sign in to comment.