From fe4c615b3d44a2979b52ca5f34cc249889c6ffd2 Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:34:21 +0100 Subject: [PATCH] Scopes: Sync nested scopes navigation open folders to URL (#114786) * Sync nav_scope_path with url * Let the current active scope remain if it is a child of the selected subscope * Remove location updates based on nav_scope_path to maintain expanded folders * Fix folder tests * Remove console logs * Better mock for changeScopes * Update test to support the new calls * Update test with function inputs * Fix failinging test * Add tests and add isEqual check for fetching new subscopes --- .../app/features/scopes/ScopesService.test.ts | 303 +++++++++++++++++- public/app/features/scopes/ScopesService.ts | 57 +++- .../scopes/dashboards/ScopesDashboards.tsx | 7 +- .../ScopesDashboardsService.test.ts | 48 +++ .../dashboards/ScopesDashboardsService.ts | 64 +++- .../dashboards/ScopesDashboardsTree.tsx | 11 +- .../ScopesDashboardsTreeFolderItem.test.tsx | 7 +- .../ScopesDashboardsTreeFolderItem.tsx | 24 +- .../ScopesNavigationTreeLink.test.tsx | 2 +- .../dashboards/ScopesNavigationTreeLink.tsx | 16 +- .../dashboards/scopeNavgiationUtils.test.ts | 199 +++++++++++- .../scopes/dashboards/scopeNavgiationUtils.ts | 53 +++ .../selector/ScopesSelectorService.test.ts | 2 +- .../scopes/selector/ScopesSelectorService.ts | 2 +- 14 files changed, 757 insertions(+), 38 deletions(-) diff --git a/public/app/features/scopes/ScopesService.test.ts b/public/app/features/scopes/ScopesService.test.ts index 51c50cc9ff9..fdc0c8bd598 100644 --- a/public/app/features/scopes/ScopesService.test.ts +++ b/public/app/features/scopes/ScopesService.test.ts @@ -22,8 +22,8 @@ describe('ScopesService', () => { | undefined; let dashboardsStateSubscription: | (( - state: { navigationScope?: string; drawerOpened: boolean }, - prevState: { navigationScope?: string; drawerOpened: boolean } + state: { navigationScope?: string; drawerOpened: boolean; navScopePath?: string[] }, + prevState: { navigationScope?: string; drawerOpened: boolean; navScopePath?: string[] } ) => void) | undefined; @@ -56,7 +56,7 @@ describe('ScopesService', () => { selectorStateSubscription = callback; return { unsubscribe: jest.fn() }; }), - changeScopes: jest.fn(), + changeScopes: jest.fn().mockResolvedValue(undefined), resolvePathToRoot: jest.fn().mockResolvedValue({ path: [], tree: {} }), } as unknown as jest.Mocked; @@ -71,6 +71,7 @@ describe('ScopesService', () => { loading: false, searchQuery: '', navigationScope: undefined, + navScopePath: undefined, }, stateObservable: new BehaviorSubject({ drawerOpened: false, @@ -82,12 +83,14 @@ describe('ScopesService', () => { loading: false, searchQuery: '', navigationScope: undefined, + navScopePath: undefined, }), subscribeToState: jest.fn((callback) => { dashboardsStateSubscription = callback; return { unsubscribe: jest.fn() }; }), setNavigationScope: jest.fn(), + setNavScopePath: jest.fn(), } as unknown as jest.Mocked; locationService = { @@ -188,7 +191,7 @@ describe('ScopesService', () => { service = new ScopesService(selectorService, dashboardsService, locationService); - expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1'); + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1', undefined, undefined); }); it('should read navigation_scope along with other scope parameters', () => { @@ -199,7 +202,7 @@ describe('ScopesService', () => { service = new ScopesService(selectorService, dashboardsService, locationService); - expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1'); + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1', undefined, undefined); expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, 'node1', false); }); @@ -213,6 +216,45 @@ describe('ScopesService', () => { expect(dashboardsService.setNavigationScope).not.toHaveBeenCalled(); }); + + it('should read nav_scope_path along with navigation_scope from URL on init', () => { + locationService.getLocation = jest.fn().mockReturnValue({ + pathname: '/test', + search: '?navigation_scope=navScope1&nav_scope_path=mimir%2Cloki', + }); + + service = new ScopesService(selectorService, dashboardsService, locationService); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1', undefined, ['mimir', 'loki']); + }); + + it('should handle nav_scope_path without navigation_scope by calling setNavScopePath after changeScopes', async () => { + locationService.getLocation = jest.fn().mockReturnValue({ + pathname: '/test', + search: '?scopes=scope1&nav_scope_path=mimir', + }); + + service = new ScopesService(selectorService, dashboardsService, locationService); + + // Wait for the changeScopes promise to resolve + await Promise.resolve(); + + expect(dashboardsService.setNavScopePath).toHaveBeenCalledWith(['mimir']); + }); + + it('should handle URL-encoded nav_scope_path values', () => { + locationService.getLocation = jest.fn().mockReturnValue({ + pathname: '/test', + search: '?navigation_scope=navScope1&nav_scope_path=' + encodeURIComponent('folder one,folder two'), + }); + + service = new ScopesService(selectorService, dashboardsService, locationService); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1', undefined, [ + 'folder one', + 'folder two', + ]); + }); }); describe('URL synchronization', () => { @@ -346,16 +388,22 @@ describe('ScopesService', () => { { navigationScope: 'navScope1', drawerOpened: true, + navScopePath: undefined, }, { navigationScope: undefined, drawerOpened: false, + navScopePath: undefined, } ); - expect(locationService.partial).toHaveBeenCalledWith({ - navigation_scope: 'navScope1', - }); + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: 'navScope1', + nav_scope_path: null, + }, + true + ); }); it('should update navigation_scope in URL when navigationScope changes', () => { @@ -367,16 +415,22 @@ describe('ScopesService', () => { { navigationScope: 'navScope2', drawerOpened: true, + navScopePath: undefined, }, { navigationScope: 'navScope1', drawerOpened: true, + navScopePath: undefined, } ); - expect(locationService.partial).toHaveBeenCalledWith({ - navigation_scope: 'navScope2', - }); + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: 'navScope2', + nav_scope_path: null, + }, + true + ); }); it('should not update URL when navigationScope has not changed', () => { @@ -390,10 +444,12 @@ describe('ScopesService', () => { { navigationScope: 'navScope1', drawerOpened: true, + navScopePath: undefined, }, { navigationScope: 'navScope1', drawerOpened: false, + navScopePath: undefined, } ); @@ -409,16 +465,126 @@ describe('ScopesService', () => { { navigationScope: undefined, drawerOpened: false, + navScopePath: undefined, }, { navigationScope: 'navScope1', drawerOpened: true, + navScopePath: undefined, } ); - expect(locationService.partial).toHaveBeenCalledWith({ - navigation_scope: undefined, - }); + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: null, + nav_scope_path: null, + }, + true + ); + }); + + it('should write nav_scope_path to URL when navScopePath changes', () => { + if (!dashboardsStateSubscription) { + throw new Error('dashboardsStateSubscription not set'); + } + + dashboardsStateSubscription( + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: ['mimir', 'loki'], + }, + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: undefined, + } + ); + + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: 'navScope1', + nav_scope_path: encodeURIComponent('mimir,loki'), + }, + true + ); + }); + + it('should update nav_scope_path in URL when navScopePath changes', () => { + if (!dashboardsStateSubscription) { + throw new Error('dashboardsStateSubscription not set'); + } + + dashboardsStateSubscription( + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: ['mimir', 'loki', 'tempo'], + }, + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: ['mimir', 'loki'], + } + ); + + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: 'navScope1', + nav_scope_path: encodeURIComponent('mimir,loki,tempo'), + }, + true + ); + }); + + it('should clear nav_scope_path from URL when navScopePath becomes empty', () => { + if (!dashboardsStateSubscription) { + throw new Error('dashboardsStateSubscription not set'); + } + + dashboardsStateSubscription( + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: [], + }, + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: ['mimir'], + } + ); + + expect(locationService.partial).toHaveBeenCalledWith( + { + navigation_scope: 'navScope1', + nav_scope_path: null, + }, + true + ); + }); + + it('should not update URL when only drawerOpened changes but navigationScope and navScopePath remain the same', () => { + if (!dashboardsStateSubscription) { + throw new Error('dashboardsStateSubscription not set'); + } + + jest.clearAllMocks(); + + dashboardsStateSubscription( + { + navigationScope: 'navScope1', + drawerOpened: false, + navScopePath: ['mimir'], + }, + { + navigationScope: 'navScope1', + drawerOpened: true, + navScopePath: ['mimir'], + } + ); + + expect(locationService.partial).not.toHaveBeenCalled(); }); }); @@ -457,4 +623,113 @@ describe('ScopesService', () => { ); }); }); + + describe('back/forward navigation handling', () => { + let locationSubject: BehaviorSubject<{ pathname: string; search: string }>; + + beforeEach(() => { + locationSubject = new BehaviorSubject({ + pathname: '/test', + search: '', + }); + + locationService.getLocation = jest.fn().mockReturnValue({ + pathname: '/test', + search: '', + }); + locationService.getLocationObservable = jest.fn().mockReturnValue(locationSubject); + + // Set initial state for dashboards service + dashboardsService.state.navigationScope = undefined; + dashboardsService.state.navScopePath = undefined; + + service = new ScopesService(selectorService, dashboardsService, locationService); + service.setEnabled(true); + + jest.clearAllMocks(); + }); + + it('should update navigation scope when URL changes via back/forward', () => { + // Simulate URL change (e.g., browser back button) + locationSubject.next({ + pathname: '/test', + search: '?navigation_scope=navScope1', + }); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope1', undefined, undefined); + }); + + it('should update nav_scope_path when URL changes via back/forward', () => { + // Set current state + dashboardsService.state.navigationScope = 'navScope1'; + dashboardsService.state.navScopePath = undefined; + + // Simulate URL change with nav_scope_path + locationSubject.next({ + pathname: '/test', + search: '?navigation_scope=navScope1&nav_scope_path=' + encodeURIComponent('mimir,loki'), + }); + + expect(dashboardsService.setNavScopePath).toHaveBeenCalledWith(['mimir', 'loki']); + }); + + it('should clear navigation scope when removed from URL via back/forward', () => { + // Set current state + dashboardsService.state.navigationScope = 'navScope1'; + dashboardsService.state.navScopePath = ['mimir']; + + // Simulate URL change (navigation scope removed) + locationSubject.next({ + pathname: '/test', + search: '', + }); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith(undefined); + }); + + it('should handle navigation scope change along with nav_scope_path', () => { + // Set current state + dashboardsService.state.navigationScope = 'navScope1'; + dashboardsService.state.navScopePath = ['mimir']; + + // Simulate URL change to different navigation scope with new path + locationSubject.next({ + pathname: '/test', + search: '?navigation_scope=navScope2&nav_scope_path=' + encodeURIComponent('loki,tempo'), + }); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('navScope2', undefined, ['loki', 'tempo']); + }); + + it('should handle URL-encoded navigation_scope from back/forward', () => { + // Set current state + dashboardsService.state.navigationScope = undefined; + + // Simulate URL change with encoded navigation scope + locationSubject.next({ + pathname: '/test', + search: '?navigation_scope=' + encodeURIComponent('scope with spaces'), + }); + + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith('scope with spaces', undefined, undefined); + }); + + it('should handle nav_scope_path change without navigation_scope', async () => { + // Set current state - no navigation scope but has nav scope path + dashboardsService.state.navigationScope = undefined; + dashboardsService.state.navScopePath = undefined; + selectorService.state.appliedScopes = [{ scopeId: 'scope1' }]; + + // Simulate URL change with only nav_scope_path + locationSubject.next({ + pathname: '/test', + search: '?scopes=scope1&nav_scope_path=mimir', + }); + + // Wait for changeScopes promise + await Promise.resolve(); + + expect(dashboardsService.setNavScopePath).toHaveBeenCalledWith(['mimir']); + }); + }); }); diff --git a/public/app/features/scopes/ScopesService.ts b/public/app/features/scopes/ScopesService.ts index 1d0be39f56c..c92d6328749 100644 --- a/public/app/features/scopes/ScopesService.ts +++ b/public/app/features/scopes/ScopesService.ts @@ -5,6 +5,7 @@ import { map, distinctUntilChanged } from 'rxjs/operators'; import { LocationService, ScopesContextValue, ScopesContextValueState } from '@grafana/runtime'; import { ScopesDashboardsService } from './dashboards/ScopesDashboardsService'; +import { deserializeFolderPath, serializeFolderPath } from './dashboards/scopeNavgiationUtils'; import { ScopesSelectorService } from './selector/ScopesSelectorService'; export interface State { @@ -72,12 +73,21 @@ export class ScopesService implements ScopesContextValue { const queryParams = new URLSearchParams(locationService.getLocation().search); const scopeNodeId = queryParams.get('scope_node'); const navigationScope = queryParams.get('navigation_scope'); + const navScopePath = queryParams.get('nav_scope_path'); if (navigationScope) { - this.dashboardsService.setNavigationScope(navigationScope); + this.dashboardsService.setNavigationScope( + navigationScope, + undefined, + navScopePath ? deserializeFolderPath(navScopePath) : undefined + ); } - this.changeScopes(queryParams.getAll('scopes'), undefined, scopeNodeId ?? undefined); + this.changeScopes(queryParams.getAll('scopes'), undefined, scopeNodeId ?? undefined).then(() => { + if (navScopePath && !navigationScope) { + this.dashboardsService.setNavScopePath(deserializeFolderPath(navScopePath)); + } + }); // Pre-load scope node (which loads parent too) const nodeToPreload = scopeNodeId; @@ -99,6 +109,9 @@ export class ScopesService implements ScopesContextValue { const scopes = queryParams.getAll('scopes'); const scopeNodeId = queryParams.get('scope_node'); + const navigationScope = queryParams.get('navigation_scope'); + const navScopePath = queryParams.get('nav_scope_path'); + // Check if new scopes are different from the old scopes const currentScopes = this.selectorService.state.appliedScopes.map((scope) => scope.scopeId); if (scopes.length && !isEqual(scopes, currentScopes)) { @@ -107,6 +120,31 @@ export class ScopesService implements ScopesContextValue { // changes the URL directly, it would trigger a reload so scopes would still be reset. this.changeScopes(scopes, undefined, scopeNodeId ?? undefined); } + + // Handle navigation_scope and nav_scope_path changes from back/forward navigation + const currentNavigationScope = this.dashboardsService.state.navigationScope; + const currentNavScopePath = this.dashboardsService.state.navScopePath; + const newNavScopePath = navScopePath ? deserializeFolderPath(navScopePath) : undefined; + const decodedNavigationScope = navigationScope ? decodeURIComponent(navigationScope) : undefined; + + const navigationScopeChanged = decodedNavigationScope !== currentNavigationScope; + const navScopePathChanged = !isEqual(newNavScopePath, currentNavScopePath); + + if (navigationScopeChanged) { + // Navigation scope changed - do full update + if (decodedNavigationScope) { + this.dashboardsService.setNavigationScope(decodedNavigationScope, undefined, newNavScopePath); + } else if (newNavScopePath?.length) { + this.changeScopes(scopes, undefined, scopeNodeId ?? undefined).then(() => { + this.dashboardsService.setNavScopePath(newNavScopePath); + }); + } else { + this.dashboardsService.setNavigationScope(undefined); + } + } else if (navScopePathChanged) { + // Navigation scope unchanged but path changed + this.dashboardsService.setNavScopePath(newNavScopePath); + } }) ); @@ -137,10 +175,17 @@ export class ScopesService implements ScopesContextValue { // Update the URL based on change in the navigation scope this.subscriptions.push( this.dashboardsService.subscribeToState((state, prevState) => { - if (state.navigationScope !== prevState.navigationScope) { - this.locationService.partial({ - navigation_scope: state.navigationScope, - }); + if ( + state.navigationScope !== prevState.navigationScope || + !isEqual(state.navScopePath, prevState.navScopePath) + ) { + this.locationService.partial( + { + navigation_scope: state.navigationScope ? encodeURIComponent(state.navigationScope) : null, + nav_scope_path: state.navScopePath?.length ? serializeFolderPath(state.navScopePath) : null, + }, + true + ); } }) ); diff --git a/public/app/features/scopes/dashboards/ScopesDashboards.tsx b/public/app/features/scopes/dashboards/ScopesDashboards.tsx index 3db9344e6c1..7a2fc7832f3 100644 --- a/public/app/features/scopes/dashboards/ScopesDashboards.tsx +++ b/public/app/features/scopes/dashboards/ScopesDashboards.tsx @@ -67,7 +67,12 @@ export function ScopesDashboards() { /> ) : filteredFolders[''] ? ( - + ) : (

diff --git a/public/app/features/scopes/dashboards/ScopesDashboardsService.test.ts b/public/app/features/scopes/dashboards/ScopesDashboardsService.test.ts index 52368b4577d..ca621106f81 100644 --- a/public/app/features/scopes/dashboards/ScopesDashboardsService.test.ts +++ b/public/app/features/scopes/dashboards/ScopesDashboardsService.test.ts @@ -839,4 +839,52 @@ describe('ScopesDashboardsService', () => { expect(service.state.drawerOpened).toBe(true); }); }); + + describe('setNavScopePath', () => { + beforeEach(() => { + (locationService.getLocation as jest.Mock).mockReturnValue({ pathname: '/' } as Location); + }); + + it('should set nav scope path', async () => { + await service.setNavScopePath(['mimir']); + expect(service.state.navScopePath).toEqual(['mimir']); + }); + + it('should replace existing path with new path', async () => { + await service.setNavScopePath(['mimir']); + expect(service.state.navScopePath).toEqual(['mimir']); + + await service.setNavScopePath(['loki']); + expect(service.state.navScopePath).toEqual(['loki']); + }); + + it('should handle multiple scopes in path', async () => { + await service.setNavScopePath(['mimir', 'loki']); + expect(service.state.navScopePath).toEqual(['mimir', 'loki']); + }); + + it('should clear path with empty array', async () => { + await service.setNavScopePath(['mimir', 'loki']); + expect(service.state.navScopePath).toEqual(['mimir', 'loki']); + + await service.setNavScopePath([]); + expect(service.state.navScopePath).toEqual([]); + }); + + it('should handle undefined path as empty array', async () => { + await service.setNavScopePath(['mimir']); + expect(service.state.navScopePath).toEqual(['mimir']); + + await service.setNavScopePath(undefined); + expect(service.state.navScopePath).toEqual([]); + }); + + it('should not update state if path is unchanged', async () => { + await service.setNavScopePath(['mimir']); + + await service.setNavScopePath(['mimir']); + // Path should remain the same + expect(service.state.navScopePath).toEqual(['mimir']); + }); + }); }); diff --git a/public/app/features/scopes/dashboards/ScopesDashboardsService.ts b/public/app/features/scopes/dashboards/ScopesDashboardsService.ts index 55ea5ab737a..96518cba5f7 100644 --- a/public/app/features/scopes/dashboards/ScopesDashboardsService.ts +++ b/public/app/features/scopes/dashboards/ScopesDashboardsService.ts @@ -7,8 +7,13 @@ import { config, locationService } from '@grafana/runtime'; import { ScopesApiClient } from '../ScopesApiClient'; import { ScopesServiceBase } from '../ScopesServiceBase'; -import { isCurrentPath } from './scopeNavgiationUtils'; -import { ScopeNavigation, SuggestedNavigationsFoldersMap, SuggestedNavigationsMap } from './types'; +import { buildSubScopePath, isCurrentPath } from './scopeNavgiationUtils'; +import { + ScopeNavigation, + SuggestedNavigationsFolder, + SuggestedNavigationsFoldersMap, + SuggestedNavigationsMap, +} from './types'; interface ScopesDashboardsServiceState { // State of the drawer showing related dashboards @@ -24,6 +29,8 @@ interface ScopesDashboardsServiceState { loading: boolean; searchQuery: string; navigationScope?: string; + // Path of subScopes which should be expanded + navScopePath?: string[]; } export class ScopesDashboardsService extends ScopesServiceBase { @@ -38,6 +45,7 @@ export class ScopesDashboardsService extends ScopesServiceBase { + const subScope = subScopePath[subScopePath.length - 1]; + const path = buildSubScopePath(subScope, this.state.folders); + + // Get path to the folder - path can now be undefined + if (path && path.length > 0) { + this.updateFolder(path, true); + } + }; + + public setNavScopePath = async (navScopePath?: string[]) => { + const navScopePathArray = navScopePath ?? []; + + if (!isEqual(navScopePathArray, this.state.navScopePath)) { + this.updateState({ navScopePath: navScopePathArray }); + + for (const subScope of navScopePathArray) { + // Find the actual path to the folder with this subScopeName + const folderPath = buildSubScopePath(subScope, this.state.folders); + if (folderPath && folderPath.length > 0) { + await this.fetchSubScopeItems(folderPath, subScope); + this.openSubScopeFolder([subScope]); + } + } + } + }; + // The fallbackScopeNames is used to fetch the ScopeNavigations for the current dashboard when the navigationScope is not set. // You only need to awaut this function if you need to wait for the dashboards to be fetched before doing something else. - public setNavigationScope = async (navigationScope?: string, fallbackScopeNames?: string[]) => { + public setNavigationScope = async ( + navigationScope?: string, + fallbackScopeNames?: string[], + navScopePath?: string[] + ) => { if (this.state.navigationScope === navigationScope) { return; } @@ -67,6 +106,7 @@ export class ScopesDashboardsService extends ScopesServiceBase 0 }); await this.fetchDashboards(forScopeNames); + await this.setNavScopePath(navScopePath); }; // Expand the group that matches the current path, if it is not already expanded @@ -148,6 +188,15 @@ export class ScopesDashboardsService extends ScopesServiceBase { + // Check if folder already has content - skip fetching to preserve existing state + const targetFolder = this.getFolder(path); + if ( + targetFolder && + (Object.keys(targetFolder.folders).length > 0 || Object.keys(targetFolder.suggestedNavigations).length > 0) + ) { + return; + } + let subScopeFolders: SuggestedNavigationsFoldersMap | undefined; try { @@ -208,6 +257,15 @@ export class ScopesDashboardsService extends ScopesServiceBase { + let folder: SuggestedNavigationsFoldersMap = this.state.folders; + for (let i = 0; i < path.length - 1; i++) { + folder = folder[path[i]]?.folders ?? {}; + } + return folder[path[path.length - 1]]; + }; + public changeSearchQuery = (searchQuery: string) => { searchQuery = searchQuery ?? ''; diff --git a/public/app/features/scopes/dashboards/ScopesDashboardsTree.tsx b/public/app/features/scopes/dashboards/ScopesDashboardsTree.tsx index 7e7601f3b06..cbaea4c3e84 100644 --- a/public/app/features/scopes/dashboards/ScopesDashboardsTree.tsx +++ b/public/app/features/scopes/dashboards/ScopesDashboardsTree.tsx @@ -12,10 +12,17 @@ export interface ScopesDashboardsTreeProps { subScope?: string; folders: SuggestedNavigationsFoldersMap; folderPath: string[]; + subScopePath?: string[]; onFolderUpdate: OnFolderUpdate; } -export function ScopesDashboardsTree({ subScope, folders, folderPath, onFolderUpdate }: ScopesDashboardsTreeProps) { +export function ScopesDashboardsTree({ + subScopePath, + subScope, + folders, + folderPath, + onFolderUpdate, +}: ScopesDashboardsTreeProps) { const [queryParams] = useQueryParams(); const styles = useStyles2(getStyles); @@ -54,6 +61,7 @@ export function ScopesDashboardsTree({ subScope, folders, folderPath, onFolderUp {regularNavigations.map((navigation) => ( ( ({ @@ -133,7 +136,7 @@ describe('ScopesDashboardsTreeFolderItem', () => { const exchangeButton = screen.getByRole('button', { name: /change root scope/i }); await user.click(exchangeButton); - expect(mockScopesDashboardsService.setNavigationScope).toHaveBeenCalledWith(undefined, ['subScope1']); + expect(mockScopesDashboardsService.setNavigationScope).toHaveBeenCalledWith(undefined, undefined, []); }); it('calls changeScopes when exchange icon is clicked', async () => { @@ -152,7 +155,7 @@ describe('ScopesDashboardsTreeFolderItem', () => { const exchangeButton = screen.getByRole('button', { name: /change root scope/i }); await user.click(exchangeButton); - expect(mockScopesSelectorService.changeScopes).toHaveBeenCalledWith(['subScope1']); + expect(mockScopesSelectorService.changeScopes).toHaveBeenCalledWith(['subScope1'], undefined, undefined, false); }); it('passes subScope prop to ScopesDashboardsTree when folder is expanded', () => { diff --git a/public/app/features/scopes/dashboards/ScopesDashboardsTreeFolderItem.tsx b/public/app/features/scopes/dashboards/ScopesDashboardsTreeFolderItem.tsx index 9d572a4d54c..d6846f432ac 100644 --- a/public/app/features/scopes/dashboards/ScopesDashboardsTreeFolderItem.tsx +++ b/public/app/features/scopes/dashboards/ScopesDashboardsTreeFolderItem.tsx @@ -14,9 +14,11 @@ export interface ScopesDashboardsTreeFolderItemProps { folderPath: string[]; folders: SuggestedNavigationsFoldersMap; onFolderUpdate: OnFolderUpdate; + subScopePath?: string[]; } export function ScopesDashboardsTreeFolderItem({ + subScopePath, folder, folderPath, folders, @@ -53,12 +55,27 @@ export function ScopesDashboardsTreeFolderItem({ scope: folder.subScopeName || '', })} name="exchange-alt" - onClick={(e) => { + onClick={async (e) => { e.preventDefault(); e.stopPropagation(); if (folder.subScopeName && scopesSelectorService) { - scopesDashboardsService?.setNavigationScope(undefined, [folder.subScopeName]); - scopesSelectorService.changeScopes([folder.subScopeName]); + const activeSubScopePath = scopesDashboardsService?.state.navScopePath; + // Check if the active scope is a child of the current folder's scope + const activeScope = activeSubScopePath?.[activeSubScopePath.length - 1]; + const folderLocationInActivePath = activeSubScopePath?.indexOf(folder.subScopeName) ?? -1; + + await scopesDashboardsService?.setNavigationScope( + folderLocationInActivePath >= 0 ? folder.subScopeName : undefined, + undefined, + activeSubScopePath?.slice(folderLocationInActivePath + 1) ?? [] + ); + // Now changeScopes will skip fetchDashboards because navigationScope is set + scopesSelectorService.changeScopes( + folderLocationInActivePath >= 0 && activeScope ? [activeScope] : [folder.subScopeName], + undefined, + undefined, + false + ); } }} /> @@ -68,6 +85,7 @@ export function ScopesDashboardsTreeFolderItem({ {folder.expanded && (

{ const link = screen.getByTestId('scopes-dashboards-test-id'); await userEvent.click(link); - expect(mockScopesDashboardsService.setNavigationScope).toHaveBeenCalledWith('currentScope'); + expect(mockScopesDashboardsService.setNavigationScope).toHaveBeenCalledWith('currentScope', undefined, undefined); }); it('should not set navigation scope when already set', async () => { diff --git a/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx index b7722edcf8c..67b1649ee3c 100644 --- a/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx +++ b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx @@ -8,16 +8,17 @@ import { Icon, useStyles2 } from '@grafana/ui'; import { useScopesServices } from '../ScopesContextProvider'; -import { isCurrentPath, normalizePath } from './scopeNavgiationUtils'; +import { isCurrentPath, normalizePath, serializeFolderPath } from './scopeNavgiationUtils'; export interface ScopesNavigationTreeLinkProps { subScope?: string; to: string; title: string; id: string; + subScopePath?: string[]; } -export function ScopesNavigationTreeLink({ subScope, to, title, id }: ScopesNavigationTreeLinkProps) { +export function ScopesNavigationTreeLink({ subScope, to, title, id, subScopePath }: ScopesNavigationTreeLinkProps) { const styles = useStyles2(getStyles); const linkIcon = useMemo(() => getLinkIcon(to), [to]); const locPathname = useLocation().pathname; @@ -25,7 +26,7 @@ export function ScopesNavigationTreeLink({ subScope, to, title, id }: ScopesNavi // Ignore query params const isCurrent = isCurrentPath(locPathname, to); - const handleClick = (e: React.MouseEvent) => { + const handleClick = async (e: React.MouseEvent) => { if (subScope) { e.preventDefault(); // Prevent default Link navigation @@ -39,11 +40,18 @@ export function ScopesNavigationTreeLink({ subScope, to, title, id }: ScopesNavi const searchParams = new URLSearchParams(url.search); if (!currentNavigationScope && currentScope) { searchParams.set('navigation_scope', currentScope); - services?.scopesDashboardsService?.setNavigationScope(currentScope); + await services?.scopesDashboardsService?.setNavigationScope( + currentScope, + undefined, + subScopePath && subScopePath.length > 0 ? subScopePath : undefined + ); } // Update query params with the new subScope searchParams.set('scopes', subScope); + + // Set nav_scope_path to the subScopePath + searchParams.set('nav_scope_path', subScopePath ? serializeFolderPath(subScopePath) : ''); // Remove scope_node and scope_parent since we're changing to a subScope searchParams.delete('scope_node'); searchParams.delete('scope_parent'); diff --git a/public/app/features/scopes/dashboards/scopeNavgiationUtils.test.ts b/public/app/features/scopes/dashboards/scopeNavgiationUtils.test.ts index ec927a3dc92..bd441b6088b 100644 --- a/public/app/features/scopes/dashboards/scopeNavgiationUtils.test.ts +++ b/public/app/features/scopes/dashboards/scopeNavgiationUtils.test.ts @@ -1,4 +1,11 @@ -import { getDashboardPathForComparison, isCurrentPath } from './scopeNavgiationUtils'; +import { + buildSubScopePath, + deserializeFolderPath, + getDashboardPathForComparison, + isCurrentPath, + serializeFolderPath, +} from './scopeNavgiationUtils'; +import { SuggestedNavigationsFoldersMap } from './types'; describe('scopeNavgiationUtils', () => { it('should return the correct path for a dashboard', () => { @@ -28,4 +35,194 @@ describe('scopeNavgiationUtils', () => { expect(isCurrentPath('/d/dashboardId/slug', '/d/dashboardId#hash')).toBe(true); expect(isCurrentPath('/d/dashboardId', '/d/dashboardId#hash')).toBe(true); }); + + describe('deserializeFolderPath', () => { + it('should return empty array for empty string', () => { + expect(deserializeFolderPath('')).toEqual([]); + }); + + it('should parse a simple comma-separated string', () => { + expect(deserializeFolderPath('mimir,loki')).toEqual(['mimir', 'loki']); + }); + + it('should handle single value', () => { + expect(deserializeFolderPath('mimir')).toEqual(['mimir']); + }); + + it('should trim whitespace around values', () => { + expect(deserializeFolderPath(' mimir , loki ')).toEqual(['mimir', 'loki']); + }); + + it('should handle URL-encoded strings', () => { + expect(deserializeFolderPath(encodeURIComponent('mimir,loki'))).toEqual(['mimir', 'loki']); + }); + + it('should handle URL-encoded strings with special characters', () => { + expect(deserializeFolderPath(encodeURIComponent('folder one,folder two'))).toEqual(['folder one', 'folder two']); + }); + + it('should fallback to split without decoding if decodeURIComponent fails', () => { + // Invalid URI sequence that would cause decodeURIComponent to throw + const invalidUri = '%E0%A4%A'; + expect(deserializeFolderPath(invalidUri)).toEqual(['%E0%A4%A']); + }); + }); + + describe('serializeFolderPath', () => { + it('should return empty string for empty array', () => { + expect(serializeFolderPath([])).toBe(''); + }); + + it('should serialize a simple array', () => { + expect(serializeFolderPath(['mimir', 'loki'])).toBe(encodeURIComponent('mimir,loki')); + }); + + it('should handle single value', () => { + expect(serializeFolderPath(['mimir'])).toBe('mimir'); + }); + + it('should handle values with spaces', () => { + expect(serializeFolderPath(['folder one', 'folder two'])).toBe(encodeURIComponent('folder one,folder two')); + }); + + it('should return empty string for null/undefined input', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(serializeFolderPath(null as any)).toBe(''); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect(serializeFolderPath(undefined as any)).toBe(''); + }); + }); + + describe('serializeFolderPath and deserializeFolderPath round-trip', () => { + it('should round-trip simple paths', () => { + const original = ['mimir', 'loki']; + const serialized = serializeFolderPath(original); + const deserialized = deserializeFolderPath(serialized); + expect(deserialized).toEqual(original); + }); + + it('should round-trip paths with spaces', () => { + const original = ['folder one', 'folder two']; + const serialized = serializeFolderPath(original); + const deserialized = deserializeFolderPath(serialized); + expect(deserialized).toEqual(original); + }); + }); + + describe('buildSubScopePath', () => { + it('should return undefined when folders is empty', () => { + const folders: SuggestedNavigationsFoldersMap = {}; + expect(buildSubScopePath('mimir', folders)).toBeUndefined(); + }); + + it('should find subScope at root level', () => { + const folders: SuggestedNavigationsFoldersMap = { + 'Mimir Dashboards': { + title: 'Mimir Dashboards', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'mimir', + }, + }; + expect(buildSubScopePath('mimir', folders)).toEqual(['Mimir Dashboards']); + }); + + it('should find subScope in nested folders', () => { + const folders: SuggestedNavigationsFoldersMap = { + '': { + title: '', + expanded: true, + folders: { + 'Parent Folder': { + title: 'Parent Folder', + expanded: false, + folders: { + 'Mimir Dashboards': { + title: 'Mimir Dashboards', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'mimir', + }, + }, + suggestedNavigations: {}, + }, + }, + suggestedNavigations: {}, + }, + }; + expect(buildSubScopePath('mimir', folders)).toEqual(['', 'Parent Folder', 'Mimir Dashboards']); + }); + + it('should return undefined when subScope is not found', () => { + const folders: SuggestedNavigationsFoldersMap = { + '': { + title: '', + expanded: true, + folders: { + 'Loki Dashboards': { + title: 'Loki Dashboards', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'loki', + }, + }, + suggestedNavigations: {}, + }, + }; + expect(buildSubScopePath('mimir', folders)).toBeUndefined(); + }); + + it('should return first match when multiple folders have the same subScope', () => { + const folders: SuggestedNavigationsFoldersMap = { + 'Mimir Dashboards': { + title: 'Mimir Dashboards', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'mimir', + }, + 'Mimir Overview': { + title: 'Mimir Overview', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'mimir', + }, + }; + // Should return the first one found (order depends on Object.entries) + const result = buildSubScopePath('mimir', folders); + expect(result).toBeDefined(); + expect(result?.length).toBe(1); + }); + + it('should find deeply nested subScope', () => { + const folders: SuggestedNavigationsFoldersMap = { + level1: { + title: 'Level 1', + expanded: true, + folders: { + level2: { + title: 'Level 2', + expanded: true, + folders: { + level3: { + title: 'Level 3', + expanded: false, + folders: {}, + suggestedNavigations: {}, + subScopeName: 'deep-scope', + }, + }, + suggestedNavigations: {}, + }, + }, + suggestedNavigations: {}, + }, + }; + expect(buildSubScopePath('deep-scope', folders)).toEqual(['level1', 'level2', 'level3']); + }); + }); }); diff --git a/public/app/features/scopes/dashboards/scopeNavgiationUtils.ts b/public/app/features/scopes/dashboards/scopeNavgiationUtils.ts index fe6d458a312..9fa4e2e05c7 100644 --- a/public/app/features/scopes/dashboards/scopeNavgiationUtils.ts +++ b/public/app/features/scopes/dashboards/scopeNavgiationUtils.ts @@ -1,3 +1,5 @@ +import { SuggestedNavigationsFoldersMap } from './types'; + // 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 @@ -5,12 +7,63 @@ export function getDashboardPathForComparison(pathname: string): string { return pathname.split('/').slice(0, 3).join('/'); } +/** + * Finds the path to a folder with the given subScopeName by searching recursively. + * @param subScope - The subScope name to find + * @param folders - The root folder structure to search + * @returns Array representing the path to the folder, or undefined if not found + */ +export function buildSubScopePath(subScope: string, folders: SuggestedNavigationsFoldersMap): string[] | undefined { + function findPath(currentFolders: SuggestedNavigationsFoldersMap, currentPath: string[]): string[] | undefined { + for (const [key, folder] of Object.entries(currentFolders)) { + const newPath = [...currentPath, key]; + if (folder.subScopeName === subScope) { + return newPath; + } + // Search in nested folders + const nestedPath = findPath(folder.folders, newPath); + if (nestedPath) { + return nestedPath; + } + } + return undefined; + } + + return findPath(folders, []); +} + 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; } +/** + * Deserializes a comma-separated folder path string into an array. + * Handles URL-encoded strings. + */ +export function deserializeFolderPath(navScopePath: string): string[] { + if (!navScopePath) { + return []; + } + try { + const decoded = decodeURIComponent(navScopePath); + return decoded.split(',').map((s) => s.trim()); + } catch { + return navScopePath.split(',').map((s) => s.trim()); + } +} + +/** + * Serializes a folder path array into a comma-separated string. + */ +export function serializeFolderPath(path: string[]): string { + if (!path) { + return ''; + } + return encodeURIComponent(path.join(',')); +} + // Pathname comes from location.pathname export function isCurrentPath(pathname: string, to: string): boolean { const isDashboard = to.startsWith('/d/'); diff --git a/public/app/features/scopes/selector/ScopesSelectorService.test.ts b/public/app/features/scopes/selector/ScopesSelectorService.test.ts index 673046adc0f..58ddb31a92f 100644 --- a/public/app/features/scopes/selector/ScopesSelectorService.test.ts +++ b/public/app/features/scopes/selector/ScopesSelectorService.test.ts @@ -444,7 +444,7 @@ describe('ScopesSelectorService', () => { await service.selectScope('test-scope-node'); await service.apply(); await service.removeAllScopes(); - expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith(undefined); + expect(dashboardsService.setNavigationScope).toHaveBeenCalledWith(undefined, undefined, undefined); }); }); diff --git a/public/app/features/scopes/selector/ScopesSelectorService.ts b/public/app/features/scopes/selector/ScopesSelectorService.ts index 17f36639f59..6f2bf86b7e0 100644 --- a/public/app/features/scopes/selector/ScopesSelectorService.ts +++ b/public/app/features/scopes/selector/ScopesSelectorService.ts @@ -405,7 +405,7 @@ export class ScopesSelectorService extends ScopesServiceBase { this.applyScopes([], false); - this.dashboardsService.setNavigationScope(undefined); + this.dashboardsService.setNavigationScope(undefined, undefined, undefined); }; private addRecentScopes = (scopes: Scope[], parentNode?: ScopeNode, scopeNodeId?: string) => {