fix: only use NE filter for Nodes tab (#88)

src/components/Executions/ExecutionDetails/ExecutionNodeViews.tsx

@@ -10,6 +10,7 @@ import { ExecutionFilters } from '../ExecutionFilters';
 import { useNodeExecutionFiltersState } from '../filters/useExecutionFiltersState';
 import { NodeExecutionsTable } from '../Tables/NodeExecutionsTable';
 import { useWorkflowExecutionState } from '../useWorkflowExecutionState';
+import { tabs } from './constants';
 import { ExecutionWorkflowGraph } from './ExecutionWorkflowGraph';
 
 const useStyles = makeStyles((theme: Theme) => ({
@@ -29,37 +30,38 @@ const useStyles = makeStyles((theme: Theme) => ({
     }
 }));
 
-interface ExecutionNodeViewsProps {
+export interface ExecutionNodeViewsProps {
     execution: Execution;
 }
 
-const tabIds = {
-    nodes: 'nodes',
-    graph: 'graph'
-};
-
 /** Contains the available ways to visualize the nodes of a WorkflowExecution */
 export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
     execution
 }) => {
     const styles = useStyles();
     const filterState = useNodeExecutionFiltersState();
-    const tabState = useTabState(tabIds, tabIds.nodes);
+    const tabState = useTabState(tabs, tabs.nodes.id);
+
+    /* We want to maintain the filter selection when switching away from the Nodes
+    tab and back, but do not want to filter the nodes when viewing the graph. So,
+    we will only pass filters to the execution state when on the nodes tab. */
+    const appliedFilters =
+        tabState.value === tabs.nodes.id ? filterState.appliedFilters : [];
 
     const {
         workflow,
         nodeExecutions,
         nodeExecutionsRequestConfig
-    } = useWorkflowExecutionState(execution, filterState.appliedFilters);
+    } = useWorkflowExecutionState(execution, appliedFilters);
 
     return (
         <WaitForData {...workflow}>
             <Tabs className={styles.tabs} {...tabState}>
-                <Tab value={tabIds.nodes} label="Nodes" />
-                <Tab value={tabIds.graph} label="Graph" />
+                <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 === tabIds.nodes && (
+                {tabState.value === tabs.nodes.id && (
                     <>
                         <div className={styles.filters}>
                             <ExecutionFilters {...filterState} />
@@ -76,7 +78,7 @@ export const ExecutionNodeViews: React.FC<ExecutionNodeViewsProps> = ({
                         </WaitForData>
                     </>
                 )}
-                {tabState.value === tabIds.graph && (
+                {tabState.value === tabs.graph.id && (
                     <WaitForData {...nodeExecutions}>
                         <ExecutionWorkflowGraph
                             nodeExecutions={nodeExecutions.value}

src/components/Executions/ExecutionDetails/constants.ts

@@ -5,3 +5,14 @@ export enum ExecutionMetadataLabels {
     time = 'Time',
     version = 'Version'
 }
+
+export const tabs = {
+    nodes: {
+        id: 'nodes',
+        label: 'Nodes'
+    },
+    graph: {
+        id: 'graph',
+        label: 'Graph'
+    }
+};

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

@@ -0,0 +1,156 @@
+import {
+    fireEvent,
+    render,
+    waitFor,
+    waitForElementToBeRemoved
+} from '@testing-library/react';
+import { mockAPIContextValue } from 'components/data/__mocks__/apiContext';
+import { APIContext, APIContextValue } from 'components/data/apiContext';
+import { createMockExecutionEntities } from 'components/Executions/__mocks__/createMockExecutionEntities';
+import {
+    ExecutionContextData,
+    ExecutionDataCacheContext
+} from 'components/Executions/contexts';
+import { filterLabels } from 'components/Executions/filters/constants';
+import { nodeExecutionStatusFilters } from 'components/Executions/filters/statusFilters';
+import { ExecutionDataCache } from 'components/Executions/types';
+import { createExecutionDataCache } from 'components/Executions/useExecutionDataCache';
+import {
+    getExecution,
+    Identifier,
+    listNodeExecutions,
+    WorkflowExecutionIdentifier
+} from 'models';
+import { createMockExecution } from 'models/__mocks__/executionsData';
+import { mockTasks } from 'models/Task/__mocks__/mockTaskData';
+import * as React from 'react';
+import { tabs } from '../constants';
+import {
+    ExecutionNodeViews,
+    ExecutionNodeViewsProps
+} from '../ExecutionNodeViews';
+
+// We don't need to verify the content of the graph component here and it is
+// difficult to make it work correctly in a test environment.
+jest.mock('../ExecutionWorkflowGraph.tsx', () => ({
+    ExecutionWorkflowGraph: () => null
+}));
+
+describe('ExecutionNodeViews', () => {
+    let props: ExecutionNodeViewsProps;
+    let apiContext: APIContextValue;
+    let executionContext: ExecutionContextData;
+    let dataCache: ExecutionDataCache;
+    let mockListNodeExecutions: jest.Mock<ReturnType<
+        typeof listNodeExecutions
+    >>;
+    let mockGetExecution: jest.Mock<ReturnType<typeof getExecution>>;
+
+    beforeEach(() => {
+        const {
+            nodeExecutions,
+            workflow,
+            workflowExecution
+        } = createMockExecutionEntities({
+            workflowName: 'SampleWorkflow',
+            nodeExecutionCount: 2
+        });
+
+        mockGetExecution = jest
+            .fn()
+            .mockImplementation(async (id: WorkflowExecutionIdentifier) => {
+                return { ...createMockExecution(id.name), id };
+            });
+
+        mockListNodeExecutions = jest
+            .fn()
+            .mockResolvedValue({ entities: nodeExecutions });
+        apiContext = mockAPIContextValue({
+            getExecution: mockGetExecution,
+            getTask: jest.fn().mockImplementation(async (id: Identifier) => {
+                return { template: { ...mockTasks[0].template, id } };
+            }),
+            listNodeExecutions: mockListNodeExecutions,
+            listTaskExecutions: jest.fn().mockResolvedValue({ entities: [] }),
+            listTaskExecutionChildren: jest
+                .fn()
+                .mockResolvedValue({ entities: [] })
+        });
+
+        dataCache = createExecutionDataCache(apiContext);
+        dataCache.insertWorkflow(workflow);
+        dataCache.insertWorkflowExecutionReference(
+            workflowExecution.id,
+            workflow.id
+        );
+
+        executionContext = {
+            execution: workflowExecution,
+            terminateExecution: jest.fn().mockRejectedValue('Not Implemented')
+        };
+
+        props = { execution: workflowExecution };
+    });
+
+    const renderViews = () =>
+        render(
+            <APIContext.Provider value={apiContext}>
+                <ExecutionDataCacheContext.Provider value={dataCache}>
+                    <ExecutionNodeViews {...props} />
+                </ExecutionDataCacheContext.Provider>
+            </APIContext.Provider>
+        );
+
+    it('only applies filter when viewing the nodes tab', async () => {
+        const { getByText } = renderViews();
+        const nodesTab = await waitFor(() => getByText(tabs.nodes.label));
+        const graphTab = await waitFor(() => getByText(tabs.graph.label));
+
+        fireEvent.click(nodesTab);
+        const statusButton = await waitFor(() =>
+            getByText(filterLabels.status)
+        );
+        fireEvent.click(statusButton);
+        const successFilter = await waitFor(() =>
+            getByText(nodeExecutionStatusFilters.succeeded.label)
+        );
+
+        mockListNodeExecutions.mockClear();
+        fireEvent.click(successFilter);
+        await waitFor(() => mockListNodeExecutions.mock.calls.length > 0);
+        // Verify at least one filter is passed
+        expect(mockListNodeExecutions).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.objectContaining({
+                filter: expect.arrayContaining([
+                    expect.objectContaining({ key: expect.any(String) })
+                ])
+            })
+        );
+
+        fireEvent.click(statusButton);
+        await waitForElementToBeRemoved(successFilter);
+        mockListNodeExecutions.mockClear();
+        fireEvent.click(graphTab);
+        await waitFor(() => mockListNodeExecutions.mock.calls.length > 0);
+        // No filter expected on the graph tab
+        expect(mockListNodeExecutions).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.objectContaining({ filter: [] })
+        );
+
+        mockListNodeExecutions.mockClear();
+        fireEvent.click(nodesTab);
+        await waitFor(() => mockListNodeExecutions.mock.calls.length > 0);
+        // Verify (again) at least one filter is passed, after changing back to
+        // nodes tab.
+        expect(mockListNodeExecutions).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.objectContaining({
+                filter: expect.arrayContaining([
+                    expect.objectContaining({ key: expect.any(String) })
+                ])
+            })
+        );
+    });
+});

src/components/Executions/__mocks__/createMockExecutionEntities.ts

@@ -35,11 +35,14 @@ export function createMockExecutionEntities({
     compiledWorkflow.tasks = tasks.concat(cloneDeep(mockTasks));
     workflow.closure = workflowClosure;
 
+    const workflowExecution = { ...mockWorkflowExecution };
+    workflowExecution.closure.workflowId = workflow.id;
+
     return {
         nodes,
         nodeExecutions,
         tasks,
         workflow,
-        workflowExecution: mockWorkflowExecution
+        workflowExecution
     };
 }

src/components/Executions/filters/constants.ts

@@ -0,0 +1,6 @@
+export const filterLabels = {
+    duration: 'Duration',
+    startTime: 'Start Time',
+    status: 'Status',
+    version: 'Version'
+};

src/components/Executions/filters/useExecutionFiltersState.ts

@@ -1,5 +1,6 @@
 import { compact, flatMap } from 'lodash';
 import { FilterOperation } from 'models';
+import { filterLabels } from './constants';
 import { durationFilters } from './durationFilters';
 import {
     nodeExecutionStartTimeFilters,
@@ -40,26 +41,26 @@ export function useWorkflowExecutionFiltersState() {
             options: workflowExecutionStatusFilters,
             defaultValue: [],
             filterKey: 'phase',
-            label: 'Status',
+            label: filterLabels.status,
             listHeader: 'Filter By',
             queryStateKey: 'status'
         }),
         useSearchFilterState({
             filterKey: 'workflow.version',
-            label: 'Version',
+            label: filterLabels.version,
             placeholder: 'Enter Version String',
             queryStateKey: 'version'
         }),
         useSingleFilterState({
             options: workflowExecutionStartTimeFilters,
             defaultValue: workflowExecutionStartTimeFilters.all,
-            label: 'Start Time',
+            label: filterLabels.startTime,
             queryStateKey: 'startTime'
         }),
         useSingleFilterState({
             options: durationFilters,
             defaultValue: durationFilters.all,
-            label: 'Duration',
+            label: filterLabels.duration,
             queryStateKey: 'duration'
         })
     ]);
@@ -71,20 +72,20 @@ export function useNodeExecutionFiltersState() {
             options: nodeExecutionStatusFilters,
             defaultValue: [],
             filterKey: 'phase',
-            label: 'Status',
+            label: filterLabels.status,
             listHeader: 'Filter By',
             queryStateKey: 'status'
         }),
         useSingleFilterState({
             options: nodeExecutionStartTimeFilters,
             defaultValue: nodeExecutionStartTimeFilters.all,
-            label: 'Start Time',
+            label: filterLabels.startTime,
             queryStateKey: 'startTime'
         }),
         useSingleFilterState({
             options: durationFilters,
             defaultValue: durationFilters.all,
-            label: 'Duration',
+            label: filterLabels.duration,
             queryStateKey: 'duration'
         })
     ]);

src/components/hooks/useTabState.ts

@@ -1,7 +1,7 @@
 import { useState } from 'react';
 
 export function useTabState(
-    tabs: { [k: string]: string },
+    tabs: { [k: string]: string | object },
     defaultValue: string
 ) {
     const [value, setValue] = useState(defaultValue);