DashboardGridItem: Fixes repeating panels issue when variable depends on time range (#92661)

* DashboardGridItem: Fixes repeating panels issue when variable depends on time range

* tests

---------

Co-authored-by: Victor Marin <victor.marin@grafana.com>
This commit is contained in:
Torkel Ödegaard 2024-08-30 14:50:09 +02:00 committed by GitHub
parent 00ae49a61a
commit 31c9084a3a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 85 additions and 17 deletions

View File

@ -1,3 +1,4 @@
import { VariableRefresh } from '@grafana/data';
import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks';
import { setPluginImportUtils } from '@grafana/runtime'; import { setPluginImportUtils } from '@grafana/runtime';
import { SceneGridLayout, VizPanel } from '@grafana/scenes'; import { SceneGridLayout, VizPanel } from '@grafana/scenes';
@ -11,6 +12,12 @@ setPluginImportUtils({
getPanelPluginFromCache: (id: string) => undefined, getPanelPluginFromCache: (id: string) => undefined,
}); });
const mockGetQueryRunnerFor = jest.fn();
jest.mock('../utils/utils', () => ({
...jest.requireActual('../utils/utils'),
getQueryRunnerFor: jest.fn().mockImplementation(() => mockGetQueryRunnerFor()),
}));
describe('PanelRepeaterGridItem', () => { describe('PanelRepeaterGridItem', () => {
it('Given scene with variable with 2 values', async () => { it('Given scene with variable with 2 values', async () => {
const { scene, repeater } = buildPanelRepeaterScene({ variableQueryTime: 0 }); const { scene, repeater } = buildPanelRepeaterScene({ variableQueryTime: 0 });
@ -40,6 +47,32 @@ describe('PanelRepeaterGridItem', () => {
expect(repeater.state.repeatedPanels?.length).toBe(5); expect(repeater.state.repeatedPanels?.length).toBe(5);
}); });
it('Should update panels on refresh if variables load on time range change', async () => {
const { scene, repeater } = buildPanelRepeaterScene({
variableQueryTime: 0,
variableRefresh: VariableRefresh.onTimeRangeChanged,
});
const notifyPanelsSpy = jest.spyOn(repeater, 'notifyRepeatedPanelsWaitingForVariables');
activateFullSceneTree(scene);
expect(repeater.state.repeatedPanels?.length).toBe(5);
expect(notifyPanelsSpy).toHaveBeenCalledTimes(0);
scene.state.$timeRange?.onRefresh();
//make sure notifier is called
expect(notifyPanelsSpy).toHaveBeenCalledTimes(1);
//make sure getQueryRunner is called for each repeated panel
expect(mockGetQueryRunnerFor).toHaveBeenCalledTimes(5);
notifyPanelsSpy.mockRestore();
mockGetQueryRunnerFor.mockClear();
});
it('Should display a panel when there are no options', async () => { it('Should display a panel when there are no options', async () => {
const { scene, repeater } = buildPanelRepeaterScene({ variableQueryTime: 1, numberOfOptions: 0 }); const { scene, repeater } = buildPanelRepeaterScene({ variableQueryTime: 1, numberOfOptions: 0 });

View File

@ -7,7 +7,6 @@ import { config } from '@grafana/runtime';
import { import {
VizPanel, VizPanel,
SceneObjectBase, SceneObjectBase,
VariableDependencyConfig,
SceneGridLayout, SceneGridLayout,
SceneVariableSet, SceneVariableSet,
SceneComponentProps, SceneComponentProps,
@ -20,10 +19,12 @@ import {
VizPanelMenu, VizPanelMenu,
VizPanelState, VizPanelState,
VariableValueSingle, VariableValueSingle,
SceneVariable,
SceneVariableDependencyConfigLike,
} from '@grafana/scenes'; } from '@grafana/scenes';
import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN } from 'app/core/constants'; import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN } from 'app/core/constants';
import { getMultiVariableValues } from '../utils/utils'; import { getMultiVariableValues, getQueryRunnerFor } from '../utils/utils';
import { AddLibraryPanelDrawer } from './AddLibraryPanelDrawer'; import { AddLibraryPanelDrawer } from './AddLibraryPanelDrawer';
import { LibraryVizPanel } from './LibraryVizPanel'; import { LibraryVizPanel } from './LibraryVizPanel';
@ -45,10 +46,7 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
private _libPanelSubscription: Unsubscribable | undefined; private _libPanelSubscription: Unsubscribable | undefined;
private _prevRepeatValues?: VariableValueSingle[]; private _prevRepeatValues?: VariableValueSingle[];
protected _variableDependency = new VariableDependencyConfig(this, { protected _variableDependency = new DashboardGridItemVariableDependencyHandler(this);
variableNames: this.state.variableName ? [this.state.variableName] : [],
onVariableUpdateCompleted: this._onVariableUpdateCompleted.bind(this),
});
public constructor(state: DashboardGridItemState) { public constructor(state: DashboardGridItemState) {
super(state); super(state);
@ -59,7 +57,7 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
private _activationHandler() { private _activationHandler() {
if (this.state.variableName) { if (this.state.variableName) {
this._subs.add(this.subscribeToState((newState, prevState) => this._handleGridResize(newState, prevState))); this._subs.add(this.subscribeToState((newState, prevState) => this._handleGridResize(newState, prevState)));
this._performRepeat(); this.performRepeat();
} }
// Subscriptions that handles body updates, i.e. VizPanel -> LibraryVizPanel, AddLibPanelWidget -> LibraryVizPanel // Subscriptions that handles body updates, i.e. VizPanel -> LibraryVizPanel, AddLibPanelWidget -> LibraryVizPanel
@ -92,23 +90,16 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
this._libPanelSubscription = panel.subscribeToState((newState) => { this._libPanelSubscription = panel.subscribeToState((newState) => {
if (newState._loadedPanel?.model.repeat) { if (newState._loadedPanel?.model.repeat) {
this._variableDependency.setVariableNames([newState._loadedPanel.model.repeat]);
this.setState({ this.setState({
variableName: newState._loadedPanel.model.repeat, variableName: newState._loadedPanel.model.repeat,
repeatDirection: newState._loadedPanel.model.repeatDirection, repeatDirection: newState._loadedPanel.model.repeatDirection,
maxPerRow: newState._loadedPanel.model.maxPerRow, maxPerRow: newState._loadedPanel.model.maxPerRow,
}); });
this._performRepeat(); this.performRepeat();
} }
}); });
} }
private _onVariableUpdateCompleted(): void {
if (this.state.variableName) {
this._performRepeat();
}
}
/** /**
* Uses the current repeat item count to calculate the user intended desired itemHeight * Uses the current repeat item count to calculate the user intended desired itemHeight
*/ */
@ -134,11 +125,12 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
} }
} }
private _performRepeat() { public performRepeat() {
if (this.state.body instanceof AddLibraryPanelDrawer) { if (this.state.body instanceof AddLibraryPanelDrawer) {
return; return;
} }
if (!this.state.variableName || this._variableDependency.hasDependencyInLoadingState()) {
if (!this.state.variableName || sceneGraph.hasVariableDependencyInLoadingState(this)) {
return; return;
} }
@ -160,6 +152,9 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
const { values, texts } = getMultiVariableValues(variable); const { values, texts } = getMultiVariableValues(variable);
if (isEqual(this._prevRepeatValues, values)) { if (isEqual(this._prevRepeatValues, values)) {
// In some cases, like for variables that depend on time range, the panel query runners are waiting for the top level variable to complete
// So even when there was no change in the variable value (like in this case) we need to notify the query runners that the variable has completed it's update
this.notifyRepeatedPanelsWaitingForVariables(variable);
return; return;
} }
@ -227,6 +222,15 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
this.publishEvent(new DashboardRepeatsProcessedEvent({ source: this }), true); this.publishEvent(new DashboardRepeatsProcessedEvent({ source: this }), true);
} }
public notifyRepeatedPanelsWaitingForVariables(variable: SceneVariable) {
for (const panel of this.state.repeatedPanels ?? []) {
const queryRunner = getQueryRunnerFor(panel);
if (queryRunner) {
queryRunner.variableDependency?.variableUpdateCompleted(variable, false);
}
}
}
public getMaxPerRow(): number { public getMaxPerRow(): number {
return this.state.maxPerRow ?? 4; return this.state.maxPerRow ?? 4;
} }
@ -278,6 +282,34 @@ export class DashboardGridItem extends SceneObjectBase<DashboardGridItemState> i
}; };
} }
export class DashboardGridItemVariableDependencyHandler implements SceneVariableDependencyConfigLike {
constructor(private _gridItem: DashboardGridItem) {}
getNames(): Set<string> {
if (this._gridItem.state.variableName) {
return new Set([this._gridItem.state.variableName]);
}
return new Set();
}
hasDependencyOn(name: string): boolean {
return this._gridItem.state.variableName === name;
}
variableUpdateCompleted(variable: SceneVariable, hasChanged: boolean): void {
if (this._gridItem.state.variableName === variable.state.name) {
/**
* We do not really care if the variable has changed or not as we do an equality check in performRepeat
* And this function needs to be called even when variable valued id not change as performRepeat calls
* notifyRepeatedPanelsWaitingForVariables which is needed to notify panels waiting for variable to complete (even when the value did not change)
* This is for scenarios where the variable used for repeating is depending on time range.
*/
this._gridItem.performRepeat();
}
}
}
function useLayoutStyle(direction: RepeatDirection, itemCount: number, maxPerRow: number, itemHeight: number) { function useLayoutStyle(direction: RepeatDirection, itemCount: number, maxPerRow: number, itemHeight: number) {
return useMemo(() => { return useMemo(() => {
const theme = config.theme2; const theme = config.theme2;

View File

@ -1,3 +1,4 @@
import { VariableRefresh } from '@grafana/data';
import { import {
DeepPartial, DeepPartial,
EmbeddedScene, EmbeddedScene,
@ -103,6 +104,7 @@ interface SceneOptions {
usePanelRepeater?: boolean; usePanelRepeater?: boolean;
useRowRepeater?: boolean; useRowRepeater?: boolean;
throwError?: string; throwError?: string;
variableRefresh?: VariableRefresh;
} }
export function buildPanelRepeaterScene(options: SceneOptions, source?: VizPanel | LibraryVizPanel) { export function buildPanelRepeaterScene(options: SceneOptions, source?: VizPanel | LibraryVizPanel) {
@ -157,6 +159,7 @@ export function buildPanelRepeaterScene(options: SceneOptions, source?: VizPanel
{ label: 'E', value: '5' }, { label: 'E', value: '5' },
].slice(0, options.numberOfOptions), ].slice(0, options.numberOfOptions),
throwError: defaults.throwError, throwError: defaults.throwError,
refresh: options.variableRefresh,
}); });
const rowRepeatVariable = new TestVariable({ const rowRepeatVariable = new TestVariable({