Skip to content

Commit

Permalink
[ML] Transforms: Replace KqlFilterBar with QueryStringInput. (#59723)
Browse files Browse the repository at this point in the history
- Replaces the custom KqlFilterBar with Kibana's QueryStringInput. This means the wizard now supports both lucene and kuery input.
- Using this component we no longer need to do cross-imports from the ML plugin. The use of setDependencyCache is no longer necessary.
- Replaces the custom AppDependencies provider code with Kibana's KibanaContextProvider.
  • Loading branch information
walterra committed Mar 12, 2020
1 parent 8b174fe commit 8ec101f
Show file tree
Hide file tree
Showing 22 changed files with 212 additions and 252 deletions.
4 changes: 0 additions & 4 deletions x-pack/plugins/transform/public/__mocks__/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

jest.mock('ui/new_platform');

export const expandLiteralStrings = jest.fn();
export const XJsonMode = jest.fn();
export const setDependencyCache = jest.fn();
export const useRequest = jest.fn(() => ({
isLoading: false,
error: null,
data: undefined,
}));
export { mlInMemoryTableBasicFactory } from '../../../../legacy/plugins/ml/public/application/components/ml_in_memory_table';
export const SORT_DIRECTION = { ASC: 'asc' };
export const KqlFilterBar = jest.fn(() => null);
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,31 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { coreMock } from '../../../../../src/core/public/mocks';
import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks';

import { getAppProviders, AppDependencies } from './app_dependencies';
import { coreMock } from '../../../../../../src/core/public/mocks';
import { dataPluginMock } from '../../../../../../src/plugins/data/public/mocks';
import { Storage } from '../../../../../../src/plugins/kibana_utils/public';

const coreSetup = coreMock.createSetup();
const coreStart = coreMock.createStart();
const dataStart = dataPluginMock.createStartContract();

const appDependencies: AppDependencies = {
const appDependencies = {
chrome: coreStart.chrome,
data: dataStart,
docLinks: coreStart.docLinks,
i18n: coreStart.i18n,
notifications: coreStart.notifications,
notifications: coreSetup.notifications,
uiSettings: coreStart.uiSettings,
savedObjects: coreStart.savedObjects,
storage: ({ get: jest.fn() } as unknown) as Storage,
overlays: coreStart.overlays,
http: coreSetup.http,
};

export const Providers = getAppProviders(appDependencies);
export const useAppDependencies = () => {
return appDependencies;
};

export const useToastNotifications = () => {
return coreSetup.notifications;
};
19 changes: 13 additions & 6 deletions x-pack/plugins/transform/public/app/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import { HashRouter, Redirect, Route, Switch } from 'react-router-dom';

import { FormattedMessage } from '@kbn/i18n/react';

import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public';

import { API_BASE_PATH } from '../../common/constants';

import { SectionError } from './components';
import { CLIENT_BASE_PATH, SECTION_SLUG } from './constants';
import { getAppProviders } from './app_dependencies';
import { AuthorizationContext } from './lib/authorization';
import { AuthorizationContext, AuthorizationProvider } from './lib/authorization';
import { AppDependencies } from './app_dependencies';

import { CloneTransformSection } from './sections/clone_transform';
Expand Down Expand Up @@ -61,12 +64,16 @@ export const App: FC = () => {
};

export const renderApp = (element: HTMLElement, appDependencies: AppDependencies) => {
const Providers = getAppProviders(appDependencies);
const I18nContext = appDependencies.i18n.Context;

render(
<Providers>
<App />
</Providers>,
<KibanaContextProvider services={appDependencies}>
<AuthorizationProvider privilegesEndpoint={`${API_BASE_PATH}privileges`}>
<I18nContext>
<App />
</I18nContext>
</AuthorizationProvider>
</KibanaContextProvider>,
element
);

Expand Down
54 changes: 5 additions & 49 deletions x-pack/plugins/transform/public/app/app_dependencies.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,54 +4,27 @@
* you may not use this file except in compliance with the Elastic License.
*/

import React, { createContext, useContext, ReactNode } from 'react';
import { HashRouter } from 'react-router-dom';

import { CoreSetup, CoreStart } from 'src/core/public';
import { DataPublicPluginStart } from 'src/plugins/data/public';

import { API_BASE_PATH } from '../../common/constants';

import { setDependencyCache } from '../shared_imports';

import { AuthorizationProvider } from './lib/authorization';
import { useKibana } from '../../../../../src/plugins/kibana_react/public';
import { Storage } from '../../../../../src/plugins/kibana_utils/public';

export interface AppDependencies {
chrome: CoreStart['chrome'];
data: DataPublicPluginStart;
docLinks: CoreStart['docLinks'];
http: CoreSetup['http'];
i18n: CoreStart['i18n'];
notifications: CoreStart['notifications'];
notifications: CoreSetup['notifications'];
uiSettings: CoreStart['uiSettings'];
savedObjects: CoreStart['savedObjects'];
storage: Storage;
overlays: CoreStart['overlays'];
}

let DependenciesContext: React.Context<AppDependencies>;

const setAppDependencies = (deps: AppDependencies) => {
const legacyBasePath = {
prepend: deps.http.basePath.prepend,
get: deps.http.basePath.get,
remove: () => {},
};

setDependencyCache({
autocomplete: deps.data.autocomplete,
docLinks: deps.docLinks,
basePath: legacyBasePath as any,
});
DependenciesContext = createContext<AppDependencies>(deps);
return DependenciesContext.Provider;
};

export const useAppDependencies = () => {
if (!DependenciesContext) {
throw new Error(`The app dependencies Context hasn't been set.
Use the "setAppDependencies()" method when bootstrapping the app.`);
}
return useContext<AppDependencies>(DependenciesContext);
return useKibana().services as AppDependencies;
};

export const useToastNotifications = () => {
Expand All @@ -60,20 +33,3 @@ export const useToastNotifications = () => {
} = useAppDependencies();
return toastNotifications;
};

export const getAppProviders = (deps: AppDependencies) => {
const I18nContext = deps.i18n.Context;

// Create App dependencies context and get its provider
const AppDependenciesProvider = setAppDependencies(deps);

return ({ children }: { children: ReactNode }) => (
<AuthorizationProvider privilegesEndpoint={`${API_BASE_PATH}privileges`}>
<I18nContext>
<HashRouter>
<AppDependenciesProvider value={deps}>{children}</AppDependenciesProvider>
</HashRouter>
</I18nContext>
</AuthorizationProvider>
);
};
1 change: 1 addition & 0 deletions x-pack/plugins/transform/public/app/common/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ describe('Transform: Common', () => {
isAdvancedPivotEditorEnabled: false,
isAdvancedSourceEditorEnabled: false,
sourceConfigUpdated: false,
searchLanguage: 'kuery',
searchString: 'the-query',
searchQuery: 'the-search-query',
valid: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import React from 'react';
import { render, wait } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { Providers } from '../../app_dependencies.mock';
import {
getPivotQuery,
PivotAggsConfig,
Expand All @@ -19,8 +18,8 @@ import {

import { PivotPreview } from './pivot_preview';

jest.mock('ui/new_platform');
jest.mock('../../../shared_imports');
jest.mock('../../../app/app_dependencies');

describe('Transform: <PivotPreview />', () => {
// Using the async/await wait()/done() pattern to avoid act() errors.
Expand All @@ -45,11 +44,7 @@ describe('Transform: <PivotPreview />', () => {
query: getPivotQuery('the-query'),
};

const { getByText } = render(
<Providers>
<PivotPreview {...props} />
</Providers>
);
const { getByText } = render(<PivotPreview {...props} />);

// Act
// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,17 @@
import React from 'react';
import { render } from '@testing-library/react';

import { Providers } from '../app_dependencies.mock';

import { ToastNotificationText } from './toast_notification_text';

jest.mock('../../shared_imports');
jest.mock('ui/new_platform');
jest.mock('../../app/app_dependencies');

describe('ToastNotificationText', () => {
test('should render the text as plain text', () => {
const props = {
text: 'a short text message',
};
const { container } = render(
<Providers>
<ToastNotificationText {...props} />
</Providers>
);
const { container } = render(<ToastNotificationText {...props} />);
expect(container.textContent).toBe('a short text message');
});

Expand All @@ -32,11 +26,7 @@ describe('ToastNotificationText', () => {
text:
'a text message that is longer than 140 characters. a text message that is longer than 140 characters. a text message that is longer than 140 characters. ',
};
const { container } = render(
<Providers>
<ToastNotificationText {...props} />
</Providers>
);
const { container } = render(<ToastNotificationText {...props} />);
expect(container.textContent).toBe(
'a text message that is longer than 140 characters. a text message that is longer than 140 characters. a text message that is longer than 140 ...View details'
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@ import React from 'react';
import { render, wait } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { Providers } from '../../../../app_dependencies.mock';
import { getPivotQuery } from '../../../../common';
import { SearchItems } from '../../../../hooks/use_search_items';

import { SourceIndexPreview } from './source_index_preview';

jest.mock('ui/new_platform');
jest.mock('../../../../../shared_imports');
jest.mock('../../../../../app/app_dependencies');

describe('Transform: <SourceIndexPreview />', () => {
// Using the async/await wait()/done() pattern to avoid act() errors.
Expand All @@ -28,11 +27,7 @@ describe('Transform: <SourceIndexPreview />', () => {
} as SearchItems['indexPattern'],
query: getPivotQuery('the-query'),
};
const { getByText } = render(
<Providers>
<SourceIndexPreview {...props} />
</Providers>
);
const { getByText } = render(<SourceIndexPreview {...props} />);

// Act
// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import React from 'react';
import { render } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { Providers } from '../../../../app_dependencies.mock';

import { StepCreateForm } from './step_create_form';

jest.mock('ui/new_platform');
jest.mock('../../../../../shared_imports');
jest.mock('../../../../../app/app_dependencies');

describe('Transform: <StepCreateForm />', () => {
test('Minimal initialization', () => {
Expand All @@ -26,11 +24,7 @@ describe('Transform: <StepCreateForm />', () => {
onChange() {},
};

const { getByText } = render(
<Providers>
<StepCreateForm {...props} />
</Providers>
);
const { getByText } = render(<StepCreateForm {...props} />);

// Act
// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ import React from 'react';
import { render, wait } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { Providers } from '../../../../app_dependencies.mock';
import { I18nProvider } from '@kbn/i18n/react';

import { KibanaContextProvider } from '../../../../../../../../../src/plugins/kibana_react/public';

import { coreMock } from '../../../../../../../../../src/core/public/mocks';
import { dataPluginMock } from '../../../../../../../../../src/plugins/data/public/mocks';
const startMock = coreMock.createStart();

import {
PivotAggsConfigDict,
PivotGroupByConfigDict,
Expand All @@ -19,8 +26,25 @@ import { SearchItems } from '../../../../hooks/use_search_items';

import { StepDefineForm, getAggNameConflictToastMessages } from './step_define_form';

jest.mock('ui/new_platform');
jest.mock('../../../../../shared_imports');
jest.mock('../../../../../app/app_dependencies');

const createMockWebStorage = () => ({
clear: jest.fn(),
getItem: jest.fn(),
key: jest.fn(),
removeItem: jest.fn(),
setItem: jest.fn(),
length: 0,
});

const createMockStorage = () => ({
storage: createMockWebStorage(),
get: jest.fn(),
set: jest.fn(),
remove: jest.fn(),
clear: jest.fn(),
});

describe('Transform: <DefinePivotForm />', () => {
// Using the async/await wait()/done() pattern to avoid act() errors.
Expand All @@ -32,10 +56,21 @@ describe('Transform: <DefinePivotForm />', () => {
fields: [] as any[],
} as SearchItems['indexPattern'],
};

// mock services for QueryStringInput
const services = {
...startMock,
data: dataPluginMock.createStartContract(),
appName: 'the-test-app',
storage: createMockStorage(),
};

const { getByLabelText } = render(
<Providers>
<StepDefineForm onChange={jest.fn()} searchItems={searchItems as SearchItems} />
</Providers>
<I18nProvider>
<KibanaContextProvider services={services}>
<StepDefineForm onChange={jest.fn()} searchItems={searchItems as SearchItems} />
</KibanaContextProvider>
</I18nProvider>
);

// Act
Expand Down
Loading

0 comments on commit 8ec101f

Please sign in to comment.