Variables: Fix repeating panels for on time range changed variables (#42828)

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

@@ -8,7 +8,8 @@ import { afterEach, beforeEach } from '../../../../test/lib/common';
 function getTestContext({
   usePanelInEdit,
   usePanelInView,
-}: { usePanelInEdit?: boolean; usePanelInView?: boolean } = {}) {
+  refreshAll = false,
+}: { usePanelInEdit?: boolean; usePanelInView?: boolean; refreshAll?: boolean } = {}) {
   jest.clearAllMocks();
 
   const dashboard = new DashboardModel({});
@@ -26,7 +27,7 @@ function getTestContext({
     panelIds.push(panelInView.id);
   }
 
-  appEvents.publish(new VariablesChanged({ panelIds }));
+  appEvents.publish(new VariablesChanged({ panelIds, refreshAll }));
 
   return { dashboard, startRefreshMock, panelInEdit, panelInView };
 }
@@ -37,7 +38,16 @@ describe('Strict panel refresh', () => {
       const { startRefreshMock } = getTestContext();
 
       expect(startRefreshMock).toHaveBeenCalledTimes(1);
-      expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3]);
+      expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3], refreshAll: false });
+    });
+  });
+
+  describe('when there is no panel in full view or panel in panel edit during variable change but refreshAll is true', () => {
+    it('then all affected panels should be refreshed', () => {
+      const { startRefreshMock } = getTestContext({ refreshAll: true });
+
+      expect(startRefreshMock).toHaveBeenCalledTimes(1);
+      expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [], refreshAll: true });
     });
   });
 
@@ -61,7 +71,7 @@ describe('Strict panel refresh', () => {
         const { startRefreshMock } = getTestContext();
 
         expect(startRefreshMock).toHaveBeenCalledTimes(1);
-        expect(startRefreshMock).toHaveBeenLastCalledWith(undefined);
+        expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [], refreshAll: true });
       });
     });
 
@@ -71,7 +81,17 @@ describe('Strict panel refresh', () => {
         const { startRefreshMock } = getTestContext();
 
         expect(startRefreshMock).toHaveBeenCalledTimes(1);
-        expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3]);
+        expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3], refreshAll: false });
+      });
+    });
+
+    describe('when the dashboard has been refreshed within the threshold but refreshAll is true', () => {
+      it('then all affected panels should be refreshed', () => {
+        isRefreshOutsideThreshold = false;
+        const { startRefreshMock } = getTestContext({ refreshAll: true });
+
+        expect(startRefreshMock).toHaveBeenCalledTimes(1);
+        expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [], refreshAll: true });
       });
     });
   });
@@ -81,7 +101,7 @@ describe('Strict panel refresh', () => {
       const { panelInView, startRefreshMock } = getTestContext({ usePanelInView: true });
 
       expect(startRefreshMock).toHaveBeenCalledTimes(1);
-      expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3, panelInView.id]);
+      expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3, panelInView.id], refreshAll: false });
     });
 
     describe('and when exitViewPanel is called', () => {
@@ -92,7 +112,7 @@ describe('Strict panel refresh', () => {
         dashboard.exitViewPanel(panelInView);
 
         expect(startRefreshMock).toHaveBeenCalledTimes(1);
-        expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3]);
+        expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3], refreshAll: false });
         expect(dashboard['panelsAffectedByVariableChange']).toBeNull();
       });
     });
@@ -102,7 +122,7 @@ describe('Strict panel refresh', () => {
     it('then all affected panels should be refreshed', () => {
       const { panelInEdit, startRefreshMock } = getTestContext({ usePanelInEdit: true });
       expect(startRefreshMock).toHaveBeenCalledTimes(1);
-      expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3, panelInEdit.id]);
+      expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3, panelInEdit.id], refreshAll: false });
     });
 
     describe('and when exitViewPanel is called', () => {
@@ -113,7 +133,7 @@ describe('Strict panel refresh', () => {
         dashboard.exitPanelEditor();
 
         expect(startRefreshMock).toHaveBeenCalledTimes(1);
-        expect(startRefreshMock).toHaveBeenLastCalledWith([1, 2, 3]);
+        expect(startRefreshMock).toHaveBeenLastCalledWith({ panelIds: [1, 2, 3], refreshAll: false });
         expect(dashboard['panelsAffectedByVariableChange']).toBeNull();
       });
     });

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

@@ -49,11 +49,7 @@ import { RefreshEvent, TimeRangeUpdatedEvent } from '@grafana/runtime';
 import { sortedDeepCloneWithoutNulls } from 'app/core/utils/object';
 import { Subscription } from 'rxjs';
 import { appEvents } from '../../../core/core';
-import {
-  VariablesChanged,
-  VariablesChangedInUrl,
-  VariablesFinishedProcessingTimeRangeChange,
-} from '../../variables/types';
+import { VariablesChanged, VariablesChangedEvent, VariablesChangedInUrl } from '../../variables/types';
 
 export interface CloneOptions {
   saveVariables?: boolean;
@@ -182,12 +178,6 @@ export class DashboardModel {
     this.appEventsSubscription = new Subscription();
     this.lastRefresh = Date.now();
     this.appEventsSubscription.add(appEvents.subscribe(VariablesChanged, this.variablesChangedHandler.bind(this)));
-    this.appEventsSubscription.add(
-      appEvents.subscribe(
-        VariablesFinishedProcessingTimeRangeChange,
-        this.variablesFinishedProcessingTimeRangeChangeHandler.bind(this)
-      )
-    );
     this.appEventsSubscription.add(
       appEvents.subscribe(VariablesChangedInUrl, this.variablesChangedInUrlHandler.bind(this))
     );
@@ -378,12 +368,12 @@ export class DashboardModel {
     dispatch(onTimeRangeUpdated(timeRange));
   }
 
-  startRefresh(affectedPanelIds?: number[]) {
+  startRefresh(event: VariablesChangedEvent = { refreshAll: true, panelIds: [] }) {
     this.events.publish(new RefreshEvent());
     this.lastRefresh = Date.now();
 
     if (this.panelInEdit) {
-      if (!affectedPanelIds || affectedPanelIds.includes(this.panelInEdit.id)) {
+      if (event.refreshAll || event.panelIds.includes(this.panelInEdit.id)) {
         this.panelInEdit.refresh();
         return;
       }
@@ -391,7 +381,7 @@ export class DashboardModel {
 
     for (const panel of this.panels) {
       if (!this.otherPanelInFullscreen(panel)) {
-        if (!affectedPanelIds || affectedPanelIds.includes(panel.id)) {
+        if (event.refreshAll || event.panelIds.includes(panel.id)) {
           panel.refresh();
         }
       }
@@ -446,7 +436,7 @@ export class DashboardModel {
       return;
     }
 
-    this.startRefresh(this.panelsAffectedByVariableChange);
+    this.startRefresh({ panelIds: this.panelsAffectedByVariableChange, refreshAll: false });
     this.panelsAffectedByVariableChange = null;
   }
 
@@ -1218,24 +1208,10 @@ export class DashboardModel {
   }
 
   private variablesChangedHandler(event: VariablesChanged) {
-    this.variablesChangedBaseHandler(event, true);
-  }
+    this.processRepeats();
 
-  private variablesFinishedProcessingTimeRangeChangeHandler(event: VariablesFinishedProcessingTimeRangeChange) {
-    this.variablesChangedBaseHandler(event);
-  }
-
-  private variablesChangedBaseHandler(
-    event: VariablesChanged | VariablesFinishedProcessingTimeRangeChange,
-    processRepeats = false
-  ) {
-    if (processRepeats) {
-      this.processRepeats();
-    }
-
-    if (!event.payload.panelIds || getTimeSrv().isRefreshOutsideThreshold(this.lastRefresh)) {
-      // passing undefined in panelIds means we want to update all panels
-      this.startRefresh(undefined);
+    if (event.payload.refreshAll || getTimeSrv().isRefreshOutsideThreshold(this.lastRefresh)) {
+      this.startRefresh({ refreshAll: true, panelIds: [] });
       return;
     }
 
@@ -1245,11 +1221,11 @@ export class DashboardModel {
       );
     }
 
-    this.startRefresh(event.payload.panelIds);
+    this.startRefresh(event.payload);
   }
 
   private variablesChangedInUrlHandler(event: VariablesChangedInUrl) {
     this.templateVariableValueUpdated();
-    this.startRefresh(event.payload.panelIds);
+    this.startRefresh(event.payload);
   }
 }

public/app/features/variables/state/actions.ts

@@ -13,8 +13,8 @@ import {
   VariableOption,
   VariableRefresh,
   VariablesChanged,
+  VariablesChangedEvent,
   VariablesChangedInUrl,
-  VariablesFinishedProcessingTimeRangeChange,
   VariableWithMultiSupport,
   VariableWithOptions,
 } from '../types';
@@ -518,9 +518,9 @@ export const variableUpdated = (
     const variables = getVariables(state);
     const g = createGraph(variables);
     const panels = state.dashboard?.getModel()?.panels ?? [];
-    const affectedPanelIds = isAdHoc(variableInState)
-      ? undefined // for adhoc variables we don't know which panels that will be impacted
-      : getAllAffectedPanelIdsForVariableChange(variableInState.id, variables, panels);
+    const event: VariablesChangedEvent = isAdHoc(variableInState)
+      ? { refreshAll: true, panelIds: [] } // for adhoc variables we don't know which panels that will be impacted
+      : { refreshAll: false, panelIds: getAllAffectedPanelIdsForVariableChange(variableInState.id, variables, panels) };
 
     const node = g.getNode(variableInState.name);
     let promises: Array<Promise<any>> = [];
@@ -537,7 +537,7 @@ export const variableUpdated = (
 
     return Promise.all(promises).then(() => {
       if (emitChangeEvents) {
-        events.publish(new VariablesChanged({ panelIds: affectedPanelIds }));
+        events.publish(new VariablesChanged(event));
         locationService.partial(getQueryWithVariables(getState));
       }
     });
@@ -569,7 +569,7 @@ export const onTimeRangeUpdated = (
 
   try {
     await Promise.all(promises);
-    dependencies.events.publish(new VariablesFinishedProcessingTimeRangeChange({ panelIds: undefined }));
+    dependencies.events.publish(new VariablesChanged({ panelIds: [], refreshAll: true }));
   } catch (error) {
     console.error(error);
     dispatch(notifyApp(createVariableErrorNotification('Template variable service failed', error)));
@@ -625,7 +625,7 @@ export const templateVarsChangedInUrl = (
 
   if (update.length) {
     await Promise.all(update);
-    events.publish(new VariablesChangedInUrl({ panelIds: undefined }));
+    events.publish(new VariablesChangedInUrl({ panelIds: [], refreshAll: true }));
   }
 };
 

public/app/features/variables/types.ts

@@ -154,17 +154,14 @@ export type VariableQueryEditorType<
 > = ComponentType<VariableQueryProps> | ComponentType<QueryEditorProps<any, TQuery, TOptions, any>> | null;
 
 export interface VariablesChangedEvent {
-  panelIds?: number[];
+  refreshAll: boolean;
+  panelIds: number[];
 }
 
 export class VariablesChanged extends BusEventWithPayload<VariablesChangedEvent> {
   static type = 'variables-changed';
 }
 
-export class VariablesFinishedProcessingTimeRangeChange extends BusEventWithPayload<VariablesChangedEvent> {
-  static type = 'variables-finished-processing-time-range-change';
-}
-
 export class VariablesChangedInUrl extends BusEventWithPayload<VariablesChangedEvent> {
   static type = 'variables-changed-in-url';
 }