diff --git a/.changeset/ninety-coats-learn.md b/.changeset/ninety-coats-learn.md new file mode 100644 index 0000000000..19d56aa128 --- /dev/null +++ b/.changeset/ninety-coats-learn.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Fix the relative `sub-paths` by concatenating the app's base URL with them. diff --git a/.changeset/wicked-feet-poke.md b/.changeset/wicked-feet-poke.md deleted file mode 100644 index 533b415cd3..0000000000 --- a/.changeset/wicked-feet-poke.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'@backstage/plugin-catalog': patch -'@backstage/plugin-search-react': patch -'@backstage/plugin-techdocs': patch ---- - -Fix the relative search result locations by concatenating the app's base URL into it. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 116b33c598..6cbff05709 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -372,6 +372,9 @@ export type FlatRoutesProps = { children: ReactNode; }; +// @public +export function getBasePath(configApi: Config): string; + // @public export class GithubAuth { // (undocumented) diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index f47ddf676a..25aa606e4e 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -95,8 +95,9 @@ const InternalAppContext = createContext<{ * Get the app base path from the configured app baseUrl. * * The returned path does not have a trailing slash. + * @public */ -function getBasePath(configApi: Config) { +export function getBasePath(configApi: Config) { let { pathname } = new URL( configApi.getOptionalString('app.baseUrl') ?? '/', 'http://dummy.dev', // baseUrl can be specified as just a path diff --git a/packages/core-app-api/src/app/index.ts b/packages/core-app-api/src/app/index.ts index 7843b36339..4d4ddf7e8b 100644 --- a/packages/core-app-api/src/app/index.ts +++ b/packages/core-app-api/src/app/index.ts @@ -16,4 +16,5 @@ export { createSpecializedApp } from './createSpecializedApp'; export { defaultConfigLoader } from './defaultConfigLoader'; +export { getBasePath } from './AppManager'; export * from './types'; diff --git a/packages/core-components/package.json b/packages/core-components/package.json index 51cbc37a1b..0202904448 100644 --- a/packages/core-components/package.json +++ b/packages/core-components/package.json @@ -34,6 +34,7 @@ }, "dependencies": { "@backstage/config": "^1.0.1", + "@backstage/core-app-api": "^1.0.4-next.0", "@backstage/core-plugin-api": "^1.0.3", "@backstage/errors": "^1.1.0-next.0", "@backstage/theme": "^0.2.16-next.0", diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index 8c6ed54d76..5cfeaa4869 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/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('', () => { it('navigates using react-router', async () => { @@ -103,6 +105,58 @@ describe('', () => { }); }); + 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( + + + {linkText} + {testString}

} /> + + , + ), + ); + + 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( + + + {linkText} + {testString}

} /> + + , + ), + ); + + 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('', () => { expect(isExternalUri(uri)).toBe(expected); }); }); + + describe('useResolvedPath', () => { + const wrapper: WrapperComponent<{}> = ({ children }) => { + const configApi = new ConfigReader({ + app: { baseUrl: 'http://localhost:3000/example' }, + }); + return ( + + {children} + + ); + }; + + 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); + }); + }); + }); }); diff --git a/packages/core-components/src/components/Link/Link.tsx b/packages/core-components/src/components/Link/Link.tsx index 5809990b92..d404755b54 100644 --- a/packages/core-components/src/components/Link/Link.tsx +++ b/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 { getBasePath } from '@backstage/core-app-api'; const useStyles = makeStyles( { @@ -52,6 +53,20 @@ export type LinkProps = MaterialLinkProps & noTrack?: boolean; }; +export const useResolvedPath = (uri: LinkProps['to']) => { + const configApi = useApi(configApiRef); + const basePath = getBasePath(configApi); + + let resolvedPath = String(uri); + const external = isExternalUri(resolvedPath); + + if (!external && !resolvedPath.startsWith(basePath)) { + resolvedPath = basePath.concat(resolvedPath); + } + + return resolvedPath; +}; + /** * Given a react node, try to retrieve its text content. */ @@ -84,7 +99,7 @@ export const Link = React.forwardRef( ({ 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 +114,11 @@ export const Link = React.forwardRef( return external ? ( // External links {props.children} @@ -112,10 +127,11 @@ export const Link = React.forwardRef( ) : ( // Interact with React Router for internal links ); }, diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index cc9dc0f441..a7307055c2 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -30,10 +30,7 @@ import { IndexableDocument, ResultHighlight, } from '@backstage/plugin-search-common'; -import { - HighlightedSearchResultText, - useSearchResultLocation, -} from '@backstage/plugin-search-react'; +import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; const useStyles = makeStyles({ flexContainer: { @@ -66,18 +63,15 @@ export function CatalogSearchResultListItem( const classes = useStyles(); const analytics = useAnalytics(); - - const to = useSearchResultLocation(result); - const handleClick = () => { analytics.captureEvent('discover', result.title, { - attributes: { to }, + attributes: { to: result.location }, value: props.rank, }); }; return ( - + {props.icon && {props.icon}}
diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index 2e81f50d2e..f4260525d6 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -29,7 +29,6 @@ import { Divider, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; -import { useSearchResultLocation } from './useSearchResultLocation'; /** * Props for {@link DefaultResultListItem} @@ -59,18 +58,15 @@ export const DefaultResultListItemComponent = ({ lineClamp = 5, }: DefaultResultListItemProps) => { const analytics = useAnalytics(); - - const to = useSearchResultLocation(result); - const handleClick = () => { analytics.captureEvent('discover', result.title, { - attributes: { to }, + attributes: { to: result.location }, value: rank, }); }; return ( - + {icon && {icon}} { - const baseUrl = 'http://localhost:3000/example'; - const configApiMock = new ConfigReader({ app: { baseUrl } }); - const wrapper: WrapperComponent<{}> = ({ children }) => ( - - {children} - - ); - - it('should concatenate base url into relative urls', () => { - const document = { - title: 'artist-lookup', - text: 'Artist Lookup', - location: '/catalog/default/component/artist-lookup', - }; - - const { result } = renderHook(() => useSearchResultLocation(document), { - wrapper, - }); - - expect(result.current).toBe(baseUrl.concat(document.location)); - }); - - it('should not concatenate base url into absolute urls', () => { - const document = { - title: 'What is Backstage?', - text: 'An open platform for building developer portals ', - location: 'https://stackoverflow.com/questions/1/what-is-backstage', - }; - - const { result } = renderHook(() => useSearchResultLocation(document), { - wrapper, - }); - - expect(result.current).toBe(document.location); - }); -}); diff --git a/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx b/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx deleted file mode 100644 index c80d335354..0000000000 --- a/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2022 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { configApiRef, useApi } from '@backstage/core-plugin-api'; -import { SearchDocument } from '@backstage/plugin-search-common'; - -/** - * Builds a search result url. - * @param document - A search result item. - * @returns Add app base url as location prefix when location is relative. - */ -export const useSearchResultLocation = (document: SearchDocument) => { - const configApi = useApi(configApiRef); - - let location = document.location; - - const isRelative = !location.match(/^([a-z]*:)?\/\//i); - if (isRelative) { - location = configApi.getString('app.baseUrl').concat(location); - } - - return location; -}; diff --git a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx index bd92bde88c..430165bf8b 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -25,10 +25,7 @@ import { import { Link } from '@backstage/core-components'; import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; -import { - HighlightedSearchResultText, - useSearchResultLocation, -} from '@backstage/plugin-search-react'; +import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; const useStyles = makeStyles({ flexContainer: { @@ -77,12 +74,9 @@ export const TechDocsSearchResultListItem = ( const classes = useStyles(); const analytics = useAnalytics(); - - const to = useSearchResultLocation(result); - const handleClick = () => { analytics.captureEvent('discover', result.title, { - attributes: { to }, + attributes: { to: result.location }, value: rank, }); }; @@ -157,7 +151,7 @@ export const TechDocsSearchResultListItem = ( const LinkWrapper = ({ children }: PropsWithChildren<{}>) => asLink ? ( - + {children} ) : (