Template settings fixes/kira pilot (#3668)

* using hours instead of seconds

* checking out

* added ttl tests

* added description validation  and tests

* added some helper text

* fix typing

* Update site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx

Co-authored-by: Cian Johnston <cian@coder.com>

* ran prettier

* added ttl of 0 test

* typo

* PR feedback

Co-authored-by: Cian Johnston <cian@coder.com>

site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.test.ts

@@ -161,7 +161,7 @@ describe("validationSchema", () => {
       ttl: 24 * 7 + 1,
     }
     const validate = () => validationSchema.validateSync(values)
-    expect(validate).toThrowError("ttl must be less than or equal to 168")
+    expect(validate).toThrowError(Language.errorTtlMax)
   })
 })
 

site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx

@@ -36,11 +36,12 @@ dayjs.extend(relativeTime)
 dayjs.extend(timezone)
 
 export const Language = {
-  errorNoDayOfWeek: "Must set at least one day of week if auto-start is enabled",
-  errorNoTime: "Start time is required when auto-start is enabled",
-  errorTime: "Time must be in HH:mm format (24 hours)",
-  errorTimezone: "Invalid timezone",
-  errorNoStop: "Time until shutdown must be greater than zero when auto-stop is enabled",
+  errorNoDayOfWeek: "Must set at least one day of week if auto-start is enabled.",
+  errorNoTime: "Start time is required when auto-start is enabled.",
+  errorTime: "Time must be in HH:mm format (24 hours).",
+  errorTimezone: "Invalid timezone.",
+  errorNoStop: "Time until shutdown must be greater than zero when auto-stop is enabled.",
+  errorTtlMax: "Please enter a limit that is less than or equal to 168 hours (7 days).",
   daysOfWeekLabel: "Days of Week",
   daySundayLabel: "Sunday",
   dayMondayLabel: "Monday",
@@ -159,7 +160,7 @@ export const validationSchema = Yup.object({
   ttl: Yup.number()
     .integer()
     .min(0)
-    .max(24 * 7 /* 7 days */)
+    .max(24 * 7 /* 7 days */, Language.errorTtlMax)
     .test("positive-if-auto-stop", Language.errorNoStop, function (value) {
       const parent = this.parent as WorkspaceScheduleFormValues
       if (parent.autoStopEnabled) {

site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx

@@ -5,6 +5,7 @@ import InputAdornment from "@material-ui/core/InputAdornment"
 import Popover from "@material-ui/core/Popover"
 import { makeStyles } from "@material-ui/core/styles"
 import TextField from "@material-ui/core/TextField"
+import Typography from "@material-ui/core/Typography"
 import { Template, UpdateTemplateMeta } from "api/typesGenerated"
 import { OpenDropdown } from "components/DropdownArrows/DropdownArrows"
 import { FormFooter } from "components/FormFooter/FormFooter"
@@ -20,16 +21,25 @@ export const Language = {
   descriptionLabel: "Description",
   maxTtlLabel: "Auto-stop limit",
   iconLabel: "Icon",
-  // This is the same from the CLI on https://github.com/coder/coder/blob/546157b63ef9204658acf58cb653aa9936b70c49/cli/templateedit.go#L59
-  maxTtlHelperText: "Edit the template maximum time before shutdown in seconds",
   formAriaLabel: "Template settings form",
   selectEmoji: "Select emoji",
+  ttlMaxError: "Please enter a limit that is less than or equal to 168 hours (7 days).",
+  descriptionMaxError: "Please enter a description that is less than or equal to 128 characters.",
+  ttlHelperText: (ttl: number): string =>
+    `Workspaces created from this template may not remain running longer than ${ttl} hours.`,
 }
 
+const MAX_DESCRIPTION_CHAR_LIMIT = 128
+const MAX_TTL_DAYS = 7
+const MS_HOUR_CONVERSION = 3600000
+
 export const validationSchema = Yup.object({
   name: nameValidator(Language.nameLabel),
-  description: Yup.string(),
-  max_ttl_ms: Yup.number(),
+  description: Yup.string().max(MAX_DESCRIPTION_CHAR_LIMIT, Language.descriptionMaxError),
+  max_ttl_ms: Yup.number()
+    .integer()
+    .min(0)
+    .max(24 * MAX_TTL_DAYS /* 7 days in hours */, Language.ttlMaxError),
 })
 
 export interface TemplateSettingsForm {
@@ -55,11 +65,18 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
     initialValues: {
       name: template.name,
       description: template.description,
-      max_ttl_ms: template.max_ttl_ms,
+      // on display, convert from ms => hours
+      max_ttl_ms: template.max_ttl_ms / MS_HOUR_CONVERSION,
       icon: template.icon,
     },
     validationSchema,
-    onSubmit,
+    onSubmit: (formData) => {
+      // on submit, convert from hours => ms
+      onSubmit({
+        ...formData,
+        max_ttl_ms: formData.max_ttl_ms ? formData.max_ttl_ms * MS_HOUR_CONVERSION : undefined,
+      })
+    },
     initialTouched,
   })
   const getFieldHelpers = getFormHelpersWithError<UpdateTemplateMeta>(form, error)
@@ -148,19 +165,21 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
 
         <TextField
           {...getFieldHelpers("max_ttl_ms")}
-          helperText={Language.maxTtlHelperText}
           disabled={isSubmitting}
           fullWidth
           inputProps={{ min: 0, step: 1 }}
           label={Language.maxTtlLabel}
           variant="outlined"
-          // Display seconds from ms
-          value={form.values.max_ttl_ms ? form.values.max_ttl_ms / 1000 : ""}
-          // Convert ms to seconds
-          onChange={(event) =>
-            form.setFieldValue("max_ttl_ms", Number(event.currentTarget.value) * 1000)
-          }
+          type="number"
         />
+        {/* If a value for max_ttl_ms has been entered and
+        there are no validation errors for that field, display helper text.
+        We do not use the MUI helper-text prop because it overrides the validation error */}
+        {form.values.max_ttl_ms && !form.errors.max_ttl_ms && (
+          <Typography variant="subtitle2">
+            {Language.ttlHelperText(form.values.max_ttl_ms)}
+          </Typography>
+        )}
       </Stack>
 
       <FormFooter onCancel={onCancel} isLoading={isSubmitting} />

site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx

@@ -5,7 +5,7 @@ import { UpdateTemplateMeta } from "api/typesGenerated"
 import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter"
 import { MockTemplate } from "../../testHelpers/entities"
 import { renderWithAuth } from "../../testHelpers/renderHelpers"
-import { Language as FormLanguage } from "./TemplateSettingsForm"
+import { Language as FormLanguage, validationSchema } from "./TemplateSettingsForm"
 import { TemplateSettingsPage } from "./TemplateSettingsPage"
 import { Language as ViewLanguage } from "./TemplateSettingsPageView"
 
@@ -19,6 +19,13 @@ const renderTemplateSettingsPage = async () => {
   return renderResult
 }
 
+const validFormValues = {
+  name: "Name",
+  description: "A description",
+  icon: "A string",
+  max_ttl_ms: 1,
+}
+
 const fillAndSubmitForm = async ({
   name,
   description,
@@ -55,18 +62,82 @@ describe("TemplateSettingsPage", () => {
   it("succeeds", async () => {
     await renderTemplateSettingsPage()
 
-    const newTemplateSettings = {
-      name: "edited-template-name",
-      description: "Edited description",
-      max_ttl_ms: 4000,
-      icon: "/icon/code.svg",
-    }
     jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({
       ...MockTemplate,
-      ...newTemplateSettings,
+      ...validFormValues,
     })
-    await fillAndSubmitForm(newTemplateSettings)
+    await fillAndSubmitForm(validFormValues)
 
     await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1))
   })
+
+  test("ttl is converted to and from hours", async () => {
+    await renderTemplateSettingsPage()
+
+    jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({
+      ...MockTemplate,
+      ...validFormValues,
+    })
+
+    await fillAndSubmitForm(validFormValues)
+    expect(screen.getByDisplayValue(1)).toBeInTheDocument() // the max_ttl_ms
+    await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1))
+
+    await waitFor(() =>
+      expect(API.updateTemplateMeta).toBeCalledWith(
+        "test-template",
+        expect.objectContaining({
+          ...validFormValues,
+          max_ttl_ms: 3600000, // the max_ttl_ms to ms
+        }),
+      ),
+    )
+  })
+
+  it("allows a ttl of 7 days", () => {
+    const values: UpdateTemplateMeta = {
+      ...validFormValues,
+      max_ttl_ms: 24 * 7,
+    }
+    const validate = () => validationSchema.validateSync(values)
+    expect(validate).not.toThrowError()
+  })
+
+  it("allows ttl of 0", () => {
+    const values: UpdateTemplateMeta = {
+      ...validFormValues,
+      max_ttl_ms: 0,
+    }
+    const validate = () => validationSchema.validateSync(values)
+    expect(validate).not.toThrowError()
+  })
+
+  it("disallows a ttl of 7 days + 1 hour", () => {
+    const values: UpdateTemplateMeta = {
+      ...validFormValues,
+      max_ttl_ms: 24 * 7 + 1,
+    }
+    const validate = () => validationSchema.validateSync(values)
+    expect(validate).toThrowError(FormLanguage.ttlMaxError)
+  })
+
+  it("allows a description of 128 chars", () => {
+    const values: UpdateTemplateMeta = {
+      ...validFormValues,
+      description:
+        "Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port",
+    }
+    const validate = () => validationSchema.validateSync(values)
+    expect(validate).not.toThrowError()
+  })
+
+  it("disallows a description of 128 + 1 chars", () => {
+    const values: UpdateTemplateMeta = {
+      ...validFormValues,
+      description:
+        "Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port a",
+    }
+    const validate = () => validationSchema.validateSync(values)
+    expect(validate).toThrowError(FormLanguage.descriptionMaxError)
+  })
 })