Merge branch 'master' into implement/ui-stateful-deps

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

@@ -36,6 +36,7 @@ import {
   IndexPattern,
   IndexPatternsContract,
   Query,
+  QueryState,
   SavedQuery,
   syncQueryStateWithUrl,
 } from '../../../../../../plugins/data/public';
@@ -132,13 +133,6 @@ export class DashboardAppController {
     const queryFilter = filterManager;
     const timefilter = queryService.timefilter.timefilter;
 
-    // starts syncing `_g` portion of url with query services
-    // note: dashboard_state_manager.ts syncs `_a` portion of url
-    const {
-      stop: stopSyncingQueryServiceStateWithUrl,
-      hasInheritedQueryFromUrl: hasInheritedGlobalStateFromUrl,
-    } = syncQueryStateWithUrl(queryService, kbnUrlStateStorage);
-
     let lastReloadRequestTime = 0;
     const dash = ($scope.dash = $route.current.locals.dash);
     if (dash.id) {
@@ -170,9 +164,24 @@ export class DashboardAppController {
 
     // The hash check is so we only update the time filter on dashboard open, not during
     // normal cross app navigation.
-    if (dashboardStateManager.getIsTimeSavedWithDashboard() && !hasInheritedGlobalStateFromUrl) {
-      dashboardStateManager.syncTimefilterWithDashboard(timefilter);
+    if (dashboardStateManager.getIsTimeSavedWithDashboard()) {
+      const initialGlobalStateInUrl = kbnUrlStateStorage.get<QueryState>('_g');
+      if (!initialGlobalStateInUrl?.time) {
+        dashboardStateManager.syncTimefilterWithDashboardTime(timefilter);
+      }
+      if (!initialGlobalStateInUrl?.refreshInterval) {
+        dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter);
+      }
     }
+
+    // starts syncing `_g` portion of url with query services
+    // note: dashboard_state_manager.ts syncs `_a` portion of url
+    // it is important to start this syncing after `dashboardStateManager.syncTimefilterWithDashboard(timefilter);` above is run,
+    // otherwise it will case redundant browser history record
+    const { stop: stopSyncingQueryServiceStateWithUrl } = syncQueryStateWithUrl(
+      queryService,
+      kbnUrlStateStorage
+    );
     $scope.showSaveQuery = dashboardCapabilities.saveQuery as boolean;
 
     const getShouldShowEditHelp = () =>
@@ -652,26 +661,21 @@ export class DashboardAppController {
         // This is only necessary for new dashboards, which will default to Edit mode.
         updateViewMode(ViewMode.VIEW);
 
+        // 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.syncTimefilterWithDashboardTime(timefilter);
+          dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter);
+        }
+
         // 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
         // so have to use implementation of dashboardStateManager.changeDashboardUrl, which workarounds those issues
         dashboardStateManager.changeDashboardUrl(
           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()) {
-          // 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);
-          });
-        }
       }
 
       overlays

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts

@@ -59,7 +59,7 @@ describe('DashboardState', function() {
       mockTime.to = '2015-09-29 06:31:44.000';
 
       initDashboardState();
-      dashboardState.syncTimefilterWithDashboard(mockTimefilter);
+      dashboardState.syncTimefilterWithDashboardTime(mockTimefilter);
 
       expect(mockTime.to).toBe('now/w');
       expect(mockTime.from).toBe('now/w');
@@ -74,7 +74,7 @@ describe('DashboardState', function() {
       mockTime.to = '2015-09-29 06:31:44.000';
 
       initDashboardState();
-      dashboardState.syncTimefilterWithDashboard(mockTimefilter);
+      dashboardState.syncTimefilterWithDashboardTime(mockTimefilter);
 
       expect(mockTime.to).toBe('now');
       expect(mockTime.from).toBe('now-13d');
@@ -89,7 +89,7 @@ describe('DashboardState', function() {
       mockTime.to = 'now/w';
 
       initDashboardState();
-      dashboardState.syncTimefilterWithDashboard(mockTimefilter);
+      dashboardState.syncTimefilterWithDashboardTime(mockTimefilter);
 
       expect(mockTime.to).toBe(savedDashboard.timeTo);
       expect(mockTime.from).toBe(savedDashboard.timeFrom);

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts

@@ -35,7 +35,7 @@ import {
   TimefilterContract as Timefilter,
 } from '../../../../../../plugins/data/public';
 
-import { getAppStateDefaults, migrateAppState } from './lib';
+import { getAppStateDefaults, migrateAppState, getDashboardIdFromUrl } from './lib';
 import { convertPanelStateToSavedDashboardPanel } from './lib/embeddable_saved_object_converters';
 import { FilterUtils } from './lib/filter_utils';
 import {
@@ -175,6 +175,14 @@ export class DashboardStateManager {
           // sync state required state container to be able to handle null
           // overriding set() so it could handle null coming from url
           if (state) {
+            // Skip this update if current dashboardId in the url is different from what we have in the current instance of state manager
+            // As dashboard is driven by angular at the moment, the destroy cycle happens async,
+            // If the dashboardId has changed it means this instance
+            // is going to be destroyed soon and we shouldn't sync state anymore,
+            // as it could potentially trigger further url updates
+            const currentDashboardIdInUrl = getDashboardIdFromUrl(history.location.pathname);
+            if (currentDashboardIdInUrl !== this.savedDashboard.id) return;
+
             this.stateContainer.set({
               ...this.stateDefaults,
               ...state,
@@ -203,6 +211,7 @@ export class DashboardStateManager {
 
   public handleDashboardContainerChanges(dashboardContainer: DashboardContainer) {
     let dirty = false;
+    let dirtyBecauseOfInitialStateMigration = false;
 
     const savedDashboardPanelMap: { [key: string]: SavedDashboardPanel } = {};
 
@@ -236,11 +245,20 @@ export class DashboardStateManager {
       ) {
         // A panel was changed
         dirty = true;
+
+        const oldVersion = savedDashboardPanelMap[panelState.explicitInput.id]?.version;
+        const newVersion = convertedPanelStateMap[panelState.explicitInput.id]?.version;
+        if (oldVersion && newVersion && oldVersion !== newVersion) {
+          dirtyBecauseOfInitialStateMigration = true;
+        }
       }
     });
 
     if (dirty) {
       this.stateContainer.transitions.set('panels', Object.values(convertedPanelStateMap));
+      if (dirtyBecauseOfInitialStateMigration) {
+        this.saveState({ replace: true });
+      }
     }
 
     if (input.isFullScreenMode !== this.getFullScreenMode()) {
@@ -498,7 +516,7 @@ export class DashboardStateManager {
    * @param timeFilter.setTime
    * @param timeFilter.setRefreshInterval
    */
-  public syncTimefilterWithDashboard(timeFilter: Timefilter) {
+  public syncTimefilterWithDashboardTime(timeFilter: Timefilter) {
     if (!this.getIsTimeSavedWithDashboard()) {
       throw new Error(
         i18n.translate('kbn.dashboard.stateManager.timeNotSavedWithDashboardErrorMessage', {
@@ -513,6 +531,20 @@ export class DashboardStateManager {
         to: this.savedDashboard.timeTo,
       });
     }
+  }
+
+  /**
+   * Updates timeFilter to match the refreshInterval saved with the dashboard.
+   * @param timeFilter
+   */
+  public syncTimefilterWithDashboardRefreshInterval(timeFilter: Timefilter) {
+    if (!this.getIsTimeSavedWithDashboard()) {
+      throw new Error(
+        i18n.translate('kbn.dashboard.stateManager.timeNotSavedWithDashboardErrorMessage', {
+          defaultMessage: 'The time is not saved with this dashboard so should not be synced.',
+        })
+      );
+    }
 
     if (this.savedDashboard.refreshInterval) {
       timeFilter.setRefreshInterval(this.savedDashboard.refreshInterval);

src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/index.ts

@@ -20,3 +20,4 @@
 export { saveDashboard } from './save_dashboard';
 export { getAppStateDefaults } from './get_app_state_defaults';
 export { migrateAppState } from './migrate_app_state';
+export { getDashboardIdFromUrl } from './url';

src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts

@@ -0,0 +1,46 @@
+/*
+ * 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 { getDashboardIdFromUrl } from './url';
+
+test('getDashboardIdFromUrl', () => {
+  let url =
+    "http://localhost:5601/wev/app/kibana#/dashboard?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))&_a=(description:'',filters:!()";
+  expect(getDashboardIdFromUrl(url)).toEqual(undefined);
+
+  url =
+    "http://localhost:5601/wev/app/kibana#/dashboard/625357282?_a=(description:'',filters:!()&_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))";
+  expect(getDashboardIdFromUrl(url)).toEqual('625357282');
+
+  url = 'http://myserver.mydomain.com:5601/wev/app/kibana#/dashboard/777182';
+  expect(getDashboardIdFromUrl(url)).toEqual('777182');
+
+  url =
+    "http://localhost:5601/app/kibana#/dashboard?_g=(refreshInterval:(pause:!t,value:0),time:(from:now-15m,to:now))&_a=(description:'',filters:!()";
+  expect(getDashboardIdFromUrl(url)).toEqual(undefined);
+
+  url = '/dashboard/test/?_g=(refreshInterval:';
+  expect(getDashboardIdFromUrl(url)).toEqual('test');
+
+  url = 'dashboard/test/?_g=(refreshInterval:';
+  expect(getDashboardIdFromUrl(url)).toEqual('test');
+
+  url = '/other-app/test/';
+  expect(getDashboardIdFromUrl(url)).toEqual(undefined);
+});

src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts

@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+/**
+ * Returns dashboard id from URL
+ * literally looks from id after `dashboard/` string and before `/`, `?` and end of string
+ * @param url to extract dashboardId from
+ * input: http://localhost:5601/lib/app/kibana#/dashboard?param1=x&param2=y&param3=z
+ * output: undefined
+ * input: http://localhost:5601/lib/app/kibana#/dashboard/39292992?param1=x&param2=y&param3=z
+ * output: 39292992
+ */
+export function getDashboardIdFromUrl(url: string): string | undefined {
+  const [, dashboardId] = url.match(/dashboard\/(.*?)(\/|\?|$)/) ?? [
+    undefined, // full match
+    undefined, // group with dashboardId
+  ];
+  return dashboardId ?? undefined;
+}

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.test.ts

@@ -76,3 +76,30 @@ describe('Test discover state', () => {
     expect(state.getPreviousAppState()).toEqual(stateA);
   });
 });
+
+describe('Test discover state with legacy migration', () => {
+  test('migration of legacy query ', async () => {
+    history = createBrowserHistory();
+    history.push(
+      "/#?_a=(query:(query_string:(analyze_wildcard:!t,query:'type:nice%20name:%22yeah%22')))"
+    );
+    state = getState({
+      defaultAppState: { index: 'test' },
+      history,
+    });
+    expect(state.appStateContainer.getState()).toMatchInlineSnapshot(`
+      Object {
+        "index": "test",
+        "query": Object {
+          "language": "lucene",
+          "query": Object {
+            "query_string": Object {
+              "analyze_wildcard": true,
+              "query": "type:nice name:\\"yeah\\"",
+            },
+          },
+        },
+      }
+    `);
+  });
+});

src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover_state.ts

@@ -129,6 +129,11 @@ export function getState({
   });
 
   const appStateFromUrl = stateStorage.get(APP_STATE_URL_KEY) as AppState;
+
+  if (appStateFromUrl && appStateFromUrl.query && !appStateFromUrl.query.language) {
+    appStateFromUrl.query = migrateLegacyQuery(appStateFromUrl.query);
+  }
+
   let initialAppState = {
     ...defaultAppState,
     ...appStateFromUrl,
@@ -179,9 +184,6 @@ export function setState(stateContainer: ReduxLikeStateContainer<AppState>, newS
   const oldState = stateContainer.getState();
   const mergedState = { ...oldState, ...newState };
   if (!isEqualState(oldState, mergedState)) {
-    if (mergedState.query) {
-      mergedState.query = migrateLegacyQuery(mergedState.query);
-    }
     stateContainer.set(mergedState);
   }
 }