From 511812f0ddf257c5bb2f49a6b004cec53d63d9e9 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 --- .../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 0290d1e34527b..48587055a8988 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 684694bdb5c9a..c467735fdb327 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 db5039e5a72f0..2ad264ab14b91 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 e75b3d66dbaa9..a1b14f31bf57c 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 d9226436e4f3b..f7af034b2a9ef 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 @@ -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((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 eee4638ede8f2..f3a19317f794c 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 0000000000000..a93562cd57785 --- /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 a219bbf37d274..7f678c29f3888 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 @@ -114,7 +114,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, @@ -122,6 +129,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 9d0f5ac135d5d..5f50eacff76c6 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 97d635a3b6840..6c8bc8a04bed3 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 0abdc7f24e08b..076f567dbad94 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}`); + }); }); });