Skip to content

Commit

Permalink
[Security Solution][Endpoint][Response Actions] Fix table navigation …
Browse files Browse the repository at this point in the history
…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 9e01dc8)
  • Loading branch information
ashokaditya committed May 23, 2023
1 parent 96c6d15 commit 7b4276e
Show file tree
Hide file tree
Showing 11 changed files with 248 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type {
import {
deleteIndexedEndpointAndFleetActions,
indexEndpointAndFleetActionsForHost,
type IndexEndpointAndFleetActionsForHostOptions,
} from './index_endpoint_fleet_actions';

import type {
Expand Down Expand Up @@ -88,6 +89,7 @@ export async function indexEndpointHostDocs({
enrollFleet,
generator,
withResponseActions = true,
numResponseActions,
}: {
numDocs: number;
client: Client;
Expand All @@ -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();
Expand Down Expand Up @@ -198,7 +201,10 @@ export async function indexEndpointHostDocs({
const actionsResponse = await indexEndpointAndFleetActionsForHost(
client,
hostMetadata,
undefined
undefined,
{
numResponseActions,
}
);
mergeAndAppendArrays(response, actionsResponse);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -117,6 +118,7 @@ export async function indexHostsAndAlerts(
enrollFleet: fleet,
generator,
withResponseActions,
numResponseActions,
});

mergeAndAppendArrays(response, indexedHosts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}) => {
Expand Down Expand Up @@ -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'}
/>
);
},
Expand Down Expand Up @@ -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 ?? [];
Expand All @@ -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
Expand Down Expand Up @@ -409,11 +416,11 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
() =>
getResponseActionListTableColumns({
getTestId,
itemIdToExpandedRowMap,
expandedRowMap,
onClickCallback,
showHostNames,
}),
[itemIdToExpandedRowMap, getTestId, onClickCallback, showHostNames]
[expandedRowMap, getTestId, onClickCallback, showHostNames]
);

return (
Expand All @@ -425,7 +432,7 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
items={items}
columns={columns}
itemId="id"
itemIdToExpandedRowMap={itemIdToExpandedRowMap}
itemIdToExpandedRowMap={expandedRowMap}
isExpandable
pagination={tablePagination}
onChange={onChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,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,
});
},

Expand Down
Loading

0 comments on commit 7b4276e

Please sign in to comment.