- Notifications
You must be signed in to change notification settings - Fork927
Template settings fixes/kira pilot#3668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
cf85bfe
8b8882f
7b5a78c
45a41d1
3bab482
94abca9
b7ce020
1eda40c
67b65d0
ac67ccb
5f74e95
db10782
15e1ad2
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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.", | ||
errorTtlMax: "Please enter a limit that is less than or equal to 168 hours (7 days).", | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
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 */, Language.errorTtlMax) | ||
.test("positive-if-auto-stop", Language.errorNoStop, function (value) { | ||
const parent = this.parent as WorkspaceScheduleFormValues | ||
if (parent.autoStopEnabled) { | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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", | ||
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.`, | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
} | ||
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(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, | ||
// on display, convert from ms => hours | ||
max_ttl_ms: template.max_ttl_ms / MS_HOUR_CONVERSION, | ||
icon: template.icon, | ||
}, | ||
validationSchema, | ||
onSubmit: (formData) => { | ||
// on submit, convert from hours => ms | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
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")} | ||
disabled={isSubmitting} | ||
fullWidth | ||
inputProps={{ min: 0, step: 1 }} | ||
label={Language.maxTtlLabel} | ||
variant="outlined" | ||
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} /> | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, 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() | ||
jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({ | ||
...MockTemplate, | ||
...validFormValues, | ||
}) | ||
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() | ||
}) | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
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) | ||
Kira-Pilot marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
}) | ||
}) |