From 0d55dad075ce0c11ac942f59e212fcdde375f29f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Fri, 13 Oct 2023 16:03:38 +0200 Subject: [PATCH] DashboardScene: Fixes full page reload of fullscreen view of a repeated panel (#76326) * Progress on view panel for repeats * Good enough * Update --- .../ngalert/migration/permissions_test.go | 4 +- .../scene/DashboardScene.test.tsx | 20 ---- .../scene/DashboardSceneUrlSync.test.ts | 91 +++++++++++++++++++ .../scene/DashboardSceneUrlSync.ts | 23 +++++ .../scene/PanelRepeaterGridItem.test.tsx | 48 ++++++---- .../scene/PanelRepeaterGridItem.tsx | 5 + .../scene/RowRepeaterBehavior.ts | 5 + .../features/dashboard-scene/scene/types.ts | 10 ++ 8 files changed, 165 insertions(+), 41 deletions(-) create mode 100644 public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts create mode 100644 public/app/features/dashboard-scene/scene/types.ts diff --git a/pkg/services/ngalert/migration/permissions_test.go b/pkg/services/ngalert/migration/permissions_test.go index c6577e5f259..bc52ddce191 100644 --- a/pkg/services/ngalert/migration/permissions_test.go +++ b/pkg/services/ngalert/migration/permissions_test.go @@ -576,7 +576,7 @@ func TestDashAlertPermissionMigration(t *testing.T) { name: "should ignore fixed roles even if they would affect access", enterprise: true, roles: map[accesscontrol.Role][]accesscontrol.Permission{ - accesscontrol.Role{Name: "fixed:dashboards:writer"}: { + {Name: "fixed:dashboards:writer"}: { {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeDashboardsAll}, {Action: dashboards.ActionDashboardsWrite, Scope: dashboards.ScopeDashboardsAll}, {Action: dashboards.ActionDashboardsDelete, Scope: dashboards.ScopeDashboardsAll}, @@ -602,7 +602,7 @@ func TestDashAlertPermissionMigration(t *testing.T) { name: "should ignore custom roles even if they would affect access", enterprise: true, roles: map[accesscontrol.Role][]accesscontrol.Permission{ - accesscontrol.Role{Name: "custom role"}: { + {Name: "custom role"}: { {Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeDashboardsAll}, {Action: dashboards.ActionDashboardsWrite, Scope: dashboards.ScopeDashboardsAll}, {Action: dashboards.ActionDashboardsDelete, Scope: dashboards.ScopeDashboardsAll}, diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx index 6ca683bbefb..262d5af0f22 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.test.tsx @@ -4,26 +4,6 @@ import { sceneGraph, SceneGridItem, SceneGridLayout, SceneQueryRunner, VizPanel import { DashboardScene } from './DashboardScene'; describe('DashboardScene', () => { - describe('Given a standard scene', () => { - it('Should set inspectPanelKey when url has inspect key', () => { - const scene = buildTestScene(); - scene.urlSync?.updateFromUrl({ inspect: '2' }); - expect(scene.state.inspectPanelKey).toBe('2'); - }); - - it('Should handle inspect key that is not found', () => { - const scene = buildTestScene(); - scene.urlSync?.updateFromUrl({ inspect: '12321' }); - expect(scene.state.inspectPanelKey).toBe(undefined); - }); - - it('Should set viewPanelKey when url has viewPanel', () => { - const scene = buildTestScene(); - scene.urlSync?.updateFromUrl({ viewPanel: '2' }); - expect(scene.state.viewPanelKey).toBe('2'); - }); - }); - describe('Editing and discarding', () => { describe('Given scene in edit mode', () => { let scene: DashboardScene; diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts new file mode 100644 index 00000000000..951187e9d2e --- /dev/null +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.test.ts @@ -0,0 +1,91 @@ +import { AppEvents } from '@grafana/data'; +import { SceneGridItem, SceneGridLayout, SceneQueryRunner, VizPanel } from '@grafana/scenes'; +import appEvents from 'app/core/app_events'; + +import { DashboardScene } from './DashboardScene'; +import { DashboardRepeatsProcessedEvent } from './types'; + +describe('DashboardSceneUrlSync', () => { + describe('Given a standard scene', () => { + it('Should set inspectPanelKey when url has inspect key', () => { + const scene = buildTestScene(); + scene.urlSync?.updateFromUrl({ inspect: '2' }); + expect(scene.state.inspectPanelKey).toBe('2'); + }); + + it('Should handle inspect key that is not found', () => { + const scene = buildTestScene(); + scene.urlSync?.updateFromUrl({ inspect: '12321' }); + expect(scene.state.inspectPanelKey).toBe(undefined); + }); + + it('Should set viewPanelKey when url has viewPanel', () => { + const scene = buildTestScene(); + scene.urlSync?.updateFromUrl({ viewPanel: '2' }); + expect(scene.state.viewPanelKey).toBe('2'); + }); + }); + + describe('Given a viewPanelKey with clone that is not found', () => { + const scene = buildTestScene(); + + let errorNotice = 0; + appEvents.on(AppEvents.alertError, (evt) => errorNotice++); + + scene.urlSync?.updateFromUrl({ viewPanel: 'panel-1-clone-1' }); + + expect(scene.state.viewPanelKey).toBeUndefined(); + // Verify no error notice was shown + expect(errorNotice).toBe(0); + + // fake adding clone panel + const layout = scene.state.body as SceneGridLayout; + layout.setState({ + children: [ + new SceneGridItem({ + key: 'griditem-1', + x: 0, + body: new VizPanel({ + title: 'Clone Panel A', + key: 'panel-1-clone-1', + pluginId: 'table', + }), + }), + ], + }); + + // Verify it subscribes to DashboardRepeatsProcessedEvent + scene.publishEvent(new DashboardRepeatsProcessedEvent({ source: scene })); + expect(scene.state.viewPanelKey).toBe('panel-1-clone-1'); + }); +}); + +function buildTestScene() { + const scene = new DashboardScene({ + title: 'hello', + uid: 'dash-1', + body: new SceneGridLayout({ + children: [ + new SceneGridItem({ + key: 'griditem-1', + x: 0, + body: new VizPanel({ + title: 'Panel A', + key: 'panel-1', + pluginId: 'table', + $data: new SceneQueryRunner({ key: 'data-query-runner', queries: [{ refId: 'A' }] }), + }), + }), + new SceneGridItem({ + body: new VizPanel({ + title: 'Panel B', + key: 'panel-2', + pluginId: 'table', + }), + }), + ], + }), + }); + + return scene; +} diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts index 58ccccc5b94..67e09082a4b 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts +++ b/public/app/features/dashboard-scene/scene/DashboardSceneUrlSync.ts @@ -1,3 +1,5 @@ +import { Unsubscribable } from 'rxjs'; + import { AppEvents } from '@grafana/data'; import { locationService } from '@grafana/runtime'; import { SceneObjectUrlSyncHandler, SceneObjectUrlValues } from '@grafana/scenes'; @@ -7,8 +9,11 @@ import { PanelInspectDrawer } from '../inspect/PanelInspectDrawer'; import { findVizPanelByKey } from '../utils/utils'; import { DashboardScene, DashboardSceneState } from './DashboardScene'; +import { DashboardRepeatsProcessedEvent } from './types'; export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { + private _eventSub?: Unsubscribable; + constructor(private _scene: DashboardScene) {} getKeys(): string[] { @@ -44,6 +49,12 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { if (typeof values.viewPanel === 'string') { const panel = findVizPanelByKey(this._scene, values.viewPanel); if (!panel) { + // // If we are trying to view a repeat clone that can't be found it might be that the repeats have not been processed yet + if (values.viewPanel.indexOf('clone')) { + this._handleViewRepeatClone(values.viewPanel); + return; + } + appEvents.emit(AppEvents.alertError, ['Panel not found']); locationService.partial({ viewPanel: null }); return; @@ -58,4 +69,16 @@ export class DashboardSceneUrlSync implements SceneObjectUrlSyncHandler { this._scene.setState(update); } } + + private _handleViewRepeatClone(viewPanel: string) { + if (!this._eventSub) { + this._eventSub = this._scene.subscribeToEvent(DashboardRepeatsProcessedEvent, () => { + const panel = findVizPanelByKey(this._scene, viewPanel); + if (panel) { + this._eventSub?.unsubscribe(); + this._scene.setState({ viewPanelKey: viewPanel }); + } + }); + } + } } diff --git a/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.test.tsx b/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.test.tsx index f853691c1be..6a58da61b87 100644 --- a/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.test.tsx +++ b/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.test.tsx @@ -108,6 +108,16 @@ describe('PanelRepeaterGridItem', () => { // given 5 rows with total height 25 gives new itemHeight of 5 expect(repeater.state.itemHeight).toBe(5); }); + + it('When updating variable should update repeats', async () => { + const { scene, repeater, variable } = buildScene({ variableQueryTime: 0 }); + + activateFullSceneTree(scene); + + variable.changeValueTo(['1', '3'], ['A', 'C']); + + expect(repeater.state.repeatedPanels?.length).toBe(2); + }); }); interface SceneOptions { @@ -130,27 +140,27 @@ function buildScene(options: SceneOptions) { }), }); + const variable = new TestVariable({ + name: 'server', + query: 'A.*', + value: ALL_VARIABLE_VALUE, + text: ALL_VARIABLE_TEXT, + isMulti: true, + 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' }, + ], + }); + const scene = new EmbeddedScene({ $timeRange: new SceneTimeRange({ from: 'now-6h', to: 'now' }), $variables: new SceneVariableSet({ - variables: [ - new TestVariable({ - name: 'server', - query: 'A.*', - value: ALL_VARIABLE_VALUE, - text: ALL_VARIABLE_TEXT, - isMulti: true, - 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' }, - ], - }), - ], + variables: [variable], }), body: new SceneGridLayout({ children: [ @@ -161,5 +171,5 @@ function buildScene(options: SceneOptions) { }), }); - return { scene, repeater }; + return { scene, repeater, variable }; } diff --git a/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.tsx b/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.tsx index a1a6044e22a..ed553c14338 100644 --- a/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.tsx +++ b/public/app/features/dashboard-scene/scene/PanelRepeaterGridItem.tsx @@ -20,6 +20,8 @@ import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN } from 'app/core/constants'; import { getMultiVariableValues } from '../utils/utils'; +import { DashboardRepeatsProcessedEvent } from './types'; + interface PanelRepeaterGridItemState extends SceneGridItemStateLike { source: VizPanel; repeatedPanels?: VizPanel[]; @@ -146,6 +148,9 @@ export class PanelRepeaterGridItem extends SceneObjectBase { + public static type = 'dashboard-repeats-processed'; +}