Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { formatHumanReadableDateTimeSeconds } from '../../../../common/util/date
import { getIndexPatternIdFromName } from '../../util/index_utils';
import { replaceStringTokens } from '../../util/string_utils';
import { ML_APP_URL_GENERATOR, ML_PAGES } from '../../../../common/constants/ml_url_generator';
import { PLUGIN_ID } from '../../../../common/constants/app';
/*
* Component for rendering the links menu inside a cell in the anomalies table.
*/
Expand Down Expand Up @@ -147,8 +146,6 @@ class LinksMenuUI extends Component {
viewSeries = async () => {
const {
services: {
application: { navigateToApp },

share: {
urlGenerators: { getUrlGenerator },
},
Expand Down Expand Up @@ -185,7 +182,7 @@ class LinksMenuUI extends Component {
}

const singleMetricViewerLink = await mlUrlGenerator.createUrl({
excludeBasePath: true,
excludeBasePath: false,
page: ML_PAGES.SINGLE_METRIC_VIEWER,
pageState: {
jobIds: [record.job_id],
Expand All @@ -211,9 +208,7 @@ class LinksMenuUI extends Component {
},
},
});
await navigateToApp(PLUGIN_ID, {
path: singleMetricViewerLink,
});
window.open(singleMetricViewerLink, '_blank');
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use MlHref instead of window.open?

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll keep this one here for now for 7.10.0.

};

viewExamples = () => {
Expand Down
61 changes: 61 additions & 0 deletions x-pack/plugins/ml/public/application/components/links/ml_href.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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 React, { ReactChild } from 'react';
import { useMlKibana, useNavigateToPath } from '../../contexts/kibana';

function isModifiedEvent(event: React.MouseEvent<HTMLAnchorElement, MouseEvent>) {
return !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey);
}

export const MlHref = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

This component deserves a brief description

href,
target,
onClick,
children,
...rest
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if ...rest should be here because you already destructured all the props.

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 ...rest is here if the user wants to pass on extra props for the tag like 'className' or style.

}: {
href: string;
target?: string;
onClick?: Function;
children?: ReactChild;
}) => {
const {
services: {
http: { basePath },
},
} = useMlKibana();

const navigateToPath = useNavigateToPath();
return (
<a
target={target}
href={basePath.prepend(`/app/ml/${href}`)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this component should also support URLs besides the ML plugin.

onClick={(event) => {
try {
if (onClick) onClick(event);
} catch (ex) {
event.preventDefault();
throw ex;
}

if (
!event.defaultPrevented && // onClick prevented default
event.button === 0 && // ignore everything but left clicks
(!target || target === '_self') && // let browser handle "target=_blank" etc.
!isModifiedEvent(event) // ignore clicks with modifier keys
) {
event.preventDefault();
navigateToPath(href);
return false;
}
return true;
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I think we should call event. preventDefault only once at the beginning of the function with isModifiedEvent check and target="_blank" maybe. But I believe we should not use target=_blank inside of Kibana. There is only one exception - external URLs, hence you can omit the target check in this component because it's not applicable.
  2. There is no need to return true or false. I recall it from old times when preventDefault wasn't a standard.
  3. The onClick callback that comes with props can cause potential issues, i.e. update state on an unmounted component. Making it Promise based should help.

{...rest}
>
{children}
</a>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, useState, useEffect, useCallback } from 'react';
import React, { FC, useState, useEffect } from 'react';
import moment from 'moment';
import { FormattedMessage } from '@kbn/i18n/react';
import { EuiFlexGroup, EuiFlexItem, EuiCard, EuiIcon } from '@elastic/eui';
import { ml } from '../../../../services/ml_api_service';
import { isFullLicense } from '../../../../license';
import { checkPermission } from '../../../../capabilities/check_capabilities';
import { mlNodesAvailable } from '../../../../ml_nodes_check/check_ml_nodes';
import { useMlKibana, useMlUrlGenerator, useNavigateToPath } from '../../../../contexts/kibana';
import { useMlKibana, useMlUrlGenerator } from '../../../../contexts/kibana';
import { ML_PAGES } from '../../../../../../common/constants/ml_url_generator';
import { MlCommonGlobalState } from '../../../../../../common/types/ml_url_generator';
import {
Expand Down Expand Up @@ -45,12 +45,16 @@ export const ResultsLinks: FC<Props> = ({
const [globalState, setGlobalState] = useState<MlCommonGlobalState | undefined>();

const [discoverLink, setDiscoverLink] = useState('');
const [indexManagementLink, setIndexManagementLink] = useState('');
const [indexPatternManagementLink, setIndexPatternManagementLink] = useState('');
const [dataVisualizerLink, setDataVisualizerLink] = useState('');
const [createJobsSelectTypePage, setCreateJobsSelectTypePage] = useState('');

const mlUrlGenerator = useMlUrlGenerator();
const navigateToPath = useNavigateToPath();

const {
services: {
application: { navigateToApp, navigateToUrl },
application: { getUrlForApp },
share: {
urlGenerators: { getUrlGenerator },
},
Expand Down Expand Up @@ -84,35 +88,52 @@ export const ResultsLinks: FC<Props> = ({
setDiscoverLink(discoverUrl);
}
};

const getDataVisualizerLink = async (): Promise<void> => {
const _dataVisualizerLink = await mlUrlGenerator.createUrl({
page: ML_PAGES.DATA_VISUALIZER_INDEX_VIEWER,
pageState: {
index: indexPatternId,
globalState,
},
});
if (!unmounted) {
setDataVisualizerLink(_dataVisualizerLink);
}
};
const getADCreateJobsSelectTypePage = async (): Promise<void> => {
const _createJobsSelectTypePage = await mlUrlGenerator.createUrl({
page: ML_PAGES.ANOMALY_DETECTION_CREATE_JOB_SELECT_TYPE,
pageState: {
index: indexPatternId,
globalState,
},
});
if (!unmounted) {
setCreateJobsSelectTypePage(_createJobsSelectTypePage);
}
};

getDiscoverUrl();
getDataVisualizerLink();
getADCreateJobsSelectTypePage();

if (!unmounted) {
setIndexManagementLink(
getUrlForApp('management', { path: '/data/index_management/indices' })
);
setIndexPatternManagementLink(
getUrlForApp('management', {
path: `/kibana/indexPatterns${createIndexPattern ? `/patterns/${indexPatternId}` : ''}`,
})
);
}

return () => {
unmounted = true;
};
}, [indexPatternId, getUrlGenerator]);

const openInDataVisualizer = useCallback(async () => {
const path = await mlUrlGenerator.createUrl({
page: ML_PAGES.DATA_VISUALIZER_INDEX_VIEWER,
pageState: {
index: indexPatternId,
globalState,
},
});
await navigateToPath(path);
}, [indexPatternId, globalState]);

const redirectToADCreateJobsSelectTypePage = useCallback(async () => {
const path = await mlUrlGenerator.createUrl({
page: ML_PAGES.ANOMALY_DETECTION_CREATE_JOB_SELECT_TYPE,
pageState: {
index: indexPatternId,
globalState,
},
});
await navigateToPath(path);
}, [indexPatternId, globalState]);

useEffect(() => {
setShowCreateJobLink(checkPermission('canCreateJob') && mlNodesAvailable());
updateTimeValues();
Expand Down Expand Up @@ -148,23 +169,6 @@ export const ResultsLinks: FC<Props> = ({
}
}

function openInDiscover(e: React.MouseEvent<HTMLButtonElement>) {
e.preventDefault();
navigateToUrl(discoverLink);
}

function openIndexManagement(e: React.MouseEvent<HTMLButtonElement>) {
e.preventDefault();
navigateToApp('management', { path: '/data/index_management/indices' });
}

function openIndexPatternManagement(e: React.MouseEvent<HTMLButtonElement>) {
e.preventDefault();
navigateToApp('management', {
path: `/kibana/indexPatterns${createIndexPattern ? `/patterns/${indexPatternId}` : ''}`,
});
}

return (
<EuiFlexGroup gutterSize="l">
{createIndexPattern && discoverLink && (
Copy link
Contributor

Choose a reason for hiding this comment

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

do you need to add a check for all the other links as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 0064254

Expand All @@ -178,7 +182,7 @@ export const ResultsLinks: FC<Props> = ({
/>
}
description=""
onClick={openInDiscover}
href={discoverLink}
/>
</EuiFlexItem>
)}
Expand All @@ -197,7 +201,7 @@ export const ResultsLinks: FC<Props> = ({
/>
}
description=""
onClick={redirectToADCreateJobsSelectTypePage}
href={createJobsSelectTypePage}
/>
</EuiFlexItem>
)}
Expand All @@ -213,7 +217,7 @@ export const ResultsLinks: FC<Props> = ({
/>
}
description=""
onClick={openInDataVisualizer}
href={dataVisualizerLink}
/>
</EuiFlexItem>
)}
Expand All @@ -228,7 +232,7 @@ export const ResultsLinks: FC<Props> = ({
/>
}
description=""
onClick={openIndexManagement}
href={indexManagementLink}
/>
</EuiFlexItem>

Expand All @@ -242,7 +246,7 @@ export const ResultsLinks: FC<Props> = ({
/>
}
description=""
onClick={openIndexPatternManagement}
href={indexPatternManagementLink}
/>
</EuiFlexItem>
<EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { MlTooltipComponent } from '../../components/chart_tooltip';
import { withKibana } from '../../../../../../../src/plugins/kibana_react/public';
import { ML_APP_URL_GENERATOR } from '../../../../common/constants/ml_url_generator';
import { PLUGIN_ID } from '../../../../common/constants/app';
import { addItemToRecentlyAccessed } from '../../util/recently_accessed';

const textTooManyBuckets = i18n.translate('xpack.ml.explorer.charts.tooManyBucketsDescription', {
Expand All @@ -55,21 +54,12 @@ function getChartId(series) {
}

// Wrapper for a single explorer chart
function ExplorerChartContainer({
series,
severity,
tooManyBuckets,
wrapLabel,
navigateToApp,
mlUrlGenerator,
}) {
function ExplorerChartContainer({ series, severity, tooManyBuckets, wrapLabel, mlUrlGenerator }) {
const redirectToSingleMetricViewer = useCallback(async () => {
const singleMetricViewerLink = await getExploreSeriesLink(mlUrlGenerator, series);
addItemToRecentlyAccessed('timeseriesexplorer', series.jobId, singleMetricViewerLink);

await navigateToApp(PLUGIN_ID, {
path: singleMetricViewerLink,
});
window.open(singleMetricViewerLink, '_blank');
}, [mlUrlGenerator]);

const { detectorLabel, entityFields } = series;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React from 'react';
import { detectorToString } from '../../../../util/string_utils';
import { formatValues, filterObjects } from './format_values';
import { i18n } from '@kbn/i18n';
import { Link } from 'react-router-dom';
import { MlHref } from '../../../../components/links/ml_href';

export function extractJobDetails(job) {
if (Object.keys(job).length === 0) {
Expand Down Expand Up @@ -61,7 +61,7 @@ export function extractJobDetails(job) {
if (job.calendars) {
calendars.items = job.calendars.map((c) => [
'',
<Link to={`/settings/calendars_list/edit_calendar/${c}?_g=()`}>{c}</Link>,
<MlHref href={`settings/calendars_list/edit_calendar/${c}?_g=()`}>{c}</MlHref>,
]);
// remove the calendars list from the general section
// so not to show it twice.
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/ml/public/application/util/chart_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export async function getExploreSeriesLink(mlUrlGenerator, series) {
},
},
},
excludeBasePath: true,
excludeBasePath: false,
});
return url;
}
Expand Down