From 716491c672a05b22bf6cd4c2fde196744e5c723e Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 10:22:19 +0200 Subject: [PATCH 1/9] fix(catalog): searh result base url Signed-off-by: Camila Belo --- .../CatalogSearchResultListItem.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index a7307055c2..fbb8f7b132 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -25,7 +25,7 @@ import { makeStyles, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; -import { useAnalytics } from '@backstage/core-plugin-api'; +import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api'; import { IndexableDocument, ResultHighlight, @@ -63,15 +63,19 @@ export function CatalogSearchResultListItem( const classes = useStyles(); const analytics = useAnalytics(); + + const configApi = useApi(configApiRef); + const to = configApi.getString('app.baseUrl').concat(result.location); + const handleClick = () => { analytics.captureEvent('discover', result.title, { - attributes: { to: result.location }, + attributes: { to }, value: props.rank, }); }; return ( - + {props.icon && {props.icon}}
From e4236a46524ff1c50e95c69eb9572386547957f9 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 10:23:34 +0200 Subject: [PATCH 2/9] fix(search-react): result base url Signed-off-by: Camila Belo --- .../DefaultResultListItem.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index f4260525d6..f410bb24f4 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -15,7 +15,12 @@ */ import React, { ReactNode } from 'react'; -import { AnalyticsContext, useAnalytics } from '@backstage/core-plugin-api'; +import { + AnalyticsContext, + configApiRef, + useAnalytics, + useApi, +} from '@backstage/core-plugin-api'; import { ResultHighlight, SearchDocument, @@ -58,15 +63,19 @@ export const DefaultResultListItemComponent = ({ lineClamp = 5, }: DefaultResultListItemProps) => { const analytics = useAnalytics(); + const configApi = useApi(configApiRef); + + const to = configApi.getString('app.baseUrl').concat(result.location); + const handleClick = () => { analytics.captureEvent('discover', result.title, { - attributes: { to: result.location }, + attributes: { to }, value: rank, }); }; return ( - + {icon && {icon}} Date: Fri, 1 Jul 2022 10:27:13 +0200 Subject: [PATCH 3/9] fix(techdocs): searh result base url Signed-off-by: Camila Belo --- .../components/TechDocsSearchResultListItem.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx index 430165bf8b..55731e6f2a 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -23,7 +23,7 @@ import { makeStyles, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; -import { useAnalytics } from '@backstage/core-plugin-api'; +import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; @@ -74,9 +74,12 @@ export const TechDocsSearchResultListItem = ( const classes = useStyles(); const analytics = useAnalytics(); + const configApi = useApi(configApiRef); + const handleClick = () => { + const to = configApi.getString('app.baseUrl').concat(result.location); analytics.captureEvent('discover', result.title, { - attributes: { to: result.location }, + attributes: { to }, value: rank, }); }; @@ -149,14 +152,16 @@ export const TechDocsSearchResultListItem = ( ); }; - const LinkWrapper = ({ children }: PropsWithChildren<{}>) => - asLink ? ( - + const LinkWrapper = ({ children }: PropsWithChildren<{}>) => { + const to = configApi.getString('app.baseUrl').concat(result.location); + return asLink ? ( + {children} ) : ( <>{children} ); + }; const ListItemWrapper = ({ children }: PropsWithChildren<{}>) => asListItem ? ( From aa9cddeaa7a429c5af1ca7e6389b23c8dddb2fe5 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 10:37:42 +0200 Subject: [PATCH 4/9] refactor: checks if search results are relatives Signed-off-by: Camila Belo --- .../CatalogSearchResultListItem.tsx | 6 +++++- .../DefaultResultListItem/DefaultResultListItem.tsx | 6 +++++- .../components/TechDocsSearchResultListItem.tsx | 12 ++++++++++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index fbb8f7b132..807ba611d9 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -65,7 +65,11 @@ export function CatalogSearchResultListItem( const analytics = useAnalytics(); const configApi = useApi(configApiRef); - const to = configApi.getString('app.baseUrl').concat(result.location); + let to = result.location; + // Is relative url + if (!to.match(/^([a-z]*:)?\/\//i)) { + to = configApi.getString('app.baseUrl').concat(to); + } const handleClick = () => { analytics.captureEvent('discover', result.title, { diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index f410bb24f4..fabf61c813 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -65,7 +65,11 @@ export const DefaultResultListItemComponent = ({ const analytics = useAnalytics(); const configApi = useApi(configApiRef); - const to = configApi.getString('app.baseUrl').concat(result.location); + let to = result.location; + // Is relative url + if (!to.match(/^([a-z]*:)?\/\//i)) { + to = configApi.getString('app.baseUrl').concat(to); + } const handleClick = () => { analytics.captureEvent('discover', result.title, { diff --git a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx index 55731e6f2a..d0969ee075 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -77,7 +77,11 @@ export const TechDocsSearchResultListItem = ( const configApi = useApi(configApiRef); const handleClick = () => { - const to = configApi.getString('app.baseUrl').concat(result.location); + let to = result.location; + // Is relative url + if (!to.match(/^([a-z]*:)?\/\//i)) { + to = configApi.getString('app.baseUrl').concat(to); + } analytics.captureEvent('discover', result.title, { attributes: { to }, value: rank, @@ -153,7 +157,11 @@ export const TechDocsSearchResultListItem = ( }; const LinkWrapper = ({ children }: PropsWithChildren<{}>) => { - const to = configApi.getString('app.baseUrl').concat(result.location); + let to = result.location; + // Is relative url + if (!to.match(/^([a-z]*:)?\/\//i)) { + to = configApi.getString('app.baseUrl').concat(to); + } return asLink ? ( {children} From ae53c3a11cdfe8a6909b7b5792ec4b200c059ce7 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 11:12:34 +0200 Subject: [PATCH 5/9] refactor: extract use search result location hook Signed-off-by: Camila Belo --- .../CatalogSearchResultListItem.tsx | 14 ++++---- .../DefaultResultListItem.tsx | 15 ++------ .../components/DefaultResultListItem/index.ts | 1 + .../useSearchResultLocation.tsx | 36 +++++++++++++++++++ .../TechDocsSearchResultListItem.tsx | 25 +++++-------- 5 files changed, 55 insertions(+), 36 deletions(-) create mode 100644 plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx diff --git a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx index 807ba611d9..cc9dc0f441 100644 --- a/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx +++ b/plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx @@ -25,12 +25,15 @@ import { makeStyles, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; -import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api'; +import { useAnalytics } from '@backstage/core-plugin-api'; import { IndexableDocument, ResultHighlight, } from '@backstage/plugin-search-common'; -import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; +import { + HighlightedSearchResultText, + useSearchResultLocation, +} from '@backstage/plugin-search-react'; const useStyles = makeStyles({ flexContainer: { @@ -64,12 +67,7 @@ export function CatalogSearchResultListItem( const classes = useStyles(); const analytics = useAnalytics(); - const configApi = useApi(configApiRef); - let to = result.location; - // Is relative url - if (!to.match(/^([a-z]*:)?\/\//i)) { - to = configApi.getString('app.baseUrl').concat(to); - } + const to = useSearchResultLocation(result); const handleClick = () => { analytics.captureEvent('discover', result.title, { diff --git a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx index fabf61c813..2e81f50d2e 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx +++ b/plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx @@ -15,12 +15,7 @@ */ import React, { ReactNode } from 'react'; -import { - AnalyticsContext, - configApiRef, - useAnalytics, - useApi, -} from '@backstage/core-plugin-api'; +import { AnalyticsContext, useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight, SearchDocument, @@ -34,6 +29,7 @@ import { Divider, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; +import { useSearchResultLocation } from './useSearchResultLocation'; /** * Props for {@link DefaultResultListItem} @@ -63,13 +59,8 @@ export const DefaultResultListItemComponent = ({ lineClamp = 5, }: DefaultResultListItemProps) => { const analytics = useAnalytics(); - const configApi = useApi(configApiRef); - let to = result.location; - // Is relative url - if (!to.match(/^([a-z]*:)?\/\//i)) { - to = configApi.getString('app.baseUrl').concat(to); - } + const to = useSearchResultLocation(result); const handleClick = () => { analytics.captureEvent('discover', result.title, { diff --git a/plugins/search-react/src/components/DefaultResultListItem/index.ts b/plugins/search-react/src/components/DefaultResultListItem/index.ts index a555f25c02..0e50f25cec 100644 --- a/plugins/search-react/src/components/DefaultResultListItem/index.ts +++ b/plugins/search-react/src/components/DefaultResultListItem/index.ts @@ -16,3 +16,4 @@ export { DefaultResultListItem } from './DefaultResultListItem'; export type { DefaultResultListItemProps } from './DefaultResultListItem'; +export { useSearchResultLocation } from './useSearchResultLocation'; diff --git a/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx b/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx new file mode 100644 index 0000000000..c80d335354 --- /dev/null +++ b/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx @@ -0,0 +1,36 @@ +/* + * 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 d0969ee075..bd92bde88c 100644 --- a/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx +++ b/plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx @@ -23,9 +23,12 @@ import { makeStyles, } from '@material-ui/core'; import { Link } from '@backstage/core-components'; -import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api'; +import { useAnalytics } from '@backstage/core-plugin-api'; import { ResultHighlight } from '@backstage/plugin-search-common'; -import { HighlightedSearchResultText } from '@backstage/plugin-search-react'; +import { + HighlightedSearchResultText, + useSearchResultLocation, +} from '@backstage/plugin-search-react'; const useStyles = makeStyles({ flexContainer: { @@ -74,14 +77,10 @@ export const TechDocsSearchResultListItem = ( const classes = useStyles(); const analytics = useAnalytics(); - const configApi = useApi(configApiRef); + + const to = useSearchResultLocation(result); const handleClick = () => { - let to = result.location; - // Is relative url - if (!to.match(/^([a-z]*:)?\/\//i)) { - to = configApi.getString('app.baseUrl').concat(to); - } analytics.captureEvent('discover', result.title, { attributes: { to }, value: rank, @@ -156,20 +155,14 @@ export const TechDocsSearchResultListItem = ( ); }; - const LinkWrapper = ({ children }: PropsWithChildren<{}>) => { - let to = result.location; - // Is relative url - if (!to.match(/^([a-z]*:)?\/\//i)) { - to = configApi.getString('app.baseUrl').concat(to); - } - return asLink ? ( + const LinkWrapper = ({ children }: PropsWithChildren<{}>) => + asLink ? ( {children} ) : ( <>{children} ); - }; const ListItemWrapper = ({ children }: PropsWithChildren<{}>) => asListItem ? ( From a6ba513f4c4d8e0eab931076141f1cccb1378fd5 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 13:26:17 +0200 Subject: [PATCH 6/9] test(search-react): cover useSearchResultLocation Signed-off-by: Camila Belo --- .../useSearchResultLocation.test.tsx | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx diff --git a/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx b/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx new file mode 100644 index 0000000000..012f648dac --- /dev/null +++ b/plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx @@ -0,0 +1,60 @@ +/* + * 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 React from 'react'; +import { ConfigReader } from '@backstage/core-app-api'; +import { configApiRef } from '@backstage/core-plugin-api'; +import { TestApiProvider } from '@backstage/test-utils'; +import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; +import { useSearchResultLocation } from './useSearchResultLocation'; + +describe('useSearchResultLocation', () => { + 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); + }); +}); From d1b99b5042040e7b8638f39563b316882772f94a Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 13:41:03 +0200 Subject: [PATCH 7/9] chore: add changeset file Signed-off-by: Camila Belo --- .changeset/wicked-feet-poke.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/wicked-feet-poke.md diff --git a/.changeset/wicked-feet-poke.md b/.changeset/wicked-feet-poke.md new file mode 100644 index 0000000000..533b415cd3 --- /dev/null +++ b/.changeset/wicked-feet-poke.md @@ -0,0 +1,7 @@ +--- +'@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. From 7f5e79961d316ceeef139e055c70b16fc0270430 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 1 Jul 2022 16:20:39 +0200 Subject: [PATCH 8/9] refactor: move fix to the Link component Signed-off-by: Camila Belo --- .changeset/ninety-coats-learn.md | 5 + .changeset/wicked-feet-poke.md | 7 -- packages/core-app-api/api-report.md | 3 + packages/core-app-api/src/app/AppManager.tsx | 3 +- packages/core-app-api/src/app/index.ts | 1 + packages/core-components/package.json | 1 + .../src/components/Link/Link.test.tsx | 99 ++++++++++++++++++- .../src/components/Link/Link.tsx | 26 ++++- .../CatalogSearchResultListItem.tsx | 12 +-- .../DefaultResultListItem.tsx | 8 +- .../components/DefaultResultListItem/index.ts | 1 - .../useSearchResultLocation.test.tsx | 60 ----------- .../useSearchResultLocation.tsx | 36 ------- .../TechDocsSearchResultListItem.tsx | 12 +-- 14 files changed, 138 insertions(+), 136 deletions(-) create mode 100644 .changeset/ninety-coats-learn.md delete mode 100644 .changeset/wicked-feet-poke.md delete mode 100644 plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx delete mode 100644 plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx 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} ) : ( From ac7470591edc05bb49c4cc27387be2bd0ad32a64 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 4 Jul 2022 09:55:23 +0200 Subject: [PATCH 9/9] refactor: apply review suggestions Signed-off-by: Camila Belo --- .changeset/ninety-coats-learn.md | 2 +- packages/core-app-api/api-report.md | 3 -- packages/core-app-api/src/app/AppManager.tsx | 3 +- packages/core-app-api/src/app/index.ts | 1 - packages/core-components/package.json | 1 - .../src/components/Link/Link.tsx | 39 +++++++++++++++---- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/.changeset/ninety-coats-learn.md b/.changeset/ninety-coats-learn.md index 19d56aa128..9d6945e9c0 100644 --- a/.changeset/ninety-coats-learn.md +++ b/.changeset/ninety-coats-learn.md @@ -2,4 +2,4 @@ '@backstage/core-components': patch --- -Fix the relative `sub-paths` by concatenating the app's base URL with them. +Fix relative `sub-paths` by concatenating the app's base path with them. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 6cbff05709..116b33c598 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -372,9 +372,6 @@ 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 25aa606e4e..f47ddf676a 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -95,9 +95,8 @@ const InternalAppContext = createContext<{ * Get the app base path from the configured app baseUrl. * * The returned path does not have a trailing slash. - * @public */ -export function getBasePath(configApi: Config) { +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 4d4ddf7e8b..7843b36339 100644 --- a/packages/core-app-api/src/app/index.ts +++ b/packages/core-app-api/src/app/index.ts @@ -16,5 +16,4 @@ 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 0202904448..51cbc37a1b 100644 --- a/packages/core-components/package.json +++ b/packages/core-components/package.json @@ -34,7 +34,6 @@ }, "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.tsx b/packages/core-components/src/components/Link/Link.tsx index d404755b54..22b203d157 100644 --- a/packages/core-components/src/components/Link/Link.tsx +++ b/packages/core-components/src/components/Link/Link.tsx @@ -25,7 +25,7 @@ import { Link as RouterLink, LinkProps as RouterLinkProps, } from 'react-router-dom'; -import { getBasePath } from '@backstage/core-app-api'; +import { trimEnd } from 'lodash'; const useStyles = makeStyles( { @@ -53,14 +53,39 @@ 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']) => { - const configApi = useApi(configApiRef); - const basePath = getBasePath(configApi); - let resolvedPath = String(uri); - const external = isExternalUri(resolvedPath); - if (!external && !resolvedPath.startsWith(basePath)) { + const basePath = useBasePath(); + const external = isExternalUri(resolvedPath); + const startsWithBasePath = resolvedPath.startsWith(basePath); + + if (!external && !startsWithBasePath) { resolvedPath = basePath.concat(resolvedPath); } @@ -130,8 +155,8 @@ export const Link = React.forwardRef( {...props} ref={ref} component={RouterLink} - onClick={handleClick} to={to} + onClick={handleClick} /> ); },