From 7994e87cd77a169f00adaec85a0f59c5b7357218 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Thu, 11 Feb 2021 23:23:14 -0500 Subject: [PATCH] [Time to Visualize] Clear All Editor State when Visualize Listing Page Loads (#91005) * Changed the embeddable state transfer service so that it is possible to clear all editor state at once. Used that method in the visualize listing page --- ...mbeddablestatetransfer.cleareditorstate.md | 2 +- .../embeddable_state_transfer.test.ts | 104 +++++++++++------- .../embeddable_state_transfer.ts | 36 +++--- src/plugins/embeddable/public/public.api.md | 2 +- .../components/visualize_listing.tsx | 4 +- .../apps/dashboard/dashboard_lens_by_value.ts | 27 ++++- 6 files changed, 111 insertions(+), 64 deletions(-) diff --git a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.cleareditorstate.md b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.cleareditorstate.md index 034f9c70e389f..d5a8ec311df31 100644 --- a/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.cleareditorstate.md +++ b/docs/development/plugins/embeddable/public/kibana-plugin-plugins-embeddable-public.embeddablestatetransfer.cleareditorstate.md @@ -9,7 +9,7 @@ Clears the [editor state](./kibana-plugin-plugins-embeddable-public.embeddableed Signature: ```typescript -clearEditorState(appId: string): void; +clearEditorState(appId?: string): void; ``` ## Parameters diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts index a8ecb384f782b..2dda0df1a85c5 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts @@ -44,8 +44,6 @@ describe('embeddable state transfer', () => { const testAppId = 'testApp'; - const buildKey = (appId: string, key: string) => `${appId}-${key}`; - beforeEach(() => { currentAppId$ = new Subject(); currentAppId$.next(originatingApp); @@ -86,8 +84,10 @@ describe('embeddable state transfer', () => { it('can send an outgoing editor state', async () => { await stateTransfer.navigateToEditor(destinationApp, { state: { originatingApp } }); expect(store.set).toHaveBeenCalledWith(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(destinationApp, EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'superUltraTestDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + [destinationApp]: { + originatingApp: 'superUltraTestDashboard', + }, }, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { @@ -104,8 +104,10 @@ describe('embeddable state transfer', () => { }); expect(store.set).toHaveBeenCalledWith(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { kibanaIsNowForSports: 'extremeSportsKibana', - [buildKey(destinationApp, EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'superUltraTestDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + [destinationApp]: { + originatingApp: 'superUltraTestDashboard', + }, }, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { @@ -125,9 +127,11 @@ describe('embeddable state transfer', () => { state: { type: 'coolestType', input: { savedObjectId: '150' } }, }); expect(store.set).toHaveBeenCalledWith(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(destinationApp, EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'coolestType', - input: { savedObjectId: '150' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [destinationApp]: { + type: 'coolestType', + input: { savedObjectId: '150' }, + }, }, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { @@ -144,9 +148,11 @@ describe('embeddable state transfer', () => { }); expect(store.set).toHaveBeenCalledWith(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { kibanaIsNowForSports: 'extremeSportsKibana', - [buildKey(destinationApp, EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'coolestType', - input: { savedObjectId: '150' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [destinationApp]: { + type: 'coolestType', + input: { savedObjectId: '150' }, + }, }, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { @@ -165,8 +171,10 @@ describe('embeddable state transfer', () => { it('can fetch an incoming editor state', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'superUltraTestDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + [testAppId]: { + originatingApp: 'superUltraTestDashboard', + }, }, }); const fetchedState = stateTransfer.getIncomingEditorState(testAppId); @@ -175,14 +183,16 @@ describe('embeddable state transfer', () => { it('can fetch an incoming editor state and ignore state for other apps', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey('otherApp1', EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'whoops not me', - }, - [buildKey('otherApp2', EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'otherTestDashboard', - }, - [buildKey(testAppId, EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'superUltraTestDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + otherApp1: { + originatingApp: 'whoops not me', + }, + otherApp2: { + originatingApp: 'otherTestDashboard', + }, + [testAppId]: { + originatingApp: 'superUltraTestDashboard', + }, }, }); const fetchedState = stateTransfer.getIncomingEditorState(testAppId); @@ -194,8 +204,10 @@ describe('embeddable state transfer', () => { it('incoming editor state returns undefined when state is not in the right shape', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_EDITOR_STATE_KEY)]: { - helloSportsKibana: 'superUltraTestDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + [testAppId]: { + helloSportsKibana: 'superUltraTestDashboard', + }, }, }); const fetchedState = stateTransfer.getIncomingEditorState(testAppId); @@ -204,9 +216,11 @@ describe('embeddable state transfer', () => { it('can fetch an incoming embeddable package state', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'skisEmbeddable', - input: { savedObjectId: '123' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [testAppId]: { + type: 'skisEmbeddable', + input: { savedObjectId: '123' }, + }, }, }); const fetchedState = stateTransfer.getIncomingEmbeddablePackage(testAppId); @@ -215,13 +229,15 @@ describe('embeddable state transfer', () => { it('can fetch an incoming embeddable package state and ignore state for other apps', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'skisEmbeddable', - input: { savedObjectId: '123' }, - }, - [buildKey('testApp2', EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'crossCountryEmbeddable', - input: { savedObjectId: '456' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [testAppId]: { + type: 'skisEmbeddable', + input: { savedObjectId: '123' }, + }, + testApp2: { + type: 'crossCountryEmbeddable', + input: { savedObjectId: '456' }, + }, }, }); const fetchedState = stateTransfer.getIncomingEmbeddablePackage(testAppId); @@ -236,7 +252,11 @@ describe('embeddable state transfer', () => { it('embeddable package state returns undefined when state is not in the right shape', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_PACKAGE_STATE_KEY)]: { kibanaIsFor: 'sports' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [testAppId]: { + kibanaIsFor: 'sports', + }, + }, }); const fetchedState = stateTransfer.getIncomingEmbeddablePackage(testAppId); expect(fetchedState).toBeUndefined(); @@ -244,9 +264,11 @@ describe('embeddable state transfer', () => { it('removes embeddable package key when removeAfterFetch is true', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_PACKAGE_STATE_KEY)]: { - type: 'coolestType', - input: { savedObjectId: '150' }, + [EMBEDDABLE_PACKAGE_STATE_KEY]: { + [testAppId]: { + type: 'coolestType', + input: { savedObjectId: '150' }, + }, }, iSHouldStillbeHere: 'doing the sports thing', }); @@ -258,8 +280,10 @@ describe('embeddable state transfer', () => { it('removes editor state key when removeAfterFetch is true', async () => { store.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, { - [buildKey(testAppId, EMBEDDABLE_EDITOR_STATE_KEY)]: { - originatingApp: 'superCoolFootballDashboard', + [EMBEDDABLE_EDITOR_STATE_KEY]: { + [testAppId]: { + originatingApp: 'superCoolFootballDashboard', + }, }, iSHouldStillbeHere: 'doing the sports thing', }); diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts index 8664a5aae7345..52a5eccac9910 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts @@ -75,10 +75,14 @@ export class EmbeddableStateTransfer { * @param appId - The app to fetch incomingEditorState for * @param removeAfterFetch - Whether to remove the package state after fetch to prevent duplicates. */ - public clearEditorState(appId: string) { + public clearEditorState(appId?: string) { const currentState = this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY); if (currentState) { - delete currentState[this.buildKey(appId, EMBEDDABLE_EDITOR_STATE_KEY)]; + if (appId) { + delete currentState[EMBEDDABLE_EDITOR_STATE_KEY]?.[appId]; + } else { + delete currentState[EMBEDDABLE_EDITOR_STATE_KEY]; + } this.storage.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, currentState); } } @@ -117,7 +121,6 @@ export class EmbeddableStateTransfer { this.isTransferInProgress = true; await this.navigateToWithState(appId, EMBEDDABLE_EDITOR_STATE_KEY, { ...options, - appendToExistingState: true, }); } @@ -132,14 +135,9 @@ export class EmbeddableStateTransfer { this.isTransferInProgress = true; await this.navigateToWithState(appId, EMBEDDABLE_PACKAGE_STATE_KEY, { ...options, - appendToExistingState: true, }); } - private buildKey(appId: string, key: string) { - return `${appId}-${key}`; - } - private getIncomingState( guard: (state: unknown) => state is IncomingStateType, appId: string, @@ -148,15 +146,13 @@ export class EmbeddableStateTransfer { keysToRemoveAfterFetch?: string[]; } ): IncomingStateType | undefined { - const incomingState = this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY)?.[ - this.buildKey(appId, key) - ]; + const incomingState = this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY)?.[key]?.[appId]; const castState = !guard || guard(incomingState) ? (cloneDeep(incomingState) as IncomingStateType) : undefined; if (castState && options?.keysToRemoveAfterFetch) { const stateReplace = { ...this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY) }; options.keysToRemoveAfterFetch.forEach((keyToRemove: string) => { - delete stateReplace[this.buildKey(appId, keyToRemove)]; + delete stateReplace[keyToRemove]; }); this.storage.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, stateReplace); } @@ -166,14 +162,16 @@ export class EmbeddableStateTransfer { private async navigateToWithState( appId: string, key: string, - options?: { path?: string; state?: OutgoingStateType; appendToExistingState?: boolean } + options?: { path?: string; state?: OutgoingStateType } ): Promise { - const stateObject = options?.appendToExistingState - ? { - ...this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY), - [this.buildKey(appId, key)]: options.state, - } - : { [this.buildKey(appId, key)]: options?.state }; + const existingAppState = this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY)?.[key] || {}; + const stateObject = { + ...this.storage.get(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY), + [key]: { + ...existingAppState, + [appId]: options?.state, + }, + }; this.storage.set(EMBEDDABLE_STATE_TRANSFER_STORAGE_KEY, stateObject); await this.navigateToApp(appId, { path: options?.path }); } diff --git a/src/plugins/embeddable/public/public.api.md b/src/plugins/embeddable/public/public.api.md index 3e7014d54958d..189f71b85206b 100644 --- a/src/plugins/embeddable/public/public.api.md +++ b/src/plugins/embeddable/public/public.api.md @@ -590,7 +590,7 @@ export class EmbeddableStateTransfer { // Warning: (ae-forgotten-export) The symbol "ApplicationStart" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "PublicAppInfo" needs to be exported by the entry point index.d.ts constructor(navigateToApp: ApplicationStart['navigateToApp'], currentAppId$: ApplicationStart['currentAppId$'], appList?: ReadonlyMap | undefined, customStorage?: Storage); - clearEditorState(appId: string): void; + clearEditorState(appId?: string): void; getAppNameFromId: (appId: string) => string | undefined; getIncomingEditorState(appId: string, removeAfterFetch?: boolean): EmbeddableEditorState | undefined; getIncomingEmbeddablePackage(appId: string, removeAfterFetch?: boolean): EmbeddablePackageState | undefined; diff --git a/src/plugins/visualize/public/application/components/visualize_listing.tsx b/src/plugins/visualize/public/application/components/visualize_listing.tsx index 87660b64bab61..024752188a88b 100644 --- a/src/plugins/visualize/public/application/components/visualize_listing.tsx +++ b/src/plugins/visualize/public/application/components/visualize_listing.tsx @@ -64,8 +64,8 @@ export const VisualizeListing = () => { }, [history, pathname, visualizations]); useMount(() => { - // Reset editor state if the visualize listing page is loaded. - stateTransferService.clearEditorState(VisualizeConstants.APP_ID); + // Reset editor state for all apps if the visualize listing page is loaded. + stateTransferService.clearEditorState(); chrome.setBreadcrumbs([ { text: i18n.translate('visualize.visualizeListingBreadcrumbsTitle', { diff --git a/x-pack/test/functional/apps/dashboard/dashboard_lens_by_value.ts b/x-pack/test/functional/apps/dashboard/dashboard_lens_by_value.ts index a962d22e16551..f270142b441e2 100644 --- a/x-pack/test/functional/apps/dashboard/dashboard_lens_by_value.ts +++ b/x-pack/test/functional/apps/dashboard/dashboard_lens_by_value.ts @@ -9,10 +9,11 @@ import expect from '@kbn/expect'; import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getPageObjects, getService }: FtrProviderContext) { - const PageObjects = getPageObjects(['common', 'dashboard', 'visualize', 'lens']); + const PageObjects = getPageObjects(['common', 'dashboard', 'visualize', 'lens', 'header']); const find = getService('find'); const esArchiver = getService('esArchiver'); + const testSubjects = getService('testSubjects'); const dashboardPanelActions = getService('dashboardPanelActions'); const dashboardVisualizations = getService('dashboardVisualizations'); @@ -69,5 +70,29 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { const titles = await PageObjects.dashboard.getPanelTitles(); expect(titles.indexOf(newTitle)).to.not.be(-1); }); + + it('is no longer linked to a dashboard after visiting the visuali1ze listing page', async () => { + await PageObjects.visualize.gotoVisualizationLandingPage(); + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickLensWidget(); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'avg', + field: 'bytes', + }); + await PageObjects.lens.notLinkedToOriginatingApp(); + await PageObjects.header.waitUntilLoadingHasFinished(); + + // return to origin should not be present in save modal + await testSubjects.click('lnsApp_saveButton'); + const redirectToOriginCheckboxExists = await testSubjects.exists('returnToOriginModeSwitch'); + expect(redirectToOriginCheckboxExists).to.be(false); + }); }); }