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

[SIEM] Fix anomaly table #5

Merged
merged 5 commits into from
Sep 5, 2019
Merged

[SIEM] Fix anomaly table #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9bbebed
refactor code to use react-router params instead of regex inside of u…
XavierM Aug 31, 2019
ecfe07a
add search in breadcrumb + fix existing unit testing
XavierM Sep 4, 2019
18179d5
[SIEM] Fixes the Anomalies table paging to work again
FrankHassanabad Sep 5, 2019
728e938
Merge branch 'pr/44671' into fix-anomaly-table
FrankHassanabad Sep 5, 2019
1d0f417
Updated to look more like the PR in flight to have a smaller diff
FrankHassanabad Sep 5, 2019
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
14 changes: 12 additions & 2 deletions x-pack/legacy/plugins/siem/public/pages/home/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,18 @@ export const HomePage = pure(() => (
<Switch>
<Redirect from="/" exact={true} to="/overview" />
<Route path="/:pageName(overview)" render={() => <Overview />} />
<Route path="/:pageName(hosts)" component={HostsContainer} />
<Route path="/:pageName(network)" component={NetworkContainer} />
<Route
path="/:pageName(hosts)"
render={({ match, location }) => (
<HostsContainer url={match.url} location={location} />
)}
/>
<Route
path="/:pageName(network)"
render={({ match, location }) => (
<NetworkContainer url={match.url} location={location} />
)}
/>
Copy link
Author

Choose a reason for hiding this comment

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

Note: Both of these MUST be render in order to avoid passing down the match as it is a newly created object every single time Route decides to render (which can be a lot).

Also even though the downstream React.memo component doesn't need location, adding the location causes it to update correctly and re-render when we switch tabs (which is what we want)

<Route path="/:pageName(timelines)" render={() => <Timelines />} />
<Route path="/link-to" component={LinkToPage} />
<Route path="/ml-hosts" component={MlHostConditionalContainer} />
Expand Down
4 changes: 1 addition & 3 deletions x-pack/legacy/plugins/siem/public/pages/hosts/details/body.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ const HostDetailsBodyComponent = React.memo<HostDetailsBodyComponentProps>(
filterQueryExpression,
from,
isInitializing,
match: {
params: { detailName, tabName },
},
detailName,
Copy link
Author

@FrankHassanabad FrankHassanabad Sep 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Note: In these cases we have to remove match and can longer pass it down. Instead we have to destructure the match into its constituent pieces of things such as strings so React.memo does not do additional rendering.

setAbsoluteRangeDatePicker,
setQuery,
to,
Expand Down
7 changes: 2 additions & 5 deletions x-pack/legacy/plugins/siem/public/pages/hosts/details/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { StickyContainer } from 'react-sticky';
import { FiltersGlobal } from '../../../components/filters_global';
import { HeaderPage } from '../../../components/header_page';
import { LastEventTime } from '../../../components/last_event_time';
import { HostComponentProps } from '../../../components/link_to/redirect_to_hosts';

import { HostOverviewByNameQuery } from '../../../containers/hosts/overview';
import { indicesExistOrDataTemporarilyUnavailable, WithSource } from '../../../containers/source';
Expand Down Expand Up @@ -47,9 +46,7 @@ const HostDetailsComponent = React.memo<HostDetailsComponentProps>(
isInitializing,
filterQueryExpression,
from,
match: {
params: { detailName, tabName },
},
detailName,
Copy link
Author

Choose a reason for hiding this comment

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

Note: In these cases we have to remove match and can longer pass it down. Instead we have to destructure the match into its constituent pieces of things such as strings so React.memo does not do additional rendering.

setQuery,
setAbsoluteRangeDatePicker,
to,
Expand Down Expand Up @@ -168,7 +165,7 @@ const HostDetailsComponent = React.memo<HostDetailsComponentProps>(

HostDetailsComponent.displayName = 'HostDetailsComponent';

export const HostDetails = compose<React.ComponentClass<HostsQueryProps & HostComponentProps>>(
export const HostDetails = compose<React.ComponentClass<HostsQueryProps & { detailName: string }>>(
connect(
makeMapStateToProps,
{
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/siem/public/pages/hosts/details/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface HostDetailsComponentDispatchProps {
from: number;
to: number;
}>;
detailName: string;
}

export interface HostDetailsBodyProps extends HostsQueryProps {
Expand All @@ -33,5 +34,4 @@ export type HostDetailsComponentProps = HostDetailsComponentReduxProps &

export type HostDetailsBodyComponentProps = HostDetailsComponentReduxProps &
HostDetailsComponentDispatchProps &
HostComponentProps &
HostDetailsBodyProps;
35 changes: 14 additions & 21 deletions x-pack/legacy/plugins/siem/public/pages/hosts/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
*/

import React from 'react';
import { Redirect, Route, Switch } from 'react-router-dom';
import { pure } from 'recompose';

import { HostComponentProps } from '../../components/link_to/redirect_to_hosts';
import { Redirect, Route, Switch, RouteComponentProps } from 'react-router-dom';

import { HostDetailsBody, HostDetails } from './details';
import {
Expand Down Expand Up @@ -40,15 +37,17 @@ const getHostDetailsTabPath = (pagePath: string) =>
`${HostsTableType.anomalies}|` +
`${HostsTableType.events})`;

export const HostsContainer = pure<HostComponentProps>(({ match }) => (
type Props = Partial<RouteComponentProps<{}>> & { url: string };

export const HostsContainer = React.memo<Props>(({ url }) => (
<GlobalTime>
{({ to, from, setQuery, isInitializing }) => (
<Switch>
<Route
strict
exact
path={hostsPagePath}
render={props => (
render={() => (
<Route
path={hostsPagePath}
render={() => (
Expand All @@ -59,7 +58,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
children={HostsQueryTabBody}
Copy link
Author

Choose a reason for hiding this comment

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

Note: I removed props spreading as that was causing additional properties such as a match and location being sent down into the component which causes additional rendering.

Props spreading is interesting in that TypeScript does not alert us to the additional un-needed attributes being pushed down. Any extra values being pushed down can cause additional rendering such as in our case where match will be a new object and cause re-rendering.

/>
</>
Expand All @@ -71,7 +69,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
strict
exact
path={getHostsTabPath(hostsPagePath)}
render={props => (
render={() => (
<>
<Hosts from={from} to={to} setQuery={setQuery} isInitializing={isInitializing} />
<Route
Expand All @@ -82,7 +80,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
Copy link
Author

Choose a reason for hiding this comment

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

Note: I removed props spreading as that was causing additional properties such as a match and location being sent down into the component which causes additional rendering.

Props spreading is interesting in that TypeScript does not alert us to the additional un-needed attributes being pushed down. Any extra values being pushed down can cause additional rendering such as in our case where match will be a new object and cause re-rendering.

children={HostsQueryTabBody}
/>
)}
Expand All @@ -95,7 +92,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
Copy link
Author

Choose a reason for hiding this comment

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

Note: I removed props spreading as that was causing additional properties such as a match and location being sent down into the component which causes additional rendering.

Props spreading is interesting in that TypeScript does not alert us to the additional un-needed attributes being pushed down. Any extra values being pushed down can cause additional rendering such as in our case where match will be a new object and cause re-rendering.

children={AuthenticationsQueryTabBody}
/>
)}
Expand All @@ -108,7 +104,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
Copy link
Author

Choose a reason for hiding this comment

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

Note: I removed props spreading as that was causing additional properties such as a match and location being sent down into the component which causes additional rendering.

Props spreading is interesting in that TypeScript does not alert us to the additional un-needed attributes being pushed down. Any extra values being pushed down can cause additional rendering such as in our case where match will be a new object and cause re-rendering.

children={UncommonProcessTabBody}
/>
)}
Expand All @@ -121,7 +116,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
children={AnomaliesTabBody}
/>
)}
Expand All @@ -134,7 +128,6 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
children={EventsTabBody}
/>
)}
Expand All @@ -153,7 +146,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
detailName={props.match.params.detailName}
Copy link
Author

Choose a reason for hiding this comment

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

Note: Changed out the props spreading which includes the match object in favor of just the detail parameter name as that is a string and will not cause additional rendering of the React.memo.

/>
<Route
path={`${hostsPagePath}/:detailName/:tabName(${HostsTableType.hosts})`}
Expand All @@ -163,8 +156,8 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
children={HostsQueryTabBody}
detailName={props.match.params.detailName}
Copy link
Author

Choose a reason for hiding this comment

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

Note: Changed out the props spreading which includes the match object in favor of just the detail parameter name as that is a string and will not cause additional rendering of the React.memo.

/>
)}
/>
Expand All @@ -176,7 +169,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
detailName={props.match.params.detailName}
children={AuthenticationsQueryTabBody}
/>
)}
Expand All @@ -189,7 +182,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
detailName={props.match.params.detailName}
children={UncommonProcessTabBody}
/>
)}
Expand All @@ -202,7 +195,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
detailName={props.match.params.detailName}
children={AnomaliesTabBody}
/>
)}
Expand All @@ -215,7 +208,7 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
to={to}
setQuery={setQuery}
isInitializing={isInitializing}
{...props}
detailName={props.match.params.detailName}
children={EventsTabBody}
/>
)}
Expand All @@ -224,8 +217,8 @@ export const HostsContainer = pure<HostComponentProps>(({ match }) => (
)}
/>
<Redirect
from={`${match.url}/:detailName`}
to={`${match.url}/:detailName/${HostsTableType.authentications}`}
from={`${url}/:detailName`}
to={`${url}/:detailName/${HostsTableType.authentications}`}
Copy link
Author

Choose a reason for hiding this comment

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

Replaced the match with just the string url so this React.memo won't re-render its self continuously along with its children since the React match will be created anew all the time.

/>
<Redirect from="/hosts/" to={`/hosts/${HostsTableType.hosts}`} />
</Switch>
Expand Down
1 change: 1 addition & 0 deletions x-pack/legacy/plugins/siem/public/pages/network/__snapshots__/ip_details.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 17 additions & 13 deletions x-pack/legacy/plugins/siem/public/pages/network/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,28 @@
*/

import React from 'react';
import { Redirect, Route, Switch } from 'react-router-dom';

import { pure } from 'recompose';

import { NetworkComponentProps } from '../../components/link_to/redirect_to_network';
import { Redirect, Route, Switch, RouteComponentProps } from 'react-router-dom';

import { IPDetails } from './ip_details';
import { Network } from './network';

const networkPath = `/:pageName(network)`;
export const NetworkContainer = pure<NetworkComponentProps>(({ match }) => (
<>
<Switch>
<Route strict exact path={networkPath} render={props => <Network {...props} />} />
<Route path={`${networkPath}/ip/:detailName`} render={props => <IPDetails {...props} />} />
<Redirect from="/network/" to="/network" />
</Switch>
</>

type Props = Partial<RouteComponentProps<{}>> & { url: string };

export const NetworkContainer = React.memo<Props>(() => (
<Switch>
<Route strict exact path={networkPath} render={() => <Network />} />
<Route
path={`${networkPath}/ip/:detailName`}
render={({
match: {
params: { detailName },
},
}) => <IPDetails detailName={detailName} />}
Copy link
Author

Choose a reason for hiding this comment

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

Note: Removed the ...props spreading as that carries all the extra properties we are not interested in here as well as above in the ``` which didn't need the additional properties.

By removing the additional props spreading and just pushing down the string of detailName this will make it so the container does not constantly re-render because of the match object re-creation from react-router

/>
<Redirect from="/network/" to="/network" />
</Switch>
));

NetworkContainer.displayName = 'NetworkContainer';
1 change: 1 addition & 0 deletions x-pack/legacy/plugins/siem/public/pages/network/ip_details.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const getMockProps = (ip: string) => ({
state: '',
hash: '',
},
detailName: ip,
match: { params: { detailName: ip, search: '' }, isExact: true, path: '', url: '' },
setAbsoluteRangeDatePicker: (jest.fn() as unknown) as ActionCreator<{
id: InputsModelId;
Expand Down
13 changes: 3 additions & 10 deletions x-pack/legacy/plugins/siem/public/pages/network/ip_details.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { ActionCreator } from 'typescript-fsa';
import { FiltersGlobal } from '../../components/filters_global';
import { HeaderPage } from '../../components/header_page';
import { LastEventTime } from '../../components/last_event_time';
import { getNetworkUrl, NetworkComponentProps } from '../../components/link_to/redirect_to_network';
import { getNetworkUrl } from '../../components/link_to/redirect_to_network';
import { manageQuery } from '../../components/page/manage_query';
import { DomainsTable } from '../../components/page/network/domains_table';
import { FlowTargetSelectConnected } from '../../components/page/network/flow_target_select_connected';
Expand Down Expand Up @@ -59,17 +59,10 @@ interface IPDetailsComponentReduxProps {
}>;
}

export type IPDetailsComponentProps = IPDetailsComponentReduxProps & NetworkComponentProps;
export type IPDetailsComponentProps = IPDetailsComponentReduxProps & { detailName: string };

export const IPDetailsComponent = pure<IPDetailsComponentProps>(
({
match: {
params: { detailName },
},
filterQuery,
flowTarget,
setAbsoluteRangeDatePicker,
}) => (
({ detailName, filterQuery, flowTarget, setAbsoluteRangeDatePicker }) => (
<>
<WithSource sourceId="default" data-test-subj="ip-details-page">
{({ indicesExist, indexPattern }) => {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/legacy/plugins/siem/public/pages/network/network.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { connect } from 'react-redux';
import { StickyContainer } from 'react-sticky';

import { ActionCreator } from 'typescript-fsa';
import { RouteComponentProps } from 'react-router-dom';
import { FiltersGlobal } from '../../components/filters_global';
import { HeaderPage } from '../../components/header_page';
import { LastEventTime } from '../../components/last_event_time';
Expand Down Expand Up @@ -50,7 +51,7 @@ interface NetworkComponentReduxProps {
}>;
}

type NetworkComponentProps = NetworkComponentReduxProps;
type NetworkComponentProps = NetworkComponentReduxProps & Partial<RouteComponentProps<{}>>;
const mediaMatch = window.matchMedia(
'screen and (min-width: ' + euiLightVars.euiBreakpoints.xl + ')'
);
Expand Down