Scopes: Move URL values from selector to facade (#96842)

public/app/features/scopes/ScopesFacadeScene.ts

@@ -1,6 +1,12 @@
 import { isEqual } from 'lodash';
 
-import { SceneObjectBase, SceneObjectState } from '@grafana/scenes';
+import {
+  SceneObjectBase,
+  SceneObjectState,
+  SceneObjectUrlSyncConfig,
+  SceneObjectUrlValues,
+  SceneObjectWithUrlSync,
+} from '@grafana/scenes';
 
 import { scopesSelectorScene } from './instance';
 import { disableScopes, enableScopes, enterScopesReadOnly, exitScopesReadOnly, getSelectedScopes } from './utils';
@@ -8,21 +14,47 @@ import { disableScopes, enableScopes, enterScopesReadOnly, exitScopesReadOnly, g
 interface ScopesFacadeState extends SceneObjectState {
   // A callback that will be executed when new scopes are set
   handler?: (facade: ScopesFacade) => void;
+  // The render count is a workaround to force the URL sync manager to update the URL with the latest scopes
+  // Basically it starts at 0, and it is increased with every scopes value update
+  renderCount?: number;
 }
 
-export class ScopesFacade extends SceneObjectBase<ScopesFacadeState> {
+export class ScopesFacade extends SceneObjectBase<ScopesFacadeState> implements SceneObjectWithUrlSync {
+  protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['scopes'] });
+
   public constructor(state: ScopesFacadeState) {
-    super(state);
+    super({
+      ...state,
+      renderCount: 0,
+    });
 
     this.addActivationHandler(this._activationHandler);
   }
 
+  public getUrlState() {
+    return {
+      scopes: this.value.map(({ metadata }) => metadata.name),
+    };
+  }
+
+  public updateFromUrl(values: SceneObjectUrlValues) {
+    if (!values.scopes && !scopesSelectorScene?.state.isEnabled) {
+      return;
+    }
+
+    let scopeNames = values.scopes ?? [];
+    scopeNames = Array.isArray(scopeNames) ? scopeNames : [scopeNames];
+
+    scopesSelectorScene?.updateScopes(scopeNames.map((scopeName) => ({ scopeName, path: [] })));
+  }
+
   private _activationHandler = () => {
     this.enable();
 
     this._subs.add(
       scopesSelectorScene?.subscribeToState((newState, prevState) => {
         if (!newState.isLoadingScopes && (prevState.isLoadingScopes || !isEqual(newState.scopes, prevState.scopes))) {
+          this.setState({ renderCount: (this.state.renderCount ?? 0) + 1 });
           this.state.handler?.(this);
         }
       })

public/app/features/scopes/instance.tsx

@@ -1,5 +1,4 @@
 import { config } from '@grafana/runtime';
-import { UrlSyncManager } from '@grafana/scenes';
 
 import { ScopesDashboardsScene } from './internal/ScopesDashboardsScene';
 import { ScopesSelectorScene } from './internal/ScopesSelectorScene';
@@ -14,8 +13,5 @@ export function initializeScopes() {
 
     scopesSelectorScene.setState({ dashboards: scopesDashboardsScene.getRef() });
     scopesDashboardsScene.setState({ selector: scopesSelectorScene.getRef() });
-
-    const urlSyncManager = new UrlSyncManager();
-    urlSyncManager.initSync(scopesSelectorScene!);
   }
 }

public/app/features/scopes/internal/ScopesDashboardsScene.tsx

@@ -12,7 +12,7 @@ import { ScopesDashboardsTreeSearch } from './ScopesDashboardsTreeSearch';
 import { ScopesSelectorScene } from './ScopesSelectorScene';
 import { fetchDashboards } from './api';
 import { SuggestedDashboardsFoldersMap } from './types';
-import { filterFolders, getScopeNamesFromSelectedScopes, groupDashboards } from './utils';
+import { filterFolders, groupDashboards } from './utils';
 
 export interface ScopesDashboardsSceneState extends SceneObjectState {
   selector: SceneObjectRef<ScopesSelectorScene> | null;
@@ -27,7 +27,6 @@ export interface ScopesDashboardsSceneState extends SceneObjectState {
   isPanelOpened: boolean;
   isEnabled: boolean;
   isReadOnly: boolean;
-  scopesSelected: boolean;
   searchQuery: string;
 }
 
@@ -40,7 +39,6 @@ export const getInitialDashboardsState: () => Omit<ScopesDashboardsSceneState, '
   isPanelOpened: false,
   isEnabled: false,
   isReadOnly: false,
-  scopesSelected: false,
   searchQuery: '',
 });
 
@@ -56,40 +54,16 @@ export class ScopesDashboardsScene extends SceneObjectBase<ScopesDashboardsScene
     });
 
     this.addActivationHandler(() => {
-      const resolvedSelector = this.state.selector?.resolve();
-
-      if (resolvedSelector?.state.scopes.length ?? 0 > 0) {
-        this.fetchDashboards();
-        this.openPanel();
-      }
-
-      if (resolvedSelector) {
-        this._subs.add(
-          resolvedSelector.subscribeToState((newState, prevState) => {
-            const newScopeNames = getScopeNamesFromSelectedScopes(newState.scopes ?? []);
-            const oldScopeNames = getScopeNamesFromSelectedScopes(prevState.scopes ?? []);
-
-            if (!isEqual(newScopeNames, oldScopeNames)) {
-              this.fetchDashboards();
-
-              if (newState.scopes.length > 0) {
-                this.openPanel();
-              } else {
-                this.closePanel();
-              }
-            }
-          })
-        );
-      }
-
       return () => {
         this.dashboardsFetchingSub?.unsubscribe();
       };
     });
   }
 
-  public async fetchDashboards() {
-    const scopeNames = getScopeNamesFromSelectedScopes(this.state.selector?.resolve().state.scopes ?? []);
+  public async fetchDashboards(scopeNames: string[]) {
+    if (isEqual(this.state.forScopeNames, scopeNames)) {
+      return;
+    }
 
     this.dashboardsFetchingSub?.unsubscribe();
 
@@ -102,7 +76,7 @@ export class ScopesDashboardsScene extends SceneObjectBase<ScopesDashboardsScene
         filteredFolders: {},
         forScopeNames: [],
         isLoading: false,
-        scopesSelected: false,
+        isPanelOpened: false,
       });
     }
 
@@ -123,7 +97,7 @@ export class ScopesDashboardsScene extends SceneObjectBase<ScopesDashboardsScene
           folders,
           filteredFolders,
           isLoading: false,
-          scopesSelected: scopeNames.length > 0,
+          isPanelOpened: scopeNames.length > 0,
         });
 
         this.dashboardsFetchingSub?.unsubscribe();
@@ -202,7 +176,7 @@ export class ScopesDashboardsScene extends SceneObjectBase<ScopesDashboardsScene
 }
 
 export function ScopesDashboardsSceneRenderer({ model }: SceneComponentProps<ScopesDashboardsScene>) {
-  const { dashboards, filteredFolders, isLoading, isPanelOpened, isEnabled, isReadOnly, searchQuery, scopesSelected } =
+  const { dashboards, filteredFolders, forScopeNames, isLoading, isPanelOpened, isEnabled, isReadOnly, searchQuery } =
     model.useState();
 
   const styles = useStyles2(getStyles);
@@ -212,7 +186,7 @@ export function ScopesDashboardsSceneRenderer({ model }: SceneComponentProps<Sco
   }
 
   if (!isLoading) {
-    if (!scopesSelected) {
+    if (forScopeNames.length === 0) {
       return (
         <div
           className={cx(styles.container, styles.noResultsContainer)}

public/app/features/scopes/internal/ScopesSelectorScene.tsx

@@ -4,15 +4,7 @@ import { finalize, from, Subscription } from 'rxjs';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { config } from '@grafana/runtime';
-import {
-  SceneComponentProps,
-  SceneObjectBase,
-  SceneObjectRef,
-  SceneObjectState,
-  SceneObjectUrlSyncConfig,
-  SceneObjectUrlValues,
-  SceneObjectWithUrlSync,
-} from '@grafana/scenes';
+import { SceneComponentProps, SceneObjectBase, SceneObjectRef, SceneObjectState } from '@grafana/scenes';
 import { Button, Drawer, IconButton, Spinner, useStyles2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
 import { t, Trans } from 'app/core/internationalization';
@@ -22,12 +14,7 @@ import { ScopesInput } from './ScopesInput';
 import { ScopesTree } from './ScopesTree';
 import { fetchNodes, fetchScope, fetchSelectedScopes } from './api';
 import { NodeReason, NodesMap, SelectedScope, TreeScope } from './types';
-import {
-  getBasicScope,
-  getScopeNamesFromSelectedScopes,
-  getScopesAndTreeScopesWithPaths,
-  getTreeScopesFromSelectedScopes,
-} from './utils';
+import { getBasicScope, getScopesAndTreeScopesWithPaths, getTreeScopesFromSelectedScopes } from './utils';
 
 export interface ScopesSelectorSceneState extends SceneObjectState {
   dashboards: SceneObjectRef<ScopesDashboardsScene> | null;
@@ -64,11 +51,9 @@ export const initialSelectorState: Omit<ScopesSelectorSceneState, 'dashboards'>
   isEnabled: false,
 };
 
-export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneState> implements SceneObjectWithUrlSync {
+export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneState> {
   static Component = ScopesSelectorSceneRenderer;
 
-  protected _urlSync = new SceneObjectUrlSyncConfig(this, { keys: ['scopes'] });
-
   private nodesFetchingSub: Subscription | undefined;
 
   constructor() {
@@ -78,7 +63,11 @@ export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneStat
     });
 
     this.addActivationHandler(() => {
-      this.fetchBaseNodes();
+      // Only fetch base nodes on activation when there are no nodes fetched
+      // This prevents an issue where base nodes are overwritten upon re-activations
+      if (Object.keys(this.state.nodes[''].nodes).length === 0) {
+        this.fetchBaseNodes();
+      }
 
       return () => {
         this.nodesFetchingSub?.unsubscribe();
@@ -86,19 +75,6 @@ export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneStat
     });
   }
 
-  public getUrlState() {
-    return {
-      scopes: this.state.isEnabled ? getScopeNamesFromSelectedScopes(this.state.scopes) : [],
-    };
-  }
-
-  public updateFromUrl(values: SceneObjectUrlValues) {
-    let scopeNames = values.scopes ?? [];
-    scopeNames = Array.isArray(scopeNames) ? scopeNames : [scopeNames];
-
-    this.updateScopes(scopeNames.map((scopeName) => ({ scopeName, path: [] })));
-  }
-
   public fetchBaseNodes() {
     return this.updateNode([''], true, '');
   }
@@ -231,6 +207,8 @@ export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneStat
       isLoadingScopes: true,
     });
 
+    this.state.dashboards?.resolve().fetchDashboards(treeScopes.map(({ scopeName }) => scopeName));
+
     const scopes = await fetchSelectedScopes(treeScopes);
 
     this.setState({ scopes, isLoadingScopes: false });
@@ -240,8 +218,8 @@ export class ScopesSelectorScene extends SceneObjectBase<ScopesSelectorSceneStat
     this.setState({ treeScopes: getTreeScopesFromSelectedScopes(this.state.scopes) });
   }
 
-  public removeAllScopes() {
-    this.setState({ scopes: [], treeScopes: [], isLoadingScopes: false });
+  public async removeAllScopes() {
+    return this.updateScopes([]);
   }
 
   public enterReadOnly() {

public/app/features/scopes/tests/tree.test.ts

@@ -126,16 +126,16 @@ describe('Tree', () => {
     await openSelector();
     await expandResultApplications();
     await searchScopes('Cloud');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
     expectResultApplicationsGrafanaNotPresent();
     expectResultApplicationsMimirNotPresent();
     expectResultApplicationsCloudPresent();
 
     await clearScopesSearch();
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(4);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
 
     await searchScopes('Grafana');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(5);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(4);
     expectResultApplicationsGrafanaPresent();
     expectResultApplicationsCloudNotPresent();
   });
@@ -156,7 +156,7 @@ describe('Tree', () => {
     await expandResultApplications();
     await selectResultApplicationsMimir();
     await searchScopes('grafana');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
     expectPersistedApplicationsMimirPresent();
     expectPersistedApplicationsGrafanaNotPresent();
     expectResultApplicationsMimirNotPresent();
@@ -168,7 +168,7 @@ describe('Tree', () => {
     await expandResultApplications();
     await selectResultApplicationsMimir();
     await searchScopes('mimir');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
     expectPersistedApplicationsMimirNotPresent();
     expectResultApplicationsMimirPresent();
   });
@@ -178,10 +178,10 @@ describe('Tree', () => {
     await expandResultApplications();
     await selectResultApplicationsMimir();
     await searchScopes('grafana');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
 
     await clearScopesSearch();
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(4);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
     expectPersistedApplicationsMimirNotPresent();
     expectPersistedApplicationsGrafanaNotPresent();
     expectResultApplicationsMimirPresent();
@@ -192,15 +192,15 @@ describe('Tree', () => {
     await openSelector();
     await expandResultApplications();
     await searchScopes('mimir');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
 
     await selectResultApplicationsMimir();
     await searchScopes('unknown');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(4);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
     expectPersistedApplicationsMimirPresent();
 
     await clearScopesSearch();
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(5);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(4);
     expectResultApplicationsMimirPresent();
     expectResultApplicationsGrafanaPresent();
   });
@@ -210,7 +210,7 @@ describe('Tree', () => {
     await expandResultApplications();
     await selectResultApplicationsMimir();
     await searchScopes('grafana');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
 
     await selectResultApplicationsGrafana();
     await applyScopes();
@@ -222,7 +222,7 @@ describe('Tree', () => {
     await expandResultApplications();
     await selectResultApplicationsMimir();
     await searchScopes('grafana');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
 
     await selectResultApplicationsGrafana();
     await applyScopes();
@@ -239,11 +239,11 @@ describe('Tree', () => {
     expectScopesHeadline('Recommended');
 
     await searchScopes('Applications');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(1);
     expectScopesHeadline('Results');
 
     await searchScopes('unknown');
-    expect(fetchNodesSpy).toHaveBeenCalledTimes(3);
+    expect(fetchNodesSpy).toHaveBeenCalledTimes(2);
     expectScopesHeadline('No results found for your query');
   });