[Dashboard] Fix cloning panels reactive issue

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

@@ -27,6 +27,7 @@ import {
   ContactCardEmbeddableInput,
   ContactCardEmbeddable,
   ContactCardEmbeddableOutput,
+  EMPTY_EMBEDDABLE,
 } from '../../embeddable_plugin_test_samples';
 import { embeddablePluginMock } from 'src/plugins/embeddable/public/mocks';
 
@@ -100,6 +101,48 @@ test('DashboardContainer.addNewEmbeddable', async () => {
   expect(embeddableInContainer.id).toBe(embeddable.id);
 });
 
+test('DashboardContainer.replacePanel', async (done) => {
+  const ID = '123';
+  const initialInput = getSampleDashboardInput({
+    panels: {
+      [ID]: getSampleDashboardPanel<ContactCardEmbeddableInput>({
+        explicitInput: { firstName: 'Sam', id: ID },
+        type: CONTACT_CARD_EMBEDDABLE,
+      }),
+    },
+  });
+
+  const container = new DashboardContainer(initialInput, options);
+  let counter = 0;
+
+  const subscriptionHandler = jest.fn(({ panels }) => {
+    counter++;
+    expect(panels[ID]).toBeDefined();
+    // It should be called exactly 2 times and exit the second time
+    switch (counter) {
+      case 1:
+        return expect(panels[ID].type).toBe(CONTACT_CARD_EMBEDDABLE);
+
+      case 2: {
+        expect(panels[ID].type).toBe(EMPTY_EMBEDDABLE);
+        subscription.unsubscribe();
+        done();
+      }
+
+      default:
+        throw Error('Called too many times!');
+    }
+  });
+
+  const subscription = container.getInput$().subscribe(subscriptionHandler);
+
+  // replace the panel now
+  container.replacePanel(container.getInput().panels[ID], {
+    type: EMPTY_EMBEDDABLE,
+    explicitInput: { id: ID },
+  });
+});
+
 test('Container view mode change propagates to existing children', async () => {
   const initialInput = getSampleDashboardInput({
     panels: {

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

@@ -168,27 +168,22 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
     previousPanelState: DashboardPanelState<EmbeddableInput>,
     newPanelState: Partial<PanelState>
   ) {
-    // 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,
+    // Because the embeddable type can change, we have to operate at the container level here
+    return this.updateInput({
+      panels: {
+        ...this.input.panels,
+        [previousPanelState.explicitInput.id]: {
+          ...previousPanelState,
+          ...newPanelState,
+          gridData: {
+            ...previousPanelState.gridData,
+          },
+          explicitInput: {
+            ...newPanelState.explicitInput,
+            id: previousPanelState.explicitInput.id,
+          },
+        },
       },
-    };
-    this.updateInput({
-      panels: finalPanels,
-      lastReloadRequestTime: new Date().getTime(),
     });
   }
 
@@ -198,16 +193,16 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
     E extends IEmbeddable<EEI, EEO> = IEmbeddable<EEI, EEO>
   >(type: string, explicitInput: Partial<EEI>) {
     if (explicitInput.id && this.input.panels[explicitInput.id]) {
-      this.replacePanel(this.input.panels[explicitInput.id], {
+      return this.replacePanel(this.input.panels[explicitInput.id], {
         type,
         explicitInput: {
           ...explicitInput,
-          id: uuid.v4(),
+          // TS does not catch up with the typeguard above, so it needs to be explicit
+          id: explicitInput.id,
         },
       });
-    } else {
-      this.addNewEmbeddable<EEI, EEO, E>(type, explicitInput);
     }
+    return this.addNewEmbeddable<EEI, EEO, E>(type, explicitInput);
   }
 
   public render(dom: HTMLElement) {

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

@@ -265,13 +265,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/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>(
@@ -328,16 +334,24 @@ 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) {
+          this.onPanelRemoved(id);
+          this.onPanelAdded(currentPanels[id]);
+        }
       }
     });
   }