Remove uiState from dashboard - communicate via the panelState instead (#14949)

* Remove uiState from dashboard - communicate via the panelState instead

* migrate old app state from the url

* Add comment about migration and why the unused uiStateJSON field is kept around still

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

@@ -15,10 +15,6 @@ export class DashboardContainerAPI extends ContainerAPI {
     return this.dashboardState.appState;
   }
 
-  createChildUistate(path, initialState) {
-    return this.dashboardState.uiState.createChild(path, initialState, true);
-  }
-
   registerPanelIndexPattern(panelIndex, pattern) {
     this.dashboardState.registerPanelIndexPatternMap(panelIndex, pattern);
     this.dashboardState.saveState();

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

@@ -16,8 +16,8 @@ import {
   updateHidePanelTitles,
 } from './actions';
 import { stateMonitorFactory } from 'ui/state_management/state_monitor_factory';
-import { createPanelState, getPersistedStateId } from './panel';
-import { getAppStateDefaults } from './lib';
+import { createPanelState } from './panel';
+import { getAppStateDefaults, migrateAppState } from './lib';
 import {
   getViewMode,
   getFullScreenMode,
@@ -39,7 +39,6 @@ import {
  *  - description
  *  - timeRestore
  *  - query
- *  - uiState
  *  - filters
  *
  * State that is only stored in the Store:
@@ -67,7 +66,13 @@ export class DashboardStateManager {
     this.stateDefaults = getAppStateDefaults(this.savedDashboard, this.hideWriteControls);
 
     this.appState = new AppState(this.stateDefaults);
-    this.uiState = this.appState.makeStateful('uiState');
+
+    // Initializing appState does two things - first it translates the defaults into AppState, second it updates
+    // appState based on the URL (the url trumps the defaults). This means if we update the state format at all and
+    // want to handle BWC, we must not only migrate the data stored with saved Dashboard, but also any old state in the
+    // url.
+    migrateAppState(this.appState);
+
     this.isDirty = false;
 
     // We can't compare the filters stored on this.appState to this.savedDashboard because in order to apply
@@ -428,7 +433,6 @@ export class DashboardStateManager {
   removePanel(panelIndex) {
     _.remove(this.getPanels(), (panel) => {
       if (panel.panelIndex === panelIndex) {
-        this.uiState.removeChild(getPersistedStateId(panel));
         delete this.panelIndexPatternMapping[panelIndex];
         return true;
       } else {

src/core_plugins/kibana/public/dashboard/lib/get_app_state_defaults.js

@@ -2,16 +2,25 @@ import { DashboardViewMode } from '../dashboard_view_mode';
 import { FilterUtils } from './filter_utils';
 
 export function getAppStateDefaults(savedDashboard, hideWriteControls) {
-  return {
+  const appState = {
     fullScreenMode: false,
     title: savedDashboard.title,
     description: savedDashboard.description,
     timeRestore: savedDashboard.timeRestore,
     panels: savedDashboard.panelsJSON ? JSON.parse(savedDashboard.panelsJSON) : [],
     options: savedDashboard.optionsJSON ? JSON.parse(savedDashboard.optionsJSON) : {},
-    uiState: savedDashboard.uiStateJSON ? JSON.parse(savedDashboard.uiStateJSON) : {},
     query: FilterUtils.getQueryFilterForDashboard(savedDashboard),
     filters: FilterUtils.getFilterBarsForDashboard(savedDashboard),
     viewMode: savedDashboard.id || hideWriteControls ? DashboardViewMode.VIEW : DashboardViewMode.EDIT,
   };
+
+  // For BWC in pre 6.1 versions where uiState was stored at the dashboard level, not at the panel level.
+  if (savedDashboard.uiStateJSON) {
+    const uiState = JSON.parse(savedDashboard.uiStateJSON);
+    appState.panels.forEach(panel => {
+      panel.embeddableConfig = uiState[`P-${panel.panelIndex}`];
+    });
+    delete savedDashboard.uiStateJSON;
+  }
+  return appState;
 }

src/core_plugins/kibana/public/dashboard/lib/index.js

@@ -1,2 +1,3 @@
 export { saveDashboard } from './save_dashboard';
 export { getAppStateDefaults } from './get_app_state_defaults';
+export { migrateAppState } from './migrate_app_state';

src/core_plugins/kibana/public/dashboard/lib/migrate_app_state.js

@@ -0,0 +1,15 @@
+/**
+ * Creates a new instance of AppState based of the saved dashboard.
+ *
+ * @param appState {AppState} AppState class to instantiate
+ */
+export function migrateAppState(appState) {
+  // For BWC in pre 6.1 versions where uiState was stored at the dashboard level, not at the panel level.
+  if (appState.uiState) {
+    appState.panels.forEach(panel => {
+      panel.embeddableConfig = appState.uiState[`P-${panel.panelIndex}`];
+    });
+    delete appState.uiState;
+    appState.save();
+  }
+}

src/core_plugins/kibana/public/dashboard/lib/save_dashboard.js

@@ -16,7 +16,7 @@ export function saveDashboard(toJson, timeFilter, dashboardStateManager) {
   const savedDashboard = dashboardStateManager.savedDashboard;
   const appState = dashboardStateManager.appState;
 
-  updateSavedDashboard(savedDashboard, appState, dashboardStateManager.uiState, timeFilter, toJson);
+  updateSavedDashboard(savedDashboard, appState, timeFilter, toJson);
 
   return savedDashboard.save()
     .then((id) => {

src/core_plugins/kibana/public/dashboard/lib/update_saved_dashboard.js

@@ -1,12 +1,11 @@
 import _ from 'lodash';
 import { FilterUtils } from './filter_utils';
 
-export function updateSavedDashboard(savedDashboard, appState, uiState, timeFilter, toJson) {
+export function updateSavedDashboard(savedDashboard, appState, timeFilter, toJson) {
   savedDashboard.title = appState.title;
   savedDashboard.description = appState.description;
   savedDashboard.timeRestore = appState.timeRestore;
   savedDashboard.panelsJSON = toJson(appState.panels);
-  savedDashboard.uiStateJSON = toJson(uiState.getChanges());
   savedDashboard.optionsJSON = toJson(appState.options);
 
   savedDashboard.timeFrom = savedDashboard.timeRestore ?

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

@@ -1,2 +1,2 @@
 export { DashboardPanelContainer as DashboardPanel } from './dashboard_panel_container';
-export { createPanelState, getPersistedStateId } from './panel_state';
+export { createPanelState } from './panel_state';

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

@@ -102,11 +102,3 @@ export function createPanelState(id, type, panelIndex, currentPanels) {
   };
 }
 
-/**
- * Returns a unique id for storing the panel state in the persistent ui.
- * @param {PanelState} panel
- * @returns {string}
- */
-export function getPersistedStateId(panel) {
-  return `P-${panel.panelIndex}`;
-}

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

@@ -30,7 +30,6 @@ module.factory('SavedDashboard', function (courier, config) {
           useMargins: id ? false : true,
           hidePanelTitles: false,
         }),
-        uiStateJSON: '{}',
         version: 1,
         timeRestore: false,
         timeTo: undefined,
@@ -54,6 +53,9 @@ module.factory('SavedDashboard', function (courier, config) {
     description: 'text',
     panelsJSON: 'text',
     optionsJSON: 'text',
+    // Note: this field is no longer used for dashboards created or saved in version 6.2 onward.  We keep it around
+    // due to BWC, until we can ensure a migration step for all old dashboards saved in an index, as well as
+    // migration steps for importing.  See https://github.com/elastic/kibana/issues/15204 for more info.
     uiStateJSON: 'text',
     version: 'integer',
     timeRestore: 'boolean',

src/core_plugins/kibana/public/discover/embeddable/search_embeddable_factory.js

@@ -3,7 +3,7 @@ import angular from 'angular';
 import 'ui/doc_table';
 
 import * as columnActions from 'ui/doc_table/actions/columns';
-import { getPersistedStateId } from 'plugins/kibana/dashboard/panel/panel_state';
+import { PersistedState } from 'ui/persisted_state';
 import { EmbeddableFactory, Embeddable } from 'ui/embeddable';
 
 export class SearchEmbeddableFactory extends EmbeddableFactory {
@@ -43,8 +43,18 @@ export class SearchEmbeddableFactory extends EmbeddableFactory {
         searchScope.columns = searchScope.panel.columns || searchScope.savedObj.columns;
         searchScope.sort = searchScope.panel.sort || searchScope.savedObj.sort;
 
-        const uiState = savedObject.uiStateJSON ? JSON.parse(savedObject.uiStateJSON) : {};
-        searchScope.uiState = container.createChildUistate(getPersistedStateId(panel), uiState);
+        const parsedUiState = savedObject.uiStateJSON ? JSON.parse(savedObject.uiStateJSON) : {};
+        searchScope.uiState = new PersistedState({
+          ...parsedUiState,
+          ...panel.embeddableConfig,
+        });
+        const uiStateChangeHandler = () => {
+          searchScope.panel = container.updatePanel(
+            searchScope.panel.panelIndex,
+            { embeddableConfig: searchScope.uiState.toJSON() }
+          );
+        };
+        searchScope.uiState.on('change', uiStateChangeHandler);
 
         searchScope.setSortOrder = function setSortOrder(columnName, direction) {
           searchScope.panel = container.updatePanel(searchScope.panel.panelIndex, { sort: [columnName, direction] });
@@ -78,6 +88,7 @@ export class SearchEmbeddableFactory extends EmbeddableFactory {
         rootNode.append(searchInstance);
 
         this.addDestroyEmeddable(panel.panelIndex, () => {
+          searchScope.uiState.off('change', uiStateChangeHandler);
           searchInstance.remove();
           searchScope.savedObj.destroy();
           searchScope.$destroy();

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

@@ -2,10 +2,10 @@ import angular from 'angular';
 import 'ui/visualize';
 
 import visualizationTemplate from './visualize_template.html';
-import { getPersistedStateId } from 'plugins/kibana/dashboard/panel/panel_state';
 import { UtilsBrushEventProvider as utilsBrushEventProvider } from 'ui/utils/brush_event';
 import { FilterBarClickHandlerProvider as filterBarClickHandlerProvider } from 'ui/filter_bar/filter_bar_click_handler';
 import { EmbeddableFactory, Embeddable } from 'ui/embeddable';
+import { PersistedState } from 'ui/persisted_state';
 
 import chrome from 'ui/chrome';
 
@@ -52,8 +52,18 @@ export class VisualizeEmbeddableFactory extends EmbeddableFactory {
         visualizeScope.savedObj = savedObject;
         visualizeScope.panel = panel;
 
-        const uiState = savedObject.uiStateJSON ? JSON.parse(savedObject.uiStateJSON) : {};
-        visualizeScope.uiState = container.createChildUistate(getPersistedStateId(panel), uiState);
+        const parsedUiState = savedObject.uiStateJSON ? JSON.parse(savedObject.uiStateJSON) : {};
+        visualizeScope.uiState = new PersistedState({
+          ...parsedUiState,
+          ...panel.embeddableConfig,
+        });
+        const uiStateChangeHandler = () => {
+          visualizeScope.panel = container.updatePanel(
+            visualizeScope.panel.panelIndex,
+            { embeddableConfig: visualizeScope.uiState.toJSON() }
+          );
+        };
+        visualizeScope.uiState.on('change', uiStateChangeHandler);
 
         visualizeScope.savedObj.vis.setUiState(visualizeScope.uiState);
 
@@ -71,6 +81,7 @@ export class VisualizeEmbeddableFactory extends EmbeddableFactory {
         rootNode.append(visualizationInstance);
 
         this.addDestroyEmeddable(panel.panelIndex, () => {
+          visualizeScope.uiState.off('change', uiStateChangeHandler);
           visualizationInstance.remove();
           visualizeScope.savedObj.destroy();
           visualizeScope.$destroy();

src/ui/public/embeddable/container_api.js

@@ -20,17 +20,6 @@ export class ContainerAPI {
     throw new Error('Must implement getAppState.');
   }
 
-  /**
-   * Creates a new state for the panel. It's passed the ui state object to use, and is returned
-   * a PersistedState.
-   * @param path {String} - the unique path for this ui state.
-   * @param initialState {Object} - the initial state to use for the child.
-   * @returns {PersistedState}
-   */
-  createChildUistate(/* path, initialState */) {
-    throw new Error('Must implement getInitalState.');
-  }
-
   /**
    * Call this to tell the container that this panel uses a particular index pattern.
    * @param {string} panelIndex - a unique id that identifies the panel to update.

test/functional/apps/dashboard/_dashboard_state.js

@@ -1,6 +1,6 @@
 import expect from 'expect.js';
 
-import { PIE_CHART_VIS_NAME } from '../../page_objects/dashboard_page';
+import { PIE_CHART_VIS_NAME, AREA_CHART_VIS_NAME } from '../../page_objects/dashboard_page';
 import {
   DEFAULT_PANEL_WIDTH,
 } from '../../../../src/core_plugins/kibana/public/dashboard/dashboard_constants';
@@ -65,7 +65,31 @@ export default function ({ getService, getPageObjects }) {
       });
     });
 
+    it('Overriding colors on an area chart is preserved', async () => {
+      await PageObjects.dashboard.gotoDashboardLandingPage();
+
+      await PageObjects.dashboard.clickNewDashboard();
+      await PageObjects.dashboard.setTimepickerInDataRange();
+
+      await PageObjects.dashboard.addVisualizations([AREA_CHART_VIS_NAME]);
+      await PageObjects.dashboard.saveDashboard('Overridden colors');
+      await PageObjects.header.clickToastOK();
+
+      await PageObjects.dashboard.clickEdit();
+      await PageObjects.visualize.clickLegendOption('Count');
+      await PageObjects.visualize.selectNewLegendColorChoice('#EA6460');
+      await PageObjects.dashboard.saveDashboard('Overridden colors');
+
+      await PageObjects.dashboard.gotoDashboardLandingPage();
+      await PageObjects.dashboard.loadSavedDashboard('Overridden colors');
+      const colorChoiceRetained = await PageObjects.visualize.doesSelectedLegendColorExist('#EA6460');
+
+      expect(colorChoiceRetained).to.be(true);
+    });
+
     it('Saved search with no changes will update when the saved object changes', async () => {
+      await PageObjects.dashboard.gotoDashboardLandingPage();
+
       await PageObjects.header.clickDiscover();
       await PageObjects.dashboard.setTimepickerInDataRange();
       await PageObjects.discover.clickFieldListItemAdd('bytes');

test/functional/page_objects/dashboard_page.js

@@ -3,6 +3,7 @@ import _ from 'lodash';
 import { DashboardConstants } from '../../../src/core_plugins/kibana/public/dashboard/dashboard_constants';
 
 export const PIE_CHART_VIS_NAME = 'Visualization PieChart';
+export const AREA_CHART_VIS_NAME = 'Visualization漢字 AreaChart';
 
 export function DashboardPageProvider({ getService, getPageObjects }) {
   const log = getService('log');
@@ -452,7 +453,7 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
       return [
         { name: PIE_CHART_VIS_NAME, description: 'PieChart' },
         { name: 'Visualization☺ VerticalBarChart', description: 'VerticalBarChart' },
-        { name: 'Visualization漢字 AreaChart', description: 'AreaChart' },
+        { name: AREA_CHART_VIS_NAME, description: 'AreaChart' },
         { name: 'Visualization☺漢字 DataTable', description: 'DataTable' },
         { name: 'Visualization漢字 LineChart', description: 'LineChart' },
         { name: 'Visualization TileMap', description: 'TileMap' },

test/functional/page_objects/visualize_page.js

@@ -724,6 +724,18 @@ export function VisualizePageProvider({ getService, getPageObjects }) {
         });
       }
     }
+
+    async clickLegendOption(name) {
+      await testSubjects.click(`legend-${name}`);
+    }
+
+    async selectNewLegendColorChoice(color) {
+      await testSubjects.click(`legendSelectColor-${color}`);
+    }
+
+    async doesSelectedLegendColorExist(color) {
+      return await testSubjects.exists(`legendSelectedColor-${color}`);
+    }
   }
 
   return new VisualizePage();