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

Check for legacy imports in vis types and fix problems #56763

Merged
merged 10 commits into from
Feb 10, 2020
1 change: 0 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ bower_components
/src/plugins/data/common/es_query/kuery/ast/_generated_/**
/src/legacy/core_plugins/vis_type_timelion/public/_generated_/**
src/legacy/core_plugins/vis_type_vislib/public/vislib/__tests__/lib/fixtures/mock_data
/src/legacy/ui/public/angular-bootstrap
/src/legacy/ui/public/flot-charts
/test/fixtures/scenarios
/src/legacy/core_plugins/console/public/webpackShims
Expand Down
2 changes: 2 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ module.exports = {
'test/plugin_functional/plugins/**/public/np_ready/**/*',
'test/plugin_functional/plugins/**/server/np_ready/**/*',
'src/legacy/core_plugins/**/public/np_ready/**/*',
'src/legacy/core_plugins/vis_type_*/public/**/*',
'!src/legacy/core_plugins/vis_type_*/public/legacy*',
'src/legacy/core_plugins/**/server/np_ready/**/*',
'x-pack/legacy/plugins/**/public/np_ready/**/*',
'x-pack/legacy/plugins/**/server/np_ready/**/*',
Expand Down
4 changes: 0 additions & 4 deletions src/dev/precommit_hook/casing_check_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export const IGNORE_DIRECTORY_GLOBS = [
'src/babel-*',
'packages/*',
'packages/kbn-ui-framework/generator-kui',
'src/legacy/ui/public/angular-bootstrap',
'src/legacy/ui/public/flot-charts',
'src/legacy/ui/public/utils/lodash-mixins',
'test/functional/fixtures/es_archiver/visualize_source-filters',
Expand Down Expand Up @@ -124,9 +123,6 @@ export const TEMPORARILY_IGNORED_PATHS = [
'src/legacy/core_plugins/timelion/server/series_functions/__tests__/fixtures/tlConfig.js',
'src/fixtures/config_upgrade_from_4.0.0_to_4.0.1-snapshot.json',
'src/legacy/core_plugins/vis_type_vislib/public/vislib/__tests__/lib/fixtures/mock_data/terms/_seriesMultiple.js',
'src/legacy/ui/public/angular-bootstrap/bindHtml/bindHtml.js',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

angular-bootstrap got moved into kibana_legacy - while doing this, I adjusted the naming conventions to our code where necessary.

'src/legacy/ui/public/angular-bootstrap/tooltip/tooltip-html-unsafe-popup.html',
'src/legacy/ui/public/angular-bootstrap/tooltip/tooltip-popup.html',
'src/legacy/ui/public/assets/favicons/android-chrome-192x192.png',
'src/legacy/ui/public/assets/favicons/android-chrome-256x256.png',
'src/legacy/ui/public/assets/favicons/android-chrome-512x512.png',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
// these are necessary to bootstrap the local angular.
// They can stay even after NP cutover
import angular from 'angular';
import 'ui/angular-bootstrap';
import { IPrivate } from 'ui/private';
import { EuiIcon } from '@elastic/eui';
// @ts-ignore
Expand Down Expand Up @@ -76,6 +75,7 @@ import { createFieldChooserDirective } from './np_ready/components/field_chooser
import { createDiscoverFieldDirective } from './np_ready/components/field_chooser/discover_field';
import { CollapsibleSidebarProvider } from './np_ready/angular/directives/collapsible_sidebar/collapsible_sidebar';
import { DiscoverStartPlugins } from './plugin';
import { initAngularBootstrap } from '../../../../../plugins/kibana_legacy/public';
import { createCssTruncateDirective } from './np_ready/angular/directives/css_truncate';
// @ts-ignore
import { FixedScrollProvider } from './np_ready/angular/directives/fixed_scroll';
Expand All @@ -88,6 +88,7 @@ import { createRenderCompleteDirective } from './np_ready/angular/directives/ren
* needs to render, so in the end the current 'kibana' angular module is no longer necessary
*/
export function getInnerAngularModule(name: string, core: CoreStart, deps: DiscoverStartPlugins) {
initAngularBootstrap();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new way how to load angular-bootstrap modules. They still have to be listed as dependencies in the angular app modules.

const module = initializeInnerAngularModule(name, core, deps.navigation, deps.data);
configureAppAngularModule(module, core as LegacyCoreStart, true);
return module;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

import $ from 'jquery';

// TODO This is an integration test and thus requires a running platform. When moving to the new platform,
// this test has to be migrated to the newly created integration test environment.
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { npStart } from 'ui/new_platform';
// @ts-ignore
import getStubIndexPattern from 'fixtures/stubbed_logstash_index_pattern';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
// these are necessary to bootstrap the local angular.
// They can stay even after NP cutover
import angular from 'angular';
import 'ui/angular-bootstrap';
import 'angular-recursion';
import { i18nDirective, i18nFilter, I18nProvider } from '@kbn/i18n/angular';
import { CoreStart, LegacyCoreStart, IUiSettingsClient } from 'kibana/public';
Expand All @@ -34,6 +33,9 @@ import {
StateManagementConfigProvider,
configureAppAngularModule,
} from './legacy_imports';
import { initAngularBootstrap } from '../../../../plugins/kibana_legacy/public';

initAngularBootstrap();

const thirdPartyAngularDependencies = ['ngSanitize', 'ui.bootstrap', 'RecursionHelper'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ import angular, { IRootScopeService, IScope, ICompileService } from 'angular';
import $ from 'jquery';
import 'angular-sanitize';
import 'angular-mocks';
import '../table_vis.mock';

import { getAngularModule } from '../get_inner_angular';
import { initTableVisLegacyModule } from '../table_vis_legacy_module';
import { npStart } from '../legacy_imports';
import { coreMock } from '../../../../../core/public/mocks';

jest.mock('ui/new_platform');
jest.mock('ui/legacy_compat', () => ({
configureAppAngularModule: () => {},
}));

interface Sort {
columnIndex: number;
Expand Down Expand Up @@ -69,7 +73,7 @@ describe('Table Vis - Paginated table', () => {
let paginatedTable: any;

const initLocalAngular = () => {
const tableVisModule = getAngularModule('kibana/table_vis', npStart.core);
const tableVisModule = getAngularModule('kibana/table_vis', coreMock.createStart());
initTableVisLegacyModule(tableVisModule);
};

Expand Down
46 changes: 0 additions & 46 deletions src/legacy/core_plugins/vis_type_table/public/table_vis.mock.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,26 @@ import angular, { IRootScopeService, IScope, ICompileService } from 'angular';
import 'angular-mocks';
import 'angular-sanitize';
import $ from 'jquery';
import './table_vis.mock';

// @ts-ignore
import StubIndexPattern from 'test_utils/stub_index_pattern';
import { getAngularModule } from './get_inner_angular';
import { initTableVisLegacyModule } from './table_vis_legacy_module';
import { npStart, AggConfig, tabifyAggResponse } from './legacy_imports';
import { tableVisTypeDefinition } from './table_vis_type';
import { Vis } from '../../visualizations/public';
import { setup as visualizationsSetup } from '../../visualizations/public/np_ready/public/legacy';
import { Vis } from '../../visualizations/public/np_ready/public/vis';
// eslint-disable-next-line
import { stubFields } from '../../../../plugins/data/public/stubs';
// eslint-disable-next-line
import { setFieldFormats } from '../../../../plugins/data/public/services';
import { tableVisResponseHandler } from './table_vis_response_handler';
import { coreMock } from '../../../../core/public/mocks';
import { tableVisTypeDefinition } from './table_vis_type';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { AggConfigs } from 'ui/agg_types';
import { tabifyAggResponse } from './legacy_imports';

jest.mock('ui/new_platform');
jest.mock('ui/legacy_compat', () => ({
configureAppAngularModule: () => {},
}));

interface TableVisScope extends IScope {
[key: string]: any;
Expand Down Expand Up @@ -79,14 +84,14 @@ describe('Table Vis - Controller', () => {
let stubIndexPattern: any;

const initLocalAngular = () => {
const tableVisModule = getAngularModule('kibana/table_vis', npStart.core);
const tableVisModule = getAngularModule('kibana/table_vis', coreMock.createStart());
initTableVisLegacyModule(tableVisModule);
};

beforeEach(initLocalAngular);
beforeAll(() => {
visualizationsSetup.types.createBaseVisualization(tableVisTypeDefinition);
});
// beforeAll(() => {
// visualizationsSetup.types.createBaseVisualization(tableVisTypeDefinition);
// });
beforeEach(angular.mock.module('kibana/table_vis'));

beforeEach(
Expand All @@ -98,38 +103,41 @@ describe('Table Vis - Controller', () => {
);

beforeEach(() => {
setFieldFormats(({
getDefaultInstance: jest.fn(),
} as unknown) as any);
// setFieldFormats(({
// getDefaultInstance: jest.fn(),
// } as unknown) as any);
stubIndexPattern = new StubIndexPattern(
'logstash-*',
(cfg: any) => cfg,
'time',
stubFields,
npStart.core
coreMock.createStart()
);
});

function getRangeVis(params?: object) {
// @ts-ignore
return new Vis(stubIndexPattern, {
type: 'table',
params: params || {},
aggs: [
{ type: 'count', schema: 'metric' },
{
type: 'range',
schema: 'bucket',
params: {
field: 'bytes',
ranges: [
{ from: 0, to: 1000 },
{ from: 1000, to: 2000 },
],
return ({
type: tableVisTypeDefinition,
params: Object.assign({}, tableVisTypeDefinition.visConfig.defaults, params),
aggs: new AggConfigs(
stubIndexPattern,
[
{ type: 'count', schema: 'metric' },
{
type: 'range',
schema: 'bucket',
params: {
field: 'bytes',
ranges: [
{ from: 0, to: 1000 },
{ from: 1000, to: 2000 },
],
},
},
},
],
});
],
tableVisTypeDefinition.editorConfig.schemas.all
),
} as unknown) as Vis;
}

const dimensions = {
Expand All @@ -148,7 +156,7 @@ describe('Table Vis - Controller', () => {

// basically a parameterized beforeEach
function initController(vis: Vis) {
vis.aggs.aggs.forEach((agg: AggConfig, i: number) => {
vis.aggs.aggs.forEach((agg: any, i: number) => {
agg.id = 'agg_' + (i + 1);
});

Expand Down Expand Up @@ -241,13 +249,13 @@ describe('Table Vis - Controller', () => {
const vis = getRangeVis({ showPartialRows: true });
initController(vis);

expect(vis.isHierarchical()).toEqual(true);
expect(vis.type.hierarchicalData(vis)).toEqual(true);
});

test('passes partialRows:false to tabify based on the vis params', () => {
const vis = getRangeVis({ showPartialRows: false });
initController(vis);

expect(vis.isHierarchical()).toEqual(false);
expect(vis.type.hierarchicalData(vis)).toEqual(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import React from 'react';
import { EuiPanel } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { ValidatedDualRange } from 'ui/validated_range';
import { VisOptionsProps } from '../../../vis_default_editor/public';
import { SelectOption, SwitchOption } from '../../../vis_type_vislib/public';
import { TagCloudVisParams } from '../types';
import { ValidatedDualRange } from '../legacy_imports';

function TagCloudOptions({ stateParams, setValue, vis }: VisOptionsProps<TagCloudVisParams>) {
const handleFontSizeChange = ([minFontSize, maxFontSize]: [string | number, string | number]) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import React from 'react';
import * as Rx from 'rxjs';
import { take } from 'rxjs/operators';
import { render, unmountComponentAtNode } from 'react-dom';
import { I18nProvider } from '@kbn/i18n/react';

import { getFormat } from 'ui/visualize/loader/pipeline_helpers/utilities';
import { I18nContext } from 'ui/i18n';
import { getFormat } from '../legacy_imports';

import { Label } from './label';
import { TagCloud } from './tag_cloud';
Expand Down Expand Up @@ -65,9 +65,9 @@ export function createTagCloudVisualization({ colors }) {
this._containerNode.appendChild(this._feedbackNode);
this._feedbackMessage = React.createRef();
render(
<I18nContext>
<I18nProvider>
<FeedbackMessage ref={this._feedbackMessage} />
</I18nContext>,
</I18nProvider>,
this._feedbackNode
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@
*/

export { Schemas } from 'ui/agg_types';
export { ValidatedDualRange } from 'ui/validated_range';
export { getFormat } from 'ui/visualize/loader/pipeline_helpers/utilities';
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
* under the License.
*/

import chrome from 'ui/chrome';
import { getUISettings } from '../../services';

export function getDefaultQueryLanguage() {
return chrome.getUiSettingsClient().get('search:queryLanguage');
return getUISettings().get('search:queryLanguage');
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@

import handlebars from 'handlebars/dist/handlebars';
import { isNumber } from 'lodash';
import { npStart } from 'ui/new_platform';
import { inputFormats, outputFormats, isDuration } from '../lib/durations';
import { getFieldFormats } from '../../services';

export const createTickFormatter = (format = '0,0.[00]', template, getConfig = null) => {
const fieldFormats = npStart.plugins.data.fieldFormats;
const fieldFormats = getFieldFormats();

if (!template) template = '{{value}}';
const render = handlebars.compile(template, { knownHelpersOnly: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
* under the License.
*/

import { npStart } from 'ui/new_platform';
import { createTickFormatter } from './tick_formatter';
import { getFieldFormatsRegistry } from '../../../../../../test_utils/public/stub_field_formats';
import { setFieldFormats } from '../../services';

const mockUiSettings = {
get: item => {
Expand All @@ -46,9 +46,7 @@ const mockCore = {
};

describe('createTickFormatter(format, template)', () => {
npStart.plugins.data = {
fieldFormats: getFieldFormatsRegistry(mockCore),
};
setFieldFormats(getFieldFormatsRegistry(mockCore));

test('returns a number with two decimal place by default', () => {
const fn = createTickFormatter();
Expand Down
Loading