[Time to Visualize] Lens By Value With AttributeService (elastic#77561)

Used the attribute service to make lens work properly with by value embeddables.
ThomThomson · Sep 23, 2020 · 6cbd28b · 6cbd28b
1 parent 3b75b1b
commit 6cbd28b
Show file tree

Hide file tree

Showing 37 changed files with 1,783 additions and 1,318 deletions.
diff --git a/src/plugins/dashboard/public/application/actions/library_notification_action.tsx b/src/plugins/dashboard/public/application/actions/library_notification_action.tsx
@@ -79,6 +79,7 @@ export class LibraryNotificationAction implements ActionByType<typeof ACTION_LIB
 
   public isCompatible = async ({ embeddable }: LibraryNotificationActionContext) => {
     return (
+      embeddable.getRoot().isContainer &&
       embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
       isReferenceOrValueEmbeddable(embeddable) &&
       embeddable.inputIsRefType(embeddable.getInput())

diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx
@@ -66,7 +66,6 @@ import {
   ViewMode,
   ContainerOutput,
   EmbeddableInput,
-  SavedObjectEmbeddableInput,
 } from '../../../embeddable/public';
 import { NavAction, SavedDashboardPanel } from '../types';
 
@@ -178,7 +177,7 @@ export class DashboardAppController {
       chrome.docTitle.change(dash.title);
     }
 
-    const incomingEmbeddable = embeddable
+    let incomingEmbeddable = embeddable
       .getStateTransfer(scopedHistory())
       .getIncomingEmbeddablePackage();
 
@@ -344,6 +343,22 @@ export class DashboardAppController {
       dashboardStateManager.getPanels().forEach((panel: SavedDashboardPanel) => {
         embeddablesMap[panel.panelIndex] = convertSavedDashboardPanelToPanelState(panel);
       });
+
+      // If the incoming embeddable state's id already exists in the embeddables map, replace the input, retaining the existing gridData for that panel.
+      if (incomingEmbeddable?.embeddableId && embeddablesMap[incomingEmbeddable.embeddableId]) {
+        const originalPanelState = embeddablesMap[incomingEmbeddable.embeddableId];
+        embeddablesMap[incomingEmbeddable.embeddableId] = {
+          gridData: originalPanelState.gridData,
+          type: incomingEmbeddable.type,
+          explicitInput: {
+            ...originalPanelState.explicitInput,
+            ...incomingEmbeddable.input,
+            id: incomingEmbeddable.embeddableId,
+          },
+        };
+        incomingEmbeddable = undefined;
+      }
+
       let expandedPanelId;
       if (dashboardContainer && !isErrorEmbeddable(dashboardContainer)) {
         expandedPanelId = dashboardContainer.getInput().expandedPanelId;
@@ -482,32 +497,16 @@ export class DashboardAppController {
               refreshDashboardContainer();
             });
 
-            if (incomingEmbeddable) {
-              if ('id' in incomingEmbeddable) {
-                container.addOrUpdateEmbeddable<SavedObjectEmbeddableInput>(
-                  incomingEmbeddable.type,
-                  {
-                    savedObjectId: incomingEmbeddable.id,
-                  }
-                );
-              } else if ('input' in incomingEmbeddable) {
-                const input = incomingEmbeddable.input;
-                // @ts-expect-error
-                delete input.id;
-                const explicitInput = {
-                  savedVis: input,
-                };
-                const embeddableId =
-                  'embeddableId' in incomingEmbeddable
-                    ? incomingEmbeddable.embeddableId
-                    : undefined;
-                container.addOrUpdateEmbeddable<EmbeddableInput>(
-                  incomingEmbeddable.type,
-                  // This ugly solution is temporary - https://github.com/elastic/kibana/pull/70272 fixes this whole section
-                  (explicitInput as unknown) as EmbeddableInput,
-                  embeddableId
-                );
-              }
+            // If the incomingEmbeddable does not yet exist in the panels listing, create a new panel using the container's addEmbeddable method.
+            if (
+              incomingEmbeddable &&
+              (!incomingEmbeddable.embeddableId ||
+                !container.getInput().panels[incomingEmbeddable.embeddableId])
+            ) {
+              container.addNewEmbeddable<EmbeddableInput>(
+                incomingEmbeddable.type,
+                incomingEmbeddable.input
+              );
             }
           }
 

diff --git a/...ribute_service/attribute_service_mock.tsx → ...ribute_service/attribute_service.mock.tsx b/...ribute_service/attribute_service_mock.tsx → ...ribute_service/attribute_service.mock.tsx
diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.test.ts b/src/plugins/dashboard/public/attribute_service/attribute_service.test.ts
@@ -18,7 +18,7 @@
  */
 
 import { ATTRIBUTE_SERVICE_KEY } from './attribute_service';
-import { mockAttributeService } from './attribute_service_mock';
+import { mockAttributeService } from './attribute_service.mock';
 import { coreMock } from '../../../../core/public/mocks';
 
 interface TestAttributes {

diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx
@@ -156,12 +156,8 @@ export class AttributeService<
   };
 
   public getExplicitInputFromEmbeddable(embeddable: IEmbeddable): ValType | RefType {
-    return embeddable.getRoot() &&
-      (embeddable.getRoot() as Container).getInput().panels[embeddable.id].explicitInput
-      ? ((embeddable.getRoot() as Container).getInput().panels[embeddable.id].explicitInput as
-          | ValType
-          | RefType)
-      : (embeddable.getInput() as ValType | RefType);
+    return ((embeddable.getRoot() as Container).getInput()?.panels?.[embeddable.id]
+      ?.explicitInput ?? embeddable.getInput()) as ValType | RefType;
   }
 
   getInputAsValueType = async (input: ValType | RefType): Promise<ValType> => {
@@ -204,7 +200,14 @@ export class AttributeService<
           const newAttributes = { ...input[ATTRIBUTE_SERVICE_KEY] };
           newAttributes.title = props.newTitle;
           const wrappedInput = (await this.wrapAttributes(newAttributes, true)) as RefType;
-          resolve(wrappedInput);
+
+          // Remove unneeded attributes from the original input.
+          delete (input as { [ATTRIBUTE_SERVICE_KEY]?: SavedObjectAttributes })[
+            ATTRIBUTE_SERVICE_KEY
+          ];
+
+          // Combine input and wrapped input to preserve any passed in explicit Input.
+          resolve({ ...input, ...wrappedInput });
           return { id: wrappedInput.savedObjectId };
         } catch (error) {
           reject(error);

diff --git a/src/plugins/dashboard/public/attribute_service/index.ts b/src/plugins/dashboard/public/attribute_service/index.ts
@@ -0,0 +1,20 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './attribute_service';
diff --git a/src/plugins/dashboard/public/index.ts b/src/plugins/dashboard/public/index.ts
@@ -31,7 +31,7 @@ export {
 } from './application';
 export { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
 
-export { DashboardStart, DashboardUrlGenerator } from './plugin';
+export { DashboardStart, DashboardUrlGenerator, DashboardFeatureFlagConfig } from './plugin';
 export {
   DASHBOARD_APP_URL_GENERATOR,
   createDashboardUrlGenerator,
@@ -40,7 +40,7 @@ export {
 export { addEmbeddableToDashboardUrl } from './url_utils/url_helper';
 export { SavedObjectDashboard } from './saved_dashboards';
 export { SavedDashboardPanel } from './types';
-export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './attribute_service/attribute_service';
+export { AttributeService, ATTRIBUTE_SERVICE_KEY } from './attribute_service';
 
 export function plugin(initializerContext: PluginInitializerContext) {
   return new DashboardPlugin(initializerContext);

diff --git a/src/plugins/dashboard/public/mocks.tsx b/src/plugins/dashboard/public/mocks.tsx
@@ -20,6 +20,7 @@
 import { DashboardStart } from './plugin';
 
 export type Start = jest.Mocked<DashboardStart>;
+export { mockAttributeService } from './attribute_service/attribute_service.mock';
 
 const createStartContract = (): DashboardStart => {
   // @ts-ignore

diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx
@@ -117,7 +117,7 @@ declare module '../../share/public' {
 
 export type DashboardUrlGenerator = UrlGeneratorContract<typeof DASHBOARD_APP_URL_GENERATOR>;
 
-interface DashboardFeatureFlagConfig {
+export interface DashboardFeatureFlagConfig {
   allowByValueEmbeddables: boolean;
 }
 

diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx
@@ -18,7 +18,7 @@
  */
 
 import { EditPanelAction } from './edit_panel_action';
-import { Embeddable, EmbeddableInput } from '../embeddables';
+import { Embeddable, EmbeddableInput, SavedObjectEmbeddableInput } from '../embeddables';
 import { ViewMode } from '../types';
 import { ContactCardEmbeddable } from '../test_samples';
 import { embeddablePluginMock } from '../../mocks';
@@ -53,20 +53,50 @@ test('is compatible when edit url is available, in edit mode and editable', asyn
   ).toBe(true);
 });
 
-test('redirects to app using state transfer', async () => {
+test('redirects to app using state transfer with by value mode', async () => {
   applicationMock.currentAppId$ = of('superCoolCurrentApp');
   const action = new EditPanelAction(getFactory, applicationMock, stateTransferMock);
-  const input = { id: '123', viewMode: ViewMode.EDIT };
-  const embeddable = new EditableEmbeddable(input, true);
+  const embeddable = new EditableEmbeddable(
+    ({
+      id: '123',
+      viewMode: ViewMode.EDIT,
+      coolInput1: 1,
+      coolInput2: 2,
+    } as unknown) as EmbeddableInput,
+    true
+  );
+  embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' }));
+  await action.execute({ embeddable });
+  expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', {
+    path: '/123',
+    state: {
+      originatingApp: 'superCoolCurrentApp',
+      embeddableId: '123',
+      valueInput: {
+        id: '123',
+        viewMode: ViewMode.EDIT,
+        coolInput1: 1,
+        coolInput2: 2,
+      },
+    },
+  });
+});
+
+test('redirects to app using state transfer without by value mode', async () => {
+  applicationMock.currentAppId$ = of('superCoolCurrentApp');
+  const action = new EditPanelAction(getFactory, applicationMock, stateTransferMock);
+  const embeddable = new EditableEmbeddable(
+    { id: '123', viewMode: ViewMode.EDIT, savedObjectId: '1234' } as SavedObjectEmbeddableInput,
+    true
+  );
   embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' }));
   await action.execute({ embeddable });
   expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', {
     path: '/123',
     state: {
       originatingApp: 'superCoolCurrentApp',
-      byValueMode: true,
       embeddableId: '123',
-      valueInput: input,
+      valueInput: undefined,
     },
   });
 });

diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts
@@ -29,6 +29,8 @@ import {
   EmbeddableEditorState,
   EmbeddableStateTransfer,
   SavedObjectEmbeddableInput,
+  EmbeddableInput,
+  Container,
 } from '../..';
 
 export const ACTION_EDIT_PANEL = 'editPanel';
@@ -118,8 +120,7 @@ export class EditPanelAction implements Action<ActionContext> {
         const byValueMode = !(embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId;
         const state: EmbeddableEditorState = {
           originatingApp: this.currentAppId,
-          byValueMode,
-          valueInput: byValueMode ? embeddable.getInput() : undefined,
+          valueInput: byValueMode ? this.getExplicitInput({ embeddable }) : undefined,
           embeddableId: embeddable.id,
         };
         return { app, path, state };
@@ -132,4 +133,11 @@ export class EditPanelAction implements Action<ActionContext> {
     const editUrl = embeddable ? embeddable.getOutput().editUrl : undefined;
     return editUrl ? editUrl : '';
   }
+
+  private getExplicitInput({ embeddable }: ActionContext): EmbeddableInput {
+    return (
+      (embeddable.getRoot() as Container)?.getInput()?.panels?.[embeddable.id]?.explicitInput ??
+      embeddable.getInput()
+    );
+  }
 }
diff --git a/src/plugins/embeddable/public/lib/containers/container.ts b/src/plugins/embeddable/public/lib/containers/container.ts
@@ -199,8 +199,8 @@ export abstract class Container<
     return {
       type: factory.type,
       explicitInput: {
-        id: embeddableId,
         ...explicitInput,
+        id: embeddableId,
       } as TEmbeddableInput,
     };
   }

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
@@ -85,23 +85,27 @@ describe('embeddable state transfer', () => {
 
   it('can send an outgoing embeddable package state', async () => {
     await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, {
-      state: { type: 'coolestType', id: '150' },
+      state: { type: 'coolestType', input: { savedObjectId: '150' } },
     });
     expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', {
-      state: { type: 'coolestType', id: '150' },
+      state: { type: 'coolestType', input: { savedObjectId: '150' } },
     });
   });
 
   it('can send an outgoing embeddable package state in append mode', async () => {
     const historyMock = mockHistoryState({ kibanaIsNowForSports: 'extremeSportsKibana' });
     stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock);
     await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, {
-      state: { type: 'coolestType', id: '150' },
+      state: { type: 'coolestType', input: { savedObjectId: '150' } },
       appendToExistingState: true,
     });
     expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', {
       path: undefined,
-      state: { kibanaIsNowForSports: 'extremeSportsKibana', type: 'coolestType', id: '150' },
+      state: {
+        kibanaIsNowForSports: 'extremeSportsKibana',
+        type: 'coolestType',
+        input: { savedObjectId: '150' },
+      },
     });
   });
 
@@ -120,10 +124,13 @@ describe('embeddable state transfer', () => {
   });
 
   it('can fetch an incoming embeddable package state', async () => {
-    const historyMock = mockHistoryState({ type: 'skisEmbeddable', id: '123' });
+    const historyMock = mockHistoryState({
+      type: 'skisEmbeddable',
+      input: { savedObjectId: '123' },
+    });
     stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock);
     const fetchedState = stateTransfer.getIncomingEmbeddablePackage();
-    expect(fetchedState).toEqual({ type: 'skisEmbeddable', id: '123' });
+    expect(fetchedState).toEqual({ type: 'skisEmbeddable', input: { savedObjectId: '123' } });
   });
 
   it('returns undefined when embeddable package is not in the right shape', async () => {
@@ -136,12 +143,12 @@ describe('embeddable state transfer', () => {
   it('removes all keys in the keysToRemoveAfterFetch array', async () => {
     const historyMock = mockHistoryState({
       type: 'skisEmbeddable',
-      id: '123',
+      input: { savedObjectId: '123' },
       test1: 'test1',
       test2: 'test2',
     });
     stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock);
-    stateTransfer.getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['type', 'id'] });
+    stateTransfer.getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['type', 'input'] });
     expect(historyMock.replace).toHaveBeenCalledWith(
       expect.objectContaining({ state: { test1: 'test1', test2: 'test2' } })
     );
@@ -150,15 +157,15 @@ describe('embeddable state transfer', () => {
   it('leaves state as is when no keysToRemove are supplied', async () => {
     const historyMock = mockHistoryState({
       type: 'skisEmbeddable',
-      id: '123',
+      input: { savedObjectId: '123' },
       test1: 'test1',
       test2: 'test2',
     });
     stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock);
     stateTransfer.getIncomingEmbeddablePackage();
     expect(historyMock.location.state).toEqual({
       type: 'skisEmbeddable',
-      id: '123',
+      input: { savedObjectId: '123' },
       test1: 'test1',
       test2: 'test2',
     });