public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Scopes: Resolve selector path on initial load (#111624)

Browse Source 
* Recusively load nodes, and insert into tree

* Remove console.logs

* Simply functions

* Always show recent scopes category

* Fix issues with parent node remaining in URL

* Always display recent scopes

* Make sure chdilren are loaded when collapsing

* Add test cases for expanded items and imrpove a11y markup

* Remove recent scopes always showing and update tests

* Fix linting issue

* Move insertPathNodesIntoTree to treeUtils

* Add test for insertPathNodesIntoTree

* Remove comment
This commit is contained in:
Tobias Skarhed
2025-09-30 11:49:22 +02:00
committed by GitHub
parent 4a229009ab
commit 3668d02650
11 changed files with 215 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/app/features/scopes/ScopesService.ts
+17 -11
View File
@@ -76,8 +76,8 @@ export class ScopesService implements ScopesContextValue {
// Pre-load parent node, to prevent UI flickering
if (parentNodeId) {
this.selectorService.getScopeNode(parentNodeId).catch((error) => {
console.error('Failed to pre-load parent node', error);
this.selectorService.resolvePathToRoot(parentNodeId, this.selectorService.state.tree!).catch((error) => {
console.error('Failed to pre-load parent node path', error);
});
}
@@ -94,7 +94,9 @@ export class ScopesService implements ScopesContextValue {
const parentNode = queryParams.get('scope_parent');
const scopes = queryParams.getAll('scopes');
if (scopes.length) {
// 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)) {
// 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.
@@ -105,17 +107,21 @@ export class ScopesService implements ScopesContextValue {
// Update the URL based on change in the scopes state
this.subscriptions.push(
selectorService.subscribeToState((state, prev) => {
const oldParentNode = prev.appliedScopes[0]?.parentNodeId;
selectorService.subscribeToState((state, prevState) => {
const oldParentNode = prevState.appliedScopes[0]?.parentNodeId;
const newParentNode = state.appliedScopes[0]?.parentNodeId;
if (oldParentNode !== newParentNode && newParentNode) {
this.locationService.partial({ scope_parent: newParentNode }, true);
}
const oldScopeNames = prev.appliedScopes.map((scope) => scope.scopeId);
const parentNodeChanged = oldParentNode !== newParentNode;
const oldScopeNames = prevState.appliedScopes.map((scope) => scope.scopeId);
const newScopeNames = state.appliedScopes.map((scope) => scope.scopeId);
if (!isEqual(oldScopeNames, newScopeNames)) {
this.locationService.partial({ scopes: newScopeNames }, true);
const scopesChanged = !isEqual(oldScopeNames, newScopeNames);
if (scopesChanged) {
this.locationService.partial(
{ scopes: newScopeNames, scope_parent: parentNodeChanged ? newParentNode || null : oldParentNode },
true
);
}
})
);
public/app/features/scopes/selector/RecentScopes.tsx
+3 -1
View File
@@ -39,7 +39,9 @@ export const RecentScopes = ({ recentScopes, onSelect }: RecentScopesProps) => {
recentScopes.map((recentScopeSet) => (
<button
className={styles.recentScopeButton}
key={recentScopeSet.map((s) => s.metadata.name).join(',')}
key={
recentScopeSet.map((s) => s.metadata.name).join(',') + recentScopeSet[0]?.parentNode?.metadata?.name
}
onClick={() => {
onSelect(
recentScopeSet.map((s) => s.metadata.name),
public/app/features/scopes/selector/ScopesSelectorService.ts
+68 -21
View File
@@ -11,6 +11,7 @@ import {
closeNodes,
expandNodes,
getPathOfNode,
insertPathNodesIntoTree,
isNodeExpandable,
isNodeSelectable,
modifyTreeNodeAtPath,
@@ -99,6 +100,32 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
}
};
private getNodePath = async (scopeNodeId: string): Promise<ScopeNode[]> => {
const node = await this.getScopeNode(scopeNodeId);
if (!node) {
return [];
}
const parentPath =
node.spec.parentName && node.spec.parentName !== '' ? await this.getNodePath(node.spec.parentName) : [];
return [...parentPath, node];
};
public resolvePathToRoot = async (
scopeNodeId: string,
tree: TreeNode
): Promise<{ path: ScopeNode[]; tree: TreeNode }> => {
if (!tree) {
throw new Error('Tree is required');
}
const nodePath = await this.getNodePath(scopeNodeId);
const newTree = insertPathNodesIntoTree(tree, nodePath);
this.updateState({ tree: newTree });
return { path: nodePath, tree: newTree };
};
// Resets query and toggles expanded state of a node
public toggleExpandedNode = async (scopeNodeId: string) => {
const path = getPathOfNode(scopeNodeId, this.state.nodes);
@@ -112,25 +139,22 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
throw new Error(`Trying to expand node at id ${scopeNodeId} that is not expandable`);
}
// Collapse if expanded
if (nodeToToggle.expanded) {
const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => {
treeNode.expanded = false;
// Resets query when collapsing
treeNode.query = '';
});
this.updateState({ tree: newTree });
return;
}
// Expand if collapsed
const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => {
treeNode.expanded = true;
treeNode.expanded = !nodeToToggle.expanded;
treeNode.query = '';
});
this.updateState({ tree: newTree });
await this.loadNodeChildren(path, nodeToToggle);
this.updateState({ tree: newTree });
// If we are collapsing, we need to make sure that all the parent's children are avilable
if (nodeToToggle.expanded === true) {
const parentPath = path.slice(0, -1);
const parentNode = treeNodeAtPath(this.state.tree!, parentPath);
if (parentNode) {
await this.loadNodeChildren(parentPath, parentNode, parentNode.query);
}
} else {
await this.loadNodeChildren(path, nodeToToggle);
}
};
public filterNode = async (scopeNodeId: string, query: string) => {
@@ -227,7 +251,9 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
}
});
// TODO: we might not want to update the tree as a side effect of this function
this.updateState({ tree: newTree, nodes: newNodes, loadingNodeName: undefined });
return { newTree };
};
/**
@@ -421,13 +447,34 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
// First close all nodes
let newTree = closeNodes(this.state.tree!);
if (this.state.selectedScopes.length && this.state.selectedScopes[0].scopeNodeId) {
let path = getPathOfNode(this.state.selectedScopes[0].scopeNodeId, this.state.nodes);
// we want to expand the nodes parent not the node itself
path = path.slice(0, path.length - 1);
if (this.state.selectedScopes.length && this.state.selectedScopes[0].parentNodeId) {
let path = getPathOfNode(this.state.selectedScopes[0].parentNodeId, this.state.nodes);
// Expand the nodes to the selected scope
newTree = expandNodes(newTree, path);
// Get node at path, and request it's children if they don't exist yet
let nodeAtPath = treeNodeAtPath(newTree, path);
// 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;
nodeAtPath = treeNodeAtPath(newTree, path);
} catch (error) {
console.error('Failed to resolve path to root', error);
}
}
if (nodeAtPath && !nodeAtPath.children) {
// This will update the tree with the children
const { newTree: newTreeWithChildren } = await this.loadNodeChildren(path, nodeAtPath, '');
newTree = newTreeWithChildren;
}
// Expand the nodes to the selected scope - must be done after loading children
try {
newTree = expandNodes(newTree, path);
} catch (error) {
console.error('Failed to expand nodes', error);
}
}
this.resetSelection();
public/app/features/scopes/selector/ScopesTreeItem.tsx
+5 -1
View File
@@ -126,7 +126,11 @@ export function ScopesTreeItem({
<button
className={styles.expand}
data-testid={`scopes-tree-${treeNode.scopeNodeId}-expand`}
aria-label={treeNode.expanded ? t('scopes.tree.collapse', 'Collapse') : t('scopes.tree.expand', 'Expand')}
aria-label={
treeNode.expanded
? t('scopes.tree.collapse', 'Collapse {{title}}', { title: titleText })
: t('scopes.tree.expand', 'Expand {{title}}', { title: titleText })
}
onClick={() => {
toggleExpandedNode(treeNode.scopeNodeId);
}}
public/app/features/scopes/selector/scopesTreeUtils.test.ts
+35
View File
@@ -8,6 +8,7 @@ import {
getPathOfNode,
modifyTreeNodeAtPath,
treeNodeAtPath,
insertPathNodesIntoTree,
} from './scopesTreeUtils';
import { TreeNode, NodesMap } from './types';
@@ -203,4 +204,38 @@ describe('scopesTreeUtils', () => {
expect(result).toBeUndefined();
});
});
describe('insertPathNodesIntoTree', () => {
it('should insert nodes into tree', () => {
const tree: TreeNode = {
expanded: false,
scopeNodeId: 'root',
query: '',
children: {},
};
const path: ScopeNode[] = [
{
metadata: { name: 'child1' },
spec: {
parentName: 'root',
nodeType: 'container',
title: 'Root',
},
},
{
metadata: { name: 'grandchild1' },
spec: {
parentName: 'child1',
nodeType: 'container',
title: 'Child 1',
},
},
];
const newTree = insertPathNodesIntoTree(tree, path);
expect(newTree.children?.child1.expanded).toBe(false);
expect(newTree.children?.child1.children?.grandchild1.expanded).toBe(false);
});
});
});
public/app/features/scopes/selector/scopesTreeUtils.ts
+30
View File
@@ -109,3 +109,33 @@ export function treeNodeAtPath(tree: TreeNode, path: string[]) {
return treeNode;
}
// Path starts with root node and goes down
export const insertPathNodesIntoTree = (tree: TreeNode, path: ScopeNode[]) => {
const stringPath = path.map((n) => n.metadata.name);
stringPath.unshift('');
let newTree = tree;
// Go down the tree, don't iterate over the last node
for (let index = 0; index < stringPath.length - 1; index++) {
const childNodeName = stringPath[index + 1];
// Path up to iteration point
const pathSlice = stringPath.slice(0, index + 1);
newTree = modifyTreeNodeAtPath(newTree, pathSlice, (treeNode) => {
treeNode.children = { ...treeNode.children };
if (!childNodeName) {
console.warn('Failed to insert full path into tree. Did not find child to' + stringPath[index]);
return treeNode;
}
treeNode.children[childNodeName] = {
expanded: false,
scopeNodeId: childNodeName,
query: '',
children: undefined,
};
return treeNode;
});
}
return newTree;
};
public/app/features/scopes/tests/selector.test.ts
+30
View File
@@ -21,6 +21,7 @@ import {
expectRecentScopeNotPresent,
expectRecentScopeNotPresentInDocument,
expectRecentScopesSection,
expectResultApplicationsGrafanaSelected,
expectScopesSelectorValue,
} from './utils/assertions';
import { getDatasource, getInstanceSettings, getMock, mocksScopes } from './utils/mocks';
@@ -44,6 +45,7 @@ describe('Selector', () => {
beforeAll(() => {
config.featureToggles.scopeFilters = true;
config.featureToggles.groupByVariable = true;
config.featureToggles.useScopeSingleNodeEndpoint = true;
});
beforeEach(async () => {
@@ -85,6 +87,29 @@ describe('Selector', () => {
expect(dashboardReloadSpy).not.toHaveBeenCalled();
});
it('Should initializae values from the URL', async () => {
const mockLocation = {
pathname: '/dashboard',
search: '?scopes=grafana&scope_parent=applications',
hash: '',
key: 'test',
state: null,
};
jest.spyOn(locationService, 'getLocation').mockReturnValue(mockLocation);
jest.spyOn(locationService, 'getSearch').mockReturnValue(new URLSearchParams(mockLocation.search));
await resetScenes([fetchSelectedScopesSpy, dashboardReloadSpy]);
await renderDashboard();
// 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();
expectResultApplicationsGrafanaSelected();
jest.spyOn(locationService, 'getLocation').mockRestore();
jest.spyOn(locationService, 'getSearch').mockRestore();
});
describe('Recent scopes', () => {
it('Recent scopes should appear after selecting a second set of scopes', async () => {
await openSelector();
@@ -109,6 +134,9 @@ describe('Selector', () => {
expectScopesSelectorValue('Grafana');
await openSelector();
// Close to root node so we can see the recent scopes
await expandResultApplications();
await expandRecentScopes();
expectRecentScope('Grafana, Mimir Applications');
expectRecentScopeNotPresent('Grafana Applications');
@@ -125,6 +153,8 @@ describe('Selector', () => {
await applyScopes();
await openSelector();
// Close to root node so we can try to see the recent scopes
await expandResultApplications();
expectRecentScopeNotPresentInDocument();
});
public/app/features/scopes/tests/tree.test.ts
+12
View File
@@ -267,6 +267,18 @@ describe('Tree', () => {
expectScopesHeadline('Recommended');
});
it('Should open to a specific path when scopes and scope_parent are provided', async () => {
await openSelector();
await expandResultApplications();
await expandResultApplicationsCloud();
await selectResultApplicationsCloudDev();
await applyScopes();
await openSelector();
// Verify that Cloud is expanded
expect(screen.getByRole('button', { name: 'Collapse Cloud' })).toBeInTheDocument();
});
describe('Keyboard Navigation', () => {
it('should navigate through items with arrow keys when search is focused', async () => {
await openSelector();
public/app/features/scopes/tests/utils/actions.ts
+9 -3
View File
@@ -1,4 +1,5 @@
import { act, fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { DateTime, makeTimeRange, dateMath } from '@grafana/data';
import { MultiValueVariable, sceneGraph, VariableValue } from '@grafana/scenes';
@@ -19,7 +20,6 @@ import {
getResultApplicationsCloudDevSelect,
getResultApplicationsCloudExpand,
getResultApplicationsCloudSelect,
getResultApplicationsExpand,
getResultApplicationsGrafanaSelect,
getResultApplicationsMimirSelect,
getResultCloudDevRadio,
@@ -31,9 +31,11 @@ import {
getSelectorClear,
getSelectorInput,
getTreeSearch,
findResultApplicationsExpand,
} from './selectors';
const click = async (selector: () => HTMLElement) => act(() => fireEvent.click(selector()));
const click = async (selector: () => HTMLElement) => act(() => userEvent.click(selector()));
const type = async (selector: () => HTMLInputElement, value: string) => {
await act(() => fireEvent.input(selector(), { target: { value } }));
await jest.runOnlyPendingTimersAsync();
@@ -51,7 +53,11 @@ export const cancelScopes = async () => click(getSelectorCancel);
export const searchScopes = async (value: string) => type(getTreeSearch, value);
export const clearScopesSearch = async () => type(getTreeSearch, '');
export const expandRecentScopes = async () => click(getRecentScopesSection);
export const expandResultApplications = async () => click(getResultApplicationsExpand);
export const expandResultApplications = async () => {
// Since this is the first in the tree after expansion, we need it to appear async, hence we use find instead of get
const el = await findResultApplicationsExpand();
await click(() => el);
};
export const expandResultApplicationsCloud = async () => click(getResultApplicationsCloudExpand);
export const expandResultCloud = async () => click(getResultCloudExpand);
export const selectRecentScope = async (scope: string) => click(() => getRecentScopeSet(scope));
public/app/features/scopes/tests/utils/selectors.ts
+4
View File
@@ -63,7 +63,11 @@ export const getNotFoundForFilterClear = () => screen.getByTestId(selectors.dash
export const getTreeSearch = () => screen.getByTestId<HTMLInputElement>(selectors.tree.search);
export const getTreeHeadline = () => screen.getByTestId(selectors.tree.headline);
export const getResultApplicationsExpand = () => screen.getByTestId(selectors.tree.expand('applications'));
export const findResultApplicationsExpand = async () =>
await screen.findByTestId(selectors.tree.expand('applications'));
export const queryResultApplicationsGrafanaSelect = () =>
screen.queryByTestId<HTMLInputElement>(selectors.tree.select('applications-grafana'));
export const getResultApplicationsGrafanaSelect = () =>
public/locales/en-US/grafana.json
+2 -2
View File
@@ -12070,8 +12070,8 @@
"title": "Select scopes"
},
"tree": {
"collapse": "Collapse",
"expand": "Expand",
"collapse": "Collapse {{title}}",
"expand": "Expand {{title}}",
"headline": {
"noResults": "No results found for your query",
"recommended": "Recommended",