Fix embeddable config url updates (#15728) (#17021)

* add failing test that would have caught this situation

* Implement onEmbeddableConfigChanged to pick up url changes from soft refreshes

* Add tests for legend and pie slice color as well as removing uiState keys to ensure it resets the config values

* address removing state from url as well as setting it.

* Fix up tests and add a link to the legend issue

* only emit once

* use for in, use better mocked clearAllKeys function

* Remove switch to setSilent

* Adjust test so it fails because of bug found where two charts will both have their slice colors updated

* Embeddable should only listen for changes to their panel's config state

* Default embeddable config to an empty object when creating a new embeddable

src/core_plugins/kibana/public/dashboard/dashboard_container_api.js

@@ -23,4 +23,8 @@ export class DashboardContainerAPI extends ContainerAPI {
   getHidePanelTitles() {
     return this.dashboardState.getHidePanelTitles();
   }
+
+  onEmbeddableConfigChanged(panelIndex, listener) {
+    this.dashboardState.registerEmbeddableConfigChangeListener(panelIndex, listener);
+  }
 }

src/core_plugins/kibana/public/dashboard/dashboard_state_manager.js

@@ -97,6 +97,7 @@ export class DashboardStateManager {
     store.dispatch(updateTitle(this.getTitle()));
     store.dispatch(updateDescription(this.getDescription()));
 
+    this.embeddableConfigChangeListeners = {};
     this.changeListeners = [];
 
     this.unsubscribe = store.subscribe(() => this._handleStoreChanges());
@@ -106,6 +107,14 @@ export class DashboardStateManager {
     });
   }
 
+  registerEmbeddableConfigChangeListener(panelIndex, callback) {
+    let panelListeners = this.embeddableConfigChangeListeners[panelIndex];
+    if (!panelListeners) {
+      panelListeners = this.embeddableConfigChangeListeners[panelIndex] = [];
+    }
+    panelListeners.push(callback);
+  }
+
   registerChangeListener(callback) {
     this.changeListeners.push(callback);
   }
@@ -126,6 +135,29 @@ export class DashboardStateManager {
     return !differencesFound;
   }
 
+  /**
+   * For each embeddable config in appState that differs from that in the redux store, trigger the change listeners
+   * using the appState version as the "source of truth". This is because currently the only way to update an embeddable
+   * config from the dashboard side is via the url. Eventually we want to let users modify it via a "reset link" in
+   * the panel config, or even a way to modify it in the panel config. When this is introduced it would go through
+   * redux and we'd have to update appState. At that point, we'll need to handle changes coming from both directions.
+   * Ideally we can introduce react-redux-router for a more seamless way to keep url changes and ui changes in sync.
+   * ... until then... we are stuck with this manual crap. :(
+   * Fixes https://github.com/elastic/kibana/issues/15720
+   */
+  triggerEmbeddableConfigUpdateListeners() {
+    const state = store.getState();
+    for(const appStatePanel of this.appState.panels) {
+      const storePanel = getPanel(state, appStatePanel.panelIndex);
+      if (storePanel && !_.isEqual(appStatePanel.embeddableConfig, storePanel.embeddableConfig)) {
+        const panelListeners = this.embeddableConfigChangeListeners[appStatePanel.panelIndex];
+        if (panelListeners) {
+          panelListeners.forEach(listener => listener(appStatePanel.embeddableConfig));
+        }
+      }
+    }
+  }
+
   /**
    * Changes made to app state outside of direct calls to this class will need to be propagated to the store.
    * @private
@@ -134,6 +166,7 @@ export class DashboardStateManager {
     // We need these checks, or you can get into a loop where a change is triggered by the store, which updates
     // AppState, which then dispatches the change here, which will end up triggering setState warnings.
     if (!this._areStoreAndAppStatePanelsEqual()) {
+      this.triggerEmbeddableConfigUpdateListeners();
       store.dispatch(setPanels(this.getPanels()));
     }
 

src/core_plugins/kibana/public/dashboard/panel/panel_state.js

@@ -98,7 +98,8 @@ export function createPanelState(id, type, panelIndex, currentPanels) {
     version: chrome.getKibanaVersion(),
     panelIndex: panelIndex.toString(),
     type: type,
-    id: id
+    id: id,
+    embeddableConfig: {},
   };
 }
 

src/core_plugins/kibana/public/visualize/embeddable/visualize_embeddable_factory.js

@@ -53,6 +53,13 @@ export class VisualizeEmbeddableFactory extends EmbeddableFactory {
           ...parsedUiState,
           ...panel.embeddableConfig,
         });
+        container.onEmbeddableConfigChanged(panel.panelIndex, newEmbeddableConfig => {
+          uiState.clearAllKeys();
+          Object.getOwnPropertyNames(newEmbeddableConfig).forEach(key => {
+            uiState.set(key, newEmbeddableConfig[key]);
+          });
+        });
+
         const uiStateChangeHandler = () => {
           panel = container.updatePanel(
             panel.panelIndex,

src/fixtures/mock_ui_state.js

@@ -15,6 +15,9 @@ export default {
   emit: _.noop,
   on: _.noop,
   off: _.noop,
+  clearAllKeys: function () {
+    values = {};
+  },
   _reset: function () {
     values = {};
   }

src/ui/public/persisted_state/persisted_state.js

@@ -86,6 +86,12 @@ export class PersistedState {
     return this._set(params.key, params.value, true);
   }
 
+  clearAllKeys() {
+    Object.getOwnPropertyNames(this._changedState).forEach(key => {
+      this.set(key, null);
+    });
+  }
+
   reset(path) {
     const keyPath = this._getIndex(path);
     const origValue = _.get(this._defaultState, keyPath);

src/ui/public/vislib/visualizations/pie_chart.js

@@ -176,6 +176,11 @@ export function VislibVisualizationsPieChartProvider(Private) {
           if (d.depth > maxDepth) maxDepth = d.depth;
           return 'slice';
         })
+        .attr('data-test-subj', function (d) {
+          if (d.name) {
+            return `pieSlice-${d.name.split(' ').join('-')}`;
+          }
+        })
         .call(self._addIdentifier, 'name')
         .style('fill', function (d) {
           if (d.depth === 0) {

test/functional/apps/dashboard/_dashboard_state.js

@@ -58,6 +58,59 @@ export default function ({ getService, getPageObjects }) {
           expect(newPanelDimensions[0].width).to.be.greaterThan(currentPanelDimensions[0].width * 2 - marginOfError);
         });
       });
+
+      describe('for embeddable config color parameters on a visualization', () => {
+        it('updates a pie slice color on a soft refresh', async function () {
+          await PageObjects.dashboard.addVisualization(PIE_CHART_VIS_NAME);
+          await PageObjects.visualize.clickLegendOption('80,000');
+          await PageObjects.visualize.selectNewLegendColorChoice('#F9D9F9');
+          const currentUrl = await remote.getCurrentUrl();
+          const newUrl = currentUrl.replace('F9D9F9', 'FFFFFF');
+          await remote.get(newUrl.toString(), false);
+          await PageObjects.header.waitUntilLoadingHasFinished();
+
+          await retry.try(async () => {
+            const allPieSlicesColor = await PageObjects.visualize.getAllPieSliceStyles('80,000');
+            let whitePieSliceCounts = 0;
+            allPieSlicesColor.forEach(style => {
+              if (style.indexOf('rgb(255, 255, 255)') > 0) {
+                whitePieSliceCounts++;
+              }
+            });
+
+            expect(whitePieSliceCounts).to.be(1);
+          });
+        });
+
+        // Unskip once https://github.com/elastic/kibana/issues/15736 is fixed.
+        it.skip('and updates the pie slice legend color', async function () {
+          await retry.try(async () => {
+            const colorExists = await PageObjects.visualize.doesSelectedLegendColorExist('#FFFFFF');
+            expect(colorExists).to.be(true);
+          });
+        });
+
+        it('resets a pie slice color to the original when removed', async function () {
+          const currentUrl = await remote.getCurrentUrl();
+          const newUrl = currentUrl.replace('vis:(colors:(%2780,000%27:%23FFFFFF))', '');
+          await remote.get(newUrl.toString(), false);
+          await PageObjects.header.waitUntilLoadingHasFinished();
+
+          await retry.try(async () => {
+            const pieSliceStyle = await PageObjects.visualize.getPieSliceStyle('80,000');
+            // The default green color that was stored with the visualization before any dashboard overrides.
+            expect(pieSliceStyle.indexOf('rgb(87, 193, 123)')).to.be.greaterThan(0);
+          });
+        });
+
+        // Unskip once https://github.com/elastic/kibana/issues/15736 is fixed.
+        it.skip('resets the legend color as well', async function () {
+          await retry.try(async () => {
+            const colorExists = await PageObjects.visualize.doesSelectedLegendColorExist('#57c17b');
+            expect(colorExists).to.be(true);
+          });
+        });
+      });
     });
 
     it('Overriding colors on an area chart is preserved', async () => {

test/functional/page_objects/visualize_page.js

@@ -798,6 +798,26 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
     async doesSelectedLegendColorExist(color) {
       return await testSubjects.exists(`legendSelectedColor-${color}`);
     }
+
+    async getPieSlice(name) {
+      return await testSubjects.find(`pieSlice-${name.split(' ').join('-')}`);
+    }
+
+    async getAllPieSlices(name) {
+      return await testSubjects.findAll(`pieSlice-${name.split(' ').join('-')}`);
+    }
+
+    async getPieSliceStyle(name) {
+      log.debug(`VisualizePage.getPieSliceStyle(${name})`);
+      const pieSlice = await this.getPieSlice(name);
+      return await pieSlice.getAttribute('style');
+    }
+
+    async getAllPieSliceStyles(name) {
+      log.debug(`VisualizePage.getAllPieSliceStyles(${name})`);
+      const pieSlices = await this.getAllPieSlices(name);
+      return await Promise.all(pieSlices.map(async pieSlice => await pieSlice.getAttribute('style')));
+    }
   }
 
   return new VisualizePage();