Alerting: Fix source and rule name decoding on Find route (#56805)

public/app/features/alerting/unified/RedirectToRuleViewer.test.tsx

@@ -1,10 +1,10 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
 import { Provider } from 'react-redux';
-import { Router } from 'react-router-dom';
+import { MemoryRouter } from 'react-router-dom';
+import { useLocation } from 'react-use';
 
 import { DataSourceJsonData, PluginMeta } from '@grafana/data';
-import { locationService } from '@grafana/runtime';
 import { configureStore } from 'app/store/configureStore';
 
 import { CombinedRule, Rule } from '../../../types/unified-alerting';
@@ -12,6 +12,7 @@ import { PromRuleType } from '../../../types/unified-alerting-dto';
 
 import { RedirectToRuleViewer } from './RedirectToRuleViewer';
 import { useCombinedRulesMatching } from './hooks/useCombinedRule';
+import * as combinedRuleHooks from './hooks/useCombinedRule';
 import { getRulesSourceByName } from './utils/datasource';
 
 jest.mock('./hooks/useCombinedRule');
@@ -21,13 +22,17 @@ jest.mock('react-router-dom', () => ({
   Redirect: jest.fn(({}) => `Redirected`),
 }));
 
+jest.mock('react-use');
+
 const store = configureStore();
-const renderRedirectToRuleViewer = () => {
+const renderRedirectToRuleViewer = (pathname: string) => {
+  jest.mocked(useLocation).mockReturnValue({ pathname, trigger: '' });
+
   return render(
     <Provider store={store}>
-      <Router history={locationService.getHistory()}>
-        <RedirectToRuleViewer {...mockRoute('prom alert', 'test prom')} />
-      </Router>
+      <MemoryRouter initialEntries={[pathname]}>
+        <RedirectToRuleViewer />
+      </MemoryRouter>
     </Provider>
   );
 };
@@ -55,7 +60,7 @@ describe('Redirect to Rule viewer', () => {
       error: undefined,
     });
     mockRuleSourceByName();
-    renderRedirectToRuleViewer();
+    renderRedirectToRuleViewer('/alerting/test prom/prom alert/find');
     expect(screen.getAllByText('Cloud test alert')).toHaveLength(2);
   });
 
@@ -68,7 +73,41 @@ describe('Redirect to Rule viewer', () => {
       error: undefined,
     });
     mockRuleSourceByName();
-    renderRedirectToRuleViewer();
+    renderRedirectToRuleViewer('/alerting/test prom/prom alert/find');
+    expect(screen.getByText('Redirected')).toBeInTheDocument();
+  });
+
+  it('should properly decode rule name', () => {
+    const rulesMatchingSpy = jest.spyOn(combinedRuleHooks, 'useCombinedRulesMatching').mockReturnValue({
+      result: [mockedRules[0]],
+      loading: false,
+      dispatched: true,
+      requestId: 'A',
+      error: undefined,
+    });
+
+    const ruleName = 'cloud rule++ !@#$%^&*()-/?';
+
+    renderRedirectToRuleViewer(`/alerting/prom-db/${encodeURIComponent(ruleName)}/find`);
+
+    expect(rulesMatchingSpy).toHaveBeenCalledWith(ruleName, 'prom-db');
+    expect(screen.getByText('Redirected')).toBeInTheDocument();
+  });
+
+  it('should properly decode source name', () => {
+    const rulesMatchingSpy = jest.spyOn(combinedRuleHooks, 'useCombinedRulesMatching').mockReturnValue({
+      result: [mockedRules[0]],
+      loading: false,
+      dispatched: true,
+      requestId: 'A',
+      error: undefined,
+    });
+
+    const sourceName = 'prom<|>++ !@#$%^&*()-/?';
+
+    renderRedirectToRuleViewer(`/alerting/${encodeURIComponent(sourceName)}/prom alert/find`);
+
+    expect(rulesMatchingSpy).toHaveBeenCalledWith('prom alert', sourceName);
     expect(screen.getByText('Redirected')).toBeInTheDocument();
   });
 });
@@ -135,17 +174,3 @@ const mockedRules: CombinedRule[] = [
     },
   },
 ];
-
-const mockRoute = (ruleName: string, sourceName: string) => {
-  return {
-    route: {
-      path: '/',
-      component: RedirectToRuleViewer,
-    },
-    queryParams: { returnTo: '/alerting/list' },
-    match: { params: { name: ruleName, sourceName: sourceName }, isExact: false, url: 'asdf', path: '' },
-    history: locationService.getHistory(),
-    location: { pathname: '', hash: '', search: '', state: '' },
-    staticContext: {},
-  };
-};

public/app/features/alerting/unified/RedirectToRuleViewer.tsx

@@ -1,10 +1,10 @@
 import { css } from '@emotion/css';
 import React from 'react';
 import { Redirect } from 'react-router-dom';
+import { useLocation } from 'react-use';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { Alert, Card, Icon, LoadingPlaceholder, useStyles2, withErrorBoundary } from '@grafana/ui';
-import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
 
 import { AlertLabels } from './components/AlertLabels';
 import { RuleViewerLayout } from './components/rule-viewer/RuleViewerLayout';
@@ -12,12 +12,28 @@ import { useCombinedRulesMatching } from './hooks/useCombinedRule';
 import { getRulesSourceByName } from './utils/datasource';
 import { createViewLink } from './utils/misc';
 
-type RedirectToRuleViewerProps = GrafanaRouteComponentProps<{ name?: string; sourceName?: string }>;
 const pageTitle = 'Find rule';
 
-export function RedirectToRuleViewer(props: RedirectToRuleViewerProps): JSX.Element | null {
-  const { name, sourceName } = props.match.params;
+function useRuleFindParams() {
+  // DO NOT USE REACT-ROUTER HOOKS FOR THIS CODE
+  // React-router's useLocation/useParams/props.match are broken and don't preserve original param values when parsing location
+  // so, they cannot be used to parse name and sourceName path params
+  // React-router messes the pathname up resulting in a string that is neither encoded nor decoded
+  // Relevant issue: https://github.com/remix-run/history/issues/505#issuecomment-453175833
+  // It was probably fixed in React-Router v6
+  const location = useLocation();
+  const segments = location.pathname?.split('/') ?? []; // ["", "alerting", "{sourceName}", "{name}]
+
+  const name = decodeURIComponent(segments[3]);
+  const sourceName = decodeURIComponent(segments[2]);
+
+  return { name, sourceName };
+}
+
+export function RedirectToRuleViewer(): JSX.Element | null {
   const styles = useStyles2(getStyles);
+
+  const { name, sourceName } = useRuleFindParams();
   const { error, loading, result: rules, dispatched } = useCombinedRulesMatching(name, sourceName);
 
   if (error) {