Dashboards: Only refresh panels on panel edit exit if refresh is set (#67652)

Closes #66838

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

.betterer.results

@@ -2372,8 +2372,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "2"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "5"]
+      [0, 0, 0, "Unexpected any. Specify a different type.", "4"]
     ],
     "public/app/features/dashboard/state/DashboardMigrator.test.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

public/app/features/dashboard/services/TimeSrv.test.ts

@@ -287,19 +287,16 @@ describe('timeSrv', () => {
     });
   });
 
-  describe('pauseAutoRefresh', () => {
-    it('should set autoRefreshPaused to true', () => {
-      _dashboard.refresh = '10s';
-      timeSrv.pauseAutoRefresh();
-      expect(timeSrv.autoRefreshPaused).toBe(true);
-    });
-  });
-
   describe('resumeAutoRefresh', () => {
-    it('should set refresh to empty value', () => {
-      timeSrv.autoRefreshPaused = true;
+    it('should set auto-refresh interval', () => {
+      timeSrv.setAutoRefresh('10s');
+      expect(timeSrv.refreshTimer).not.toBeUndefined();
+
+      timeSrv.stopAutoRefresh();
+      expect(timeSrv.refreshTimer).toBeUndefined();
+
       timeSrv.resumeAutoRefresh();
-      expect(timeSrv.autoRefreshPaused).toBe(false);
+      expect(timeSrv.refreshTimer).not.toBeUndefined();
     });
   });
 

public/app/features/dashboard/services/TimeSrv.ts

@@ -23,9 +23,8 @@ import { getRefreshFromUrl } from '../utils/getRefreshFromUrl';
 
 export class TimeSrv {
   time: any;
-  refreshTimer: any;
+  refreshTimer: number | undefined;
   refresh: any;
-  autoRefreshPaused = false;
   oldRefresh: string | null | undefined;
   timeModel?: TimeModel;
   timeAtLoad: any;
@@ -235,9 +234,9 @@ export class TimeSrv {
     const validInterval = this.contextSrv.getValidInterval(interval);
     const intervalMs = rangeUtil.intervalToMs(validInterval);
 
-    this.refreshTimer = setTimeout(() => {
+    this.refreshTimer = window.setTimeout(() => {
       this.startNextRefreshTimer(intervalMs);
-      !this.autoRefreshPaused && this.refreshTimeModel();
+      this.refreshTimeModel();
     }, intervalMs);
 
     const refresh = this.contextSrv.getValidInterval(interval);
@@ -252,10 +251,10 @@ export class TimeSrv {
   }
 
   private startNextRefreshTimer(afterMs: number) {
-    this.refreshTimer = setTimeout(() => {
+    this.refreshTimer = window.setTimeout(() => {
       this.startNextRefreshTimer(afterMs);
       if (this.contextSrv.isGrafanaVisible()) {
-        !this.autoRefreshPaused && this.refreshTimeModel();
+        this.refreshTimeModel();
       } else {
         this.autoRefreshBlocked = true;
       }
@@ -264,18 +263,14 @@ export class TimeSrv {
 
   stopAutoRefresh() {
     clearTimeout(this.refreshTimer);
-  }
-
-  // store timeModel refresh value and pause auto-refresh in some places
-  // i.e panel edit
-  pauseAutoRefresh() {
-    this.autoRefreshPaused = true;
+    this.refreshTimer = undefined;
   }
 
   // resume auto-refresh based on old dashboard refresh property
   resumeAutoRefresh() {
-    this.autoRefreshPaused = false;
-    this.refreshTimeModel();
+    if (this.timeModel?.refresh) {
+      this.setAutoRefresh(this.timeModel.refresh);
+    }
   }
 
   setTime(time: RawTimeRange, updateUrl = true) {

public/app/features/dashboard/state/DashboardModel.test.ts

@@ -1,6 +1,6 @@
 import { keys as _keys } from 'lodash';
 
-import { VariableHide } from '@grafana/data';
+import { dateTime, TimeRange, VariableHide } from '@grafana/data';
 import { defaultVariableModel } from '@grafana/schema';
 import { contextSrv } from 'app/core/services/context_srv';
 
@@ -1158,8 +1158,35 @@ describe('exitViewPanel', () => {
   });
 });
 
-describe('exitPanelEditor', () => {
-  function getTestContext(pauseAutoRefresh = false) {
+describe('when initEditPanel is called', () => {
+  function getTestContext() {
+    const dashboard = createDashboardModelFixture();
+    const timeSrvMock = {
+      pauseAutoRefresh: jest.fn(),
+      resumeAutoRefresh: jest.fn(),
+      stopAutoRefresh: jest.fn(),
+    } as unknown as TimeSrv;
+    setTimeSrv(timeSrvMock);
+    return { dashboard, timeSrvMock };
+  }
+
+  it('should set panelInEdit', () => {
+    const { dashboard } = getTestContext();
+    dashboard.addPanel({ type: 'timeseries' });
+    dashboard.initEditPanel(dashboard.panels[0]);
+    expect(dashboard.panelInEdit).not.toBeUndefined();
+  });
+
+  it('should stop auto-refresh', () => {
+    const { dashboard, timeSrvMock } = getTestContext();
+    dashboard.addPanel({ type: 'timeseries' });
+    dashboard.initEditPanel(dashboard.panels[0]);
+    expect(timeSrvMock.stopAutoRefresh).toHaveBeenCalled();
+  });
+});
+
+describe('when exitPanelEditor is called', () => {
+  function getTestContext() {
     const panel = new PanelModel({ destroy: jest.fn() });
     const dashboard = createDashboardModelFixture();
     const timeSrvMock = {
@@ -1169,70 +1196,54 @@ describe('exitPanelEditor', () => {
     } as unknown as TimeSrv;
     dashboard.startRefresh = jest.fn();
     dashboard.panelInEdit = panel;
-    if (pauseAutoRefresh) {
-      timeSrvMock.autoRefreshPaused = true;
-    }
     setTimeSrv(timeSrvMock);
     return { dashboard, panel, timeSrvMock };
   }
 
-  describe('when called', () => {
-    it('then panelInEdit is set to undefined', () => {
-      const { dashboard } = getTestContext();
-
-      dashboard.exitPanelEditor();
+  it('should set panelInEdit to undefined', () => {
+    const { dashboard } = getTestContext();
 
-      expect(dashboard.panelInEdit).toBeUndefined();
-    });
+    dashboard.exitPanelEditor();
 
-    it('then destroy is called on panel', () => {
-      const { dashboard, panel } = getTestContext();
+    expect(dashboard.panelInEdit).toBeUndefined();
+  });
 
-      dashboard.exitPanelEditor();
+  it('should destroy panel', () => {
+    const { dashboard, panel } = getTestContext();
 
-      expect(panel.destroy).toHaveBeenCalled();
-    });
+    dashboard.exitPanelEditor();
 
-    it('then startRefresh is not called', () => {
-      const { dashboard } = getTestContext();
+    expect(panel.destroy).toHaveBeenCalled();
+  });
 
-      dashboard.exitPanelEditor();
+  it('should not call startRefresh', () => {
+    const { dashboard } = getTestContext();
 
-      expect(dashboard.startRefresh).not.toHaveBeenCalled();
-    });
+    dashboard.exitPanelEditor();
 
-    it('then auto refresh property is resumed', () => {
-      const { dashboard, timeSrvMock } = getTestContext(true);
-      dashboard.exitPanelEditor();
-      expect(timeSrvMock.resumeAutoRefresh).toHaveBeenCalled();
-    });
+    expect(dashboard.startRefresh).not.toHaveBeenCalled();
   });
-});
 
-describe('initEditPanel', () => {
-  function getTestContext() {
-    const dashboard = createDashboardModelFixture();
-    const timeSrvMock = {
-      pauseAutoRefresh: jest.fn(),
-      resumeAutoRefresh: jest.fn(),
-    } as unknown as TimeSrv;
-    setTimeSrv(timeSrvMock);
-    return { dashboard, timeSrvMock };
-  }
-
-  describe('when called', () => {
-    it('then panelInEdit is not undefined', () => {
-      const { dashboard } = getTestContext();
-      dashboard.addPanel({ type: 'timeseries' });
-      dashboard.initEditPanel(dashboard.panels[0]);
-      expect(dashboard.panelInEdit).not.toBeUndefined();
-    });
+  it('should call startRefresh if time range changed during edit', () => {
+    const { dashboard } = getTestContext();
+
+    const range: TimeRange = {
+      from: dateTime(new Date().getTime()).subtract(1, 'minutes'),
+      to: dateTime(new Date().getTime()),
+      raw: {
+        from: 'now-1m',
+        to: 'now',
+      },
+    };
+    dashboard.timeRangeUpdated(range);
+    dashboard.exitPanelEditor();
+
+    expect(dashboard.startRefresh).toHaveBeenCalled();
+  });
 
-    it('then auto-refresh is paused', () => {
-      const { dashboard, timeSrvMock } = getTestContext();
-      dashboard.addPanel({ type: 'timeseries' });
-      dashboard.initEditPanel(dashboard.panels[0]);
-      expect(timeSrvMock.pauseAutoRefresh).toHaveBeenCalled();
-    });
+  it('then auto refresh property is resumed', () => {
+    const { dashboard, timeSrvMock } = getTestContext();
+    dashboard.exitPanelEditor();
+    expect(timeSrvMock.resumeAutoRefresh).toHaveBeenCalled();
   });
 });

public/app/features/dashboard/state/DashboardModel.ts

@@ -101,6 +101,7 @@ export class DashboardModel implements TimeModel {
   private panelsAffectedByVariableChange: number[] | null;
   private appEventsSubscription: Subscription;
   private lastRefresh: number;
+  private timeRangeUpdatedDuringEdit = false;
 
   // ------------------
   // not persisted
@@ -126,6 +127,7 @@ export class DashboardModel implements TimeModel {
     appEventsSubscription: true,
     panelsAffectedByVariableChange: true,
     lastRefresh: true,
+    timeRangeUpdatedDuringEdit: true,
   };
 
   constructor(
@@ -379,6 +381,10 @@ export class DashboardModel implements TimeModel {
   timeRangeUpdated(timeRange: TimeRange) {
     this.events.publish(new TimeRangeUpdatedEvent(timeRange));
     dispatch(onTimeRangeUpdated(this.uid, timeRange));
+
+    if (this.panelInEdit) {
+      this.timeRangeUpdatedDuringEdit = true;
+    }
   }
 
   startRefresh(event: VariablesChangedEvent = { refreshAll: true, panelIds: [] }) {
@@ -417,11 +423,28 @@ export class DashboardModel implements TimeModel {
   }
 
   initEditPanel(sourcePanel: PanelModel): PanelModel {
-    getTimeSrv().pauseAutoRefresh();
+    getTimeSrv().stopAutoRefresh();
     this.panelInEdit = sourcePanel.getEditClone();
+    this.timeRangeUpdatedDuringEdit = false;
     return this.panelInEdit;
   }
 
+  exitPanelEditor() {
+    this.panelInEdit!.destroy();
+    this.panelInEdit = undefined;
+
+    getTimeSrv().resumeAutoRefresh();
+
+    if (this.panelsAffectedByVariableChange || this.timeRangeUpdatedDuringEdit) {
+      this.startRefresh({
+        panelIds: this.panelsAffectedByVariableChange ?? [],
+        refreshAll: this.timeRangeUpdatedDuringEdit,
+      });
+      this.panelsAffectedByVariableChange = null;
+      this.timeRangeUpdatedDuringEdit = false;
+    }
+  }
+
   initViewPanel(panel: PanelModel) {
     this.panelInView = panel;
     panel.setIsViewing(true);
@@ -433,13 +456,6 @@ export class DashboardModel implements TimeModel {
     this.refreshIfPanelsAffectedByVariableChange();
   }
 
-  exitPanelEditor() {
-    this.panelInEdit!.destroy();
-    this.panelInEdit = undefined;
-    getTimeSrv().resumeAutoRefresh();
-    this.refreshIfPanelsAffectedByVariableChange();
-  }
-
   private refreshIfPanelsAffectedByVariableChange() {
     if (!this.panelsAffectedByVariableChange) {
       return;