diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 3be881f0d5b03..9eddafc7800fe 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -127,9 +127,15 @@ export const templateVersions = (templateId: string) => { }; }; +export const templateVersionVariablesKey = (versionId: string) => [ + "templateVersion", + versionId, + "variables", +]; + export const templateVersionVariables = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "variables"], + queryKey: templateVersionVariablesKey(versionId), queryFn: () => API.getTemplateVersionVariables(versionId), }; }; diff --git a/site/src/pages/TemplateVersionEditorPage/ProvisionerTagsPopover.tsx b/site/src/pages/TemplateVersionEditorPage/ProvisionerTagsPopover.tsx index 9d65021dc6b77..07c5f7df0796d 100644 --- a/site/src/pages/TemplateVersionEditorPage/ProvisionerTagsPopover.tsx +++ b/site/src/pages/TemplateVersionEditorPage/ProvisionerTagsPopover.tsx @@ -6,7 +6,7 @@ import { PopoverTrigger, } from "components/Popover/Popover"; import { ProvisionerTag } from "pages/HealthPage/ProvisionerDaemonsPage"; -import { type FC } from "react"; +import { Fragment, type FC } from "react"; import useTheme from "@mui/system/useTheme"; import { useFormik } from "formik"; import * as Yup from "yup"; @@ -111,18 +111,13 @@ export const ProvisionerTagsPopover: FC = ({ return key !== "owner"; }) .map((k) => ( - <> + {k === "scope" ? ( - + ) : ( - + )} - + ))} diff --git a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditor.tsx b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditor.tsx index 995c4267ca586..96d9f8c0d20b0 100644 --- a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditor.tsx +++ b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditor.tsx @@ -689,7 +689,7 @@ const styles = { padding: "0 16px", fontFamily: MONOSPACE_FONT_FAMILY, - "&:first-child": { + "&:first-of-type": { paddingTop: 16, }, @@ -715,7 +715,7 @@ const styles = { borderLeft: 0, borderRight: 0, - "&:first-child": { + "&:first-of-type": { borderTop: 0, }, diff --git a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx index 67ec4c92b1e21..5a94169f7b2d2 100644 --- a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx +++ b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx @@ -1,14 +1,26 @@ -import { renderWithAuth } from "testHelpers/renderHelpers"; +import { + renderWithAuth, + waitForLoaderToBeRemoved, +} from "testHelpers/renderHelpers"; import TemplateVersionEditorPage from "./TemplateVersionEditorPage"; -import { screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import * as api from "api/api"; import { MockTemplate, MockTemplateVersion, + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, MockWorkspaceBuildLogs, } from "testHelpers/entities"; import { Language } from "./PublishTemplateVersionDialog"; +import { QueryClient } from "react-query"; +import { templateVersionVariablesKey } from "api/queries/templates"; +import { RouterProvider, createMemoryRouter } from "react-router-dom"; +import { RequireAuth } from "contexts/auth/RequireAuth"; +import { server } from "testHelpers/server"; +import { rest } from "msw"; +import { AppProviders } from "App"; // For some reason this component in Jest is throwing a MUI style warning so, // since we don't need it for this test, we can mock it out @@ -208,3 +220,111 @@ test("Patch request is not send when there are no changes", async () => { ); expect(patchTemplateVersion).toBeCalledTimes(0); }); + +describe.each([ + { + testName: "Do not ask when template version has no errors", + initialVariables: undefined, + loadedVariables: undefined, + templateVersion: MockTemplateVersion, + askForVariables: false, + }, + { + testName: + "Do not ask when template version has no errors even when having previously loaded variables", + initialVariables: [ + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, + ], + loadedVariables: undefined, + templateVersion: MockTemplateVersion, + askForVariables: false, + }, + { + testName: "Ask when template version has errors", + initialVariables: undefined, + templateVersion: { + ...MockTemplateVersion, + job: { + ...MockTemplateVersion.job, + error_code: "REQUIRED_TEMPLATE_VARIABLES", + }, + }, + loadedVariables: [ + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, + ], + askForVariables: true, + }, +])( + "Missing template variables", + ({ + testName, + initialVariables, + loadedVariables, + templateVersion, + askForVariables, + }) => { + it(testName, async () => { + jest.resetAllMocks(); + const queryClient = new QueryClient(); + queryClient.setQueryData( + templateVersionVariablesKey(MockTemplateVersion.id), + initialVariables, + ); + + server.use( + rest.get( + "/api/v2/organizations/:org/templates/:template/versions/:version", + (req, res, ctx) => { + return res(ctx.json(templateVersion)); + }, + ), + ); + + if (loadedVariables) { + server.use( + rest.get( + "/api/v2/templateversions/:version/variables", + (req, res, ctx) => { + return res(ctx.json(loadedVariables)); + }, + ), + ); + } + + render( + + , + children: [ + { + element: , + path: "/templates/:template/versions/:version/edit", + }, + ], + }, + ], + { + initialEntries: [ + `/templates/${MockTemplate.name}/versions/${MockTemplateVersion.name}/edit`, + ], + }, + )} + /> + , + ); + await waitForLoaderToBeRemoved(); + + const dialogSelector = /template variables/i; + if (askForVariables) { + await screen.findByText(dialogSelector); + } else { + expect(screen.queryByText(dialogSelector)).not.toBeInTheDocument(); + } + }); + }, +); diff --git a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx index 742a10316d378..143002096ddc7 100644 --- a/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx +++ b/site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx @@ -302,21 +302,23 @@ const useVersionLogs = ( }; const useMissingVariables = (templateVersion: TemplateVersion | undefined) => { - const { data: missingVariables } = useQuery({ + const isRequiringVariables = + templateVersion?.job.error_code === "REQUIRED_TEMPLATE_VARIABLES"; + const { data: variables } = useQuery({ ...templateVersionVariables(templateVersion?.id ?? ""), - enabled: templateVersion?.job.error_code === "REQUIRED_TEMPLATE_VARIABLES", + enabled: isRequiringVariables, }); const [isMissingVariablesDialogOpen, setIsMissingVariablesDialogOpen] = useState(false); useEffect(() => { - if (missingVariables) { + if (isRequiringVariables) { setIsMissingVariablesDialogOpen(true); } - }, [missingVariables]); + }, [isRequiringVariables]); return { - missingVariables, + missingVariables: isRequiringVariables ? variables : undefined, isMissingVariablesDialogOpen, setIsMissingVariablesDialogOpen, }; diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 416f10ef8acdc..61f33d5cc1ac2 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -82,6 +82,12 @@ export const handlers = [ ctx.json([M.MockTemplateVersion2, M.MockTemplateVersion]), ); }), + rest.patch( + "/api/v2/templates/:templateId/versions", + async (req, res, ctx) => { + return res(ctx.status(200), ctx.json({})); + }, + ), rest.patch("/api/v2/templates/:templateId", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockTemplate)); }),