-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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.
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
[ML] Migrate internal urls to non-hash paths #76735
Changes from 21 commits
5ff3100
df33079
8c3adc1
0333310
65de961
4ec96fd
49d0e7d
6827517
6f86f70
7f8decf
ce2f3ec
6fbb599
195929f
f175d27
8ffd123
a30ec90
0e9915b
5b8c82e
7742303
f56525c
25272ca
adc1047
5d0c800
c62a924
ac70ed2
ad88c4e
fb40acb
738f0b7
2d317c1
df40e5f
a10374a
4ee28a9
0e2ed56
bb8230c
7176749
f561cf4
ffa623a
9272d37
dd3517b
4a50f1f
5eda3bf
fe633ae
6461407
cf3edb7
787bade
de7c268
d0e20a4
853d5a3
662c756
b530216
a58a477
b76adb0
13abdba
dc3ba76
40f8845
733e9fa
1fa7026
68e7519
3439383
cea397e
0c29605
e224267
f85413f
1efaed2
29d8674
d3edff7
42313ae
f762277
ba56a81
0d83eac
cc8f5dd
92dad64
0a4a559
efe49ea
abe0ce6
b7f9be7
5dd7373
0a0436b
75352f9
ee507b4
a39face
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
export const ANALYSIS_CONFIG_TYPE = { | ||
OUTLIER_DETECTION: 'outlier_detection', | ||
REGRESSION: 'regression', | ||
CLASSIFICATION: 'classification', | ||
} as const; | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
*/ | ||
|
||
import { CustomHttpResponseOptions, ResponseError } from 'kibana/server'; | ||
import { ANALYSIS_CONFIG_TYPE } from '../constants/data_frame_analytics'; | ||
|
||
export interface DeleteDataFrameAnalyticsWithIndexStatus { | ||
success: boolean; | ||
|
@@ -79,3 +80,5 @@ export interface DataFrameAnalyticsConfig { | |
version: string; | ||
allow_lazy_start?: boolean; | ||
} | ||
|
||
export type DataFrameAnalyticsType = typeof ANALYSIS_CONFIG_TYPE[keyof typeof ANALYSIS_CONFIG_TYPE]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed here d3edff7 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,14 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React, { FC, useState } from 'react'; | ||
import { encode } from 'rison-node'; | ||
import React, { FC, useState, useEffect } from 'react'; | ||
|
||
import { EuiTabs, EuiTab, EuiLink } from '@elastic/eui'; | ||
import { i18n } from '@kbn/i18n'; | ||
|
||
import { useUrlState } from '../../util/url_state'; | ||
|
||
import { TabId } from './navigation_menu'; | ||
import { useMlUrlGenerator, useNavigateToPath } from '../../contexts/kibana'; | ||
import { MlUrlGeneratorState } from '../../../../common/types/ml_url_generator'; | ||
import { useUrlState } from '../../util/url_state'; | ||
|
||
export interface Tab { | ||
id: TabId; | ||
|
@@ -66,17 +65,49 @@ function getTabs(disableLinks: boolean): Tab[] { | |
} | ||
interface TabData { | ||
testSubject: string; | ||
pathId?: string; | ||
pathId?: MlUrlGeneratorState['page']; | ||
name: string; | ||
} | ||
|
||
const TAB_DATA: Record<TabId, TabData> = { | ||
overview: { testSubject: 'mlMainTab overview' }, | ||
overview: { | ||
testSubject: 'mlMainTab overview', | ||
name: i18n.translate('xpack.ml.overviewTabLabel', { | ||
defaultMessage: 'Overview', | ||
}), | ||
}, | ||
// Note that anomaly detection jobs list is mapped to ml#/jobs. | ||
anomaly_detection: { testSubject: 'mlMainTab anomalyDetection', pathId: 'jobs' }, | ||
data_frame_analytics: { testSubject: 'mlMainTab dataFrameAnalytics' }, | ||
datavisualizer: { testSubject: 'mlMainTab dataVisualizer' }, | ||
settings: { testSubject: 'mlMainTab settings' }, | ||
'access-denied': { testSubject: 'mlMainTab overview' }, | ||
anomaly_detection: { | ||
testSubject: 'mlMainTab anomalyDetection', | ||
name: i18n.translate('xpack.ml.anomalyDetectionTabLabel', { | ||
defaultMessage: 'Anomaly Detection', | ||
}), | ||
pathId: 'jobs', | ||
}, | ||
data_frame_analytics: { | ||
testSubject: 'mlMainTab dataFrameAnalytics', | ||
name: i18n.translate('xpack.ml.dataFrameAnalyticsTabLabel', { | ||
defaultMessage: 'Data Frame Analytics', | ||
}), | ||
}, | ||
datavisualizer: { | ||
testSubject: 'mlMainTab dataVisualizer', | ||
name: i18n.translate('xpack.ml.dataVisualizerTabLabel', { | ||
defaultMessage: 'Data Visualizer', | ||
}), | ||
}, | ||
settings: { | ||
testSubject: 'mlMainTab settings', | ||
name: i18n.translate('xpack.ml.settingsTabLabel', { | ||
defaultMessage: 'Settings', | ||
}), | ||
}, | ||
'access-denied': { | ||
testSubject: 'mlMainTab overview', | ||
name: i18n.translate('xpack.ml.accessDeniedTabLabel', { | ||
defaultMessage: 'Access Denied', | ||
}), | ||
}, | ||
}; | ||
|
||
export const MainTabs: FC<Props> = ({ tabId, disableLinks }) => { | ||
|
@@ -87,16 +118,32 @@ export const MainTabs: FC<Props> = ({ tabId, disableLinks }) => { | |
} | ||
|
||
const tabs = getTabs(disableLinks); | ||
const mlUrlGenerator = useMlUrlGenerator(); | ||
const navigateToPath = useNavigateToPath(); | ||
|
||
const redirectToTab = async (defaultPathId: MlUrlGeneratorState['page']) => { | ||
// TODO: fix ts | ||
// @ts-ignore | ||
qn895 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const path = await mlUrlGenerator.createUrl({ | ||
page: defaultPathId, | ||
pageState: { globalState }, | ||
}); | ||
|
||
await navigateToPath(path, false); | ||
}; | ||
|
||
useEffect(() => { | ||
document.title = `ML - ${TAB_DATA[selectedTabId].name} - Kibana`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if Kibana offers a way to do this more "officially" without directly manipulating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point! I do see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to using Kibana's |
||
}, [selectedTabId]); | ||
|
||
return ( | ||
<EuiTabs display="condensed"> | ||
{tabs.map((tab: Tab) => { | ||
const { id, disabled } = tab; | ||
const testSubject = TAB_DATA[id].testSubject; | ||
const defaultPathId = TAB_DATA[id].pathId || id; | ||
const defaultPathId = (TAB_DATA[id].pathId || id) as MlUrlGeneratorState['page']; | ||
// globalState (e.g. selected jobs and time range) should be retained when changing pages. | ||
// appState will not be considered. | ||
const fullGlobalStateString = globalState !== undefined ? `?_g=${encode(globalState)}` : ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the global state no longer necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still retaining the globalState here, just within |
||
|
||
return disabled ? ( | ||
<EuiTab key={`${id}-key`} className={'mlNavigationMenu__mainTab'} disabled={true}> | ||
|
@@ -106,7 +153,7 @@ export const MainTabs: FC<Props> = ({ tabId, disableLinks }) => { | |
<div className="euiTab" key={`div-${id}-key`}> | ||
<EuiLink | ||
data-test-subj={testSubject + (id === selectedTabId ? ' selected' : '')} | ||
href={`#/${defaultPathId}${fullGlobalStateString}`} | ||
onClick={() => redirectToTab(defaultPathId)} | ||
key={`${id}-key`} | ||
color="text" | ||
> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the same as an
enum
already inplugins/ml/public/application/data_frame_analytics/common/analytics.ts
. This one using theconst
approach as well as the location here LGTM so suggest to try to get rid of the on mentioned in this comment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went through and updated the usage of
enum ANALYSIS_CONFIG_TYPE
toconst ANALYSIS_CONFIG_TYPE
here dd3517b