[State Management] State syncing utilities - remove AppState from Dashboard app

src/legacy/core_plugins/data/public/filter/filter_manager/filter_state_manager.ts

@@ -18,17 +18,20 @@
  */
 
 import _ from 'lodash';
+import { Subscription } from 'rxjs';
 import { State } from 'ui/state_management/state';
 import { FilterManager, esFilters } from '../../../../../../plugins/data/public';
 
-type GetAppStateFunc = () => State | undefined | null;
+type GetAppStateFunc = () => { filters?: esFilters.Filter[]; save?: () => void } | undefined | null;
 
 /**
  * FilterStateManager is responsible for watching for filter changes
  * and syncing with FilterManager, as well as syncing FilterManager changes
  * back to the URL.
  **/
 export class FilterStateManager {
+  private subs: Subscription[] = [];
+
   filterManager: FilterManager;
   globalState: State;
   getAppState: GetAppStateFunc;
@@ -41,15 +44,18 @@ export class FilterStateManager {
 
     this.watchFilterState();
 
-    this.filterManager.getUpdates$().subscribe(() => {
-      this.updateAppState();
-    });
+    this.subs.push(
+      this.filterManager.getUpdates$().subscribe(() => {
+        this.updateAppState();
+      })
+    );
   }
 
   destroy() {
     if (this.interval) {
       clearInterval(this.interval);
     }
+    this.subs.forEach(s => s.unsubscribe());
   }
 
   private watchFilterState() {
@@ -80,7 +86,7 @@ export class FilterStateManager {
 
   private saveState() {
     const appState = this.getAppState();
-    if (appState) appState.save();
+    if (appState && appState.save) appState.save();
     this.globalState.save();
   }
 

src/legacy/core_plugins/kibana/public/dashboard/__tests__/index.ts

@@ -17,6 +17,5 @@
  * under the License.
  */
 
-export { getAppStateMock } from './get_app_state_mock';
 export { getSavedDashboardMock } from './get_saved_dashboard_mock';
 export { getEmbeddableFactoryMock } from './get_embeddable_factories_mock';

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app.tsx

@@ -20,12 +20,7 @@
 import moment from 'moment';
 import { Subscription } from 'rxjs';
 
-import {
-  AppStateClass as TAppStateClass,
-  AppState as TAppState,
-  IInjector,
-  KbnUrl,
-} from '../legacy_imports';
+import { IInjector } from '../legacy_imports';
 
 import { ViewMode } from '../../../../embeddable_api/public/np_ready/public';
 import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';
@@ -43,7 +38,7 @@ import { RenderDeps } from './application';
 
 export interface DashboardAppScope extends ng.IScope {
   dash: SavedObjectDashboard;
-  appState: TAppState;
+  appState: DashboardAppState;
   screenTitle: string;
   model: {
     query: Query;
@@ -60,7 +55,6 @@ export interface DashboardAppScope extends ng.IScope {
   refreshInterval: any;
   panels: SavedDashboardPanel[];
   indexPatterns: IIndexPattern[];
-  $evalAsync: any;
   dashboardViewMode: ViewMode;
   expandedPanel?: string;
   getShouldShowEditHelp: () => boolean;
@@ -91,8 +85,6 @@ export interface DashboardAppScope extends ng.IScope {
 
 export function initDashboardAppDirective(app: any, deps: RenderDeps) {
   app.directive('dashboardApp', function($injector: IInjector) {
-    const AppState = $injector.get<TAppStateClass<DashboardAppState>>('AppState');
-    const kbnUrl = $injector.get<KbnUrl>('kbnUrl');
     const confirmModal = $injector.get<ConfirmModalFn>('confirmModal');
     const config = deps.uiSettings;
 
@@ -105,17 +97,13 @@ export function initDashboardAppDirective(app: any, deps: RenderDeps) {
         $routeParams: {
           id?: string;
         },
-        getAppState: any,
         globalState: any
       ) =>
         new DashboardAppController({
           $route,
           $scope,
           $routeParams,
-          getAppState,
           globalState,
-          kbnUrl,
-          AppStateClass: AppState,
           config,
           confirmModal,
           indexPatterns: deps.npDataStart.indexPatterns,

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

@@ -17,57 +17,56 @@
  * under the License.
  */
 
-import _ from 'lodash';
+import _, { uniq } from 'lodash';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
 import angular from 'angular';
-import { uniq } from 'lodash';
 
 import { Subscription } from 'rxjs';
+import { createHashHistory } from 'history';
 import { DashboardEmptyScreen, DashboardEmptyScreenProps } from './dashboard_empty_screen';
 
 import {
-  subscribeWithScope,
   ConfirmationButtonTypes,
-  showSaveModal,
-  SaveResult,
   migrateLegacyQuery,
-  State,
-  AppStateClass as TAppStateClass,
-  KbnUrl,
   SavedObjectSaveOpts,
+  SaveResult,
+  showSaveModal,
+  State,
+  subscribeWithScope,
   unhashUrl,
 } from '../legacy_imports';
 import { FilterStateManager } from '../../../../data/public';
 import {
+  esFilters,
   IndexPattern,
+  IndexPatternsContract,
   Query,
   SavedQuery,
-  IndexPatternsContract,
 } from '../../../../../../plugins/data/public';
 
 import {
-  DashboardContainer,
   DASHBOARD_CONTAINER_TYPE,
+  DashboardContainer,
   DashboardContainerFactory,
   DashboardContainerInput,
   DashboardPanelState,
 } from '../../../../dashboard_embeddable_container/public/np_ready/public';
 import {
-  isErrorEmbeddable,
+  EmbeddableFactoryNotFoundError,
   ErrorEmbeddable,
-  ViewMode,
+  isErrorEmbeddable,
   openAddPanelFlyout,
-  EmbeddableFactoryNotFoundError,
+  ViewMode,
 } from '../../../../embeddable_api/public/np_ready/public';
-import { DashboardAppState, NavAction, ConfirmModalFn, SavedDashboardPanel } from './types';
+import { ConfirmModalFn, NavAction, SavedDashboardPanel } from './types';
 
 import { showOptionsPopover } from './top_nav/show_options_popover';
 import { DashboardSaveModal } from './top_nav/save_modal';
 import { showCloneModal } from './top_nav/show_clone_modal';
 import { saveDashboard } from './lib';
 import { DashboardStateManager } from './dashboard_state_manager';
-import { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
+import { createDashboardEditUrl, DashboardConstants } from './dashboard_constants';
 import { getTopNavConfig } from './top_nav/get_top_nav_config';
 import { TopNavIds } from './top_nav/top_nav_ids';
 import { getDashboardTitle } from './dashboard_strings';
@@ -78,17 +77,15 @@ import {
   SavedObjectFinderProps,
   SavedObjectFinderUi,
 } from '../../../../../../plugins/kibana_react/public';
+import { removeQueryParam } from '../../../../../../plugins/kibana_utils/public';
 
 export interface DashboardAppControllerDependencies extends RenderDeps {
   $scope: DashboardAppScope;
   $route: any;
   $routeParams: any;
-  getAppState: any;
   globalState: State;
   indexPatterns: IndexPatternsContract;
   dashboardConfig: any;
-  kbnUrl: KbnUrl;
-  AppStateClass: TAppStateClass<DashboardAppState>;
   config: any;
   confirmModal: ConfirmModalFn;
 }
@@ -103,12 +100,9 @@ export class DashboardAppController {
     $scope,
     $route,
     $routeParams,
-    getAppState,
     globalState,
     dashboardConfig,
     localStorage,
-    kbnUrl,
-    AppStateClass,
     indexPatterns,
     config,
     confirmModal,
@@ -124,7 +118,6 @@ export class DashboardAppController {
     },
     core: { notifications, overlays, chrome, injectedMetadata, uiSettings, savedObjects, http },
   }: DashboardAppControllerDependencies) {
-    new FilterStateManager(globalState, getAppState, filterManager);
     const queryFilter = filterManager;
 
     let lastReloadRequestTime = 0;
@@ -134,14 +127,30 @@ export class DashboardAppController {
       chrome.docTitle.change(dash.title);
     }
 
+    const history = createHashHistory();
     const dashboardStateManager = new DashboardStateManager({
       savedDashboard: dash,
-      AppStateClass,
+      useHashedUrl: config.get('state:storeInSessionStorage'),
       hideWriteControls: dashboardConfig.getHideWriteControls(),
       kibanaVersion: injectedMetadata.getKibanaVersion(),
+      history,
     });
 
-    $scope.appState = dashboardStateManager.getAppState();
+    const filterStateManager = new FilterStateManager(
+      globalState,
+      () => {
+        // Temporary AppState replacement
+        return {
+          set filters(_filters: esFilters.Filter[]) {
+            dashboardStateManager.setFilters(_filters);
+          },
+          get filters() {
+            return dashboardStateManager.appState.filters;
+          },
+        };
+      },
+      filterManager
+    );
 
     // The hash check is so we only update the time filter on dashboard open, not during
     // normal cross app navigation.
@@ -316,8 +325,8 @@ export class DashboardAppController {
               dirty = true;
             }
 
+            dashboardStateManager.handleDashboardContainerChanges(container);
             $scope.$evalAsync(() => {
-              dashboardStateManager.handleDashboardContainerChanges(container);
               if (dirty) {
                 updateState();
               }
@@ -337,8 +346,8 @@ export class DashboardAppController {
             const type = $routeParams[DashboardConstants.ADD_EMBEDDABLE_TYPE];
             const id = $routeParams[DashboardConstants.ADD_EMBEDDABLE_ID];
             container.addSavedObjectEmbeddable(type, id);
-            kbnUrl.removeParam(DashboardConstants.ADD_EMBEDDABLE_TYPE);
-            kbnUrl.removeParam(DashboardConstants.ADD_EMBEDDABLE_ID);
+            removeQueryParam(history, DashboardConstants.ADD_EMBEDDABLE_TYPE);
+            removeQueryParam(history, DashboardConstants.ADD_EMBEDDABLE_ID);
           }
         }
 
@@ -409,7 +418,9 @@ export class DashboardAppController {
           key
         ];
         if (!_.isEqual(containerValue, appStateValue)) {
-          (differences as { [key: string]: unknown })[key] = appStateValue;
+          // cloneDeep hack is needed, as there are multiple place, where container's input mutated,
+          // but values from appStateValue are deeply frozen, as they can't be mutated directly
+          (differences as { [key: string]: unknown })[key] = _.cloneDeep(appStateValue);
         }
       });
 
@@ -559,6 +570,22 @@ export class DashboardAppController {
       })
     );
 
+    // TODO: find nicer solution for this
+    // this function helps to make just 1 browser history update, when we imperatively changing the dashboard url
+    // It could be that there is pending *dashboardStateManager* updates, which aren't flushed yet to the url.
+    // So to prevent 2 browser updates:
+    // 1. Force flush any pending state updates (syncing state to query)
+    // 2. If url was updated, then apply path change with replace
+    function changeUrl(pathname: string) {
+      // synchronously persist current state to url with push()
+      const updated = dashboardStateManager.saveState({ replace: false });
+      // change pathname
+      history[updated ? 'replace' : 'push']({
+        ...history.location,
+        pathname,
+      });
+    }
+
     function updateViewMode(newMode: ViewMode) {
       $scope.topNavMenu = getTopNavConfig(
         newMode,
@@ -580,17 +607,24 @@ export class DashboardAppController {
 
       function revertChangesAndExitEditMode() {
         dashboardStateManager.resetState();
-        kbnUrl.change(
-          dash.id ? createDashboardEditUrl(dash.id) : DashboardConstants.CREATE_NEW_DASHBOARD_URL
-        );
         // This is only necessary for new dashboards, which will default to Edit mode.
         updateViewMode(ViewMode.VIEW);
 
+        changeUrl(
+          dash.id ? createDashboardEditUrl(dash.id) : DashboardConstants.CREATE_NEW_DASHBOARD_URL
+        );
+
         // We need to do a hard reset of the timepicker. appState will not reload like
         // it does on 'open' because it's been saved to the url and the getAppState.previouslyStored() check on
         // reload will cause it not to sync.
         if (dashboardStateManager.getIsTimeSavedWithDashboard()) {
-          dashboardStateManager.syncTimefilterWithDashboard(timefilter);
+          // have to use $evalAsync here until '_g' is migrated from $location to state sync utility ('history')
+          // When state sync utility changes url, angular's $location is missing it's own updates which happen during the same digest cycle
+          // temporary solution is to delay $location updates to next digest cycle
+          // unfortunately, these causes 2 browser history entries, but this is temporary and will be fixed after migrating '_g' to state_sync utilities
+          $scope.$evalAsync(() => {
+            dashboardStateManager.syncTimefilterWithDashboard(timefilter);
+          });
         }
       }
 
@@ -642,7 +676,12 @@ export class DashboardAppController {
             });
 
             if (dash.id !== $routeParams.id) {
-              kbnUrl.change(createDashboardEditUrl(dash.id));
+              // kbnUrl.change(createDashboardEditUrl(dash.id))
+              // Angular's $location skips this update because of history updates from syncState which happen simultaneously
+              // when calling kbnUrl.change() angular schedules url update and when angular finally starts to process it,
+              // the update is considered outdated and angular skips it
+
+              changeUrl(createDashboardEditUrl(dash.id));
             } else {
               chrome.docTitle.change(dash.lastSavedTitle);
               updateViewMode(ViewMode.VIEW);
@@ -844,6 +883,12 @@ export class DashboardAppController {
       });
     });
 
+    dashboardStateManager.registerChangeListener(() => {
+      // some angular dependant properties could have changed: e.g. view mode or full screen mode
+      // run digest cycle to be sure, we are not missing those update on UI
+      $scope.$applyAsync();
+    });
+
     $scope.$on('$destroy', () => {
       updateSubscription.unsubscribe();
       visibleSubscription.unsubscribe();
@@ -859,6 +904,9 @@ export class DashboardAppController {
       if (dashboardContainer) {
         dashboardContainer.destroy();
       }
+      if (filterStateManager) {
+        filterStateManager.destroy();
+      }
     });
   }
 }