From e1bf92ce034b348709e74890264811408134db2d Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Wed, 3 Nov 2021 02:27:45 -0700 Subject: [PATCH 1/6] [Reporting] Internally correct the hostname to "localhost" if "server.host" is "0.0.0.0" (#117022) * [Reporting] Internal correction for server hostname if "server.host" is "0.0.0.0" * add schema validation * correction to the failover logic the test is validation * update per feedback * more check against invalid hostnames * remove silly translations for server logs * remove unused translations * improve the tests --- .../server/config/create_config.test.ts | 92 +++++++++++++------ .../reporting/server/config/create_config.ts | 51 +++++----- .../reporting/server/config/schema.test.ts | 28 ++++-- .../plugins/reporting/server/config/schema.ts | 14 ++- .../test_helpers/create_mock_levellogger.ts | 3 +- .../translations/translations/ja-JP.json | 4 - .../translations/translations/zh-CN.json | 4 - 7 files changed, 123 insertions(+), 73 deletions(-) diff --git a/x-pack/plugins/reporting/server/config/create_config.test.ts b/x-pack/plugins/reporting/server/config/create_config.test.ts index e042142a54a0f..718638e1ba62b 100644 --- a/x-pack/plugins/reporting/server/config/create_config.test.ts +++ b/x-pack/plugins/reporting/server/config/create_config.test.ts @@ -5,27 +5,29 @@ * 2.0. */ -import { CoreSetup, PluginInitializerContext } from 'src/core/server'; +import * as Rx from 'rxjs'; +import { CoreSetup, HttpServerInfo, PluginInitializerContext } from 'src/core/server'; import { coreMock } from 'src/core/server/mocks'; -import { LevelLogger } from '../lib'; -import { createMockConfigSchema } from '../test_helpers'; +import { LevelLogger } from '../lib/level_logger'; +import { createMockConfigSchema, createMockLevelLogger } from '../test_helpers'; +import { ReportingConfigType } from './'; import { createConfig$ } from './create_config'; +const createMockConfig = ( + mockInitContext: PluginInitializerContext +): Rx.Observable => mockInitContext.config.create(); + describe('Reporting server createConfig$', () => { let mockCoreSetup: CoreSetup; let mockInitContext: PluginInitializerContext; - let mockLogger: LevelLogger; + let mockLogger: jest.Mocked; beforeEach(() => { mockCoreSetup = coreMock.createSetup(); mockInitContext = coreMock.createPluginInitializerContext( createMockConfigSchema({ kibanaServer: {} }) ); - mockLogger = { - warn: jest.fn(), - debug: jest.fn(), - clone: jest.fn().mockImplementation(() => mockLogger), - } as unknown as LevelLogger; + mockLogger = createMockLevelLogger(); }); afterEach(() => { @@ -37,19 +39,12 @@ describe('Reporting server createConfig$', () => { ...createMockConfigSchema({ kibanaServer: {} }), encryptionKey: undefined, }); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.encryptionKey).toMatch(/\S{32,}/); // random 32 characters - expect(result.kibanaServer).toMatchInlineSnapshot(` - Object { - "hostname": "localhost", - "port": 80, - "protocol": "http", - } - `); - expect((mockLogger.warn as any).mock.calls.length).toBe(1); - expect((mockLogger.warn as any).mock.calls[0]).toMatchObject([ + expect(mockLogger.warn.mock.calls.length).toBe(1); + expect(mockLogger.warn.mock.calls[0]).toMatchObject([ 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.', ]); }); @@ -60,10 +55,10 @@ describe('Reporting server createConfig$', () => { encryptionKey: 'iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii', }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.encryptionKey).toMatch('iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii'); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('uses the user-provided encryption key, reporting kibanaServer settings to override server info', async () => { @@ -77,7 +72,7 @@ describe('Reporting server createConfig$', () => { }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result).toMatchInlineSnapshot(` @@ -108,7 +103,7 @@ describe('Reporting server createConfig$', () => { }, } `); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('uses user-provided disableSandbox: false', async () => { @@ -118,11 +113,11 @@ describe('Reporting server createConfig$', () => { capture: { browser: { chromium: { disableSandbox: false } } }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: false }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('uses user-provided disableSandbox: true', async () => { @@ -132,11 +127,11 @@ describe('Reporting server createConfig$', () => { capture: { browser: { chromium: { disableSandbox: true } } }, }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: true }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); it('provides a default for disableSandbox', async () => { @@ -145,10 +140,49 @@ describe('Reporting server createConfig$', () => { encryptionKey: '888888888888888888888888888888888', }) ); - const mockConfig$: any = mockInitContext.config.create(); + const mockConfig$ = createMockConfig(mockInitContext); const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); expect(result.capture.browser.chromium).toMatchObject({ disableSandbox: expect.any(Boolean) }); - expect((mockLogger.warn as any).mock.calls.length).toBe(0); + expect(mockLogger.warn.mock.calls.length).toBe(0); }); + + for (const hostname of [ + '0', + '0.0', + '0.0.0', + '0.0.0.0', + '0000:0000:0000:0000:0000:0000:0000:0000', + '::', + ]) { + it(`apply failover logic when hostname is given as ${hostname}`, async () => { + mockInitContext = coreMock.createPluginInitializerContext( + createMockConfigSchema({ + encryptionKey: 'aaaaaaaaaaaaabbbbbbbbbbbbaaaaaaaaa', + // overwrite settings added by createMockConfigSchema and apply the default settings + // TODO make createMockConfigSchema _not_ default xpack.reporting.kibanaServer.hostname to 'localhost' + kibanaServer: { + hostname: undefined, + port: undefined, + }, + }) + ); + mockCoreSetup.http.getServerInfo = jest.fn().mockImplementation( + (): HttpServerInfo => ({ + name: 'cool server', + hostname, + port: 5601, + protocol: 'http', + }) + ); + + const mockConfig$ = createMockConfig(mockInitContext); + const result = await createConfig$(mockCoreSetup, mockConfig$, mockLogger).toPromise(); + expect(result.kibanaServer).toMatchObject({ + hostname: 'localhost', + port: 5601, + protocol: 'http', + }); + }); + } }); diff --git a/x-pack/plugins/reporting/server/config/create_config.ts b/x-pack/plugins/reporting/server/config/create_config.ts index b1d69d17334be..5de54a43582ab 100644 --- a/x-pack/plugins/reporting/server/config/create_config.ts +++ b/x-pack/plugins/reporting/server/config/create_config.ts @@ -5,9 +5,9 @@ * 2.0. */ -import { i18n } from '@kbn/i18n'; import crypto from 'crypto'; -import { upperFirst } from 'lodash'; +import ipaddr from 'ipaddr.js'; +import { sum, upperFirst } from 'lodash'; import { Observable } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { CoreSetup } from 'src/core/server'; @@ -33,20 +33,29 @@ export function createConfig$( let encryptionKey = config.encryptionKey; if (encryptionKey === undefined) { logger.warn( - i18n.translate('xpack.reporting.serverConfig.randomEncryptionKey', { - defaultMessage: - 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on ' + - 'restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.', - }) + 'Generating a random key for xpack.reporting.encryptionKey. To prevent sessions from being invalidated on ' + + 'restart, please set xpack.reporting.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.' ); encryptionKey = crypto.randomBytes(16).toString('hex'); } const { kibanaServer: reportingServer } = config; const serverInfo = core.http.getServerInfo(); - // kibanaServer.hostname, default to server.host - const kibanaServerHostname = reportingServer.hostname + // set kibanaServer.hostname, default to server.host, don't allow "0.0.0.0" as it breaks in Windows + let kibanaServerHostname = reportingServer.hostname ? reportingServer.hostname : serverInfo.hostname; + + if ( + ipaddr.isValid(kibanaServerHostname) && + !sum(ipaddr.parse(kibanaServerHostname).toByteArray()) + ) { + logger.warn( + `Found 'server.host: "0.0.0.0"' in Kibana configuration. Reporting is not able to use this as the Kibana server hostname.` + + ` To enable PNG/PDF Reporting to work, 'xpack.reporting.kibanaServer.hostname: localhost' is automatically set in the configuration.` + + ` You can prevent this message by adding 'xpack.reporting.kibanaServer.hostname: localhost' in kibana.yml.` + ); + kibanaServerHostname = 'localhost'; + } // kibanaServer.port, default to server.port const kibanaServerPort = reportingServer.port ? reportingServer.port : serverInfo.port; // kibanaServer.protocol, default to server.protocol @@ -66,36 +75,24 @@ export function createConfig$( mergeMap(async (config) => { if (config.capture.browser.chromium.disableSandbox != null) { // disableSandbox was set by user - return config; + return { ...config }; } // disableSandbox was not set by user, apply default for OS const { os, disableSandbox } = await getDefaultChromiumSandboxDisabled(); const osName = [os.os, os.dist, os.release].filter(Boolean).map(upperFirst).join(' '); - logger.debug( - i18n.translate('xpack.reporting.serverConfig.osDetected', { - defaultMessage: `Running on OS: '{osName}'`, - values: { osName }, - }) - ); + logger.debug(`Running on OS: '{osName}'`); if (disableSandbox === true) { logger.warn( - i18n.translate('xpack.reporting.serverConfig.autoSet.sandboxDisabled', { - defaultMessage: `Chromium sandbox provides an additional layer of protection, but is not supported for {osName} OS. Automatically setting '{configKey}: true'.`, - values: { - configKey: 'xpack.reporting.capture.browser.chromium.disableSandbox', - osName, - }, - }) + `Chromium sandbox provides an additional layer of protection, but is not supported for ${osName} OS.` + + ` Automatically setting 'xpack.reporting.capture.browser.chromium.disableSandbox: true'.` ); } else { logger.info( - i18n.translate('xpack.reporting.serverConfig.autoSet.sandboxEnabled', { - defaultMessage: `Chromium sandbox provides an additional layer of protection, and is supported for {osName} OS. Automatically enabling Chromium sandbox.`, - values: { osName }, - }) + `Chromium sandbox provides an additional layer of protection, and is supported for ${osName} OS.` + + ` Automatically enabling Chromium sandbox.` ); } diff --git a/x-pack/plugins/reporting/server/config/schema.test.ts b/x-pack/plugins/reporting/server/config/schema.test.ts index 6ad7d03bd1a8f..7f1da5b55ccb6 100644 --- a/x-pack/plugins/reporting/server/config/schema.test.ts +++ b/x-pack/plugins/reporting/server/config/schema.test.ts @@ -288,11 +288,25 @@ describe('Reporting Config Schema', () => { `); }); - it(`logs the proper validation messages`, () => { - // kibanaServer - const throwValidationErr = () => ConfigSchema.validate({ kibanaServer: { hostname: '0' } }); - expect(throwValidationErr).toThrowError( - `[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).` - ); - }); + for (const address of ['0', '0.0', '0.0.0']) { + it(`fails to validate "kibanaServer.hostname" with an invalid hostname: "${address}"`, () => { + expect(() => + ConfigSchema.validate({ + kibanaServer: { hostname: address }, + }) + ).toThrowError(`[kibanaServer.hostname]: value must be a valid hostname (see RFC 1123).`); + }); + } + + for (const address of ['0.0.0.0', '0000:0000:0000:0000:0000:0000:0000:0000', '::']) { + it(`fails to validate "kibanaServer.hostname" hostname as zero: "${address}"`, () => { + expect(() => + ConfigSchema.validate({ + kibanaServer: { hostname: address }, + }) + ).toThrowError( + `[kibanaServer.hostname]: cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead` + ); + }); + } }); diff --git a/x-pack/plugins/reporting/server/config/schema.ts b/x-pack/plugins/reporting/server/config/schema.ts index 5b15260be06cb..4c56fc4c6db60 100644 --- a/x-pack/plugins/reporting/server/config/schema.ts +++ b/x-pack/plugins/reporting/server/config/schema.ts @@ -6,10 +6,22 @@ */ import { ByteSizeValue, schema, TypeOf } from '@kbn/config-schema'; +import ipaddr from 'ipaddr.js'; +import { sum } from 'lodash'; import moment from 'moment'; const KibanaServerSchema = schema.object({ - hostname: schema.maybe(schema.string({ hostname: true })), + hostname: schema.maybe( + schema.string({ + hostname: true, + validate(value) { + if (ipaddr.isValid(value) && !sum(ipaddr.parse(value).toByteArray())) { + // prevent setting a hostname that fails in Chromium on Windows + return `cannot use '0.0.0.0' as Kibana host name, consider using the default (localhost) instead`; + } + }, + }) + ), port: schema.maybe(schema.number()), protocol: schema.maybe( schema.string({ diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts index 1a8bfe7b70208..a6e6be47bdfcd 100644 --- a/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts +++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_levellogger.ts @@ -16,7 +16,6 @@ export function createMockLevelLogger() { const logger = new LevelLogger(loggingSystemMock.create()) as jest.Mocked; - logger.clone.mockImplementation(createMockLevelLogger); // logger.debug.mockImplementation(consoleLogger('debug')); // uncomment this to see debug logs in jest tests logger.info.mockImplementation(consoleLogger('info')); logger.warn.mockImplementation(consoleLogger('warn')); @@ -24,5 +23,7 @@ export function createMockLevelLogger() { logger.error.mockImplementation(consoleLogger('error')); logger.trace.mockImplementation(consoleLogger('trace')); + logger.clone.mockImplementation(() => logger); + return logger; } diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 857e97daa3515..47876265894ec 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -19243,10 +19243,6 @@ "xpack.reporting.screenCapturePanelContent.canvasLayoutLabel": "全ページレイアウト", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingHelpText": "複数のページを使用します。ページごとに最大2のビジュアライゼーションが表示されます", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingLabel": "印刷用に最適化", - "xpack.reporting.serverConfig.autoSet.sandboxDisabled": "Chromiumサンドボックスは保護が強化されていますが、{osName} OSではサポートされていません。自動的に'{configKey}: true'を設定しています。", - "xpack.reporting.serverConfig.autoSet.sandboxEnabled": "Chromiumサンドボックスは保護が強化され、{osName} OSでサポートされています。自動的にChromiumサンドボックスを有効にしています。", - "xpack.reporting.serverConfig.osDetected": "OSは'{osName}'で実行しています", - "xpack.reporting.serverConfig.randomEncryptionKey": "xpack.reporting.encryptionKey のランダムキーを生成しています。再起動時にセッションが無効にならないようにするには、kibana.yml でxpack.reporting.encryptionKey を設定するか、bin/kibana-encryption-keys コマンドを使用してください。", "xpack.reporting.shareContextMenu.csvReportsButtonLabel": "CSV レポート", "xpack.reporting.shareContextMenu.pdfReportsButtonLabel": "PDF レポート", "xpack.reporting.shareContextMenu.pngReportsButtonLabel": "PNG レポート", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 6c860a5b2cc4d..0fd4fea956a11 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -19520,10 +19520,6 @@ "xpack.reporting.screenCapturePanelContent.canvasLayoutLabel": "全页面布局", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingHelpText": "使用多页，每页最多显示 2 个可视化", "xpack.reporting.screenCapturePanelContent.optimizeForPrintingLabel": "打印优化", - "xpack.reporting.serverConfig.autoSet.sandboxDisabled": "Chromium 沙盒提供附加保护层，但不受 {osName} OS 支持。自动设置“{configKey}: true”。", - "xpack.reporting.serverConfig.autoSet.sandboxEnabled": "Chromium 沙盒提供附加保护层，受 {osName} OS 支持。自动启用 Chromium 沙盒。", - "xpack.reporting.serverConfig.osDetected": "正在以下 OS 上运行：“{osName}”", - "xpack.reporting.serverConfig.randomEncryptionKey": "为 xpack.reporting.encryptionKey 生成随机密钥。为防止会话在重启时失效，请在 kibana.yml 中设置 xpack.reporting.encryptionKey 或使用 bin/kibana-encryption-keys 命令。", "xpack.reporting.shareContextMenu.csvReportsButtonLabel": "CSV 报告", "xpack.reporting.shareContextMenu.pdfReportsButtonLabel": "PDF 报告", "xpack.reporting.shareContextMenu.pngReportsButtonLabel": "PNG 报告", From 2ad1f9139f0d7a15697cdc15c04971373516f555 Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Wed, 3 Nov 2021 10:44:11 +0100 Subject: [PATCH 2/6] [Lens] Removing updateVisualizationState updater (#113779) * make editVisualizationAction separate action to get rid of updater * add insertLayer to remove updater when inserting new Layer * wip * removeLayers before merging into one function * chart switch refactor to single updater * replace updater with newState * check onEditAction * remove layer remove updater * addLayer updater * set Layer Default Dimension * wip * mocks divided * visualization mock lint fix * add tests for layers * further divide * rename ts to tsx * Rename index.tsx to index.ts * Update x-pack/plugins/lens/public/state_management/index.ts * Revert "[Lens] cleanup divide mock file to smaller pieces" This reverts commit 1bb2e1a4f1431f843cf777ad6f90eeca047e0774. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../config_panel/config_panel.test.tsx | 203 ++++------ .../config_panel/config_panel.tsx | 179 +-------- .../config_panel/layer_actions.test.ts | 130 ------ .../config_panel/layer_actions.ts | 95 ----- .../editor_frame/editor_frame.test.tsx | 51 ++- .../workspace_panel/chart_switch.test.tsx | 158 ++++---- .../workspace_panel/chart_switch.tsx | 50 +-- .../workspace_panel/workspace_panel.tsx | 6 +- .../workspace_panel_wrapper.tsx | 3 +- .../lens/public/state_management/index.ts | 7 +- .../state_management/lens_slice.test.ts | 117 +++++- .../public/state_management/lens_slice.ts | 377 +++++++++++++++--- x-pack/plugins/lens/public/types.ts | 2 +- x-pack/plugins/lens/public/utils.ts | 17 +- 14 files changed, 680 insertions(+), 715 deletions(-) delete mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.test.ts delete mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.ts diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx index a8436edb63f63..cd26cd3197587 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx @@ -7,7 +7,13 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; -import { createMockFramePublicAPI, visualizationMap, datasourceMap } from '../../../mocks'; +import { + createMockFramePublicAPI, + mockVisualizationMap, + mockDatasourceMap, + mockStoreDeps, + MountStoreProps, +} from '../../../mocks'; import { Visualization } from '../../../types'; import { LayerPanels } from './config_panel'; import { LayerPanel } from './layer_panel'; @@ -16,6 +22,7 @@ import { generateId } from '../../../id_generator'; import { mountWithProvider } from '../../../mocks'; import { layerTypes } from '../../../../common'; import { ReactWrapper } from 'enzyme'; +import { addLayer } from '../../../state_management'; jest.mock('../../../id_generator'); @@ -39,8 +46,40 @@ afterEach(() => { describe('ConfigPanel', () => { const frame = createMockFramePublicAPI(); - - function getDefaultProps() { + function prepareAndMountComponent( + props: ReturnType, + customStoreProps?: Partial + ) { + (generateId as jest.Mock).mockReturnValue(`newId`); + return mountWithProvider( + , + { + preloadedState: { + datasourceStates: { + testDatasource: { + isLoading: false, + state: 'state', + }, + }, + activeDatasourceId: 'testDatasource', + }, + storeDeps: mockStoreDeps({ + datasourceMap: props.datasourceMap, + visualizationMap: props.visualizationMap, + }), + ...customStoreProps, + }, + { + attachTo: container, + } + ); + } + function getDefaultProps( + { datasourceMap = mockDatasourceMap(), visualizationMap = mockVisualizationMap() } = { + datasourceMap: mockDatasourceMap(), + visualizationMap: mockVisualizationMap(), + } + ) { frame.datasourceLayers = { first: datasourceMap.testDatasource.publicAPIMock, }; @@ -75,22 +114,13 @@ describe('ConfigPanel', () => { it('should fail to render layerPanels if the public API is out of date', async () => { const props = getDefaultProps(); props.framePublicAPI.datasourceLayers = {}; - const { instance } = await mountWithProvider(); + const { instance } = await prepareAndMountComponent(props); expect(instance.find(LayerPanel).exists()).toBe(false); }); it('allow datasources and visualizations to use setters', async () => { const props = getDefaultProps(); - const { instance, lensStore } = await mountWithProvider(, { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - }, - }); + const { instance, lensStore } = await prepareAndMountComponent(props); const { updateDatasource, updateAll } = instance.find(LayerPanel).props(); const updater = () => 'updated'; @@ -116,22 +146,7 @@ describe('ConfigPanel', () => { describe('focus behavior when adding or removing layers', () => { it('should focus the only layer when resetting the layer', async () => { - const { instance } = await mountWithProvider( - , - { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - }, - }, - { - attachTo: container, - } - ); + const { instance } = await prepareAndMountComponent(getDefaultProps()); const firstLayerFocusable = instance .find(LayerPanel) .first() @@ -146,29 +161,15 @@ describe('ConfigPanel', () => { }); it('should focus the second layer when removing the first layer', async () => { - const defaultProps = getDefaultProps(); + const datasourceMap = mockDatasourceMap(); + const defaultProps = getDefaultProps({ datasourceMap }); // overwriting datasourceLayers to test two layers frame.datasourceLayers = { first: datasourceMap.testDatasource.publicAPIMock, second: datasourceMap.testDatasource.publicAPIMock, }; - const { instance } = await mountWithProvider( - , - { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - }, - }, - { - attachTo: container, - } - ); + const { instance } = await prepareAndMountComponent(defaultProps); const secondLayerFocusable = instance .find(LayerPanel) .at(1) @@ -183,28 +184,14 @@ describe('ConfigPanel', () => { }); it('should focus the first layer when removing the second layer', async () => { - const defaultProps = getDefaultProps(); + const datasourceMap = mockDatasourceMap(); + const defaultProps = getDefaultProps({ datasourceMap }); // overwriting datasourceLayers to test two layers frame.datasourceLayers = { first: datasourceMap.testDatasource.publicAPIMock, second: datasourceMap.testDatasource.publicAPIMock, }; - const { instance } = await mountWithProvider( - , - { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - }, - }, - { - attachTo: container, - } - ); + const { instance } = await prepareAndMountComponent(defaultProps); const firstLayerFocusable = instance .find(LayerPanel) .first() @@ -219,31 +206,22 @@ describe('ConfigPanel', () => { }); it('should focus the added layer', async () => { - (generateId as jest.Mock).mockReturnValue(`second`); + const datasourceMap = mockDatasourceMap(); + frame.datasourceLayers = { + first: datasourceMap.testDatasource.publicAPIMock, + newId: datasourceMap.testDatasource.publicAPIMock, + }; - const { instance } = await mountWithProvider( - , + const defaultProps = getDefaultProps({ datasourceMap }); + + const { instance } = await prepareAndMountComponent(defaultProps, { + dispatch: jest.fn((x) => { + if (x.type === addLayer.type) { + frame.datasourceLayers.newId = datasourceMap.testDatasource.publicAPIMock; + } + }), + }); - { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - activeDatasourceId: 'testDatasource', - }, - dispatch: jest.fn((x) => { - if (x.payload.subType === 'ADD_LAYER') { - frame.datasourceLayers.second = datasourceMap.testDatasource.publicAPIMock; - } - }), - }, - { - attachTo: container, - } - ); act(() => { instance.find('[data-test-subj="lnsLayerAddButton"]').first().simulate('click'); }); @@ -253,26 +231,6 @@ describe('ConfigPanel', () => { }); describe('initial default value', () => { - function prepareAndMountComponent(props: ReturnType) { - (generateId as jest.Mock).mockReturnValue(`newId`); - return mountWithProvider( - , - { - preloadedState: { - datasourceStates: { - testDatasource: { - isLoading: false, - state: 'state', - }, - }, - activeDatasourceId: 'testDatasource', - }, - }, - { - attachTo: container, - } - ); - } function clickToAddLayer(instance: ReactWrapper) { act(() => { instance.find('[data-test-subj="lnsLayerAddButton"]').first().simulate('click'); @@ -297,8 +255,10 @@ describe('ConfigPanel', () => { } it('should not add an initial dimension when not specified', async () => { - const props = getDefaultProps(); - props.activeVisualization.getSupportedLayers = jest.fn(() => [ + const datasourceMap = mockDatasourceMap(); + const visualizationMap = mockVisualizationMap(); + + visualizationMap.testVis.getSupportedLayers = jest.fn(() => [ { type: layerTypes.DATA, label: 'Data Layer' }, { type: layerTypes.REFERENCELINE, @@ -306,6 +266,7 @@ describe('ConfigPanel', () => { }, ]); datasourceMap.testDatasource.initializeDimension = jest.fn(); + const props = getDefaultProps({ datasourceMap, visualizationMap }); const { instance, lensStore } = await prepareAndMountComponent(props); await clickToAddLayer(instance); @@ -315,8 +276,11 @@ describe('ConfigPanel', () => { }); it('should not add an initial dimension when initialDimensions are not available for the given layer type', async () => { - const props = getDefaultProps(); - props.activeVisualization.getSupportedLayers = jest.fn(() => [ + const datasourceMap = mockDatasourceMap(); + const visualizationMap = mockVisualizationMap(); + datasourceMap.testDatasource.initializeDimension = jest.fn(); + + visualizationMap.testVis.getSupportedLayers = jest.fn(() => [ { type: layerTypes.DATA, label: 'Data Layer', @@ -335,8 +299,7 @@ describe('ConfigPanel', () => { label: 'Reference layer', }, ]); - datasourceMap.testDatasource.initializeDimension = jest.fn(); - + const props = getDefaultProps({ datasourceMap, visualizationMap }); const { instance, lensStore } = await prepareAndMountComponent(props); await clickToAddLayer(instance); @@ -345,8 +308,9 @@ describe('ConfigPanel', () => { }); it('should use group initial dimension value when adding a new layer if available', async () => { - const props = getDefaultProps(); - props.activeVisualization.getSupportedLayers = jest.fn(() => [ + const datasourceMap = mockDatasourceMap(); + const visualizationMap = mockVisualizationMap(); + visualizationMap.testVis.getSupportedLayers = jest.fn(() => [ { type: layerTypes.DATA, label: 'Data Layer' }, { type: layerTypes.REFERENCELINE, @@ -363,6 +327,7 @@ describe('ConfigPanel', () => { }, ]); datasourceMap.testDatasource.initializeDimension = jest.fn(); + const props = getDefaultProps({ datasourceMap, visualizationMap }); const { instance, lensStore } = await prepareAndMountComponent(props); await clickToAddLayer(instance); @@ -378,8 +343,10 @@ describe('ConfigPanel', () => { }); it('should add an initial dimension value when clicking on the empty dimension button', async () => { - const props = getDefaultProps(); - props.activeVisualization.getSupportedLayers = jest.fn(() => [ + const datasourceMap = mockDatasourceMap(); + + const visualizationMap = mockVisualizationMap(); + visualizationMap.testVis.getSupportedLayers = jest.fn(() => [ { type: layerTypes.DATA, label: 'Data Layer', @@ -395,7 +362,7 @@ describe('ConfigPanel', () => { }, ]); datasourceMap.testDatasource.initializeDimension = jest.fn(); - + const props = getDefaultProps({ visualizationMap, datasourceMap }); const { instance, lensStore } = await prepareAndMountComponent(props); await clickToAddDimension(instance); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx index 0b6223ac87ce2..d3574abe4f57a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx @@ -7,26 +7,26 @@ import React, { useMemo, memo } from 'react'; import { EuiForm } from '@elastic/eui'; -import { mapValues } from 'lodash'; import { Visualization } from '../../../types'; import { LayerPanel } from './layer_panel'; import { trackUiEvent } from '../../../lens_ui_telemetry'; import { generateId } from '../../../id_generator'; -import { appendLayer } from './layer_actions'; import { ConfigPanelWrapperProps } from './types'; import { useFocusUpdate } from './use_focus_update'; import { + setLayerDefaultDimension, useLensDispatch, + removeOrClearLayer, + addLayer, updateState, updateDatasourceState, updateVisualizationState, setToggleFullscreen, useLensSelector, selectVisualization, - VisualizationState, - LensAppState, } from '../../../state_management'; -import { AddLayerButton, getLayerType } from './add_layer'; +import { AddLayerButton } from './add_layer'; +import { getRemoveOperation } from '../../../utils'; export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: ConfigPanelWrapperProps) { const visualization = useLensSelector(selectVisualization); @@ -39,18 +39,6 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config ) : null; }); -function getRemoveOperation( - activeVisualization: Visualization, - visualizationState: VisualizationState['state'], - layerId: string, - layerCount: number -) { - if (activeVisualization.getRemoveOperation) { - return activeVisualization.getRemoveOperation(visualizationState, layerId); - } - // fallback to generic count check - return layerCount === 1 ? 'clear' : 'remove'; -} export function LayerPanels( props: ConfigPanelWrapperProps & { activeVisualization: Visualization; @@ -73,8 +61,7 @@ export function LayerPanels( dispatchLens( updateVisualizationState({ visualizationId: activeVisualization.id, - updater: newState, - clearStagedPreview: false, + newState, }) ); }, @@ -110,7 +97,6 @@ export function LayerPanels( setTimeout(() => { dispatchLens( updateState({ - subType: 'UPDATE_ALL_STATES', updater: (prevState) => { const updatedDatasourceState = typeof newDatasourceState === 'function' @@ -133,7 +119,6 @@ export function LayerPanels( ...prevState.visualization, state: updatedVisualizationState, }, - stagedPreview: undefined, }; }, }) @@ -183,66 +168,22 @@ export function LayerPanels( datasourceMap[activeDatasourceId]?.initializeDimension ) { dispatchLens( - updateState({ - subType: 'LAYER_DEFAULT_DIMENSION', - updater: (state) => - addInitialValueIfAvailable({ - ...props, - state, - activeDatasourceId, - layerId, - layerType: getLayerType( - activeVisualization, - state.visualization.state, - layerId - ), - columnId, - groupId, - }), + setLayerDefaultDimension({ + layerId, + columnId, + groupId, }) ); } }} onRemoveLayer={() => { dispatchLens( - updateState({ - subType: 'REMOVE_OR_CLEAR_LAYER', - updater: (state) => { - const isOnlyLayer = - getRemoveOperation( - activeVisualization, - state.visualization.state, - layerId, - layerIds.length - ) === 'clear'; - - return { - ...state, - datasourceStates: mapValues( - state.datasourceStates, - (datasourceState, datasourceId) => { - const datasource = datasourceMap[datasourceId!]; - return { - ...datasourceState, - state: isOnlyLayer - ? datasource.clearLayer(datasourceState.state, layerId) - : datasource.removeLayer(datasourceState.state, layerId), - }; - } - ), - visualization: { - ...state.visualization, - state: - isOnlyLayer || !activeVisualization.removeLayer - ? activeVisualization.clearLayer(state.visualization.state, layerId) - : activeVisualization.removeLayer(state.visualization.state, layerId), - }, - stagedPreview: undefined, - }; - }, + removeOrClearLayer({ + visualizationId: activeVisualization.id, + layerId, + layerIds, }) ); - removeLayerRef(layerId); }} toggleFullscreen={toggleFullscreen} @@ -254,96 +195,12 @@ export function LayerPanels( visualizationState={visualization.state} layersMeta={props.framePublicAPI} onAddLayerClick={(layerType) => { - const id = generateId(); - dispatchLens( - updateState({ - subType: 'ADD_LAYER', - updater: (state) => { - const newState = appendLayer({ - activeVisualization, - generateId: () => id, - trackUiEvent, - activeDatasource: datasourceMap[activeDatasourceId!], - state, - layerType, - }); - return addInitialValueIfAvailable({ - ...props, - activeDatasourceId: activeDatasourceId!, - state: newState, - layerId: id, - layerType, - }); - }, - }) - ); - setNextFocusedLayerId(id); + const layerId = generateId(); + dispatchLens(addLayer({ layerId, layerType })); + trackUiEvent('layer_added'); + setNextFocusedLayerId(layerId); }} /> ); } - -function addInitialValueIfAvailable({ - state, - activeVisualization, - framePublicAPI, - layerType, - activeDatasourceId, - datasourceMap, - layerId, - columnId, - groupId, -}: ConfigPanelWrapperProps & { - state: LensAppState; - activeDatasourceId: string; - activeVisualization: Visualization; - layerId: string; - layerType: string; - columnId?: string; - groupId?: string; -}) { - const layerInfo = activeVisualization - .getSupportedLayers(state.visualization.state, framePublicAPI) - .find(({ type }) => type === layerType); - - const activeDatasource = datasourceMap[activeDatasourceId]; - - if (layerInfo?.initialDimensions && activeDatasource?.initializeDimension) { - const info = groupId - ? layerInfo.initialDimensions.find(({ groupId: id }) => id === groupId) - : // pick the first available one if not passed - layerInfo.initialDimensions[0]; - - if (info) { - return { - ...state, - datasourceStates: { - ...state.datasourceStates, - [activeDatasourceId]: { - ...state.datasourceStates[activeDatasourceId], - state: activeDatasource.initializeDimension( - state.datasourceStates[activeDatasourceId].state, - layerId, - { - ...info, - columnId: columnId || info.columnId, - } - ), - }, - }, - visualization: { - ...state.visualization, - state: activeVisualization.setDimension({ - groupId: info.groupId, - layerId, - columnId: columnId || info.columnId, - prevState: state.visualization.state, - frame: framePublicAPI, - }), - }, - }; - } - } - return state; -} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.test.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.test.ts deleted file mode 100644 index 44cefb0bf8ec4..0000000000000 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.test.ts +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ -import { layerTypes } from '../../../../common'; -import { LensAppState } from '../../../state_management/types'; -import { removeLayer, appendLayer } from './layer_actions'; - -function createTestArgs(initialLayerIds: string[]) { - const trackUiEvent = jest.fn(); - const testDatasource = (datasourceId: string) => ({ - id: datasourceId, - clearLayer: (layerIds: unknown, layerId: string) => - (layerIds as string[]).map((id: string) => - id === layerId ? `${datasourceId}_clear_${layerId}` : id - ), - removeLayer: (layerIds: unknown, layerId: string) => - (layerIds as string[]).filter((id: string) => id !== layerId), - insertLayer: (layerIds: unknown, layerId: string) => [...(layerIds as string[]), layerId], - }); - - const activeVisualization = { - clearLayer: (layerIds: unknown, layerId: string) => - (layerIds as string[]).map((id: string) => (id === layerId ? `vis_clear_${layerId}` : id)), - removeLayer: (layerIds: unknown, layerId: string) => - (layerIds as string[]).filter((id: string) => id !== layerId), - getLayerIds: (layerIds: unknown) => layerIds as string[], - appendLayer: (layerIds: unknown, layerId: string) => [...(layerIds as string[]), layerId], - }; - - const datasourceStates = { - ds1: { - isLoading: false, - state: initialLayerIds.slice(0, 1), - }, - ds2: { - isLoading: false, - state: initialLayerIds.slice(1), - }, - }; - - return { - state: { - activeDatasourceId: 'ds1', - datasourceStates, - title: 'foo', - visualization: { - activeId: 'testVis', - state: initialLayerIds, - }, - } as unknown as LensAppState, - activeVisualization, - datasourceMap: { - ds1: testDatasource('ds1'), - ds2: testDatasource('ds2'), - }, - trackUiEvent, - stagedPreview: { - visualization: { - activeId: 'testVis', - state: initialLayerIds, - }, - datasourceStates, - }, - }; -} - -describe('removeLayer', () => { - it('should clear the layer if it is the only layer', () => { - const { state, trackUiEvent, datasourceMap, activeVisualization } = createTestArgs(['layer1']); - const newState = removeLayer({ - activeVisualization, - datasourceMap, - layerId: 'layer1', - state, - trackUiEvent, - }); - - expect(newState.visualization.state).toEqual(['vis_clear_layer1']); - expect(newState.datasourceStates.ds1.state).toEqual(['ds1_clear_layer1']); - expect(newState.datasourceStates.ds2.state).toEqual([]); - expect(newState.stagedPreview).not.toBeDefined(); - expect(trackUiEvent).toHaveBeenCalledWith('layer_cleared'); - }); - - it('should remove the layer if it is not the only layer', () => { - const { state, trackUiEvent, datasourceMap, activeVisualization } = createTestArgs([ - 'layer1', - 'layer2', - ]); - const newState = removeLayer({ - activeVisualization, - datasourceMap, - layerId: 'layer1', - state, - trackUiEvent, - }); - - expect(newState.visualization.state).toEqual(['layer2']); - expect(newState.datasourceStates.ds1.state).toEqual([]); - expect(newState.datasourceStates.ds2.state).toEqual(['layer2']); - expect(newState.stagedPreview).not.toBeDefined(); - expect(trackUiEvent).toHaveBeenCalledWith('layer_removed'); - }); -}); - -describe('appendLayer', () => { - it('should add the layer to the datasource and visualization', () => { - const { state, trackUiEvent, datasourceMap, activeVisualization } = createTestArgs([ - 'layer1', - 'layer2', - ]); - const newState = appendLayer({ - activeDatasource: datasourceMap.ds1, - activeVisualization, - generateId: () => 'foo', - state, - trackUiEvent, - layerType: layerTypes.DATA, - }); - - expect(newState.visualization.state).toEqual(['layer1', 'layer2', 'foo']); - expect(newState.datasourceStates.ds1.state).toEqual(['layer1', 'foo']); - expect(newState.datasourceStates.ds2.state).toEqual(['layer2']); - expect(newState.stagedPreview).not.toBeDefined(); - expect(trackUiEvent).toHaveBeenCalledWith('layer_added'); - }); -}); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.ts deleted file mode 100644 index c0f0847e8ff5c..0000000000000 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions.ts +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { mapValues } from 'lodash'; -import type { LayerType } from '../../../../common'; -import { LensAppState } from '../../../state_management'; - -import { Datasource, Visualization } from '../../../types'; - -interface RemoveLayerOptions { - trackUiEvent: (name: string) => void; - state: LensAppState; - layerId: string; - activeVisualization: Pick; - datasourceMap: Record>; -} - -interface AppendLayerOptions { - trackUiEvent: (name: string) => void; - state: LensAppState; - generateId: () => string; - activeDatasource: Pick; - activeVisualization: Pick; - layerType: LayerType; -} - -export function removeLayer(opts: RemoveLayerOptions): LensAppState { - const { state, trackUiEvent: trackUiEvent, activeVisualization, layerId, datasourceMap } = opts; - const isOnlyLayer = activeVisualization - .getLayerIds(state.visualization.state) - .every((id) => id === opts.layerId); - - trackUiEvent(isOnlyLayer ? 'layer_cleared' : 'layer_removed'); - - return { - ...state, - datasourceStates: mapValues(state.datasourceStates, (datasourceState, datasourceId) => { - const datasource = datasourceMap[datasourceId!]; - return { - ...datasourceState, - state: isOnlyLayer - ? datasource.clearLayer(datasourceState.state, layerId) - : datasource.removeLayer(datasourceState.state, layerId), - }; - }), - visualization: { - ...state.visualization, - state: - isOnlyLayer || !activeVisualization.removeLayer - ? activeVisualization.clearLayer(state.visualization.state, layerId) - : activeVisualization.removeLayer(state.visualization.state, layerId), - }, - stagedPreview: undefined, - }; -} - -export function appendLayer({ - trackUiEvent, - activeVisualization, - state, - generateId, - activeDatasource, - layerType, -}: AppendLayerOptions): LensAppState { - trackUiEvent('layer_added'); - - if (!activeVisualization.appendLayer) { - return state; - } - - const layerId = generateId(); - - return { - ...state, - datasourceStates: { - ...state.datasourceStates, - [activeDatasource.id]: { - ...state.datasourceStates[activeDatasource.id], - state: activeDatasource.insertLayer( - state.datasourceStates[activeDatasource.id].state, - layerId - ), - }, - }, - visualization: { - ...state.visualization, - state: activeVisualization.appendLayer(state.visualization.state, layerId, layerType), - }, - stagedPreview: undefined, - }; -} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx index d289b69f4105e..37191ffa89fdc 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx @@ -38,6 +38,7 @@ import { createMockDatasource, DatasourceMock, createExpressionRendererMock, + mockStoreDeps, } from '../../mocks'; import { inspectorPluginMock } from 'src/plugins/inspector/public/mocks'; import { ReactExpressionRendererType } from 'src/plugins/expressions/public'; @@ -144,21 +145,19 @@ describe('editor_frame', () => { ExpressionRenderer: expressionRendererMock, }; - const lensStore = ( - await mountWithProvider(, { - preloadedState: { - activeDatasourceId: 'testDatasource', - datasourceStates: { - testDatasource: { - isLoading: true, - state: { - internalState1: '', - }, + const { lensStore } = await mountWithProvider(, { + preloadedState: { + activeDatasourceId: 'testDatasource', + datasourceStates: { + testDatasource: { + isLoading: true, + state: { + internalState1: '', }, }, }, - }) - ).lensStore; + }, + }); expect(mockDatasource.renderDataPanel).not.toHaveBeenCalled(); lensStore.dispatch( setState({ @@ -553,6 +552,7 @@ describe('editor_frame', () => { } beforeEach(async () => { + mockVisualization2.initialize.mockReturnValue({ initial: true }); mockDatasource.getLayers.mockReturnValue(['first', 'second']); mockDatasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ { @@ -567,20 +567,27 @@ describe('editor_frame', () => { }, ]); + const visualizationMap = { + testVis: mockVisualization, + testVis2: mockVisualization2, + }; + + const datasourceMap = { + testDatasource: mockDatasource, + testDatasource2: mockDatasource2, + }; + const props = { ...getDefaultProps(), - visualizationMap: { - testVis: mockVisualization, - testVis2: mockVisualization2, - }, - datasourceMap: { - testDatasource: mockDatasource, - testDatasource2: mockDatasource2, - }, - + visualizationMap, + datasourceMap, ExpressionRenderer: expressionRendererMock, }; - instance = (await mountWithProvider()).instance; + instance = ( + await mountWithProvider(, { + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), + }) + ).instance; // necessary to flush elements to dom synchronously instance.update(); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx index 7cb97882a5e03..c325e6d516c8b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.test.tsx @@ -9,8 +9,9 @@ import React from 'react'; import { ReactWrapper } from 'enzyme'; import { createMockVisualization, + mockStoreDeps, createMockFramePublicAPI, - createMockDatasource, + mockDatasourceMap, mockDatasourceStates, } from '../../../mocks'; import { mountWithProvider } from '../../../mocks'; @@ -71,7 +72,7 @@ describe('chart_switch', () => { * - Allows a switch to subvisC3 * - Allows a switch to subvisC1 */ - function mockVisualizations() { + function mockVisualizationMap() { return { visA: generateVisualization('visA'), visB: generateVisualization('visB'), @@ -143,27 +144,6 @@ describe('chart_switch', () => { } as FramePublicAPI; } - function mockDatasourceMap() { - const datasource = createMockDatasource('testDatasource'); - datasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ - { - state: {}, - table: { - columns: [], - isMultiRow: true, - layerId: 'a', - changeType: 'unchanged', - }, - keptLayerIds: ['a'], - }, - ]); - - datasource.getLayers.mockReturnValue(['a']); - return { - testDatasource: datasource, - }; - } - function showFlyout(instance: ReactWrapper) { instance.find('[data-test-subj="lnsChartSwitchPopover"]').first().simulate('click'); } @@ -178,10 +158,10 @@ describe('chart_switch', () => { return instance.find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`).first(); } it('should use suggested state if there is a suggestion from the target visualization', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const { instance, lensStore } = await mountWithProvider( , @@ -212,19 +192,20 @@ describe('chart_switch', () => { }); it('should use initial state if there is no suggestion from the target visualization', async () => { - const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visB.getSuggestions.mockReturnValueOnce([]); const frame = mockFrame(['a']); (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); const datasourceMap = mockDatasourceMap(); const datasourceStates = mockDatasourceStates(); const { instance, lensStore } = await mountWithProvider( , { + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), preloadedState: { datasourceStates, activeDatasourceId: 'testDatasource', @@ -249,18 +230,14 @@ describe('chart_switch', () => { }, }); expect(lensStore.dispatch).toHaveBeenCalledWith({ - type: 'lens/updateLayer', - payload: expect.objectContaining({ - datasourceId: 'testDatasource', - layerId: 'a', - }), + type: 'lens/removeLayers', + payload: { layerIds: ['a'], visualizationId: 'visA' }, }); }); it('should indicate data loss if not all columns will be used', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const frame = mockFrame(['a']); - const datasourceMap = mockDatasourceMap(); datasourceMap.testDatasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ { @@ -299,9 +276,9 @@ describe('chart_switch', () => { const { instance } = await mountWithProvider( , { preloadedState: { @@ -322,12 +299,11 @@ describe('chart_switch', () => { }); it('should indicate data loss if not all layers will be used', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const frame = mockFrame(['a', 'b']); - const { instance } = await mountWithProvider( , @@ -350,9 +326,8 @@ describe('chart_switch', () => { }); it('should support multi-layer suggestions without data loss', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const frame = mockFrame(['a', 'b']); - const datasourceMap = mockDatasourceMap(); datasourceMap.testDatasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ { @@ -378,7 +353,7 @@ describe('chart_switch', () => { const { instance } = await mountWithProvider( , @@ -398,13 +373,13 @@ describe('chart_switch', () => { }); it('should indicate data loss if no data will be used', async () => { - const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visB.getSuggestions.mockReturnValueOnce([]); const frame = mockFrame(['a']); const { instance } = await mountWithProvider( , @@ -427,14 +402,14 @@ describe('chart_switch', () => { }); it('should not indicate data loss if there is no data', async () => { - const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visB.getSuggestions.mockReturnValueOnce([]); const frame = mockFrame(['a']); (frame.datasourceLayers.a.getTableSpec as jest.Mock).mockReturnValue([]); const { instance } = await mountWithProvider( , @@ -456,20 +431,19 @@ describe('chart_switch', () => { it('should not show a warning when the subvisualization is the same', async () => { const frame = mockFrame(['a', 'b', 'c']); - const visualizations = mockVisualizations(); - visualizations.visC.getVisualizationTypeId.mockReturnValue('subvisC2'); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visC.getVisualizationTypeId.mockReturnValue('subvisC2'); const switchVisualizationType = jest.fn(() => ({ type: 'subvisC1' })); - visualizations.visC.switchVisualizationType = switchVisualizationType; + visualizationMap.visC.switchVisualizationType = switchVisualizationType; - const datasourceMap = mockDatasourceMap(); const datasourceStates = mockDatasourceStates(); const { instance } = await mountWithProvider( , { preloadedState: { @@ -491,8 +465,8 @@ describe('chart_switch', () => { }); it('should get suggestions when switching subvisualization', async () => { - const visualizations = mockVisualizations(); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visB.getSuggestions.mockReturnValueOnce([]); const frame = mockFrame(['a', 'b', 'c']); const datasourceMap = mockDatasourceMap(); datasourceMap.testDatasource.getLayers.mockReturnValue(['a', 'b', 'c']); @@ -500,11 +474,12 @@ describe('chart_switch', () => { const { instance, lensStore } = await mountWithProvider( , { + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), preloadedState: { datasourceStates, visualization: { @@ -519,7 +494,7 @@ describe('chart_switch', () => { expect(datasourceMap.testDatasource.removeLayer).toHaveBeenCalledWith({}, 'a'); expect(datasourceMap.testDatasource.removeLayer).toHaveBeenCalledWith(undefined, 'b'); expect(datasourceMap.testDatasource.removeLayer).toHaveBeenCalledWith(undefined, 'c'); - expect(visualizations.visB.getSuggestions).toHaveBeenCalledWith( + expect(visualizationMap.visB.getSuggestions).toHaveBeenCalledWith( expect.objectContaining({ keptLayerIds: ['a'], }) @@ -540,19 +515,18 @@ describe('chart_switch', () => { }); it('should query main palette from active chart and pass into suggestions', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const mockPalette: PaletteOutput = { type: 'palette', name: 'mock' }; - visualizations.visA.getMainPalette = jest.fn(() => mockPalette); - visualizations.visB.getSuggestions.mockReturnValueOnce([]); + visualizationMap.visA.getMainPalette = jest.fn(() => mockPalette); + visualizationMap.visB.getSuggestions.mockReturnValueOnce([]); const frame = mockFrame(['a', 'b', 'c']); const currentVisState = {}; - const datasourceMap = mockDatasourceMap(); datasourceMap.testDatasource.getLayers.mockReturnValue(['a', 'b', 'c']); const { instance } = await mountWithProvider( , @@ -563,14 +537,15 @@ describe('chart_switch', () => { state: currentVisState, }, }, + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), } ); switchTo('visB', instance); - expect(visualizations.visA.getMainPalette).toHaveBeenCalledWith(currentVisState); + expect(visualizationMap.visA.getMainPalette).toHaveBeenCalledWith(currentVisState); - expect(visualizations.visB.getSuggestions).toHaveBeenCalledWith( + expect(visualizationMap.visB.getSuggestions).toHaveBeenCalledWith( expect.objectContaining({ keptLayerIds: ['a'], mainPalette: mockPalette, @@ -580,14 +555,14 @@ describe('chart_switch', () => { it('should not remove layers when switching between subtypes', async () => { const frame = mockFrame(['a', 'b', 'c']); - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const switchVisualizationType = jest.fn(() => 'switched'); - visualizations.visC.switchVisualizationType = switchVisualizationType; + visualizationMap.visC.switchVisualizationType = switchVisualizationType; const datasourceMap = mockDatasourceMap(); const { instance, lensStore } = await mountWithProvider( , @@ -598,6 +573,7 @@ describe('chart_switch', () => { state: { type: 'subvisC1' }, }, }, + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), } ); @@ -622,13 +598,13 @@ describe('chart_switch', () => { it('should not remove layers and initialize with existing state when switching between subtypes without data', async () => { const frame = mockFrame(['a']); frame.datasourceLayers.a.getTableSpec = jest.fn().mockReturnValue([]); - const visualizations = mockVisualizations(); - visualizations.visC.getSuggestions = jest.fn().mockReturnValue([]); - visualizations.visC.switchVisualizationType = jest.fn(() => 'switched'); + const visualizationMap = mockVisualizationMap(); + visualizationMap.visC.getSuggestions = jest.fn().mockReturnValue([]); + visualizationMap.visC.switchVisualizationType = jest.fn(() => 'switched'); const datasourceMap = mockDatasourceMap(); const { instance } = await mountWithProvider( , @@ -639,21 +615,21 @@ describe('chart_switch', () => { state: { type: 'subvisC1' }, }, }, + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), } ); switchTo('subvisC3', instance); - expect(visualizations.visC.switchVisualizationType).toHaveBeenCalledWith('subvisC3', { + expect(visualizationMap.visC.switchVisualizationType).toHaveBeenCalledWith('subvisC3', { type: 'subvisC1', }); expect(datasourceMap.testDatasource.removeLayer).not.toHaveBeenCalled(); }); it('should switch to the updated datasource state', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const frame = mockFrame(['a', 'b']); - const datasourceMap = mockDatasourceMap(); datasourceMap.testDatasource.getDatasourceSuggestionsFromCurrentState.mockReturnValue([ { @@ -684,14 +660,14 @@ describe('chart_switch', () => { keptLayerIds: [], }, ]); - const { instance, lensStore } = await mountWithProvider( , { + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), preloadedState: { visualization: { activeId: 'visA', @@ -718,20 +694,21 @@ describe('chart_switch', () => { }); it('should ensure the new visualization has the proper subtype', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const switchVisualizationType = jest.fn( (visualizationType, state) => `${state} ${visualizationType}` ); - visualizations.visB.switchVisualizationType = switchVisualizationType; - + visualizationMap.visB.switchVisualizationType = switchVisualizationType; + const datasourceMap = mockDatasourceMap(); const { instance, lensStore } = await mountWithProvider( , { + storeDeps: mockStoreDeps({ datasourceMap, visualizationMap }), preloadedState: { visualization: { activeId: 'visA', @@ -758,16 +735,16 @@ describe('chart_switch', () => { }); it('should use the suggestion that matches the subtype', async () => { - const visualizations = mockVisualizations(); + const visualizationMap = mockVisualizationMap(); const switchVisualizationType = jest.fn(); - visualizations.visC.switchVisualizationType = switchVisualizationType; - + visualizationMap.visC.switchVisualizationType = switchVisualizationType; + const datasourceMap = mockDatasourceMap(); const { instance } = await mountWithProvider( , { preloadedState: { @@ -787,11 +764,12 @@ describe('chart_switch', () => { }); it('should show all visualization types', async () => { + const datasourceMap = mockDatasourceMap(); const { instance } = await mountWithProvider( , { preloadedState: { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx index a5ba12941cf7f..427306cb54fb9 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/chart_switch.tsx @@ -31,8 +31,8 @@ import { getSuggestions, switchToSuggestion, Suggestion } from '../suggestion_he import { trackUiEvent } from '../../../lens_ui_telemetry'; import { ToolbarButton } from '../../../../../../../src/plugins/kibana_react/public'; import { - updateLayer, - updateVisualizationState, + insertLayer, + removeLayers, useLensDispatch, useLensSelector, VisualizationState, @@ -120,41 +120,6 @@ export const ChartSwitch = memo(function ChartSwitch(props: Props) { const visualization = useLensSelector(selectVisualization); const datasourceStates = useLensSelector(selectDatasourceStates); - function removeLayers(layerIds: string[]) { - const activeVisualization = - visualization.activeId && props.visualizationMap[visualization.activeId]; - if (activeVisualization && activeVisualization.removeLayer && visualization.state) { - dispatchLens( - updateVisualizationState({ - visualizationId: activeVisualization.id, - updater: layerIds.reduce( - (acc, layerId) => - activeVisualization.removeLayer ? activeVisualization.removeLayer(acc, layerId) : acc, - visualization.state - ), - }) - ); - } - layerIds.forEach((layerId) => { - const [layerDatasourceId] = - Object.entries(props.datasourceMap).find(([datasourceId, datasource]) => { - return ( - datasourceStates[datasourceId] && - datasource.getLayers(datasourceStates[datasourceId].state).includes(layerId) - ); - }) ?? []; - if (layerDatasourceId) { - dispatchLens( - updateLayer({ - layerId, - datasourceId: layerDatasourceId, - updater: props.datasourceMap[layerDatasourceId].removeLayer, - }) - ); - } - }); - } - const commitSelection = (selection: VisualizationSelection) => { setFlyoutOpen(false); @@ -173,7 +138,12 @@ export const ChartSwitch = memo(function ChartSwitch(props: Props) { (!selection.datasourceId && !selection.sameDatasources) || selection.dataLoss === 'everything' ) { - removeLayers(Object.keys(props.framePublicAPI.datasourceLayers)); + dispatchLens( + removeLayers({ + visualizationId: visualization.activeId, + layerIds: Object.keys(props.framePublicAPI.datasourceLayers), + }) + ); } }; @@ -231,13 +201,11 @@ export const ChartSwitch = memo(function ChartSwitch(props: Props) { function addNewLayer() { const newLayerId = generateId(); dispatchLens( - updateLayer({ + insertLayer({ datasourceId: activeDatasourceId!, layerId: newLayerId, - updater: props.datasourceMap[activeDatasourceId!].insertLayer, }) ); - return newLayerId; } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index a03c9292d2da8..4b98b2842a01b 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -52,7 +52,7 @@ import { DefaultInspectorAdapters } from '../../../../../../../src/plugins/expre import { onActiveDataChange, useLensDispatch, - updateVisualizationState, + editVisualizationAction, updateDatasourceState, setSaveable, useLensSelector, @@ -246,9 +246,9 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ } if (isLensEditEvent(event) && activeVisualization?.onEditAction) { dispatchLens( - updateVisualizationState({ + editVisualizationAction({ visualizationId: activeVisualization.id, - updater: (oldState: unknown) => activeVisualization.onEditAction!(oldState, event), + event, }) ); } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx index d8959e714d16e..f13ecb78593d9 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx @@ -54,8 +54,7 @@ export function WorkspacePanelWrapper({ dispatchLens( updateVisualizationState({ visualizationId: activeVisualization.id, - updater: newState, - clearStagedPreview: false, + newState, }) ); }, diff --git a/x-pack/plugins/lens/public/state_management/index.ts b/x-pack/plugins/lens/public/state_management/index.ts index 0aa7185931c5a..5f3b60d95d77d 100644 --- a/x-pack/plugins/lens/public/state_management/index.ts +++ b/x-pack/plugins/lens/public/state_management/index.ts @@ -25,13 +25,18 @@ export const { updateState, updateDatasourceState, updateVisualizationState, - updateLayer, + insertLayer, switchVisualization, rollbackSuggestion, submitSuggestion, switchDatasource, setToggleFullscreen, initEmpty, + editVisualizationAction, + removeLayers, + removeOrClearLayer, + addLayer, + setLayerDefaultDimension, } = lensActions; export const makeConfigureStore = ( diff --git a/x-pack/plugins/lens/public/state_management/lens_slice.test.ts b/x-pack/plugins/lens/public/state_management/lens_slice.test.ts index 7d88e6ceb616c..85061f36ce35e 100644 --- a/x-pack/plugins/lens/public/state_management/lens_slice.test.ts +++ b/x-pack/plugins/lens/public/state_management/lens_slice.test.ts @@ -13,8 +13,13 @@ import { updateState, updateDatasourceState, updateVisualizationState, + removeOrClearLayer, + addLayer, + LensRootStore, } from '.'; -import { makeLensStore, defaultState } from '../mocks'; +import { layerTypes } from '../../common'; +import { makeLensStore, defaultState, mockStoreDeps } from '../mocks'; +import { DatasourceMap, VisualizationMap } from '../types'; describe('lensSlice', () => { const { store } = makeLensStore({}); @@ -31,7 +36,7 @@ describe('lensSlice', () => { it('updateState: updates state with updater', () => { const customUpdater = jest.fn((state) => ({ ...state, query: customQuery })); - store.dispatch(updateState({ subType: 'UPDATE', updater: customUpdater })); + store.dispatch(updateState({ updater: customUpdater })); const changedState = store.getState().lens; expect(changedState).toEqual({ ...defaultState, query: customQuery }); }); @@ -40,7 +45,7 @@ describe('lensSlice', () => { store.dispatch( updateVisualizationState({ visualizationId: 'testVis', - updater: newVisState, + newState: newVisState, }) ); @@ -117,7 +122,7 @@ describe('lensSlice', () => { expect(store.getState().lens.datasourceStates.testDatasource2.isLoading).toEqual(true); }); - it('not initialize already initialized datasource on switch', () => { + it('should not initialize already initialized datasource on switch', () => { const datasource2State = {}; const { store: customStore } = makeLensStore({ preloadedState: { @@ -146,5 +151,109 @@ describe('lensSlice', () => { datasource2State ); }); + + describe('adding or removing layer', () => { + const testDatasource = (datasourceId: string) => { + return { + id: datasourceId, + getPublicAPI: () => ({ + datasourceId: 'testDatasource', + getOperationForColumnId: jest.fn(), + getTableSpec: jest.fn(), + }), + getLayers: () => ['layer1'], + clearLayer: (layerIds: unknown, layerId: string) => + (layerIds as string[]).map((id: string) => + id === layerId ? `${datasourceId}_clear_${layerId}` : id + ), + removeLayer: (layerIds: unknown, layerId: string) => + (layerIds as string[]).filter((id: string) => id !== layerId), + insertLayer: (layerIds: unknown, layerId: string) => [...(layerIds as string[]), layerId], + }; + }; + const datasourceStates = { + testDatasource: { + isLoading: false, + state: ['layer1'], + }, + testDatasource2: { + isLoading: false, + state: ['layer2'], + }, + }; + const datasourceMap = { + testDatasource: testDatasource('testDatasource'), + testDatasource2: testDatasource('testDatasource2'), + }; + const visualizationMap = { + testVis: { + clearLayer: (layerIds: unknown, layerId: string) => + (layerIds as string[]).map((id: string) => + id === layerId ? `vis_clear_${layerId}` : id + ), + removeLayer: (layerIds: unknown, layerId: string) => + (layerIds as string[]).filter((id: string) => id !== layerId), + getLayerIds: (layerIds: unknown) => layerIds as string[], + appendLayer: (layerIds: unknown, layerId: string) => [...(layerIds as string[]), layerId], + getSupportedLayers: jest.fn(() => [{ type: layerTypes.DATA, label: 'Data Layer' }]), + }, + }; + + let customStore: LensRootStore; + beforeEach(() => { + customStore = makeLensStore({ + preloadedState: { + activeDatasourceId: 'testDatasource', + datasourceStates, + visualization: { + activeId: 'testVis', + state: ['layer1', 'layer2'], + }, + stagedPreview: { + visualization: { + activeId: 'testVis', + state: ['layer1', 'layer2'], + }, + datasourceStates, + }, + }, + storeDeps: mockStoreDeps({ + visualizationMap: visualizationMap as unknown as VisualizationMap, + datasourceMap: datasourceMap as unknown as DatasourceMap, + }), + }).store; + }); + + it('addLayer: should add the layer to the datasource and visualization', () => { + customStore.dispatch( + addLayer({ + layerId: 'foo', + layerType: layerTypes.DATA, + }) + ); + const state = customStore.getState().lens; + + expect(state.visualization.state).toEqual(['layer1', 'layer2', 'foo']); + expect(state.datasourceStates.testDatasource.state).toEqual(['layer1', 'foo']); + expect(state.datasourceStates.testDatasource2.state).toEqual(['layer2']); + expect(state.stagedPreview).not.toBeDefined(); + }); + + it('removeLayer: should remove the layer if it is not the only layer', () => { + customStore.dispatch( + removeOrClearLayer({ + visualizationId: 'testVis', + layerId: 'layer1', + layerIds: ['layer1', 'layer2'], + }) + ); + const state = customStore.getState().lens; + + expect(state.visualization.state).toEqual(['layer2']); + expect(state.datasourceStates.testDatasource.state).toEqual([]); + expect(state.datasourceStates.testDatasource2.state).toEqual(['layer2']); + expect(state.stagedPreview).not.toBeDefined(); + }); + }); }); }); diff --git a/x-pack/plugins/lens/public/state_management/lens_slice.ts b/x-pack/plugins/lens/public/state_management/lens_slice.ts index df178cadf6c30..af9897581fcf4 100644 --- a/x-pack/plugins/lens/public/state_management/lens_slice.ts +++ b/x-pack/plugins/lens/public/state_management/lens_slice.ts @@ -7,16 +7,22 @@ import { createAction, createReducer, current, PayloadAction } from '@reduxjs/toolkit'; import { VisualizeFieldContext } from 'src/plugins/ui_actions/public'; +import { mapValues } from 'lodash'; import { History } from 'history'; import { LensEmbeddableInput } from '..'; +import { getDatasourceLayers } from '../editor_frame_service/editor_frame'; import { TableInspectorAdapter } from '../editor_frame_service/types'; -import { getInitialDatasourceId, getResolvedDateRange } from '../utils'; -import { LensAppState, LensStoreDeps } from './types'; +import { getInitialDatasourceId, getResolvedDateRange, getRemoveOperation } from '../utils'; +import { LensAppState, LensStoreDeps, VisualizationState } from './types'; +import { Datasource, Visualization } from '../types'; import { generateId } from '../id_generator'; +import type { LayerType } from '../../common/types'; +import { getLayerType } from '../editor_frame_service/editor_frame/config_panel/add_layer'; import { getVisualizeFieldSuggestions, Suggestion, } from '../editor_frame_service/editor_frame/suggestion_helpers'; +import { FramePublicAPI, LensEditContextMapping, LensEditEvent } from '../types'; export const initialState: LensAppState = { persistedDoc: undefined, @@ -80,7 +86,6 @@ export const setState = createAction>('lens/setState'); export const onActiveDataChange = createAction('lens/onActiveDataChange'); export const setSaveable = createAction('lens/setSaveable'); export const updateState = createAction<{ - subType: string; updater: (prevState: LensAppState) => LensAppState; }>('lens/updateState'); export const updateDatasourceState = createAction<{ @@ -90,15 +95,13 @@ export const updateDatasourceState = createAction<{ }>('lens/updateDatasourceState'); export const updateVisualizationState = createAction<{ visualizationId: string; - updater: unknown; - clearStagedPreview?: boolean; + newState: unknown; }>('lens/updateVisualizationState'); -export const updateLayer = createAction<{ +export const insertLayer = createAction<{ layerId: string; datasourceId: string; - updater: (state: unknown, layerId: string) => unknown; -}>('lens/updateLayer'); +}>('lens/insertLayer'); export const switchVisualization = createAction<{ suggestion: { @@ -133,6 +136,29 @@ export const initEmpty = createAction( return { payload: { layerId: generateId(), newState, initialContext } }; } ); +export const editVisualizationAction = createAction<{ + visualizationId: string; + event: LensEditEvent; +}>('lens/editVisualizationAction'); +export const removeLayers = createAction<{ + visualizationId: VisualizationState['activeId']; + layerIds: string[]; +}>('lens/removeLayers'); +export const removeOrClearLayer = createAction<{ + visualizationId: string; + layerId: string; + layerIds: string[]; +}>('lens/removeOrClearLayer'); +export const addLayer = createAction<{ + layerId: string; + layerType: LayerType; +}>('lens/addLayer'); + +export const setLayerDefaultDimension = createAction<{ + layerId: string; + columnId: string; + groupId: string; +}>('lens/setLayerDefaultDimension'); export const lensActions = { setState, @@ -141,7 +167,7 @@ export const lensActions = { updateState, updateDatasourceState, updateVisualizationState, - updateLayer, + insertLayer, switchVisualization, rollbackSuggestion, setToggleFullscreen, @@ -150,6 +176,11 @@ export const lensActions = { navigateAway, loadInitial, initEmpty, + editVisualizationAction, + removeLayers, + removeOrClearLayer, + addLayer, + setLayerDefaultDimension, }; export const makeLensReducer = (storeDeps: LensStoreDeps) => { @@ -175,14 +206,58 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { }, [updateState.type]: ( state, - action: { + { + payload: { updater }, + }: { payload: { - subType: string; updater: (prevState: LensAppState) => LensAppState; }; } ) => { - return action.payload.updater(current(state) as LensAppState); + const newState = updater(current(state) as LensAppState); + return { + ...newState, + stagedPreview: undefined, + }; + }, + [removeOrClearLayer.type]: ( + state, + { + payload: { visualizationId, layerId, layerIds }, + }: { + payload: { + visualizationId: string; + layerId: string; + layerIds: string[]; + }; + } + ) => { + const activeVisualization = visualizationMap[visualizationId]; + const isOnlyLayer = + getRemoveOperation( + activeVisualization, + state.visualization.state, + layerId, + layerIds.length + ) === 'clear'; + + state.datasourceStates = mapValues( + state.datasourceStates, + (datasourceState, datasourceId) => { + const datasource = datasourceMap[datasourceId!]; + return { + ...datasourceState, + state: isOnlyLayer + ? datasource.clearLayer(datasourceState.state, layerId) + : datasource.removeLayer(datasourceState.state, layerId), + }; + } + ); + state.stagedPreview = undefined; + state.visualization.state = + isOnlyLayer || !activeVisualization.removeLayer + ? activeVisualization.clearLayer(state.visualization.state, layerId) + : activeVisualization.removeLayer(state.visualization.state, layerId); }, [updateDatasourceState.type]: ( state, @@ -218,8 +293,7 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { }: { payload: { visualizationId: string; - updater: unknown | ((state: unknown) => unknown); - clearStagedPreview?: boolean; + newState: unknown; }; } ) => { @@ -236,37 +310,7 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { ...state, visualization: { ...state.visualization, - state: - typeof payload.updater === 'function' - ? payload.updater(current(state.visualization.state)) - : payload.updater, - }, - stagedPreview: payload.clearStagedPreview ? undefined : state.stagedPreview, - }; - }, - [updateLayer.type]: ( - state, - { - payload, - }: { - payload: { - layerId: string; - datasourceId: string; - updater: (state: unknown, layerId: string) => unknown; - }; - } - ) => { - return { - ...state, - datasourceStates: { - ...state.datasourceStates, - [payload.datasourceId]: { - ...state.datasourceStates[payload.datasourceId], - state: payload.updater( - current(state).datasourceStates[payload.datasourceId].state, - payload.layerId - ), - }, + state: payload.newState, }, }; }, @@ -433,5 +477,248 @@ export const makeLensReducer = (storeDeps: LensStoreDeps) => { } return newState; }, + [editVisualizationAction.type]: ( + state, + { + payload, + }: { + payload: { + visualizationId: string; + event: LensEditEvent; + }; + } + ) => { + if (!state.visualization.activeId) { + throw new Error('Invariant: visualization state got updated without active visualization'); + } + // This is a safeguard that prevents us from accidentally updating the + // wrong visualization. This occurs in some cases due to the uncoordinated + // way we manage state across plugins. + if (state.visualization.activeId !== payload.visualizationId) { + return state; + } + const activeVisualization = visualizationMap[payload.visualizationId]; + if (activeVisualization?.onEditAction) { + state.visualization.state = activeVisualization.onEditAction( + state.visualization.state, + payload.event + ); + } + }, + [insertLayer.type]: ( + state, + { + payload, + }: { + payload: { + layerId: string; + datasourceId: string; + }; + } + ) => { + const updater = datasourceMap[payload.datasourceId].insertLayer; + return { + ...state, + datasourceStates: { + ...state.datasourceStates, + [payload.datasourceId]: { + ...state.datasourceStates[payload.datasourceId], + state: updater( + current(state).datasourceStates[payload.datasourceId].state, + payload.layerId + ), + }, + }, + }; + }, + [removeLayers.type]: ( + state, + { + payload: { visualizationId, layerIds }, + }: { + payload: { + visualizationId: VisualizationState['activeId']; + layerIds: string[]; + }; + } + ) => { + if (!state.visualization.activeId) { + throw new Error('Invariant: visualization state got updated without active visualization'); + } + + const activeVisualization = visualizationId && visualizationMap[visualizationId]; + + // This is a safeguard that prevents us from accidentally updating the + // wrong visualization. This occurs in some cases due to the uncoordinated + // way we manage state across plugins. + if ( + state.visualization.activeId === visualizationId && + activeVisualization && + activeVisualization.removeLayer && + state.visualization.state + ) { + const updater = layerIds.reduce( + (acc, layerId) => + activeVisualization.removeLayer ? activeVisualization.removeLayer(acc, layerId) : acc, + state.visualization.state + ); + + state.visualization.state = + typeof updater === 'function' ? updater(current(state.visualization.state)) : updater; + } + layerIds.forEach((layerId) => { + const [layerDatasourceId] = + Object.entries(datasourceMap).find(([datasourceId, datasource]) => { + return ( + state.datasourceStates[datasourceId] && + datasource.getLayers(state.datasourceStates[datasourceId].state).includes(layerId) + ); + }) ?? []; + if (layerDatasourceId) { + state.datasourceStates[layerDatasourceId].state = datasourceMap[ + layerDatasourceId + ].removeLayer(current(state).datasourceStates[layerDatasourceId].state, layerId); + } + }); + }, + + [addLayer.type]: ( + state, + { + payload: { layerId, layerType }, + }: { + payload: { + layerId: string; + layerType: LayerType; + }; + } + ) => { + if (!state.activeDatasourceId || !state.visualization.activeId) { + return state; + } + + const activeDatasource = datasourceMap[state.activeDatasourceId]; + const activeVisualization = visualizationMap[state.visualization.activeId]; + + const datasourceState = activeDatasource.insertLayer( + state.datasourceStates[state.activeDatasourceId].state, + layerId + ); + + const visualizationState = activeVisualization.appendLayer!( + state.visualization.state, + layerId, + layerType + ); + + const { activeDatasourceState, activeVisualizationState } = addInitialValueIfAvailable({ + datasourceState, + visualizationState, + framePublicAPI: { + // any better idea to avoid `as`? + activeData: state.activeData as TableInspectorAdapter, + datasourceLayers: getDatasourceLayers(state.datasourceStates, datasourceMap), + }, + activeVisualization, + activeDatasource, + layerId, + layerType, + }); + + state.visualization.state = activeVisualizationState; + state.datasourceStates[state.activeDatasourceId].state = activeDatasourceState; + state.stagedPreview = undefined; + }, + [setLayerDefaultDimension.type]: ( + state, + { + payload: { layerId, columnId, groupId }, + }: { + payload: { + layerId: string; + columnId: string; + groupId: string; + }; + } + ) => { + if (!state.activeDatasourceId || !state.visualization.activeId) { + return state; + } + + const activeDatasource = datasourceMap[state.activeDatasourceId]; + const activeVisualization = visualizationMap[state.visualization.activeId]; + const layerType = getLayerType(activeVisualization, state.visualization.state, layerId); + const { activeDatasourceState, activeVisualizationState } = addInitialValueIfAvailable({ + datasourceState: state.datasourceStates[state.activeDatasourceId].state, + visualizationState: state.visualization.state, + framePublicAPI: { + // any better idea to avoid `as`? + activeData: state.activeData as TableInspectorAdapter, + datasourceLayers: getDatasourceLayers(state.datasourceStates, datasourceMap), + }, + activeVisualization, + activeDatasource, + layerId, + layerType, + columnId, + groupId, + }); + + state.visualization.state = activeVisualizationState; + state.datasourceStates[state.activeDatasourceId].state = activeDatasourceState; + }, }); }; + +function addInitialValueIfAvailable({ + visualizationState, + datasourceState, + activeVisualization, + activeDatasource, + framePublicAPI, + layerType, + layerId, + columnId, + groupId, +}: { + framePublicAPI: FramePublicAPI; + visualizationState: unknown; + datasourceState: unknown; + activeDatasource: Datasource; + activeVisualization: Visualization; + layerId: string; + layerType: string; + columnId?: string; + groupId?: string; +}) { + const layerInfo = activeVisualization + .getSupportedLayers(visualizationState, framePublicAPI) + .find(({ type }) => type === layerType); + + if (layerInfo?.initialDimensions && activeDatasource?.initializeDimension) { + const info = groupId + ? layerInfo.initialDimensions.find(({ groupId: id }) => id === groupId) + : // pick the first available one if not passed + layerInfo.initialDimensions[0]; + + if (info) { + return { + activeDatasourceState: activeDatasource.initializeDimension(datasourceState, layerId, { + ...info, + columnId: columnId || info.columnId, + }), + activeVisualizationState: activeVisualization.setDimension({ + groupId: info.groupId, + layerId, + columnId: columnId || info.columnId, + prevState: visualizationState, + frame: framePublicAPI, + }), + }; + } + } + return { + activeDatasourceState: datasourceState, + activeVisualizationState: visualizationState, + }; +} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index e207f2938dd3c..975e44f703959 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -776,7 +776,7 @@ export interface LensBrushEvent { } // Use same technique as TriggerContext -interface LensEditContextMapping { +export interface LensEditContextMapping { [LENS_EDIT_SORT_ACTION]: LensSortActionData; [LENS_EDIT_RESIZE_ACTION]: LensResizeActionData; [LENS_TOGGLE_ACTION]: LensToggleActionData; diff --git a/x-pack/plugins/lens/public/utils.ts b/x-pack/plugins/lens/public/utils.ts index 993be9a06a2d9..921cc8fb364a2 100644 --- a/x-pack/plugins/lens/public/utils.ts +++ b/x-pack/plugins/lens/public/utils.ts @@ -16,8 +16,8 @@ import type { import type { IUiSettingsClient } from 'kibana/public'; import type { SavedObjectReference } from 'kibana/public'; import type { Document } from './persistence/saved_object_store'; -import type { Datasource, DatasourceMap } from './types'; -import type { DatasourceStates } from './state_management'; +import type { Datasource, DatasourceMap, Visualization } from './types'; +import type { DatasourceStates, VisualizationState } from './state_management'; export function getVisualizeGeoFieldMessage(fieldType: string) { return i18n.translate('xpack.lens.visualizeGeoFieldMessage', { @@ -94,3 +94,16 @@ export async function getIndexPatternsObjects( // return also the rejected ids in case we want to show something later on return { indexPatterns: fullfilled.map((response) => response.value), rejectedIds }; } + +export function getRemoveOperation( + activeVisualization: Visualization, + visualizationState: VisualizationState['state'], + layerId: string, + layerCount: number +) { + if (activeVisualization.getRemoveOperation) { + return activeVisualization.getRemoveOperation(visualizationState, layerId); + } + // fallback to generic count check + return layerCount === 1 ? 'clear' : 'remove'; +} From c84d20b8924e3a5dd3fc6ad8f0730fc6b8758298 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Wed, 3 Nov 2021 13:00:21 +0300 Subject: [PATCH 3/6] [TSVB] Fix cannot override the timerange mode on the override index pattern series settings (#116955) Closes: #116745 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../timeseries/common/types/panel_model.ts | 1 - .../application/components/aggs/agg.tsx | 22 +++++++++++++------ .../application/components/aggs/aggs.tsx | 2 +- .../components/aggs/histogram_support.test.js | 2 +- .../application/components/index_pattern.js | 10 ++++++--- .../lib/convert_series_to_datatable.test.ts | 1 - .../components/vis_types/timeseries/series.js | 2 -- 7 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/plugins/vis_types/timeseries/common/types/panel_model.ts b/src/plugins/vis_types/timeseries/common/types/panel_model.ts index f71602fdf0443..b4b167310a194 100644 --- a/src/plugins/vis_types/timeseries/common/types/panel_model.ts +++ b/src/plugins/vis_types/timeseries/common/types/panel_model.ts @@ -115,7 +115,6 @@ export interface Series { terms_size?: string; time_range_mode?: string; trend_arrows?: number; - type?: string; value_template?: string; var_name?: string; } diff --git a/src/plugins/vis_types/timeseries/public/application/components/aggs/agg.tsx b/src/plugins/vis_types/timeseries/public/application/components/aggs/agg.tsx index 08f8c072eef3b..bc74be2a562f9 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/aggs/agg.tsx +++ b/src/plugins/vis_types/timeseries/public/application/components/aggs/agg.tsx @@ -33,13 +33,13 @@ interface AggProps extends HTMLAttributes { siblings: Metric[]; uiRestrictions: TimeseriesUIRestrictions; dragHandleProps: DragHandleProps; - onChange: (part: Partial) => void; + onModelChange: (part: Partial) => void; onAdd: () => void; onDelete: () => void; } export function Agg(props: AggProps) { - const { model, uiRestrictions, series, name, onChange, fields, siblings } = props; + const { model, uiRestrictions, series, name, onModelChange, fields, siblings } = props; let Component = aggToComponent[model.type]; @@ -72,8 +72,8 @@ export function Agg(props: AggProps) { const isKibanaIndexPattern = props.panel.use_kibana_indexes || indexPattern === ''; const onAggChange = useMemo( - () => seriesChangeHandler({ name, model: series, onChange }, siblings), - [name, onChange, siblings, series] + () => seriesChangeHandler({ name, model: series, onChange: onModelChange }, siblings), + [name, onModelChange, siblings, series] ); useEffect(() => { @@ -86,17 +86,25 @@ export function Agg(props: AggProps) { ); if (isNumberFormatter && !isNumericMetric) { - onChange({ formatter: DATA_FORMATTERS.DEFAULT }); + onModelChange({ formatter: DATA_FORMATTERS.DEFAULT }); } // in case of string index pattern mode, change default formatter depending on metric type // "number" formatter for numeric metric and "" as custom formatter for any other type if (formatterType === DATA_FORMATTERS.DEFAULT && !isKibanaIndexPattern) { - onChange({ + onModelChange({ formatter: isNumericMetric ? DATA_FORMATTERS.NUMBER : '', }); } } - }, [indexPattern, model, onChange, fields, series.formatter, isKibanaIndexPattern, siblings]); + }, [ + indexPattern, + model, + onModelChange, + fields, + series.formatter, + isKibanaIndexPattern, + siblings, + ]); return (
diff --git a/src/plugins/vis_types/timeseries/public/application/components/aggs/aggs.tsx b/src/plugins/vis_types/timeseries/public/application/components/aggs/aggs.tsx index 516e3551fb010..f0172eaba0189 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/aggs/aggs.tsx +++ b/src/plugins/vis_types/timeseries/public/application/components/aggs/aggs.tsx @@ -51,7 +51,7 @@ export class Aggs extends PureComponent { name={name} model={row} onAdd={() => handleAdd(this.props, newMetricAggFn)} - onChange={onChange} + onModelChange={onChange} onDelete={() => handleDelete(this.props, row)} panel={panel} series={model} diff --git a/src/plugins/vis_types/timeseries/public/application/components/aggs/histogram_support.test.js b/src/plugins/vis_types/timeseries/public/application/components/aggs/histogram_support.test.js index c131ba2ae804c..ff96e476814ff 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/aggs/histogram_support.test.js +++ b/src/plugins/vis_types/timeseries/public/application/components/aggs/histogram_support.test.js @@ -34,7 +34,7 @@ const runTest = (aggType, name, test, additionalProps = {}) => {
{ onChange({ @@ -126,7 +130,7 @@ export const IndexPattern = ({ const selectedTimeRangeOption = timeRangeOptions.find( ({ value }) => model[TIME_RANGE_MODE_KEY] === value ); - const isTimeSeries = model.type === PANEL_TYPES.TIMESERIES; + const isDataTimerangeModeInvalid = !disabled && selectedTimeRangeOption && diff --git a/src/plugins/vis_types/timeseries/public/application/components/lib/convert_series_to_datatable.test.ts b/src/plugins/vis_types/timeseries/public/application/components/lib/convert_series_to_datatable.test.ts index bffc9200c9d6f..3df52223c253a 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/lib/convert_series_to_datatable.test.ts +++ b/src/plugins/vis_types/timeseries/public/application/components/lib/convert_series_to_datatable.test.ts @@ -154,7 +154,6 @@ describe('convert series to datatables', () => { ], split_mode: 'terms', terms_field: 'Cancelled', - type: 'timeseries', }, ], } as TimeseriesVisParams; diff --git a/src/plugins/vis_types/timeseries/public/application/components/vis_types/timeseries/series.js b/src/plugins/vis_types/timeseries/public/application/components/vis_types/timeseries/series.js index 53ded44353ddb..30a5867d799cb 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/vis_types/timeseries/series.js +++ b/src/plugins/vis_types/timeseries/public/application/components/vis_types/timeseries/series.js @@ -24,7 +24,6 @@ import { import { Split } from '../../split'; import { createTextHandler } from '../../lib/create_text_handler'; import { FormattedMessage, injectI18n } from '@kbn/i18n/react'; -import { PANEL_TYPES } from '../../../../../common/enums'; const TimeseriesSeriesUI = injectI18n(function (props) { const { @@ -45,7 +44,6 @@ const TimeseriesSeriesUI = injectI18n(function (props) { const defaults = { label: '', - type: PANEL_TYPES.TIMESERIES, }; const model = { ...defaults, ...props.model }; From 2ba7cdcf3053452ec797eae502e95ede9af21281 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 3 Nov 2021 11:04:04 +0100 Subject: [PATCH 4/6] use more robust way of entering value (#116566) --- test/functional/apps/visualize/_tsvb_time_series.ts | 3 +-- test/functional/page_objects/visual_builder_page.ts | 10 ++++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/functional/apps/visualize/_tsvb_time_series.ts b/test/functional/apps/visualize/_tsvb_time_series.ts index 4354e8bb44172..009e4a07cd42a 100644 --- a/test/functional/apps/visualize/_tsvb_time_series.ts +++ b/test/functional/apps/visualize/_tsvb_time_series.ts @@ -193,8 +193,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { }); }); - // FLAKY: https://github.com/elastic/kibana/issues/115529 - describe.skip('Elastic charts', () => { + describe('Elastic charts', () => { beforeEach(async () => { await visualBuilder.toggleNewChartsLibraryWithDebug(true); await visualBuilder.clickPanelOptions('timeSeries'); diff --git a/test/functional/page_objects/visual_builder_page.ts b/test/functional/page_objects/visual_builder_page.ts index f6e6caf102004..b87962b34291c 100644 --- a/test/functional/page_objects/visual_builder_page.ts +++ b/test/functional/page_objects/visual_builder_page.ts @@ -664,7 +664,10 @@ export class VisualBuilderPageObject extends FtrService { public async setBackgroundColor(colorHex: string): Promise { await this.clickColorPicker(); await this.checkColorPickerPopUpIsPresent(); - await this.find.setValue('.euiColorPicker input', colorHex); + await this.testSubjects.setValue('euiColorPickerInput_top', colorHex, { + clearWithKeyboard: true, + typeCharByChar: true, + }); await this.clickColorPicker(); await this.visChart.waitForVisualizationRenderingStabilized(); } @@ -677,7 +680,10 @@ export class VisualBuilderPageObject extends FtrService { public async setColorPickerValue(colorHex: string, nth: number = 0): Promise { await this.clickColorPicker(nth); await this.checkColorPickerPopUpIsPresent(); - await this.find.setValue('.euiColorPicker input', colorHex); + await this.testSubjects.setValue('euiColorPickerInput_top', colorHex, { + clearWithKeyboard: true, + typeCharByChar: true, + }); await this.clickColorPicker(nth); await this.visChart.waitForVisualizationRenderingStabilized(); } From b56979a97fbb81405386aaf54433820673e913ca Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 3 Nov 2021 11:04:48 +0100 Subject: [PATCH 5/6] retry visualize link navigation (#116582) --- test/functional/page_objects/visualize_page.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/functional/page_objects/visualize_page.ts b/test/functional/page_objects/visualize_page.ts index 7356ea3fa44c3..7624699928666 100644 --- a/test/functional/page_objects/visualize_page.ts +++ b/test/functional/page_objects/visualize_page.ts @@ -315,7 +315,10 @@ export class VisualizePageObject extends FtrService { public async openSavedVisualization(vizName: string) { const dataTestSubj = `visListingTitleLink-${vizName.split(' ').join('-')}`; - await this.testSubjects.click(dataTestSubj, 20000); + await this.retry.try(async () => { + await this.testSubjects.click(dataTestSubj, 20000); + await this.notOnLandingPageOrFail(); + }); await this.header.waitUntilLoadingHasFinished(); } @@ -337,6 +340,11 @@ export class VisualizePageObject extends FtrService { return await this.testSubjects.exists('visualizationLandingPage'); } + public async notOnLandingPageOrFail() { + this.log.debug(`VisualizePage.notOnLandingPageOrFail`); + return await this.testSubjects.missingOrFail('visualizationLandingPage'); + } + public async gotoLandingPage() { this.log.debug('VisualizePage.gotoLandingPage'); const onPage = await this.onLandingPage(); From 0681348c3a652015ee69cf5428618df5499a4859 Mon Sep 17 00:00:00 2001 From: Gloria Hornero Date: Wed, 3 Nov 2021 12:44:21 +0100 Subject: [PATCH 6/6] [Security Solution] Fixes skipped test (#115757) * waits for the page to be loaded before continuing with the actions * changes the order of the wait * unskip data provider test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../cypress/integration/timelines/data_providers.spec.ts | 2 +- x-pack/plugins/security_solution/cypress/tasks/timeline.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/cypress/integration/timelines/data_providers.spec.ts b/x-pack/plugins/security_solution/cypress/integration/timelines/data_providers.spec.ts index eeb7f91f8c050..de754f8602667 100644 --- a/x-pack/plugins/security_solution/cypress/integration/timelines/data_providers.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/timelines/data_providers.spec.ts @@ -20,7 +20,7 @@ import { addDataProvider, closeTimeline, createNewTimeline } from '../../tasks/t import { HOSTS_URL } from '../../urls/navigation'; import { cleanKibana, scrollToBottom } from '../../tasks/common'; -describe.skip('timeline data providers', () => { +describe('timeline data providers', () => { before(() => { cleanKibana(); loginAndWaitForPage(HOSTS_URL); diff --git a/x-pack/plugins/security_solution/cypress/tasks/timeline.ts b/x-pack/plugins/security_solution/cypress/tasks/timeline.ts index 6841a9afdc42b..01a3b6f18be80 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/timeline.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/timeline.ts @@ -175,6 +175,7 @@ export const addFilter = (filter: TimelineFilter): Cypress.Chainable> => { cy.get(TIMELINE_ADD_FIELD_BUTTON).click(); + cy.get(LOADING_INDICATOR).should('not.exist'); cy.get(TIMELINE_DATA_PROVIDER_VALUE).should('have.focus'); // make sure the focus is ready before start typing cy.get(TIMELINE_DATA_PROVIDER_FIELD)