From 9a98dfc82627eaf90ddd627d4ec4061a3fd36175 Mon Sep 17 00:00:00 2001
From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com>
Date: Wed, 14 May 2025 14:18:24 +0200
Subject: [PATCH] Scopes: Fix highlighting of current active dashboard.
 (#105333)

* Do not match current link based on startsWith

* Add test and take query params into account

* Extract dashboard path handling

* Expand comment
---
 .../ScopesNavigationTreeLink.test.tsx         | 108 ++++++++++++++++++
 .../dashboards/ScopesNavigationTreeLink.tsx   |  16 ++-
 2 files changed, 121 insertions(+), 3 deletions(-)
 create mode 100644 public/app/features/scopes/dashboards/ScopesNavigationTreeLink.test.tsx

diff --git a/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.test.tsx b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.test.tsx
new file mode 100644
index 00000000000..b8775a3c21b
--- /dev/null
+++ b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.test.tsx
@@ -0,0 +1,108 @@
+import { render, screen } from '@testing-library/react';
+import { MemoryRouter, useLocation } from 'react-router-dom-v5-compat';
+
+import { ScopesNavigationTreeLink } from './ScopesNavigationTreeLink';
+
+// Mock react-router-dom's useLocation
+jest.mock('react-router-dom-v5-compat', () => ({
+  ...jest.requireActual('react-router-dom-v5-compat'),
+  useLocation: jest.fn(),
+}));
+
+const renderWithRouter = (ui: React.ReactElement) => {
+  return render(<MemoryRouter>{ui}</MemoryRouter>);
+};
+
+describe('ScopesNavigationTreeLink', () => {
+  const mockUseLocation = useLocation as jest.Mock;
+
+  beforeEach(() => {
+    mockUseLocation.mockReturnValue({ pathname: '/current-path' });
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('renders link with correct props', () => {
+    renderWithRouter(<ScopesNavigationTreeLink to="/test-path" title="Test Link" id="test-id" />);
+
+    const link = screen.getByTestId('scopes-dashboards-test-id');
+    expect(link).toBeInTheDocument();
+    expect(link).toHaveAttribute('href', '/test-path');
+    expect(link).toHaveAttribute('role', 'treeitem');
+    expect(link).toHaveTextContent('Test Link');
+  });
+
+  it('sets aria-current when path matches', () => {
+    mockUseLocation.mockReturnValue({ pathname: '/test-path' });
+
+    renderWithRouter(<ScopesNavigationTreeLink to="/test-path" title="Test Link" id="test-id" />);
+
+    const link = screen.getByTestId('scopes-dashboards-test-id');
+    expect(link).toHaveAttribute('aria-current', 'page');
+  });
+
+  it('does not set aria-current when path does not match', () => {
+    mockUseLocation.mockReturnValue({ pathname: '/different-path' });
+
+    renderWithRouter(<ScopesNavigationTreeLink to="/test-path" title="Test Link" id="test-id" />);
+
+    const link = screen.getByTestId('scopes-dashboards-test-id');
+    expect(link).not.toHaveAttribute('aria-current');
+  });
+
+  it('handles dashboard paths correctly', () => {
+    mockUseLocation.mockReturnValue({ pathname: '/d/dashboard1/some-details' });
+
+    renderWithRouter(<ScopesNavigationTreeLink to="/d/dashboard1" title="Dashboard Link" id="dashboard-id" />);
+
+    const link = screen.getByTestId('scopes-dashboards-dashboard-id');
+    expect(link).toHaveAttribute('aria-current', 'page');
+  });
+
+  it('does not match when path is just the start of another path', () => {
+    mockUseLocation.mockReturnValue({ pathname: '/test-path/extra' });
+
+    renderWithRouter(<ScopesNavigationTreeLink to="/test-path" title="Test Link" id="test-id" />);
+
+    const link = screen.getByTestId('scopes-dashboards-test-id');
+    expect(link).not.toHaveAttribute('aria-current');
+  });
+
+  it('only highlights the matching link when multiple links are present', () => {
+    mockUseLocation.mockReturnValue({ pathname: '/test-path' });
+
+    renderWithRouter(
+      <>
+        <ScopesNavigationTreeLink to="/test-path" title="Matching Link" id="matching-id" />
+        <ScopesNavigationTreeLink to="/test-path-extra" title="Other Link" id="other-id" />
+      </>
+    );
+
+    const matchingLink = screen.getByTestId('scopes-dashboards-matching-id');
+    const otherLink = screen.getByTestId('scopes-dashboards-other-id');
+
+    expect(matchingLink).toHaveAttribute('aria-current', 'page');
+    expect(otherLink).not.toHaveAttribute('aria-current');
+  });
+
+  it('matches path correctly when current location has query parameters', () => {
+    mockUseLocation.mockReturnValue({
+      pathname: '/test-path',
+    });
+
+    renderWithRouter(
+      <>
+        <ScopesNavigationTreeLink to="/test-path?param1=value1&param2=value2" title="Matching Link" id="matching-id" />
+        <ScopesNavigationTreeLink to="/test-path-other" title="Other Link" id="other-id" />
+      </>
+    );
+
+    const matchingLink = screen.getByTestId('scopes-dashboards-matching-id');
+    const otherLink = screen.getByTestId('scopes-dashboards-other-id');
+
+    expect(matchingLink).toHaveAttribute('aria-current', 'page');
+    expect(otherLink).not.toHaveAttribute('aria-current');
+  });
+});
diff --git a/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx
index a1876b22741..589cbe9c743 100644
--- a/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx
+++ b/public/app/features/scopes/dashboards/ScopesNavigationTreeLink.tsx
@@ -11,19 +11,29 @@ export interface ScopesNavigationTreeLinkProps {
   id: string;
 }
 
+// Helper function to get the base path for a dashboard URL for comparison purposes.
+// e.g., /d/dashboardId/slug -> /d/dashboardId
+//       /d/dashboardId      -> /d/dashboardId
+function getDashboardPathForComparison(pathname: string): string {
+  return pathname.split('/').slice(0, 3).join('/');
+}
+
 export function ScopesNavigationTreeLink({ to, title, id }: ScopesNavigationTreeLinkProps) {
   const styles = useStyles2(getStyles);
   const linkIcon = useMemo(() => getLinkIcon(to), [to]);
   const isDashboard = to.startsWith('/d/');
+  const locPathname = useLocation().pathname;
 
-  // For dashboards, the title is appended to the path. We need to diregard this
-  const currentPath = isDashboard ? useLocation().pathname.split('/').slice(0, 3).join('/') : useLocation().pathname;
+  // For dashboards, the title is appended to the path when we navigate to just the dashboard id, hence we need to disregard this
+  const currentPath = isDashboard ? getDashboardPathForComparison(locPathname) : locPathname;
 
-  const isCurrent = to.startsWith(currentPath);
+  // Ignore query params
+  const isCurrent = to.split('?')[0] === currentPath;
 
   return (
     <Link
       to={to}
+      aria-current={isCurrent ? 'page' : undefined}
       className={cx(styles.container, isCurrent && styles.current)}
       data-testid={`scopes-dashboards-${id}`}
       role="treeitem"