From 12c486f86b2e06a81ed3415f6e60e0bf874a0b88 Mon Sep 17 00:00:00 2001
From: Randy Schott <1815175+schottra@users.noreply.github.com>
Date: Fri, 6 Nov 2020 10:53:31 -0800
Subject: [PATCH] fix: stale selected item reference in NE details panel (#115)

* fix: stale selected item reference in NE details panel

* refactor: syntax cleanup
---
 .../Tables/test/NodeExecutionsTable.test.tsx  | 58 ++++++++++++++-----
 .../Tables/useNodeExecutionsTableState.ts     | 21 +++++--
 src/components/common/DetailsPanel.tsx        |  1 +
 3 files changed, 62 insertions(+), 18 deletions(-)

diff --git a/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx
index 9467f188f..27d34de29 100644
--- 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
index 579d44abb..62ee6010b 100644
--- 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
index bc00d1b85..3f9f41258 100644
--- 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,