[v11.0.x] DashboardScene: Fixing major row repeat issues (#87800)

DashboardScene: Fixing major row repeat issues (#87539)

* DashboardScene: Fixing major row repeat issues

* Fixing edit scope

* Use dashboard variableDependendency to notify row repeat behaviors

* update scenes lib

* Do not repeat if values are the same

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

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>

* Updated scenes

* Update

* Update

* Do not render row actions for repeated rows

* Fixed e2e

---------

Co-authored-by: Dominik Prokop <dominik.prokop@grafana.com>
(cherry picked from commit 9cd7c87)

e2e/various-suite/solo-route.spec.ts

@@ -35,7 +35,7 @@ describe('Solo Route', () => {
   it('Can view solo in repeaterd row and panel in scenes', () => {
     // open Panel Tests - Graph NG
     e2e.pages.SoloPanel.visit(
-      'Repeating-rows-uid/repeating-rows?orgId=1&var-server=A&var-server=B&var-server=D&var-pod=1&var-pod=2&var-pod=3&panelId=panel-2-row-2-clone-2&__feature.dashboardSceneSolo=true'
+      'Repeating-rows-uid/repeating-rows?orgId=1&var-server=A&var-server=B&var-server=D&var-pod=1&var-pod=2&var-pod=3&panelId=panel-2-clone-D-clone-2&__feature.dashboardSceneSolo=true'
     );
 
     e2e.components.Panels.Panel.title('server = D, pod = Sod').should('exist');

package.json

@@ -254,7 +254,7 @@
     "@grafana/prometheus": "workspace:*",
     "@grafana/runtime": "workspace:*",
     "@grafana/saga-icons": "workspace:*",
-    "@grafana/scenes": "^4.14.0",
+    "@grafana/scenes": "^4.21.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

@@ -4,7 +4,14 @@ import { DataQueryRequest, DataSourceApi, DataSourceInstanceSettings, LoadingSta
 import { calculateFieldTransformer } from '@grafana/data/src/transformations/transformers/calculateField';
 import { mockTransformationsRegistry } from '@grafana/data/src/utils/tests/mockTransformationsRegistry';
 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';
@@ -813,6 +820,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

@@ -23,6 +23,7 @@ import {
   SceneObjectState,
   SceneObjectStateChangedEvent,
   SceneQueryRunner,
+  SceneVariables,
   VizPanel,
   sceneUtils,
 } from '@grafana/scenes';
@@ -91,7 +92,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
@@ -846,6 +847,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;
   }
@@ -859,5 +862,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: [] }));
     }
+
+    /**
+     * Propagate 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,42 @@ 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';
+import { RowActions } from './row-actions/RowActions';
 
 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');
+      expect(row1.state.actions).toBeDefined();
 
       const row2 = grid.state.children[2] as SceneGridRow;
       expect(row2.state.$variables!.state.variables[0].getValueText?.()).toBe('B');
+      expect(row2.state.actions).toBeUndefined();
 
       // 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 +75,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 +127,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: [
@@ -140,14 +145,28 @@ function buildScene(options: SceneOptions) {
         y: 10,
         width: 24,
         height: 1,
+        actions: new RowActions({}),
         $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,
         y: 16,
         width: 24,
         height: 5,
         title: 'Row at the bottom',
+
         children: [
           new SceneGridItem({
             key: 'griditem-2',
@@ -172,7 +191,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 +204,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 };
 }