From ea9690c1a0dc25bfad962a90b9ba68095e1c14ee Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 2 Mar 2023 18:11:23 +0000 Subject: [PATCH 1/9] Change the create template flow to use variables --- .../CreateTemplatePage/CreateTemplateForm.tsx | 36 +++++ .../CreateTemplatePage/CreateTemplatePage.tsx | 12 +- .../CreateTemplatePage/VariableInput.tsx | 126 ++++++++++++++++++ .../createTemplate/createTemplateXService.ts | 118 ++++++++++------ 4 files changed, 249 insertions(+), 43 deletions(-) create mode 100644 site/src/pages/CreateTemplatePage/VariableInput.tsx diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 48fe9dd55f8d0..5d2be075029d0 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -5,6 +5,7 @@ import { ParameterSchema, ProvisionerJobLog, TemplateExample, + TemplateVersionVariable, } from "api/typesGenerated" import { FormFooter } from "components/FormFooter/FormFooter" import { ParameterInput } from "components/ParameterInput/ParameterInput" @@ -23,6 +24,7 @@ import * as Yup from "yup" import { WorkspaceBuildLogs } from "components/WorkspaceBuildLogs/WorkspaceBuildLogs" import { HelpTooltip, HelpTooltipText } from "components/Tooltips/HelpTooltip" import { LazyIconField } from "components/IconField/LazyIconField" +import { VariableInput } from "./VariableInput" const validationSchema = Yup.object({ name: nameValidator("Name"), @@ -32,6 +34,9 @@ const validationSchema = Yup.object({ default_ttl_hours: Yup.number(), allow_user_cancel_workspace_jobs: Yup.boolean(), parameter_values_by_name: Yup.object().optional(), + user_variable_values: Yup.array() + .of(Yup.object({ name: Yup.string(), value: Yup.string().optional() })) + .optional(), }) const defaultInitialValues: CreateTemplateData = { @@ -42,6 +47,7 @@ const defaultInitialValues: CreateTemplateData = { default_ttl_hours: 24, allow_user_cancel_workspace_jobs: false, parameter_values_by_name: undefined, + user_variable_values: undefined, } const getInitialValues = (starterTemplate?: TemplateExample) => { @@ -62,6 +68,7 @@ interface CreateTemplateFormProps { starterTemplate?: TemplateExample error?: unknown parameters?: ParameterSchema[] + variables?: TemplateVersionVariable[] isSubmitting: boolean onCancel: () => void onSubmit: (data: CreateTemplateData) => void @@ -74,6 +81,7 @@ export const CreateTemplateForm: FC = ({ starterTemplate, error, parameters, + variables, isSubmitting, onCancel, onSubmit, @@ -268,6 +276,34 @@ export const CreateTemplateForm: FC = ({ )} + {/* Variables */} + {variables && ( +
+
+

Variables

+

+ Enter the user variables +

+
+ + + {variables.map((variable, index) => ( + { + await form.setFieldValue("user_variable_values." + index, { + name: variable.name, + value: value, + }) + }} + /> + ))} + +
+ )} + {jobError && (
diff --git a/site/src/pages/CreateTemplatePage/CreateTemplatePage.tsx b/site/src/pages/CreateTemplatePage/CreateTemplatePage.tsx index 66adcca70fc7b..dabe9628feb4b 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplatePage.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplatePage.tsx @@ -30,8 +30,15 @@ const CreateTemplatePage: FC = () => { }, }, }) - const { starterTemplate, parameters, error, file, jobError, jobLogs } = - state.context + const { + starterTemplate, + parameters, + error, + file, + jobError, + jobLogs, + variables, + } = state.context const shouldDisplayForm = !state.hasTag("loading") const onCancel = () => { @@ -59,6 +66,7 @@ const CreateTemplatePage: FC = () => { error={error} starterTemplate={starterTemplate} isSubmitting={state.hasTag("submitting")} + variables={variables} parameters={parameters} onCancel={onCancel} onSubmit={(data) => { diff --git a/site/src/pages/CreateTemplatePage/VariableInput.tsx b/site/src/pages/CreateTemplatePage/VariableInput.tsx new file mode 100644 index 0000000000000..1d5b4b2027e0f --- /dev/null +++ b/site/src/pages/CreateTemplatePage/VariableInput.tsx @@ -0,0 +1,126 @@ +import FormControlLabel from "@material-ui/core/FormControlLabel" +import Radio from "@material-ui/core/Radio" +import RadioGroup from "@material-ui/core/RadioGroup" +import { makeStyles } from "@material-ui/core/styles" +import TextField from "@material-ui/core/TextField" +import { Stack } from "components/Stack/Stack" +import { FC } from "react" +import { TemplateVersionVariable } from "../../api/typesGenerated" + +const isBoolean = (variable: TemplateVersionVariable) => { + return variable.type === "bool" +} + +const VariableLabel: React.FC<{ variable: TemplateVersionVariable }> = ({ + variable, +}) => { + const styles = useStyles() + + return ( + + ) +} + +export interface VariableInputProps { + disabled?: boolean + variable: TemplateVersionVariable + onChange: (value: string) => void + defaultValue?: string +} + +export const VariableInput: FC = ({ + disabled, + onChange, + variable, + defaultValue, +}) => { + const styles = useStyles() + + return ( + + +
+ +
+ + ) +} + +const VariableField: React.FC = ({ + disabled, + onChange, + variable, + defaultValue, +}) => { + if (isBoolean(variable)) { + return ( + { + onChange(event.target.value) + }} + > + } + label="True" + /> + } + label="False" + /> + + ) + } + + return ( + { + onChange(event.target.value) + }} + /> + ) +} + +const useStyles = makeStyles((theme) => ({ + labelName: { + fontSize: 14, + color: theme.palette.text.secondary, + display: "block", + marginBottom: theme.spacing(0.5), + }, + labelDescription: { + fontSize: 16, + color: theme.palette.text.primary, + display: "block", + fontWeight: 600, + }, + input: { + display: "flex", + flexDirection: "column", + }, + checkbox: { + display: "flex", + alignItems: "center", + gap: theme.spacing(1), + }, +})) diff --git a/site/src/xServices/createTemplate/createTemplateXService.ts b/site/src/xServices/createTemplate/createTemplateXService.ts index c533715371602..ec499c1ef26b2 100644 --- a/site/src/xServices/createTemplate/createTemplateXService.ts +++ b/site/src/xServices/createTemplate/createTemplateXService.ts @@ -6,6 +6,7 @@ import { getTemplateVersionSchema, uploadTemplateFile, getTemplateVersionLogs, + getTemplateVersionVariables, } from "api/api" import { CreateTemplateVersionRequest, @@ -14,7 +15,9 @@ import { Template, TemplateExample, TemplateVersion, + TemplateVersionVariable, UploadResponse, + VariableValue, } from "api/typesGenerated" import { displayError } from "components/GlobalSnackbar/utils" import { delay } from "util/delay" @@ -24,7 +27,7 @@ import { assign, createMachine } from "xstate" // 1. upload template tar or use the example ID // 2. create template version // 3. wait for it to complete -// 4. if the job failed with the missing parameter error then: +// 4. verify if template has missing parameters or variables // a. prompt for params // b. create template version again with the same file hash // c. wait for it to complete @@ -39,6 +42,7 @@ export interface CreateTemplateData { default_ttl_hours: number allow_user_cancel_workspace_jobs: boolean parameter_values_by_name?: Record + user_variable_values?: VariableValue[] } interface CreateTemplateContext { organizationId: string @@ -50,6 +54,7 @@ interface CreateTemplateContext { version?: TemplateVersion templateData?: CreateTemplateData parameters?: ParameterSchema[] + variables?: TemplateVersionVariable[] // file is used in the FE to show the filename and some other visual stuff // uploadedFile is the response from the server to use in the API file?: File @@ -78,7 +83,7 @@ export const createTemplateMachine = createFirstVersion: { data: TemplateVersion } - createVersionWithParameters: { + createVersionWithParametersAndVariables: { data: TemplateVersion } waitForJobToBeCompleted: { @@ -87,6 +92,12 @@ export const createTemplateMachine = loadParameterSchema: { data: ParameterSchema[] } + checkParametersAndVariables: { + data: { + parameters?: ParameterSchema[] + variables: TemplateVersionVariable[] + } + } createTemplate: { data: Template } @@ -170,17 +181,15 @@ export const createTemplateMachine = invoke: { src: "waitForJobToBeCompleted", onDone: [ - { - target: "loadingMissingParameters", - cond: "hasMissingParameters", - actions: ["assignVersion"], - }, { target: "loadingVersionLogs", actions: ["assignJobError", "assignVersion"], cond: "hasFailed", }, - { target: "creatingTemplate", actions: ["assignVersion"] }, + { + target: "checkingParametersAndVariables", + actions: ["assignVersion"], + }, ], onError: { target: "#createTemplate.idle", @@ -189,51 +198,43 @@ export const createTemplateMachine = }, tags: ["submitting"], }, - loadingVersionLogs: { + checkingParametersAndVariables: { invoke: { - src: "loadVersionLogs", - onDone: { - target: "#createTemplate.idle", - actions: ["assignJobLogs"], - }, - onError: { - target: "#createTemplate.idle", - actions: ["assignError"], - }, - }, - }, - loadingMissingParameters: { - invoke: { - src: "loadParameterSchema", - onDone: { - target: "promptParameters", - actions: ["assignParameters"], - }, + src: "checkParametersAndVariables", + onDone: [ + { + target: "creatingTemplate", + cond: "hasNoParametersOrVariables", + }, + { + target: "promptParametersAndVariables", + actions: ["assignParametersAndVariables"], + }, + ], onError: { target: "#createTemplate.idle", actions: ["assignError"], }, }, - tags: ["submitting"], }, - promptParameters: { + promptParametersAndVariables: { on: { CREATE: { - target: "creatingVersionWithParameters", + target: "creatingVersionWithParametersAndVariables", actions: ["assignTemplateData"], }, }, }, - creatingVersionWithParameters: { + creatingVersionWithParametersAndVariables: { invoke: { - src: "createVersionWithParameters", + src: "createVersionWithParametersAndVariables", onDone: { target: "waitingForJobToBeCompleted", actions: ["assignVersion"], }, onError: { actions: ["assignError"], - target: "promptParameters", + target: "promptParametersAndVariables", }, }, tags: ["submitting"], @@ -255,6 +256,19 @@ export const createTemplateMachine = created: { type: "final", }, + loadingVersionLogs: { + invoke: { + src: "loadVersionLogs", + onDone: { + target: "#createTemplate.idle", + actions: ["assignJobLogs"], + }, + onError: { + target: "#createTemplate.idle", + actions: ["assignError"], + }, + }, + }, }, }, }, @@ -300,7 +314,7 @@ export const createTemplateMachine = throw new Error("No file or example provided") }, - createVersionWithParameters: async ({ + createVersionWithParametersAndVariables: async ({ organizationId, parameters, templateData, @@ -313,11 +327,11 @@ export const createTemplateMachine = throw new Error("No template data defined") } - const { parameter_values_by_name } = templateData // Get parameter values if they are needed/present const parameterValues: CreateTemplateVersionRequest["parameter_values"] = [] if (parameters) { + const { parameter_values_by_name } = templateData parameters.forEach((schema) => { const value = parameter_values_by_name?.[schema.name] parameterValues.push({ @@ -354,12 +368,24 @@ export const createTemplateMachine = } return version }, - loadParameterSchema: async ({ version }) => { + checkParametersAndVariables: async ({ version }) => { if (!version) { throw new Error("Version not defined") } - return getTemplateVersionSchema(version.id) + if (isMissingParameter(version)) { + const [parameters, variables] = await Promise.all([ + getTemplateVersionSchema(version.id), + getTemplateVersionVariables(version.id), + ]) + + return { parameters, variables } + } + + return { + parameters: undefined, + variables: await getTemplateVersionVariables(version.id), + } }, createTemplate: async ({ organizationId, version, templateData }) => { if (!version) { @@ -401,7 +427,10 @@ export const createTemplateMachine = }), assignVersion: assign({ version: (_, { data }) => data }), assignTemplateData: assign({ templateData: (_, { data }) => data }), - assignParameters: assign({ parameters: (_, { data }) => data }), + assignParametersAndVariables: assign({ + parameters: (_, { data }) => data.parameters, + variables: (_, { data }) => data.variables, + }), assignFile: assign({ file: (_, { file }) => file }), assignUploadResponse: assign({ uploadResponse: (_, { data }) => data }), removeFile: assign({ @@ -414,11 +443,18 @@ export const createTemplateMachine = isExampleProvided: ({ exampleId }) => Boolean(exampleId), isNotUsingExample: ({ exampleId }) => !exampleId, hasFile: ({ file }) => Boolean(file), - hasFailed: (_, { data }) => data.job.status === "failed", - hasMissingParameters: (_, { data }) => + hasFailed: (_, { data }) => Boolean( - data.job.error && data.job.error.includes("missing parameter"), + data.job.status === "failed" && !isMissingParameter(data), // This should not be considered as a "hard" failure ), + hasNoParametersOrVariables: (_, { data }) => + data.parameters === undefined && data.variables.length === 0, }, }, ) + +const isMissingParameter = (version: TemplateVersion) => { + return Boolean( + version.job.error && version.job.error.includes("missing parameter"), + ) +} From 772a0aad3f6a277ecdd0e71c5c4049e8edb16457 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 2 Mar 2023 18:53:26 +0000 Subject: [PATCH 2/9] Send right variable values --- .../CreateTemplatePage/VariableInput.tsx | 13 +++++- .../createTemplate/createTemplateXService.ts | 40 ++++++++++++++----- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/VariableInput.tsx b/site/src/pages/CreateTemplatePage/VariableInput.tsx index 1d5b4b2027e0f..25ce27541fee3 100644 --- a/site/src/pages/CreateTemplatePage/VariableInput.tsx +++ b/site/src/pages/CreateTemplatePage/VariableInput.tsx @@ -18,7 +18,10 @@ const VariableLabel: React.FC<{ variable: TemplateVersionVariable }> = ({ return ( ) @@ -91,12 +94,20 @@ const VariableField: React.FC = ({ size="small" disabled={disabled} placeholder={variable.sensitive ? "" : variable.default_value} + required={variable.required} defaultValue={ variable.sensitive ? "" : defaultValue ?? variable.default_value } onChange={(event) => { onChange(event.target.value) }} + type={ + variable.type === "number" + ? "number" + : variable.sensitive + ? "password" + : "string" + } /> ) } diff --git a/site/src/xServices/createTemplate/createTemplateXService.ts b/site/src/xServices/createTemplate/createTemplateXService.ts index ec499c1ef26b2..e256a71870ce7 100644 --- a/site/src/xServices/createTemplate/createTemplateXService.ts +++ b/site/src/xServices/createTemplate/createTemplateXService.ts @@ -95,7 +95,7 @@ export const createTemplateMachine = checkParametersAndVariables: { data: { parameters?: ParameterSchema[] - variables: TemplateVersionVariable[] + variables?: TemplateVersionVariable[] } } createTemplate: { @@ -216,6 +216,7 @@ export const createTemplateMachine = actions: ["assignError"], }, }, + tags: ["submitting"], }, promptParametersAndVariables: { on: { @@ -348,6 +349,7 @@ export const createTemplateMachine = file_id: version.job.file_id, provisioner: "terraform", parameter_values: parameterValues, + user_variable_values: templateData.user_variable_values, tags: {}, }) }, @@ -373,18 +375,27 @@ export const createTemplateMachine = throw new Error("Version not defined") } + let promiseParameter: Promise | undefined = + undefined + let promiseVariables: Promise | undefined = + undefined + if (isMissingParameter(version)) { - const [parameters, variables] = await Promise.all([ - getTemplateVersionSchema(version.id), - getTemplateVersionVariables(version.id), - ]) + promiseParameter = getTemplateVersionSchema(version.id) + } - return { parameters, variables } + if (isMissingVariables(version)) { + promiseVariables = getTemplateVersionVariables(version.id) } + const [parameters, variables] = await Promise.all([ + promiseParameter, + promiseVariables, + ]) + return { - parameters: undefined, - variables: await getTemplateVersionVariables(version.id), + parameters, + variables, } }, createTemplate: async ({ organizationId, version, templateData }) => { @@ -445,10 +456,12 @@ export const createTemplateMachine = hasFile: ({ file }) => Boolean(file), hasFailed: (_, { data }) => Boolean( - data.job.status === "failed" && !isMissingParameter(data), // This should not be considered as a "hard" failure + data.job.status === "failed" && + !isMissingParameter(data) && + !isMissingVariables(data), ), hasNoParametersOrVariables: (_, { data }) => - data.parameters === undefined && data.variables.length === 0, + data.parameters === undefined && data.variables === undefined, }, }, ) @@ -458,3 +471,10 @@ const isMissingParameter = (version: TemplateVersion) => { version.job.error && version.job.error.includes("missing parameter"), ) } + +const isMissingVariables = (version: TemplateVersion) => { + return Boolean( + version.job.error && + version.job.error.includes("required template variables"), + ) +} From 42bff4985c624ad93c8166c7acd3669f129af5ea Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 2 Mar 2023 19:06:06 +0000 Subject: [PATCH 3/9] Remove unecessary validation --- .../CreateTemplatePage/CreateTemplateForm.tsx | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 5d2be075029d0..f89b6eebc9173 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -18,7 +18,12 @@ import { useFormik } from "formik" import { SelectedTemplate } from "pages/CreateWorkspacePage/SelectedTemplate" import { FC } from "react" import { useTranslation } from "react-i18next" -import { nameValidator, getFormHelpers, onChangeTrimmed } from "util/formUtils" +import { + nameValidator, + getFormHelpers, + onChangeTrimmed, + templateDisplayNameValidator, +} from "util/formUtils" import { CreateTemplateData } from "xServices/createTemplate/createTemplateXService" import * as Yup from "yup" import { WorkspaceBuildLogs } from "components/WorkspaceBuildLogs/WorkspaceBuildLogs" @@ -28,15 +33,7 @@ import { VariableInput } from "./VariableInput" const validationSchema = Yup.object({ name: nameValidator("Name"), - display_name: Yup.string().optional(), - description: Yup.string().optional(), - icon: Yup.string().optional(), - default_ttl_hours: Yup.number(), - allow_user_cancel_workspace_jobs: Yup.boolean(), - parameter_values_by_name: Yup.object().optional(), - user_variable_values: Yup.array() - .of(Yup.object({ name: Yup.string(), value: Yup.string().optional() })) - .optional(), + display_name: templateDisplayNameValidator("Display name"), }) const defaultInitialValues: CreateTemplateData = { @@ -46,8 +43,6 @@ const defaultInitialValues: CreateTemplateData = { icon: "", default_ttl_hours: 24, allow_user_cancel_workspace_jobs: false, - parameter_values_by_name: undefined, - user_variable_values: undefined, } const getInitialValues = (starterTemplate?: TemplateExample) => { @@ -65,27 +60,27 @@ const getInitialValues = (starterTemplate?: TemplateExample) => { } interface CreateTemplateFormProps { - starterTemplate?: TemplateExample - error?: unknown - parameters?: ParameterSchema[] - variables?: TemplateVersionVariable[] - isSubmitting: boolean onCancel: () => void onSubmit: (data: CreateTemplateData) => void + isSubmitting: boolean upload: TemplateUploadProps + starterTemplate?: TemplateExample + parameters?: ParameterSchema[] + variables?: TemplateVersionVariable[] + error?: unknown jobError?: string logs?: ProvisionerJobLog[] } export const CreateTemplateForm: FC = ({ + onCancel, + onSubmit, starterTemplate, - error, parameters, variables, isSubmitting, - onCancel, - onSubmit, upload, + error, jobError, logs, }) => { @@ -126,6 +121,7 @@ export const CreateTemplateForm: FC = ({ onChange={onChangeTrimmed(form)} autoFocus fullWidth + required label={t("form.fields.name")} variant="outlined" /> From c8da058f5d6e5e9eaf81463d79f1e84210e6800f Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 2 Mar 2023 19:08:58 +0000 Subject: [PATCH 4/9] Tempalte display name is optional --- site/src/util/formUtils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/util/formUtils.ts b/site/src/util/formUtils.ts index 029a40727f087..2a6f7eb8f2853 100644 --- a/site/src/util/formUtils.ts +++ b/site/src/util/formUtils.ts @@ -100,3 +100,4 @@ export const templateDisplayNameValidator = ( templateDisplayNameMaxLength, Language.nameTooLong(displayName, templateDisplayNameMaxLength), ) + .optional() From 2965b3c26011e03d96f5222cc696a2f9ddf038a7 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 3 Mar 2023 12:48:43 +0000 Subject: [PATCH 5/9] A few refactorings --- .../CreateTemplatePage/CreateTemplateForm.tsx | 503 ++++++++---------- 1 file changed, 210 insertions(+), 293 deletions(-) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index f89b6eebc9173..96380431241b0 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -7,7 +7,6 @@ import { TemplateExample, TemplateVersionVariable, } from "api/typesGenerated" -import { FormFooter } from "components/FormFooter/FormFooter" import { ParameterInput } from "components/ParameterInput/ParameterInput" import { Stack } from "components/Stack/Stack" import { @@ -30,6 +29,14 @@ import { WorkspaceBuildLogs } from "components/WorkspaceBuildLogs/WorkspaceBuild import { HelpTooltip, HelpTooltipText } from "components/Tooltips/HelpTooltip" import { LazyIconField } from "components/IconField/LazyIconField" import { VariableInput } from "./VariableInput" +import { + FormFields, + FormFooter, + FormSection, + HorizontalForm, +} from "components/HorizontalForm/HorizontalForm" +import camelCase from "lodash/camelCase" +import capitalize from "lodash/capitalize" const validationSchema = Yup.object({ name: nameValidator("Name"), @@ -85,7 +92,6 @@ export const CreateTemplateForm: FC = ({ logs, }) => { const styles = useStyles() - const formFooterStyles = useFormFooterStyles() const form = useFormik({ initialValues: getInitialValues(starterTemplate), validationSchema, @@ -95,287 +101,220 @@ export const CreateTemplateForm: FC = ({ const { t } = useTranslation("createTemplatePage") return ( -
- - {/* General info */} -
-
-

- {t("form.generalInfo.title")} -

-

- {t("form.generalInfo.description")} -

-
- - - {starterTemplate ? ( - - ) : ( - - )} - - - -
- - {/* Display info */} -
-
-

- {t("form.displayInfo.title")} -

-

- {t("form.displayInfo.description")} -

-
- - - - - - - form.setFieldValue("icon", value)} - /> - -
- - {/* Schedule */} -
-
-

- {t("form.schedule.title")} -

-

- {t("form.schedule.description")} -

-
- - - + {/* General info */} + + + {starterTemplate ? ( + + ) : ( + { + await fillNameAndDisplayWithFilename(file.name, form) + upload.onUpload(file) + }} /> - -
- - {/* Operations */} -
-
-

- {t("form.operations.title")} -

-

- {t("form.operations.description")} -

-
- - -
- - {/* Parameters */} - {parameters && ( -
-
-

- {t("form.parameters.title")} -

-

- {t("form.parameters.description")} -

-
- - - {parameters.map((schema) => ( - { - await form.setFieldValue( - `parameter_values_by_name.${schema.name}`, - value, - ) - }} - /> - ))} -
- )} - - {/* Variables */} - {variables && ( -
-
-

Variables

-

- Enter the user variables -

-
+ + + + + {/* Parameters */} + {parameters && ( + + + {parameters.map((schema) => ( + { + await form.setFieldValue( + `parameter_values_by_name.${schema.name}`, + value, + ) + }} + /> + ))} + + + )} + + {/* Variables */} + {variables && ( + + + {variables.map((variable, index) => ( + { + await form.setFieldValue("user_variable_values." + index, { + name: variable.name, + value: value, + }) + }} + /> + ))} + + + )} + + {jobError && ( + +
+
Error during provisioning
+

+ Looks like we found an error during the template provisioning. You + can see the logs bellow. +

- - {variables.map((variable, index) => ( - { - await form.setFieldValue("user_variable_values." + index, { - name: variable.name, - value: value, - }) - }} - /> - ))} - + {jobError}
- )} - - {jobError && ( - -
-
Error during provisioning
-

- Looks like we found an error during the template provisioning. - You can see the logs bellow. -

- {jobError} -
+ + + )} - - - )} - - - - + + ) } -const useStyles = makeStyles((theme) => ({ - formSections: { - [theme.breakpoints.down("sm")]: { - gap: theme.spacing(8), - }, - }, - - formSection: { - display: "flex", - alignItems: "flex-start", - gap: theme.spacing(15), - - [theme.breakpoints.down("sm")]: { - flexDirection: "column", - gap: theme.spacing(2), - }, - }, - - formSectionInfo: { - width: 312, - flexShrink: 0, - position: "sticky", - top: theme.spacing(3), - - [theme.breakpoints.down("sm")]: { - width: "100%", - position: "initial", - }, - }, - - formSectionInfoTitle: { - fontSize: 20, - color: theme.palette.text.primary, - fontWeight: 400, - margin: 0, - marginBottom: theme.spacing(1), - }, - - formSectionInfoDescription: { - fontSize: 14, - color: theme.palette.text.secondary, - lineHeight: "160%", - margin: 0, - }, - - formSectionFields: { - width: "100%", - }, +const fillNameAndDisplayWithFilename = async ( + filename: string, + form: ReturnType>, +) => { + const [name, _extension] = filename.split(".") + await Promise.all([ + form.setFieldValue( + "name", + // Camel case will remove special chars and spaces + camelCase(name).toLowerCase(), + ), + form.setFieldValue("display_name", capitalize(name)), + ]) +} +const useStyles = makeStyles((theme) => ({ optionText: { fontSize: theme.spacing(2), color: theme.palette.text.primary, @@ -411,25 +350,3 @@ const useStyles = makeStyles((theme) => ({ fontSize: theme.spacing(2), }, })) - -const useFormFooterStyles = makeStyles((theme) => ({ - button: { - minWidth: theme.spacing(23), - - [theme.breakpoints.down("sm")]: { - width: "100%", - }, - }, - footer: { - display: "flex", - alignItems: "center", - justifyContent: "flex-start", - flexDirection: "row-reverse", - gap: theme.spacing(2), - - [theme.breakpoints.down("sm")]: { - flexDirection: "column", - gap: theme.spacing(1), - }, - }, -})) From d88b3d906fe00d67bbf580cb492ec262dc1b1e7b Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 6 Mar 2023 14:31:03 +0000 Subject: [PATCH 6/9] Add storybook --- .../CreateTemplateForm.stories.tsx | 50 +++++++++++++++++++ .../CreateTemplatePage/CreateTemplateForm.tsx | 2 +- .../CreateWorkspacePageView.stories.tsx | 37 +------------- site/src/testHelpers/entities.ts | 36 +++++++++++++ 4 files changed, 89 insertions(+), 36 deletions(-) create mode 100644 site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx new file mode 100644 index 0000000000000..9189b8fbf9f09 --- /dev/null +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx @@ -0,0 +1,50 @@ +import { ComponentMeta, Story } from "@storybook/react" +import { + MockParameterSchemas, + MockTemplateExample, + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, + MockTemplateVersionVariable3, + MockTemplateVersionVariable4, + MockTemplateVersionVariable5, +} from "testHelpers/entities" +import { + CreateTemplateForm, + CreateTemplateFormProps, +} from "./CreateTemplateForm" + +export default { + title: "components/CreateTemplateForm", + component: CreateTemplateForm, + args: { + isSubmitting: false, + }, +} as ComponentMeta + +const Template: Story = (args) => ( + +) + +export const Initial = Template.bind({}) +Initial.args = {} + +export const WithStarterTemplate = Template.bind({}) +WithStarterTemplate.args = { + starterTemplate: MockTemplateExample, +} + +export const WithParameters = Template.bind({}) +WithParameters.args = { + parameters: MockParameterSchemas, +} + +export const WithVariables = Template.bind({}) +WithVariables.args = { + variables: [ + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, + MockTemplateVersionVariable3, + MockTemplateVersionVariable4, + MockTemplateVersionVariable5, + ], +} diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index 96380431241b0..cd85d4439b3e4 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -66,7 +66,7 @@ const getInitialValues = (starterTemplate?: TemplateExample) => { } } -interface CreateTemplateFormProps { +export interface CreateTemplateFormProps { onCancel: () => void onSubmit: (data: CreateTemplateData) => void isSubmitting: boolean diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index be8f907ea7f64..3a600db95e6b4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -2,6 +2,7 @@ import { ComponentMeta, Story } from "@storybook/react" import { makeMockApiError, mockParameterSchema, + MockParameterSchemas, MockTemplate, MockTemplateVersionParameter1, MockTemplateVersionParameter2, @@ -34,41 +35,7 @@ export const Parameters = Template.bind({}) Parameters.args = { templates: [MockTemplate], selectedTemplate: MockTemplate, - templateSchema: [ - mockParameterSchema({ - name: "region", - default_source_value: "🏈 US Central", - description: "Where would you like your workspace to live?", - redisplay_value: true, - validation_contains: [ - "🏈 US Central", - "⚽ Brazil East", - "💶 EU West", - "🦘 Australia South", - ], - }), - mockParameterSchema({ - name: "instance_size", - default_source_value: "Big", - description: "How large should you instance be?", - validation_contains: ["Small", "Medium", "Big"], - redisplay_value: true, - }), - mockParameterSchema({ - name: "instance_size", - default_source_value: "Big", - description: "How large should your instance be?", - validation_contains: ["Small", "Medium", "Big"], - redisplay_value: true, - }), - mockParameterSchema({ - name: "disable_docker", - description: "Disable Docker?", - validation_value_type: "bool", - default_source_value: "false", - redisplay_value: true, - }), - ], + templateSchema: MockParameterSchemas, createWorkspaceErrors: {}, } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 373d2d192ca90..1a37ac193849d 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1462,6 +1462,42 @@ export const mockParameterSchema = ( } } +export const MockParameterSchemas: TypesGen.ParameterSchema[] = [ + mockParameterSchema({ + name: "region", + default_source_value: "🏈 US Central", + description: "Where would you like your workspace to live?", + redisplay_value: true, + validation_contains: [ + "🏈 US Central", + "⚽ Brazil East", + "💶 EU West", + "🦘 Australia South", + ], + }), + mockParameterSchema({ + name: "instance_size", + default_source_value: "Big", + description: "How large should you instance be?", + validation_contains: ["Small", "Medium", "Big"], + redisplay_value: true, + }), + mockParameterSchema({ + name: "instance_size", + default_source_value: "Big", + description: "How large should your instance be?", + validation_contains: ["Small", "Medium", "Big"], + redisplay_value: true, + }), + mockParameterSchema({ + name: "disable_docker", + description: "Disable Docker?", + validation_value_type: "bool", + default_source_value: "false", + redisplay_value: true, + }), +] + export const MockTemplateVersionGitAuth: TypesGen.TemplateVersionGitAuth = { id: "github", type: "github", From 23b58fa5367784d32ee139f0abdf5da5012aaed3 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 6 Mar 2023 14:49:48 +0000 Subject: [PATCH 7/9] Add error storybook --- .../CreateTemplateForm.stories.tsx | 314 ++++++++++++++++++ 1 file changed, 314 insertions(+) diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx index 9189b8fbf9f09..a25384e9f46f8 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.stories.tsx @@ -48,3 +48,317 @@ WithVariables.args = { MockTemplateVersionVariable5, ], } + +export const WithJobError = Template.bind({}) +WithJobError.args = { + jobError: + "template import provision for start: recv import provision: plan terraform: terraform plan: exit status 1", + logs: [ + { + id: 461061, + created_at: "2023-03-06T14:47:32.501Z", + log_source: "provisioner_daemon", + log_level: "info", + stage: "Adding README.md...", + output: "", + }, + { + id: 461062, + created_at: "2023-03-06T14:47:32.501Z", + log_source: "provisioner_daemon", + log_level: "info", + stage: "Setting up", + output: "", + }, + { + id: 461063, + created_at: "2023-03-06T14:47:32.528Z", + log_source: "provisioner_daemon", + log_level: "info", + stage: "Parsing template parameters", + output: "", + }, + { + id: 461064, + created_at: "2023-03-06T14:47:32.552Z", + log_source: "provisioner_daemon", + log_level: "info", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461065, + created_at: "2023-03-06T14:47:32.633Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461066, + created_at: "2023-03-06T14:47:32.633Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "Initializing the backend...", + }, + { + id: 461067, + created_at: "2023-03-06T14:47:32.71Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461068, + created_at: "2023-03-06T14:47:32.711Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "Initializing provider plugins...", + }, + { + id: 461069, + created_at: "2023-03-06T14:47:32.712Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: '- Finding coder/coder versions matching "~\u003e 0.6.12"...', + }, + { + id: 461070, + created_at: "2023-03-06T14:47:32.922Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: '- Finding hashicorp/aws versions matching "~\u003e 4.55"...', + }, + { + id: 461071, + created_at: "2023-03-06T14:47:33.132Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "- Installing hashicorp/aws v4.57.0...", + }, + { + id: 461072, + created_at: "2023-03-06T14:47:37.364Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "- Installed hashicorp/aws v4.57.0 (signed by HashiCorp)", + }, + { + id: 461073, + created_at: "2023-03-06T14:47:38.142Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "- Installing coder/coder v0.6.15...", + }, + { + id: 461074, + created_at: "2023-03-06T14:47:39.083Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "- Installed coder/coder v0.6.15 (signed by a HashiCorp partner, key ID 93C75807601AA0EC)", + }, + { + id: 461075, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461076, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "Partner and community providers are signed by their developers.", + }, + { + id: 461077, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "If you'd like to know more about provider signing, you can read about it here:", + }, + { + id: 461078, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "https://www.terraform.io/docs/cli/plugins/signing.html", + }, + { + id: 461079, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461080, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "Terraform has created a lock file .terraform.lock.hcl to record the provider", + }, + { + id: 461081, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "selections it made above. Include this file in your version control repository", + }, + { + id: 461082, + created_at: "2023-03-06T14:47:39.394Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "so that Terraform can guarantee to make the same selections by default when", + }, + { + id: 461083, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: 'you run "terraform init" in the future.', + }, + { + id: 461084, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461085, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "Terraform has been successfully initialized!", + }, + { + id: 461086, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461087, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + 'You may now begin working with Terraform. Try running "terraform plan" to see', + }, + { + id: 461088, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "any changes that are required for your infrastructure. All Terraform commands", + }, + { + id: 461089, + created_at: "2023-03-06T14:47:39.395Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "should now work.", + }, + { + id: 461090, + created_at: "2023-03-06T14:47:39.397Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461091, + created_at: "2023-03-06T14:47:39.397Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "If you ever set or change modules or backend configuration for Terraform,", + }, + { + id: 461092, + created_at: "2023-03-06T14:47:39.397Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: + "rerun this command to reinitialize your working directory. If you forget, other", + }, + { + id: 461093, + created_at: "2023-03-06T14:47:39.397Z", + log_source: "provisioner", + log_level: "debug", + stage: "Detecting persistent resources", + output: "commands will detect it and remind you to do so if necessary.", + }, + { + id: 461094, + created_at: "2023-03-06T14:47:39.431Z", + log_source: "provisioner", + log_level: "info", + stage: "Detecting persistent resources", + output: "Terraform 1.1.9", + }, + { + id: 461095, + created_at: "2023-03-06T14:47:43.759Z", + log_source: "provisioner", + log_level: "error", + stage: "Detecting persistent resources", + output: + "Error: configuring Terraform AWS Provider: no valid credential sources for Terraform AWS Provider found.\n\nPlease see https://registry.terraform.io/providers/hashicorp/aws\nfor more information about providing credentials.\n\nError: failed to refresh cached credentials, no EC2 IMDS role found, operation error ec2imds: GetMetadata, http response error StatusCode: 404, request to EC2 IMDS failed\n", + }, + { + id: 461096, + created_at: "2023-03-06T14:47:43.759Z", + log_source: "provisioner", + log_level: "error", + stage: "Detecting persistent resources", + output: "", + }, + { + id: 461097, + created_at: "2023-03-06T14:47:43.777Z", + log_source: "provisioner_daemon", + log_level: "info", + stage: "Cleaning Up", + output: "", + }, + ], +} From ccb05f80f428aad7e3cafbb3d718a7164a402e2f Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 6 Mar 2023 18:18:18 +0000 Subject: [PATCH 8/9] Add tests --- site/jest.setup.ts | 17 +++ site/package.json | 1 + .../RequireAuth/RequireAuth.test.tsx | 8 +- .../CreateTemplatePage/CreateTemplateForm.tsx | 5 +- .../CreateTemplatePage.test.tsx | 122 ++++++++++++++++++ .../TemplateSettingsPage.test.tsx | 38 +++--- .../TemplateVariablesPage.test.tsx | 5 +- site/src/testHelpers/entities.ts | 26 ++-- site/src/testHelpers/renderHelpers.tsx | 58 +++++---- .../createTemplate/createTemplateXService.ts | 16 ++- site/yarn.lock | 69 +++++++++- 11 files changed, 292 insertions(+), 73 deletions(-) create mode 100644 site/src/pages/CreateTemplatePage/CreateTemplatePage.test.tsx diff --git a/site/jest.setup.ts b/site/jest.setup.ts index d6eb950443481..2a93969f6def7 100644 --- a/site/jest.setup.ts +++ b/site/jest.setup.ts @@ -5,6 +5,7 @@ import { server } from "./src/testHelpers/server" import "jest-location-mock" import { TextEncoder, TextDecoder } from "util" import { Blob } from "buffer" +import { fetch, Request, Response, Headers } from "@remix-run/web-fetch" global.TextEncoder = TextEncoder // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Polyfill for jsdom @@ -12,6 +13,22 @@ global.TextDecoder = TextDecoder as any // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Polyfill for jsdom global.Blob = Blob as any +// From REMIX https://github.com/remix-run/react-router/blob/main/packages/react-router-dom/__tests__/setup.ts +if (!global.fetch) { + // Built-in lib.dom.d.ts expects `fetch(Request | string, ...)` but the web + // fetch API allows a URL so @remix-run/web-fetch defines + // `fetch(string | URL | Request, ...)` + // @ts-expect-error -- Polyfill for jsdom + global.fetch = fetch + // Same as above, lib.dom.d.ts doesn't allow a URL to the Request constructor + // @ts-expect-error -- Polyfill for jsdom + global.Request = Request + // web-std/fetch Response does not currently implement Response.error() + // @ts-expect-error -- Polyfill for jsdom + global.Response = Response + global.Headers = Headers +} + // Polyfill the getRandomValues that is used on utils/random.ts Object.defineProperty(global.self, "crypto", { value: { diff --git a/site/package.json b/site/package.json index 0460dea4957b7..21854772d21b5 100644 --- a/site/package.json +++ b/site/package.json @@ -36,6 +36,7 @@ "@material-ui/icons": "4.5.1", "@material-ui/lab": "4.0.0-alpha.42", "@monaco-editor/react": "4.4.6", + "@remix-run/web-fetch": "4.3.2", "@tanstack/react-query": "4.22.4", "@testing-library/react-hooks": "8.0.1", "@types/color-convert": "2.0.0", diff --git a/site/src/components/RequireAuth/RequireAuth.test.tsx b/site/src/components/RequireAuth/RequireAuth.test.tsx index a71dd7aedcbdd..ec2518d50085a 100644 --- a/site/src/components/RequireAuth/RequireAuth.test.tsx +++ b/site/src/components/RequireAuth/RequireAuth.test.tsx @@ -1,6 +1,5 @@ import { screen } from "@testing-library/react" import { rest } from "msw" -import { Route } from "react-router-dom" import { renderWithAuth } from "testHelpers/renderHelpers" import { server } from "testHelpers/server" @@ -20,7 +19,12 @@ describe("RequireAuth", () => { ) renderWithAuth(

Test

, { - routes: Setup} />, + extraRoutes: [ + { + path: "setup", + element:

Setup

, + }, + ], }) await screen.findByText("Setup") diff --git a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx index cd85d4439b3e4..5e0a8043e1cbe 100644 --- a/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx +++ b/site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx @@ -255,7 +255,10 @@ export const CreateTemplateForm: FC = ({ {/* Variables */} {variables && ( - + {variables.map((variable, index) => ( { + // Render with the example ID so we don't need to upload a file + const result = renderWithAuth(, { + route: `/templates/new?exampleId=${MockTemplateExample.id}`, + path: "/templates/new", + // We need this because after creation, the user will be redirected to here + extraRoutes: [{ path: "templates/:template", element: <> }], + }) + // It is lazy loaded, so we have to wait for it to be rendered to not get an + // act error + await screen.findByLabelText("Icon") + return result +} + +test("Create template with variables", async () => { + // Return pending when creating the first template version + jest.spyOn(API, "createTemplateVersion").mockResolvedValueOnce({ + ...MockTemplateVersion, + job: { + ...MockTemplateVersion.job, + status: "pending", + }, + }) + // Return an error requesting for template variables + jest.spyOn(API, "getTemplateVersion").mockResolvedValue({ + ...MockTemplateVersion, + job: { + ...MockTemplateVersion.job, + status: "failed", + error: "required template variables", + }, + }) + // Return the template variables + jest + .spyOn(API, "getTemplateVersionVariables") + .mockResolvedValue([ + MockTemplateVersionVariable1, + MockTemplateVersionVariable2, + MockTemplateVersionVariable3, + MockTemplateVersionVariable4, + MockTemplateVersionVariable5, + ]) + + // Render page, fill the name and submit + const { router } = await renderPage() + await userEvent.type(screen.getByLabelText(/Name/), "my-template") + await userEvent.click( + screen.getByRole("button", { name: /create template/i }), + ) + + // Wait for the variables form to be rendered and fill it + await screen.findByText(/Variables/) + // Type first variable + await userEvent.clear(screen.getByLabelText(/var.first_variable/)) + await userEvent.type( + screen.getByLabelText(/var.first_variable/), + "First value", + ) + // Type second variable + await userEvent.clear(screen.getByLabelText(/var.second_variable/)) + await userEvent.type(screen.getByLabelText(/var.second_variable/), "2") + // Select third variable on radio + await userEvent.click(screen.getByLabelText(/True/)) + // Type fourth variable + await userEvent.clear(screen.getByLabelText(/var.fourth_variable/)) + await userEvent.type( + screen.getByLabelText(/var.fourth_variable/), + "Fourth value", + ) + // Type fifth variable + await userEvent.clear(screen.getByLabelText(/var.fifth_variable/)) + await userEvent.type( + screen.getByLabelText(/var.fifth_variable/), + "Fifth value", + ) + // Setup the mock for the second template version creation before submit the form + jest.clearAllMocks() + jest + .spyOn(API, "createTemplateVersion") + .mockResolvedValue(MockTemplateVersion) + jest.spyOn(API, "createTemplate").mockResolvedValue(MockTemplate) + await userEvent.click( + screen.getByRole("button", { name: /create template/i }), + ) + + await waitFor(() => expect(API.createTemplate).toBeCalledTimes(1)) + expect(router.state.location.pathname).toEqual( + `/templates/${MockTemplate.name}`, + ) + expect(API.createTemplateVersion).toHaveBeenCalledWith(MockOrganization.id, { + file_id: MockProvisionerJob.file_id, + parameter_values: [], + provisioner: "terraform", + storage_method: "file", + tags: {}, + user_variable_values: [ + { name: "first_variable", value: "First value" }, + { name: "second_variable", value: "2" }, + { name: "third_variable", value: "true" }, + { name: "fourth_variable", value: "Fourth value" }, + { name: "fifth_variable", value: "Fifth value" }, + ], + }) +}) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx index 94d227e67d4b9..fba091e4d7c63 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx @@ -11,17 +11,6 @@ import i18next from "i18next" const { t } = i18next -const renderTemplateSettingsPage = async () => { - const renderResult = renderWithAuth(, { - route: `/templates/${MockTemplate.name}/settings`, - path: `/templates/:templateId/settings`, - }) - // Wait the form to be rendered - const label = t("nameLabel", { ns: "templateSettingsPage" }) - await screen.findAllByLabelText(label) - return renderResult -} - const validFormValues = { name: "Name", display_name: "A display name", @@ -31,6 +20,17 @@ const validFormValues = { allow_user_cancel_workspace_jobs: false, } +const renderTemplateSettingsPage = async () => { + renderWithAuth(, { + route: `/templates/${MockTemplate.name}/settings`, + path: `/templates/:template/settings`, + extraRoutes: [{ path: "templates/:template", element: <> }], + }) + // Wait the form to be rendered + const label = t("nameLabel", { ns: "templateSettingsPage" }) + await screen.findAllByLabelText(label) +} + const fillAndSubmitForm = async ({ name, display_name, @@ -109,17 +109,13 @@ describe("TemplateSettingsPage", () => { }) await fillAndSubmitForm(validFormValues) - expect(screen.getByDisplayValue(1)).toBeInTheDocument() // the default_ttl_ms await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1)) - - await waitFor(() => - expect(API.updateTemplateMeta).toBeCalledWith( - "test-template", - expect.objectContaining({ - ...validFormValues, - default_ttl_ms: 3600000, // the default_ttl_ms to ms - }), - ), + expect(API.updateTemplateMeta).toBeCalledWith( + "test-template", + expect.objectContaining({ + ...validFormValues, + default_ttl_ms: 3600000, // the default_ttl_ms to ms + }), ) }) diff --git a/site/src/pages/TemplateVariablesPage/TemplateVariablesPage.test.tsx b/site/src/pages/TemplateVariablesPage/TemplateVariablesPage.test.tsx index 240ef507a86d2..135f980fd1238 100644 --- a/site/src/pages/TemplateVariablesPage/TemplateVariablesPage.test.tsx +++ b/site/src/pages/TemplateVariablesPage/TemplateVariablesPage.test.tsx @@ -13,7 +13,6 @@ import * as API from "api/api" import i18next from "i18next" import TemplateVariablesPage from "./TemplateVariablesPage" import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter" -import { Route } from "react-router-dom" import * as router from "react-router" const navigate = jest.fn() @@ -35,9 +34,7 @@ const renderTemplateVariablesPage = () => { return renderWithAuth(, { route: `/templates/${MockTemplate.name}/variables`, path: `/templates/:template/variables`, - routes: ( - }> - ), + extraRoutes: [{ path: `/templates/${MockTemplate.name}`, element: <> }], }) } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 1a37ac193849d..3baafe214f829 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -8,6 +8,13 @@ import { Permissions } from "xServices/auth/authXService" import { TemplateVersionFiles } from "util/templateVersion" import { FileTree } from "util/filetree" +export const MockOrganization: TypesGen.Organization = { + id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", + name: "Test Organization", + created_at: "", + updated_at: "", +} + export const MockTemplateDAUResponse: TypesGen.TemplateDAUsResponse = { entries: [ { date: "2022-08-27T00:00:00Z", amount: 1 }, @@ -140,7 +147,7 @@ export const MockUser: TypesGen.User = { email: "test@coder.com", created_at: "", status: "active", - organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], + organization_ids: [MockOrganization.id], roles: [MockOwnerRole], avatar_url: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4", last_seen_at: "", @@ -152,7 +159,7 @@ export const MockUserAdmin: TypesGen.User = { email: "test@coder.com", created_at: "", status: "active", - organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], + organization_ids: [MockOrganization.id], roles: [MockUserAdminRole], avatar_url: "", last_seen_at: "", @@ -164,7 +171,7 @@ export const MockUser2: TypesGen.User = { email: "test2@coder.com", created_at: "", status: "active", - organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], + organization_ids: [MockOrganization.id], roles: [], avatar_url: "", last_seen_at: "2022-09-14T19:12:21Z", @@ -176,19 +183,12 @@ export const SuspendedMockUser: TypesGen.User = { email: "iamsuspendedsad!@coder.com", created_at: "", status: "suspended", - organization_ids: ["fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0"], + organization_ids: [MockOrganization.id], roles: [], avatar_url: "", last_seen_at: "", } -export const MockOrganization: TypesGen.Organization = { - id: "test-org", - name: "Test Organization", - created_at: "", - updated_at: "", -} - export const MockProvisioner: TypesGen.ProvisionerDaemon = { created_at: "", id: "test-provisioner", @@ -201,7 +201,7 @@ export const MockProvisionerJob: TypesGen.ProvisionerJob = { created_at: "", id: "test-provisioner-job", status: "succeeded", - file_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", + file_id: MockOrganization.id, completed_at: "2022-05-17T17:39:01.382927298Z", tags: {}, } @@ -1240,7 +1240,7 @@ export const MockAuditLog: TypesGen.AuditLog = { id: "fbd2116a-8961-4954-87ae-e4575bd29ce0", request_id: "53bded77-7b9d-4e82-8771-991a34d759f9", time: "2022-05-19T16:45:57.122Z", - organization_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", + organization_id: MockOrganization.id, ip: "127.0.0.1", user_agent: '"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36"', diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index 6190247f26cf9..fbd36eae72540 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -11,10 +11,10 @@ import { i18n } from "i18n" import { FC, ReactElement } from "react" import { I18nextProvider } from "react-i18next" import { - MemoryRouter, - Route, - Routes, unstable_HistoryRouter as HistoryRouter, + RouterProvider, + createMemoryRouter, + RouteObject, } from "react-router-dom" import { RequireAuth } from "../components/RequireAuth/RequireAuth" import { MockUser } from "./entities" @@ -35,41 +35,45 @@ export const render = (component: ReactElement): RenderResult => { return wrappedRender({component}) } -type RenderWithAuthResult = RenderResult & { user: typeof MockUser } +type RenderWithAuthOptions = { + // The current URL, /workspaces/123 + route?: string + // The route path, /workspaces/:workspaceId + path?: string + // Extra routes to add to the router. It is helpful when having redirecting + // routes or multiple routes during the test flow + extraRoutes?: RouteObject[] +} -/** - * - * @param ui The component to render and test - * @param options Can contain `route`, the URL to use, such as /users/user1, and `path`, - * such as /users/:userid. When there are no parameters, they are the same and you can just supply `route`. - */ export function renderWithAuth( - ui: JSX.Element, - { - route = "/", - path, - routes, - }: { route?: string; path?: string; routes?: JSX.Element } = {}, -): RenderWithAuthResult { + element: JSX.Element, + { path = "/", route = "/", extraRoutes = [] }: RenderWithAuthOptions = {}, +) { + const routes: RouteObject[] = [ + { + element: , + children: [ + { + element: , + children: [{ path, element }, ...extraRoutes], + }, + ], + }, + ] + + const router = createMemoryRouter(routes, { initialEntries: [route] }) + const renderResult = wrappedRender( - - - }> - }> - - - - {routes} - - + , ) return { user: MockUser, + router, ...renderResult, } } diff --git a/site/src/xServices/createTemplate/createTemplateXService.ts b/site/src/xServices/createTemplate/createTemplateXService.ts index e256a71870ce7..8e6dc3b779caa 100644 --- a/site/src/xServices/createTemplate/createTemplateXService.ts +++ b/site/src/xServices/createTemplate/createTemplateXService.ts @@ -11,6 +11,7 @@ import { import { CreateTemplateVersionRequest, ParameterSchema, + ProvisionerJob, ProvisionerJobLog, Template, TemplateExample, @@ -358,15 +359,18 @@ export const createTemplateMachine = throw new Error("Version not defined") } - let status = version.job.status - while (["pending", "running"].includes(status)) { + let job = version.job + while (isPendingOrRunning(job)) { version = await getTemplateVersion(version.id) - status = version.job.status + job = version.job + // Delay the verification in two seconds to not overload the server // with too many requests Maybe at some point we could have a // websocket for template version Also, preferred doing this way to // avoid a new state since we don't need to reflect it on the UI - await delay(2_000) + if (isPendingOrRunning(job)) { + await delay(2_000) + } } return version }, @@ -478,3 +482,7 @@ const isMissingVariables = (version: TemplateVersion) => { version.job.error.includes("required template variables"), ) } + +const isPendingOrRunning = (job: ProvisionerJob) => { + return job.status === "pending" || job.status === "running" +} diff --git a/site/yarn.lock b/site/yarn.lock index 9c267ebe9265f..63d936e046b4f 100644 --- a/site/yarn.lock +++ b/site/yarn.lock @@ -1844,6 +1844,41 @@ resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.0.1.tgz#88d7ac31811ab0cef14aaaeae2a0474923b278bc" integrity sha512-eBV5rvW4dRFOU1eajN7FmYxjAIVz/mRHgUE9En9mBn6m3mulK3WTR5C3iQhL9MZ14rWAq+xOlEaCkDiW0/heOg== +"@remix-run/web-blob@^3.0.4": + version "3.0.4" + resolved "https://registry.yarnpkg.com/@remix-run/web-blob/-/web-blob-3.0.4.tgz#99c67b9d0fb641bd0c07d267fd218ae5aa4ae5ed" + integrity sha512-AfegzZvSSDc+LwnXV+SwROTrDtoLiPxeFW+jxgvtDAnkuCX1rrzmVJ6CzqZ1Ai0bVfmJadkG5GxtAfYclpPmgw== + dependencies: + "@remix-run/web-stream" "^1.0.0" + web-encoding "1.1.5" + +"@remix-run/web-fetch@4.3.2": + version "4.3.2" + resolved "https://registry.yarnpkg.com/@remix-run/web-fetch/-/web-fetch-4.3.2.tgz#193758bb7a301535540f0e3a86c743283f81cf56" + integrity sha512-aRNaaa0Fhyegv/GkJ/qsxMhXvyWGjPNgCKrStCvAvV1XXphntZI0nQO/Fl02LIQg3cGL8lDiOXOS1gzqDOlG5w== + dependencies: + "@remix-run/web-blob" "^3.0.4" + "@remix-run/web-form-data" "^3.0.3" + "@remix-run/web-stream" "^1.0.3" + "@web3-storage/multipart-parser" "^1.0.0" + abort-controller "^3.0.0" + data-uri-to-buffer "^3.0.1" + mrmime "^1.0.0" + +"@remix-run/web-form-data@^3.0.3": + version "3.0.4" + resolved "https://registry.yarnpkg.com/@remix-run/web-form-data/-/web-form-data-3.0.4.tgz#18c5795edaffbc88c320a311766dc04644125bab" + integrity sha512-UMF1jg9Vu9CLOf8iHBdY74Mm3PUvMW8G/XZRJE56SxKaOFWGSWlfxfG+/a3boAgHFLTkP7K4H1PxlRugy1iQtw== + dependencies: + web-encoding "1.1.5" + +"@remix-run/web-stream@^1.0.0", "@remix-run/web-stream@^1.0.3": + version "1.0.3" + resolved "https://registry.yarnpkg.com/@remix-run/web-stream/-/web-stream-1.0.3.tgz#3284a6a45675d1455c4d9c8f31b89225c9006438" + integrity sha512-wlezlJaA5NF6SsNMiwQnnAW6tnPzQ5I8qk0Y0pSohm0eHKa2FQ1QhEKLVVcDDu02TmkfHgnux0igNfeYhDOXiA== + dependencies: + web-streams-polyfill "^3.1.1" + "@sinclair/typebox@^0.24.1": version "0.24.51" resolved "https://registry.yarnpkg.com/@sinclair/typebox/-/typebox-0.24.51.tgz#645f33fe4e02defe26f2f5c0410e1c094eac7f5f" @@ -3623,6 +3658,11 @@ magic-string "^0.26.2" react-refresh "^0.14.0" +"@web3-storage/multipart-parser@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@web3-storage/multipart-parser/-/multipart-parser-1.0.0.tgz#6b69dc2a32a5b207ba43e556c25cc136a56659c4" + integrity sha512-BEO6al7BYqcnfX15W2cnGR+Q566ACXAT9UQykORCWW80lmkpWsnEob6zJS1ZVBKsSJC8+7vJkHwlp+lXG1UCdw== + "@webassemblyjs/ast@1.11.1": version "1.11.1" resolved "https://registry.yarnpkg.com/@webassemblyjs/ast/-/ast-1.11.1.tgz#2bfd767eae1a6996f432ff7e8d7fc75679c0b6a7" @@ -4055,6 +4095,13 @@ abbrev@1: resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8" integrity sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q== +abort-controller@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/abort-controller/-/abort-controller-3.0.0.tgz#eaf54d53b62bae4138e809ca225c8439a6efb392" + integrity sha512-h8lQ8tacZYnR3vNQTgibj+tODHI5/+l06Au2Pcriv/Gmet0eaj4TwWH41sO9wnHDiQsEj19q0drzdWdeAHtweg== + dependencies: + event-target-shim "^5.0.0" + accepts@~1.3.5, accepts@~1.3.8: version "1.3.8" resolved "https://registry.yarnpkg.com/accepts/-/accepts-1.3.8.tgz#0bf0be125b67014adcb0b0921e62db7bffe16b2e" @@ -5887,6 +5934,11 @@ damerau-levenshtein@^1.0.8: resolved "https://registry.yarnpkg.com/damerau-levenshtein/-/damerau-levenshtein-1.0.8.tgz#b43d286ccbd36bc5b2f7ed41caf2d0aba1f8a6e7" integrity sha512-sdQSFB7+llfUcQHUQO3+B8ERRj0Oa4w9POWMI/puGtuf7gFywGmkaLCElnudfTiKZV+NvHqL0ifzdrI8Ro7ESA== +data-uri-to-buffer@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/data-uri-to-buffer/-/data-uri-to-buffer-3.0.1.tgz#594b8973938c5bc2c33046535785341abc4f3636" + integrity sha512-WboRycPNsVw3B3TL559F7kuBUM4d8CgMEvk6xEJlOp7OBPjt6G7z8WMWlD2rOFZLk6OYfFIUGsCOWzcQH9K2og== + data-urls@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/data-urls/-/data-urls-2.0.0.tgz#156485a72963a970f5d5821aaf642bef2bf2db9b" @@ -6978,6 +7030,11 @@ event-loop-spinner@^2.0.0, event-loop-spinner@^2.1.0: dependencies: tslib "^2.1.0" +event-target-shim@^5.0.0: + version "5.0.1" + resolved "https://registry.yarnpkg.com/event-target-shim/-/event-target-shim-5.0.1.tgz#5d4d3ebdf9583d63a5333ce2deb7480ab2b05789" + integrity sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ== + events@^3.0.0, events@^3.2.0, events@^3.3.0: version "3.3.0" resolved "https://registry.yarnpkg.com/events/-/events-3.3.0.tgz#31a95ad0a924e2d2c419a813aeb2c4e878ea7400" @@ -10862,6 +10919,11 @@ mri@^1.1.0: resolved "https://registry.yarnpkg.com/mri/-/mri-1.2.0.tgz#6721480fec2a11a4889861115a48b6cbe7cc8f0b" integrity sha512-tzzskb3bG8LvYGFF/mDTpq3jpI6Q9wc3LEmBaghu+DdCssd1FakN7Bc0hVNmEyGq1bq3RgfkCb3cmQLpNPOroA== +mrmime@^1.0.0: + version "1.0.1" + resolved "https://registry.yarnpkg.com/mrmime/-/mrmime-1.0.1.tgz#5f90c825fad4bdd41dc914eff5d1a8cfdaf24f27" + integrity sha512-hzzEagAgDyoU1Q6yg5uI+AorQgdvMCur3FcKf7NhMKWsaYg+RnbTyHRa/9IlLF9rf455MOCtcqqrQQ83pPP7Uw== + ms@2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/ms/-/ms-2.0.0.tgz#5608aeadfc00be6c2901df5f9861788de0d597c8" @@ -14653,7 +14715,7 @@ wcwidth@^1.0.1: dependencies: defaults "^1.0.3" -web-encoding@^1.1.5: +web-encoding@1.1.5, web-encoding@^1.1.5: version "1.1.5" resolved "https://registry.yarnpkg.com/web-encoding/-/web-encoding-1.1.5.tgz#fc810cf7667364a6335c939913f5051d3e0c4864" integrity sha512-HYLeVCdJ0+lBYV2FvNZmv3HJ2Nt0QYXqZojk3d9FJOLkwnuhzM9tmamh8d7HPM8QqjKH8DeHkFTx+CFlWpZZDA== @@ -14667,6 +14729,11 @@ web-namespaces@^1.0.0: resolved "https://registry.yarnpkg.com/web-namespaces/-/web-namespaces-1.1.4.tgz#bc98a3de60dadd7faefc403d1076d529f5e030ec" integrity sha512-wYxSGajtmoP4WxfejAPIr4l0fVh+jeMXZb08wNc0tMg6xsfZXj3cECqIK0G7ZAqUq0PP8WlMDtaOGVBTAWztNw== +web-streams-polyfill@^3.1.1: + version "3.2.1" + resolved "https://registry.yarnpkg.com/web-streams-polyfill/-/web-streams-polyfill-3.2.1.tgz#71c2718c52b45fd49dbeee88634b3a60ceab42a6" + integrity sha512-e0MO3wdXWKrLbL0DgGnUV7WHVuw9OUvL4hjgnPkIeEvESk74gAITi5G606JtZPp39cd8HA9VQzCIvA49LpPN5Q== + webidl-conversions@^3.0.0: version "3.0.1" resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-3.0.1.tgz#24534275e2a7bc6be7bc86611cc16ae0a5654871" From 9870b6891306d84fc3e3e50c0cfddb56dc06f677 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 6 Mar 2023 18:20:58 +0000 Subject: [PATCH 9/9] Fix require auth test --- site/src/components/RequireAuth/RequireAuth.test.tsx | 2 +- site/src/testHelpers/renderHelpers.tsx | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/site/src/components/RequireAuth/RequireAuth.test.tsx b/site/src/components/RequireAuth/RequireAuth.test.tsx index ec2518d50085a..a6abeead64b59 100644 --- a/site/src/components/RequireAuth/RequireAuth.test.tsx +++ b/site/src/components/RequireAuth/RequireAuth.test.tsx @@ -19,7 +19,7 @@ describe("RequireAuth", () => { ) renderWithAuth(

Test

, { - extraRoutes: [ + nonAuthenticatedRoutes: [ { path: "setup", element:

Setup

, diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index fbd36eae72540..80845a70682d2 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -43,11 +43,18 @@ type RenderWithAuthOptions = { // Extra routes to add to the router. It is helpful when having redirecting // routes or multiple routes during the test flow extraRoutes?: RouteObject[] + // The same as extraRoutes but for routes that don't require authentication + nonAuthenticatedRoutes?: RouteObject[] } export function renderWithAuth( element: JSX.Element, - { path = "/", route = "/", extraRoutes = [] }: RenderWithAuthOptions = {}, + { + path = "/", + route = "/", + extraRoutes = [], + nonAuthenticatedRoutes = [], + }: RenderWithAuthOptions = {}, ) { const routes: RouteObject[] = [ { @@ -59,6 +66,7 @@ export function renderWithAuth( }, ], }, + ...nonAuthenticatedRoutes, ] const router = createMemoryRouter(routes, { initialEntries: [route] })