diff --git a/x-pack/plugins/ml/public/application/jobs/new_job/utils/new_job_utils.ts b/x-pack/plugins/ml/public/application/jobs/new_job/utils/new_job_utils.ts index b7b6aa15ffe44..543e6898193a4 100644 --- a/x-pack/plugins/ml/public/application/jobs/new_job/utils/new_job_utils.ts +++ b/x-pack/plugins/ml/public/application/jobs/new_job/utils/new_job_utils.ts @@ -30,7 +30,7 @@ export function getDefaultDatafeedQuery() { export function createSearchItems( kibanaConfig: IUiSettingsClient, - indexPattern: IIndexPattern, + indexPattern: IIndexPattern | undefined, savedSearch: SavedSearchSavedObject | null ) { // query is only used by the data visualizer as it needs diff --git a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_job_exploration.tsx b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_job_exploration.tsx index e436ff468ccf0..771123532dcbf 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_job_exploration.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_job_exploration.tsx @@ -38,7 +38,7 @@ export const analyticsJobExplorationRouteFactory = ( }); const PageWrapper: FC = ({ location, deps }) => { - const { context } = useResolver('', undefined, deps.config, basicResolvers(deps)); + const { context } = useResolver(undefined, undefined, deps.config, basicResolvers(deps)); const [globalState] = useUrlState('_g'); diff --git a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_jobs_list.tsx b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_jobs_list.tsx index 80706a82121d5..3b68c5078e99e 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_jobs_list.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_jobs_list.tsx @@ -34,7 +34,7 @@ export const analyticsJobsListRouteFactory = ( }); const PageWrapper: FC = ({ location, deps }) => { - const { context } = useResolver('', undefined, deps.config, basicResolvers(deps)); + const { context } = useResolver(undefined, undefined, deps.config, basicResolvers(deps)); return ( diff --git a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_map.tsx b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_map.tsx index 18002648cfaa6..3acd12402932f 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_map.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/analytics_map.tsx @@ -34,7 +34,7 @@ export const analyticsMapRouteFactory = ( }); const PageWrapper: FC = ({ deps }) => { - const { context } = useResolver('', undefined, deps.config, basicResolvers(deps)); + const { context } = useResolver(undefined, undefined, deps.config, basicResolvers(deps)); return ( diff --git a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/models_list.tsx b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/models_list.tsx index b1fd6e93a744c..2f58ef756e51c 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/models_list.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/data_frame_analytics/models_list.tsx @@ -34,7 +34,7 @@ export const modelsListRouteFactory = ( }); const PageWrapper: FC = ({ location, deps }) => { - const { context } = useResolver('', undefined, deps.config, basicResolvers(deps)); + const { context } = useResolver(undefined, undefined, deps.config, basicResolvers(deps)); return ( diff --git a/x-pack/plugins/ml/public/application/routing/routes/datavisualizer/file_based.tsx b/x-pack/plugins/ml/public/application/routing/routes/datavisualizer/file_based.tsx index 837616a8a76d2..f651d17e02de4 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/datavisualizer/file_based.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/datavisualizer/file_based.tsx @@ -45,7 +45,7 @@ export const fileBasedRouteFactory = ( const PageWrapper: FC = ({ location, deps }) => { const { redirectToMlAccessDeniedPage } = deps; - const { context } = useResolver('', undefined, deps.config, { + const { context } = useResolver(undefined, undefined, deps.config, { checkBasicLicense, loadIndexPatterns: () => loadIndexPatterns(deps.indexPatterns), checkFindFileStructurePrivilege: () => diff --git a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx index df92c77252565..7de59cba495af 100644 --- a/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx +++ b/x-pack/plugins/ml/public/application/routing/routes/timeseriesexplorer.tsx @@ -63,7 +63,7 @@ export const timeSeriesExplorerRouteFactory = ( }); const PageWrapper: FC = ({ deps }) => { - const { context, results } = useResolver('', undefined, deps.config, { + const { context, results } = useResolver(undefined, undefined, deps.config, { ...basicResolvers(deps), jobs: mlJobService.loadJobsWrapper, jobsWithTimeRange: () => ml.jobs.jobsWithTimerange(getDateFormatTz()), diff --git a/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts b/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts new file mode 100644 index 0000000000000..07cc038538745 --- /dev/null +++ b/x-pack/plugins/ml/public/application/routing/use_resolver.test.ts @@ -0,0 +1,89 @@ +/* + * 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 { renderHook, act } from '@testing-library/react-hooks'; + +import { IUiSettingsClient } from 'kibana/public'; + +import { useCreateAndNavigateToMlLink } from '../contexts/kibana/use_create_url'; +import { useNotifications } from '../contexts/kibana'; + +import { useResolver } from './use_resolver'; + +jest.mock('../contexts/kibana/use_create_url', () => { + return { + useCreateAndNavigateToMlLink: jest.fn(), + }; +}); + +jest.mock('../contexts/kibana', () => { + return { + useMlUrlGenerator: () => ({ + createUrl: jest.fn(), + }), + useNavigateToPath: () => jest.fn(), + useNotifications: jest.fn(), + }; +}); + +const addError = jest.fn(); +(useNotifications as jest.Mock).mockImplementation(() => ({ + toasts: { addSuccess: jest.fn(), addDanger: jest.fn(), addError }, +})); + +const redirectToJobsManagementPage = jest.fn(() => Promise.resolve()); +(useCreateAndNavigateToMlLink as jest.Mock).mockImplementation(() => redirectToJobsManagementPage); + +describe('useResolver', () => { + afterEach(() => { + jest.useFakeTimers(); + }); + afterEach(() => { + jest.advanceTimersByTime(0); + jest.useRealTimers(); + }); + + it('should accept undefined as indexPatternId and savedSearchId.', async () => { + const { result, waitForNextUpdate } = renderHook(() => + useResolver(undefined, undefined, {} as IUiSettingsClient, {}) + ); + + await act(async () => { + await waitForNextUpdate(); + }); + + expect(result.current).toStrictEqual({ + context: { + combinedQuery: { + bool: { + must: [ + { + match_all: {}, + }, + ], + }, + }, + currentIndexPattern: null, + currentSavedSearch: null, + indexPatterns: null, + kibanaConfig: {}, + }, + results: {}, + }); + expect(addError).toHaveBeenCalledTimes(0); + expect(redirectToJobsManagementPage).toHaveBeenCalledTimes(0); + }); + + it('should add an error toast and redirect if indexPatternId is an empty string.', async () => { + const { result } = renderHook(() => useResolver('', undefined, {} as IUiSettingsClient, {})); + + await act(async () => {}); + + expect(result.current).toStrictEqual({ context: null, results: {} }); + expect(addError).toHaveBeenCalledTimes(1); + expect(redirectToJobsManagementPage).toHaveBeenCalledTimes(1); + }); +}); diff --git a/x-pack/plugins/ml/public/application/routing/use_resolver.ts b/x-pack/plugins/ml/public/application/routing/use_resolver.ts index e4cd90145bee4..3ce23f8b8a19c 100644 --- a/x-pack/plugins/ml/public/application/routing/use_resolver.ts +++ b/x-pack/plugins/ml/public/application/routing/use_resolver.ts @@ -11,6 +11,7 @@ import { getIndexPatternById, getIndexPatternsContract, getIndexPatternAndSavedSearch, + IndexPatternAndSavedSearch, } from '../util/index_utils'; import { createSearchItems } from '../jobs/new_job/utils/new_job_utils'; import { ResolverResults, Resolvers } from './resolvers'; @@ -19,6 +20,14 @@ import { useNotifications } from '../contexts/kibana'; import { useCreateAndNavigateToMlLink } from '../contexts/kibana/use_create_url'; import { ML_PAGES } from '../../../common/constants/ml_url_generator'; +/** + * Hook to resolve route specific requirements + * @param indexPatternId optional Kibana index pattern id, used for wizards + * @param savedSearchId optional Kibana saved search id, used for wizards + * @param config Kibana UI Settings + * @param resolvers an array of resolvers to be executed for the route + * @return { context, results } returns the ML context and resolver results + */ export const useResolver = ( indexPatternId: string | undefined, savedSearchId: string | undefined, @@ -52,36 +61,49 @@ export const useResolver = ( return; } - if (indexPatternId !== undefined || savedSearchId !== undefined) { - try { - // note, currently we're using our own kibana context that requires a current index pattern to be set - // this means, if the page uses this context, useResolver must be passed a string for the index pattern id - // and loadIndexPatterns must be part of the resolvers. - const { indexPattern, savedSearch } = - savedSearchId !== undefined - ? await getIndexPatternAndSavedSearch(savedSearchId) - : { savedSearch: null, indexPattern: await getIndexPatternById(indexPatternId!) }; + try { + if (indexPatternId === '') { + throw new Error( + i18n.translate('xpack.ml.useResolver.errorIndexPatternIdEmptyString', { + defaultMessage: 'indexPatternId must not be empty string.', + }) + ); + } - const { combinedQuery } = createSearchItems(config, indexPattern!, savedSearch); + let indexPatternAndSavedSearch: IndexPatternAndSavedSearch = { + savedSearch: null, + indexPattern: null, + }; - setContext({ - combinedQuery, - currentIndexPattern: indexPattern, - currentSavedSearch: savedSearch, - indexPatterns: getIndexPatternsContract()!, - kibanaConfig: config, - }); - } catch (error) { - // an unexpected error has occurred. This could be caused by an incorrect index pattern or saved search ID - notifications.toasts.addError(new Error(error), { - title: i18n.translate('xpack.ml.useResolver.errorTitle', { - defaultMessage: 'An error has occurred', - }), - }); - await redirectToJobsManagementPage(); + if (savedSearchId !== undefined) { + indexPatternAndSavedSearch = await getIndexPatternAndSavedSearch(savedSearchId); + } else if (indexPatternId !== undefined) { + indexPatternAndSavedSearch.indexPattern = await getIndexPatternById(indexPatternId); } - } else { - setContext({}); + + const { savedSearch, indexPattern } = indexPatternAndSavedSearch; + + const { combinedQuery } = createSearchItems( + config, + indexPattern !== null ? indexPattern : undefined, + savedSearch + ); + + setContext({ + combinedQuery, + currentIndexPattern: indexPattern, + currentSavedSearch: savedSearch, + indexPatterns: getIndexPatternsContract(), + kibanaConfig: config, + }); + } catch (error) { + // an unexpected error has occurred. This could be caused by an incorrect index pattern or saved search ID + notifications.toasts.addError(new Error(error), { + title: i18n.translate('xpack.ml.useResolver.errorTitle', { + defaultMessage: 'An error has occurred', + }), + }); + await redirectToJobsManagementPage(); } })(); }, []); diff --git a/x-pack/plugins/ml/public/application/util/index_utils.ts b/x-pack/plugins/ml/public/application/util/index_utils.ts index 42be3dd8252f9..de08553af9906 100644 --- a/x-pack/plugins/ml/public/application/util/index_utils.ts +++ b/x-pack/plugins/ml/public/application/util/index_utils.ts @@ -73,9 +73,12 @@ export function getIndexPatternIdFromName(name: string) { } return null; } - +export interface IndexPatternAndSavedSearch { + savedSearch: SavedSearchSavedObject | null; + indexPattern: IIndexPattern | null; +} export async function getIndexPatternAndSavedSearch(savedSearchId: string) { - const resp: { savedSearch: SavedSearchSavedObject | null; indexPattern: IIndexPattern | null } = { + const resp: IndexPatternAndSavedSearch = { savedSearch: null, indexPattern: null, };