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 link on overview page #60348

Merged
merged 7 commits into from
Mar 17, 2020
Merged

[SIEM] Fix link on overview page #60348

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
54cf075
Fix link on overview page
XavierM Mar 17, 2020
3c1d9cc
no needs of useMemo
XavierM Mar 17, 2020
6f000ef
clean up
XavierM Mar 17, 2020
1378b84
review I
XavierM Mar 17, 2020
6828519
review II
XavierM Mar 17, 2020
68e8a97
review III
XavierM Mar 17, 2020
31e7961
Merge branch 'master' of github.com:elastic/kibana into overview-link
XavierM Mar 17, 2020
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
7 changes: 4 additions & 3 deletions x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_detection_engine.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ export const RedirectToEditRulePage = ({

const baseDetectionEngineUrl = `#/link-to/${DETECTION_ENGINE_PAGE_NAME}`;

export const getDetectionEngineUrl = () => `${baseDetectionEngineUrl}`;
export const getDetectionEngineAlertUrl = () =>
`${baseDetectionEngineUrl}/${DetectionEngineTab.alerts}`;
export const getDetectionEngineUrl = (search?: string) =>
`${baseDetectionEngineUrl}${search != null ? `?${search}` : ''}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider creating a utility function for ${search != null ? ?${search} : ''}, because there are six instances of it in this PR, and likely additional instances in the future

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it's possible / valid to change the type signatures of the functions that take (search?: string) to something like (search: string | null), but an advantage of such a change is the type checker would force the caller to always handle this case by passing search or an explicit null when search should not be provided.

For example, making this parameter non-optional makes the type checker flag the following code in x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/index.tsx:

        <DetectionEngineHeaderPage
          backOptions={{
            href: getDetectionEngineUrl(),
            text: i18n.BACK_TO_DETECTION_ENGINE,
          }}
          title={i18n.PAGE_TITLE}
        >
  • If the above code really should be passing search, then the type checker just found a bug
  • If the above code is a valid case to not pass search, we would be forced to explicitly pass null, which feels OK to me
  • Making search non-optional also causes the type checker to flag getBreadcrumbs in x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/utils.ts because it uses a slight variation of the "search checking pattern", where in this case it's using search[0] instead:
${!isEmpty(search[0]) ? search[0] : ''}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why I did it like that because I just wanted to focus this PR on the overview page. Also, I remember that we made a decision that we did not want the breadcrumb to follow the url state for some reason. (maybe we can discuss it in one of our meetings)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but I mean you are correct, we should have a follow-up PRs to get all link through the app corrected and discussed breadcrumb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like all breadcrumbs have the URL state, so that resolved the question above. I will go over in another PR to make everything following the same pattern. so the breadcrumb's URL can be also simplified from this PR.

export const getDetectionEngineAlertUrl = (search?: string) =>
`${baseDetectionEngineUrl}/${DetectionEngineTab.alerts}${search != null ? `?${search}` : ''}`;
export const getDetectionEngineTabUrl = (tabPath: string) => `${baseDetectionEngineUrl}/${tabPath}`;
export const getRulesUrl = () => `${baseDetectionEngineUrl}/rules`;
export const getCreateRuleUrl = () => `${baseDetectionEngineUrl}/rules/create`;
Expand Down
6 changes: 4 additions & 2 deletions x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_hosts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ export const RedirectToHostDetailsPage = ({

const baseHostsUrl = `#/link-to/${SiemPageName.hosts}`;

export const getHostsUrl = () => baseHostsUrl;
export const getHostsUrl = (search?: string) =>
`${baseHostsUrl}${search != null ? `?${search}` : ''}`;

export const getTabsOnHostsUrl = (tabName: HostsTableType) => `${baseHostsUrl}/${tabName}`;
export const getTabsOnHostsUrl = (tabName: HostsTableType, search?: string) =>
`${baseHostsUrl}/${tabName}${search != null ? `?${search}` : ''}`;

export const getHostDetailsUrl = (detailName: string) => `${baseHostsUrl}/${detailName}`;

Expand Down
3 changes: 2 additions & 1 deletion x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_network.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export const RedirectToNetworkPage = ({
);

const baseNetworkUrl = `#/link-to/${SiemPageName.network}`;
export const getNetworkUrl = () => baseNetworkUrl;
export const getNetworkUrl = (search?: string) =>
`${baseNetworkUrl}${search != null ? `?${search}` : ''}`;
export const getIPDetailsUrl = (
detailName: string,
flowTarget?: FlowTarget | FlowTargetSourceDest
Expand Down
3 changes: 2 additions & 1 deletion x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_timelines.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ export const RedirectToTimelinesPage = ({ location: { search } }: TimelineCompon
<RedirectWrapper to={`/${SiemPageName.timelines}${search}`} />
);

export const getTimelinesUrl = () => `#/link-to/${SiemPageName.timelines}`;
export const getTimelinesUrl = (search?: string) =>
`#/link-to/${SiemPageName.timelines}${search != null ? `?${search}` : ''}`;
7 changes: 3 additions & 4 deletions x-pack/legacy/plugins/siem/public/components/navigation/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@ import { Location } from 'history';
import { UrlInputsModel } from '../../store/inputs/model';
import { TimelineUrl } from '../../store/timeline/model';
import { CONSTANTS } from '../url_state/constants';
import { URL_STATE_KEYS, KeyUrlState } from '../url_state/types';
import { URL_STATE_KEYS, KeyUrlState, UrlState } from '../url_state/types';
import {
replaceQueryStringInLocation,
replaceStateKeyInQueryString,
getQueryStringFromLocation,
} from '../url_state/helpers';
import { Query, Filter } from '../../../../../../../src/plugins/data/public';

import { TabNavigationProps } from './tab_navigation/types';
import { SearchNavTab } from './types';

export const getSearch = (tab: SearchNavTab, urlState: TabNavigationProps): string => {
export const getSearch = (tab: SearchNavTab, urlState: UrlState): string => {
if (tab && tab.urlKey != null && URL_STATE_KEYS[tab.urlKey] != null) {
return URL_STATE_KEYS[tab.urlKey].reduce<Location>(
(myLocation: Location, urlKey: KeyUrlState) => {
Expand Down Expand Up @@ -58,7 +57,7 @@ export const getSearch = (tab: SearchNavTab, urlState: TabNavigationProps): stri
);
},
{
pathname: urlState.pathName,
pathname: '',
hash: '',
search: '',
state: '',
Expand Down
4 changes: 3 additions & 1 deletion x-pack/legacy/plugins/siem/public/components/navigation/tab_navigation/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ export const TabNavigationComponent = (props: TabNavigationProps) => {
() =>
Object.values(navTabs).map(tab => {
const isSelected = selectedTabId === tab.id;
const hrefWithSearch = tab.href + getSearch(tab, props);
const { query, filters, savedQuery, timerange, timeline } = props;
const hrefWithSearch =
tab.href + getSearch(tab, { query, filters, savedQuery, timerange, timeline });

return (
<TabNavigationItem
Expand Down
20 changes: 20 additions & 0 deletions x-pack/legacy/plugins/siem/public/components/navigation/use_get_url_search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { isEqual } from 'lodash/fp';
import { useMemo } from 'react';
import { useSelector } from 'react-redux';

import { makeMapStateToProps } from '../url_state/helpers';
import { getSearch } from './helpers';
import { SearchNavTab } from './types';

export const useGetUrlSearch = (tab: SearchNavTab) => {
const mapState = makeMapStateToProps();
const { urlState } = useSelector(mapState, isEqual);
const urlSearch = useMemo(() => getSearch(tab, urlState), [tab, urlState]);
return urlSearch;
};
21 changes: 13 additions & 8 deletions x-pack/legacy/plugins/siem/public/components/page/overview/overview_host/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { isEmpty } from 'lodash/fp';
import { EuiButton, EuiFlexItem, EuiPanel } from '@elastic/eui';
import numeral from '@elastic/numeral';
import { FormattedMessage } from '@kbn/i18n/react';
import React from 'react';
import React, { useMemo } from 'react';

import { DEFAULT_NUMBER_FORMAT } from '../../../../../common/constants';
import { ESQuery } from '../../../../../common/typed_json';
Expand All @@ -23,6 +23,8 @@ import { getOverviewHostStats, OverviewHostStats } from '../overview_host_stats'
import { manageQuery } from '../../../page/manage_query';
import { inputsModel } from '../../../../store/inputs';
import { InspectButtonContainer } from '../../../inspect';
import { useGetUrlSearch } from '../../../navigation/use_get_url_search';
import { navTabs } from '../../../../pages/home/home_navigations';

export interface OwnProps {
startDate: number;
Expand Down Expand Up @@ -51,7 +53,15 @@ const OverviewHostComponent: React.FC<OverviewHostProps> = ({
setQuery,
}) => {
const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);

const urlSearch = useGetUrlSearch(navTabs.hosts);
const hostPageButton = useMemo(
() => (
<EuiButton href={getHostsUrl(urlSearch)}>
<FormattedMessage id="xpack.siem.overview.hostsAction" defaultMessage="View hosts" />
</EuiButton>
),
[urlSearch]
);
return (
<EuiFlexItem>
<InspectButtonContainer>
Expand Down Expand Up @@ -95,12 +105,7 @@ const OverviewHostComponent: React.FC<OverviewHostProps> = ({
/>
}
>
<EuiButton href={getHostsUrl()}>
<FormattedMessage
id="xpack.siem.overview.hostsAction"
defaultMessage="View hosts"
/>
</EuiButton>
{hostPageButton}
</HeaderSection>

<OverviewHostStatsManage
Expand Down
21 changes: 13 additions & 8 deletions x-pack/legacy/plugins/siem/public/components/page/overview/overview_network/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { isEmpty } from 'lodash/fp';
import { EuiButton, EuiFlexItem, EuiPanel } from '@elastic/eui';
import numeral from '@elastic/numeral';
import { FormattedMessage } from '@kbn/i18n/react';
import React from 'react';
import React, { useMemo } from 'react';

import { DEFAULT_NUMBER_FORMAT } from '../../../../../common/constants';
import { ESQuery } from '../../../../../common/typed_json';
Expand All @@ -23,6 +23,8 @@ import { inputsModel } from '../../../../store/inputs';
import { getOverviewNetworkStats, OverviewNetworkStats } from '../overview_network_stats';
import { getNetworkUrl } from '../../../link_to';
import { InspectButtonContainer } from '../../../inspect';
import { useGetUrlSearch } from '../../../navigation/use_get_url_search';
import { navTabs } from '../../../../pages/home/home_navigations';

export interface OverviewNetworkProps {
startDate: number;
Expand Down Expand Up @@ -50,7 +52,15 @@ const OverviewNetworkComponent: React.FC<OverviewNetworkProps> = ({
setQuery,
}) => {
const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);

const urlSearch = useGetUrlSearch(navTabs.network);
const networkPageButton = useMemo(
() => (
<EuiButton href={getNetworkUrl(urlSearch)}>
<FormattedMessage id="xpack.siem.overview.networkAction" defaultMessage="View network" />
</EuiButton>
),
[urlSearch]
);
return (
<EuiFlexItem>
<InspectButtonContainer>
Expand Down Expand Up @@ -96,12 +106,7 @@ const OverviewNetworkComponent: React.FC<OverviewNetworkProps> = ({
/>
}
>
<EuiButton href={getNetworkUrl()}>
<FormattedMessage
id="xpack.siem.overview.networkAction"
defaultMessage="View network"
/>
</EuiButton>
{networkPageButton}
</HeaderSection>

<OverviewNetworkStatsManage
Expand Down
14 changes: 10 additions & 4 deletions x-pack/legacy/plugins/siem/public/components/recent_timelines/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import ApolloClient from 'apollo-client';
import { EuiHorizontalRule, EuiLink, EuiText } from '@elastic/eui';
import React, { useCallback } from 'react';
import React, { useCallback, useMemo } from 'react';
import { connect, ConnectedProps } from 'react-redux';
import { Dispatch } from 'redux';
import { ActionCreator } from 'typescript-fsa';
Expand All @@ -21,6 +21,9 @@ import { updateIsLoading as dispatchUpdateIsLoading } from '../../store/timeline
import { RecentTimelines } from './recent_timelines';
import * as i18n from './translations';
import { FilterMode } from './types';
import { useGetUrlSearch } from '../navigation/use_get_url_search';
import { navTabs } from '../../pages/home/home_navigations';
import { getTimelinesUrl } from '../link_to/redirect_to_timelines';

interface OwnProps {
apolloClient: ApolloClient<{}>;
Expand All @@ -47,6 +50,11 @@ const StatefulRecentTimelinesComponent = React.memo<Props>(

const noTimelinesMessage =
filterBy === 'favorites' ? i18n.NO_FAVORITE_TIMELINES : i18n.NO_TIMELINES;
const urlSearch = useGetUrlSearch(navTabs.timelines);
const linkAllTimelines = useMemo(
() => <EuiLink href={getTimelinesUrl(urlSearch)}>{i18n.VIEW_ALL_TIMELINES}</EuiLink>,
[urlSearch]
);

return (
<AllTimelinesQuery
Expand All @@ -73,9 +81,7 @@ const StatefulRecentTimelinesComponent = React.memo<Props>(
/>
)}
<EuiHorizontalRule margin="s" />
<EuiText size="xs">
<EuiLink href="#/link-to/timelines">{i18n.VIEW_ALL_TIMELINES}</EuiLink>
</EuiText>
<EuiText size="xs">{linkAllTimelines}</EuiText>
</>
)}
</AllTimelinesQuery>
Expand Down
38 changes: 2 additions & 36 deletions x-pack/legacy/plugins/siem/public/components/url_state/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@ import { TimelineUrl } from '../../store/timeline/model';
import { formatDate } from '../super_date_picker';
import { NavTab } from '../navigation/types';
import { CONSTANTS, UrlStateType } from './constants';
import {
LocationTypes,
UrlStateContainerPropTypes,
ReplaceStateInLocation,
UpdateUrlStateString,
} from './types';
import { ReplaceStateInLocation, UpdateUrlStateString } from './types';

export const decodeRisonUrlState = <T>(value: string | undefined): T | null => {
try {
Expand Down Expand Up @@ -113,42 +108,13 @@ export const getTitle = (
return navTabs[pageName] != null ? navTabs[pageName].name : '';
};

export const getCurrentLocation = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI -> It was not used @stephmilovic told me and when I went in this files, I had to delete it ;)

pageName: string,
detailName: string | undefined
): LocationTypes => {
if (pageName === SiemPageName.overview) {
return CONSTANTS.overviewPage;
} else if (pageName === SiemPageName.hosts) {
if (detailName != null) {
return CONSTANTS.hostsDetails;
}
return CONSTANTS.hostsPage;
} else if (pageName === SiemPageName.network) {
if (detailName != null) {
return CONSTANTS.networkDetails;
}
return CONSTANTS.networkPage;
} else if (pageName === SiemPageName.detections) {
return CONSTANTS.detectionsPage;
} else if (pageName === SiemPageName.timelines) {
return CONSTANTS.timelinePage;
} else if (pageName === SiemPageName.case) {
if (detailName != null) {
return CONSTANTS.caseDetails;
}
return CONSTANTS.casePage;
}
return CONSTANTS.unknown;
};

export const makeMapStateToProps = () => {
const getInputsSelector = inputsSelectors.inputsSelector();
const getGlobalQuerySelector = inputsSelectors.globalQuerySelector();
const getGlobalFiltersQuerySelector = inputsSelectors.globalFiltersQuerySelector();
const getGlobalSavedQuerySelector = inputsSelectors.globalSavedQuerySelector();
const getTimelines = timelineSelectors.getTimelines();
const mapStateToProps = (state: State, { pageName, detailName }: UrlStateContainerPropTypes) => {
const mapStateToProps = (state: State) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pageName and detailName are not used in the function

const inputState = getInputsSelector(state);
const { linkTo: globalLinkTo, timerange: globalTimerange } = inputState.global;
const { linkTo: timelineLinkTo, timerange: timelineTimerange } = inputState.timeline;
Expand Down
33 changes: 21 additions & 12 deletions ...y/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ import styled from 'styled-components';
import { isEmpty } from 'lodash/fp';

import { HeaderSection } from '../../../../components/header_section';
import { SignalsHistogram } from './signals_histogram';

import { Filter, esQuery, Query } from '../../../../../../../../../src/plugins/data/public';
import { RegisterQuery, SignalsHistogramOption, SignalsAggregation, SignalsTotal } from './types';
import { signalsHistogramOptions } from './config';
import { getDetectionEngineUrl } from '../../../../components/link_to';
import { DEFAULT_NUMBER_FORMAT } from '../../../../../common/constants';
import { useKibana, useUiSetting$ } from '../../../../lib/kibana';
import { InspectButtonContainer } from '../../../../components/inspect';
import { useQuerySignals } from '../../../../containers/detection_engine/signals/use_query';
import { getDetectionEngineUrl } from '../../../../components/link_to';
import { InspectButtonContainer } from '../../../../components/inspect';
import { useGetUrlSearch } from '../../../../components/navigation/use_get_url_search';
import { MatrixLoader } from '../../../../components/matrix_histogram/matrix_loader';

import { useKibana, useUiSetting$ } from '../../../../lib/kibana';
import { navTabs } from '../../../home/home_navigations';
import { signalsHistogramOptions } from './config';
import { formatSignalsData, getSignalsHistogramQuery, showInitialLoadingSpinner } from './helpers';
import { SignalsHistogram } from './signals_histogram';
import * as i18n from './translations';
import { RegisterQuery, SignalsHistogramOption, SignalsAggregation, SignalsTotal } from './types';

const DEFAULT_PANEL_HEIGHT = 300;

Expand Down Expand Up @@ -101,6 +103,7 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
signalIndexName
);
const kibana = useKibana();
const urlSearch = useGetUrlSearch(navTabs.detections);

const totalSignals = useMemo(
() =>
Expand Down Expand Up @@ -184,6 +187,16 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
);
}, [selectedStackByOption.value, from, to, query, filters]);

const linkButton = useMemo(() => {
if (showLinkToSignals) {
return (
<ViewSignalsFlexItem grow={false}>
<EuiButton href={getDetectionEngineUrl(urlSearch)}>{i18n.VIEW_SIGNALS}</EuiButton>
</ViewSignalsFlexItem>
);
}
}, [showLinkToSignals, urlSearch]);

return (
<InspectButtonContainer show={!isInitialLoading}>
<StyledEuiPanel height={panelHeight}>
Expand All @@ -210,11 +223,7 @@ export const SignalsHistogramPanel = memo<SignalsHistogramPanelProps>(
/>
)}
</EuiFlexItem>
{showLinkToSignals && (
<ViewSignalsFlexItem grow={false}>
<EuiButton href={getDetectionEngineUrl()}>{i18n.VIEW_SIGNALS}</EuiButton>
</ViewSignalsFlexItem>
)}
{linkButton}
</EuiFlexGroup>
</HeaderSection>

Expand Down
7 changes: 5 additions & 2 deletions x-pack/legacy/plugins/siem/public/pages/overview/alerts_by_category/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {
histogramConfigs,
} from '../../../components/alerts_viewer/histogram_configs';
import { MatrixHisrogramConfigs } from '../../../components/matrix_histogram/types';
import { useGetUrlSearch } from '../../../components/navigation/use_get_url_search';
import { navTabs } from '../../home/home_navigations';

const ID = 'alertsByCategoryOverview';

Expand Down Expand Up @@ -73,10 +75,11 @@ const AlertsByCategoryComponent: React.FC<Props> = ({

const kibana = useKibana();
const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);
const urlSearch = useGetUrlSearch(navTabs.detections);

const alertsCountViewAlertsButton = useMemo(
() => <EuiButton href={getDetectionEngineAlertUrl()}>{i18n.VIEW_ALERTS}</EuiButton>,
[]
() => <EuiButton href={getDetectionEngineAlertUrl(urlSearch)}>{i18n.VIEW_ALERTS}</EuiButton>,
[urlSearch]
);

const alertsByCategoryHistogramConfigs: MatrixHisrogramConfigs = useMemo(
Expand Down
Loading