Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Security Solution][Endpoint][Response Actions] Fix table navigation when trays are expanded #157777

Merged
merged 19 commits into from
May 23, 2023
Merged

[Security Solution][Endpoint][Response Actions] Fix table navigation when trays are expanded #157777

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fa0c798
fix error when action details trays are expanded and paging is used
ashokaditya May 15, 2023
890eb1d
rename redundant callback
ashokaditya May 16, 2023
9e743e2
rename for readability
ashokaditya May 17, 2023
cf8dbfc
update missing dependencies
ashokaditya May 17, 2023
3bc094b
expand already open trays on page re-load
ashokaditya May 17, 2023
08ffdc7
tests to verify paging shows expanded rows
ashokaditya May 18, 2023
2801a75
add test for page reload using withOutputs
ashokaditya May 18, 2023
cbf0aa1
review changes
ashokaditya May 18, 2023
b6f3bbe
test for setting URL param
ashokaditya May 22, 2023
c09b974
Merge branch 'main' into fix/dw-response-actions-history-tray-back-pa…
ashokaditya May 22, 2023
bdac04c
typo
ashokaditya May 22, 2023
a173f3e
Merge branch 'main' into fix/dw-response-actions-history-tray-back-pa…
ashokaditya May 22, 2023
df6d583
Cypress tests for page reload withOutputs
ashokaditya May 22, 2023
fd389bf
Merge branch 'main' into fix/dw-response-actions-history-tray-back-pa…
ashokaditya May 23, 2023
e3b26f4
Merge branch 'main' into fix/dw-response-actions-history-tray-back-pa…
ashokaditya May 23, 2023
555a9bf
remvoe redundant test
ashokaditya May 23, 2023
ca1919b
update cypress task
ashokaditya May 23, 2023
50e8995
remove redundant assertion
ashokaditya May 23, 2023
82b4a74
Merge branch 'main' into fix/dw-response-actions-history-tray-back-pa…
ashokaditya May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 44 additions & 34 deletions ...lic/management/components/endpoint_response_actions_list/components/actions_log_table.tsx
View file
Open in desktop
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,51 @@ 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 {
Comment on lines +316 to +318
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was not handled earlier and adding this change fixes the issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix 👍
Just a detail that I was thinking about and maybe would make sense to you - it's personal preference, I can't justify which approach is better :P Would you consider wrapping openDetails in useMemo and use it with setExpandedRowMap in useEffect instead of using redoOpenTrans wrapper in useCallback?

idToRowMap[actionId] = (
<ActionsLogExpandedTray
action={items.filter((item) => item.id === actionId)[0]}
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved
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 +419,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 +435,7 @@ export const ActionsLogTable = memo<ActionsLogTableProps>(
items={items}
columns={columns}
itemId="id"
itemIdToExpandedRowMap={itemIdToExpandedRowMap}
itemIdToExpandedRowMap={expandedRowMap}
isExpandable
pagination={tablePagination}
onChange={onChange}
Expand Down
2 changes: 1 addition & 1 deletion ...tion/public/management/components/endpoint_response_actions_list/response_actions_log.tsx
View file
Open in desktop
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]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not related to the bug, only a missing dependency

);

if (error?.body?.statusCode === 404 && error?.body?.message === 'index_not_found_exception') {
Expand Down