From 7b3246f163210392d521eaf9163f9d8100f2d981 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Fri, 22 Nov 2024 13:04:58 -0600 Subject: [PATCH 1/6] Fix links to build artifacts --- .../artifacts/components/ArtifactsItem.tsx | 8 +++---- .../metadata/components/EnvBuildStatus.tsx | 9 ++----- src/hooks.ts | 24 +++++++++++++++++++ test/artifacts/ArtifactsList.test.tsx | 9 +++---- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/features/artifacts/components/ArtifactsItem.tsx b/src/features/artifacts/components/ArtifactsItem.tsx index 34166d88..2a12c126 100644 --- a/src/features/artifacts/components/ArtifactsItem.tsx +++ b/src/features/artifacts/components/ArtifactsItem.tsx @@ -5,8 +5,8 @@ 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 +16,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/test/artifacts/ArtifactsList.test.tsx b/test/artifacts/ArtifactsList.test.tsx index 69754409..68330069 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 { prefGlobal } from "../../src/preferences"; const ARTIFACTS = [ { @@ -12,15 +13,15 @@ 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(prefGlobal.apiUrl).toBe("http://localhost:8080/conda-store/"); expect(screen.getByText("Show lockfile")).toHaveAttribute( "href", - "http://localhost:8080/api/v1/build/1/lockfile/" + "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" ); }); }); From e8003626bb736367b2d2292336602583ddfe1b88 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Fri, 22 Nov 2024 13:12:16 -0600 Subject: [PATCH 2/6] remove unused code --- src/utils/helpers/parseArtifactList.ts | 12 ------------ 1 file changed, 12 deletions(-) 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}`; - } -}; From 620eab833e71fb9585a490cbb9078c86124062ff Mon Sep 17 00:00:00 2001 From: gabalafou Date: Fri, 22 Nov 2024 13:47:39 -0600 Subject: [PATCH 3/6] lint --- src/features/artifacts/components/ArtifactsItem.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/features/artifacts/components/ArtifactsItem.tsx b/src/features/artifacts/components/ArtifactsItem.tsx index 2a12c126..d3f50993 100644 --- a/src/features/artifacts/components/ArtifactsItem.tsx +++ b/src/features/artifacts/components/ArtifactsItem.tsx @@ -7,7 +7,6 @@ import { useTheme } from "@mui/material/styles"; import { Artifact } from "../../../common/models"; import { useApiUrl } from "../../../hooks"; - interface IArtifactsProps { /** * @param artifact type with the name and route properties From e067009e3f9a8ef8e39915630f9c3b2c7f5d51b0 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Fri, 22 Nov 2024 14:20:01 -0600 Subject: [PATCH 4/6] add test without trailing slash --- test/artifacts/ArtifactsList.test.tsx | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/artifacts/ArtifactsList.test.tsx b/test/artifacts/ArtifactsList.test.tsx index 68330069..705a280f 100644 --- a/test/artifacts/ArtifactsList.test.tsx +++ b/test/artifacts/ArtifactsList.test.tsx @@ -2,7 +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 { prefGlobal } from "../../src/preferences"; +import { prefGlobal, prefDefault } from "../../src/preferences"; const ARTIFACTS = [ { @@ -24,4 +24,17 @@ describe("", () => { "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" ); }); + + it("should render correct URL if API base url lacks trailing slash", () => { + prefGlobal.set({ + ...prefDefault, + apiUrl: "http://localhost:8080/conda-store" + }); + + render(mockTheme()); + expect(screen.getByText("Show lockfile")).toHaveAttribute( + "href", + "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" + ); + }); }); From 02ecd5527a0f2e1bb09ade4467febabe41c3a47b Mon Sep 17 00:00:00 2001 From: gabalafou Date: Fri, 22 Nov 2024 14:44:13 -0600 Subject: [PATCH 5/6] more comprehensive tests --- test/artifacts/ArtifactsList.test.tsx | 54 +++++++++++++++++---------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/test/artifacts/ArtifactsList.test.tsx b/test/artifacts/ArtifactsList.test.tsx index 705a280f..da6b6b45 100644 --- a/test/artifacts/ArtifactsList.test.tsx +++ b/test/artifacts/ArtifactsList.test.tsx @@ -2,7 +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 { prefGlobal, prefDefault } from "../../src/preferences"; +import { PrefContext, prefDefault } from "../../src/preferences"; const ARTIFACTS = [ { @@ -14,27 +14,43 @@ const ARTIFACTS = [ describe("", () => { it("should render component", () => { render(mockTheme()); - expect(screen.getByText("Logs and Artifacts")).toBeInTheDocument(); expect(screen.getByText("Show lockfile")).toBeVisible(); - - expect(prefGlobal.apiUrl).toBe("http://localhost:8080/conda-store/"); - expect(screen.getByText("Show lockfile")).toHaveAttribute( - "href", - "http://localhost:8080/conda-store/api/v1/build/1/lockfile/" - ); }); - it("should render correct URL if API base url lacks trailing slash", () => { - prefGlobal.set({ - ...prefDefault, - apiUrl: "http://localhost:8080/conda-store" - }); - - render(mockTheme()); - expect(screen.getByText("Show lockfile")).toHaveAttribute( - "href", + 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 + ); + }); + }); + } }); From 6980f9ceb97557ca60d1ea932c9901a66b793006 Mon Sep 17 00:00:00 2001 From: gabalafou Date: Sat, 23 Nov 2024 13:30:48 -0600 Subject: [PATCH 6/6] Update test/artifacts/ArtifactsList.test.tsx --- test/artifacts/ArtifactsList.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/artifacts/ArtifactsList.test.tsx b/test/artifacts/ArtifactsList.test.tsx index da6b6b45..67a4b3b1 100644 --- a/test/artifacts/ArtifactsList.test.tsx +++ b/test/artifacts/ArtifactsList.test.tsx @@ -32,7 +32,7 @@ describe("", () => { ["/", "/api/v1/build/1/lockfile/"], ["", "/api/v1/build/1/lockfile/"] ]) { - describe(`with REACT_APP_API_URL set to ${apiUrl}`, () => { + describe(`with REACT_APP_API_URL set to "${apiUrl}"`, () => { it(`should render expected artifact URL ${expectedArtifactUrl}`, () => { render( mockTheme(