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

Combine visualizations and visualize plugins #121550

Merged
merged 27 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0309a8c
[WIP] Combine visualizations and visualize plugins
DianaDerevyankina Dec 17, 2021
8204d60
Merge branch 'main' into issues/113737
DianaDerevyankina Dec 23, 2021
1c5d5ed
Revert some changes and do some refactoring
DianaDerevyankina Dec 23, 2021
4e4e91d
Refactor some code
DianaDerevyankina Dec 23, 2021
9f7f2ca
Fix some tests
DianaDerevyankina Dec 28, 2021
940d622
Fix functional tests and some jest test cases
DianaDerevyankina Dec 28, 2021
9daa15b
Update telemetry
DianaDerevyankina Dec 28, 2021
2c602c4
Fix get_visualization_instance.test and plugin-list.asciidoc
DianaDerevyankina Dec 29, 2021
d69fe69
Refactor some code
DianaDerevyankina Dec 29, 2021
611d63e
Merge branch 'main' into issues/113737
kibanamachine Dec 29, 2021
3bfad19
Merge branch 'main' into issues/113737
kibanamachine Dec 29, 2021
434989d
fix CI
alexwizp Dec 29, 2021
ec538b3
Add visualizations to vis_default_editor tsconfig
DianaDerevyankina Dec 30, 2021
cf8e8c6
Revert changes related to telemetry and permissions
DianaDerevyankina Dec 30, 2021
7a87489
Add dashboard to timeseries tsconfig.json
DianaDerevyankina Dec 30, 2021
e0d7ca0
Update limits file
DianaDerevyankina Dec 30, 2021
0af27e2
Update translation keys
DianaDerevyankina Dec 30, 2021
712433c
Merge branch 'main' into issues/113737
kibanamachine Jan 3, 2022
543173c
Add capabilitiesProvider back to server and replace visEditorsRegistr…
DianaDerevyankina Jan 3, 2022
0f31cdb
Update mocks.ts
DianaDerevyankina Jan 3, 2022
c199925
Revert changes related to visEditorsRegistry
DianaDerevyankina Jan 4, 2022
60bda6d
Get rid of visEditorsRegistry getter and setter
DianaDerevyankina Jan 4, 2022
d4fcb65
Merge branch 'main' into issues/113737
DianaDerevyankina Jan 5, 2022
33d3443
Remove dashboard from timeseries/tsconfig.json
DianaDerevyankina Jan 5, 2022
88f25b8
Return back dashboard dependency to timeseries/tsconfig.json, rename …
DianaDerevyankina Jan 5, 2022
37e4d71
Merge branch 'main' into issues/113737
DianaDerevyankina Jan 12, 2022
ebd3fd0
Remove comma in .i18nrc.json
DianaDerevyankina Jan 12, 2022
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
1 change: 0 additions & 1 deletion .i18nrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
"visTypePie": "src/plugins/vis_types/pie",
"visTypeHeatmap": "src/plugins/vis_types/heatmap",
"visualizations": "src/plugins/visualizations",
"visualize": "src/plugins/visualize",
"apmOss": "src/plugins/apm_oss",
"usageCollection": "src/plugins/usage_collection"
},
Expand Down
4 changes: 0 additions & 4 deletions docs/developer/plugin-list.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,6 @@ The plugin exposes the static DefaultEditorController class to consume.
|WARNING: Missing README.


|{kib-repo}blob/{branch}/src/plugins/visualize[visualize]
|WARNING: Missing README.


|===

[discrete]
Expand Down
1 change: 0 additions & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ pageLoadAssetSize:
visTypeVislib: 242838
visTypeXy: 113478
visualizations: 90000
visualize: 57431
watcher: 43598
runtimeFields: 41752
stackAlerts: 29684
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_default_editor/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"id": "visDefaultEditor",
"version": "kibana",
"ui": true,
"optionalPlugins": ["visualize"],
"optionalPlugins": ["visualizations"],
"requiredBundles": ["kibanaUtils", "kibanaReact", "data", "fieldFormats", "discover", "esUiShared"],
"owner": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the description from the other kibana.json here as well?

"name": "Vis Editors",
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/vis_default_editor/public/default_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ import React, { useEffect, useRef, useState, useCallback } from 'react';
import { EventEmitter } from 'events';
import { EuiResizableContainer } from '@elastic/eui';

import { Vis, VisualizeEmbeddableContract } from 'src/plugins/visualizations/public';
import { EditorRenderProps } from 'src/plugins/visualize/public';
import {
Vis,
VisualizeEmbeddableContract,
EditorRenderProps,
} from 'src/plugins/visualizations/public';
import { KibanaContextProvider } from '../../kibana_react/public';
import { Storage } from '../../kibana_utils/public';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { EventEmitter } from 'events';
import { EuiErrorBoundary, EuiLoadingChart } from '@elastic/eui';

import { Vis, VisualizeEmbeddableContract } from 'src/plugins/visualizations/public';
import { IEditorController, EditorRenderProps } from 'src/plugins/visualize/public';
import { IEditorController, EditorRenderProps } from 'src/plugins/visualizations/public';
import { KibanaThemeProvider } from '../../kibana_react/public';
import { getTheme } from './services';

Expand Down
10 changes: 5 additions & 5 deletions src/plugins/vis_default_editor/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@

import { CoreSetup, Plugin } from 'kibana/public';

import { VisualizePluginSetup } from '../../visualize/public';
import { DefaultEditorController } from './default_editor_controller';
import { setTheme } from './services';
import type { VisualizationsSetup } from '../../visualizations/public';

export interface VisDefaultEditorSetupDependencies {
visualize: VisualizePluginSetup;
visualizations: VisualizationsSetup;
}

export class VisDefaultEditorPlugin
implements Plugin<void, void, VisDefaultEditorSetupDependencies, {}>
{
public setup(core: CoreSetup, { visualize }: VisDefaultEditorSetupDependencies) {
public setup(core: CoreSetup, { visualizations }: VisDefaultEditorSetupDependencies) {
setTheme(core.theme);
if (visualize) {
visualize.visEditorsRegistry.registerDefault(DefaultEditorController);
if (visualizations) {
visualizations.visEditorsRegistry.registerDefault(DefaultEditorController);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_default_editor/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"references": [
{ "path": "../../core/tsconfig.json" },
{ "path": "../data/tsconfig.json" },
{ "path": "../visualize/tsconfig.json" },
{ "path": "../visualizations/tsconfig.json" },
{ "path": "../discover/tsconfig.json" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have we added the discover dependency here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think in case of adding visualizations/tsconfig.json it should still work without /discover/tsconfig.json
but for that case I prefer to keep discover cause it's in requiredBundles.

image

{ "path": "../kibana_utils/tsconfig.json" },
{ "path": "../kibana_react/tsconfig.json" },
{ "path": "../field_formats/tsconfig.json" }
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_types/timeseries/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"kibanaVersion": "kibana",
"server": true,
"ui": true,
"requiredPlugins": ["charts", "data", "expressions", "visualizations", "visualize"],
"requiredPlugins": ["charts", "data", "expressions", "visualizations"],
"optionalPlugins": ["home","usageCollection"],
"requiredBundles": ["kibanaUtils", "kibanaReact", "fieldFormats"],
"owner": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { fetchFields, VisFields } from '../lib/fetch_fields';
import { getDataStart, getCoreStart } from '../../services';
import type { TimeseriesVisParams } from '../../types';
import { UseIndexPatternModeCallout } from './use_index_patter_mode_callout';
import type { EditorRenderProps } from '../../../../../visualize/public';
import type { EditorRenderProps } from '../../../../../visualizations/public';

const VIS_STATE_DEBOUNCE_DELAY = 200;
const APP_NAME = 'VisEditor';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
import type { EventEmitter } from 'events';
import type { Vis, VisualizeEmbeddableContract } from 'src/plugins/visualizations/public';
import type { IEditorController, EditorRenderProps } from 'src/plugins/visualize/public';

import type {
Vis,
VisualizeEmbeddableContract,
IEditorController,
EditorRenderProps,
} from 'src/plugins/visualizations/public';
import { getUISettings, getI18n, getCoreStart } from '../services';
import { VisEditor } from './components/vis_editor_lazy';
import type { TimeseriesVisParams } from '../types';
Expand Down
9 changes: 2 additions & 7 deletions src/plugins/vis_types/timeseries/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'kibana/public';
import { Plugin as ExpressionsPublicPlugin } from '../../../expressions/public';
import { VisualizationsSetup } from '../../../visualizations/public';
import { VisualizePluginSetup } from '../../../visualize/public';
import { EditorController, TSVB_EDITOR_NAME } from './application/editor_controller';

import { createMetricsFn } from './metrics_fn';
Expand All @@ -30,7 +29,6 @@ import { getTimeseriesVisRenderer } from './timeseries_vis_renderer';
export interface MetricsPluginSetupDependencies {
expressions: ReturnType<ExpressionsPublicPlugin['setup']>;
visualizations: VisualizationsSetup;
visualize: VisualizePluginSetup;
}

/** @internal */
Expand All @@ -47,11 +45,8 @@ export class MetricsPlugin implements Plugin<void, void> {
this.initializerContext = initializerContext;
}

public setup(
core: CoreSetup,
{ expressions, visualizations, visualize }: MetricsPluginSetupDependencies
) {
visualize.visEditorsRegistry.register(TSVB_EDITOR_NAME, EditorController);
public setup(core: CoreSetup, { expressions, visualizations }: MetricsPluginSetupDependencies) {
visualizations.visEditorsRegistry.register(TSVB_EDITOR_NAME, EditorController);
expressions.registerFunction(createMetricsFn);
expressions.registerRenderer(
getTimeseriesVisRenderer({
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_types/timeseries/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
{ "path": "../../data/tsconfig.json" },
{ "path": "../../expressions/tsconfig.json" },
{ "path": "../../visualizations/tsconfig.json" },
{ "path": "../../visualize/tsconfig.json" },
{ "path": "../../dashboard/tsconfig.json" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding the dashboard dependency here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In src/plugins/vis_types/timeseries/server/usage_collector/get_usage_collector.ts we have an import from dashboard. Dashboard plugin should be added to rootDir

image

{ "path": "../../kibana_utils/tsconfig.json" },
{ "path": "../../kibana_react/tsconfig.json" },
{ "path": "../../usage_collection/tsconfig.json" },
Expand Down
16 changes: 16 additions & 0 deletions src/plugins/visualizations/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,19 @@

export const VISUALIZE_ENABLE_LABS_SETTING = 'visualize:enableLabs';
export const VISUALIZE_EMBEDDABLE_TYPE = 'visualization';

export const STATE_STORAGE_KEY = '_a';
export const GLOBAL_STATE_STORAGE_KEY = '_g';

export const APP_NAME = 'visualize';
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename this constant to VISUALIZE_APP_NAME as the current app is called visualizations and it can be confusing.


export const VisualizeConstants = {
VISUALIZE_BASE_PATH: '/app/visualize',
LANDING_PAGE_PATH: '/',
WIZARD_STEP_1_PAGE_PATH: '/new',
WIZARD_STEP_2_PAGE_PATH: '/new/configure',
CREATE_PATH: '/create',
EDIT_PATH: '/edit',
EDIT_BY_VALUE_PATH: '/edit_by_value',
APP_ID: 'visualize',
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type { LocatorDefinition, LocatorPublic } from 'src/plugins/share/common'
import { isFilterPinned } from '../../data/common';
import { url } from '../../kibana_utils/common';
import { GLOBAL_STATE_STORAGE_KEY, STATE_STORAGE_KEY, VisualizeConstants } from './constants';
import { PureVisState } from './types';
import type { SavedVisState } from './types';

const removeEmptyKeys = (o: Record<string, Serializable>): Record<string, Serializable> =>
omitBy(o, (v) => v == null);
Expand Down Expand Up @@ -59,7 +59,7 @@ export type VisualizeLocatorParams = {
*
* @note This is required to navigate to "create" page (i.e., when no `visId` has been provided).
*/
vis?: PureVisState;
vis?: SavedVisState;

/**
* Whether this visualization is linked a saved search.
Expand Down
9 changes: 6 additions & 3 deletions src/plugins/visualizations/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@
"data",
"expressions",
"uiActions",
"urlForwarding",
"navigation",
"embeddable",
"inspector",
"savedObjects"
"savedObjects",
"presentationUtil"
],
"optionalPlugins": ["usageCollection", "spaces", "savedObjectsTaggingOss"],
"requiredBundles": ["kibanaUtils", "discover", "kibanaReact"],
"optionalPlugins": [ "home", "share", "usageCollection", "spaces", "savedObjectsTaggingOss"],
"requiredBundles": ["kibanaUtils", "discover", "kibanaReact", "home"],
"extraPublicDirs": ["common/constants", "common/prepare_log_table", "common/expression_functions"],
"owner": {
"name": "Vis Editors",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
VisualizeNoMatch,
VisualizeByValueEditor,
} from './components';
import { VisualizeConstants } from './visualize_constants';
import { VisualizeConstants } from '../../common/constants';

export interface VisualizeAppProps {
onAppLeave: AppMountParameters['onAppLeave'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const InfoComponent = () => {
const title = (
<>
<FormattedMessage
id="visualize.experimentalVisInfoText"
id="visualizations.experimentalVisInfoText"
defaultMessage="This visualization is experimental and is not subject to the support SLA of official GA features.
For feedback, please create an issue in {githubLink}."
values={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ export const SplitChartWarning = () => {
data-test-subj="vizSplitChartWarning"
title={
<FormattedMessage
id="visualize.newHeatmapChart.notificationMessage"
id="visualizations.newHeatmapChart.notificationMessage"
defaultMessage="The new heatmap charts library does not yet support split chart aggregation. {conditionalMessage}"
values={{
conditionalMessage: (
<>
{canEditAdvancedSettings && (
<FormattedMessage
id="visualize.newHeatmapChart.conditionalMessage.newLibrary"
id="visualizations.newHeatmapChart.conditionalMessage.newLibrary"
defaultMessage="Switch to the old library in {link}"
values={{
link: (
<EuiLink href={advancedSettingsLink}>
<FormattedMessage
id="visualize.newHeatmapChart.conditionalMessage.advanced settings link"
id="visualizations.newHeatmapChart.conditionalMessage.advancedSettingsLink"
defaultMessage="Advanced Settings."
/>
</EuiLink>
Expand All @@ -49,7 +49,7 @@ export const SplitChartWarning = () => {
)}
{!canEditAdvancedSettings && (
<FormattedMessage
id="visualize.legacyCharts.conditionalMessage.noPermissions"
id="visualizations.legacyCharts.conditionalMessage.noPermissions"
defaultMessage="Contact your system administrator to switch to the old library."
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { VisualizeServices } from '../types';
import { VisualizeEditorCommon } from './visualize_editor_common';
import { VisualizeAppProps } from '../app';
import { VisualizeConstants } from '../..';
import { VisualizeConstants } from '../../../common/constants';

export const VisualizeByValueEditor = ({ onAppLeave }: VisualizeAppProps) => {
const [originatingApp, setOriginatingApp] = useState<string>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { VisualizeServices } from '../types';
import { VisualizeEditorCommon } from './visualize_editor_common';
import { VisualizeAppProps } from '../app';
import { VisualizeConstants } from '../..';
import { VisualizeConstants } from '../../../common/constants';

export const VisualizeEditor = ({ onAppLeave }: VisualizeAppProps) => {
const { id: visualizationIdFromUrl } = useParams<{ id: string }>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const VisualizeEditorCommon = ({
const newPath = `${urlFor(newObjectId!)}${services.history.location.search}`;
await services.spaces.ui.redirectLegacyUrl(
newPath,
i18n.translate('visualize.legacyUrlConflict.objectNoun', {
i18n.translate('visualizations.legacyUrlConflict.objectNoun', {
defaultMessage: '{visName} visualization',
values: {
visName: visInstance?.vis?.type.title,
Expand All @@ -96,7 +96,7 @@ export const VisualizeEditorCommon = ({
const otherObjectId = sharingSavedObjectProps?.aliasTargetId!; // This is always defined if outcome === 'conflict'
const otherObjectPath = `${urlFor(otherObjectId)}${services.history.location.search}`;
return services.spaces.ui.components.getLegacyUrlConflict({
objectNoun: i18n.translate('visualize.legacyUrlConflict.objectNoun', {
objectNoun: i18n.translate('visualizations.legacyUrlConflict.objectNoun', {
defaultMessage: '{visName} visualization',
values: {
visName: visInstance?.vis?.type.title,
Expand Down Expand Up @@ -144,7 +144,7 @@ export const VisualizeEditorCommon = ({
<h1>
{'savedVis' in visInstance && visInstance.savedVis.id ? (
<FormattedMessage
id="visualize.pageHeading"
id="visualizations.pageHeading"
defaultMessage="{chartName} {chartType} visualization"
values={{
chartName: (visInstance as SavedVisInstance).savedVis.title,
Expand All @@ -153,7 +153,7 @@ export const VisualizeEditorCommon = ({
/>
) : (
<FormattedMessage
id="visualize.byValue_pageHeading"
id="visualizations.byValue_pageHeading"
defaultMessage="Visualization of type {chartType} embedded into {originatingApp} app"
values={{
chartType: visInstance.vis.type.title,
Expand Down
Loading