Skip to content

Commit

Permalink
[Dashboard] Fix Time Range Regression (elastic#159337)
Browse files Browse the repository at this point in the history
Fixed Dashboard loading with a saved time range when the URL also contains a time range.
  • Loading branch information
ThomThomson authored Jun 15, 2023
1 parent ae068a6 commit f60d43e
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,20 @@ 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 { LoadDashboardReturn } from '../../../services/dashboard_content_management/types';
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';

Expand Down Expand Up @@ -175,34 +176,49 @@ 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
filterManager.setAppFilters(cloneDeep(filters ?? []));
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<GlobalQueryStateFromUrl>(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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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<GlobalQueryStateFromUrl>(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<GlobalQueryStateFromUrl>(
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$()
Expand Down
31 changes: 31 additions & 0 deletions test/functional/apps/dashboard/group4/dashboard_time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 () {
Expand All @@ -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
Expand Down

0 comments on commit f60d43e

Please sign in to comment.