Dashboard: Fix Editing a panel with auto-refresh enabled auto-refresh in the edit screen (#34128)

* Dashboard: Fix editing panel with auto-refresh

- Add default hidden and disabled auto-refresh option on the editing panel
- Extend unit test for DashboardModel
- Add unit test DashNavTimeControls component

packages/grafana-e2e-selectors/src/selectors/components.ts

@@ -120,6 +120,7 @@ export const Components = {
   },
   RefreshPicker: {
     runButton: 'RefreshPicker run button',
+    intervalButton: 'RefreshPicker interval button',
   },
   QueryTab: {
     content: 'Query editor tab content',

packages/grafana-ui/src/components/RefreshPicker/RefreshPicker.tsx

@@ -82,6 +82,7 @@ export class RefreshPicker extends PureComponent<Props> {
               options={options}
               onChange={this.onChangeSelect as any}
               variant={variant}
+              aria-label={selectors.components.RefreshPicker.intervalButton}
             />
           )}
         </ButtonGroup>

public/app/features/dashboard/components/DashNav/DashNavTimeControls.test.tsx

@@ -0,0 +1,66 @@
+import React from 'react';
+import { render } from '@testing-library/react';
+import { DashNavTimeControls } from './DashNavTimeControls';
+import { DashboardModel } from '../../state/DashboardModel';
+import { getDashboardModel } from '../../../../../test/helpers/getDashboardModel';
+
+describe('DashNavTimeControls', () => {
+  let dashboardModel: DashboardModel;
+
+  beforeEach(() => {
+    const json = {
+      panels: [
+        {
+          datasource: null,
+          gridPos: {
+            h: 3,
+            w: 24,
+            x: 0,
+            y: 8,
+          },
+          id: 1,
+          type: 'welcome',
+        },
+      ],
+      refresh: '',
+      templating: {
+        list: [],
+      },
+    };
+    dashboardModel = getDashboardModel(json);
+  });
+
+  it('renders RefreshPicker with run button in panel view', () => {
+    const container = render(
+      <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
+    );
+    expect(container.queryByLabelText(/RefreshPicker run button/i)).toBeInTheDocument();
+  });
+
+  it('renders RefreshPicker with interval button in panel view', () => {
+    const container = render(
+      <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
+    );
+    expect(container.queryByLabelText(/RefreshPicker interval button/i)).toBeInTheDocument();
+  });
+
+  it('should not render RefreshPicker interval button in panel edit', () => {
+    const panel: any = { destroy: jest.fn(), isEditing: true };
+    dashboardModel.startRefresh = jest.fn();
+    dashboardModel.panelInEdit = panel;
+    const container = render(
+      <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
+    );
+    expect(container.queryByLabelText(/RefreshPicker interval button/i)).not.toBeInTheDocument();
+  });
+
+  it('should render RefreshPicker run button in panel edit', () => {
+    const panel: any = { destroy: jest.fn(), isEditing: true };
+    dashboardModel.startRefresh = jest.fn();
+    dashboardModel.panelInEdit = panel;
+    const container = render(
+      <DashNavTimeControls dashboard={dashboardModel} onChangeTimeZone={jest.fn()} key="time-controls" />
+    );
+    expect(container.queryByLabelText(/RefreshPicker run button/i)).toBeInTheDocument();
+  });
+});

public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx

@@ -88,6 +88,7 @@ export class DashNavTimeControls extends Component<Props> {
     const timePickerValue = getTimeSrv().timeRange();
     const timeZone = dashboard.getTimezone();
     const styles = getStyles();
+    const hideIntervalPicker = dashboard.panelInEdit?.isEditing;
 
     return (
       <div className={styles.container}>
@@ -106,6 +107,7 @@ export class DashNavTimeControls extends Component<Props> {
           value={dashboard.refresh}
           intervals={intervals}
           tooltip="Refresh dashboard"
+          noIntervalPicker={hideIntervalPicker}
         />
       </div>
     );

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

@@ -187,4 +187,26 @@ describe('timeSrv', () => {
       expect(_dashboard.refresh).toBe('10s');
     });
   });
+
+  describe('pauseAutoRefresh', () => {
+    it('should set refresh to empty value', () => {
+      _dashboard.refresh = '10s';
+      timeSrv.pauseAutoRefresh();
+      expect(_dashboard.refresh).toBe('');
+    });
+
+    it('should set previousAutoRefresh value', () => {
+      _dashboard.refresh = '10s';
+      timeSrv.pauseAutoRefresh();
+      expect(timeSrv.previousAutoRefresh).toBe('10s');
+    });
+  });
+
+  describe('resumeAutoRefresh', () => {
+    it('should set refresh to empty value', () => {
+      timeSrv.previousAutoRefresh = '10s';
+      timeSrv.resumeAutoRefresh();
+      expect(_dashboard.refresh).toBe('10s');
+    });
+  });
 });

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

@@ -22,6 +22,7 @@ export class TimeSrv {
   time: any;
   refreshTimer: any;
   refresh: any;
+  previousAutoRefresh: any;
   oldRefresh: string | null | undefined;
   dashboard?: DashboardModel;
   timeAtLoad: any;
@@ -233,6 +234,18 @@ export class TimeSrv {
     clearTimeout(this.refreshTimer);
   }
 
+  // store dashboard refresh value and pause auto-refresh in some places
+  // i.e panel edit
+  pauseAutoRefresh() {
+    this.previousAutoRefresh = this.dashboard?.refresh;
+    this.setAutoRefresh('');
+  }
+
+  // resume auto-refresh based on old dashboard refresh property
+  resumeAutoRefresh() {
+    this.setAutoRefresh(this.previousAutoRefresh);
+  }
+
   setTime(time: RawTimeRange, fromRouteUpdate?: boolean) {
     extend(this.time, time);
 

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

@@ -7,8 +7,10 @@ import { createAdHocVariableAdapter } from '../../variables/adhoc/adapter';
 import { createQueryVariableAdapter } from '../../variables/query/adapter';
 import { createCustomVariableAdapter } from '../../variables/custom/adapter';
 import { expect } from '../../../../test/lib/common';
+import { setTimeSrv, TimeSrv } from '../services/TimeSrv';
 
 jest.mock('app/core/services/context_srv', () => ({}));
+
 variableAdapters.setInit(() => [
   createQueryVariableAdapter(),
   createAdHocVariableAdapter(),
@@ -815,13 +817,21 @@ describe('exitViewPanel', () => {
 });
 
 describe('exitPanelEditor', () => {
-  function getTestContext() {
+  function getTestContext(setPreviousAutoRefresh = false) {
     const panel: any = { destroy: jest.fn() };
     const dashboard = new DashboardModel({});
+    const timeSrvMock = ({
+      pauseAutoRefresh: jest.fn(),
+      resumeAutoRefresh: jest.fn(),
+      setAutoRefresh: jest.fn(),
+    } as unknown) as TimeSrv;
     dashboard.startRefresh = jest.fn();
     dashboard.panelInEdit = panel;
-
-    return { dashboard, panel };
+    if (setPreviousAutoRefresh) {
+      timeSrvMock.previousAutoRefresh = '5s';
+    }
+    setTimeSrv(timeSrvMock);
+    return { dashboard, panel, timeSrvMock };
   }
 
   describe('when called', () => {
@@ -849,8 +859,14 @@ describe('exitPanelEditor', () => {
       expect(dashboard.startRefresh).not.toHaveBeenCalled();
     });
 
+    it('then auto refresh property is resumed', () => {
+      const { dashboard, timeSrvMock } = getTestContext(true);
+      dashboard.exitPanelEditor();
+      expect(timeSrvMock.resumeAutoRefresh).toHaveBeenCalled();
+    });
+
     describe('and there is a change that affects all panels', () => {
-      it('then startRefresh is not called', () => {
+      it('then startRefresh is called', () => {
         const { dashboard } = getTestContext();
         dashboard.setChangeAffectsAllPanels();
 
@@ -885,3 +901,31 @@ describe('setChangeAffectsAllPanels', () => {
     }
   );
 });
+
+describe('initEditPanel', () => {
+  function getTestContext() {
+    const dashboard = new DashboardModel({});
+    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('then auto-refresh is paused', () => {
+      const { dashboard, timeSrvMock } = getTestContext();
+      dashboard.addPanel({ type: 'timeseries' });
+      dashboard.initEditPanel(dashboard.panels[0]);
+      expect(timeSrvMock.pauseAutoRefresh).toHaveBeenCalled();
+    });
+  });
+});

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

@@ -42,6 +42,7 @@ import { onTimeRangeUpdated } from 'app/features/variables/state/actions';
 import { dispatch } from '../../../store/store';
 import { isAllVariable } from '../../variables/utils';
 import { DashboardPanelsChangedEvent, RefreshEvent, RenderEvent } from 'app/types/events';
+import { getTimeSrv } from '../services/TimeSrv';
 
 export interface CloneOptions {
   saveVariables?: boolean;
@@ -356,6 +357,7 @@ export class DashboardModel {
   }
 
   initEditPanel(sourcePanel: PanelModel): PanelModel {
+    getTimeSrv().pauseAutoRefresh();
     this.panelInEdit = sourcePanel.getEditClone();
     return this.panelInEdit;
   }
@@ -372,6 +374,7 @@ export class DashboardModel {
   }
 
   exitPanelEditor() {
+    getTimeSrv().resumeAutoRefresh();
     this.panelInEdit!.destroy();
     this.panelInEdit = undefined;
     this.refreshIfChangeAffectsAllPanels();