Merge pull request #12388 from backstage/camilal/fix-search-results-subpaths

[Search] fix result location for subpaths

.changeset/ninety-coats-learn.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': patch
+---
+
+Fix relative `sub-paths` by concatenating the app's base path with them.

packages/core-components/src/components/Link/Link.test.tsx

@@ -21,9 +21,11 @@ import {
   TestApiProvider,
   wrapInTestApp,
 } from '@backstage/test-utils';
-import { analyticsApiRef } from '@backstage/core-plugin-api';
-import { isExternalUri, Link } from './Link';
+import { analyticsApiRef, configApiRef } from '@backstage/core-plugin-api';
+import { isExternalUri, Link, useResolvedPath } from './Link';
 import { Route, Routes } from 'react-router';
+import { renderHook, WrapperComponent } from '@testing-library/react-hooks';
+import { ConfigReader } from '@backstage/config';
 
 describe('<Link />', () => {
   it('navigates using react-router', async () => {
@@ -103,6 +105,58 @@ describe('<Link />', () => {
     });
   });
 
+  describe('resolves a sub-path correctly', () => {
+    it('when it starts with base path', async () => {
+      const testString = 'This is test string';
+      const linkText = 'Navigate!';
+      const configApi = new ConfigReader({
+        app: { baseUrl: 'http://localhost:3000/example' },
+      });
+
+      const { getByText } = render(
+        wrapInTestApp(
+          <TestApiProvider apis={[[configApiRef, configApi]]}>
+            <Routes>
+              <Link to="/example/test">{linkText}</Link>
+              <Route path="/example/test" element={<p>{testString}</p>} />
+            </Routes>
+          </TestApiProvider>,
+        ),
+      );
+
+      expect(() => getByText(testString)).toThrow();
+      fireEvent.click(getByText(linkText));
+      await waitFor(() => {
+        expect(getByText(testString)).toBeInTheDocument();
+      });
+    });
+
+    it('when it does not start with base path', async () => {
+      const testString = 'This is test string';
+      const linkText = 'Navigate!';
+      const configApi = new ConfigReader({
+        app: { baseUrl: 'http://localhost:3000/example' },
+      });
+
+      const { getByText } = render(
+        wrapInTestApp(
+          <TestApiProvider apis={[[configApiRef, configApi]]}>
+            <Routes>
+              <Link to="/test">{linkText}</Link>
+              <Route path="/example/test" element={<p>{testString}</p>} />
+            </Routes>
+          </TestApiProvider>,
+        ),
+      );
+
+      expect(() => getByText(testString)).toThrow();
+      fireEvent.click(getByText(linkText));
+      await waitFor(() => {
+        expect(getByText(testString)).toBeInTheDocument();
+      });
+    });
+  });
+
   describe('isExternalUri', () => {
     it.each([
       [true, 'http://'],
@@ -128,4 +182,45 @@ describe('<Link />', () => {
       expect(isExternalUri(uri)).toBe(expected);
     });
   });
+
+  describe('useResolvedPath', () => {
+    const wrapper: WrapperComponent<{}> = ({ children }) => {
+      const configApi = new ConfigReader({
+        app: { baseUrl: 'http://localhost:3000/example' },
+      });
+      return (
+        <TestApiProvider apis={[[configApiRef, configApi]]}>
+          {children}
+        </TestApiProvider>
+      );
+    };
+
+    describe('concatenate base path', () => {
+      it('when uri is internal and does not start with base path', () => {
+        const path = '/catalog/default/component/artist-lookup';
+        const { result } = renderHook(() => useResolvedPath(path), {
+          wrapper,
+        });
+        expect(result.current).toBe('/example'.concat(path));
+      });
+    });
+
+    describe('does not concatenate base path', () => {
+      it('when uri is external', () => {
+        const path = 'https://stackoverflow.com/questions/1/example';
+        const { result } = renderHook(() => useResolvedPath(path), {
+          wrapper,
+        });
+        expect(result.current).toBe(path);
+      });
+
+      it('when uri already starts with base path', () => {
+        const path = '/example/catalog/default/component/artist-lookup';
+        const { result } = renderHook(() => useResolvedPath(path), {
+          wrapper,
+        });
+        expect(result.current).toBe(path);
+      });
+    });
+  });
 });

packages/core-components/src/components/Link/Link.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { useAnalytics } from '@backstage/core-plugin-api';
+import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api';
 import classnames from 'classnames';
 import MaterialLink, {
   LinkProps as MaterialLinkProps,
@@ -25,6 +25,7 @@ import {
   Link as RouterLink,
   LinkProps as RouterLinkProps,
 } from 'react-router-dom';
+import { trimEnd } from 'lodash';
 
 const useStyles = makeStyles(
   {
@@ -52,6 +53,45 @@ export type LinkProps = MaterialLinkProps &
     noTrack?: boolean;
   };
 
+/**
+ * Returns the app base url that could be empty if the Config API is not properly implemented.
+ * The only cases there would be no Config API are in tests and in storybook stories, and in those cases, it's unlikely that callers would rely on this subpath behavior.
+ */
+const useBaseUrl = () => {
+  try {
+    const config = useApi(configApiRef);
+    return config.getOptionalString('app.baseUrl');
+  } catch {
+    return undefined;
+  }
+};
+
+/**
+ * Get the app base path from the configured app baseUrl.
+ * The returned path does not have a trailing slash.
+ */
+const useBasePath = () => {
+  // baseUrl can be specified as just a path
+  const base = 'http://dummy.dev';
+  const url = useBaseUrl() ?? '/';
+  const { pathname } = new URL(url, base);
+  return trimEnd(pathname, '/');
+};
+
+export const useResolvedPath = (uri: LinkProps['to']) => {
+  let resolvedPath = String(uri);
+
+  const basePath = useBasePath();
+  const external = isExternalUri(resolvedPath);
+  const startsWithBasePath = resolvedPath.startsWith(basePath);
+
+  if (!external && !startsWithBasePath) {
+    resolvedPath = basePath.concat(resolvedPath);
+  }
+
+  return resolvedPath;
+};
+
 /**
  * Given a react node, try to retrieve its text content.
  */
@@ -84,7 +124,7 @@ export const Link = React.forwardRef<any, LinkProps>(
   ({ onClick, noTrack, ...props }, ref) => {
     const classes = useStyles();
     const analytics = useAnalytics();
-    const to = String(props.to);
+    const to = useResolvedPath(props.to);
     const linkText = getNodeText(props.children) || to;
     const external = isExternalUri(to);
     const newWindow = external && !!/^https?:/.exec(to);
@@ -99,11 +139,11 @@ export const Link = React.forwardRef<any, LinkProps>(
     return external ? (
       // External links
       <MaterialLink
+        {...(newWindow ? { target: '_blank', rel: 'noopener' } : {})}
+        {...props}
         ref={ref}
         href={to}
         onClick={handleClick}
-        {...(newWindow ? { target: '_blank', rel: 'noopener' } : {})}
-        {...props}
         className={classnames(classes.externalLink, props.className)}
       >
         {props.children}
@@ -112,10 +152,11 @@ export const Link = React.forwardRef<any, LinkProps>(
     ) : (
       // Interact with React Router for internal links
       <MaterialLink
+        {...props}
         ref={ref}
         component={RouterLink}
+        to={to}
         onClick={handleClick}
-        {...props}
       />
     );
   },