From ee5df165542ca8609e22b90885bba647723febda Mon Sep 17 00:00:00 2001 From: gabalafou Date: Tue, 26 Nov 2024 10:21:42 -0600 Subject: [PATCH] Fix links to build artifacts (#443) --- .../artifacts/components/ArtifactsItem.tsx | 7 +-- .../metadata/components/EnvBuildStatus.tsx | 9 +--- src/hooks.ts | 24 ++++++++++ src/utils/helpers/parseArtifactList.ts | 12 ----- test/artifacts/ArtifactsList.test.tsx | 46 +++++++++++++++---- 5 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/features/artifacts/components/ArtifactsItem.tsx b/src/features/artifacts/components/ArtifactsItem.tsx index 34166d88..d3f50993 100644 --- a/src/features/artifacts/components/ArtifactsItem.tsx +++ b/src/features/artifacts/components/ArtifactsItem.tsx @@ -5,8 +5,7 @@ import OpenInNewIcon from "@mui/icons-material/OpenInNew"; import { useTheme } from "@mui/material/styles"; import { Artifact } from "../../../common/models"; -import { PrefContext } from "../../../preferences"; -import { artifactBaseUrl } from "../../../utils/helpers"; +import { useApiUrl } from "../../../hooks"; interface IArtifactsProps { /** @@ -16,9 +15,7 @@ interface IArtifactsProps { } export const ArtifactItem = ({ artifact }: IArtifactsProps) => { - const pref = React.useContext(PrefContext); - const url = artifactBaseUrl(pref.apiUrl, window.location.origin); - const route = new URL(artifact.route, url).toString(); + const route = useApiUrl(artifact.route); const theme = useTheme(); return ( diff --git a/src/features/metadata/components/EnvBuildStatus.tsx b/src/features/metadata/components/EnvBuildStatus.tsx index e05f2943..7df22e1f 100644 --- a/src/features/metadata/components/EnvBuildStatus.tsx +++ b/src/features/metadata/components/EnvBuildStatus.tsx @@ -3,18 +3,13 @@ import { CircularProgress, Typography } from "@mui/material"; import Link from "@mui/material/Link"; import OpenInNewIcon from "@mui/icons-material/OpenInNew"; import { Artifact, Build } from "../../../common/models"; -import { PrefContext } from "../../../preferences"; import { StyledMetadataItem } from "../../../styles/StyledMetadataItem"; import artifactList from "../../../utils/helpers/artifact"; -import { artifactBaseUrl } from "../../../utils/helpers/parseArtifactList"; import { buildStatus } from "../../../utils/helpers/buildMapper"; +import { useApiUrl } from "../../../hooks"; const LogLink = ({ logArtifact }: { logArtifact: Artifact }) => { - const pref = React.useContext(PrefContext); - const url = new URL( - logArtifact.route, - artifactBaseUrl(pref.apiUrl, window.location.origin) - ); + const url = useApiUrl(logArtifact.route); return ( AppDispatch = useDispatch; export const useAppSelector: TypedUseSelectorHook = useSelector; + +/** + * React Hook to prefix API routes with the base API URL + * + * @param {string} route - A route in the API + * @return {string} + * + * @example + * apiUrl = "/conda-store" + * useApiUrl("api/v1/namespace") + * "/conda-store/api/v1/namespace" + */ +export const useApiUrl = (route: string): string => { + const { apiUrl } = React.useContext(PrefContext); + return ( + // remove slash from end (if there is one) + apiUrl.replace(/\/$/, "") + + "/" + + // remove slash from start (if there is one) + route.replace(/^\//, "") + ); +}; diff --git a/src/utils/helpers/parseArtifactList.ts b/src/utils/helpers/parseArtifactList.ts index cd8aa130..ba0900f5 100644 --- a/src/utils/helpers/parseArtifactList.ts +++ b/src/utils/helpers/parseArtifactList.ts @@ -8,15 +8,3 @@ export const parseArtifacts = (artifact_list: string[] | undefined) => { return artifact_list.includes(artifact); }); }; - -const isPathAbsolute = (path: string) => { - return new RegExp("^(?:[a-z]+:)?//", "i").test(path); -}; - -export const artifactBaseUrl = (apiUrl: string, baseUrl: string) => { - if (isPathAbsolute(apiUrl)) { - return apiUrl; - } else { - return `${baseUrl}${apiUrl}`; - } -}; diff --git a/test/artifacts/ArtifactsList.test.tsx b/test/artifacts/ArtifactsList.test.tsx index 69754409..67a4b3b1 100644 --- a/test/artifacts/ArtifactsList.test.tsx +++ b/test/artifacts/ArtifactsList.test.tsx @@ -2,6 +2,7 @@ import React from "react"; import { render, screen } from "@testing-library/react"; import { ArtifactList } from "../../src/features/artifacts/components"; import { mockTheme } from "../testutils"; +import { PrefContext, prefDefault } from "../../src/preferences"; const ARTIFACTS = [ { @@ -12,15 +13,44 @@ const ARTIFACTS = [ describe("", () => { it("should render component", () => { - render( - mockTheme() - ); - + render(mockTheme()); expect(screen.getByText("Logs and Artifacts")).toBeInTheDocument(); expect(screen.getByText("Show lockfile")).toBeVisible(); - expect(screen.getByText("Show lockfile")).toHaveAttribute( - "href", - "http://localhost:8080/api/v1/build/1/lockfile/" - ); }); + + for (const [apiUrl, expectedArtifactUrl] of [ + [ + "http://localhost:8080/conda-store/", + "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" + ], + [ + "http://localhost:8080/conda-store", + "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" + ], + ["http://localhost:8080", "http://localhost:8080/api/v1/build/1/lockfile/"], + ["/conda-store", "/conda-store/api/v1/build/1/lockfile/"], + ["/", "/api/v1/build/1/lockfile/"], + ["", "/api/v1/build/1/lockfile/"] + ]) { + describe(`with REACT_APP_API_URL set to "${apiUrl}"`, () => { + it(`should render expected artifact URL ${expectedArtifactUrl}`, () => { + render( + mockTheme( + + + + ) + ); + expect(screen.getByText("Show lockfile")).toHaveAttribute( + "href", + expectedArtifactUrl + ); + }); + }); + } });