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.

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] Use context and custom hooks to manage legacy dependencies. #42244

Merged
merged 17 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ import { uiModules } from 'ui/modules';
const module = uiModules.get('apps/ml');

import 'ui/directives/kbn_href';
import chrome from 'ui/chrome';
import { timefilter } from 'ui/timefilter';
import { timeHistory } from 'ui/timefilter/time_history';

import { NavigationMenuContext } from '../../util/context_utils';

import { NavigationMenu } from './navigation_menu';

Expand All @@ -25,12 +20,7 @@ module.directive('mlNavMenu', function () {
restrict: 'E',
transclude: true,
link: function (scope, element, attrs) {
ReactDOM.render(
<NavigationMenuContext.Provider value={{ chrome, timefilter, timeHistory }}>
<NavigationMenu tabId={attrs.name} />
</NavigationMenuContext.Provider>,
element[0]
);
ReactDOM.render(<NavigationMenu tabId={attrs.name} />, element[0]);

element.on('$destroy', () => {
ReactDOM.unmountComponentAtNode(element[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { FC, Fragment, useContext, useState, useEffect } from 'react';
import React, { FC, Fragment, useState, useEffect } from 'react';
import { EuiSuperDatePicker } from '@elastic/eui';
import { TimeHistory, TimeRange } from 'ui/timefilter/time_history';

import { mlTimefilterRefresh$ } from '../../../services/timefilter_refresh_service';
import { NavigationMenuContext } from '../../../util/context_utils';
import { useUiContext } from '../../../contexts/ui/use_ui_context';

interface Duration {
start: string;
Expand All @@ -28,9 +28,8 @@ function getRecentlyUsedRangesFactory(timeHistory: TimeHistory) {
}

export const TopNav: FC = () => {
const navigationMenuContext = useContext(NavigationMenuContext);
const timefilter = navigationMenuContext.timefilter;
const getRecentlyUsedRanges = getRecentlyUsedRangesFactory(navigationMenuContext.timeHistory);
const { chrome, timefilter, timeHistory } = useUiContext();
const getRecentlyUsedRanges = getRecentlyUsedRangesFactory(timeHistory);

const [refreshInterval, setRefreshInterval] = useState(timefilter.getRefreshInterval());
const [time, setTime] = useState(timefilter.getTime());
Expand All @@ -42,7 +41,7 @@ export const TopNav: FC = () => {
timefilter.isTimeRangeSelectorEnabled
);

const dateFormat = navigationMenuContext.chrome.getUiSettingsClient().get('dateFormat');
const dateFormat = chrome.getUiSettingsClient().get('dateFormat');

useEffect(() => {
timefilter.on('refreshIntervalUpdate', timefilterUpdateListener);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 { IndexPattern } from 'ui/index_patterns';

export const indexPatternMock = ({
id: 'the-index-pattern-id',
title: 'the-index-pattern-title',
fields: [],
} as unknown) as IndexPattern;
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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 { IndexPatterns } from 'ui/index_patterns';

export const indexPatternsMock = (new (class {
fieldFormats = [];
config = {};
savedObjectsClient = {};
refreshSavedObjectsCache = {};
clearCache = jest.fn();
get = jest.fn();
getDefault = jest.fn();
getFields = jest.fn();
getIds = jest.fn();
getTitles = jest.fn();
make = jest.fn();
})() as unknown) as IndexPatterns;
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 kibanaConfigMock = {
get: <T>(key: string): T => ({} as T),
has: (key: string) => false,
set: (key: string, value: any) => {},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* 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 { indexPatternMock } from './index_pattern';
import { indexPatternsMock } from './index_patterns';
import { kibanaConfigMock } from './kibana_config';
import { savedSearchMock } from './saved_search';

export const kibanaContextValueMock = {
combinedQuery: {
query: 'the-query-string',
language: 'the-query-language',
},
currentIndexPattern: indexPatternMock,
currentSavedSearch: savedSearchMock,
indexPatterns: indexPatternsMock,
kbnBaseUrl: 'url',
kibanaConfig: kibanaConfigMock,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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 savedSearchMock = {
id: 'the-saved-search-id',
title: 'the-saved-search-title',
searchSource: {},
columns: [],
sort: [],
destroy: () => {},
};
10 changes: 10 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/kibana/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* 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 { KibanaContext, KibanaContextValue, SavedSearchQuery } from './kibana_context';
export { useKibanaContext } from './use_kibana_context';
export { useCurrentIndexPattern } from './use_current_index_pattern';
export { useCurrentSavedSearch } from './use_current_saved_search';
45 changes: 45 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/kibana/kibana_context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* 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 from 'react';

import { KibanaConfig } from 'src/legacy/server/kbn_server';
import { SavedSearch } from 'src/legacy/core_plugins/kibana/public/discover/types';

import { IndexPattern, IndexPatterns } from 'ui/index_patterns';

// set() method is missing in original d.ts
export interface KibanaConfigTypeFix extends KibanaConfig {
set(key: string, value: any): void;
}

export interface KibanaContextValue {
combinedQuery: any;
currentIndexPattern: IndexPattern;
currentSavedSearch: SavedSearch;
indexPatterns: IndexPatterns;
kbnBaseUrl: string;
kibanaConfig: KibanaConfigTypeFix;
}

export type SavedSearchQuery = object;

// This context provides dependencies which can be injected
// via angularjs only (like services, currentIndexPattern etc.).
// Because we cannot just import these dependencies, the default value
// for the context is just {} and of type `Partial<KibanaContextValue>`
// for the angularjs based dependencies. Therefore, the
// actual dependencies are set like we did previously with KibanaContext
// in the wrapping angularjs directive. In the custom hook we check if
// the dependencies are present with error reporting if they weren't
// added properly. That's why in tests, these custom hooks must not
// be mocked, instead <UiChrome.Provider value="mocked-value">` needs
// to be used. This guarantees that we have both properly set up
// TypeScript support and runtime checks for these dependencies.
// Multiple custom hooks can be created to access subsets of
// the overall context value if necessary too,
// see useCurrentIndexPattern() for example.
export const KibanaContext = React.createContext<Partial<KibanaContextValue>>({});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 { useContext } from 'react';

import { KibanaContext } from './kibana_context';

export const useCurrentIndexPattern = () => {
const context = useContext(KibanaContext);

if (context.currentIndexPattern === undefined) {
throw new Error('currentIndexPattern is undefined');
}

return context.currentIndexPattern;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* 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 { useContext } from 'react';

import { KibanaContext } from './kibana_context';

export const useCurrentSavedSearch = () => {
const context = useContext(KibanaContext);

if (context.currentSavedSearch === undefined) {
throw new Error('currentSavedSearch is undefined');
}

return context.currentSavedSearch;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { useContext } from 'react';

import { KibanaContext, KibanaContextValue } from './kibana_context';

export const useKibanaContext = () => {
const context = useContext(KibanaContext);

if (
context.combinedQuery === undefined ||
context.currentIndexPattern === undefined ||
context.currentSavedSearch === undefined ||
context.indexPatterns === undefined ||
context.kbnBaseUrl === undefined ||
context.kibanaConfig === undefined
) {
throw new Error('required attribute is undefined');
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be useful here to loop through the required attributes to check for undefined and then throw the error with the specific missing dependency. Just for future debugging purposes.

Copy link
Member

Choose a reason for hiding this comment

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

this would potentially throw multiple times if more than one property is missing unless the loop builds up an array of missing properties and joins them for the error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the second thing you said. Should've been more clear. Not a must-have but would be nice to have.

}

return context as KibanaContextValue;
};
34 changes: 34 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/ui/__mocks__/mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* 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 uiChromeMock = {
getBasePath: () => 'basePath',
getUiSettingsClient: () => {
return {
get: (key: string) => {
switch (key) {
case 'dateFormat':
case 'timepicker:timeDefaults':
return {};
case 'timepicker:refreshIntervalDefaults':
return { pause: false, value: 0 };
default:
throw new Error(`Unexpected config key: ${key}`);
}
},
};
},
};

export const uiTimefilterMock = {
getRefreshInterval: () => '30s',
getTime: () => ({ from: 0, to: 0 }),
on: (event: string, reload: () => void) => {},
};

export const uiTimeHistoryMock = {
get: () => [{ from: 0, to: 0 }],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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 { uiChromeMock } from './mocks';

export const useUiChromeContext = () => uiChromeMock;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 { uiChromeMock, uiTimefilterMock, uiTimeHistoryMock } from './mocks';

export const useUiContext = () => ({
chrome: uiChromeMock,
timefilter: uiTimefilterMock,
timeHistory: uiTimeHistoryMock,
});
9 changes: 9 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/ui/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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.
*/

// We only export UiContext but not any custom hooks, because if we'd import them
// from here, mocking the hook from jest tests won't work as expected.
export { UiContext } from './ui_context';
27 changes: 27 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/ui/ui_context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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 from 'react';

import chrome from 'ui/chrome';
import { timefilter } from 'ui/timefilter';
import { timeHistory } from 'ui/timefilter/time_history';

// This provides ui/* based imports via React Context.
// Because these dependencies can use regular imports,
// they are just passed on as the default value
// of the Context which means it's not necessary
// to add <UiContext.Provider value="..." />... to the
// wrapping angular directive, reducing a lot of boilerplate.
// The custom hooks like useUiContext() need to be mocked in
// tests because we rely on the properly set up default value.
// Different custom hooks can be created to access parts only
// from the full context value, see useUiChromeContext() as an example.
export const UiContext = React.createContext({
chrome,
timefilter,
timeHistory,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 { useContext } from 'react';

import { UiContext } from './ui_context';

export const useUiChromeContext = () => {
return useContext(UiContext).chrome;
};
13 changes: 13 additions & 0 deletions x-pack/legacy/plugins/ml/public/contexts/ui/use_ui_context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 { useContext } from 'react';

import { UiContext } from './ui_context';

export const useUiContext = () => {
return useContext(UiContext);
};
Loading