Skip to content

feat: add locked TTL field to template meta #8020

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 20, 2023
Merged

feat: add locked TTL field to template meta #8020

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2ea96c5
feat: add locked TTL field to template meta
sreya Jun 14, 2023
939d8ab
add frontend
sreya Jun 14, 2023
f89fb32
lint
sreya Jun 14, 2023
2b61d3f
i18
sreya Jun 14, 2023
9528ca2
fix a query
sreya Jun 14, 2023
1cf2bbb
fix migration
sreya Jun 14, 2023
38303a2
fix enterprise scheduler
sreya Jun 15, 2023
c5ac779
fix migration
sreya Jun 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add frontend
  • Loading branch information
@sreya
sreya committed Jun 20, 2023
commit 939d8ab40672fdf764d25ec84db3cb13e9e4f3d2
62 changes: 61 additions & 1 deletion ...s/TemplateSettingsPage/TemplateSchedulePage/TemplateScheduleForm/TemplateScheduleForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const MS_HOUR_CONVERSION = 3600000
const MS_DAY_CONVERSION = 86400000
const FAILURE_CLEANUP_DEFAULT = 7
const INACTIVITY_CLEANUP_DEFAULT = 180
const LOCKED_CLEANUP_DEFAULT = 30

export interface TemplateScheduleForm {
template: Template
Expand Down Expand Up @@ -65,13 +66,18 @@ export const TemplateScheduleForm: FC<TemplateScheduleForm> = ({
inactivity_ttl_ms: allowAdvancedScheduling
? template.inactivity_ttl_ms / MS_DAY_CONVERSION
: 0,
locked_ttl_ms: allowAdvancedScheduling
? template.locked_ttl_ms / MS_DAY_CONVERSION
: 0,

allow_user_autostart: template.allow_user_autostart,
allow_user_autostop: template.allow_user_autostop,
failure_cleanup_enabled:
allowAdvancedScheduling && Boolean(template.failure_ttl_ms),
inactivity_cleanup_enabled:
allowAdvancedScheduling && Boolean(template.inactivity_ttl_ms),
locked_cleanup_enabled:
allowAdvancedScheduling && Boolean(template.locked_ttl_ms),
},
validationSchema,
onSubmit: () => {
Expand Down Expand Up @@ -114,6 +120,9 @@ export const TemplateScheduleForm: FC<TemplateScheduleForm> = ({
inactivity_ttl_ms: form.values.inactivity_ttl_ms
? form.values.inactivity_ttl_ms * MS_DAY_CONVERSION
: undefined,
locked_ttl_ms: form.values.locked_ttl_ms
? form.values.locked_ttl_ms * MS_DAY_CONVERSION
: undefined,

allow_user_autostart: form.values.allow_user_autostart,
allow_user_autostop: form.values.allow_user_autostop,
Expand Down Expand Up @@ -158,6 +167,25 @@ export const TemplateScheduleForm: FC<TemplateScheduleForm> = ({
}
}

const handleToggleLockedCleanup = async (e: ChangeEvent) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Since we now have 3 toggling functions, we might be able to refactor into one function that takes two form values, e.g. locked_cleanup_enabled and locked_ttl_ms and appropriately toggles TTL.

form.handleChange(e)
if (!form.values.locked_cleanup_enabled) {
// fill failure_ttl_ms with defaults
await form.setValues({
...form.values,
locked_cleanup_enabled: true,
locked_ttl_ms: LOCKED_CLEANUP_DEFAULT,
})
} else {
// clear failure_ttl_ms
await form.setValues({
...form.values,
locked_cleanup_enabled: false,
locked_ttl_ms: 0,
})
}
}

return (
<HorizontalForm
onSubmit={form.handleSubmit}
Expand Down Expand Up @@ -298,7 +326,7 @@ export const TemplateScheduleForm: FC<TemplateScheduleForm> = ({
</FormSection>
<FormSection
title="Inactivity Cleanup"
description="When enabled, Coder will automatically delete workspaces that are in an inactive state after a specified number of days."
description="When enabled, Coder will lock workspaces that have not been accessed after a specified number of days."
>
<FormFields>
<FormControlLabel
Expand Down Expand Up @@ -330,6 +358,38 @@ export const TemplateScheduleForm: FC<TemplateScheduleForm> = ({
/>
</FormFields>
</FormSection>
<FormSection
title="Locked Cleanup"
description="When enabled, Coder will permanently delete workspaces that have been locked for a specified number of days."
>
<FormFields>
<FormControlLabel
control={
<Switch
name="lockedCleanupEnabled"
checked={form.values.locked_cleanup_enabled}
onChange={handleToggleLockedCleanup}
/>
}
label="Enable Locked Cleanup"
/>
<TextField
{...getFieldHelpers(
"locked_ttl_ms",
<TTLHelperText
translationName="lockedTTLHelperText"
ttl={form.values.locked_ttl_ms}
/>,
)}
disabled={isSubmitting || !form.values.locked_cleanup_enabled}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this field also be disabled if Inactivity Cleanup is disabled? Not sure that it makes sense to have one without the other, although perhaps we'll have manual locking down the road:
Screenshot 2023-06-15 at 3 17 22 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that probably makes sense, we can fix it in a follow up PR.

fullWidth
inputProps={{ min: 0, step: "any" }}
label="Time until cleanup (days)"
type="number"
aria-label="Locked Cleanup"
/>
</FormFields>
</FormSection>
</>
)}
<InactivityDialog
Expand Down
15 changes: 15 additions & 0 deletions .../src/pages/TemplateSettingsPage/TemplateSchedulePage/TemplateScheduleForm/formHelpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import i18next from "i18next"
export interface TemplateScheduleFormValues extends UpdateTemplateMeta {
failure_cleanup_enabled: boolean
inactivity_cleanup_enabled: boolean
locked_cleanup_enabled: boolean
}

const MAX_TTL_DAYS = 7
Expand Down Expand Up @@ -63,6 +64,20 @@ export const getValidationSchema = (): Yup.AnyObjectSchema =>
}
},
),
locked_ttl_ms: Yup.number()
.min(0, "Locked cleanup days must not be less than 0.")
.test(
"positive-if-enabled",
"Locked cleanup days must be greater than zero when enabled.",
function (value) {
const parent = this.parent as TemplateScheduleFormValues
if (parent.locked_cleanup_enabled) {
return Boolean(value)
} else {
return true
}
},
),
allow_user_autostart: Yup.boolean(),
allow_user_autostop: Yup.boolean(),
})
40 changes: 39 additions & 1 deletion site/src/pages/TemplateSettingsPage/TemplateSchedulePage/TemplateSchedulePage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const validFormValues = {
max_ttl_ms: 2,
failure_ttl_ms: 7,
inactivity_ttl_ms: 180,
locked_ttl_ms: 30,
}

const renderTemplateSchedulePage = async () => {
Expand All @@ -37,11 +38,13 @@ const fillAndSubmitForm = async ({
max_ttl_ms,
failure_ttl_ms,
inactivity_ttl_ms,
locked_ttl_ms,
}: {
default_ttl_ms: number
max_ttl_ms: number
failure_ttl_ms: number
inactivity_ttl_ms: number
locked_ttl_ms: number
}) => {
const user = userEvent.setup()
const defaultTtlLabel = t("defaultTtlLabel", { ns: "templateSettingsPage" })
Expand All @@ -64,6 +67,11 @@ const fillAndSubmitForm = async ({
})
await user.type(inactivityTtlField, inactivity_ttl_ms.toString())

const lockedTtlField = screen.getByRole("checkbox", {
name: /Locked Cleanup/i,
})
await user.type(lockedTtlField, locked_ttl_ms.toString())

const submitButton = await screen.findByText(
FooterFormLanguage.defaultSubmitLabel,
)
Expand Down Expand Up @@ -111,7 +119,7 @@ describe("TemplateSchedulePage", () => {
)
})

test("failure and inactivity ttl converted to and from days", async () => {
test("failure, inactivity, and locked ttl converted to and from days", async () => {
await renderTemplateSchedulePage()

jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({
Expand All @@ -127,6 +135,7 @@ describe("TemplateSchedulePage", () => {
expect.objectContaining({
failure_ttl_ms: validFormValues.failure_ttl_ms * 86400000,
inactivity_ttl_ms: validFormValues.inactivity_ttl_ms * 86400000,
locked_ttl_ms: validFormValues.locked_ttl_ms * 86400000,
}),
),
)
Expand Down Expand Up @@ -218,4 +227,33 @@ describe("TemplateSchedulePage", () => {
"Inactivity cleanup days must not be less than 0.",
)
})

it("allows a locked ttl of 7 days", () => {
const values: UpdateTemplateMeta = {
...validFormValues,
locked_ttl_ms: 86400000 * 7,
}
const validate = () => getValidationSchema().validateSync(values)
expect(validate).not.toThrowError()
})

it("allows a locked ttl of 0", () => {
const values: UpdateTemplateMeta = {
...validFormValues,
locked_ttl_ms: 0,
}
const validate = () => getValidationSchema().validateSync(values)
expect(validate).not.toThrowError()
})

it("disallows a negative inactivity ttl", () => {
const values: UpdateTemplateMeta = {
...validFormValues,
locked_ttl_ms: -1,
}
const validate = () => getValidationSchema().validateSync(values)
expect(validate).toThrowError(
"Locked cleanup days must not be less than 0.",
)
})
})
1 change: 1 addition & 0 deletions site/src/testHelpers/entities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ export const MockTemplate: TypesGen.Template = {
allow_user_cancel_workspace_jobs: true,
failure_ttl_ms: 0,
inactivity_ttl_ms: 0,
locked_ttl_ms: 0,
allow_user_autostart: false,
allow_user_autostop: false,
}
Expand Down