Scopes: Selector search highlighting (#111295)

* Add scopes search highlight with wildcard support * Change so query is not being applied to parent * Update e2e tests with more stable selectors * Fix initial load query * Use the Highlighter library instead * Remove unused sanitization * Remove undefined export
2025-09-19 15:51:24 +02:00
parent 460f776e6f
commit 00c9145929
5 changed files with 219 additions and 35 deletions
@@ -169,12 +169,15 @@ test.describe(
        await searchScopes(page, scopeSearchOne, [secondLevelScopes[0]]);

        await expect.soft(scopeTreeCheckboxes).toHaveCount(1);
-        expect.soft(await scopeTreeCheckboxes.first().locator('../..').textContent()).toBe(scopeSearchOne);
+
+        // Check grandparent title, to make sure we get what we want
+        expect.soft(await scopeTreeCheckboxes.first().locator('../../..').textContent()).toBe(scopeSearchOne);

        await searchScopes(page, scopeSearchTwo, [secondLevelScopes[1]]);

        await expect.soft(scopeTreeCheckboxes).toHaveCount(1);
-        expect.soft(await scopeTreeCheckboxes.first().locator('../..').textContent()).toBe(scopeSearchTwo);
+        // Check grandparent title, to make sure we get what we want
+        expect.soft(await scopeTreeCheckboxes.first().locator('../../..').textContent()).toBe(scopeSearchTwo);
      });
    });
  }
@@ -262,13 +262,15 @@ export async function getScopeLeafName(page: Page, nth: number): Promise<string>
 }

 export async function getScopeLeafTitle(page: Page, nth: number): Promise<string> {
-  const locator = page.getByTestId(/^scopes-tree-.*-(checkbox|radio)/).nth(nth);
-  const scopeTitle = await locator.locator('../..').textContent();
-
+  const leafLocator = page.getByTestId(/^scopes-tree-.*-(checkbox|radio)/).nth(nth);
+  // Find the closest ancestor element that has the main tree item test id
+  const titleLocator = leafLocator.locator(
+    'xpath=ancestor::*[@data-testid][starts-with(@data-testid, "scopes-tree-") and not(contains(@data-testid, "-checkbox")) and not(contains(@data-testid, "-radio")) and not(contains(@data-testid, "-expand"))]'
+  );
+  const scopeTitle = await titleLocator.textContent();
  if (!scopeTitle) {
    throw new Error('There are no scopes in the selector');
  }
-
  return scopeTitle;
 }

@@ -98,6 +98,7 @@ describe('ScopesSelectorService', () => {
    });

    it('should update node query and fetch children when query changes', async () => {
+      await service.updateNode('', true, ''); // Expand first
      await service.updateNode('', true, 'new-query');
      expect(service.state.tree).toMatchObject({
        children: {},
@@ -116,6 +117,122 @@ describe('ScopesSelectorService', () => {
      // Only the first expansion should trigger fetchNodes
      expect(apiClient.fetchNodes).toHaveBeenCalledTimes(1);
    });
+
+    it('should clear query on first expansion but keep it when filtering within populated node', async () => {
+      const mockChildNode: ScopeNode = {
+        metadata: { name: 'child-node' },
+        spec: { linkId: 'child-scope', linkType: 'scope', parentName: '', nodeType: 'leaf', title: 'child-node' },
+      };
+
+      apiClient.fetchNodes.mockResolvedValue([mockChildNode]);
+
+      // Scenario 1: First expansion (no children yet) - clear query for unfiltered view
+      await service.updateNode('', true, 'search-query');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined });
+
+      // Parent query should be cleared and child nodes should have no query (first expansion)
+      expect(service.state.tree?.query).toBe('');
+      let childTreeNode = service.state.tree?.children?.['child-node'];
+      expect(childTreeNode?.query).toBe('');
+
+      // Scenario 2: Filtering within node that already has children
+      await service.updateNode('', true, 'new-search');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: 'new-search' });
+
+      // Parent and child nodes should have the filter query (filtering within existing children)
+      expect(service.state.tree?.query).toBe('new-search');
+      childTreeNode = service.state.tree?.children?.['child-node'];
+      expect(childTreeNode?.query).toBe('new-search');
+
+      expect(apiClient.fetchNodes).toHaveBeenCalledTimes(2);
+    });
+
+    it('should always reset query on any expansion', async () => {
+      const mockChildNode: ScopeNode = {
+        metadata: { name: 'child-node' },
+        spec: { linkId: 'child-scope', linkType: 'scope', parentName: '', nodeType: 'leaf', title: 'child-node' },
+      };
+
+      apiClient.fetchNodes.mockResolvedValue([mockChildNode]);
+
+      // First expansion with any query should reset parent query and not pass query to API
+      await service.updateNode('', true, 'some-search-query');
+
+      // Verify query is reset and API called without query for first expansion
+      expect(service.state.tree?.query).toBe('');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined });
+      expect(service.state.tree?.children?.['child-node']?.query).toBe('');
+    });
+
+    it('should handle query reset correctly for nested levels beyond root', async () => {
+      // Set up mock nodes for multi-level hierarchy
+      const mockParentNode: ScopeNode = {
+        metadata: { name: 'parent-container' },
+        spec: { linkId: '', linkType: 'scope', parentName: '', nodeType: 'container', title: 'Parent Container' },
+      };
+
+      const mockChildNode: ScopeNode = {
+        metadata: { name: 'child-container' },
+        spec: {
+          linkId: '',
+          linkType: 'scope',
+          parentName: 'parent-container',
+          nodeType: 'container',
+          title: 'Child Container',
+        },
+      };
+
+      const mockGrandchildNode: ScopeNode = {
+        metadata: { name: 'grandchild-leaf' },
+        spec: {
+          linkId: 'leaf-scope',
+          linkType: 'scope',
+          parentName: 'child-container',
+          nodeType: 'leaf',
+          title: 'Grandchild Leaf',
+        },
+      };
+
+      // Mock different responses for different parent nodes
+      apiClient.fetchNodes.mockImplementation((options: { parent?: string; query?: string; limit?: number }) => {
+        if (options.parent === '') {
+          return Promise.resolve([mockParentNode]);
+        } else if (options.parent === 'parent-container') {
+          return Promise.resolve([mockChildNode]);
+        } else if (options.parent === 'child-container') {
+          return Promise.resolve([mockGrandchildNode]);
+        }
+        return Promise.resolve([]);
+      });
+
+      // Step 1: Expand root node with search query
+      await service.updateNode('', true, 'search-query');
+
+      // Root should have query reset, API called without query
+      expect(service.state.tree?.query).toBe('');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: '', query: undefined });
+      expect(service.state.tree?.children?.['parent-container']?.query).toBe('');
+
+      // Step 2: Expand first-level child with search query
+      await service.updateNode('parent-container', true, 'open-search-query');
+
+      // First-level child should have query reset, API called without query
+      const parentContainer = service.state.tree?.children?.['parent-container'];
+      expect(parentContainer?.query).toBe('');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: 'parent-container', query: undefined });
+      expect(parentContainer?.children?.['child-container']?.query).toBe('');
+
+      // Step 3: Now filter within the first-level child (second call to same node)
+      await service.updateNode('parent-container', true, 'filter-search');
+
+      // Now both parent and children should show the filter query since we're filtering within existing children
+      const newParentContainer = service.state.tree?.children?.['parent-container'];
+      expect(newParentContainer?.query).toBe('filter-search');
+      expect(apiClient.fetchNodes).toHaveBeenCalledWith({ parent: 'parent-container', query: 'filter-search' });
+      expect(newParentContainer?.children?.['child-container']?.query).toBe('filter-search');
+
+      expect(apiClient.fetchNodes).toHaveBeenCalledTimes(3);
+    });
  });

  describe('selectScope and deselectScope', () => {
@@ -98,21 +98,28 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi

    const nodeToExpand = treeNodeAtPath(this.state.tree!, path);

-    if (nodeToExpand) {
-      if (nodeToExpand.scopeNodeId === '' || isNodeExpandable(this.state.nodes[nodeToExpand.scopeNodeId])) {
-        if (!nodeToExpand.expanded || nodeToExpand.query !== query) {
-          const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => {
-            treeNode.expanded = true;
-            treeNode.query = query || '';
-          });
-          this.updateState({ tree: newTree });
-          await this.loadNodeChildren(path, nodeToExpand, query);
-        }
-      } else {
-        throw new Error(`Trying to expand node at id ${scopeNodeId} that is not expandable`);
-      }
-    } else {
-      throw new Error(`Trying to expand node at id ${scopeNodeId} not found`);
+    if (!nodeToExpand) {
+      throw new Error(`Node ${scopeNodeId} not found in tree`);
+    }
+
+    if (nodeToExpand.scopeNodeId !== '' && !isNodeExpandable(this.state.nodes[nodeToExpand.scopeNodeId])) {
+      throw new Error(`Trying to expand node at id ${scopeNodeId} that is not expandable`);
+    }
+
+    // Check if this is first expansion or filtering within existing children
+    const haveChildrenLoaded = nodeToExpand.children && Object.keys(nodeToExpand.children).length > 0;
+
+    if (!nodeToExpand.expanded || nodeToExpand.query !== query || !haveChildrenLoaded) {
+      const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => {
+        treeNode.expanded = true;
+        // Reset query on first expansion, keep it only when filtering within existing children
+        treeNode.query = '';
+      });
+      this.updateState({ tree: newTree });
+
+      // For API call: only pass query if filtering within existing children
+      const queryForAPI = haveChildrenLoaded ? query : query === '' ? '' : undefined;
+      await this.loadNodeChildren(path, nodeToExpand, queryForAPI, haveChildrenLoaded);
    }
  };

@@ -131,10 +138,9 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
    }
  };

-  private loadNodeChildren = async (path: string[], treeNode: TreeNode, query?: string) => {
+  private loadNodeChildren = async (path: string[], treeNode: TreeNode, query?: string, haveChildrenLoaded = false) => {
    this.updateState({ loadingNodeName: treeNode.scopeNodeId });

-    // We are expanding node that wasn't yet expanded so we don't have any query to filter by yet.
    const childNodes = await this.apiClient.fetchNodes({ parent: treeNode.scopeNodeId, query });

    const newNodes = { ...this.state.nodes };
@@ -144,12 +150,15 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
    }

    const newTree = modifyTreeNodeAtPath(this.state.tree!, path, (treeNode) => {
+      // Set parent query only when filtering within existing children
+      treeNode.query = haveChildrenLoaded ? query || '' : '';
      treeNode.children = {};
      for (const node of childNodes) {
        treeNode.children[node.metadata.name] = {
          expanded: false,
          scopeNodeId: node.metadata.name,
-          query: '',
+          // Only set query on tree nodes if parent already has children (filtering vs first expansion). This is used for saerch highlighting.
+          query: haveChildrenLoaded ? query || '' : '',
          children: undefined,
        };
      }
@@ -233,6 +242,7 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
    this.updateState({ selectedScopes: newSelectedScopes });
  };

+  // TODO: We should split this into two functions: expandNode and filterNode.
  public updateNode = async (scopeNodeId: string, expanded: boolean, query: string) => {
    if (expanded) {
      return this.expandOrFilterNode(scopeNodeId, query);
@@ -1,4 +1,5 @@
 import { css, cx } from '@emotion/css';
+import Highlighter from 'react-highlight-words';

 import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
@@ -51,6 +52,12 @@ export function ScopesTreeItem({
  const isSelectable = isNodeSelectable(scopeNode);
  const isExpandable = isNodeExpandable(scopeNode);

+  // Create search words for highlighting if there's a query
+  // Only highlight if we have a query AND this node is not expanded (not a parent showing children)
+  const titleText = scopeNode.spec.title;
+  const shouldHighlight = treeNode.query && !treeNode.expanded;
+  const searchWords = shouldHighlight ? getSearchWordsFromQuery(treeNode.query) : [];
+
  return (
    <div
      key={treeNode.scopeNodeId}
@@ -67,6 +74,7 @@ export function ScopesTreeItem({
          isSelectable && !treeNode.expanded && styles.titlePadding,
          highlighted && styles.highlighted
        )}
+        data-testid={`scopes-tree-${treeNode.scopeNodeId}`}
      >
        {isSelectable && !treeNode.expanded ? (
          disableMultiSelect ? (
@@ -74,22 +82,41 @@ export function ScopesTreeItem({
              id={treeNode.scopeNodeId}
              name={treeNode.scopeNodeId}
              checked={selected}
-              label={isExpandable ? '' : scopeNode.spec.title}
+              label={
+                isExpandable ? (
+                  ''
+                ) : shouldHighlight ? (
+                  <Highlighter textToHighlight={titleText} searchWords={searchWords} autoEscape />
+                ) : (
+                  titleText
+                )
+              }
              data-testid={`scopes-tree-${treeNode.scopeNodeId}-radio`}
              onClick={() => {
                selected ? deselectScope(treeNode.scopeNodeId) : selectScope(treeNode.scopeNodeId);
              }}
            />
          ) : (
-            <Checkbox
-              id={treeNode.scopeNodeId}
-              checked={selected}
-              data-testid={`scopes-tree-${treeNode.scopeNodeId}-checkbox`}
-              label={isExpandable ? '' : scopeNode.spec.title}
-              onChange={() => {
-                selected ? deselectScope(treeNode.scopeNodeId) : selectScope(treeNode.scopeNodeId);
-              }}
-            />
+            <div className={styles.checkboxWithLabel}>
+              <Checkbox
+                id={treeNode.scopeNodeId}
+                checked={selected}
+                data-testid={`scopes-tree-${treeNode.scopeNodeId}-checkbox`}
+                label=""
+                onChange={() => {
+                  selected ? deselectScope(treeNode.scopeNodeId) : selectScope(treeNode.scopeNodeId);
+                }}
+              />
+              {!isExpandable && (
+                <label htmlFor={treeNode.scopeNodeId} className={styles.checkboxLabel}>
+                  {shouldHighlight ? (
+                    <Highlighter textToHighlight={titleText} searchWords={searchWords} autoEscape />
+                  ) : (
+                    titleText
+                  )}
+                </label>
+              )}
+            </div>
          )
        ) : null}

@@ -104,7 +131,11 @@ export function ScopesTreeItem({
          >
            <Icon name={!treeNode.expanded ? 'angle-right' : 'angle-down'} />

-            {scopeNode.spec.title}
+            {shouldHighlight ? (
+              <Highlighter textToHighlight={titleText} searchWords={searchWords} autoEscape />
+            ) : (
+              titleText
+            )}
          </button>
        )}
      </div>
@@ -126,6 +157,15 @@ export function ScopesTreeItem({
  );
 }

+// Convert a query string with wildcards into search words for react-highlight-words
+function getSearchWordsFromQuery(query: string): string[] {
+  if (!query) {
+    return [];
+  }
+  // Split query string on wildcard and filter out empty parts
+  return query.split('*').filter((part) => part.length > 0);
+}
+
 export const getTreeItemElementId = (scopeNodeId?: string) => {
  return scopeNodeId ? `scopes-tree-item-${scopeNodeId}` : undefined;
 };
@@ -159,6 +199,18 @@ const getStyles = (theme: GrafanaTheme2) => {
      // Fix for checkboxes and radios outline overflow due to scrollbars
      paddingLeft: theme.spacing(0.5),
    }),
+    checkboxWithLabel: css({
+      alignItems: 'center',
+      display: 'flex',
+      gap: theme.spacing(1),
+    }),
+    checkboxLabel: css({
+      fontSize: theme.typography.pxToRem(14),
+      lineHeight: theme.typography.pxToRem(22),
+      fontWeight: theme.typography.fontWeightRegular,
+      cursor: 'pointer',
+      margin: 0,
+    }),
    expand: css({
      alignItems: 'center',
      background: 'none',