From 7b4276eee66faec12d636e9055dee95cfcf93b57 Mon Sep 17 00:00:00 2001 From: Ashokaditya <1849116+ashokaditya@users.noreply.github.com> Date: Tue, 23 May 2023 20:49:02 +0200 Subject: [PATCH] [Security Solution][Endpoint][Response Actions] Fix table navigation when trays are expanded (#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 (cherry picked from commit 9e01dc815f87ccb70f961852f6d7c014e3e43c0b) --- .../index_endpoint_fleet_actions.ts | 9 ++- .../data_loaders/index_endpoint_hosts.ts | 8 +- .../common/endpoint/index_data.ts | 4 +- .../components/actions_log_table.tsx | 75 ++++++++++--------- .../response_actions_log.test.tsx | 59 +++++++++++++++ .../response_actions_log.tsx | 2 +- .../mocked_data/reponse_actions_history.cy.ts | 52 +++++++++++++ .../cypress/support/data_loaders.ts | 10 ++- .../plugin_handlers/endpoint_data_loader.ts | 5 +- .../public/management/cypress/types.ts | 2 +- .../view/response_actions_list_page.test.tsx | 64 ++++++++++++++++ 11 files changed, 248 insertions(+), 42 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/management/cypress/e2e/mocked_data/reponse_actions_history.cy.ts diff --git a/x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_fleet_actions.ts b/x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_fleet_actions.ts index 0290d1e34527b2..48587055a8988f 100644 --- a/x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_fleet_actions.ts +++ b/x-pack/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 => { 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 index 684694bdb5c9ab..c467735fdb3273 100644 --- 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 { 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 index db5039e5a72f03..2ad264ab14b914 100644 --- 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 { 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/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/components/actions_log_table.tsx b/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/components/actions_log_table.tsx index f89046154d1d37..b740f73a3abc2c 100644 --- a/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/components/actions_log_table.tsx +++ b/x-pack/plugins/security_solution/public/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 = ({ ); }, @@ -290,13 +289,13 @@ export const ActionsLogTable = memo( showHostNames, totalItemCount, }) => { - const [itemIdToExpandedRowMap, setItemIdToExpandedRowMap] = useState({}); - const getTestId = useTestIdGenerator(dataTestSubj); const { pagination: paginationFromUrlParams } = useUrlPagination(); const { withOutputs: withOutputsFromUrl } = useActionHistoryUrlParams(); - const getActionIdsWithDetails = useCallback((): string[] => { + const [expandedRowMap, setExpandedRowMap] = useState({}); + + 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( : []; }, [isFlyout, queryParams.withOutputs, withOutputsFromUrl]); + const redoOpenTrays = useCallback(() => { + if (actionIdsWithOpenTrays.length && items.length) { + const openDetails = actionIdsWithOpenTrays.reduce( + (idToRowMap, actionId) => { + const actionItem = items.find((item) => item.id === actionId); + if (!actionItem) { + idToRowMap[actionId] = ; + } else { + idToRowMap[actionId] = ( + + ); + } + 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( - (idToRowMap, actionId) => { - idToRowMap[actionId] = ( - 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] = ( ); } - 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( () => getResponseActionListTableColumns({ getTestId, - itemIdToExpandedRowMap, + expandedRowMap, onClickCallback, showHostNames, }), - [itemIdToExpandedRowMap, getTestId, onClickCallback, showHostNames] + [expandedRowMap, getTestId, onClickCallback, showHostNames] ); return ( @@ -425,7 +432,7 @@ export const ActionsLogTable = memo( items={items} columns={columns} itemId="id" - itemIdToExpandedRowMap={itemIdToExpandedRowMap} + itemIdToExpandedRowMap={expandedRowMap} isExpandable pagination={tablePagination} onChange={onChange} diff --git a/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx b/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx index 6fc62dd6cf8519..17591310062728 100644 --- a/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx +++ b/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/integration_tests/response_actions_log.test.tsx @@ -462,6 +462,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((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/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/response_actions_log.tsx b/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/response_actions_log.tsx index eee4638ede8f2b..f3a19317f794cd 100644 --- a/x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/response_actions_log.tsx +++ b/x-pack/plugins/security_solution/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/x-pack/plugins/security_solution/public/management/cypress/e2e/mocked_data/reponse_actions_history.cy.ts b/x-pack/plugins/security_solution/public/management/cypress/e2e/mocked_data/reponse_actions_history.cy.ts new file mode 100644 index 00000000000000..a93562cd57785e --- /dev/null +++ b/x-pack/plugins/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; + // let actionData: ReturnTypeFromChainable; + + 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 index ffcca01a6f1e96..865e7c866b3d6f 100644 --- 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 @@ -111,7 +111,14 @@ 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, @@ -119,6 +126,7 @@ export const dataLoaders = ( os, isolation, withResponseActions, + numResponseActions, }); }, diff --git a/x-pack/plugins/security_solution/public/management/cypress/support/plugin_handlers/endpoint_data_loader.ts b/x-pack/plugins/security_solution/public/management/cypress/support/plugin_handlers/endpoint_data_loader.ts index 9d0f5ac135d5d1..5f50eacff76c6a 100644 --- a/x-pack/plugins/security_solution/public/management/cypress/support/plugin_handlers/endpoint_data_loader.ts +++ b/x-pack/plugins/security_solution/public/management/cypress/support/plugin_handlers/endpoint_data_loader.ts @@ -37,6 +37,7 @@ export interface CyLoadEndpointDataOptions generatorSeed: string; waitUntilTransformed: boolean; withResponseActions: boolean; + numResponseActions?: number; isolation: boolean; bothIsolatedAndNormalEndpoints?: boolean; } @@ -63,6 +64,7 @@ export const cyLoadEndpointDataHandler = async ( os, withResponseActions, isolation, + numResponseActions, } = options; const DocGenerator = EndpointDocGenerator.custom({ @@ -91,7 +93,8 @@ export const cyLoadEndpointDataHandler = async ( enableFleetIntegration, undefined, DocGenerator, - withResponseActions + withResponseActions, + numResponseActions ); if (waitUntilTransformed) { diff --git a/x-pack/plugins/security_solution/public/management/cypress/types.ts b/x-pack/plugins/security_solution/public/management/cypress/types.ts index 97d635a3b68408..6c8bc8a04bed34 100644 --- a/x-pack/plugins/security_solution/public/management/cypress/types.ts +++ b/x-pack/plugins/security_solution/public/management/cypress/types.ts @@ -42,7 +42,7 @@ export type ReturnTypeFromChainable = C extends Cyp : never; export type IndexEndpointHostsCyTaskOptions = Partial< - { count: number; withResponseActions: boolean } & Pick< + { count: number; withResponseActions: boolean; numResponseActions?: number } & Pick< CyLoadEndpointDataOptions, 'version' | 'os' | 'isolation' > diff --git a/x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx b/x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx index 0abdc7f24e08bd..076f567dbad949 100644 --- a/x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/response_actions/view/response_actions_list_page.test.tsx @@ -343,6 +343,42 @@ describe('Response actions history page', () => { ); expect(history.location.search).toEqual(`?startDate=${startDate}&endDate=${endDate}`); }); + + it('should read and expand actions using `withOutputs`', () => { + const allActionIds = mockUseGetEndpointActionList.data?.data.map((action) => action.id) ?? []; + // select 5 actions to show details + const actionIdsWithDetails = allActionIds.filter((_, i) => [0, 2, 3, 4, 5].includes(i)); + reactTestingLibrary.act(() => { + // load page 1 but with expanded actions. + history.push( + `/administration/response_actions_history?withOutputs=${actionIdsWithDetails.join( + ',' + )}&page=1&pageSize=10` + ); + }); + + const { getByTestId, getAllByTestId } = render(); + + // verify on page 1 + expect(getByTestId(`${testPrefix}-endpointListTableTotal`)).toHaveTextContent( + 'Showing 1-10 of 43 response actions' + ); + + const traysOnPage1 = getAllByTestId(`${testPrefix}-details-tray`); + const expandButtonsOnPage1 = getAllByTestId(`${testPrefix}-expand-button`); + const expandedButtons = expandButtonsOnPage1.reduce((acc, button, i) => { + // find expanded rows + if (button.getAttribute('aria-label') === 'Collapse') { + acc.push(i); + } + return acc; + }, []); + + // verify 5 rows are expanded + expect(traysOnPage1.length).toEqual(5); + // verify 5 rows that are expanded are the ones from before + expect(expandedButtons).toEqual([0, 2, 3, 4, 5]); + }); }); describe('Set selected/set values to URL params', () => { @@ -442,5 +478,33 @@ describe('Response actions history page', () => { expect(history.location.search).toEqual('?endDate=now&startDate=now-15m'); }); + + it('should set actionIds using `withOutputs` to URL params ', async () => { + const allActionIds = mockUseGetEndpointActionList.data?.data.map((action) => action.id) ?? []; + const actionIdsWithDetails = allActionIds + .reduce((acc, e, i) => { + if ([0, 1].includes(i)) { + acc.push(e); + } + return acc; + }, []) + .join() + .split(',') + .join('%2C'); + + render(); + const { getAllByTestId } = renderResult; + + const expandButtons = getAllByTestId(`${testPrefix}-expand-button`); + // expand some rows + expandButtons.forEach((button, i) => { + if ([0, 1].includes(i)) { + userEvent.click(button); + } + }); + + // verify 2 rows are expanded and are the ones from before + expect(history.location.search).toEqual(`?withOutputs=${actionIdsWithDetails}`); + }); }); });