From fa39cc691a8d9a7eb0ed7bf7b4e5e183d5f5007a Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Mon, 18 Sep 2023 23:02:50 +0000 Subject: [PATCH 1/5] refactor: get rid of `templateVariablesXService` --- site/src/api/errors.ts | 18 +- site/src/api/queries/templates.ts | 43 ++- .../TemplateVariablesPage.tsx | 119 ++++++--- .../TemplateVariablesPageView.stories.tsx | 33 ++- .../TemplateVariablesPageView.tsx | 45 ++-- .../template/templateVariablesXService.ts | 244 ------------------ 6 files changed, 188 insertions(+), 314 deletions(-) delete mode 100644 site/src/xServices/template/templateVariablesXService.ts diff --git a/site/src/api/errors.ts b/site/src/api/errors.ts index f8d9421631ca1..54721ef2280b1 100644 --- a/site/src/api/errors.ts +++ b/site/src/api/errors.ts @@ -61,12 +61,18 @@ export const mapApiErrorToFieldErrors = ( export const getErrorMessage = ( error: unknown, defaultMessage: string, -): string => - isApiError(error) - ? error.response.data.message - : error instanceof Error - ? error.message - : defaultMessage; +): string => { + if (isApiError(error)) { + return error.response.data.message; + } + if (error instanceof Error) { + return error.message; + } + if (typeof error === "string") { + return error; + } + return defaultMessage; +}; /** * diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 85d33b7921baa..deef101b834ab 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -1,6 +1,10 @@ import * as API from "api/api"; -import { type Template, type AuthorizationResponse } from "api/typesGenerated"; -import { type QueryOptions } from "@tanstack/react-query"; +import { + type Template, + type AuthorizationResponse, + type CreateTemplateVersionRequest, +} from "api/typesGenerated"; +import { type QueryClient, type QueryOptions } from "@tanstack/react-query"; export const templateByNameKey = (orgId: string, name: string) => [ orgId, @@ -63,3 +67,38 @@ export const templateVersions = (templateId: string) => { queryFn: () => API.getTemplateVersions(templateId), }; }; + +export const templateVersionVariables = (versionId: string) => { + return { + queryKey: ["templateVersionVariables", versionId], + queryFn: () => API.getTemplateVersionVariables(versionId), + }; +}; + +export const createTemplateVersion = ( + orgId: string, + queryClient: QueryClient, +) => { + return { + mutationFn: (request: CreateTemplateVersionRequest) => + API.createTemplateVersion(orgId, request), + }; +}; + +export const updateActiveTemplateVersion = ( + template: Template, + queryClient: QueryClient, +) => { + return { + mutationFn: (versionId: string) => + API.updateActiveTemplateVersion(template.id, { + id: versionId, + }), + onSuccess: async () => { + // invalidated because of `active_version_id` + await queryClient.invalidateQueries( + templateByNameKey(template.organization_id, template.name), + ); + }, + }; +}; diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx index d7b61022d6275..12f4a7a6cb013 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx @@ -6,40 +6,103 @@ import { } from "api/typesGenerated"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { useOrganizationId } from "hooks/useOrganizationId"; -import { FC } from "react"; +import { useEffect, type FC, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useNavigate, useParams } from "react-router-dom"; -import { templateVariablesMachine } from "xServices/template/templateVariablesXService"; -import { pageTitle } from "../../../utils/page"; +import { pageTitle } from "utils/page"; import { useTemplateSettings } from "../TemplateSettingsLayout"; import { TemplateVariablesPageView } from "./TemplateVariablesPageView"; +import { + createTemplateVersion, + templateVersion, + templateVersionVariables, + updateActiveTemplateVersion, +} from "api/queries/templates"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { Loader } from "components/Loader/Loader"; export const TemplateVariablesPage: FC = () => { const { template: templateName } = useParams() as { organization: string; template: string; }; - const organizationId = useOrganizationId(); + const orgId = useOrganizationId(); const { template } = useTemplateSettings(); const navigate = useNavigate(); - const [state, send] = useMachine(templateVariablesMachine, { - context: { - organizationId, - template, - }, - actions: { - onUpdateTemplate: () => { - displaySuccess("Template updated successfully"); + const queryClient = useQueryClient(); + const versionId = template.active_version_id; + + const [newVersionId, setNewVersionId] = useState(null); + + const { + data: version, + error: versionError, + isLoading: isVersionLoading, + } = useQuery({ ...templateVersion(versionId), keepPreviousData: true }); + const { + data: variables, + error: variablesError, + isLoading: isVariablesLoading, + } = useQuery({ + ...templateVersionVariables(versionId), + keepPreviousData: true, + }); + + const createVersionMutation = createTemplateVersion(orgId, queryClient); + const { mutate: sendCreateTemplateVersion, error: createVersionError } = + useMutation({ + ...createVersionMutation, + onSuccess: (data) => { + setNewVersionId(data.id); }, + }); + const publishMutation = updateActiveTemplateVersion(template, queryClient); + const { mutate: publishVersion, error: publishError } = useMutation({ + ...publishMutation, + onSuccess: () => { + publishMutation.onSuccess(); + displaySuccess("Template updated successfully"); + }, + onSettled: () => { + setNewVersionId(null); }, }); + const { - activeTemplateVersion, - templateVariables, - getTemplateDataError, - updateTemplateError, - jobError, - } = state.context; + data: status, + error: statusError, + refetch: refetchStatus, + isRefetching, + } = useQuery( + newVersionId ? templateVersion(newVersionId) : { enabled: false }, + ); + + const isSubmitting = Boolean(newVersionId && !statusError); + + // Poll build status while we're updating the template + useEffect(() => { + if (!newVersionId || !status || isRefetching) { + return; + } + + const jobStatus = status.job.status; + if (jobStatus === "pending" || jobStatus === "running") { + setTimeout(() => refetchStatus(), 2_000); + return; + } + + publishVersion(newVersionId); + }, [newVersionId, status, isRefetching]); + + const error = versionError ?? variablesError; + if (error) { + return ; + } + + if (isVersionLoading || isVariablesLoading) { + return ; + } return ( <> @@ -48,23 +111,21 @@ export const TemplateVariablesPage: FC = () => { { navigate(`/templates/${templateName}`); }} onSubmit={(formData) => { - const request = filterEmptySensitiveVariables( - formData, - templateVariables, - ); - send({ type: "UPDATE_TEMPLATE_EVENT", request: request }); + const request = filterEmptySensitiveVariables(formData, variables); + sendCreateTemplateVersion(request); }} /> diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx index dfd0baf83e2b6..51035305f83ca 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx @@ -49,7 +49,7 @@ export const RequiredVariable: Story = { }, }; -export const WithUpdateTemplateError: Story = { +export const WithErrors: Story = { args: { templateVersion: MockTemplateVersion, templateVariables: [ @@ -59,25 +59,24 @@ export const WithUpdateTemplateError: Story = { MockTemplateVersionVariable4, ], errors: { - updateTemplateError: mockApiError({ - message: "Something went wrong.", + createVersionError: mockApiError({ + message: "createVersionError", }), + statusError: mockApiError({ + message: "statusError", + validations: [ + { + field: `user_variable_values[0].value`, + detail: "Variable is required.", + }, + ], + }), + jobError: "jobError", + publishError: mockApiError({ message: "publishError" }), }, - }, -}; -export const WithJobError: Story = { - args: { - templateVersion: MockTemplateVersion, - templateVariables: [ - MockTemplateVersionVariable1, - MockTemplateVersionVariable2, - MockTemplateVersionVariable3, - MockTemplateVersionVariable4, - ], - errors: { - jobError: - "template import provision for start: recv import provision: plan terraform: terraform plan: exit status 1", + initialTouched: { + user_variable_values: true, }, }, }; diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx index 19b1c8d015966..d539ce774f711 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx @@ -4,12 +4,12 @@ import { TemplateVersionVariable, } from "api/typesGenerated"; import { Alert } from "components/Alert/Alert"; -import { Loader } from "components/Loader/Loader"; import { ComponentProps, FC } from "react"; import { TemplateVariablesForm } from "./TemplateVariablesForm"; import { makeStyles } from "@mui/styles"; import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader"; import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { Stack } from "components/Stack/Stack"; export interface TemplateVariablesPageViewProps { templateVersion?: TemplateVersion; @@ -18,9 +18,22 @@ export interface TemplateVariablesPageViewProps { onCancel: () => void; isSubmitting: boolean; errors?: { - getTemplateDataError?: unknown; - updateTemplateError?: unknown; + /** + * Failed to create a new template version + */ + createVersionError?: unknown; + /** + * Failed to poll job status of new template version + */ + statusError?: unknown; + /** + * Job for new template version failed + */ jobError?: TemplateVersion["job"]["error"]; + /** + * New version was created successfully, but publishing it failed + */ + publishError?: unknown; }; initialTouched?: ComponentProps< typeof TemplateVariablesForm @@ -37,29 +50,29 @@ export const TemplateVariablesPageView: FC = ({ initialTouched, }) => { const classes = useStyles(); - const isLoading = - !templateVersion && - !templateVariables && - !errors.getTemplateDataError && - !errors.updateTemplateError; const hasError = Object.values(errors).some((error) => Boolean(error)); + + console.log("LOOK AT ME", errors); + return ( <> Template variables {hasError && ( -
- {Boolean(errors.getTemplateDataError) && ( - + + {Boolean(errors.createVersionError) && ( + )} - {Boolean(errors.updateTemplateError) && ( - + {Boolean(errors.statusError) && ( + )} {Boolean(errors.jobError) && } -
+ {Boolean(errors.publishError) && ( + + )} + )} - {isLoading && } {templateVersion && templateVariables && templateVariables.length > 0 && ( = ({ templateVariables={templateVariables} onSubmit={onSubmit} onCancel={onCancel} - error={errors.updateTemplateError} + error={errors.statusError} /> )} {templateVariables && templateVariables.length === 0 && ( diff --git a/site/src/xServices/template/templateVariablesXService.ts b/site/src/xServices/template/templateVariablesXService.ts deleted file mode 100644 index f2920d804eb58..0000000000000 --- a/site/src/xServices/template/templateVariablesXService.ts +++ /dev/null @@ -1,244 +0,0 @@ -import { - createTemplateVersion, - getTemplateVersion, - getTemplateVersionVariables, - updateActiveTemplateVersion, -} from "api/api"; -import { - CreateTemplateVersionRequest, - Response, - Template, - TemplateVersion, - TemplateVersionVariable, -} from "api/typesGenerated"; -import { assign, createMachine } from "xstate"; -import { delay } from "utils/delay"; - -type TemplateVariablesContext = { - organizationId: string; - - template: Template; - activeTemplateVersion?: TemplateVersion; - templateVariables?: TemplateVersionVariable[]; - - createTemplateVersionRequest?: CreateTemplateVersionRequest; - newTemplateVersion?: TemplateVersion; - - getTemplateDataError?: unknown; - updateTemplateError?: unknown; - - jobError?: TemplateVersion["job"]["error"]; -}; - -type UpdateTemplateEvent = { - type: "UPDATE_TEMPLATE_EVENT"; - request: CreateTemplateVersionRequest; -}; - -export const templateVariablesMachine = createMachine( - { - id: "templateVariablesState", - predictableActionArguments: true, - tsTypes: {} as import("./templateVariablesXService.typegen").Typegen0, - schema: { - context: {} as TemplateVariablesContext, - events: {} as UpdateTemplateEvent, - services: {} as { - getActiveTemplateVersion: { - data: TemplateVersion; - }; - getTemplateVariables: { - data: TemplateVersionVariable[]; - }; - createNewTemplateVersion: { - data: TemplateVersion; - }; - waitForJobToBeCompleted: { - data: TemplateVersion; - }; - updateTemplate: { - data: Response; - }; - }, - }, - initial: "gettingActiveTemplateVersion", - states: { - gettingActiveTemplateVersion: { - entry: "clearGetTemplateDataError", - invoke: { - src: "getActiveTemplateVersion", - onDone: [ - { - actions: ["assignActiveTemplateVersion"], - target: "gettingTemplateVariables", - }, - ], - onError: { - actions: ["assignGetTemplateDataError"], - target: "error", - }, - }, - }, - gettingTemplateVariables: { - entry: "clearGetTemplateDataError", - invoke: { - src: "getTemplateVariables", - onDone: [ - { - actions: ["assignTemplateVariables"], - target: "fillingParams", - }, - ], - onError: { - actions: ["assignGetTemplateDataError"], - target: "error", - }, - }, - }, - fillingParams: { - on: { - UPDATE_TEMPLATE_EVENT: { - actions: ["assignCreateTemplateVersionRequest", "clearJobError"], - target: "creatingTemplateVersion", - }, - }, - }, - creatingTemplateVersion: { - entry: "clearUpdateTemplateError", - invoke: { - src: "createNewTemplateVersion", - onDone: { - actions: ["assignNewTemplateVersion"], - target: "waitingForJobToBeCompleted", - }, - onError: { - actions: ["assignGetTemplateDataError"], - target: "fillingParams", - }, - }, - tags: ["submitting"], - }, - waitingForJobToBeCompleted: { - invoke: { - src: "waitForJobToBeCompleted", - onDone: [ - { - target: "fillingParams", - cond: "hasJobError", - actions: ["assignJobError"], - }, - { - actions: ["assignNewTemplateVersion"], - target: "updatingTemplate", - }, - ], - onError: { - actions: ["assignUpdateTemplateError"], - target: "fillingParams", - }, - }, - tags: ["submitting"], - }, - updatingTemplate: { - invoke: { - src: "updateTemplate", - onDone: { - target: "updated", - actions: ["onUpdateTemplate"], - }, - onError: { - actions: ["assignUpdateTemplateError"], - target: "fillingParams", - }, - }, - tags: ["submitting"], - }, - updated: { - entry: "onUpdateTemplate", - type: "final", - }, - error: {}, - }, - }, - { - services: { - getActiveTemplateVersion: ({ template }) => { - return getTemplateVersion(template.active_version_id); - }, - getTemplateVariables: ({ template }) => { - return getTemplateVersionVariables(template.active_version_id); - }, - createNewTemplateVersion: ({ - organizationId, - createTemplateVersionRequest, - }) => { - if (!createTemplateVersionRequest) { - throw new Error("Missing request body"); - } - return createTemplateVersion( - organizationId, - createTemplateVersionRequest, - ); - }, - waitForJobToBeCompleted: async ({ newTemplateVersion }) => { - if (!newTemplateVersion) { - throw new Error("Template version is undefined"); - } - - let status = newTemplateVersion.job.status; - while (["pending", "running"].includes(status)) { - newTemplateVersion = await getTemplateVersion(newTemplateVersion.id); - status = newTemplateVersion.job.status; - await delay(2_000); - } - return newTemplateVersion; - }, - updateTemplate: ({ template, newTemplateVersion }) => { - if (!newTemplateVersion) { - throw new Error("New template version is undefined"); - } - - return updateActiveTemplateVersion(template.id, { - id: newTemplateVersion.id, - }); - }, - }, - actions: { - assignActiveTemplateVersion: assign({ - activeTemplateVersion: (_, event) => event.data, - }), - assignTemplateVariables: assign({ - templateVariables: (_, event) => event.data, - }), - assignCreateTemplateVersionRequest: assign({ - createTemplateVersionRequest: (_, event) => event.request, - }), - assignNewTemplateVersion: assign({ - newTemplateVersion: (_, event) => event.data, - }), - assignGetTemplateDataError: assign({ - getTemplateDataError: (_, event) => event.data, - }), - clearGetTemplateDataError: assign({ - getTemplateDataError: (_) => undefined, - }), - assignUpdateTemplateError: assign({ - updateTemplateError: (_, event) => event.data, - }), - clearUpdateTemplateError: assign({ - updateTemplateError: (_) => undefined, - }), - assignJobError: assign({ - jobError: (_, event) => event.data.job.error, - }), - clearJobError: assign({ - jobError: (_) => undefined, - }), - }, - guards: { - hasJobError: (_, { data }) => { - return Boolean(data.job.error); - }, - }, - }, -); From f5e72066f57c8cac021f5c061a59361faa7be68a Mon Sep 17 00:00:00 2001 From: Kayla Washburn Date: Tue, 19 Sep 2023 09:09:48 -0600 Subject: [PATCH 2/5] change queryKey Co-authored-by: Bruno Quaresma --- site/src/api/queries/templates.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index deef101b834ab..43f5e743374a9 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -70,7 +70,7 @@ export const templateVersions = (templateId: string) => { export const templateVersionVariables = (versionId: string) => { return { - queryKey: ["templateVersionVariables", versionId], + queryKey: ["templateVersion", versionId, "variables"], queryFn: () => API.getTemplateVersionVariables(versionId), }; }; From a4032d091a4d89960b5d2963c16cce29172ffb91 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 19 Sep 2023 16:20:18 +0000 Subject: [PATCH 3/5] feedback --- site/src/api/queries/templates.ts | 30 +++++-- .../TemplateVariablesPage.tsx | 78 +++++++------------ .../TemplateVariablesPageView.stories.tsx | 6 +- .../TemplateVariablesPageView.tsx | 24 ++---- 4 files changed, 58 insertions(+), 80 deletions(-) diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 43f5e743374a9..7bb34b74df124 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -3,8 +3,11 @@ import { type Template, type AuthorizationResponse, type CreateTemplateVersionRequest, + type ProvisionerJobStatus, + type TemplateVersion, } from "api/typesGenerated"; import { type QueryClient, type QueryOptions } from "@tanstack/react-query"; +import { delay } from "utils/delay"; export const templateByNameKey = (orgId: string, name: string) => [ orgId, @@ -75,13 +78,28 @@ export const templateVersionVariables = (versionId: string) => { }; }; -export const createTemplateVersion = ( - orgId: string, - queryClient: QueryClient, -) => { +export const createAndBuildTemplateVersion = (orgId: string) => { return { - mutationFn: (request: CreateTemplateVersionRequest) => - API.createTemplateVersion(orgId, request), + mutationFn: async ( + request: CreateTemplateVersionRequest, + ): Promise => { + const newVersion = await API.createTemplateVersion(orgId, request); + + let data: TemplateVersion; + let jobStatus: ProvisionerJobStatus; + do { + await delay(1000); + data = await API.getTemplateVersion(newVersion.id); + jobStatus = data.job.status; + + if (jobStatus === "succeeded") { + return newVersion.id; + } + } while (jobStatus === "pending" || jobStatus === "running"); + + // No longer pending/running, but didn't succeed + throw data.job.error; + }, }; }; diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx index 12f4a7a6cb013..db02b3f10b46e 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.tsx @@ -1,4 +1,3 @@ -import { useMachine } from "@xstate/react"; import { CreateTemplateVersionRequest, TemplateVersionVariable, @@ -6,14 +5,14 @@ import { } from "api/typesGenerated"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { useOrganizationId } from "hooks/useOrganizationId"; -import { useEffect, type FC, useState } from "react"; +import { useCallback, type FC } from "react"; import { Helmet } from "react-helmet-async"; import { useNavigate, useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import { useTemplateSettings } from "../TemplateSettingsLayout"; import { TemplateVariablesPageView } from "./TemplateVariablesPageView"; import { - createTemplateVersion, + createAndBuildTemplateVersion, templateVersion, templateVersionVariables, updateActiveTemplateVersion, @@ -33,8 +32,6 @@ export const TemplateVariablesPage: FC = () => { const queryClient = useQueryClient(); const versionId = template.active_version_id; - const [newVersionId, setNewVersionId] = useState(null); - const { data: version, error: versionError, @@ -49,51 +46,34 @@ export const TemplateVariablesPage: FC = () => { keepPreviousData: true, }); - const createVersionMutation = createTemplateVersion(orgId, queryClient); - const { mutate: sendCreateTemplateVersion, error: createVersionError } = - useMutation({ - ...createVersionMutation, - onSuccess: (data) => { - setNewVersionId(data.id); - }, - }); - const publishMutation = updateActiveTemplateVersion(template, queryClient); - const { mutate: publishVersion, error: publishError } = useMutation({ - ...publishMutation, - onSuccess: () => { - publishMutation.onSuccess(); + const { + mutateAsync: sendCreateAndBuildTemplateVersion, + error: buildError, + isLoading: isBuilding, + } = useMutation(createAndBuildTemplateVersion(orgId)); + const { + mutateAsync: sendUpdateActiveTemplateVersion, + error: publishError, + isLoading: isPublishing, + } = useMutation(updateActiveTemplateVersion(template, queryClient)); + + const publishVersion = useCallback( + async (versionId: string) => { + await sendUpdateActiveTemplateVersion(versionId); displaySuccess("Template updated successfully"); }, - onSettled: () => { - setNewVersionId(null); - }, - }); - - const { - data: status, - error: statusError, - refetch: refetchStatus, - isRefetching, - } = useQuery( - newVersionId ? templateVersion(newVersionId) : { enabled: false }, + [sendUpdateActiveTemplateVersion], ); - const isSubmitting = Boolean(newVersionId && !statusError); - - // Poll build status while we're updating the template - useEffect(() => { - if (!newVersionId || !status || isRefetching) { - return; - } - - const jobStatus = status.job.status; - if (jobStatus === "pending" || jobStatus === "running") { - setTimeout(() => refetchStatus(), 2_000); - return; - } + const buildVersion = useCallback( + async (req: CreateTemplateVersionRequest) => { + const newVersionId = await sendCreateAndBuildTemplateVersion(req); + await publishVersion(newVersionId); + }, + [sendCreateAndBuildTemplateVersion, publishVersion], + ); - publishVersion(newVersionId); - }, [newVersionId, status, isRefetching]); + const isSubmitting = Boolean(isBuilding || isPublishing); const error = versionError ?? variablesError; if (error) { @@ -115,17 +95,15 @@ export const TemplateVariablesPage: FC = () => { templateVersion={version} templateVariables={variables} errors={{ - createVersionError, - statusError, - jobError: status?.job.error, + buildError, publishError, }} onCancel={() => { navigate(`/templates/${templateName}`); }} - onSubmit={(formData) => { + onSubmit={async (formData) => { const request = filterEmptySensitiveVariables(formData, variables); - sendCreateTemplateVersion(request); + await buildVersion(request); }} /> diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx index 51035305f83ca..d104e9ab82ab7 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx @@ -59,10 +59,7 @@ export const WithErrors: Story = { MockTemplateVersionVariable4, ], errors: { - createVersionError: mockApiError({ - message: "createVersionError", - }), - statusError: mockApiError({ + buildError: mockApiError({ message: "statusError", validations: [ { @@ -71,7 +68,6 @@ export const WithErrors: Story = { }, ], }), - jobError: "jobError", publishError: mockApiError({ message: "publishError" }), }, diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx index d539ce774f711..2c3cd8d5100df 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.tsx @@ -19,17 +19,9 @@ export interface TemplateVariablesPageViewProps { isSubmitting: boolean; errors?: { /** - * Failed to create a new template version + * Failed to build a new template version */ - createVersionError?: unknown; - /** - * Failed to poll job status of new template version - */ - statusError?: unknown; - /** - * Job for new template version failed - */ - jobError?: TemplateVersion["job"]["error"]; + buildError?: unknown; /** * New version was created successfully, but publishing it failed */ @@ -52,8 +44,6 @@ export const TemplateVariablesPageView: FC = ({ const classes = useStyles(); const hasError = Object.values(errors).some((error) => Boolean(error)); - console.log("LOOK AT ME", errors); - return ( <> @@ -61,13 +51,9 @@ export const TemplateVariablesPageView: FC = ({ {hasError && ( - {Boolean(errors.createVersionError) && ( - - )} - {Boolean(errors.statusError) && ( - + {Boolean(errors.buildError) && ( + )} - {Boolean(errors.jobError) && } {Boolean(errors.publishError) && ( )} @@ -81,7 +67,7 @@ export const TemplateVariablesPageView: FC = ({ templateVariables={templateVariables} onSubmit={onSubmit} onCancel={onCancel} - error={errors.statusError} + error={errors.buildError} /> )} {templateVariables && templateVariables.length === 0 && ( From 687afa66cc91122e3fbfbc9886ece09a70ce9165 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 19 Sep 2023 17:38:33 +0000 Subject: [PATCH 4/5] fix tests --- .../TemplateVariablesPage.test.tsx | 48 ++----------------- .../TemplateVariablesPageView.stories.tsx | 2 +- 2 files changed, 5 insertions(+), 45 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx index 39adf1c02bd82..7bd9877a4808b 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx @@ -13,8 +13,8 @@ import { MockTemplateVersionVariable1, MockTemplateVersionVariable2, MockTemplateVersion2, - MockTemplateVersionVariable5, } from "testHelpers/entities"; +import { delay } from "utils/delay"; const validFormValues = { first_variable: "Hello world", @@ -62,7 +62,7 @@ describe("TemplateVariablesPage", () => { jest.spyOn(API, "getTemplateByName").mockResolvedValueOnce(MockTemplate); jest .spyOn(API, "getTemplateVersion") - .mockResolvedValueOnce(MockTemplateVersion); + .mockResolvedValue(MockTemplateVersion); jest .spyOn(API, "getTemplateVersionVariables") .mockResolvedValueOnce([ @@ -106,49 +106,9 @@ describe("TemplateVariablesPage", () => { FooterFormLanguage.defaultSubmitLabel, ); await userEvent.click(submitButton); - // Wait for the success message - await screen.findByText("Template updated successfully"); - }); - - it("user forgets to fill the required field", async () => { - jest.spyOn(API, "getTemplateByName").mockResolvedValueOnce(MockTemplate); - jest - .spyOn(API, "getTemplateVersion") - .mockResolvedValueOnce(MockTemplateVersion); - jest - .spyOn(API, "getTemplateVersionVariables") - .mockResolvedValueOnce([ - MockTemplateVersionVariable1, - MockTemplateVersionVariable5, - ]); - jest - .spyOn(API, "createTemplateVersion") - .mockResolvedValueOnce(MockTemplateVersion2); - jest.spyOn(API, "updateActiveTemplateVersion").mockResolvedValueOnce({ - message: "done", - }); - - await renderTemplateVariablesPage(); - - const firstVariable = await screen.findByLabelText( - MockTemplateVersionVariable1.name, - ); - expect(firstVariable).toBeDefined(); + await delay(1500); - const fifthVariable = await screen.findByLabelText( - MockTemplateVersionVariable5.name, - ); - expect(fifthVariable).toBeDefined(); - - // Submit the form - const submitButton = await screen.findByText( - FooterFormLanguage.defaultSubmitLabel, - ); - await userEvent.click(submitButton); - - // Check validation error - const validationError = await screen.findByText(validationRequiredField); - expect(validationError).toBeDefined(); + await screen.findByText("Template updated successfully"); }); }); diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx index d104e9ab82ab7..e52d07e5713f3 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx @@ -60,7 +60,7 @@ export const WithErrors: Story = { ], errors: { buildError: mockApiError({ - message: "statusError", + message: "buildError", validations: [ { field: `user_variable_values[0].value`, From 2fc9ffa22d76f46b5ca21e31c4fc7815900f83a8 Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Tue, 19 Sep 2023 17:42:10 +0000 Subject: [PATCH 5/5] lint --- .../TemplateVariablesPage/TemplateVariablesPage.test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx index 7bd9877a4808b..2ce13b21fbf10 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPage.test.tsx @@ -21,8 +21,6 @@ const validFormValues = { second_variable: "123", }; -const validationRequiredField = "Variable is required."; - const renderTemplateVariablesPage = async () => { renderWithTemplateSettingsLayout(, { route: `/templates/${MockTemplate.name}/variables`,