Skip to content

UX: Workspace Schedule Form is too top-down restricted #1958

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

Jump to bottom
Closed
Tracked by #1939
greyscaled opened this issue Jun 1, 2022 · 0 comments · Fixed by #2008
Closed
Tracked by #1939

UX: Workspace Schedule Form is too top-down restricted #1958

greyscaled opened this issue Jun 1, 2022 · 0 comments · Fixed by #2008
Assignees
@greyscaled
Labels
site Area: frontend dashboard
Milestone
Community MVP

Comments

@greyscaled
Copy link
Contributor

User Story 👥

A real user story (thanks @ammario 🎉):

Allow setting "Days of Week" when "Start time" is unset
This one confused me. We shouldn't assume that users will fill out the form in top to bottom order.

Implementation Notes

Under the hood, the form has logic like:

coder/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx

Line 192 in b85de3e

disabled={form.isSubmitting || isLoading || !form.values.startTime}

At the parent level, whereby the form values are transformed into an API request, nothing should change:

coder/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx

Lines 24 to 92 in b85de3e

export const formValuesToAutoStartRequest = (
values: WorkspaceScheduleFormValues,
): TypesGen.UpdateWorkspaceAutostartRequest => {
if (!values.startTime) {
return {
schedule: "",
}
}
const [HH, mm] = values.startTime.split(":")
// Note: Space after CRON_TZ if timezone is defined
const preparedTZ = values.timezone ? `CRON_TZ=${values.timezone} ` : ""
const makeCronString = (dow: string) => `${preparedTZ}${mm} ${HH} * * ${dow}`
const days = [
values.sunday,
values.monday,
values.tuesday,
values.wednesday,
values.thursday,
values.friday,
values.saturday,
]
const isEveryDay = days.every((day) => day)
const isMonThroughFri =
!values.sunday &&
values.monday &&
values.tuesday &&
values.wednesday &&
values.thursday &&
values.friday &&
!values.saturday &&
!values.sunday
// Handle special cases, falling through to comma-separation
if (isEveryDay) {
return {
schedule: makeCronString("*"),
}
} else if (isMonThroughFri) {
return {
schedule: makeCronString("1-5"),
}
} else {
const dow = days.reduce((previous, current, idx) => {
if (!current) {
return previous
} else {
const prefix = previous ? "," : ""
return previous + prefix + idx
}
}, "")
return {
schedule: makeCronString(dow),
}
}
}
export const formValuesToTTLRequest = (values: WorkspaceScheduleFormValues): TypesGen.UpdateWorkspaceTTLRequest => {
return {
// minutes to nanoseconds
ttl: values.ttl ? values.ttl * 60 * 60 * 1000 * 1_000_000 : undefined,
}
}

@greyscaled greyscaled added bug 🐛 site Area: frontend dashboard labels Jun 1, 2022
@greyscaled greyscaled mentioned this issue Jun 1, 2022
Closed
@greyscaled greyscaled self-assigned this Jun 2, 2022
@misskniss misskniss added this to the Community MVP milestone Jun 2, 2022
@misskniss misskniss mentioned this issue Jun 2, 2022
Closed
90 tasks
greyscaled added a commit that referenced this issue Jun 2, 2022
@greyscaled
fix: ws schedule top-down restriction
4c3a655 
Resolves: #1958

Summary:

The workspace schedule form no longer disables certain fields based on
whether or not a start time is filled out. Instead, we validate that a
start time is provided if any of the days are checked.
@greyscaled greyscaled mentioned this issue Jun 2, 2022
Merged
greyscaled added a commit that referenced this issue Jun 3, 2022
@greyscaled
fix: ws schedule top-down restriction (#2008)
c272057 
Resolves: #1958

Summary:

The workspace schedule form no longer disables certain fields based on
whether or not a start time is filled out. Instead, we validate that a
start time is provided if any of the days are checked.
@greyscaled greyscaled mentioned this issue Jun 3, 2022
Closed
kylecarbs pushed a commit that referenced this issue Jun 10, 2022
@greyscaled @kylecarbs
fix: ws schedule top-down restriction (#2008)
a92aea9 
Resolves: #1958

Summary:

The workspace schedule form no longer disables certain fields based on
whether or not a start time is filled out. Instead, we validate that a
start time is provided if any of the days are checked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@greyscaled greyscaled

Labels
site Area: frontend dashboard
Projects
None yet
Milestone
Community MVP
Development

Successfully merging a pull request may close this issue.

fix: ws schedule top-down restriction coder/coder
2 participants
@misskniss @greyscaled