[Testing] Embeddable Panel updateInput Reactivity Issue

src/plugins/dashboard/public/application/dashboard_app_controller.tsx

@@ -57,7 +57,6 @@ import {
   ViewMode,
   ContainerOutput,
   EmbeddableInput,
-  SavedObjectEmbeddableInput,
 } from '../../../embeddable/public';
 import { NavAction, SavedDashboardPanel } from '../types';
 
@@ -328,6 +327,21 @@ 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: {
+      //       ...incomingEmbeddable.input,
+      //       id: incomingEmbeddable.embeddableId,
+      //     },
+      //   };
+      //   incomingEmbeddable = undefined;
+      // }
+
       let expandedPanelId;
       if (dashboardContainer && !isErrorEmbeddable(dashboardContainer)) {
         expandedPanelId = dashboardContainer.getInput().expandedPanelId;
@@ -455,24 +469,10 @@ export class DashboardAppController {
             });
 
             if (incomingEmbeddable) {
-              if ('id' in incomingEmbeddable) {
-                container.addOrUpdateEmbeddable<SavedObjectEmbeddableInput>(
-                  incomingEmbeddable.type,
-                  {
-                    savedObjectId: incomingEmbeddable.id,
-                  }
-                );
-              } else if ('input' in incomingEmbeddable) {
-                const input = incomingEmbeddable.input;
-                delete input.id;
-                const explicitInput = {
-                  savedVis: input,
-                };
-                container.addOrUpdateEmbeddable<EmbeddableInput>(
-                  incomingEmbeddable.type,
-                  explicitInput
-                );
-              }
+              container.addOrUpdateEmbeddable<EmbeddableInput>(incomingEmbeddable.type, {
+                ...incomingEmbeddable.input,
+                id: incomingEmbeddable.embeddableId,
+              });
             }
           }
 

src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx

@@ -130,7 +130,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
   }
 
   public showPlaceholderUntil<TPlacementMethodArgs extends IPanelPlacementArgs>(
-    newStateComplete: Promise<Partial<PanelState>>,
+    newStateComplete: Promise<Partial<PanelState> & { explicitInput: EmbeddableInput }>,
     placementMethod?: PanelPlacementMethod<TPlacementMethodArgs>,
     placementArgs?: TPlacementMethodArgs
   ): void {
@@ -159,37 +159,20 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
         [placeholderPanelState.explicitInput.id]: placeholderPanelState,
       },
     });
-    newStateComplete.then((newPanelState: Partial<PanelState>) =>
-      this.replacePanel(placeholderPanelState, newPanelState)
+    newStateComplete.then(
+      (newPanelState: Partial<PanelState> & { explicitInput: EmbeddableInput }) =>
+        this.replacePanel(placeholderPanelState, newPanelState)
     );
   }
 
-  public replacePanel(
+  public async replacePanel(
     previousPanelState: DashboardPanelState<EmbeddableInput>,
-    newPanelState: Partial<PanelState>
+    newPanelState: Partial<PanelState> & { explicitInput: EmbeddableInput }
   ) {
-    // TODO: In the current infrastructure, embeddables in a container do not react properly to
-    // changes. Removing the existing embeddable, and adding a new one is a temporary workaround
-    // until the container logic is fixed.
-    const finalPanels = { ...this.input.panels };
-    delete finalPanels[previousPanelState.explicitInput.id];
-    const newPanelId = newPanelState.explicitInput?.id ? newPanelState.explicitInput.id : uuid.v4();
-    finalPanels[newPanelId] = {
-      ...previousPanelState,
-      ...newPanelState,
-      gridData: {
-        ...previousPanelState.gridData,
-        i: newPanelId,
-      },
-      explicitInput: {
-        ...newPanelState.explicitInput,
-        id: newPanelId,
-      },
-    };
-    this.updateInput({
-      panels: finalPanels,
-      lastReloadRequestTime: new Date().getTime(),
-    });
+    const embeddableToReplace = await this.untilEmbeddableLoaded(
+      previousPanelState.explicitInput.id
+    );
+    embeddableToReplace.updateInput(newPanelState.explicitInput);
   }
 
   public async addOrUpdateEmbeddable<
@@ -202,7 +185,7 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
         type,
         explicitInput: {
           ...explicitInput,
-          id: uuid.v4(),
+          id: explicitInput.id!,
         },
       });
     } else {

src/plugins/dashboard/public/application/embeddable/grid/dashboard_grid.tsx

@@ -263,13 +263,16 @@ class DashboardGridUi extends React.Component<DashboardGridProps, State> {
         <div
           style={{ zIndex: focusedPanelIndex === panel.explicitInput.id ? 2 : 'auto' }}
           className={classes}
+          // This key is required for the ReactGridLayout to work properly
           key={panel.explicitInput.id}
           data-test-subj="dashboardPanel"
           ref={(reactGridItem) => {
             this.gridItems[panel.explicitInput.id] = reactGridItem;
           }}
         >
           <EmbeddableChildPanel
+            // This key is used to force rerendering on embeddable type change while the id remains the same
+            key={panel.type}
             embeddableId={panel.explicitInput.id}
             container={this.props.container}
             PanelComponent={this.props.PanelComponent}

src/plugins/embeddable/public/index.ts

@@ -75,6 +75,8 @@ export {
   EmbeddablePackageState,
   EmbeddableRenderer,
   EmbeddableRendererProps,
+  ReferenceOrValueEmbeddable,
+  isReferenceOrValueEmbeddable,
 } from './lib';
 
 export function plugin(initializerContext: PluginInitializerContext) {

src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx

@@ -53,15 +53,46 @@ 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 embeddable = new EditableEmbeddable({ id: '123', viewMode: ViewMode.EDIT }, true);
+  const embeddable = new EditableEmbeddable(
+    { id: '123', viewMode: ViewMode.EDIT, coolInput1: 1, coolInput2: 2 },
+    true
+  );
   embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' }));
   await action.execute({ embeddable });
   expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', {
     path: '/123',
-    state: { originatingApp: 'superCoolCurrentApp' },
+    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' },
+    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: undefined,
+    },
   });
 });
 

src/plugins/embeddable/public/lib/actions/edit_panel_action.ts

@@ -24,7 +24,8 @@ import { take } from 'rxjs/operators';
 import { ViewMode } from '../types';
 import { EmbeddableFactoryNotFoundError } from '../errors';
 import { EmbeddableStart } from '../../plugin';
-import { IEmbeddable, EmbeddableEditorState, EmbeddableStateTransfer } from '../..';
+import { IEmbeddable, EmbeddableStateTransfer, EmbeddableEditorState } from '../..';
+import { SavedObjectEmbeddableInput } from '..';
 
 export const ACTION_EDIT_PANEL = 'editPanel';
 
@@ -109,8 +110,16 @@ export class EditPanelAction implements Action<ActionContext> {
     const app = embeddable ? embeddable.getOutput().editApp : undefined;
     const path = embeddable ? embeddable.getOutput().editPath : undefined;
     if (app && path) {
-      const state = this.currentAppId ? { originatingApp: this.currentAppId } : undefined;
-      return { app, path, state };
+      if (this.currentAppId) {
+        const byValueMode = !(embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId;
+        const state: EmbeddableEditorState = {
+          originatingApp: this.currentAppId,
+          valueInput: byValueMode ? embeddable.getInput() : undefined,
+          embeddableId: embeddable.getInput().id,
+        };
+        return { app, path, state };
+      }
+      return { app, path };
     }
   }
 

src/plugins/embeddable/public/lib/containers/container.ts

@@ -19,6 +19,7 @@
 
 import uuid from 'uuid';
 import { merge, Subscription } from 'rxjs';
+import { startWith, pairwise } from 'rxjs/operators';
 import {
   Embeddable,
   EmbeddableInput,
@@ -54,7 +55,12 @@ export abstract class Container<
     parent?: Container
   ) {
     super(input, output, parent);
-    this.subscription = this.getInput$().subscribe(() => this.maybeUpdateChildren());
+    this.subscription = this.getInput$()
+      // At each update event, get both the previous and current state
+      .pipe(startWith(input), pairwise())
+      .subscribe(([{ panels: prevPanels }, { panels: currentPanels }]) => {
+        this.maybeUpdateChildren(currentPanels, prevPanels);
+      });
   }
 
   public updateInputForChild<EEI extends EmbeddableInput = EmbeddableInput>(
@@ -198,8 +204,8 @@ export abstract class Container<
     return {
       type: factory.type,
       explicitInput: {
-        id: embeddableId,
         ...explicitInput,
+        id: embeddableId,
       } as TEmbeddableInput,
     };
   }
@@ -328,16 +334,27 @@ export abstract class Container<
     return embeddable;
   }
 
-  private maybeUpdateChildren() {
-    const allIds = Object.keys({ ...this.input.panels, ...this.output.embeddableLoaded });
+  private maybeUpdateChildren(
+    currentPanels: TContainerInput['panels'],
+    prevPanels: TContainerInput['panels']
+  ) {
+    const allIds = Object.keys({ ...currentPanels, ...this.output.embeddableLoaded });
     allIds.forEach((id) => {
-      if (this.input.panels[id] !== undefined && this.output.embeddableLoaded[id] === undefined) {
-        this.onPanelAdded(this.input.panels[id]);
-      } else if (
-        this.input.panels[id] === undefined &&
-        this.output.embeddableLoaded[id] !== undefined
-      ) {
-        this.onPanelRemoved(id);
+      if (currentPanels[id] !== undefined && this.output.embeddableLoaded[id] === undefined) {
+        return this.onPanelAdded(currentPanels[id]);
+      }
+      if (currentPanels[id] === undefined && this.output.embeddableLoaded[id] !== undefined) {
+        return this.onPanelRemoved(id);
+      }
+      // In case of type change, remove and add a panel with the same id
+      if (currentPanels[id] && prevPanels[id]) {
+        if (
+          currentPanels[id].type !== prevPanels[id].type ||
+          currentPanels[id].explicitInput !== prevPanels[id].explicitInput
+        ) {
+          this.onPanelRemoved(id);
+          this.onPanelAdded(currentPanels[id]);
+        }
       }
     });
   }

src/plugins/embeddable/public/lib/embeddables/index.ts

@@ -25,5 +25,5 @@ export { ErrorEmbeddable, isErrorEmbeddable } from './error_embeddable';
 export { withEmbeddableSubscription } from './with_subscription';
 export { EmbeddableRoot } from './embeddable_root';
 export * from './saved_object_embeddable';
-export { AttributeService } from './attribute_service';
+export * from './ref_or_val_embeddable';
 export { EmbeddableRenderer, EmbeddableRendererProps } from './embeddable_renderer';

src/plugins/embeddable/public/lib/embeddables/attribute_service.ts → src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/attribute_service.ts

@@ -17,14 +17,14 @@
  * under the License.
  */
 
-import { SavedObjectsClientContract } from '../../../../../core/public';
+import { SavedObjectsClientContract } from '../../../../../../core/public';
 import {
   SavedObjectEmbeddableInput,
   isSavedObjectEmbeddableInput,
   EmbeddableInput,
   IEmbeddable,
-} from '.';
-import { SimpleSavedObject } from '../../../../../core/public';
+} from '../';
+import { SimpleSavedObject } from '../../../../../../core/public';
 
 export class AttributeService<
   SavedObjectAttributes,

src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts

@@ -0,0 +1,21 @@
+/*
+ * 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 { ReferenceOrValueEmbeddable, isReferenceOrValueEmbeddable } from './types';
+export { AttributeService } from './attribute_service';

src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts

@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+import { EmbeddableInput, SavedObjectEmbeddableInput } from '..';
+
+export interface ReferenceOrValueEmbeddable<
+  ValTypeInput extends EmbeddableInput = EmbeddableInput,
+  RefTypeInput extends SavedObjectEmbeddableInput = SavedObjectEmbeddableInput
+> {
+  inputIsRefType: (input: ValTypeInput | RefTypeInput) => input is RefTypeInput;
+  getInputAsValueType: () => Promise<ValTypeInput>;
+  getInputAsRefType: () => Promise<RefTypeInput>;
+}
+
+export function isReferenceOrValueEmbeddable(
+  incoming: unknown
+): incoming is ReferenceOrValueEmbeddable {
+  return (
+    !!(incoming as ReferenceOrValueEmbeddable).inputIsRefType &&
+    !!(incoming as ReferenceOrValueEmbeddable).getInputAsValueType &&
+    !!(incoming as ReferenceOrValueEmbeddable).getInputAsRefType
+  );
+}