Global time range in URL overwritten in certain situations. (#21440) (#21568)

* move global state update from kbn_global_timepicker to timefilter

* add functional test

* update time and refreshInterval when either update

* use same function name from kbn_global_timepicker

* call globalState.save in context of a digest cycle

* ensure callers of registerTimefilterWithGlobalState pass in

src/ui/public/timefilter/timefilter.js

@@ -175,7 +175,7 @@ function convertISO8601(stringTime) {
 // and require it to be executed to properly function.
 // This function is exposed for applications that do not use uiRoutes like APM
 // Kibana issue https://github.com/elastic/kibana/issues/19110 tracks the removal of this dependency on uiRouter
-export const registerTimefilterWithGlobalState = _.once((globalState) => {
+export const registerTimefilterWithGlobalState = _.once((globalState, $rootScope) => {
   const uiSettings = chrome.getUiSettingsClient();
   const timeDefaults = uiSettings.get('timepicker:timeDefaults');
   const refreshIntervalDefaults = uiSettings.get('timepicker:refreshIntervalDefaults');
@@ -196,9 +196,19 @@ export const registerTimefilterWithGlobalState = _.once((globalState) => {
     timefilter.setTime(newTime);
     timefilter.setRefreshInterval(newRefreshInterval);
   });
+
+  const updateGlobalStateWithTime = () => {
+    globalState.time = timefilter.getTime();
+    globalState.refreshInterval = timefilter.getRefreshInterval();
+    globalState.save();
+  };
+
+  $rootScope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', updateGlobalStateWithTime);
+
+  $rootScope.$listenAndDigestAsync(timefilter, 'timeUpdate', updateGlobalStateWithTime);
 });
 
 uiRoutes
-  .addSetupWork((globalState) => {
-    return registerTimefilterWithGlobalState(globalState);
+  .addSetupWork((globalState, $rootScope) => {
+    return registerTimefilterWithGlobalState(globalState, $rootScope);
   });

src/ui/public/timepicker/kbn_global_timepicker.js

@@ -30,19 +30,12 @@ uiModules
   .directive('kbnGlobalTimepicker', (globalState, config) => {
     const getConfig = (...args) => config.get(...args);
 
-    const updateGlobalStateWithTime = ($scope) => {
-      globalState.refreshInterval = timefilter.getRefreshInterval();
-      globalState.time = timefilter.getTime();
-      globalState.save();
-      setTimefilterValues($scope);
-    };
-
     const listenForUpdates = ($scope) => {
       $scope.$listenAndDigestAsync(timefilter, 'refreshIntervalUpdate', () => {
-        updateGlobalStateWithTime($scope);
+        setTimefilterValues($scope);
       });
       $scope.$listenAndDigestAsync(timefilter, 'timeUpdate', () => {
-        updateGlobalStateWithTime($scope);
+        setTimefilterValues($scope);
       });
       $scope.$listenAndDigestAsync(timefilter, 'enabledUpdated', () => {
         setTimefilterValues($scope);
@@ -70,7 +63,8 @@ uiModules
       require: '^kbnTopNav',
       link: ($scope, element, attributes, kbnTopNav) => {
         listenForUpdates($scope);
-        updateGlobalStateWithTime($scope);
+
+        setTimefilterValues($scope);
 
         $scope.toggleRefresh = () => {
           timefilter.toggleRefresh();

test/functional/apps/dashboard/_dashboard_time.js

@@ -24,8 +24,9 @@ const dashboardName = 'Dashboard Test Time';
 const fromTime = '2015-09-19 06:31:44.000';
 const toTime = '2015-09-23 18:31:44.000';
 
-export default function ({ getPageObjects }) {
+export default function ({ getPageObjects, getService }) {
   const PageObjects = getPageObjects(['dashboard', 'header']);
+  const remote = getService('remote');
 
   describe('dashboard time', () => {
     before(async function () {
@@ -91,6 +92,22 @@ export default function ({ getPageObjects }) {
         expect(fromTimeNext).to.equal(fromTime);
         expect(toTimeNext).to.equal(toTime);
       });
+
+      // If time is stored with a dashboard, it's supposed to override the current time settings when opened.
+      // However, if the URL also contains time in the global state, then the global state
+      // time should take precedence.
+      it('should be overwritten by global state', async function () {
+        const currentUrl = await remote.getCurrentUrl();
+        const kibanaBaseUrl = currentUrl.substring(0, currentUrl.indexOf('#'));
+        const id = await PageObjects.dashboard.getDashboardIdFromCurrentUrl();
+
+        await PageObjects.dashboard.gotoDashboardLandingPage();
+
+        const urlWithGlobalTime = `${kibanaBaseUrl}#/dashboard/${id}?_g=(time:(from:now-1h,mode:quick,to:now))`;
+        await remote.get(urlWithGlobalTime, false);
+        const prettyPrint = await PageObjects.header.getPrettyDuration();
+        expect(prettyPrint).to.equal('Last 1 hour');
+      });
     });
 
     // If the user has time stored with a dashboard, it's supposed to override the current time settings
@@ -99,6 +116,8 @@ export default function ({ getPageObjects }) {
     // during navigation or page refreshes.
     describe('time changes', function () {
       it('preserved during navigation', async function () {
+        await PageObjects.dashboard.loadSavedDashboard(dashboardName);
+
         await PageObjects.header.setQuickTime('Today');
         await PageObjects.header.clickVisualize();
         await PageObjects.header.clickDashboard();

x-pack/plugins/apm/public/utils/timepicker/index.js

@@ -37,7 +37,7 @@ export function initTimepicker(history, dispatch) {
 
     uiModules
       .get('app/apm', [])
-      .controller('TimePickerController', ($scope, globalState) => {
+      .controller('TimePickerController', ($scope, globalState, $rootScope) => {
         // Add APM feedback menu
         // TODO: move this somewhere else
         $scope.topNavMenu = [];
@@ -59,7 +59,7 @@ export function initTimepicker(history, dispatch) {
         );
 
         // ensure that timepicker updates when global state changes
-        registerTimefilterWithGlobalState(globalState);
+        registerTimefilterWithGlobalState(globalState, $rootScope);
 
         timefilter.enableTimeRangeSelector();
         timefilter.enableAutoRefreshSelector();