fix: stale selected item reference in NE details panel (#115)

* fix: stale selected item reference in NE details panel * refactor: syntax cleanup
flyteorg · Nov 6, 2020 · 12c486f · 12c486f
1 parent a3a6e2f
commit 12c486f
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 18 deletions.
diff --git a/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
@@ -1,8 +1,10 @@
 import {
     fireEvent,
     getAllByRole,
+    getByText,
     getByTitle,
     render,
+    screen,
     waitFor
 } from '@testing-library/react';
 import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
@@ -45,6 +47,7 @@ import {
     listTaskExecutionChildren,
     listTaskExecutions
 } from 'models/Execution/api';
+import { NodeExecutionPhase } from 'models/Execution/enums';
 import { mockTasks } from 'models/Task/__mocks__/mockTaskData';
 import * as React from 'react';
 import { makeIdentifier } from 'test/modelUtils';
@@ -137,20 +140,19 @@ describe('NodeExecutionsTable', () => {
         };
     });
 
-    const renderTable = () =>
-        render(
-            <APIContext.Provider value={apiContext}>
-                <NodeExecutionsRequestConfigContext.Provider
-                    value={requestConfig}
-                >
-                    <ExecutionContext.Provider value={executionContext}>
-                        <ExecutionDataCacheContext.Provider value={dataCache}>
-                            <NodeExecutionsTable {...props} />
-                        </ExecutionDataCacheContext.Provider>
-                    </ExecutionContext.Provider>
-                </NodeExecutionsRequestConfigContext.Provider>
-            </APIContext.Provider>
-        );
+    const Table = (props: NodeExecutionsTableProps) => (
+        <APIContext.Provider value={apiContext}>
+            <NodeExecutionsRequestConfigContext.Provider value={requestConfig}>
+                <ExecutionContext.Provider value={executionContext}>
+                    <ExecutionDataCacheContext.Provider value={dataCache}>
+                        <NodeExecutionsTable {...props} />
+                    </ExecutionDataCacheContext.Provider>
+                </ExecutionContext.Provider>
+            </NodeExecutionsRequestConfigContext.Provider>
+        </APIContext.Provider>
+    );
+
+    const renderTable = () => render(<Table {...props} />);
 
     it('renders task name for task nodes', async () => {
         const { queryAllByText } = renderTable();
@@ -413,4 +415,32 @@ describe('NodeExecutionsTable', () => {
             })
         );
     });
+
+    describe('when rendering the DetailsPanel', () => {
+        const selectFirstNode = async (container: HTMLElement) => {
+            const { nodeId } = mockNodeExecutions[0].id;
+            const nodeNameAnchor = await waitFor(() =>
+                getByText(container, nodeId)
+            );
+            fireEvent.click(nodeNameAnchor);
+            // Wait for Details Panel to render and then for the nodeId header
+            const detailsPanel = await waitFor(() =>
+                screen.getByTestId('details-panel')
+            );
+            await waitFor(() => getByText(detailsPanel, nodeId));
+            return detailsPanel;
+        };
+        it('should render updated state if selected nodeExecution object changes', async () => {
+            mockNodeExecutions[0].closure.phase = NodeExecutionPhase.RUNNING;
+            // Render table, click first node
+            const { container, rerender } = renderTable();
+            const detailsPanel = await selectFirstNode(container);
+            await waitFor(() => getByText(detailsPanel, 'Running'));
+
+            props.value = cloneDeep(mockNodeExecutions);
+            props.value[0].closure.phase = NodeExecutionPhase.FAILED;
+            rerender(<Table {...props} />);
+            await waitFor(() => getByText(detailsPanel, 'Failed'));
+        });
+    });
 });
diff --git a/src/components/Executions/Tables/useNodeExecutionsTableState.ts b/src/components/Executions/Tables/useNodeExecutionsTableState.ts
@@ -14,10 +14,23 @@ export function useNodeExecutionsTableState({
         [value]
     );
 
-    const [
-        selectedExecution,
-        setSelectedExecution
-    ] = useState<DetailedNodeExecution | null>(null);
+    const [selectedExecutionKey, setSelectedExecutionKey] = useState<
+        string | null
+    >(null);
+
+    const selectedExecution = useMemo(
+        () =>
+            executions.find(
+                ({ cacheKey }) => cacheKey === selectedExecutionKey
+            ) || null,
+        [executions, selectedExecutionKey]
+    );
+
+    const setSelectedExecution = useMemo(
+        () => (newValue: DetailedNodeExecution | null) =>
+            setSelectedExecutionKey(newValue?.cacheKey ?? null),
+        [setSelectedExecutionKey]
+    );
 
     return {
         executions,

diff --git a/src/components/common/DetailsPanel.tsx b/src/components/common/DetailsPanel.tsx
@@ -41,6 +41,7 @@ export const DetailsPanel: React.FC<DetailsPanelProps> = ({
     return (
         <Drawer
             anchor="right"
+            data-testid="details-panel"
             ModalProps={{
                 className: styles.modal,
                 hideBackdrop: true,