Scopes: Resolve path directly from leaf node bugfix (#114507)
* Resolve path directly from leaf node * Add childrenLoaded field * Add tests and remove parentNodeId from changeScopes * Move parentNodeId patameter order * Resotre call order * Undo superflous change * Add comments * Make sure childrenLoaded state is properly set default to false * Reference parent path * Look for parent in state and fetch scopeNode if it is not avilable * Check for undefined * Add mock to test * Set scopeNodeId with recent scopes * Improve test selector * Add scope node endpoint to mocks * Never set childrenLoaded to true when inserting * Remove unused import * Pass on the already set childrenLoaded value * Fix test
This commit is contained in:
@@ -117,7 +117,8 @@ describe('ScopesService', () => {
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, 'node1', false);
|
||||
});
|
||||
|
||||
it('should read scope_parent for backward compatibility', () => {
|
||||
// TODO: remove when parentNodeId is removed
|
||||
it('should ignore scope_parent from URL (only used for recent scopes)', () => {
|
||||
locationService.getLocation = jest.fn().mockReturnValue({
|
||||
pathname: '/test',
|
||||
search: '?scopes=scope1&scope_parent=parent1',
|
||||
@@ -125,10 +126,12 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', undefined, false);
|
||||
// parentNodeId should be undefined since we don't read it from URL
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, undefined, false);
|
||||
});
|
||||
|
||||
it('should prefer scope_node when both scope_node and scope_parent exist', () => {
|
||||
// TODO: remove when parentNodeId is removed
|
||||
it('should only use scope_node when both scope_node and scope_parent exist in URL', () => {
|
||||
locationService.getLocation = jest.fn().mockReturnValue({
|
||||
pathname: '/test',
|
||||
search: '?scopes=scope1&scope_node=node1&scope_parent=parent1',
|
||||
@@ -136,9 +139,9 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
// Should call with parent1 as parentNodeId and node1 as scopeNodeId
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', 'node1', false);
|
||||
// Should preload node1 (not parent1)
|
||||
// Should only use scopeNodeId from URL, parentNodeId is undefined
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, 'node1', false);
|
||||
// Should preload node1
|
||||
expect(selectorService.resolvePathToRoot).toHaveBeenCalledWith('node1', expect.anything());
|
||||
});
|
||||
|
||||
@@ -153,7 +156,8 @@ describe('ScopesService', () => {
|
||||
expect(selectorService.resolvePathToRoot).toHaveBeenCalledWith('node1', expect.anything());
|
||||
});
|
||||
|
||||
it('should fallback to preload scope_parent when scope_node is not provided', () => {
|
||||
// TODO: remove when parentNodeId is removed
|
||||
it('should not preload when only scope_parent is in URL', () => {
|
||||
locationService.getLocation = jest.fn().mockReturnValue({
|
||||
pathname: '/test',
|
||||
search: '?scopes=scope1&scope_parent=parent1',
|
||||
@@ -161,7 +165,8 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
expect(selectorService.resolvePathToRoot).toHaveBeenCalledWith('parent1', expect.anything());
|
||||
// Should not preload since we don't read scope_parent from URL
|
||||
expect(selectorService.resolvePathToRoot).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle multiple scopes from URL', () => {
|
||||
|
||||
@@ -71,18 +71,16 @@ export class ScopesService implements ScopesContextValue {
|
||||
// Init from the URL when we first load
|
||||
const queryParams = new URLSearchParams(locationService.getLocation().search);
|
||||
const scopeNodeId = queryParams.get('scope_node');
|
||||
// TODO: figure out when to remove this. scope_parent is for backward compatibility only
|
||||
const parentNodeId = queryParams.get('scope_parent');
|
||||
const navigationScope = queryParams.get('navigation_scope');
|
||||
|
||||
if (navigationScope) {
|
||||
this.dashboardsService.setNavigationScope(navigationScope);
|
||||
}
|
||||
|
||||
this.changeScopes(queryParams.getAll('scopes'), parentNodeId ?? undefined, scopeNodeId ?? undefined);
|
||||
this.changeScopes(queryParams.getAll('scopes'), undefined, scopeNodeId ?? undefined);
|
||||
|
||||
// Pre-load scope node (which loads parent too) or fallback to parent node for old URLs
|
||||
const nodeToPreload = scopeNodeId ?? parentNodeId;
|
||||
// Pre-load scope node (which loads parent too)
|
||||
const nodeToPreload = scopeNodeId;
|
||||
if (nodeToPreload) {
|
||||
this.selectorService.resolvePathToRoot(nodeToPreload, this.selectorService.state.tree!).catch((error) => {
|
||||
console.error('Failed to pre-load node path', error);
|
||||
@@ -100,8 +98,6 @@ export class ScopesService implements ScopesContextValue {
|
||||
|
||||
const scopes = queryParams.getAll('scopes');
|
||||
const scopeNodeId = queryParams.get('scope_node');
|
||||
// scope_parent is for backward compatibility only
|
||||
const parentNodeId = queryParams.get('scope_parent');
|
||||
|
||||
// Check if new scopes are different from the old scopes
|
||||
const currentScopes = this.selectorService.state.appliedScopes.map((scope) => scope.scopeId);
|
||||
@@ -109,7 +105,7 @@ export class ScopesService implements ScopesContextValue {
|
||||
// We only update scopes but never delete them. This is to keep the scopes in memory if user navigates to
|
||||
// page that does not use scopes (like from dashboard to dashboard list back to dashboard). If user
|
||||
// changes the URL directly, it would trigger a reload so scopes would still be reset.
|
||||
this.changeScopes(scopes, parentNodeId ?? undefined, scopeNodeId ?? undefined);
|
||||
this.changeScopes(scopes, undefined, scopeNodeId ?? undefined);
|
||||
}
|
||||
})
|
||||
);
|
||||
|
||||
@@ -9,7 +9,7 @@ import { RecentScope } from './types';
|
||||
|
||||
interface RecentScopesProps {
|
||||
recentScopes: RecentScope[][];
|
||||
onSelect: (scopeIds: string[], parentNodeId?: string) => void;
|
||||
onSelect: (scopeIds: string[], parentNodeId?: string, scopeNodeId?: string) => void;
|
||||
}
|
||||
|
||||
export const RecentScopes = ({ recentScopes, onSelect }: RecentScopesProps) => {
|
||||
@@ -45,7 +45,8 @@ export const RecentScopes = ({ recentScopes, onSelect }: RecentScopesProps) => {
|
||||
onClick={() => {
|
||||
onSelect(
|
||||
recentScopeSet.map((s) => s.metadata.name),
|
||||
recentScopeSet[0]?.parentNode?.metadata?.name
|
||||
recentScopeSet[0]?.parentNode?.metadata?.name,
|
||||
recentScopeSet[0]?.scopeNodeId
|
||||
);
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -32,12 +32,12 @@ export function ScopesInput({
|
||||
onRemoveAllClick,
|
||||
}: ScopesInputProps) {
|
||||
const scopeNodeId = appliedScopes[0]?.scopeNodeId;
|
||||
const parentNodeIdFromUrl = appliedScopes[0]?.parentNodeId;
|
||||
const styles = useStyles2(getStyles);
|
||||
const parentNodeIdFromRecentScopes = appliedScopes[0]?.parentNodeId; // This is only set from recent scopes TODO: remove after recent scopes refactor
|
||||
const { node: scopeNode, isLoading: scopeNodeLoading } = useScopeNode(scopeNodeId);
|
||||
|
||||
// Get parent from scope node if available, otherwise use parentNodeId from URL (for backward compatibility)
|
||||
const parentNodeId = scopeNode?.spec.parentName ?? parentNodeIdFromUrl;
|
||||
// Get parent from scope node if available, otherwise fallback to parent
|
||||
const parentNodeId = scopeNode?.spec.parentName ?? parentNodeIdFromRecentScopes;
|
||||
const { node: parentNode, isLoading: parentNodeLoading } = useScopeNode(parentNodeId);
|
||||
|
||||
// Prioritize scope node subtitle over parent node title
|
||||
|
||||
@@ -132,8 +132,8 @@ export const ScopesSelector = () => {
|
||||
selectScope={selectScope}
|
||||
deselectScope={deselectScope}
|
||||
toggleExpandedNode={toggleExpandedNode}
|
||||
onRecentScopesSelect={(scopeIds: string[], parentNodeId?: string) => {
|
||||
scopesSelectorService.changeScopes(scopeIds, parentNodeId);
|
||||
onRecentScopesSelect={(scopeIds: string[], parentNodeId?: string, scopeNodeId?: string) => {
|
||||
scopesSelectorService.changeScopes(scopeIds, parentNodeId, scopeNodeId);
|
||||
scopesSelectorService.closeAndReset();
|
||||
}}
|
||||
/>
|
||||
|
||||
@@ -15,6 +15,14 @@ jest.mock('@grafana/runtime', () => ({
|
||||
push: jest.fn(),
|
||||
getLocation: jest.fn(),
|
||||
},
|
||||
config: {
|
||||
...jest.requireActual('@grafana/runtime').config,
|
||||
|
||||
featureToggles: {
|
||||
...jest.requireActual('@grafana/runtime').config.featureToggles,
|
||||
useScopeSingleNodeEndpoint: true,
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
describe('ScopesSelectorService', () => {
|
||||
@@ -69,6 +77,7 @@ describe('ScopesSelectorService', () => {
|
||||
}),
|
||||
fetchDashboards: jest.fn().mockResolvedValue([]),
|
||||
fetchScopeNavigations: jest.fn().mockResolvedValue([]),
|
||||
fetchScopeNode: jest.fn().mockResolvedValue(mockNode),
|
||||
} as unknown as jest.Mocked<ScopesApiClient>;
|
||||
|
||||
dashboardsService = {
|
||||
@@ -197,6 +206,190 @@ describe('ScopesSelectorService', () => {
|
||||
await service.open();
|
||||
expect(service.state.opened).toBe(true);
|
||||
});
|
||||
|
||||
it('should use scopeNodeId to resolve path when opening selector', async () => {
|
||||
const parentNode: ScopeNode = {
|
||||
metadata: { name: 'parent-container' },
|
||||
spec: {
|
||||
linkId: '',
|
||||
linkType: 'scope',
|
||||
//parentName: '',
|
||||
nodeType: 'container',
|
||||
title: 'Parent Container',
|
||||
},
|
||||
};
|
||||
|
||||
const childNode: ScopeNode = {
|
||||
metadata: { name: 'child-1' },
|
||||
spec: {
|
||||
linkId: 'scope-1',
|
||||
linkType: 'scope',
|
||||
parentName: 'parent-container',
|
||||
nodeType: 'leaf',
|
||||
title: 'Child 1',
|
||||
},
|
||||
};
|
||||
|
||||
// Mock API responses
|
||||
apiClient.fetchNodes.mockImplementation((options: { parent?: string; query?: string; limit?: number }) => {
|
||||
if (options.parent === '') {
|
||||
return Promise.resolve([parentNode]);
|
||||
} else if (options.parent === 'parent-container') {
|
||||
return Promise.resolve([childNode]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
apiClient.fetchScopeNode.mockImplementation((scopeNodeId: string) => {
|
||||
if (scopeNodeId === 'child-1') {
|
||||
return Promise.resolve(childNode);
|
||||
}
|
||||
return Promise.resolve(undefined);
|
||||
});
|
||||
|
||||
// Apply scope with scopeNodeId and parentNodeId set
|
||||
await service.changeScopes(['scope-1'], 'parent-container', 'child-1');
|
||||
|
||||
// Open the selector
|
||||
await service.open();
|
||||
|
||||
// Verify the tree is expanded to the selected scope's parent
|
||||
// The key fix: it should resolve path using scopeNodeId (child-1), not parentNodeId
|
||||
expect(service.state.tree?.expanded).toBe(true);
|
||||
expect(service.state.tree?.children?.['parent-container']?.expanded).toBe(true);
|
||||
expect(service.state.tree?.children?.['parent-container']?.children?.['child-1']).toBeDefined();
|
||||
});
|
||||
|
||||
it('should load parent node children when opening to selected scope', async () => {
|
||||
const parentNode: ScopeNode = {
|
||||
metadata: { name: 'parent-container' },
|
||||
spec: {
|
||||
linkId: '',
|
||||
linkType: 'scope',
|
||||
parentName: '',
|
||||
nodeType: 'container',
|
||||
title: 'Parent Container',
|
||||
},
|
||||
};
|
||||
|
||||
const childNode1: ScopeNode = {
|
||||
metadata: { name: 'child-1' },
|
||||
spec: {
|
||||
linkId: 'scope-1',
|
||||
linkType: 'scope',
|
||||
parentName: 'parent-container',
|
||||
nodeType: 'leaf',
|
||||
title: 'Child 1',
|
||||
},
|
||||
};
|
||||
|
||||
const childNode2: ScopeNode = {
|
||||
metadata: { name: 'child-2' },
|
||||
spec: {
|
||||
linkId: 'scope-2',
|
||||
linkType: 'scope',
|
||||
parentName: 'parent-container',
|
||||
nodeType: 'leaf',
|
||||
title: 'Child 2',
|
||||
},
|
||||
};
|
||||
|
||||
const childNode3: ScopeNode = {
|
||||
metadata: { name: 'child-3' },
|
||||
spec: {
|
||||
linkId: 'scope-3',
|
||||
linkType: 'scope',
|
||||
parentName: 'parent-container',
|
||||
nodeType: 'leaf',
|
||||
title: 'Child 3',
|
||||
},
|
||||
};
|
||||
|
||||
// Mock API responses
|
||||
apiClient.fetchNodes.mockImplementation((options: { parent?: string; query?: string; limit?: number }) => {
|
||||
if (options.parent === '') {
|
||||
return Promise.resolve([parentNode]);
|
||||
} else if (options.parent === 'parent-container') {
|
||||
return Promise.resolve([childNode1, childNode2, childNode3]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
apiClient.fetchScopeNode.mockImplementation((scopeNodeId: string) => {
|
||||
if (scopeNodeId === 'child-2') {
|
||||
return Promise.resolve(childNode2);
|
||||
} else if (scopeNodeId === 'parent-container') {
|
||||
return Promise.resolve(parentNode);
|
||||
}
|
||||
return Promise.resolve(undefined);
|
||||
});
|
||||
|
||||
await service.changeScopes(['scope-2'], 'parent-container', 'child-2');
|
||||
await service.open();
|
||||
|
||||
// Verify all sibling nodes are loaded (not just the selected one)
|
||||
expect(service.state.tree?.children?.['parent-container']?.children?.['child-1']).toBeDefined();
|
||||
expect(service.state.tree?.children?.['parent-container']?.children?.['child-2']).toBeDefined();
|
||||
expect(service.state.tree?.children?.['parent-container']?.children?.['child-3']).toBeDefined();
|
||||
|
||||
// Verify childrenLoaded flag is set on the parent
|
||||
expect(service.state.tree?.children?.['parent-container']?.childrenLoaded).toBe(true);
|
||||
});
|
||||
|
||||
it('should only load children if childrenLoaded is false', async () => {
|
||||
const parentNode: ScopeNode = {
|
||||
metadata: { name: 'parent-container' },
|
||||
spec: {
|
||||
linkId: '',
|
||||
linkType: 'scope',
|
||||
parentName: '',
|
||||
nodeType: 'container',
|
||||
title: 'Parent Container',
|
||||
},
|
||||
};
|
||||
|
||||
const childNode: ScopeNode = {
|
||||
metadata: { name: 'child-1' },
|
||||
spec: {
|
||||
linkId: 'scope-1',
|
||||
linkType: 'scope',
|
||||
parentName: 'parent-container',
|
||||
nodeType: 'leaf',
|
||||
title: 'Child 1',
|
||||
},
|
||||
};
|
||||
|
||||
apiClient.fetchNodes.mockImplementation((options: { parent?: string; query?: string; limit?: number }) => {
|
||||
if (options.parent === '') {
|
||||
return Promise.resolve([parentNode]);
|
||||
} else if (options.parent === 'parent-container') {
|
||||
return Promise.resolve([childNode]);
|
||||
}
|
||||
return Promise.resolve([]);
|
||||
});
|
||||
|
||||
apiClient.fetchScopeNode.mockImplementation((scopeNodeId: string) => {
|
||||
if (scopeNodeId === 'child-1') {
|
||||
return Promise.resolve(childNode);
|
||||
} else if (scopeNodeId === 'parent-container') {
|
||||
return Promise.resolve(parentNode);
|
||||
}
|
||||
return Promise.resolve(undefined);
|
||||
});
|
||||
|
||||
await service.changeScopes(['scope-1'], 'parent-container', 'child-1');
|
||||
|
||||
// First open
|
||||
await service.open();
|
||||
|
||||
// Close and open again
|
||||
service.closeAndReset();
|
||||
await service.open();
|
||||
|
||||
// The key: childrenLoaded flag should prevent redundant fetches
|
||||
// Verify the flag is set correctly
|
||||
expect(service.state.tree?.children?.['parent-container']?.childrenLoaded).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('closeAndReset', () => {
|
||||
|
||||
@@ -218,6 +218,8 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
children: undefined,
|
||||
};
|
||||
}
|
||||
// Set loaded to true if node is a container
|
||||
treeNode.childrenLoaded = true;
|
||||
});
|
||||
|
||||
// TODO: we might not want to update the tree as a side effect of this function
|
||||
@@ -256,7 +258,6 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
const selectedScope = {
|
||||
scopeId: scopeNode.spec.linkId,
|
||||
scopeNodeId: scopeNode.metadata.name,
|
||||
parentNodeId: parentNode?.metadata.name,
|
||||
};
|
||||
|
||||
// if something is selected we look at parent and see if we are selecting in the same category or not. As we
|
||||
@@ -342,20 +343,29 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
|
||||
if (scopes.length > 0) {
|
||||
const fetchedScopes = await this.apiClient.fetchMultipleScopes(scopes.map((s) => s.scopeId));
|
||||
|
||||
// Fetch the scope node if it is not available
|
||||
let newNodesState = { ...this.state.nodes };
|
||||
let scopeNode = scopes[0]?.scopeNodeId ? this.state.nodes[scopes[0]?.scopeNodeId] : undefined;
|
||||
|
||||
if (!scopeNode && config.featureToggles.useScopeSingleNodeEndpoint && scopes[0]?.scopeNodeId) {
|
||||
scopeNode = await this.apiClient.fetchScopeNode(scopes[0]?.scopeNodeId);
|
||||
if (scopeNode) {
|
||||
newNodesState[scopeNode.metadata.name] = scopeNode;
|
||||
}
|
||||
}
|
||||
|
||||
const newScopesState = { ...this.state.scopes };
|
||||
for (const scope of fetchedScopes) {
|
||||
newScopesState[scope.metadata.name] = scope;
|
||||
}
|
||||
|
||||
const scopeNode = scopes[0]?.scopeNodeId ? this.state.nodes[scopes[0]?.scopeNodeId] : undefined;
|
||||
|
||||
// If parentNodeId is provided, use it directly as the parent node
|
||||
// If not provided, try to get the parent from the scope node
|
||||
// When selected from recent scopes, we don't have access to the scope node (if it hasn't been loaded), but we do have access to the parent node from local storage.
|
||||
const parentNodeId = scopes[0]?.parentNodeId || scopeNode?.spec.parentName;
|
||||
const parentNodeId = scopes[0]?.parentNodeId ?? scopeNode?.spec.parentName;
|
||||
const parentNode = parentNodeId ? this.state.nodes[parentNodeId] : undefined;
|
||||
|
||||
this.addRecentScopes(fetchedScopes, parentNode);
|
||||
this.addRecentScopes(fetchedScopes, parentNode, scopes[0]?.scopeNodeId);
|
||||
this.updateState({ scopes: newScopesState, loading: false });
|
||||
}
|
||||
};
|
||||
@@ -398,16 +408,19 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
this.dashboardsService.setNavigationScope(undefined);
|
||||
};
|
||||
|
||||
private addRecentScopes = (scopes: Scope[], parentNode?: ScopeNode) => {
|
||||
private addRecentScopes = (scopes: Scope[], parentNode?: ScopeNode, scopeNodeId?: string) => {
|
||||
if (scopes.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
const newScopes: RecentScope[] = structuredClone(scopes);
|
||||
// Set parent node for the first scope. We don't currently support multiple parent nodes being displayed, hence we only add for the first one
|
||||
// Set parent node and scopeNodeId for the first scope. We don't currently support multiple parent nodes being displayed, hence we only add for the first one
|
||||
if (parentNode) {
|
||||
newScopes[0].parentNode = parentNode;
|
||||
}
|
||||
if (scopeNodeId) {
|
||||
newScopes[0].scopeNodeId = scopeNodeId;
|
||||
}
|
||||
|
||||
const RECENT_SCOPES_MAX_LENGTH = 5;
|
||||
|
||||
@@ -452,15 +465,31 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
* Opens the scopes selector drawer and loads the root nodes if they are not loaded yet.
|
||||
*/
|
||||
public open = async () => {
|
||||
if (!this.state.tree.children || Object.keys(this.state.tree.children).length === 0) {
|
||||
if (
|
||||
!this.state.tree.children ||
|
||||
Object.keys(this.state.tree.children).length === 0 ||
|
||||
!this.state.tree.childrenLoaded
|
||||
) {
|
||||
await this.filterNode('', '');
|
||||
}
|
||||
|
||||
// If the scopeNode isn't avilable, fetch it and add it to the nodes cache
|
||||
if (
|
||||
config.featureToggles.useScopeSingleNodeEndpoint &&
|
||||
this.state.selectedScopes[0]?.scopeNodeId &&
|
||||
!this.state.nodes[this.state.selectedScopes[0].scopeNodeId]
|
||||
) {
|
||||
const scopeNode = await this.apiClient.fetchScopeNode(this.state.selectedScopes[0].scopeNodeId);
|
||||
if (scopeNode) {
|
||||
this.updateState({ nodes: { ...this.state.nodes, [scopeNode.metadata.name]: scopeNode } });
|
||||
}
|
||||
}
|
||||
|
||||
// First close all nodes
|
||||
let newTree = closeNodes(this.state.tree);
|
||||
|
||||
if (this.state.selectedScopes.length && this.state.selectedScopes[0].parentNodeId) {
|
||||
let path = getPathOfNode(this.state.selectedScopes[0].parentNodeId, this.state.nodes);
|
||||
if (this.state.selectedScopes.length && this.state.selectedScopes[0].scopeNodeId) {
|
||||
let path = getPathOfNode(this.state.selectedScopes[0].scopeNodeId, this.state.nodes);
|
||||
|
||||
// Get node at path, and request it's children if they don't exist yet
|
||||
let nodeAtPath = treeNodeAtPath(newTree, path);
|
||||
@@ -468,22 +497,30 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
// In the cases where nodes are not in the tree yet
|
||||
if (!nodeAtPath) {
|
||||
try {
|
||||
newTree = (await this.resolvePathToRoot(this.state.selectedScopes[0].parentNodeId, newTree)).tree;
|
||||
const result = await this.resolvePathToRoot(this.state.selectedScopes[0].scopeNodeId, newTree);
|
||||
newTree = result.tree;
|
||||
// Update path to use the resolved path since nodes have been fetched
|
||||
path = result.path.map((n) => n.metadata.name);
|
||||
path.unshift('');
|
||||
nodeAtPath = treeNodeAtPath(newTree, path);
|
||||
} catch (error) {
|
||||
console.error('Failed to resolve path to root', error);
|
||||
}
|
||||
}
|
||||
|
||||
if (nodeAtPath && !nodeAtPath.children) {
|
||||
// We have resolved to root, which means the parent node should be available
|
||||
let parentPath = path.slice(0, -1);
|
||||
let parentNodeAtPath = treeNodeAtPath(newTree, parentPath);
|
||||
|
||||
if (parentNodeAtPath && !parentNodeAtPath.childrenLoaded) {
|
||||
// This will update the tree with the children
|
||||
const { newTree: newTreeWithChildren } = await this.loadNodeChildren(path, nodeAtPath, '');
|
||||
const { newTree: newTreeWithChildren } = await this.loadNodeChildren(parentPath, parentNodeAtPath, '');
|
||||
newTree = newTreeWithChildren;
|
||||
}
|
||||
|
||||
// Expand the nodes to the selected scope - must be done after loading children
|
||||
try {
|
||||
newTree = expandNodes(newTree, path);
|
||||
newTree = expandNodes(newTree, parentPath);
|
||||
} catch (error) {
|
||||
console.error('Failed to expand nodes', error);
|
||||
}
|
||||
|
||||
@@ -25,7 +25,7 @@ export interface ScopesTreeProps {
|
||||
|
||||
// Recent scopes are only shown at the root node
|
||||
recentScopes?: Scope[][];
|
||||
onRecentScopesSelect?: (scopeIds: string[], parentNodeId?: string) => void;
|
||||
onRecentScopesSelect?: (scopeIds: string[], parentNodeId?: string, scopeNodeId?: string) => void;
|
||||
|
||||
toggleExpandedNode: (scopeNodeId: string) => void;
|
||||
}
|
||||
|
||||
@@ -237,5 +237,120 @@ describe('scopesTreeUtils', () => {
|
||||
expect(newTree.children?.child1.expanded).toBe(false);
|
||||
expect(newTree.children?.child1.children?.grandchild1.expanded).toBe(false);
|
||||
});
|
||||
|
||||
it('should set childrenLoaded to false for newly inserted nodes', () => {
|
||||
const tree: TreeNode = {
|
||||
expanded: false,
|
||||
scopeNodeId: 'root',
|
||||
query: '',
|
||||
children: {},
|
||||
};
|
||||
|
||||
const path: ScopeNode[] = [
|
||||
{
|
||||
metadata: { name: 'child1' },
|
||||
spec: {
|
||||
parentName: 'root',
|
||||
nodeType: 'container',
|
||||
title: 'Child 1',
|
||||
},
|
||||
},
|
||||
{
|
||||
metadata: { name: 'grandchild1' },
|
||||
spec: {
|
||||
parentName: 'child1',
|
||||
nodeType: 'container',
|
||||
title: 'Grandchild 1',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const newTree = insertPathNodesIntoTree(tree, path);
|
||||
|
||||
// Since we only handle insertion, it should never be true
|
||||
expect(newTree.childrenLoaded).toBe(false);
|
||||
|
||||
// Newly inserted nodes should have childrenLoaded set to false
|
||||
expect(newTree.children?.child1.childrenLoaded).toBe(false);
|
||||
expect(newTree.children?.child1.children?.grandchild1.childrenLoaded).toBe(false);
|
||||
});
|
||||
|
||||
it('should preserve existing children when inserting path', () => {
|
||||
const tree: TreeNode = {
|
||||
expanded: true,
|
||||
scopeNodeId: 'root',
|
||||
query: '',
|
||||
childrenLoaded: true,
|
||||
children: {
|
||||
existingChild: {
|
||||
expanded: false,
|
||||
scopeNodeId: 'existingChild',
|
||||
query: '',
|
||||
childrenLoaded: true,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const path: ScopeNode[] = [
|
||||
{
|
||||
metadata: { name: 'newChild' },
|
||||
spec: {
|
||||
parentName: 'root',
|
||||
nodeType: 'container',
|
||||
title: 'New Child',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const newTree = insertPathNodesIntoTree(tree, path);
|
||||
|
||||
// Existing child should still be there
|
||||
expect(newTree.children?.existingChild).toBeDefined();
|
||||
expect(newTree.children?.existingChild.childrenLoaded).toBe(true);
|
||||
|
||||
// New child should be added
|
||||
expect(newTree.children?.newChild).toBeDefined();
|
||||
expect(newTree.children?.newChild.childrenLoaded).toBe(false);
|
||||
|
||||
// Since we only handle insertion, it should never be true
|
||||
expect(newTree.childrenLoaded).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle empty path', () => {
|
||||
const tree: TreeNode = {
|
||||
expanded: false,
|
||||
scopeNodeId: 'root',
|
||||
query: '',
|
||||
children: {},
|
||||
};
|
||||
|
||||
const path: ScopeNode[] = [];
|
||||
|
||||
const newTree = insertPathNodesIntoTree(tree, path);
|
||||
|
||||
// Tree should remain unchanged except for childrenLoaded
|
||||
expect(newTree.scopeNodeId).toBe('root');
|
||||
expect(newTree.childrenLoaded).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should maintain the childrenLoaded value of the root node when inserting path', () => {
|
||||
const tree: TreeNode = {
|
||||
expanded: false,
|
||||
scopeNodeId: 'root',
|
||||
query: '',
|
||||
childrenLoaded: true,
|
||||
};
|
||||
|
||||
const path: ScopeNode[] = [
|
||||
{
|
||||
metadata: { name: 'child1' },
|
||||
spec: { parentName: 'root', nodeType: 'container', title: 'Child 1' },
|
||||
},
|
||||
];
|
||||
|
||||
const newTree = insertPathNodesIntoTree(tree, path);
|
||||
|
||||
expect(newTree.childrenLoaded).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -126,6 +126,7 @@ export const insertPathNodesIntoTree = (tree: TreeNode, path: ScopeNode[]) => {
|
||||
treeNode.children = { ...treeNode.children };
|
||||
if (!childNodeName) {
|
||||
console.warn('Failed to insert full path into tree. Did not find child to' + stringPath[index]);
|
||||
treeNode.childrenLoaded = treeNode.childrenLoaded ?? false;
|
||||
return treeNode;
|
||||
}
|
||||
treeNode.children[childNodeName] = {
|
||||
@@ -133,7 +134,9 @@ export const insertPathNodesIntoTree = (tree: TreeNode, path: ScopeNode[]) => {
|
||||
scopeNodeId: childNodeName,
|
||||
query: '',
|
||||
children: undefined,
|
||||
childrenLoaded: false,
|
||||
};
|
||||
treeNode.childrenLoaded = treeNode.childrenLoaded ?? false;
|
||||
return treeNode;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ export type ScopesMap = Record<string, Scope>;
|
||||
export interface SelectedScope {
|
||||
scopeId: string;
|
||||
scopeNodeId?: string;
|
||||
// @deprecated Used to display title next to selected scope. scopeNodeId is used to resolve this anyways. Remove if we can confirm it doesn't break anything.
|
||||
// Used for recent scopes functionality when scope node isn't loaded yet
|
||||
parentNodeId?: string;
|
||||
}
|
||||
|
||||
@@ -17,10 +17,13 @@ export interface TreeNode {
|
||||
expanded: boolean;
|
||||
query: string;
|
||||
children?: Record<string, TreeNode>;
|
||||
// Check if we have loaded all the children. Used when resolving to root.
|
||||
childrenLoaded?: boolean;
|
||||
}
|
||||
|
||||
export interface RecentScope extends Scope {
|
||||
parentNode?: ScopeNode;
|
||||
scopeNodeId?: string;
|
||||
}
|
||||
|
||||
// Zod schemas for type validation
|
||||
@@ -64,4 +67,5 @@ export const ScopeNodeSchema = z.object({
|
||||
|
||||
export const RecentScopeSchema = ScopeSchema.extend({
|
||||
parentNode: ScopeNodeSchema.optional(),
|
||||
scopeNodeId: z.string().optional(),
|
||||
});
|
||||
|
||||
@@ -62,7 +62,7 @@ export function useScopesHighlighting({
|
||||
: undefined;
|
||||
|
||||
if (parentNode?.spec.disableMultiSelect && changeScopes && scopeNodes[nodeId]?.spec.linkId) {
|
||||
changeScopes([scopeNodes[nodeId].spec.linkId], parentNode.metadata.name);
|
||||
changeScopes([scopeNodes[nodeId].spec.linkId], undefined, nodeId);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -91,7 +91,7 @@ describe('Selector', () => {
|
||||
it('Should initializae values from the URL', async () => {
|
||||
const mockLocation = {
|
||||
pathname: '/dashboard',
|
||||
search: '?scopes=grafana&scope_parent=applications',
|
||||
search: '?scopes=grafana&scope_node=applications-grafana',
|
||||
hash: '',
|
||||
key: 'test',
|
||||
state: null,
|
||||
@@ -105,6 +105,7 @@ describe('Selector', () => {
|
||||
// Lowercase because we don't have any backend that returns the correct case, then it falls back to the value in the URL
|
||||
expectScopesSelectorValue('grafana');
|
||||
await openSelector();
|
||||
//screen.debug(undefined, 100000);
|
||||
expectResultApplicationsGrafanaSelected();
|
||||
|
||||
jest.spyOn(locationService, 'getLocation').mockRestore();
|
||||
|
||||
@@ -194,6 +194,45 @@ describe('Tree', () => {
|
||||
expectResultApplicationsMimirPresent();
|
||||
});
|
||||
|
||||
it('Opens to a selected scope and shows all sibling nodes', async () => {
|
||||
// Select a scope and apply
|
||||
await openSelector();
|
||||
await expandResultApplications();
|
||||
await selectResultApplicationsMimir();
|
||||
await applyScopes();
|
||||
|
||||
// Reopen selector - should show the selected scope AND all its siblings
|
||||
await openSelector();
|
||||
|
||||
// Verify all sibling nodes (Grafana, Mimir, Cloud) are visible
|
||||
expectResultApplicationsGrafanaPresent();
|
||||
expectResultApplicationsMimirPresent();
|
||||
expectResultApplicationsCloudPresent();
|
||||
|
||||
// Verify the Applications container is expanded
|
||||
expect(screen.getByRole('button', { name: 'Applications' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('Opens to a nested selected scope and shows all siblings at that level', async () => {
|
||||
// Select a nested scope
|
||||
await openSelector();
|
||||
await expandResultApplications();
|
||||
await expandResultApplicationsCloud();
|
||||
await selectResultApplicationsCloudDev();
|
||||
await applyScopes();
|
||||
|
||||
// Reopen selector - should expand to Cloud and show all its children
|
||||
await openSelector();
|
||||
|
||||
// Verify the full path is expanded
|
||||
expect(screen.getByRole('button', { name: 'Cloud' })).toBeInTheDocument();
|
||||
|
||||
// Verify all siblings at the Cloud level are visible
|
||||
// The test should verify that when Cloud is expanded, we see all its children
|
||||
// (This depends on what siblings Dev has - at minimum, we should see Dev itself)
|
||||
expect(screen.getByRole('treeitem', { name: 'Dev' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('Persists a scope', async () => {
|
||||
await openSelector();
|
||||
await expandResultApplications();
|
||||
@@ -291,7 +330,7 @@ describe('Tree', () => {
|
||||
expectScopesHeadline('Recommended');
|
||||
});
|
||||
|
||||
it('Should open to a specific path when scopes and scope_parent are provided', async () => {
|
||||
it('Should open to a specific path when scopes and scope_node are applied', async () => {
|
||||
await openSelector();
|
||||
await expandResultApplications();
|
||||
await expandResultApplicationsCloud();
|
||||
|
||||
@@ -412,6 +412,12 @@ export const getMock = jest
|
||||
return mocksScopes.find((scope) => scope.metadata.name.toLowerCase() === name.toLowerCase()) ?? {};
|
||||
}
|
||||
|
||||
if (url.startsWith('/apis/scope.grafana.app/v0alpha1/namespaces/default/scopenodes/')) {
|
||||
const name = url.replace('/apis/scope.grafana.app/v0alpha1/namespaces/default/scopenodes/', '');
|
||||
|
||||
return mocksNodes.find((node) => node.metadata.name === name);
|
||||
}
|
||||
|
||||
if (url.startsWith('/apis/scope.grafana.app/v0alpha1/namespaces/default/find/scope_dashboard_bindings')) {
|
||||
return {
|
||||
items: mocksScopeDashboardBindings.filter(({ spec: { scope: bindingScope } }) =>
|
||||
|
||||
Reference in New Issue
Block a user