From 71fe97c60582b0bb099ff790b02f4af6c19c1aac Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Fri, 27 Mar 2020 11:27:50 +0100 Subject: [PATCH 1/4] dashboard state fixes for drilldowns --- .../np_ready/dashboard_app_controller.tsx | 30 ++++++++++------ .../np_ready/dashboard_state.test.ts | 6 ++-- .../np_ready/dashboard_state_manager.ts | 35 ++++++++++++++++++- .../dashboard/np_ready/url_helper.test.ts | 27 ++++++++++++++ .../public/dashboard/np_ready/url_helper.ts | 21 +++++++++++ .../apps/dashboard/bwc_shared_urls.js | 21 +++++++++++ .../apps/dashboard/dashboard_time.js | 14 ++++++++ .../apps/management/_kibana_settings.js | 27 +++++++------- 8 files changed, 155 insertions(+), 26 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx index 0c6686c993371..1e26a0ee8f964 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx +++ b/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('_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 = () => @@ -660,7 +669,8 @@ export class DashboardAppController { // 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); + dashboardStateManager.syncTimefilterWithDashboardTime(timefilter); + dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter); }); } } diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts index 08ccc1e0d1e89..14af89f80f9aa 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state.test.ts +++ b/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); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts index 171f08b45cf8d..6b44350d268be 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts @@ -51,6 +51,7 @@ import { ReduxLikeStateContainer, syncState, } from '../../../../../../plugins/kibana_utils/public'; +import { getDashboardIdFromUrl } from './url_helper'; /** * Dashboard state manager handles connecting angular and redux state between the angular and react portions of the @@ -175,6 +176,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 destroy 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 +212,7 @@ export class DashboardStateManager { public handleDashboardContainerChanges(dashboardContainer: DashboardContainer) { let dirty = false; + let dirtyBecauseOfInitialStateMigration = false; const savedDashboardPanelMap: { [key: string]: SavedDashboardPanel } = {}; @@ -236,11 +246,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) { + 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 +517,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 +532,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); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.test.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.test.ts index 60ca1b39d29d6..15079d86f2679 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.test.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.test.ts @@ -45,6 +45,7 @@ jest.mock('../legacy_imports', () => { import { addEmbeddableToDashboardUrl, + getDashboardIdFromUrl, getLensUrlFromDashboardAbsoluteUrl, getUrlVars, } from './url_helper'; @@ -115,4 +116,30 @@ describe('Dashboard URL Helper', () => { 'http://localhost:5601/app/kibana#/lens/edit/1244' ); }); + + it('getDashboardIdFromDashboardUrl', () => { + 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); + }); }); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.ts index 73383f2ff3f68..dfa1e77fa54cd 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/url_helper.ts @@ -98,3 +98,24 @@ export function getLensUrlFromDashboardAbsoluteUrl( function getUrlWithoutQueryParams(url: string): string { return url.split('?')[0]; } + +/** + * 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¶m2=y¶m3=z + * output: undefined + * input: http://localhost:5601/lib/app/kibana#/dashboard/39292992?param1=x¶m2=y¶m3=z + * output: 39292992 + */ +export function getDashboardIdFromUrl(url: string): string | undefined { + const [, match1, match2, match3] = url.match( + /dashboard\/(.*)\/|dashboard\/(.*)\?|dashboard\/(.*)$/ + ) ?? [ + undefined, // full match + undefined, // group1 - dashboardId is before `/` + undefined, // group2 - dashboardId is before `?` + undefined, // group3 - dashboardID is in the end + ]; + return match1 ?? match2 ?? match3 ?? undefined; +} diff --git a/test/functional/apps/dashboard/bwc_shared_urls.js b/test/functional/apps/dashboard/bwc_shared_urls.js index fb1e580135e5a..b56cb658b80bb 100644 --- a/test/functional/apps/dashboard/bwc_shared_urls.js +++ b/test/functional/apps/dashboard/bwc_shared_urls.js @@ -135,6 +135,27 @@ export default function({ getService, getPageObjects }) { await dashboardExpect.selectedLegendColorCount('#000000', 5); }); + + it('back button works for old dashboards after state migrations', async () => { + await PageObjects.dashboard.preserveCrossAppState(); + const oldId = await PageObjects.dashboard.getDashboardIdFromCurrentUrl(); + await PageObjects.dashboard.waitForRenderComplete(); + await dashboardExpect.selectedLegendColorCount('#000000', 5); + + const url = `${kibanaBaseUrl}#/dashboard?${urlQuery}`; + log.debug(`Navigating to ${url}`); + await browser.get(url); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.dashboard.waitForRenderComplete(); + await dashboardExpect.selectedLegendColorCount('#F9D9F9', 5); + await browser.goBack(); + + await PageObjects.header.waitUntilLoadingHasFinished(); + const newId = await PageObjects.dashboard.getDashboardIdFromCurrentUrl(); + expect(newId).to.be.equal(oldId); + await PageObjects.dashboard.waitForRenderComplete(); + await dashboardExpect.selectedLegendColorCount('#000000', 5); + }); }); }); } diff --git a/test/functional/apps/dashboard/dashboard_time.js b/test/functional/apps/dashboard/dashboard_time.js index 2e7b7f9a6dbb1..5a2628f42ded5 100644 --- a/test/functional/apps/dashboard/dashboard_time.js +++ b/test/functional/apps/dashboard/dashboard_time.js @@ -91,6 +91,20 @@ export default function({ getPageObjects, getService }) { expect(time.start).to.equal('~ an hour ago'); expect(time.end).to.equal('now'); }); + + it('should use saved time, if time is missing in global state, but _g is present in the url', async function() { + const currentUrl = await browser.getCurrentUrl(); + const kibanaBaseUrl = currentUrl.substring(0, currentUrl.indexOf('#')); + const id = await PageObjects.dashboard.getDashboardIdFromCurrentUrl(); + + await PageObjects.dashboard.gotoDashboardLandingPage(); + + const urlWithGlobalTime = `${kibanaBaseUrl}#/dashboard/${id}?_g=(filters:!())`; + await browser.get(urlWithGlobalTime, false); + const time = await PageObjects.timePicker.getTimeConfig(); + expect(time.start).to.equal(PageObjects.timePicker.defaultStartTime); + expect(time.end).to.equal(PageObjects.timePicker.defaultEndTime); + }); }); // If the user has time stored with a dashboard, it's supposed to override the current time settings diff --git a/test/functional/apps/management/_kibana_settings.js b/test/functional/apps/management/_kibana_settings.js index c99368ba4e859..97337d4573e2a 100644 --- a/test/functional/apps/management/_kibana_settings.js +++ b/test/functional/apps/management/_kibana_settings.js @@ -46,6 +46,18 @@ export default function({ getService, getPageObjects }) { }); describe('state:storeInSessionStorage', () => { + async function getStateFromUrl() { + const currentUrl = await browser.getCurrentUrl(); + let match = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); + if (match) return [match[2], match[3]]; + match = currentUrl.match(/(.*)?_a=(.*)&_g=(.*)/); + if (match) return [match[3], match[2]]; + + if (!match) { + throw new Error('State in url is missing or malformed'); + } + } + it('defaults to null', async () => { await PageObjects.settings.clickKibanaSettings(); const storeInSessionStorage = await PageObjects.settings.getAdvancedSettingCheckbox( @@ -58,10 +70,7 @@ export default function({ getService, getPageObjects }) { await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.clickNewDashboard(); await PageObjects.timePicker.setDefaultAbsoluteRange(); - const currentUrl = await browser.getCurrentUrl(); - const urlPieces = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); - const globalState = urlPieces[2]; - const appState = urlPieces[3]; + const [globalState, appState] = await getStateFromUrl(); // We don't have to be exact, just need to ensure it's greater than when the hashed variation is being used, // which is less than 20 characters. @@ -83,10 +92,7 @@ export default function({ getService, getPageObjects }) { await PageObjects.common.navigateToApp('dashboard'); await PageObjects.dashboard.clickNewDashboard(); await PageObjects.timePicker.setDefaultAbsoluteRange(); - const currentUrl = await browser.getCurrentUrl(); - const urlPieces = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); - const globalState = urlPieces[2]; - const appState = urlPieces[3]; + const [globalState, appState] = await getStateFromUrl(); // We don't have to be exact, just need to ensure it's less than the unhashed version, which will be // greater than 20 characters with the default state plus a time. @@ -100,10 +106,7 @@ export default function({ getService, getPageObjects }) { await PageObjects.settings.clickKibanaSettings(); await PageObjects.settings.toggleAdvancedSettingCheckbox('state:storeInSessionStorage'); await PageObjects.header.clickDashboard(); - const currentUrl = await browser.getCurrentUrl(); - const urlPieces = currentUrl.match(/(.*)?_g=(.*)&_a=(.*)/); - const globalState = urlPieces[2]; - const appState = urlPieces[3]; + const [globalState, appState] = await getStateFromUrl(); // We don't have to be exact, just need to ensure it's greater than when the hashed variation is being used, // which is less than 20 characters. expect(globalState.length).to.be.greaterThan(20); From b67dc19051af782d87efc5dde556f6cc870d7656 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 31 Mar 2020 13:27:24 +0200 Subject: [PATCH 2/4] post merge fixes --- .../np_ready/dashboard_state_manager.ts | 3 +- .../public/dashboard/np_ready/lib/index.ts | 1 + .../public/dashboard/np_ready/lib/url.test.ts | 46 +++++++++++++++++++ .../public/dashboard/np_ready/lib/url.ts | 39 ++++++++++++++++ 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts index 6b44350d268be..ec4c9610024e0 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts +++ b/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 { @@ -51,7 +51,6 @@ import { ReduxLikeStateContainer, syncState, } from '../../../../../../plugins/kibana_utils/public'; -import { getDashboardIdFromUrl } from './url_helper'; /** * Dashboard state manager handles connecting angular and redux state between the angular and react portions of the diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/index.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/index.ts index b4c9e939d3083..e9ebe73c3b34d 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/index.ts +++ b/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'; diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts index e69de29bb2d1d..3ac7aab235412 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts +++ b/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('getDashboardIdFromDashboardUrl', () => { + 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); +}); diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts index e69de29bb2d1d..2c4f93aeabc7a 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts @@ -0,0 +1,39 @@ +/* + * 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¶m2=y¶m3=z + * output: undefined + * input: http://localhost:5601/lib/app/kibana#/dashboard/39292992?param1=x¶m2=y¶m3=z + * output: 39292992 + */ +export function getDashboardIdFromUrl(url: string): string | undefined { + const [, match1, match2, match3] = url.match( + /dashboard\/(.*)\/|dashboard\/(.*)\?|dashboard\/(.*)$/ + ) ?? [ + undefined, // full match + undefined, // group1 - dashboardId is before `/` + undefined, // group2 - dashboardId is before `?` + undefined, // group3 - dashboardID is in the end + ]; + return match1 ?? match2 ?? match3 ?? undefined; +} From 97500ed1e2edb8076c8dca10d3368fa39a374038 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Wed, 1 Apr 2020 16:22:03 +0200 Subject: [PATCH 3/4] review fixes --- .../np_ready/dashboard_app_controller.tsx | 22 +++++++------------ .../np_ready/dashboard_state_manager.ts | 4 ++-- .../public/dashboard/np_ready/lib/url.test.ts | 2 +- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx index 8843281fc02c0..eefbb8786f033 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx @@ -652,6 +652,14 @@ 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 @@ -659,20 +667,6 @@ export class DashboardAppController { 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.syncTimefilterWithDashboardTime(timefilter); - dashboardStateManager.syncTimefilterWithDashboardRefreshInterval(timefilter); - }); - } } overlays diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts index ec4c9610024e0..9b8f75bdcf953 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_state_manager.ts @@ -178,7 +178,7 @@ export class DashboardStateManager { // 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 destroy soon and we shouldn't sync state anymore, + // 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; @@ -248,7 +248,7 @@ export class DashboardStateManager { const oldVersion = savedDashboardPanelMap[panelState.explicitInput.id]?.version; const newVersion = convertedPanelStateMap[panelState.explicitInput.id]?.version; - if (oldVersion !== newVersion) { + if (oldVersion && newVersion && oldVersion !== newVersion) { dirtyBecauseOfInitialStateMigration = true; } } diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts index 3ac7aab235412..70a9d86206fd6 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.test.ts @@ -19,7 +19,7 @@ import { getDashboardIdFromUrl } from './url'; -test('getDashboardIdFromDashboardUrl', () => { +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); From 72323601f56b9262cb52db6b6fd949c9c76de77b Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Thu, 2 Apr 2020 17:21:59 +0200 Subject: [PATCH 4/4] improve regex --- .../kibana/public/dashboard/np_ready/lib/url.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts index 2c4f93aeabc7a..2489867fa6233 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/np_ready/lib/url.ts @@ -27,13 +27,9 @@ * output: 39292992 */ export function getDashboardIdFromUrl(url: string): string | undefined { - const [, match1, match2, match3] = url.match( - /dashboard\/(.*)\/|dashboard\/(.*)\?|dashboard\/(.*)$/ - ) ?? [ + const [, dashboardId] = url.match(/dashboard\/(.*?)(\/|\?|$)/) ?? [ undefined, // full match - undefined, // group1 - dashboardId is before `/` - undefined, // group2 - dashboardId is before `?` - undefined, // group3 - dashboardID is in the end + undefined, // group with dashboardId ]; - return match1 ?? match2 ?? match3 ?? undefined; + return dashboardId ?? undefined; }