Scopes: Redirect to first available dashboard on select (#112257)

* Redirect to first available dashboard on select

* Check currently active url before redirecting

* Expand currently selected group on selection

* Add unit test

* Test group expansion on URL change

* Don't expand any group if the active item is already in an expanded one

* Mock dashboardsService better

* Fix linitng issue

* Add and remove subscrioption based on open state

* Extract scope navigation utils

* Fix import order

* Fix import path

* Add redirection tests to ScopesSelectorService

* Fix import order

public/app/features/scopes/dashboards/ScopesDashboardsService.test.ts

@@ -1,4 +1,5 @@
 import { Location } from 'history';
+import { Subject } from 'rxjs';
 
 import { ScopeDashboardBinding } from '@grafana/data';
 import { config, locationService } from '@grafana/runtime';
@@ -17,6 +18,10 @@ jest.mock('@grafana/runtime', () => ({
   },
   locationService: {
     getLocation: jest.fn(),
+    // Mock getLocationObservable to return a mock observable
+    getLocationObservable: jest.fn().mockReturnValue({
+      subscribe: jest.fn(),
+    }),
   },
 }));
 
@@ -61,12 +66,11 @@ describe('ScopesDashboardsService', () => {
       expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
     });
 
-    it('should expand folders when current location matches URL path and navigation endpoint is enabled', async () => {
-      // Enable the navigation endpoint feature toggle
+    it('should expand folder when location changes and matches a navigation URL', async () => {
       config.featureToggles.useScopesNavigationEndpoint = true;
 
-      // Mock current location to match a URL-based navigation
-      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/test-url' } as Location);
+      // Mock initial location
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/' } as Location);
 
       const mockNavigations: ScopeNavigation[] = [
         {
@@ -85,10 +89,73 @@ describe('ScopesDashboardsService', () => {
       ];
 
       mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
-      await service.fetchDashboards(['scope1']);
 
-      // Verify that the folder is expanded because the current URL path matches
-      expect(service.state.folders[''].folders['group1'].expanded).toBe(true);
+      // Set up mock observable to emit location changes
+      const locationSubject = new Subject<Location>();
+      (locationService.getLocationObservable as jest.Mock).mockReturnValue(locationSubject);
+
+      // Create a new service instance that will subscribe to our mocked observable
+      const testService = new ScopesDashboardsService(mockApiClient);
+      await testService.fetchDashboards(['scope1']);
+
+      // Initially, folder should not be expanded since we're at '/'
+      expect(testService.state.folders[''].folders['group1'].expanded).toBe(false);
+
+      // Simulate location change to a URL that matches a navigation
+      locationSubject.next({ pathname: '/test-url' } as Location);
+
+      // Now the folder should be expanded because the location matches a navigation URL
+      expect(testService.state.folders[''].folders['group1'].expanded).toBe(true);
+
+      // Reset the feature toggle
+      config.featureToggles.useScopesNavigationEndpoint = false;
+    });
+
+    it('should not expand folder when location changes, matches a navigation URL in a folder which is already expanded', async () => {
+      config.featureToggles.useScopesNavigationEndpoint = true;
+
+      // Mock initial location
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/' } as Location);
+
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'scope1',
+            url: '/test-url',
+          },
+          status: {
+            title: 'Test URL',
+            groups: ['group1', 'group2'],
+          },
+          metadata: {
+            name: 'url1',
+          },
+        },
+      ];
+
+      mockApiClient.fetchScopeNavigations.mockResolvedValue(mockNavigations);
+
+      // Set up mock observable to emit location changes
+      const locationSubject = new Subject<Location>();
+      (locationService.getLocationObservable as jest.Mock).mockReturnValue(locationSubject);
+
+      // Create a new service instance that will subscribe to our mocked observable
+      const testService = new ScopesDashboardsService(mockApiClient);
+      await testService.fetchDashboards(['scope1']);
+
+      // Initially, folder should not be expanded since we're at '/'
+      expect(testService.state.folders[''].folders['group1'].expanded).toBe(false);
+
+      // Manually expand group1 to simulate it being already expanded
+      testService.updateFolder(['', 'group2'], true);
+      expect(testService.state.folders[''].folders['group2'].expanded).toBe(true);
+
+      // Simulate location change to a URL that matches a navigation
+      locationSubject.next({ pathname: '/test-url' } as Location);
+
+      // The folder should still be expanded (no change since it was already expanded)
+      expect(testService.state.folders[''].folders['group1'].expanded).toBe(false);
+      expect(testService.state.folders[''].folders['group2'].expanded).toBe(true);
 
       // Reset the feature toggle
       config.featureToggles.useScopesNavigationEndpoint = false;

public/app/features/scopes/dashboards/ScopesDashboardsService.ts

@@ -1,4 +1,5 @@
 import { isEqual } from 'lodash';
+import { Subscription } from 'rxjs';
 
 import { ScopeDashboardBinding } from '@grafana/data';
 import { config, locationService } from '@grafana/runtime';
@@ -6,6 +7,7 @@ import { config, locationService } from '@grafana/runtime';
 import { ScopesApiClient } from '../ScopesApiClient';
 import { ScopesServiceBase } from '../ScopesServiceBase';
 
+import { isCurrentPath } from './scopeNavgiationUtils';
 import { ScopeNavigation, SuggestedNavigationsFoldersMap } from './types';
 
 interface ScopesDashboardsServiceState {
@@ -24,6 +26,7 @@ interface ScopesDashboardsServiceState {
 }
 
 export class ScopesDashboardsService extends ScopesServiceBase<ScopesDashboardsServiceState> {
+  private locationSubscription: Subscription | undefined;
   constructor(private apiClient: ScopesApiClient) {
     super({
       drawerOpened: false,
@@ -35,8 +38,56 @@ export class ScopesDashboardsService extends ScopesServiceBase<ScopesDashboardsS
       loading: false,
       searchQuery: '',
     });
+
+    // Add/ remove location subscribtion based on the drawer opened state
+    this.subscribeToState((state, prevState) => {
+      if (state.drawerOpened === prevState.drawerOpened) {
+        return;
+      }
+      if (state.drawerOpened && !prevState.drawerOpened) {
+        // Before creating a new subscription, ensure any existing subscription is disposed to avoid multiple active subscriptions and potential memory leaks.
+        this.locationSubscription?.unsubscribe();
+        this.locationSubscription = locationService.getLocationObservable().subscribe((location) => {
+          this.onLocationChange(location.pathname);
+        });
+      } else if (!state.drawerOpened && prevState.drawerOpened) {
+        this.locationSubscription?.unsubscribe();
+      }
+    });
   }
 
+  // Expand the group that matches the current path, if it is not already expanded
+  private onLocationChange = (pathname: string) => {
+    if (!this.state.drawerOpened) {
+      return;
+    }
+    const currentPath = pathname;
+    const activeScopeNavigation = this.state.scopeNavigations.find((s) => {
+      if (!('url' in s.spec) || typeof s.spec.url !== 'string') {
+        return false;
+      }
+      return isCurrentPath(currentPath, s.spec.url);
+    });
+
+    if (!activeScopeNavigation) {
+      return;
+    }
+
+    // Check if the activeScopeNavigation is in a folder that is already expanded
+    if (activeScopeNavigation.status.groups) {
+      for (const group of activeScopeNavigation.status.groups) {
+        if (this.state.folders[''].folders[group].expanded) {
+          return;
+        }
+      }
+    }
+
+    // Expand the first group, as we don't know which one to prioritize
+    if (activeScopeNavigation.status.groups) {
+      this.updateFolder(['', activeScopeNavigation.status.groups[0]], true);
+    }
+  };
+
   public updateFolder = (path: string[], expanded: boolean) => {
     let folders = { ...this.state.folders };
     let filteredFolders = { ...this.state.filteredFolders };

public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx

@@ -5,30 +5,21 @@ import { Link, useLocation } from 'react-router-dom-v5-compat';
 import { GrafanaTheme2, IconName, locationUtil } from '@grafana/data';
 import { Icon, useStyles2 } from '@grafana/ui';
 
+import { isCurrentPath } from './scopeNavgiationUtils';
+
 export interface ScopesNavigationTreeLinkProps {
   to: string;
   title: string;
   id: string;
 }
 
-// Helper function to get the base path for a dashboard URL for comparison purposes.
-// e.g., /d/dashboardId/slug -> /d/dashboardId
-//       /d/dashboardId      -> /d/dashboardId
-function getDashboardPathForComparison(pathname: string): string {
-  return pathname.split('/').slice(0, 3).join('/');
-}
-
 export function ScopesNavigationTreeLink({ to, title, id }: ScopesNavigationTreeLinkProps) {
   const styles = useStyles2(getStyles);
   const linkIcon = useMemo(() => getLinkIcon(to), [to]);
-  const isDashboard = to.startsWith('/d/');
   const locPathname = useLocation().pathname;
 
-  // For dashboards, the title is appended to the path when we navigate to just the dashboard id, hence we need to disregard this
-  const currentPath = isDashboard ? getDashboardPathForComparison(locPathname) : locPathname;
-
   // Ignore query params
-  const isCurrent = to.split('?')[0] === currentPath;
+  const isCurrent = isCurrentPath(locPathname, to);
 
   return (
     <Link

public/app/features/scopes/dashboards/scopeNavgiationUtils.test.ts

@@ -0,0 +1,31 @@
+import { getDashboardPathForComparison, isCurrentPath } from './scopeNavgiationUtils';
+
+describe('scopeNavgiationUtils', () => {
+  it('should return the correct path for a dashboard', () => {
+    expect(getDashboardPathForComparison('/d/dashboardId/slug')).toBe('/d/dashboardId');
+    expect(getDashboardPathForComparison('/d/dashboardId')).toBe('/d/dashboardId');
+    expect(getDashboardPathForComparison('/d/dashboardId/slug?query=param')).toBe('/d/dashboardId');
+  });
+
+  it('should return the correct path for a navigation', () => {
+    expect(isCurrentPath('/d/dashboardId/slug', '/d/dashboardId')).toBe(true);
+    expect(isCurrentPath('/d/dashboardId', '/d/dashboardId')).toBe(true);
+  });
+
+  it('shoudl handle non-dashboard paths', () => {
+    expect(isCurrentPath('/other/path', '/other/path')).toBe(true);
+    expect(isCurrentPath('/other/path', '/other/path?query=param')).toBe(true);
+    expect(isCurrentPath('/other/path', '/other/path#hash')).toBe(true);
+    expect(isCurrentPath('/other/path', '/other/path?query=param#hash')).toBe(true);
+  });
+
+  it('should return the correct path for a navigation with query params', () => {
+    expect(isCurrentPath('/d/dashboardId/slug', '/d/dashboardId?query=param')).toBe(true);
+    expect(isCurrentPath('/d/dashboardId', '/d/dashboardId?query=param')).toBe(true);
+  });
+
+  it('should return the correct path for a navigation with hash', () => {
+    expect(isCurrentPath('/d/dashboardId/slug', '/d/dashboardId#hash')).toBe(true);
+    expect(isCurrentPath('/d/dashboardId', '/d/dashboardId#hash')).toBe(true);
+  });
+});

public/app/features/scopes/dashboards/scopeNavgiationUtils.ts

@@ -0,0 +1,24 @@
+// Helper function to get the base path for a dashboard URL for comparison purposes.
+// e.g., /d/dashboardId/slug -> /d/dashboardId
+//       /d/dashboardId      -> /d/dashboardId
+export function getDashboardPathForComparison(pathname: string): string {
+  return pathname.split('/').slice(0, 3).join('/');
+}
+
+export function normalizePath(path: string): string {
+  // Remove query + hash + trailing slash (except root)
+  const noQuery = path.split('?')[0].split('#')[0];
+  return noQuery !== '/' && noQuery.endsWith('/') ? noQuery.slice(0, -1) : noQuery;
+}
+
+// Pathname comes from location.pathname
+export function isCurrentPath(pathname: string, to: string): boolean {
+  const isDashboard = to.startsWith('/d/');
+
+  if (isDashboard) {
+    // For dashboards, the title is appended to the path when we navigate to just the dashboard id, hence we need to disregard this
+    return getDashboardPathForComparison(pathname) === normalizePath(to);
+  }
+  //Ignore query params
+  return pathname === normalizePath(to);
+}

public/app/features/scopes/selector/ScopesSelectorService.test.ts

@@ -1,11 +1,22 @@
 import { Scope, ScopeNode, Store } from '@grafana/data';
+import { locationService } from '@grafana/runtime';
 
 import { ScopesApiClient } from '../ScopesApiClient';
 import { ScopesDashboardsService } from '../dashboards/ScopesDashboardsService';
+import { ScopeNavigation } from '../dashboards/types';
 
 import { RECENT_SCOPES_KEY, ScopesSelectorService } from './ScopesSelectorService';
 import { RecentScope } from './types';
 
+// Mock locationService
+jest.mock('@grafana/runtime', () => ({
+  ...jest.requireActual('@grafana/runtime'),
+  locationService: {
+    push: jest.fn(),
+    getLocation: jest.fn(),
+  },
+}));
+
 describe('ScopesSelectorService', () => {
   let service: ScopesSelectorService;
   let apiClient: jest.Mocked<ScopesApiClient>;
@@ -40,6 +51,12 @@ describe('ScopesSelectorService', () => {
   let store: Store;
 
   beforeEach(() => {
+    // Clear all mocks
+    jest.clearAllMocks();
+
+    // Mock locationService to return a default location
+    (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-page' });
+
     apiClient = {
       fetchScope: jest.fn().mockResolvedValue(mockScope),
       fetchMultipleScopes: jest.fn().mockResolvedValue([mockScope]),
@@ -55,7 +72,17 @@ describe('ScopesSelectorService', () => {
     } as unknown as jest.Mocked<ScopesApiClient>;
 
     dashboardsService = {
-      fetchDashboards: jest.fn(),
+      fetchDashboards: jest.fn().mockResolvedValue(undefined),
+      state: {
+        scopeNavigations: [],
+        dashboards: [],
+        drawerOpened: false,
+        filteredFolders: {},
+        folders: {},
+        forScopeNames: [],
+        loading: false,
+        searchQuery: '',
+      },
     } as unknown as jest.Mocked<ScopesDashboardsService>;
 
     storeValue = {};
@@ -532,4 +559,155 @@ describe('ScopesSelectorService', () => {
       expect(recentScopes[2][0].parentNode).toBeUndefined(); // invalid parent node should be removed
     });
   });
+
+  describe('redirect on scope selection', () => {
+    it('should redirect to the first scopeNavigation with /d/ URL when current URL is not a scopeNavigation', async () => {
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'test-scope',
+            url: '/d/dashboard1',
+          },
+          status: {
+            title: 'Dashboard 1',
+            groups: [],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ];
+
+      dashboardsService.state.scopeNavigations = mockNavigations;
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' });
+
+      await service.changeScopes(['test-scope']);
+
+      expect(locationService.push).toHaveBeenCalledWith('/d/dashboard1');
+    });
+
+    it('should NOT redirect when the first scopeNavigation does not contain /d/ (e.g., logs drilldown)', async () => {
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'test-scope',
+            url: '/explore',
+          },
+          status: {
+            title: 'Explore',
+            groups: [],
+          },
+          metadata: {
+            name: 'explore1',
+          },
+        },
+      ];
+
+      dashboardsService.state.scopeNavigations = mockNavigations;
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' });
+
+      await service.changeScopes(['test-scope']);
+
+      expect(locationService.push).not.toHaveBeenCalled();
+    });
+
+    it('should NOT redirect when current URL matches a scopeNavigation', async () => {
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'test-scope',
+            url: '/d/dashboard1',
+          },
+          status: {
+            title: 'Dashboard 1',
+            groups: [],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ];
+
+      dashboardsService.state.scopeNavigations = mockNavigations;
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/d/dashboard1' });
+
+      await service.changeScopes(['test-scope']);
+
+      expect(locationService.push).not.toHaveBeenCalled();
+    });
+
+    it('should NOT redirect when there are no scopeNavigations', async () => {
+      dashboardsService.state.scopeNavigations = [];
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' });
+
+      await service.changeScopes(['test-scope']);
+
+      expect(locationService.push).not.toHaveBeenCalled();
+    });
+
+    it('should NOT redirect when scopeNavigation does not have a url property', async () => {
+      const mockNavigations = [
+        {
+          spec: {
+            scope: 'test-scope',
+            // Missing url property
+          },
+          status: {
+            title: 'Dashboard 1',
+            groups: [],
+          },
+          metadata: {
+            name: 'dashboard1',
+          },
+        },
+      ] as unknown as ScopeNavigation[];
+
+      dashboardsService.state.scopeNavigations = mockNavigations;
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' });
+
+      await service.changeScopes(['test-scope']);
+
+      expect(locationService.push).not.toHaveBeenCalled();
+    });
+
+    it('should handle multiple scopeNavigations and redirect to the first dashboard one', async () => {
+      const mockNavigations: ScopeNavigation[] = [
+        {
+          spec: {
+            scope: 'test-scope',
+            url: '/d/first-dashboard',
+          },
+          status: {
+            title: 'First Dashboard',
+            groups: [],
+          },
+          metadata: {
+            name: 'first-dashboard',
+          },
+        },
+        {
+          spec: {
+            scope: 'test-scope',
+            url: '/d/second-dashboard',
+          },
+          status: {
+            title: 'Second Dashboard',
+            groups: [],
+          },
+          metadata: {
+            name: 'second-dashboard',
+          },
+        },
+      ];
+
+      dashboardsService.state.scopeNavigations = mockNavigations;
+      (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/some-other-page' });
+
+      await service.changeScopes(['test-scope']);
+
+      // Should redirect to the first one
+      expect(locationService.push).toHaveBeenCalledWith('/d/first-dashboard');
+      expect(locationService.push).toHaveBeenCalledTimes(1);
+    });
+  });
 });

public/app/features/scopes/selector/ScopesSelectorService.ts

@@ -1,11 +1,12 @@
 import { Scope, ScopeNode, store as storeImpl } from '@grafana/data';
-import { config } from '@grafana/runtime';
+import { config, locationService } from '@grafana/runtime';
 import { SceneRenderProfiler } from '@grafana/scenes';
 import { getDashboardSceneProfiler } from 'app/features/dashboard/services/DashboardProfiler';
 
 import { ScopesApiClient } from '../ScopesApiClient';
 import { ScopesServiceBase } from '../ScopesServiceBase';
 import { ScopesDashboardsService } from '../dashboards/ScopesDashboardsService';
+import { isCurrentPath } from '../dashboards/scopeNavgiationUtils';
 
 import {
   closeNodes,
@@ -18,7 +19,6 @@ import {
   treeNodeAtPath,
 } from './scopesTreeUtils';
 import { NodesMap, RecentScope, RecentScopeSchema, ScopeSchema, ScopesMap, SelectedScope, TreeNode } from './types';
-
 export const RECENT_SCOPES_KEY = 'grafana.scopes.recent';
 
 export interface ScopesSelectorServiceState {
@@ -362,7 +362,31 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
 
     // Fetches both dashboards and scope navigations
     // We call this even if we have 0 scope because in that case it also closes the dashboard drawer.
-    this.dashboardsService.fetchDashboards(scopes.map((s) => s.scopeId));
+    this.dashboardsService.fetchDashboards(scopes.map((s) => s.scopeId)).then(() => {
+      // Redirect to first scopeNavigation if current URL isn't a scopeNavigation
+      const currentPath = locationService.getLocation().pathname;
+      const activeScopeNavigation = this.dashboardsService.state.scopeNavigations.find((s) => {
+        if (!('url' in s.spec) || typeof s.spec.url !== 'string') {
+          return false;
+        }
+        return isCurrentPath(currentPath, s.spec.url);
+      });
+
+      if (!activeScopeNavigation && this.dashboardsService.state.scopeNavigations.length > 0) {
+        // Redirect to the first available scopeNavigation
+        const firstScopeNavigation = this.dashboardsService.state.scopeNavigations[0];
+
+        if (
+          firstScopeNavigation &&
+          'url' in firstScopeNavigation.spec &&
+          typeof firstScopeNavigation.spec.url === 'string' &&
+          // Only redirect to dashboards TODO: Remove this once Logs Drilldown has Scopes support
+          firstScopeNavigation.spec.url.includes('/d/')
+        ) {
+          locationService.push(firstScopeNavigation.spec.url);
+        }
+      }
+    });
 
     if (scopes.length > 0) {
       const fetchedScopes = await this.apiClient.fetchMultipleScopes(scopes.map((s) => s.scopeId));

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

@@ -4,6 +4,7 @@ import { config, locationService } from '@grafana/runtime';
 
 import { ScopesService } from '../ScopesService';
 import { ScopesDashboardsService } from '../dashboards/ScopesDashboardsService';
+import { ScopeNavigation } from '../dashboards/types';
 
 import {
   clearNotFound,
@@ -261,6 +262,35 @@ describe('Dashboards list', () => {
     expectDashboardLength('billing-usage', 1);
   });
 
+  it('redirects to the first scope navigation if your current dashboard is not a scope navigation', async () => {
+    // Render another dashboard, which is not a scope navigation
+    const mockNavigations: ScopeNavigation[] = [
+      {
+        spec: {
+          scope: 'grafana',
+          url: '/d/dashboard1',
+        },
+        status: {
+          title: 'Dashboard 1',
+          groups: ['group1'],
+        },
+        metadata: {
+          name: 'dashboard1',
+        },
+      },
+    ];
+    fetchDashboardsSpy.mockResolvedValue(mockNavigations);
+
+    await renderDashboard();
+    expect(locationService.getLocation().pathname).toBe('/');
+
+    await updateScopes(scopesService, ['grafana']);
+    expect(locationService.getLocation().pathname).toBe('/d/dashboard1');
+    // renderDashboard defaults to home dashboard
+    expect(locationService.getLocation().pathname).not.toBe('/');
+    expect(fetchDashboardsSpy).toHaveBeenCalled();
+  });
+
   it('Shows a proper message when no scopes are selected', async () => {
     await toggleDashboards();
     expectNoDashboardsNoScopes();