From 1f88c97a5aba6152006a2700c9fcb5e7084816b5 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 19 Nov 2020 18:03:04 -0500 Subject: [PATCH 1/3] Fixed via rollback of replacePanel changes --- .../embeddable/dashboard_container.tsx | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index 051a7ef8bfb92..d1102c71000a4 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -175,9 +175,29 @@ export class DashboardContainer extends Container, newPanelState: Partial ) { - // Because the embeddable type can change, we have to operate at the container level here - return this.updateInput({ - panels: { + let panels; + if ( + newPanelState.explicitInput?.id && + newPanelState.explicitInput.id !== previousPanelState.explicitInput.id + ) { + // replace panel can be called with a new id in order to destroy and recreate the embeddable + panels = { ...this.input.panels }; + delete panels[previousPanelState.explicitInput.id]; + panels[newPanelState.explicitInput.id] = { + ...previousPanelState, + ...newPanelState, + gridData: { + ...previousPanelState.gridData, + i: newPanelState.explicitInput.id, + }, + explicitInput: { + ...newPanelState.explicitInput, + id: newPanelState.explicitInput.id, + }, + }; + } else { + // Because the embeddable type can change, we have to operate at the container level here + panels = { ...this.input.panels, [previousPanelState.explicitInput.id]: { ...previousPanelState, @@ -190,7 +210,11 @@ export class DashboardContainer extends Container Date: Mon, 23 Nov 2020 15:00:32 -0500 Subject: [PATCH 2/3] Replace Panel generateNewId arg --- .../actions/add_to_library_action.tsx | 2 +- .../actions/unlink_from_library_action.tsx | 2 +- .../embeddable/dashboard_container.tsx | 17 ++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx b/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx index 179e5d522a2b3..f0dee5bfb5eab 100644 --- a/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx +++ b/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx @@ -91,7 +91,7 @@ export class AddToLibraryAction implements ActionByType, - newPanelState: Partial + newPanelState: Partial, + generateNewId?: boolean ) { let panels; - if ( - newPanelState.explicitInput?.id && - newPanelState.explicitInput.id !== previousPanelState.explicitInput.id - ) { - // replace panel can be called with a new id in order to destroy and recreate the embeddable + if (generateNewId) { + // replace panel can be called with generateNewId in order to totally destroy and recreate the embeddable panels = { ...this.input.panels }; delete panels[previousPanelState.explicitInput.id]; - panels[newPanelState.explicitInput.id] = { + const newId = newPanelState.explicitInput?.id ?? uuid.v4(); + panels[newId] = { ...previousPanelState, ...newPanelState, gridData: { ...previousPanelState.gridData, - i: newPanelState.explicitInput.id, + i: newId, }, explicitInput: { ...newPanelState.explicitInput, - id: newPanelState.explicitInput.id, + id: newId, }, }; } else { From eca655b2e003f6de16d7b8040fcaa2740761ea9e Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 23 Nov 2020 15:52:27 -0500 Subject: [PATCH 3/3] generate new id all the time when requested. Update tests --- .../actions/add_to_library_action.test.tsx | 18 ++++++++++++++---- .../actions/add_to_library_action.tsx | 3 +-- .../unlink_from_library_action.test.tsx | 18 ++++++++++++++---- .../actions/unlink_from_library_action.tsx | 3 +-- .../embeddable/dashboard_container.tsx | 2 +- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/plugins/dashboard/public/application/actions/add_to_library_action.test.tsx b/src/plugins/dashboard/public/application/actions/add_to_library_action.test.tsx index feb30b248c066..5f3945e733527 100644 --- a/src/plugins/dashboard/public/application/actions/add_to_library_action.test.tsx +++ b/src/plugins/dashboard/public/application/actions/add_to_library_action.test.tsx @@ -137,12 +137,17 @@ test('Add to library is not compatible when embeddable is not in a dashboard con test('Add to library replaces embeddableId and retains panel count', async () => { const dashboard = embeddable.getRoot() as IContainer; const originalPanelCount = Object.keys(dashboard.getInput().panels).length; + const originalPanelKeySet = new Set(Object.keys(dashboard.getInput().panels)); const action = new AddToLibraryAction({ toasts: coreStart.notifications.toasts }); await action.execute({ embeddable }); expect(Object.keys(container.getInput().panels).length).toEqual(originalPanelCount); - expect(Object.keys(container.getInput().panels)).toContain(embeddable.id); - const newPanel = container.getInput().panels[embeddable.id!]; + + const newPanelId = Object.keys(container.getInput().panels).find( + (key) => !originalPanelKeySet.has(key) + ); + expect(newPanelId).toBeDefined(); + const newPanel = container.getInput().panels[newPanelId!]; expect(newPanel.type).toEqual(embeddable.type); }); @@ -158,10 +163,15 @@ test('Add to library returns reference type input', async () => { mockedByReferenceInput: { savedObjectId: 'testSavedObjectId', id: embeddable.id }, mockedByValueInput: { attributes: complicatedAttributes, id: embeddable.id } as EmbeddableInput, }); + const dashboard = embeddable.getRoot() as IContainer; + const originalPanelKeySet = new Set(Object.keys(dashboard.getInput().panels)); const action = new AddToLibraryAction({ toasts: coreStart.notifications.toasts }); await action.execute({ embeddable }); - expect(Object.keys(container.getInput().panels)).toContain(embeddable.id); - const newPanel = container.getInput().panels[embeddable.id!]; + const newPanelId = Object.keys(container.getInput().panels).find( + (key) => !originalPanelKeySet.has(key) + ); + expect(newPanelId).toBeDefined(); + const newPanel = container.getInput().panels[newPanelId!]; expect(newPanel.type).toEqual(embeddable.type); expect(newPanel.explicitInput.attributes).toBeUndefined(); expect(newPanel.explicitInput.savedObjectId).toBe('testSavedObjectId'); diff --git a/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx b/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx index f0dee5bfb5eab..08cd0c7a15381 100644 --- a/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx +++ b/src/plugins/dashboard/public/application/actions/add_to_library_action.tsx @@ -19,7 +19,6 @@ import { i18n } from '@kbn/i18n'; import _ from 'lodash'; -import uuid from 'uuid'; import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin'; import { ViewMode, PanelState, IEmbeddable } from '../../embeddable_plugin'; import { @@ -89,7 +88,7 @@ export class AddToLibraryAction implements ActionByType = { type: embeddable.type, - explicitInput: { ...newInput, id: uuid.v4() }, + explicitInput: { ...newInput }, }; dashboard.replacePanel(panelToReplace, newPanel, true); diff --git a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.test.tsx b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.test.tsx index f191be6f7baad..6a9769b0c8d16 100644 --- a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.test.tsx +++ b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.test.tsx @@ -135,11 +135,16 @@ test('Unlink is not compatible when embeddable is not in a dashboard container', test('Unlink replaces embeddableId and retains panel count', async () => { const dashboard = embeddable.getRoot() as IContainer; const originalPanelCount = Object.keys(dashboard.getInput().panels).length; + const originalPanelKeySet = new Set(Object.keys(dashboard.getInput().panels)); const action = new UnlinkFromLibraryAction({ toasts: coreStart.notifications.toasts }); await action.execute({ embeddable }); expect(Object.keys(container.getInput().panels).length).toEqual(originalPanelCount); - expect(Object.keys(container.getInput().panels)).toContain(embeddable.id); - const newPanel = container.getInput().panels[embeddable.id!]; + + const newPanelId = Object.keys(container.getInput().panels).find( + (key) => !originalPanelKeySet.has(key) + ); + expect(newPanelId).toBeDefined(); + const newPanel = container.getInput().panels[newPanelId!]; expect(newPanel.type).toEqual(embeddable.type); }); @@ -159,10 +164,15 @@ test('Unlink unwraps all attributes from savedObject', async () => { mockedByReferenceInput: { savedObjectId: 'testSavedObjectId', id: embeddable.id }, mockedByValueInput: { attributes: complicatedAttributes, id: embeddable.id }, }); + const dashboard = embeddable.getRoot() as IContainer; + const originalPanelKeySet = new Set(Object.keys(dashboard.getInput().panels)); const action = new UnlinkFromLibraryAction({ toasts: coreStart.notifications.toasts }); await action.execute({ embeddable }); - expect(Object.keys(container.getInput().panels)).toContain(embeddable.id); - const newPanel = container.getInput().panels[embeddable.id!]; + const newPanelId = Object.keys(container.getInput().panels).find( + (key) => !originalPanelKeySet.has(key) + ); + expect(newPanelId).toBeDefined(); + const newPanel = container.getInput().panels[newPanelId!]; expect(newPanel.type).toEqual(embeddable.type); expect(newPanel.explicitInput.attributes).toEqual(complicatedAttributes); }); diff --git a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx index a4f7a2531a8da..b20bbc6350aaa 100644 --- a/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx +++ b/src/plugins/dashboard/public/application/actions/unlink_from_library_action.tsx @@ -19,7 +19,6 @@ import { i18n } from '@kbn/i18n'; import _ from 'lodash'; -import uuid from 'uuid'; import { ActionByType, IncompatibleActionError } from '../../ui_actions_plugin'; import { ViewMode, PanelState, IEmbeddable } from '../../embeddable_plugin'; import { @@ -88,7 +87,7 @@ export class UnlinkFromLibraryAction implements ActionByType = { type: embeddable.type, - explicitInput: { ...newInput, id: uuid.v4() }, + explicitInput: { ...newInput }, }; dashboard.replacePanel(panelToReplace, newPanel, true); diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index af78db3ec77a2..e80d387fa3066 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -181,7 +181,7 @@ export class DashboardContainer extends Container