Skip to content

Commit 57c202d

Browse files
Kira-Pilotjohnstcn
Kira-Pilot
and
johnstcn
authored
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>
1 parent 4e3b212 commit 57c202d

File tree

4 files changed

+120
-29
lines changed

4 files changed

+120
-29
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ describe("validationSchema", () => {
161161
ttl: 24 * 7 + 1,
162162
}
163163
const validate = () => validationSchema.validateSync(values)
164-
expect(validate).toThrowError("ttl must be less than or equal to 168")
164+
expect(validate).toThrowError(Language.errorTtlMax)
165165
})
166166
})
167167

‎site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ dayjs.extend(relativeTime)
3636
dayjs.extend(timezone)
3737

3838
export const Language = {
39-
errorNoDayOfWeek: "Must set at least one day of week if auto-start is enabled",
40-
errorNoTime: "Start time is required when auto-start is enabled",
41-
errorTime: "Time must be in HH:mm format (24 hours)",
42-
errorTimezone: "Invalid timezone",
43-
errorNoStop: "Time until shutdown must be greater than zero when auto-stop is enabled",
39+
errorNoDayOfWeek: "Must set at least one day of week if auto-start is enabled.",
40+
errorNoTime: "Start time is required when auto-start is enabled.",
41+
errorTime: "Time must be in HH:mm format (24 hours).",
42+
errorTimezone: "Invalid timezone.",
43+
errorNoStop: "Time until shutdown must be greater than zero when auto-stop is enabled.",
44+
errorTtlMax: "Please enter a limit that is less than or equal to 168 hours (7 days).",
4445
daysOfWeekLabel: "Days of Week",
4546
daySundayLabel: "Sunday",
4647
dayMondayLabel: "Monday",
@@ -159,7 +160,7 @@ export const validationSchema = Yup.object({
159160
ttl: Yup.number()
160161
.integer()
161162
.min(0)
162-
.max(24 * 7 /* 7 days */)
163+
.max(24 * 7 /* 7 days */, Language.errorTtlMax)
163164
.test("positive-if-auto-stop", Language.errorNoStop, function (value) {
164165
const parent = this.parent as WorkspaceScheduleFormValues
165166
if (parent.autoStopEnabled) {

‎site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import InputAdornment from "@material-ui/core/InputAdornment"
55
import Popover from "@material-ui/core/Popover"
66
import { makeStyles } from "@material-ui/core/styles"
77
import TextField from "@material-ui/core/TextField"
8+
import Typography from "@material-ui/core/Typography"
89
import { Template, UpdateTemplateMeta } from "api/typesGenerated"
910
import { OpenDropdown } from "components/DropdownArrows/DropdownArrows"
1011
import { FormFooter } from "components/FormFooter/FormFooter"
@@ -20,16 +21,25 @@ export const Language = {
2021
descriptionLabel: "Description",
2122
maxTtlLabel: "Auto-stop limit",
2223
iconLabel: "Icon",
23-
// This is the same from the CLI on https://github.com/coder/coder/blob/546157b63ef9204658acf58cb653aa9936b70c49/cli/templateedit.go#L59
24-
maxTtlHelperText: "Edit the template maximum time before shutdown in seconds",
2524
formAriaLabel: "Template settings form",
2625
selectEmoji: "Select emoji",
26+
ttlMaxError: "Please enter a limit that is less than or equal to 168 hours (7 days).",
27+
descriptionMaxError: "Please enter a description that is less than or equal to 128 characters.",
28+
ttlHelperText: (ttl: number): string =>
29+
`Workspaces created from this template may not remain running longer than ${ttl} hours.`,
2730
}
2831

32+
const MAX_DESCRIPTION_CHAR_LIMIT = 128
33+
const MAX_TTL_DAYS = 7
34+
const MS_HOUR_CONVERSION = 3600000
35+
2936
export const validationSchema = Yup.object({
3037
name: nameValidator(Language.nameLabel),
31-
description: Yup.string(),
32-
max_ttl_ms: Yup.number(),
38+
description: Yup.string().max(MAX_DESCRIPTION_CHAR_LIMIT, Language.descriptionMaxError),
39+
max_ttl_ms: Yup.number()
40+
.integer()
41+
.min(0)
42+
.max(24 * MAX_TTL_DAYS /* 7 days in hours */, Language.ttlMaxError),
3343
})
3444

3545
export interface TemplateSettingsForm {
@@ -55,11 +65,18 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
5565
initialValues: {
5666
name: template.name,
5767
description: template.description,
58-
max_ttl_ms: template.max_ttl_ms,
68+
// on display, convert from ms => hours
69+
max_ttl_ms: template.max_ttl_ms / MS_HOUR_CONVERSION,
5970
icon: template.icon,
6071
},
6172
validationSchema,
62-
onSubmit,
73+
onSubmit: (formData) => {
74+
// on submit, convert from hours => ms
75+
onSubmit({
76+
...formData,
77+
max_ttl_ms: formData.max_ttl_ms ? formData.max_ttl_ms * MS_HOUR_CONVERSION : undefined,
78+
})
79+
},
6380
initialTouched,
6481
})
6582
const getFieldHelpers = getFormHelpersWithError<UpdateTemplateMeta>(form, error)
@@ -148,19 +165,21 @@ export const TemplateSettingsForm: FC<TemplateSettingsForm> = ({
148165

149166
<TextField
150167
{...getFieldHelpers("max_ttl_ms")}
151-
helperText={Language.maxTtlHelperText}
152168
disabled={isSubmitting}
153169
fullWidth
154170
inputProps={{ min: 0, step: 1 }}
155171
label={Language.maxTtlLabel}
156172
variant="outlined"
157-
// Display seconds from ms
158-
value={form.values.max_ttl_ms ? form.values.max_ttl_ms / 1000 : ""}
159-
// Convert ms to seconds
160-
onChange={(event) =>
161-
form.setFieldValue("max_ttl_ms", Number(event.currentTarget.value) * 1000)
162-
}
173+
type="number"
163174
/>
175+
{/* If a value for max_ttl_ms has been entered and
176+
there are no validation errors for that field, display helper text.
177+
We do not use the MUI helper-text prop because it overrides the validation error */}
178+
{form.values.max_ttl_ms && !form.errors.max_ttl_ms && (
179+
<Typography variant="subtitle2">
180+
{Language.ttlHelperText(form.values.max_ttl_ms)}
181+
</Typography>
182+
)}
164183
</Stack>
165184

166185
<FormFooter onCancel={onCancel} isLoading={isSubmitting} />

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

Lines changed: 80 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { UpdateTemplateMeta } from "api/typesGenerated"
55
import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter"
66
import { MockTemplate } from "../../testHelpers/entities"
77
import { renderWithAuth } from "../../testHelpers/renderHelpers"
8-
import { Language as FormLanguage } from "./TemplateSettingsForm"
8+
import { Language as FormLanguage, validationSchema } from "./TemplateSettingsForm"
99
import { TemplateSettingsPage } from "./TemplateSettingsPage"
1010
import { Language as ViewLanguage } from "./TemplateSettingsPageView"
1111

@@ -19,6 +19,13 @@ const renderTemplateSettingsPage = async () => {
1919
return renderResult
2020
}
2121

22+
const validFormValues = {
23+
name: "Name",
24+
description: "A description",
25+
icon: "A string",
26+
max_ttl_ms: 1,
27+
}
28+
2229
const fillAndSubmitForm = async ({
2330
name,
2431
description,
@@ -55,18 +62,82 @@ describe("TemplateSettingsPage", () => {
5562
it("succeeds", async () => {
5663
await renderTemplateSettingsPage()
5764

58-
const newTemplateSettings = {
59-
name: "edited-template-name",
60-
description: "Edited description",
61-
max_ttl_ms: 4000,
62-
icon: "/icon/code.svg",
63-
}
6465
jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({
6566
...MockTemplate,
66-
...newTemplateSettings,
67+
...validFormValues,
68+
})
69+
await fillAndSubmitForm(validFormValues)
70+
71+
await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1))
72+
})
73+
74+
test("ttl is converted to and from hours", async () => {
75+
await renderTemplateSettingsPage()
76+
77+
jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({
78+
...MockTemplate,
79+
...validFormValues,
6780
})
68-
await fillAndSubmitForm(newTemplateSettings)
6981

82+
await fillAndSubmitForm(validFormValues)
83+
expect(screen.getByDisplayValue(1)).toBeInTheDocument() // the max_ttl_ms
7084
await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1))
85+
86+
await waitFor(() =>
87+
expect(API.updateTemplateMeta).toBeCalledWith(
88+
"test-template",
89+
expect.objectContaining({
90+
...validFormValues,
91+
max_ttl_ms: 3600000, // the max_ttl_ms to ms
92+
}),
93+
),
94+
)
95+
})
96+
97+
it("allows a ttl of 7 days", () => {
98+
const values: UpdateTemplateMeta = {
99+
...validFormValues,
100+
max_ttl_ms: 24 * 7,
101+
}
102+
const validate = () => validationSchema.validateSync(values)
103+
expect(validate).not.toThrowError()
104+
})
105+
106+
it("allows ttl of 0", () => {
107+
const values: UpdateTemplateMeta = {
108+
...validFormValues,
109+
max_ttl_ms: 0,
110+
}
111+
const validate = () => validationSchema.validateSync(values)
112+
expect(validate).not.toThrowError()
113+
})
114+
115+
it("disallows a ttl of 7 days + 1 hour", () => {
116+
const values: UpdateTemplateMeta = {
117+
...validFormValues,
118+
max_ttl_ms: 24 * 7 + 1,
119+
}
120+
const validate = () => validationSchema.validateSync(values)
121+
expect(validate).toThrowError(FormLanguage.ttlMaxError)
122+
})
123+
124+
it("allows a description of 128 chars", () => {
125+
const values: UpdateTemplateMeta = {
126+
...validFormValues,
127+
description:
128+
"Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port",
129+
}
130+
const validate = () => validationSchema.validateSync(values)
131+
expect(validate).not.toThrowError()
132+
})
133+
134+
it("disallows a description of 128 + 1 chars", () => {
135+
const values: UpdateTemplateMeta = {
136+
...validFormValues,
137+
description:
138+
"Nam quis nulla. Integer malesuada. In in enim a arcu imperdiet malesuada. Sed vel lectus. Donec odio urna, tempus molestie, port a",
139+
}
140+
const validate = () => validationSchema.validateSync(values)
141+
expect(validate).toThrowError(FormLanguage.descriptionMaxError)
71142
})
72143
})

0 commit comments

Comments
 (0)