Skip to content

Commit

Permalink
[SIEM] Migrating frontend services to NP (elastic#53669)
Browse files Browse the repository at this point in the history
* [SIEM] Migrating frontend services to NP (elastic#52783)

* Remove legacy index_patterns import

We'd already brought in the new interface in a previous commit; this was
just used as an unnecessary type assertion.

* Update snapshots following new docLink mocks

* Remove unused manual mocks

These are not picked up by jest; calling
jest.mock('lib/compose/kibana_core') has the same effect whether or not
these files exist.

* WIP: Use kibana core mock everywhere we're doing it manually

The timeline tests are the last place we're explicitly mocking
useKibanaCore; removing the mocks cause tests to hang. I think  hey're
relying on the side effects of importing the mock/ui_settings file, but
I'll figure that out next.

* Replace ui/documentation_links with core NP service

In most instances, this meant using the useContext hook with our NP core
context.

This also updates our mocks to leverage the factory so graciously
provided by platform.

There are a few failing tests, mostly due to links being previously
undefined in tests.

* Use new mocks on timeline test that doesn't hang

The rest of these do, though.

* Remove remaining uses of mockUiSettings in useKibanaCore mocks

These have to be evaluated immediately so that we always return the
same core object. Otherwise we get stuck in a loop between
render/useEffect/setState due to the savedObjects client being different
on each invocation.

* Invoke platform's mock factory at mock time

Previously, we were invoking it any time someone called `useKibanaCore`,
getting a new object back each time. This both caused some bugs (looping
with useEffect) and was not representative of how the actual hook
worked.

This also moves that invokation into the mock function, along with
shaping the mocked module so that we don't have to do it in every call
to jest.mock.

* WIP: migrating to use kibana_react's provider and helpers

We're re-exporting these locally to have more control around mocking
them (until platform implements that).

This breaks everything that was using the old mocks. Will fix.

* WIP: Migrating to use kibana_react

Instead of our homegrown hooks we can use these utilities instead.

Unfortunately kibana_react doesn't yet have mocks, so we had to implement
that ourselves. Luckily, we already had local mocks for the settings
service. This migrates to a the new format. For clarity and consistency,
we also re-export new platform's mocks here and use them to populate our
kibana_react mocks.

We started by migrating the UiSettings service to new platform, and let
that drive the rest. With the mocks in place for kibana_react, removing
the usage of useKibanaCore was a natural step as well.

The next step is removing the usage of chrome.getUiSettingsClient with
our useUiSetting$ hook, and with that (and maybe some config setup; I'm
seeing errors at runtime), we should be ready to start migrating other
services.

* Bind a copy of kibana at mock creation

We were previously returning a new copy any time e.g. useKibana was
called, which is not the contract that consumers are expecting. and in
fact caused looping with components employing useEffect etc.

* Remove internal context providers and last usage

We're now using kibana_react fully.

* Fix tests failing due to wrong mocks

Remaining failures are either due to a date format issue, or something
being rendered differently. Those are up next.

Still haven't touched use of chrome.getUiSettingsClient, that's after.

* Fix test failures related to date formatting

* mocks missing UI Setting (DEFAULT_TIMEZONE_BROWSER) which is required
by our formatted_date utilities
* mock timepicker ranges in the one test that uses it (SuperDatePicker)

* Remove unnecessary and/or redundant mocks

Since our TestProvider now mocks new platform, the only tests that
should need to mock uiSettings related stuff (e.g. timezone preferences)
would be the tests that (directly or no) use kibana_react to get it.

* Refactor kibana_react mocks

* adds a mock for the non-observable useUiSetting
* removes the unmockable HOC withKibana

* Replace usage of chrome.getUiSettingsClient with useUiSetting

We're opting for the non-observable behavior here because I believe
that's more analagous.

There are a few remaining usages in non-react code.

Tests are still using the mocks, those'll be removed next.

* Remove ui_settings mocks

Now that we're not using this hook there's no need for the mocks. Tests
are green.

* Remove siem's UI settings hook

We're now using the ones provided by kibana_react.

* Use withKibana HOC on our component classes

React was kind enough to remind me that I can't put hooks in classes.
Whoops.

* Set defaults for some unknown UI settings

The service claims not to know about these settings we're retrieving.
Until I can figure out where they should come from, we're going to
initialize them with what seem to be the defaults at plugin
initialization.

* Remove old hooks

These have now been replaced with kibana_react's equivalents.

* Fix type error on usage of useKibana hook

This is one of the few places where we're using another plugin, which
are not present in the default typings due to their opt-in nature.

* Fix type error on ML call

The indexPattern we get back is actually an array. The endpoint seems to
handle this just fine (at least, it doesn't blow up), but once we
started retrieving a typed value this error surfaced.

* Export a 'bound' version of the useKibana function

Rather than having to type this on each invocation. This requires us to
define which plugins we depend on, which is a good thing.

* Instantiate our mock function

We aren't using these right now I didn't notice, but that wasn't the
right reference.

* Fix test that relies on unmocked service

Our QueryBar component relies very (very, very) indirectly on a storage
service that does not exist in New Platform, nor its corresponding
mocks. To get it passing for now, we're just gonna pretend like it's
there.

* Remove use of ui/chrome in our charts

Replaces with hooks that accomplish the same.

* Remove last use of chrome.getUiSettingsClient

This function is itself a hook, so we should be good here.

* Remove unnecessary non-null assertions

Now that we're using our typed version of the useKibana hook, typescript
knows that these services will be available (once we actually enforce
that in our kibana.json, of course).

* Fix chart tests

These rely on a kibana hook now, so we need to mock it out for these
renders lest we blow up when the context isn't there.

* Replace missing mock

I deleted this in a previous commit, thinking it unneeded.
However, getHostDetailsBreadcrumbs ultimately asks for some
default date parameters for the timerange boundaries.

* Add back tests for our theming hook

* Style: cleanup

* Remove unneeded default UI Settings values

We were previously getting errors due to these values not being known to
the client, but it looks like that was either fixed upstream, or a
temporary issue caused by some improper context setup.

* Simplify kibana_react mocks

Let's leave JSX out of it.

Co-authored-by: Elastic Machine <[email protected]>

* Update references to now-deleted hooks

These hooks were deleted on a recent branch, but new usages were merged
to master in the meantime.

* Fix remaining uses of hooks/chrome that were not merge conflicts

* Use HOC on class component

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
2 people authored and jkelastic committed Jan 3, 2020
1 parent 6dbb46c commit 2c22739
Show file tree
Hide file tree
Showing 166 changed files with 853 additions and 1,127 deletions.
8 changes: 7 additions & 1 deletion x-pack/legacy/plugins/siem/public/apps/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import template from './template.html';

export const ROOT_ELEMENT_ID = 'react-siem-root';

export type StartCore = LegacyCoreStart;
export type StartPlugins = Required<
Pick<PluginsStart, 'data' | 'embeddable' | 'inspector' | 'uiActions'>
>;
export type StartServices = StartCore & StartPlugins;

export class Plugin {
constructor(
// @ts-ignore this is added to satisfy the New Platform typing constraint,
Expand All @@ -25,7 +31,7 @@ export class Plugin {
this.chrome = chrome;
}

public start(core: LegacyCoreStart, plugins: PluginsStart) {
public start(core: StartCore, plugins: StartPlugins) {
// @ts-ignore improper type description
this.chrome.setRootTemplate(template);
const checkForRoot = () => {
Expand Down
40 changes: 15 additions & 25 deletions x-pack/legacy/plugins/siem/public/apps/start_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import React, { memo, FC } from 'react';
import { ApolloProvider } from 'react-apollo';
import { Provider as ReduxStoreProvider } from 'react-redux';
import { ThemeProvider } from 'styled-components';
import { LegacyCoreStart } from 'kibana/public';
import { PluginsStart } from 'ui/new_platform/new_platform';

import { EuiErrorBoundary } from '@elastic/eui';
import euiDarkVars from '@elastic/eui/dist/eui_theme_dark.json';
Expand All @@ -19,16 +17,14 @@ import { BehaviorSubject } from 'rxjs';
import { pluck } from 'rxjs/operators';
import { I18nContext } from 'ui/i18n';

import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public';
import { KibanaContextProvider, useUiSetting$ } from '../lib/kibana';
import { Storage } from '../../../../../../src/plugins/kibana_utils/public';

import { DEFAULT_DARK_MODE } from '../../common/constants';
import { ErrorToastDispatcher } from '../components/error_toast_dispatcher';
import { compose } from '../lib/compose/kibana_compose';
import { AppFrontendLibs } from '../lib/lib';
import { KibanaCoreContextProvider } from '../lib/compose/kibana_core';
import { KibanaPluginsContextProvider } from '../lib/compose/kibana_plugins';
import { useKibanaUiSetting } from '../lib/settings/use_kibana_ui_setting';
import { StartCore, StartPlugins } from './plugin';
import { PageRouter } from '../routes';
import { createStore } from '../store';
import { GlobalToaster, ManageGlobalToaster } from '../components/toasters';
Expand All @@ -44,7 +40,7 @@ const StartApp: FC<AppFrontendLibs> = memo(libs => {
const store = createStore(undefined, libs$.pipe(pluck('apolloClient')));

const AppPluginRoot = memo(() => {
const [darkMode] = useKibanaUiSetting(DEFAULT_DARK_MODE);
const [darkMode] = useUiSetting$<boolean>(DEFAULT_DARK_MODE);
return (
<EuiErrorBoundary>
<I18nContext>
Expand Down Expand Up @@ -77,21 +73,15 @@ const StartApp: FC<AppFrontendLibs> = memo(libs => {

export const ROOT_ELEMENT_ID = 'react-siem-root';

export const SiemApp = memo<{ core: LegacyCoreStart; plugins: PluginsStart }>(
({ core, plugins }) => (
<KibanaContextProvider
services={{
appName: 'siem',
data: plugins.data,
storage: new Storage(localStorage),
...core,
}}
>
<KibanaCoreContextProvider core={core}>
<KibanaPluginsContextProvider plugins={plugins}>
<StartApp {...compose()} />
</KibanaPluginsContextProvider>
</KibanaCoreContextProvider>
</KibanaContextProvider>
)
);
export const SiemApp = memo<{ core: StartCore; plugins: StartPlugins }>(({ core, plugins }) => (
<KibanaContextProvider
services={{
appName: 'siem',
data: plugins.data,
storage: new Storage(localStorage),
...core,
}}
>
<StartApp {...compose()} />
</KibanaContextProvider>
));
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import { useMountAppended } from '../../utils/use_mount_appended';

import { Bytes } from '.';

jest.mock('../../lib/settings/use_kibana_ui_setting');

describe('Bytes', () => {
const mount = useMountAppended();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { AreaChartBaseComponent, AreaChartComponent } from './areachart';
import { ChartSeriesData } from './common';
import { ScaleType, AreaSeries, Axis } from '@elastic/charts';

jest.mock('../../lib/kibana');

const customHeight = '100px';
const customWidth = '120px';
const chartDataSets = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ import { getOr, get, isNull, isNumber } from 'lodash/fp';
import { AutoSizer } from '../auto_sizer';
import { ChartPlaceHolder } from './chart_place_holder';
import {
browserTimezone,
chartDefaultSettings,
ChartSeriesConfigs,
ChartSeriesData,
getChartHeight,
getChartWidth,
getSeriesStyle,
WrappedByAutoSizer,
useTheme,
useBrowserTimeZone,
} from './common';

// custom series styles: https://ela.st/areachart-styling
Expand Down Expand Up @@ -72,12 +73,15 @@ export const AreaChartBaseComponent = ({
height: string | null | undefined;
configs?: ChartSeriesConfigs | undefined;
}) => {
const theme = useTheme();
const timeZone = useBrowserTimeZone();
const xTickFormatter = get('configs.axis.xTickFormatter', chartConfigs);
const yTickFormatter = get('configs.axis.yTickFormatter', chartConfigs);
const xAxisId = getAxisId(`group-${data[0].key}-x`);
const yAxisId = getAxisId(`group-${data[0].key}-y`);
const settings = {
...chartDefaultSettings,
theme,
...get('configs.settings', chartConfigs),
};
return chartConfigs.width && chartConfigs.height ? (
Expand All @@ -95,7 +99,7 @@ export const AreaChartBaseComponent = ({
data={series.value || undefined}
xScaleType={getOr(ScaleType.Linear, 'configs.series.xScaleType', chartConfigs)}
yScaleType={getOr(ScaleType.Linear, 'configs.series.yScaleType', chartConfigs)}
timeZone={browserTimezone}
timeZone={timeZone}
xAccessor="x"
yAccessors={['y']}
areaSeriesStyle={getSeriesLineStyle()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { BarChartBaseComponent, BarChartComponent } from './barchart';
import { ChartSeriesData } from './common';
import { BarSeries, ScaleType, Axis } from '@elastic/charts';

jest.mock('../../lib/kibana');

const customHeight = '100px';
const customWidth = '120px';
const chartDataSets = [
Expand Down
10 changes: 7 additions & 3 deletions x-pack/legacy/plugins/siem/public/components/charts/barchart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ import { getOr, get, isNumber } from 'lodash/fp';
import { AutoSizer } from '../auto_sizer';
import { ChartPlaceHolder } from './chart_place_holder';
import {
browserTimezone,
chartDefaultSettings,
ChartSeriesConfigs,
ChartSeriesData,
checkIfAllValuesAreZero,
getSeriesStyle,
getChartHeight,
getChartWidth,
getSeriesStyle,
SeriesType,
WrappedByAutoSizer,
useBrowserTimeZone,
useTheme,
} from './common';

const checkIfAllTheDataInTheSeriesAreValid = (series: ChartSeriesData): series is ChartSeriesData =>
Expand All @@ -53,13 +54,16 @@ export const BarChartBaseComponent = ({
height: string | null | undefined;
configs?: ChartSeriesConfigs | undefined;
}) => {
const theme = useTheme();
const timeZone = useBrowserTimeZone();
const xTickFormatter = get('configs.axis.xTickFormatter', chartConfigs);
const yTickFormatter = get('configs.axis.yTickFormatter', chartConfigs);
const tickSize = getOr(0, 'configs.axis.tickSize', chartConfigs);
const xAxisId = getAxisId(`stat-items-barchart-${data[0].key}-x`);
const yAxisId = getAxisId(`stat-items-barchart-${data[0].key}-y`);
const settings = {
...chartDefaultSettings,
theme,
...get('configs.settings', chartConfigs),
};

Expand All @@ -79,7 +83,7 @@ export const BarChartBaseComponent = ({
yScaleType={getOr(ScaleType.Linear, 'configs.series.yScaleType', chartConfigs)}
xAccessor="x"
yAccessors={['y']}
timeZone={browserTimezone}
timeZone={timeZone}
splitSeriesAccessors={['g']}
data={series.value!}
stackAccessors={get('configs.series.stackAccessors', chartConfigs)}
Expand Down
44 changes: 26 additions & 18 deletions x-pack/legacy/plugins/siem/public/components/charts/common.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,27 @@
*/
import { shallow } from 'enzyme';
import React from 'react';
import { renderHook } from '@testing-library/react-hooks';

import { useUiSetting } from '../../lib/kibana';
import {
checkIfAllValuesAreZero,
defaultChartHeight,
getChartHeight,
getChartWidth,
getSeriesStyle,
getTheme,
SeriesType,
WrappedByAutoSizer,
ChartSeriesData,
useTheme,
} from './common';
import { mergeWithDefaultTheme, LIGHT_THEME } from '@elastic/charts';

jest.mock('../../lib/kibana');

jest.mock('@elastic/charts', () => {
return {
...jest.requireActual('@elastic/charts'),
getSpecId: jest.fn(() => {}),
mergeWithDefaultTheme: jest.fn(),
};
});

Expand Down Expand Up @@ -57,21 +61,6 @@ describe('getSeriesStyle', () => {
});
});

describe('getTheme', () => {
it('should merge custom theme with default theme', () => {
const defaultTheme = {
chartMargins: { bottom: 0, left: 0, right: 0, top: 4 },
chartPaddings: { bottom: 0, left: 0, right: 0, top: 0 },
scales: {
barsPadding: 0.05,
},
};
getTheme();
expect((mergeWithDefaultTheme as jest.Mock).mock.calls[0][0]).toMatchObject(defaultTheme);
expect((mergeWithDefaultTheme as jest.Mock).mock.calls[0][1]).toEqual(LIGHT_THEME);
});
});

describe('getChartHeight', () => {
it('should render customHeight', () => {
const height = getChartHeight(10, 100);
Expand Down Expand Up @@ -197,4 +186,23 @@ describe('checkIfAllValuesAreZero', () => {
expect(result).toBeTruthy();
});
});

describe('useTheme', () => {
it('merges our spacing with the default theme', () => {
const { result } = renderHook(() => useTheme());

expect(result.current).toEqual(
expect.objectContaining({ chartMargins: expect.objectContaining({ top: 4, bottom: 0 }) })
);
});

it('returns a different theme depending on user settings', () => {
const { result: defaultResult } = renderHook(() => useTheme());
(useUiSetting as jest.Mock).mockImplementation(() => true);

const { result: darkResult } = renderHook(() => useTheme());

expect(defaultResult.current).not.toMatchObject(darkResult.current);
});
});
});
50 changes: 26 additions & 24 deletions x-pack/legacy/plugins/siem/public/components/charts/common.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import chrome from 'ui/chrome';
import {
CustomSeriesColorsMap,
DARK_THEME,
Expand All @@ -21,6 +20,7 @@ import {
} from '@elastic/charts';
import moment from 'moment-timezone';
import styled from 'styled-components';
import { useUiSetting } from '../../lib/kibana';
import { DEFAULT_DATE_FORMAT_TZ, DEFAULT_DARK_MODE } from '../../../common/constants';

export const defaultChartHeight = '100%';
Expand Down Expand Up @@ -95,27 +95,28 @@ export const getSeriesStyle = (
};

// Apply margins and paddings: https://ela.st/charts-spacing
export const getTheme = () => {
const theme: PartialTheme = {
chartMargins: {
left: 0,
right: 0,
// Apply some paddings to the top to avoid chopping the y tick https://ela.st/chopping-edge
top: 4,
bottom: 0,
},
chartPaddings: {
left: 0,
right: 0,
top: 0,
bottom: 0,
},
scales: {
barsPadding: 0.05,
},
};
const isDarkMode: boolean = chrome.getUiSettingsClient().get(DEFAULT_DARK_MODE);
const theme: PartialTheme = {
chartMargins: {
left: 0,
right: 0,
// Apply some paddings to the top to avoid chopping the y tick https://ela.st/chopping-edge
top: 4,
bottom: 0,
},
chartPaddings: {
left: 0,
right: 0,
top: 0,
bottom: 0,
},
scales: {
barsPadding: 0.05,
},
};
export const useTheme = () => {
const isDarkMode = useUiSetting<boolean>(DEFAULT_DARK_MODE);
const defaultTheme = isDarkMode ? DARK_THEME : LIGHT_THEME;

return mergeWithDefaultTheme(theme, defaultTheme);
};

Expand All @@ -126,11 +127,12 @@ export const chartDefaultSettings = {
showLegend: false,
showLegendDisplayValue: false,
debug: false,
theme: getTheme(),
};

const kibanaTimezone: string = chrome.getUiSettingsClient().get(DEFAULT_DATE_FORMAT_TZ);
export const browserTimezone = kibanaTimezone === 'Browser' ? moment.tz.guess() : kibanaTimezone;
export const useBrowserTimeZone = () => {
const kibanaTimezone = useUiSetting<string>(DEFAULT_DATE_FORMAT_TZ);
return kibanaTimezone === 'Browser' ? moment.tz.guess() : kibanaTimezone;
};

export const getChartHeight = (customHeight?: number, autoSizerHeight?: number): string => {
const height = customHeight || autoSizerHeight;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { mount, shallow } from 'enzyme';
import toJson from 'enzyme-to-json';
import React from 'react';

import '../../../mock/ui_settings';
import { TestProviders } from '../../../mock';
import {
UtilityBar,
Expand All @@ -19,8 +18,6 @@ import {
UtilityBarText,
} from './index';

jest.mock('../../../lib/settings/use_kibana_ui_setting');

describe('UtilityBar', () => {
test('it renders', () => {
const wrapper = shallow(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ import { mount, shallow } from 'enzyme';
import toJson from 'enzyme-to-json';
import React from 'react';

import '../../../mock/ui_settings';
import { TestProviders } from '../../../mock';
import { UtilityBarAction } from './index';

jest.mock('../../../lib/settings/use_kibana_ui_setting');

describe('UtilityBarAction', () => {
test('it renders', () => {
const wrapper = shallow(
Expand Down
Loading

0 comments on commit 2c22739

Please sign in to comment.