[Security Solution] Refactor ShowTopN Action so it doesn’t depend on App context providers (#161550)

## Summary
### Context: 
`TopN`components need the application context to work. So, `showTopN`
actions wrap the `TopN` component on a custom copy of the entire
application context. That is very error-prone and not performative.

### Solution:
* Create a service that the actions have access to
* Update the action code to call the service
* Move the rendering of `TopN` to the App rendering tree and listen to
the service for changes


### How to test it?
* Hover fields and use `showTopN` actions on different pages


<img width="400" alt="Screenshot 2023-07-10 at 16 41 13"
src="442c8c94-37c2-4cc3-a101-ca310d956670">
<img width="400" alt="Screenshot 2023-07-10 at 16 40 38"
src="b82e4188-8649-427d-9282-6d6911c8823e">
<img width="400" alt="Screenshot 2023-07-10 at 16 40 18"
src="497786ae-0136-4225-8230-399182e5a0b8">

### 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
This commit is contained in:
Pablo Machado 2023-07-25 16:15:44 +02:00 committed by GitHub
parent 1ea9d406c1
commit 7f002de706
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 200 additions and 209 deletions

View file

@ -103,7 +103,7 @@ const registerCellActions = (
filterOut: createFilterOutCellActionFactory({ store, services }),
addToTimeline: createAddToTimelineCellActionFactory({ store, services }),
investigateInNewTimeline: createInvestigateInNewTimelineCellActionFactory({ store, services }),
showTopN: createShowTopNCellActionFactory({ store, history, services }),
showTopN: createShowTopNCellActionFactory({ services }),
copyToClipboard: createCopyToClipboardCellActionFactory({ services }),
toggleColumn: createToggleColumnCellActionFactory({ store }),
};

View file

@ -6,36 +6,24 @@
*/
import type { CellActionExecutionContext } from '@kbn/cell-actions';
import {
createSecuritySolutionStorageMock,
kibanaObservable,
mockGlobalState,
SUB_PLUGINS_REDUCER,
} from '../../../common/mock';
import { mockHistory } from '../../../common/mock/router';
import { createStore } from '../../../common/store';
import { createShowTopNCellActionFactory } from './show_top_n';
import React from 'react';
import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock';
import { KBN_FIELD_TYPES } from '@kbn/field-types';
import type { StartServices } from '../../../types';
jest.mock('../../../common/lib/kibana');
jest.mock('../show_top_n_component', () => ({
TopNAction: () => <span>{'TEST COMPONENT'}</span>,
}));
const mockServices = createStartServicesMock();
const { storage } = createSecuritySolutionStorageMock();
const mockStore = createStore(mockGlobalState, SUB_PLUGINS_REDUCER, kibanaObservable, storage);
const mockServices = {
...createStartServicesMock(),
topValuesPopover: { showPopover: jest.fn() },
} as unknown as StartServices;
const element = document.createElement('div');
document.body.appendChild(element);
describe('createShowTopNCellActionFactory', () => {
const showTopNActionFactory = createShowTopNCellActionFactory({
store: mockStore,
history: mockHistory,
services: mockServices,
});
const showTopNAction = showTopNActionFactory({ id: 'testAction' });
@ -125,7 +113,8 @@ describe('createShowTopNCellActionFactory', () => {
describe('execute', () => {
it('should execute normally', async () => {
await showTopNAction.execute(context);
expect(document.body.textContent).toContain('TEST COMPONENT');
expect(mockServices.topValuesPopover.showPopover).toHaveBeenCalled();
});
});
});

View file

@ -4,20 +4,11 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import React from 'react';
import ReactDOM, { unmountComponentAtNode } from 'react-dom';
import type { History } from 'history';
import { Provider } from 'react-redux';
import { Router } from '@kbn/shared-ux-router';
import { i18n } from '@kbn/i18n';
import { createCellActionFactory, type CellActionTemplate } from '@kbn/cell-actions';
import { EuiThemeProvider } from '@kbn/kibana-react-plugin/common';
import { isDataViewFieldSubtypeNested } from '@kbn/es-query';
import { KibanaContextProvider } from '../../../common/lib/kibana';
import { APP_NAME, DEFAULT_DARK_MODE } from '../../../../common/constants';
import type { SecurityAppStore } from '../../../common/store';
import { first } from 'lodash/fp';
import { fieldHasCellActions } from '../../utils';
import { TopNAction } from '../show_top_n_component';
import type { StartServices } from '../../../types';
import type { SecurityCellAction } from '../../types';
import { SecurityCellActionType } from '../../constants';
@ -32,15 +23,7 @@ const SHOW_TOP = (fieldName: string) =>
const ICON = 'visBarVertical';
export const createShowTopNCellActionFactory = createCellActionFactory(
({
store,
history,
services,
}: {
store: SecurityAppStore;
history: History;
services: StartServices;
}): CellActionTemplate<SecurityCellAction> => ({
({ services }: { services: StartServices }): CellActionTemplate<SecurityCellAction> => ({
type: SecurityCellActionType.SHOW_TOP_N,
getIconType: () => ICON,
getDisplayName: ({ data }) => SHOW_TOP(data[0]?.field.name),
@ -57,34 +40,14 @@ export const createShowTopNCellActionFactory = createCellActionFactory(
);
},
execute: async (context) => {
if (!context.nodeRef.current) return;
const firstItem = first(context.data);
if (!context.nodeRef.current || !firstItem) return;
const node = document.createElement('div');
document.body.appendChild(node);
const onClose = () => {
unmountComponentAtNode(node);
document.body.removeChild(node);
};
const element = (
<KibanaContextProvider
services={{
appName: APP_NAME,
...services,
}}
>
<EuiThemeProvider darkMode={services.uiSettings.get(DEFAULT_DARK_MODE)}>
<Provider store={store}>
<Router history={history}>
<TopNAction onClose={onClose} context={context} casesService={services.cases} />
</Router>
</Provider>
</EuiThemeProvider>
</KibanaContextProvider>
);
ReactDOM.render(element, node);
services.topValuesPopover.showPopover({
fieldName: firstItem.field.name,
scopeId: context.metadata?.scopeId,
nodeRef: context.nodeRef.current,
});
},
})
);

View file

@ -1,78 +0,0 @@
/*
* 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 { render } from '@testing-library/react';
import React from 'react';
import { mockCasesContext } from '@kbn/cases-plugin/public/mocks/mock_cases_context';
import { TestProviders } from '../../common/mock';
import { TopNAction } from './show_top_n_component';
import type { CellActionExecutionContext } from '@kbn/cell-actions';
import type { CasesUiStart } from '@kbn/cases-plugin/public';
jest.mock('react-router-dom', () => {
const original = jest.requireActual('react-router-dom');
return {
...original,
useLocation: jest.fn().mockReturnValue({ pathname: '/test' }),
};
});
jest.mock('../../common/components/visualization_actions/actions');
jest.mock('../../common/components/visualization_actions/lens_embeddable');
const casesService = {
ui: { getCasesContext: () => mockCasesContext },
} as unknown as CasesUiStart;
const element = document.createElement('div');
document.body.appendChild(element);
const context = {
data: [
{
value: 'the-value',
field: {
name: 'user.name',
type: 'keyword',
searchable: true,
aggregatable: true,
},
},
],
trigger: { id: 'trigger' },
nodeRef: {
current: element,
},
metadata: undefined,
} as CellActionExecutionContext;
describe('TopNAction', () => {
it('renders', () => {
const { getByTestId } = render(
<TopNAction onClose={() => {}} context={context} casesService={casesService} />,
{
wrapper: TestProviders,
}
);
expect(getByTestId('topN-container')).toBeInTheDocument();
});
it('does not render when nodeRef is null', () => {
const { queryByTestId } = render(
<TopNAction
onClose={() => {}}
context={{ ...context, nodeRef: { current: null } }}
casesService={casesService}
/>,
{
wrapper: TestProviders,
}
);
expect(queryByTestId('topN-container')).not.toBeInTheDocument();
});
});

View file

@ -1,61 +0,0 @@
/*
* 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 React from 'react';
import { EuiWrappingPopover } from '@elastic/eui';
import { useLocation } from 'react-router-dom';
import type { CasesUiStart } from '@kbn/cases-plugin/public';
import { first } from 'lodash/fp';
import { StatefulTopN } from '../../common/components/top_n';
import { useGetUserCasesPermissions } from '../../common/lib/kibana';
import { APP_ID } from '../../../common/constants';
import { getScopeFromPath, useSourcererDataView } from '../../common/containers/sourcerer';
import type { SecurityCellActionExecutionContext } from '../types';
export const TopNAction = ({
onClose,
context,
casesService,
}: {
onClose: () => void;
context: SecurityCellActionExecutionContext;
casesService: CasesUiStart;
}) => {
const { pathname } = useLocation();
const { browserFields, indexPattern } = useSourcererDataView(getScopeFromPath(pathname));
const userCasesPermissions = useGetUserCasesPermissions();
const CasesContext = casesService.ui.getCasesContext();
const { data, nodeRef, metadata } = context;
const firstItem = first(data);
if (!nodeRef?.current || !firstItem) return null;
return (
<CasesContext owner={[APP_ID]} permissions={userCasesPermissions}>
<EuiWrappingPopover
button={nodeRef.current}
isOpen={true}
closePopover={onClose}
anchorPosition={'downCenter'}
hasArrow={false}
repositionOnScroll
ownFocus
attachToAnchor={false}
>
<StatefulTopN
field={firstItem.field.name}
showLegend
scopeId={metadata?.scopeId}
toggleTopN={onClose}
indexPattern={indexPattern}
browserFields={browserFields}
/>
</EuiWrappingPopover>
</CasesContext>
);
};

View file

@ -0,0 +1,81 @@
/*
* 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 { render } from '@testing-library/react';
import React from 'react';
import { TestProviders } from '../../../common/mock';
import { TopValuesPopover } from './top_values_popover';
jest.mock('../../../common/components/visualization_actions/lens_embeddable');
jest.mock('react-router-dom', () => {
const original = jest.requireActual('react-router-dom');
return {
...original,
useLocation: jest.fn().mockReturnValue({ pathname: '/test' }),
};
});
const element = document.createElement('button');
document.body.appendChild(element);
const data = {
fieldName: 'user.name',
nodeRef: element,
};
const mockUseObservable = jest.fn();
jest.mock('react-use', () => ({
...jest.requireActual('react-use'),
useObservable: () => mockUseObservable(),
}));
jest.mock('../../../common/lib/kibana', () => {
const original = jest.requireActual('../../../common/lib/kibana');
return {
...original,
useKibana: () => ({
...original.useKibana(),
services: {
...original.useKibana().services,
topValuesPopover: { getObservable: jest.fn() },
},
}),
};
});
describe('TopNAction', () => {
it('renders', async () => {
mockUseObservable.mockReturnValue(data);
const { queryByTestId } = render(<TopValuesPopover />, {
wrapper: TestProviders,
});
expect(queryByTestId('topN-container')).toBeInTheDocument();
});
it('does not render when nodeRef is null', () => {
mockUseObservable.mockReturnValue({ ...data, nodeRef: undefined });
const { queryByTestId } = render(<TopValuesPopover />, {
wrapper: TestProviders,
});
expect(queryByTestId('topN-container')).not.toBeInTheDocument();
});
it('does not render when data is undefined', () => {
mockUseObservable.mockReturnValue(undefined);
const { queryByTestId } = render(<TopValuesPopover />, {
wrapper: TestProviders,
});
expect(queryByTestId('topN-container')).not.toBeInTheDocument();
});
});

View file

@ -0,0 +1,53 @@
/*
* 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 React, { useCallback } from 'react';
import { EuiWrappingPopover } from '@elastic/eui';
import { useLocation } from 'react-router-dom';
import { useObservable } from 'react-use';
import { StatefulTopN } from '../../../common/components/top_n';
import { getScopeFromPath, useSourcererDataView } from '../../../common/containers/sourcerer';
import { useKibana } from '../../../common/lib/kibana';
export const TopValuesPopover = React.memo(() => {
const { pathname } = useLocation();
const { browserFields, indexPattern } = useSourcererDataView(getScopeFromPath(pathname));
const {
services: { topValuesPopover },
} = useKibana();
const data = useObservable(topValuesPopover.getObservable());
const onClose = useCallback(() => {
topValuesPopover.closePopover();
}, [topValuesPopover]);
if (!data || !data.nodeRef) return null;
return (
<EuiWrappingPopover
isOpen
button={data.nodeRef}
closePopover={onClose}
anchorPosition={'downCenter'}
hasArrow={false}
repositionOnScroll
ownFocus
attachToAnchor={false}
>
<StatefulTopN
field={data.fieldName}
showLegend
scopeId={data.scopeId}
toggleTopN={onClose}
indexPattern={indexPattern}
browserFields={browserFields}
/>
</EuiWrappingPopover>
);
});
TopValuesPopover.displayName = 'TopValuesPopover';

View file

@ -0,0 +1,34 @@
/*
* 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 { BehaviorSubject } from 'rxjs';
interface TopValuesData {
fieldName: string;
scopeId?: string;
nodeRef: HTMLElement;
}
export class TopValuesPopoverService {
private currentPopoverSubject$: BehaviorSubject<TopValuesData | undefined>;
constructor() {
this.currentPopoverSubject$ = new BehaviorSubject<TopValuesData | undefined>(undefined);
}
showPopover(data: TopValuesData) {
return this.currentPopoverSubject$.next(data);
}
closePopover() {
return this.currentPopoverSubject$.next(undefined);
}
getObservable() {
return this.currentPopoverSubject$.asObservable();
}
}

View file

@ -33,6 +33,7 @@ import type { TimelineUrl } from '../../timelines/store/timeline/model';
import { timelineDefaults } from '../../timelines/store/timeline/defaults';
import { URL_PARAM_KEY } from '../../common/hooks/use_url_state';
import { InputsModelId } from '../../common/store/inputs/constants';
import { TopValuesPopoverService } from '../components/top_values_popover/top_values_popover_service';
jest.mock('../../common/store/inputs/actions');
@ -95,11 +96,11 @@ jest.mock('../../timelines/components/open_timeline/helpers', () => {
};
});
const mockGetTimiline = jest.fn();
const mockGetTimeline = jest.fn();
jest.mock('../../timelines/store/timeline', () => ({
timelineSelectors: {
getTimelineByIdSelector: () => mockGetTimiline,
getTimelineByIdSelector: () => mockGetTimeline,
},
}));
@ -124,6 +125,8 @@ const dummyFilter: Filter = {
},
};
const mockTopValuesPopoverService = new TopValuesPopoverService();
jest.mock('../../common/lib/kibana', () => {
const original = jest.requireActual('../../common/lib/kibana');
return {
@ -132,6 +135,7 @@ jest.mock('../../common/lib/kibana', () => {
...original.useKibana(),
services: {
...original.useKibana().services,
topValuesPopover: mockTopValuesPopoverService,
data: {
...original.useKibana().services.data,
dataViews: {
@ -641,7 +645,7 @@ describe('HomePage', () => {
const { rerender } = render(<TestComponent />);
jest.clearAllMocks();
mockGetTimiline.mockReturnValue({ ...timelineDefaults, savedObjectId: null });
mockGetTimeline.mockReturnValue({ ...timelineDefaults, savedObjectId: null });
rerender(<TestComponent />);
@ -669,7 +673,7 @@ describe('HomePage', () => {
const { rerender } = render(<TestComponent />);
jest.clearAllMocks();
mockGetTimiline.mockReturnValue({ ...timelineDefaults, savedObjectId });
mockGetTimeline.mockReturnValue({ ...timelineDefaults, savedObjectId });
rerender(<TestComponent />);

View file

@ -28,6 +28,7 @@ import { useUpdateBrowserTitle } from '../../common/hooks/use_update_browser_tit
import { useUpdateExecutionContext } from '../../common/hooks/use_update_execution_context';
import { useUpgradeSecurityPackages } from '../../detection_engine/rule_management/logic/use_upgrade_security_packages';
import { useSetupDetectionEngineHealthApi } from '../../detection_engine/rule_monitoring';
import { TopValuesPopover } from '../components/top_values_popover/top_values_popover';
interface HomePageProps {
children: React.ReactNode;
@ -61,6 +62,7 @@ const HomePageComponent: React.FC<HomePageProps> = ({ children, setHeaderActionM
{children}
</DragDropContextWrapper>
<HelpMenu />
<TopValuesPopover />
</>
</TourContextProvider>
</ConsoleManager>

View file

@ -54,6 +54,7 @@ import { LazyEndpointCustomAssetsExtension } from './management/pages/policy/vie
import type { SecurityAppStore } from './common/store/types';
import { PluginContract } from './plugin_contract';
import { TopValuesPopoverService } from './app/components/top_values_popover/top_values_popover_service';
export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, StartPlugins> {
/**
@ -195,6 +196,7 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
telemetry: this.telemetry.start(),
discoverFilterManager: filterManager,
customDataService,
topValuesPopover: new TopValuesPopoverService(),
};
return services;
};

View file

@ -75,6 +75,7 @@ import type { TelemetryClientStart } from './common/lib/telemetry';
import type { Dashboards } from './dashboards';
import type { UpsellingService } from './common/lib/upsellings';
import type { BreadcrumbsNav } from './common/breadcrumbs/types';
import type { TopValuesPopoverService } from './app/components/top_values_popover/top_values_popover_service';
export interface SetupPlugins {
cloud?: CloudSetup;
@ -161,6 +162,7 @@ export type StartServices = CoreStart &
telemetry: TelemetryClientStart;
discoverFilterManager: FilterManager;
customDataService: DataPublicPluginStart;
topValuesPopover: TopValuesPopoverService;
};
export interface PluginSetup {