From ecda618aa17dcf9f4b4f12e6fc5dfc5bfbebf641 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Thu, 15 Jun 2023 16:10:55 -0400 Subject: [PATCH] [Dashboard] Fix Time Range Regression (#159337) Fixed Dashboard loading with a saved time range when the URL also contains a time range. (cherry picked from commit f60d43e27b7c28e73f13b122453ac27c8e2e62bd) --- .../create/create_dashboard.test.ts | 24 +++++- .../embeddable/create/create_dashboard.ts | 42 +++++++---- .../sync_dashboard_unified_search_state.ts | 74 +++++++++++++++++-- .../apps/dashboard/group4/dashboard_time.ts | 31 ++++++++ 4 files changed, 151 insertions(+), 20 deletions(-) diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts index 4bc188c69a2c1..f53ecf9674dca 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts @@ -146,7 +146,29 @@ test('applies time range and refresh interval from initial input to query servic ).toHaveBeenCalledWith(refreshInterval); }); -test('applied time range from query service to initial input if time restore is off', async () => { +test('applies time range from query service to initial input if time restore is on but there is an explicit time range in the URL', async () => { + const urlTimeRange = { from: new Date().toISOString(), to: new Date().toISOString() }; + const savedTimeRange = { from: 'now - 7 days', to: 'now' }; + pluginServices.getServices().data.query.timefilter.timefilter.getTime = jest + .fn() + .mockReturnValue(urlTimeRange); + const kbnUrlStateStorage = createKbnUrlStateStorage(); + kbnUrlStateStorage.get = jest.fn().mockReturnValue({ time: urlTimeRange }); + + const dashboard = await createDashboard({ + useUnifiedSearchIntegration: true, + unifiedSearchSettings: { + kbnUrlStateStorage, + }, + getInitialInput: () => ({ + timeRestore: true, + timeRange: savedTimeRange, + }), + }); + expect(dashboard.getState().explicitInput.timeRange).toEqual(urlTimeRange); +}); + +test('applies time range from query service to initial input if time restore is off', async () => { const timeRange = { from: new Date().toISOString(), to: new Date().toISOString() }; pluginServices.getServices().data.query.timefilter.timefilter.getTime = jest .fn() diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts index 58983196ea231..7bf0ce5accb48 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.ts @@ -13,18 +13,19 @@ import { CONTROL_GROUP_TYPE, getDefaultControlGroupInput, } from '@kbn/controls-plugin/common'; -import { syncGlobalQueryStateWithUrl } from '@kbn/data-plugin/public'; +import { TimeRange } from '@kbn/es-query'; import { isErrorEmbeddable, ViewMode } from '@kbn/embeddable-plugin/public'; import { lazyLoadReduxToolsPackage } from '@kbn/presentation-util-plugin/public'; import { type ControlGroupContainer, ControlGroupOutput } from '@kbn/controls-plugin/public'; +import { GlobalQueryStateFromUrl, syncGlobalQueryStateWithUrl } from '@kbn/data-plugin/public'; import { DashboardContainerInput } from '../../../../common'; import { DashboardContainer } from '../dashboard_container'; import { pluginServices } from '../../../services/plugin_services'; -import { DEFAULT_DASHBOARD_INPUT } from '../../../dashboard_constants'; import { DashboardCreationOptions } from '../dashboard_container_factory'; import { startSyncingDashboardDataViews } from './data_views/sync_dashboard_data_views'; import { syncUnifiedSearchState } from './unified_search/sync_dashboard_unified_search_state'; +import { DEFAULT_DASHBOARD_INPUT, GLOBAL_STATE_STORAGE_KEY } from '../../../dashboard_constants'; import { startSyncingDashboardControlGroup } from './controls/dashboard_control_group_integration'; import { startDashboardSearchSessionIntegration } from './search_sessions/start_dashboard_search_session_integration'; import { LoadDashboardFromSavedObjectReturn } from '../../../services/dashboard_saved_object/lib/load_dashboard_state_from_saved_object'; @@ -174,7 +175,13 @@ export const initializeDashboard = async ({ // Set up unified search integration. // -------------------------------------------------------------------------------------- if (useUnifiedSearchIntegration && unifiedSearchSettings?.kbnUrlStateStorage) { - const { filters, query, timeRestore, timeRange, refreshInterval } = initialInput; + const { + query, + filters, + timeRestore, + timeRange: savedTimeRange, + refreshInterval: savedRefreshInterval, + } = initialInput; const { kbnUrlStateStorage } = unifiedSearchSettings; // apply filters and query to the query service @@ -182,26 +189,35 @@ export const initializeDashboard = async ({ queryString.setQuery(query ?? queryString.getDefaultQuery()); /** - * If a global time range is not set explicitly and the time range was saved with the dashboard, apply - * time range and refresh interval to the query service. Otherwise, set the current dashboard time range - * from the query service. The order of the following lines is very important. + * Get initial time range, and set up dashboard time restore if applicable */ + const initialTimeRange: TimeRange = (() => { + // if there is an explicit time range in the URL it always takes precedence. + const urlOverrideTimeRange = + kbnUrlStateStorage.get(GLOBAL_STATE_STORAGE_KEY)?.time; + if (urlOverrideTimeRange) return urlOverrideTimeRange; + + // if this Dashboard has timeRestore return the time range that was saved with the dashboard. + if (timeRestore && savedTimeRange) return savedTimeRange; + + // otherwise fall back to the time range from the timefilterService. + return timefilterService.getTime(); + })(); + initialInput.timeRange = initialTimeRange; if (timeRestore) { - if (timeRange) timefilterService.setTime(timeRange); - if (refreshInterval) timefilterService.setRefreshInterval(refreshInterval); + if (savedTimeRange) timefilterService.setTime(savedTimeRange); + if (savedRefreshInterval) timefilterService.setRefreshInterval(savedRefreshInterval); } + // start syncing global query state with the URL. const { stop: stopSyncingQueryServiceStateWithUrl } = syncGlobalQueryStateWithUrl( queryService, kbnUrlStateStorage ); - if (!timeRestore) { - initialInput.timeRange = timefilterService.getTime(); - } - untilDashboardReady().then((dashboardContainer) => { - const stopSyncingUnifiedSearchState = syncUnifiedSearchState.bind(dashboardContainer)(); + const stopSyncingUnifiedSearchState = + syncUnifiedSearchState.bind(dashboardContainer)(kbnUrlStateStorage); dashboardContainer.stopSyncingWithUnifiedSearch = () => { stopSyncingUnifiedSearchState(); stopSyncingQueryServiceStateWithUrl(); diff --git a/src/plugins/dashboard/public/dashboard_container/embeddable/create/unified_search/sync_dashboard_unified_search_state.ts b/src/plugins/dashboard/public/dashboard_container/embeddable/create/unified_search/sync_dashboard_unified_search_state.ts index 43220de9b69e4..d65ddb18fa40b 100644 --- a/src/plugins/dashboard/public/dashboard_container/embeddable/create/unified_search/sync_dashboard_unified_search_state.ts +++ b/src/plugins/dashboard/public/dashboard_container/embeddable/create/unified_search/sync_dashboard_unified_search_state.ts @@ -7,20 +7,31 @@ */ import { Subject } from 'rxjs'; +import fastIsEqual from 'fast-deep-equal'; import { distinctUntilChanged, finalize, switchMap, tap } from 'rxjs/operators'; import type { Filter, Query } from '@kbn/es-query'; +import { IKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public'; import { cleanFiltersForSerialize } from '@kbn/presentation-util-plugin/public'; -import { connectToQueryState, waitUntilNextSessionCompletes$ } from '@kbn/data-plugin/public'; +import { + connectToQueryState, + GlobalQueryStateFromUrl, + waitUntilNextSessionCompletes$, +} from '@kbn/data-plugin/public'; import { DashboardContainer } from '../../dashboard_container'; import { pluginServices } from '../../../../services/plugin_services'; +import { GLOBAL_STATE_STORAGE_KEY } from '../../../../dashboard_constants'; +import { areTimesEqual } from '../../../state/diffing/dashboard_diffing_utils'; /** * Sets up syncing and subscriptions between the filter state from the Data plugin * and the dashboard Redux store. */ -export function syncUnifiedSearchState(this: DashboardContainer) { +export function syncUnifiedSearchState( + this: DashboardContainer, + kbnUrlStateStorage: IKbnUrlStateStorage +) { const { data: { query: queryService, search }, } = pluginServices.getServices(); @@ -65,13 +76,64 @@ export function syncUnifiedSearchState(this: DashboardContainer) { } ); - const timeUpdateSubscription = timefilterService - .getTimeUpdate$() - .subscribe(() => this.dispatch.setTimeRange(timefilterService.getTime())); + const timeUpdateSubscription = timefilterService.getTimeUpdate$().subscribe(() => { + const newTimeRange = (() => { + // if there is an override time range in the URL, use it. + const urlOverrideTimeRange = + kbnUrlStateStorage.get(GLOBAL_STATE_STORAGE_KEY)?.time; + if (urlOverrideTimeRange) return urlOverrideTimeRange; + + // if there is no url override time range, check if this dashboard uses time restore, and restore to that. + const timeRestoreTimeRange = + this.getState().explicitInput.timeRestore && + this.getState().componentState.lastSavedInput.timeRange; + if (timeRestoreTimeRange) { + timefilterService.setTime(timeRestoreTimeRange); + return timeRestoreTimeRange; + } + + // otherwise fall back to the time range from the time filter service + return timefilterService.getTime(); + })(); + + const lastTimeRange = this.getState().explicitInput.timeRange; + if ( + !areTimesEqual(newTimeRange.from, lastTimeRange?.from) || + !areTimesEqual(newTimeRange.to, lastTimeRange?.to) + ) { + this.dispatch.setTimeRange(newTimeRange); + } + }); const refreshIntervalSubscription = timefilterService .getRefreshIntervalUpdate$() - .subscribe(() => this.dispatch.setRefreshInterval(timefilterService.getRefreshInterval())); + .subscribe(() => { + const newRefreshInterval = (() => { + // if there is an override refresh interval in the URL, dispatch that to the dashboard. + const urlOverrideRefreshInterval = + kbnUrlStateStorage.get( + GLOBAL_STATE_STORAGE_KEY + )?.refreshInterval; + if (urlOverrideRefreshInterval) return urlOverrideRefreshInterval; + + // if there is no url override refresh interval, check if this dashboard uses time restore, and restore to that. + const timeRestoreRefreshInterval = + this.getState().explicitInput.timeRestore && + this.getState().componentState.lastSavedInput.refreshInterval; + if (timeRestoreRefreshInterval) { + timefilterService.setRefreshInterval(timeRestoreRefreshInterval); + return timeRestoreRefreshInterval; + } + + // otherwise fall back to the refresh interval from the time filter service + return timefilterService.getRefreshInterval(); + })(); + + const lastRefreshInterval = this.getState().explicitInput.refreshInterval; + if (!fastIsEqual(newRefreshInterval, lastRefreshInterval)) { + this.dispatch.setRefreshInterval(newRefreshInterval); + } + }); const autoRefreshSubscription = timefilterService .getAutoRefreshFetch$() diff --git a/test/functional/apps/dashboard/group4/dashboard_time.ts b/test/functional/apps/dashboard/group4/dashboard_time.ts index 2ff91185be60a..e1dbefa63ac74 100644 --- a/test/functional/apps/dashboard/group4/dashboard_time.ts +++ b/test/functional/apps/dashboard/group4/dashboard_time.ts @@ -14,6 +14,7 @@ const dashboardName = 'Dashboard Test Time'; export default function ({ getService, getPageObjects }: FtrProviderContext) { const PageObjects = getPageObjects(['common', 'dashboard', 'header', 'timePicker']); + const pieChart = getService('pieChart'); const browser = getService('browser'); describe('dashboard time', () => { @@ -81,6 +82,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const time = await PageObjects.timePicker.getTimeConfig(); expect(time.start).to.equal('~ an hour ago'); expect(time.end).to.equal('now'); + + /** + * With the time range set to an hour ago until now there should be no data. This ensures that the URL time + * range and NOT the saved time range was properly set on the Dashboard and passed down to its children. + */ + await pieChart.expectEmptyPieChart(); }); it('should use saved time, if time is missing in global state, but _g is present in the url', async function () { @@ -96,6 +103,30 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(time.start).to.equal(PageObjects.timePicker.defaultStartTime); expect(time.end).to.equal(PageObjects.timePicker.defaultEndTime); }); + + it('should use saved time after time change is undone', 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}#/view/${id}?_g=(filters:!())`; + await browser.get(urlWithGlobalTime, false); + + // set the time to something else + await PageObjects.timePicker.setAbsoluteRange( + 'Jan 1, 2019 @ 00:00:00.000', + 'Jan 2, 2019 @ 00:00:00.000' + ); + await PageObjects.dashboard.waitForRenderComplete(); + await browser.goBack(); + + // time should have restored to the saved time range. + 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