DashboardScene: Fixing major row repeat issues

package.json

@@ -258,7 +258,7 @@
     "@grafana/prometheus": "workspace:*",
     "@grafana/runtime": "workspace:*",
     "@grafana/saga-icons": "workspace:*",
-    "@grafana/scenes": "^4.14.0",
+    "@grafana/scenes": "4.17.4--canary.732.9028593616.0",
     "@grafana/schema": "workspace:*",
     "@grafana/sql": "workspace:*",
     "@grafana/ui": "workspace:*",

public/app/features/dashboard-scene/panel-edit/PanelEditor.tsx

@@ -113,29 +113,30 @@ export class PanelEditor extends SceneObjectBase<PanelEditorState> {
       return;
     }
 
-    if (gridItem instanceof DashboardGridItem) {
-      this.handleRepeatOptionChanges(gridItem);
-    } else {
+    if (!(gridItem instanceof DashboardGridItem)) {
       console.error('Unsupported scene object type');
+      return;
     }
+
+    this.commitChangesToSource(gridItem);
   }
 
-  private handleRepeatOptionChanges(panelRepeater: DashboardGridItem) {
-    let width = panelRepeater.state.width ?? 1;
-    let height = panelRepeater.state.height;
+  private commitChangesToSource(gridItem: DashboardGridItem) {
+    let width = gridItem.state.width ?? 1;
+    let height = gridItem.state.height;
 
     const panelManager = this.state.vizManager;
     const horizontalToVertical =
       this._initialRepeatOptions.repeatDirection === 'h' && panelManager.state.repeatDirection === 'v';
     const verticalToHorizontal =
       this._initialRepeatOptions.repeatDirection === 'v' && panelManager.state.repeatDirection === 'h';
     if (horizontalToVertical) {
-      width = Math.floor(width / (panelRepeater.state.maxPerRow ?? 1));
+      width = Math.floor(width / (gridItem.state.maxPerRow ?? 1));
     } else if (verticalToHorizontal) {
       width = 24;
     }
 
-    panelRepeater.setState({
+    gridItem.setState({
       body: panelManager.state.panel.clone(),
       repeatDirection: panelManager.state.repeatDirection,
       variableName: panelManager.state.repeat,

public/app/features/dashboard-scene/panel-edit/VizPanelManager.test.tsx

@@ -2,7 +2,14 @@ import { map, of } from 'rxjs';
 
 import { DataQueryRequest, DataSourceApi, DataSourceInstanceSettings, LoadingState, PanelData } from '@grafana/data';
 import { config, locationService } from '@grafana/runtime';
-import { SceneQueryRunner, VizPanel } from '@grafana/scenes';
+import {
+  LocalValueVariable,
+  SceneGridRow,
+  SceneQueryRunner,
+  SceneVariableSet,
+  VizPanel,
+  sceneGraph,
+} from '@grafana/scenes';
 import { DataQuery, DataSourceJsonData, DataSourceRef } from '@grafana/schema';
 import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
 import { InspectTab } from 'app/features/inspector/types';
@@ -784,6 +791,25 @@ describe('VizPanelManager', () => {
     expect(vizPanelManager.state.datasource).toEqual(ds1Mock);
     expect(vizPanelManager.state.dsSettings).toEqual(instance1SettingsMock);
   });
+
+  describe('Given a panel inside repeated row', () => {
+    it('Should include row variable scope', () => {
+      const { panel } = setupTest('panel-9');
+
+      const row = panel.parent?.parent;
+      if (!(row instanceof SceneGridRow)) {
+        throw new Error('Did not find parent row');
+      }
+
+      row.setState({
+        $variables: new SceneVariableSet({ variables: [new LocalValueVariable({ name: 'hello', value: 'A' })] }),
+      });
+
+      const editor = buildPanelEditScene(panel);
+      const variable = sceneGraph.lookupVariable('hello', editor.state.vizManager);
+      expect(variable?.getValue()).toBe('A');
+    });
+  });
 });
 
 const setupTest = (panelId: string) => {

public/app/features/dashboard-scene/panel-edit/VizPanelManager.tsx

@@ -21,6 +21,7 @@ import {
   SceneObjectRef,
   SceneObjectState,
   SceneQueryRunner,
+  SceneVariables,
   VizPanel,
   sceneUtils,
 } from '@grafana/scenes';
@@ -86,7 +87,14 @@ export class VizPanelManager extends SceneObjectBase<VizPanelManagerState> {
     const { variableName: repeat, repeatDirection, maxPerRow } = gridItem.state;
     repeatOptions = { repeat, repeatDirection, maxPerRow };
 
+    let variables: SceneVariables | undefined;
+
+    if (gridItem.parent?.state.$variables) {
+      variables = gridItem.parent.state.$variables.clone();
+    }
+
     return new VizPanelManager({
+      $variables: variables,
       panel: sourcePanel.clone(),
       sourcePanel: sourcePanel.getRef(),
       ...repeatOptions,

public/app/features/dashboard-scene/panel-edit/testfiles/testDashboard.ts

@@ -591,6 +591,18 @@ export const panelWithQueriesAndMixedDatasource = {
   type: 'timeseries',
 };
 
+const row = {
+  id: 8,
+  type: 'row',
+  gridPos: { h: 1, w: 24, x: 0, y: 20 },
+};
+
+const rowChild = {
+  id: 9,
+  type: 'timeseries',
+  gridPos: { h: 2, w: 24, x: 0, y: 21 },
+};
+
 export const testDashboard = {
   annotations: {
     list: [
@@ -622,6 +634,8 @@ export const testDashboard = {
     panelWithNoDataSource,
     panelWithDataSourceNotFound,
     panelWithQueriesAndMixedDatasource,
+    row,
+    rowChild,
   ],
   refresh: '',
   schemaVersion: 39,

public/app/features/dashboard-scene/scene/DashboardScene.tsx

@@ -64,6 +64,7 @@ import { DashboardGridItem } from './DashboardGridItem';
 import { DashboardSceneRenderer } from './DashboardSceneRenderer';
 import { DashboardSceneUrlSync } from './DashboardSceneUrlSync';
 import { LibraryVizPanel } from './LibraryVizPanel';
+import { RowRepeaterBehavior } from './RowRepeaterBehavior';
 import { ScopesScene } from './ScopesScene';
 import { ViewPanelScene } from './ViewPanelScene';
 import { setupKeyboardShortcuts } from './keyboardShortcuts';
@@ -127,7 +128,7 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
   /**
    * Get notified when variables change
    */
-  protected _variableDependency = new DashboardVariableDependency();
+  protected _variableDependency = new DashboardVariableDependency(this);
 
   /**
    * State before editing started
@@ -847,6 +848,8 @@ export class DashboardScene extends SceneObjectBase<DashboardSceneState> {
 export class DashboardVariableDependency implements SceneVariableDependencyConfigLike {
   private _emptySet = new Set<string>();
 
+  public constructor(private _dashboard: DashboardScene) {}
+
   getNames(): Set<string> {
     return this._emptySet;
   }
@@ -860,5 +863,28 @@ export class DashboardVariableDependency implements SceneVariableDependencyConfi
       // Temp solution for some core panels (like dashlist) to know that variables have changed
       appEvents.publish(new VariablesChanged({ refreshAll: true, panelIds: [] }));
     }
+
+    /**
+     * Propagete variable changes to repeat row behavior as it does not get it when it's nested under local value
+     * The first repeated row has the row repeater behavior but it also has a local SceneVariableSet with a local variable value
+     */
+    const layout = this._dashboard.state.body;
+    if (!(layout instanceof SceneGridLayout)) {
+      return;
+    }
+
+    for (const child of layout.state.children) {
+      if (!(child instanceof SceneGridRow) || !child.state.$behaviors) {
+        continue;
+      }
+
+      for (const behavior of child.state.$behaviors) {
+        if (behavior instanceof RowRepeaterBehavior) {
+          if (behavior.isWaitingForVariables || (behavior.state.variableName === variable.state.name && hasChanged)) {
+            behavior.performRepeat();
+          }
+        }
+      }
+    }
   }
 }

public/app/features/dashboard-scene/scene/RowRepeaterBehavior.test.tsx

@@ -1,5 +1,4 @@
 import {
-  EmbeddedScene,
   SceneCanvasText,
   SceneGridLayout,
   SceneGridRow,
@@ -13,32 +12,39 @@ import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE } from 'app/features/variables/co
 import { activateFullSceneTree } from '../utils/test-utils';
 
 import { RepeatDirection } from './DashboardGridItem';
+import { DashboardScene } from './DashboardScene';
 import { RowRepeaterBehavior } from './RowRepeaterBehavior';
 
 describe('RowRepeaterBehavior', () => {
   describe('Given scene with variable with 5 values', () => {
-    let scene: EmbeddedScene, grid: SceneGridLayout;
+    let scene: DashboardScene, grid: SceneGridLayout, repeatBehavior: RowRepeaterBehavior;
+    let gridStateUpdates: unknown[];
 
     beforeEach(async () => {
-      ({ scene, grid } = buildScene({ variableQueryTime: 0 }));
+      ({ scene, grid, repeatBehavior } = buildScene({ variableQueryTime: 0 }));
+
+      gridStateUpdates = [];
+      grid.subscribeToState((state) => gridStateUpdates.push(state));
+
       activateFullSceneTree(scene);
       await new Promise((r) => setTimeout(r, 1));
     });
 
     it('Should repeat row', () => {
       // Verify that panel above row remains
       expect(grid.state.children[0]).toBeInstanceOf(SceneGridItem);
+
       // Verify that first row still has repeat behavior
       const row1 = grid.state.children[1] as SceneGridRow;
       expect(row1.state.$behaviors?.[0]).toBeInstanceOf(RowRepeaterBehavior);
-      expect(row1.state.$variables!.state.variables[0].getValue()).toBe('1');
+      expect(row1.state.$variables!.state.variables[0].getValue()).toBe('A1');
 
       const row2 = grid.state.children[2] as SceneGridRow;
       expect(row2.state.$variables!.state.variables[0].getValueText?.()).toBe('B');
 
       // Should give repeated panels unique keys
       const gridItem = row2.state.children[0] as SceneGridItem;
-      expect(gridItem.state.body?.state.key).toBe('canvas-1-row-1');
+      expect(gridItem.state.body?.state.key).toBe('canvas-1-clone-B1');
     });
 
     it('Should push row at the bottom down', () => {
@@ -66,24 +72,34 @@ describe('RowRepeaterBehavior', () => {
     it('Should handle second repeat cycle and update remove old repeats', async () => {
       // trigger another repeat cycle by changing the variable
       const variable = scene.state.$variables!.state.variables[0] as TestVariable;
-      variable.changeValueTo(['2', '3']);
+      variable.changeValueTo(['B1', 'C1']);
 
       await new Promise((r) => setTimeout(r, 1));
 
       // should now only have 2 repeated rows (and the panel above + the row at the bottom)
       expect(grid.state.children.length).toBe(4);
     });
+
+    it('Should ignore repeat process if variable values are the same', async () => {
+      // trigger another repeat cycle by changing the variable
+      repeatBehavior.performRepeat();
+
+      await new Promise((r) => setTimeout(r, 1));
+
+      expect(gridStateUpdates.length).toBe(1);
+    });
   });
 
   describe('Given scene empty row', () => {
-    let scene: EmbeddedScene;
+    let scene: DashboardScene;
     let grid: SceneGridLayout;
-    let repeatBehavior: RowRepeaterBehavior;
+    let rowToRepeat: SceneGridRow;
 
     beforeEach(async () => {
-      ({ scene, grid, repeatBehavior } = buildScene({ variableQueryTime: 0 }));
+      ({ scene, grid, rowToRepeat } = buildScene({ variableQueryTime: 0 }));
+
+      rowToRepeat.setState({ children: [] });
 
-      repeatBehavior.setState({ sources: [] });
       activateFullSceneTree(scene);
       await new Promise((r) => setTimeout(r, 1));
     });
@@ -108,21 +124,7 @@ interface SceneOptions {
 }
 
 function buildScene(options: SceneOptions) {
-  const repeatBehavior = new RowRepeaterBehavior({
-    variableName: 'server',
-    sources: [
-      new SceneGridItem({
-        x: 0,
-        y: 11,
-        width: 24,
-        height: 5,
-        body: new SceneCanvasText({
-          key: 'canvas-1',
-          text: 'Panel inside repeated row, server = $server',
-        }),
-      }),
-    ],
-  });
+  const repeatBehavior = new RowRepeaterBehavior({ variableName: 'server' });
 
   const grid = new SceneGridLayout({
     children: [
@@ -141,6 +143,18 @@ function buildScene(options: SceneOptions) {
         width: 24,
         height: 1,
         $behaviors: [repeatBehavior],
+        children: [
+          new SceneGridItem({
+            x: 0,
+            y: 11,
+            width: 24,
+            height: 5,
+            body: new SceneCanvasText({
+              key: 'canvas-1',
+              text: 'Panel inside repeated row, server = $server',
+            }),
+          }),
+        ],
       }),
       new SceneGridRow({
         x: 0,
@@ -172,7 +186,7 @@ function buildScene(options: SceneOptions) {
     ],
   });
 
-  const scene = new EmbeddedScene({
+  const scene = new DashboardScene({
     $timeRange: new SceneTimeRange({ from: 'now-6h', to: 'now' }),
     $variables: new SceneVariableSet({
       variables: [
@@ -185,17 +199,19 @@ function buildScene(options: SceneOptions) {
           includeAll: true,
           delayMs: options.variableQueryTime,
           optionsToReturn: [
-            { label: 'A', value: '1' },
-            { label: 'B', value: '2' },
-            { label: 'C', value: '3' },
-            { label: 'D', value: '4' },
-            { label: 'E', value: '5' },
+            { label: 'A', value: 'A1' },
+            { label: 'B', value: 'B1' },
+            { label: 'C', value: 'C1' },
+            { label: 'D', value: 'D1' },
+            { label: 'E', value: 'E1' },
           ],
         }),
       ],
     }),
     body: grid,
   });
 
-  return { scene, grid, repeatBehavior };
+  const rowToRepeat = repeatBehavior.parent as SceneGridRow;
+
+  return { scene, grid, repeatBehavior, rowToRepeat };
 }