[Security Solution][Endpoint][Response Actions] Fix table navigation …

…when trays are expanded (elastic#157777) ## Summary Fixes an issue where when an action detail is shown via an expanded tray item and the total number of items goes beyond the first page on the response actions history page/flyout, switching between pages while the tray is open breaks the page. - [x] fix paging with trays expanded on a flyout - [x] fix paging with trays expanded on a page - [x] ensure when the page is loaded with `?withOutputs=` with action ids from different sets of pages, table paging doesn't break when paged, and trays show open for the action ids in `?withOutputs=` URL param - tests: - [x] page navigation flyout/page view - [x] page reload with URL params (cypress) **flyout** ![response-logs-flyout](https://github.com/elastic/kibana/assets/1849116/c7c91d0d-3279-4813-b7dd-1365313b7fe4) **page** ![response-logs-page](https://github.com/elastic/kibana/assets/1849116/b68f6e5d-9d0e-456f-8b07-dea07526571b) *page with URL load* - three actions are open on two different pages - we re-load page 2 with two open trays and then navigate to page 1 to see the third one open - also re-load page 1; we see the tray open, then navigate to page 2 to see the other two trays open. ![response-logs-page-reload](https://github.com/elastic/kibana/assets/1849116/58896b2e-4078-42c0-ac6c-c34d5b1cd42b) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine · May 23, 2023 · 9e01dc8 · 9e01dc8
1 parent b50c358
commit 9e01dc8
Show file tree

Hide file tree

Showing 11 changed files with 248 additions and 42 deletions.
diff --git a/...ck/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_fleet_actions.ts b/...ck/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_fleet_actions.ts
@@ -32,6 +32,9 @@ export interface IndexedEndpointAndFleetActionsForHostResponse {
   endpointActionResponsesIndex: string;
 }
 
+export interface IndexEndpointAndFleetActionsForHostOptions {
+  numResponseActions?: number;
+}
 /**
  * Indexes a random number of Endpoint (via Fleet) Actions for a given host
  * (NOTE: ensure that fleet is setup first before calling this loading function)
@@ -43,11 +46,13 @@ export interface IndexedEndpointAndFleetActionsForHostResponse {
 export const indexEndpointAndFleetActionsForHost = async (
   esClient: Client,
   endpointHost: HostMetadata,
-  fleetActionGenerator: FleetActionGenerator = defaultFleetActionGenerator
+  fleetActionGenerator: FleetActionGenerator = defaultFleetActionGenerator,
+  options: IndexEndpointAndFleetActionsForHostOptions = {}
 ): Promise<IndexedEndpointAndFleetActionsForHostResponse> => {
   const ES_INDEX_OPTIONS = { headers: { 'X-elastic-product-origin': 'fleet' } };
   const agentId = endpointHost.elastic.agent.id;
-  const total = fleetActionGenerator.randomN(5) + 1; // generate at least one
+  const actionsCount = options.numResponseActions ?? 1;
+  const total = fleetActionGenerator.randomN(5) + actionsCount;
   const response: IndexedEndpointAndFleetActionsForHostResponse = {
     actions: [],
     actionResponses: [],

diff --git a/x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_hosts.ts b/x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_hosts.ts
@@ -26,6 +26,7 @@ import type {
 import {
   deleteIndexedEndpointAndFleetActions,
   indexEndpointAndFleetActionsForHost,
+  type IndexEndpointAndFleetActionsForHostOptions,
 } from './index_endpoint_fleet_actions';
 
 import type {
@@ -88,6 +89,7 @@ export async function indexEndpointHostDocs({
   enrollFleet,
   generator,
   withResponseActions = true,
+  numResponseActions,
 }: {
   numDocs: number;
   client: Client;
@@ -99,6 +101,7 @@ export async function indexEndpointHostDocs({
   enrollFleet: boolean;
   generator: EndpointDocGenerator;
   withResponseActions?: boolean;
+  numResponseActions?: IndexEndpointAndFleetActionsForHostOptions['numResponseActions'];
 }): Promise<IndexedHostsResponse> {
   const timeBetweenDocs = 6 * 3600 * 1000; // 6 hours between metadata documents
   const timestamp = new Date().getTime();
@@ -198,7 +201,10 @@ export async function indexEndpointHostDocs({
         const actionsResponse = await indexEndpointAndFleetActionsForHost(
           client,
           hostMetadata,
-          undefined
+          undefined,
+          {
+            numResponseActions,
+          }
         );
         mergeAndAppendArrays(response, actionsResponse);
       }

diff --git a/x-pack/plugins/security_solution/common/endpoint/index_data.ts b/x-pack/plugins/security_solution/common/endpoint/index_data.ts
@@ -64,7 +64,8 @@ export async function indexHostsAndAlerts(
   fleet: boolean,
   options: TreeOptions = {},
   DocGenerator: typeof EndpointDocGenerator = EndpointDocGenerator,
-  withResponseActions = true
+  withResponseActions = true,
+  numResponseActions?: number
 ): Promise<IndexedHostsAndAlertsResponse> {
   const random = seedrandom(seed);
   const epmEndpointPackage = await getEndpointPackageInfo(kbnClient);
@@ -117,6 +118,7 @@ export async function indexHostsAndAlerts(
       enrollFleet: fleet,
       generator,
       withResponseActions,
+      numResponseActions,
     });
 
     mergeAndAppendArrays(response, indexedHosts);

diff --git a/...lic/management/components/endpoint_response_actions_list/components/actions_log_table.tsx b/...lic/management/components/endpoint_response_actions_list/components/actions_log_table.tsx
@@ -19,6 +19,7 @@ import {
   EuiToolTip,
   type HorizontalAlignment,
   type CriteriaWithPagination,
+  EuiSkeletonText,
 } from '@elastic/eui';
 import { euiStyled } from '@kbn/kibana-react-plugin/common';
 import { FormattedMessage } from '@kbn/i18n-react';
@@ -55,12 +56,12 @@ interface ExpandedRowMapType {
 
 const getResponseActionListTableColumns = ({
   getTestId,
-  itemIdToExpandedRowMap,
+  expandedRowMap,
   showHostNames,
   onClickCallback,
 }: {
   getTestId: (suffix?: string | undefined) => string | undefined;
-  itemIdToExpandedRowMap: ExpandedRowMapType;
+  expandedRowMap: ExpandedRowMapType;
   showHostNames: boolean;
   onClickCallback: (actionListDataItem: ActionListApiResponse['data'][number]) => () => void;
 }) => {
@@ -245,10 +246,8 @@ const getResponseActionListTableColumns = ({
           <EuiButtonIcon
             data-test-subj={getTestId('expand-button')}
             onClick={onClickCallback(actionListDataItem)}
-            aria-label={
-              itemIdToExpandedRowMap[actionId] ? ARIA_LABELS.collapse : ARIA_LABELS.expand
-            }
-            iconType={itemIdToExpandedRowMap[actionId] ? 'arrowUp' : 'arrowDown'}
+            aria-label={expandedRowMap[actionId] ? ARIA_LABELS.collapse : ARIA_LABELS.expand}
+            iconType={expandedRowMap[actionId] ? 'arrowUp' : 'arrowDown'}
           />
         );
       },
@@ -290,13 +289,13 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
     showHostNames,
     totalItemCount,
   }) => {
-    const [itemIdToExpandedRowMap, setItemIdToExpandedRowMap] = useState<ExpandedRowMapType>({});
-
     const getTestId = useTestIdGenerator(dataTestSubj);
     const { pagination: paginationFromUrlParams } = useUrlPagination();
     const { withOutputs: withOutputsFromUrl } = useActionHistoryUrlParams();
 
-    const getActionIdsWithDetails = useCallback((): string[] => {
+    const [expandedRowMap, setExpandedRowMap] = useState<ExpandedRowMapType>({});
+
+    const actionIdsWithOpenTrays = useMemo((): string[] => {
       // get the list of action ids from URL params on the history page
       if (!isFlyout) {
         return withOutputsFromUrl ?? [];
@@ -309,40 +308,48 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
         : [];
     }, [isFlyout, queryParams.withOutputs, withOutputsFromUrl]);
 
+    const redoOpenTrays = useCallback(() => {
+      if (actionIdsWithOpenTrays.length && items.length) {
+        const openDetails = actionIdsWithOpenTrays.reduce<ExpandedRowMapType>(
+          (idToRowMap, actionId) => {
+            const actionItem = items.find((item) => item.id === actionId);
+            if (!actionItem) {
+              idToRowMap[actionId] = <EuiSkeletonText size="relative" lines={8} />;
+            } else {
+              idToRowMap[actionId] = (
+                <ActionsLogExpandedTray action={actionItem} data-test-subj={dataTestSubj} />
+              );
+            }
+            return idToRowMap;
+          },
+          {}
+        );
+        setExpandedRowMap(openDetails);
+      }
+    }, [actionIdsWithOpenTrays, dataTestSubj, items]);
+
+    // open trays that were open using URL params/ query params
     useEffect(() => {
-      const actionIdsWithDetails = getActionIdsWithDetails();
-      const openDetails = actionIdsWithDetails.reduce<ExpandedRowMapType>(
-        (idToRowMap, actionId) => {
-          idToRowMap[actionId] = (
-            <ActionsLogExpandedTray
-              action={items.filter((item) => item.id === actionId)[0]}
-              data-test-subj={dataTestSubj}
-            />
-          );
-          return idToRowMap;
-        },
-        {}
-      );
-      setItemIdToExpandedRowMap(openDetails);
-    }, [dataTestSubj, getActionIdsWithDetails, items, queryParams.withOutputs, withOutputsFromUrl]);
+      redoOpenTrays();
+    }, [redoOpenTrays]);
 
     const toggleDetails = useCallback(
       (action: ActionListApiResponse['data'][number]) => {
-        const itemIdToExpandedRowMapValues = { ...itemIdToExpandedRowMap };
-        if (itemIdToExpandedRowMapValues[action.id]) {
+        const expandedRowMapCopy = { ...expandedRowMap };
+        if (expandedRowMapCopy[action.id]) {
           // close tray
-          delete itemIdToExpandedRowMapValues[action.id];
+          delete expandedRowMapCopy[action.id];
         } else {
           // assign the expanded tray content to the map
           // with action details
-          itemIdToExpandedRowMapValues[action.id] = (
+          expandedRowMapCopy[action.id] = (
             <ActionsLogExpandedTray action={action} data-test-subj={dataTestSubj} />
           );
         }
-        onShowActionDetails(Object.keys(itemIdToExpandedRowMapValues));
-        setItemIdToExpandedRowMap(itemIdToExpandedRowMapValues);
+        onShowActionDetails(Object.keys(expandedRowMapCopy));
+        setExpandedRowMap(expandedRowMapCopy);
       },
-      [itemIdToExpandedRowMap, onShowActionDetails, dataTestSubj]
+      [expandedRowMap, onShowActionDetails, dataTestSubj]
     );
 
     // memoized callback for toggleDetails
@@ -409,11 +416,11 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
       () =>
         getResponseActionListTableColumns({
           getTestId,
-          itemIdToExpandedRowMap,
+          expandedRowMap,
           onClickCallback,
           showHostNames,
         }),
-      [itemIdToExpandedRowMap, getTestId, onClickCallback, showHostNames]
+      [expandedRowMap, getTestId, onClickCallback, showHostNames]
     );
 
     return (
@@ -425,7 +432,7 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
           items={items}
           columns={columns}
           itemId="id"
-          itemIdToExpandedRowMap={itemIdToExpandedRowMap}
+          itemIdToExpandedRowMap={expandedRowMap}
           isExpandable
           pagination={tablePagination}
           onChange={onChange}

diff --git a/...components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx b/...components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx
@@ -465,6 +465,65 @@ describe('Response actions history', () => {
       expect(noTrays).toEqual([]);
     });
 
+    it('should show already expanded trays on page navigation', async () => {
+      // start with two pages worth of response actions
+      // 10 on page 1, 3 on page 2
+      useGetEndpointActionListMock.mockReturnValue({
+        ...getBaseMockedActionList(),
+        data: await getActionListMock({ actionCount: 13 }),
+      });
+      render();
+      const { getByTestId, getAllByTestId } = renderResult;
+
+      // on page 1
+      expect(getByTestId(`${testPrefix}-endpointListTableTotal`)).toHaveTextContent(
+        'Showing 1-10 of 13 response actions'
+      );
+      const expandButtonsOnPage1 = getAllByTestId(`${testPrefix}-expand-button`);
+      // expand 2nd, 4th, 6th rows
+      expandButtonsOnPage1.forEach((button, i) => {
+        if ([1, 3, 5].includes(i)) {
+          userEvent.click(button);
+        }
+      });
+      // verify 3 rows are expanded
+      const traysOnPage1 = getAllByTestId(`${testPrefix}-details-tray`);
+      expect(traysOnPage1).toBeTruthy();
+      expect(traysOnPage1.length).toEqual(3);
+
+      // go to 2nd page
+      const page2 = getByTestId('pagination-button-1');
+      userEvent.click(page2);
+
+      // verify on page 2
+      expect(getByTestId(`${testPrefix}-endpointListTableTotal`)).toHaveTextContent(
+        'Showing 11-13 of 13 response actions'
+      );
+
+      // go back to 1st page
+      userEvent.click(getByTestId('pagination-button-0'));
+      // verify on page 1
+      expect(getByTestId(`${testPrefix}-endpointListTableTotal`)).toHaveTextContent(
+        'Showing 1-10 of 13 response actions'
+      );
+
+      const traysOnPage1back = getAllByTestId(`${testPrefix}-details-tray`);
+      const expandButtonsOnPage1back = getAllByTestId(`${testPrefix}-expand-button`);
+      const expandedButtons = expandButtonsOnPage1back.reduce<number[]>((acc, button, i) => {
+        // find expanded rows
+        if (button.getAttribute('aria-label') === 'Collapse') {
+          acc.push(i);
+        }
+        return acc;
+      }, []);
+
+      // verify 3 rows are expanded
+      expect(traysOnPage1back).toBeTruthy();
+      expect(traysOnPage1back.length).toEqual(3);
+      // verify 3 rows that are expanded are the ones from before
+      expect(expandedButtons).toEqual([1, 3, 5]);
+    });
+
     it('should contain relevant details in each expanded row', async () => {
       render();
       const { getAllByTestId } = renderResult;

diff --git a/...tion/public/management/components/endpoint_response_actions_list/response_actions_log.tsx b/...tion/public/management/components/endpoint_response_actions_list/response_actions_log.tsx
@@ -226,7 +226,7 @@ export const ResponseActionsLog = memo<
           setUrlWithOutputs(actionIds.join());
         }
       },
-      [isFlyout, setUrlWithOutputs]
+      [isFlyout, setUrlWithOutputs, setQueryParams]
     );
 
     if (error?.body?.statusCode === 404 && error?.body?.message === 'index_not_found_exception') {

diff --git a/...security_solution/public/management/cypress/e2e/mocked_data/reponse_actions_history.cy.ts b/...security_solution/public/management/cypress/e2e/mocked_data/reponse_actions_history.cy.ts
@@ -0,0 +1,52 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import type { ReturnTypeFromChainable } from '../../types';
+import { indexEndpointHosts } from '../../tasks/index_endpoint_hosts';
+import { login } from '../../tasks/login';
+
+describe('Response actions history page', () => {
+  let endpointData: ReturnTypeFromChainable<typeof indexEndpointHosts>;
+  // let actionData: ReturnTypeFromChainable<typeof indexActionResponses>;
+
+  before(() => {
+    indexEndpointHosts({ numResponseActions: 11 }).then((indexEndpoints) => {
+      endpointData = indexEndpoints;
+    });
+  });
+
+  beforeEach(() => {
+    login();
+  });
+
+  after(() => {
+    if (endpointData) {
+      endpointData.cleanup();
+      // @ts-expect-error ignore setting to undefined
+      endpointData = undefined;
+    }
+  });
+
+  it('retains expanded action details on page reload', () => {
+    cy.visit(`/app/security/administration/response_actions_history`);
+    cy.getByTestSubj('response-actions-list-expand-button').eq(3).click(); // 4th row on 1st page
+    cy.getByTestSubj('response-actions-list-details-tray').should('exist');
+    cy.url().should('include', 'withOutputs');
+
+    // navigate to page 2
+    cy.getByTestSubj('pagination-button-1').click();
+    cy.getByTestSubj('response-actions-list-details-tray').should('not.exist');
+
+    // reload with URL params on page 2 with existing URL
+    cy.reload();
+    cy.getByTestSubj('response-actions-list-details-tray').should('not.exist');
+
+    // navigate to page 1
+    cy.getByTestSubj('pagination-button-0').click();
+    cy.getByTestSubj('response-actions-list-details-tray').should('exist');
+  });
+});
diff --git a/x-pack/plugins/security_solution/public/management/cypress/support/data_loaders.ts b/x-pack/plugins/security_solution/public/management/cypress/support/data_loaders.ts
@@ -114,14 +114,22 @@ export const dataLoaders = (
 
     indexEndpointHosts: async (options: IndexEndpointHostsCyTaskOptions = {}) => {
       const { kbnClient, esClient } = await stackServicesPromise;
-      const { count: numHosts, version, os, isolation, withResponseActions } = options;
+      const {
+        count: numHosts,
+        version,
+        os,
+        isolation,
+        withResponseActions,
+        numResponseActions,
+      } = options;
 
       return cyLoadEndpointDataHandler(esClient, kbnClient, {
         numHosts,
         version,
         os,
         isolation,
         withResponseActions,
+        numResponseActions,
       });
     },