fix: stop time incrementer on workspace page (#5406)

* Check template default ttl while setting max

* Lint

* Remove template default from max ttl consideration

* Finish removing template

* Fix disabling buttons

* Simplify, wip

* Handle NaN

* Format

* Add aria labels

* Explain NaN handling

* Use more realistic storybook args
This commit is contained in:
Presley Pizzo
2022-12-21 10:44:18 -05:00
committed by GitHub
parent 8d9528545a
commit 0eb25306ad
9 changed files with 54 additions and 211 deletions
@@ -1,7 +1,5 @@
import { action } from "@storybook/addon-actions"
import { Story } from "@storybook/react"
import dayjs from "dayjs"
import { canExtendDeadline, canReduceDeadline } from "util/schedule"
import * as Mocks from "../../testHelpers/entities"
import { Workspace, WorkspaceErrors, WorkspaceProps } from "./Workspace"
@@ -22,18 +20,8 @@ Running.args = {
onDeadlinePlus: () => {
// do nothing, this is just for storybook
},
deadlineMinusEnabled: () => {
return canReduceDeadline(dayjs(Mocks.MockWorkspace.latest_build.deadline))
},
deadlinePlusEnabled: () => {
return canExtendDeadline(
dayjs(Mocks.MockWorkspace.latest_build.deadline),
Mocks.MockWorkspace,
Mocks.MockTemplate,
)
},
maxDeadlineDecrease: 1000,
maxDeadlineIncrease: 1000,
maxDeadlineDecrease: 0,
maxDeadlineIncrease: 24,
},
workspace: Mocks.MockWorkspace,
handleStart: action("start"),
@@ -35,8 +35,6 @@ export interface WorkspaceProps {
scheduleProps: {
onDeadlinePlus: (hours: number) => void
onDeadlineMinus: (hours: number) => void
deadlinePlusEnabled: () => boolean
deadlineMinusEnabled: () => boolean
maxDeadlineIncrease: number
maxDeadlineDecrease: number
}
@@ -131,8 +129,6 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
workspace={workspace}
onDeadlineMinus={scheduleProps.onDeadlineMinus}
onDeadlinePlus={scheduleProps.onDeadlinePlus}
deadlineMinusEnabled={scheduleProps.deadlineMinusEnabled}
deadlinePlusEnabled={scheduleProps.deadlinePlusEnabled}
maxDeadlineDecrease={scheduleProps.maxDeadlineDecrease}
maxDeadlineIncrease={scheduleProps.maxDeadlineIncrease}
canUpdateWorkspace={canUpdateWorkspace}
@@ -19,13 +19,14 @@ export const EditHours = ({
const styles = useStyles()
return (
<form onSubmit={() => handleSubmit(hours)}>
// hours is NaN when user deletes the value, so treat it as 0
<form onSubmit={() => handleSubmit(Number.isNaN(hours) ? 0 : hours)}>
<Stack direction="row" alignItems="baseline" spacing={1}>
<TextField
className={styles.inputField}
inputProps={{ min: 0, max, step: 1 }}
label={t("workspaceScheduleButton.hours")}
value={hours}
value={hours.toString()}
onChange={(e) => setHours(parseInt(e.target.value))}
type="number"
/>
@@ -48,8 +48,6 @@ export interface WorkspaceScheduleButtonProps {
workspace: Workspace
onDeadlinePlus: (hours: number) => void
onDeadlineMinus: (hours: number) => void
deadlineMinusEnabled: () => boolean
deadlinePlusEnabled: () => boolean
maxDeadlineIncrease: number
maxDeadlineDecrease: number
canUpdateWorkspace: boolean
@@ -63,8 +61,6 @@ export const WorkspaceScheduleButton: React.FC<
workspace,
onDeadlinePlus,
onDeadlineMinus,
deadlinePlusEnabled,
deadlineMinusEnabled,
maxDeadlineDecrease,
maxDeadlineIncrease,
canUpdateWorkspace,
@@ -75,6 +71,8 @@ export const WorkspaceScheduleButton: React.FC<
const [editMode, setEditMode] = useState<EditMode>("off")
const id = isOpen ? "schedule-popover" : undefined
const styles = useStyles({ editMode })
const deadlinePlusEnabled = maxDeadlineIncrease >= 1
const deadlineMinusEnabled = maxDeadlineDecrease >= 1
const onClose = () => {
setIsOpen(false)
@@ -107,8 +105,9 @@ export const WorkspaceScheduleButton: React.FC<
<span className={styles.actions}>
<IconButton
className={styles.subtractButton}
aria-label="Subtract hours from deadline"
size="small"
disabled={!deadlineMinusEnabled()}
disabled={!deadlineMinusEnabled}
onClick={() => {
setEditMode("subtract")
}}
@@ -121,8 +120,9 @@ export const WorkspaceScheduleButton: React.FC<
</IconButton>
<IconButton
className={styles.addButton}
aria-label="Add hours to deadline"
size="small"
disabled={!deadlinePlusEnabled()}
disabled={!deadlinePlusEnabled}
onClick={() => {
setEditMode("add")
}}
@@ -6,6 +6,7 @@ import { Helmet } from "react-helmet-async"
import { useTranslation } from "react-i18next"
import { useNavigate } from "react-router-dom"
import {
getDeadline,
getMaxDeadline,
getMaxDeadlineChange,
getMinDeadline,
@@ -37,10 +38,9 @@ export const WorkspaceReadyPage = ({
quotaState,
workspaceSend,
}: WorkspaceReadyPageProps): JSX.Element => {
const [bannerState, bannerSend] = useActor(
const [_, bannerSend] = useActor(
workspaceState.children["scheduleBannerMachine"],
)
const deadline = bannerState.context.deadline
const xServices = useContext(XServiceContext)
const featureVisibility = useSelector(
xServices.entitlementsXService,
@@ -61,6 +61,7 @@ export const WorkspaceReadyPage = ({
if (workspace === undefined) {
throw Error("Workspace is undefined")
}
const deadline = getDeadline(workspace)
const canUpdateWorkspace = Boolean(permissions?.updateWorkspace)
const { t } = useTranslation("workspacePage")
const favicon = getFaviconByStatus(workspace.latest_build)
@@ -101,18 +102,11 @@ export const WorkspaceReadyPage = ({
hours,
})
},
deadlineMinusEnabled: () => !bannerState.matches("atMinDeadline"),
deadlinePlusEnabled: () => !bannerState.matches("atMaxDeadline"),
maxDeadlineDecrease: deadline
? getMaxDeadlineChange(deadline, getMinDeadline())
: 0,
maxDeadlineIncrease:
deadline && template
? getMaxDeadlineChange(
getMaxDeadline(workspace, template),
deadline,
)
: 0,
maxDeadlineDecrease: getMaxDeadlineChange(deadline, getMinDeadline()),
maxDeadlineIncrease: getMaxDeadlineChange(
getMaxDeadline(workspace),
deadline,
),
}}
isUpdating={workspaceState.hasTag("updating")}
workspace={workspace}
+6 -73
View File
@@ -2,11 +2,9 @@ import dayjs from "dayjs"
import duration from "dayjs/plugin/duration"
import { emptySchedule } from "pages/WorkspaceSchedulePage/schedule"
import { emptyTTL } from "pages/WorkspaceSchedulePage/ttl"
import { Template, Workspace } from "../api/typesGenerated"
import { Workspace } from "../api/typesGenerated"
import * as Mocks from "../testHelpers/entities"
import {
canExtendDeadline,
canReduceDeadline,
deadlineExtensionMax,
deadlineExtensionMin,
extractTimezone,
@@ -19,6 +17,7 @@ import {
dayjs.extend(duration)
const now = dayjs()
const startTime = dayjs(Mocks.MockWorkspaceBuild.updated_at)
describe("util/schedule", () => {
describe("stripTimezone", () => {
@@ -43,38 +42,16 @@ describe("util/schedule", () => {
})
describe("maxDeadline", () => {
// Given: a workspace built from a template with a max deadline equal to 25 hours which isn't really possible
const workspace: Workspace = {
...Mocks.MockWorkspace,
latest_build: {
...Mocks.MockWorkspaceBuild,
deadline: now.add(8, "hours").utc().format(),
deadline: startTime.add(8, "hours").utc().format(),
},
}
describe("given a template with 25 hour max ttl", () => {
it("should be never be greater than global max deadline", () => {
const template: Template = {
...Mocks.MockTemplate,
default_ttl_ms: 25 * 60 * 60 * 1000,
}
// Then: deadlineMinusDisabled should be falsy
const delta = getMaxDeadline(workspace, template).diff(now)
expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
})
})
describe("given a template with 4 hour max ttl", () => {
it("should be never be greater than global max deadline", () => {
const template: Template = {
...Mocks.MockTemplate,
default_ttl_ms: 4 * 60 * 60 * 1000,
}
// Then: deadlineMinusDisabled should be falsy
const delta = getMaxDeadline(workspace, template).diff(now)
expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
})
it("should be 24 hours from the workspace start time", () => {
const delta = getMaxDeadline(workspace).diff(startTime)
expect(delta).toEqual(deadlineExtensionMax.asMilliseconds())
})
})
@@ -85,50 +62,6 @@ describe("minDeadline", () => {
})
})
describe("canExtendDeadline", () => {
it("should be falsy if the deadline is more than 24 hours in the future", () => {
expect(
canExtendDeadline(
dayjs().add(25, "hours"),
Mocks.MockWorkspace,
Mocks.MockTemplate,
),
).toBeFalsy()
})
it("should be falsy if the deadline is more than the template max_ttl", () => {
const tooFarAhead = dayjs().add(
dayjs.duration(Mocks.MockTemplate.default_ttl_ms, "milliseconds"),
)
expect(
canExtendDeadline(tooFarAhead, Mocks.MockWorkspace, Mocks.MockTemplate),
).toBeFalsy()
})
it("should be truth if the deadline is within the template max_ttl", () => {
const okDeadline = dayjs().add(
dayjs.duration(Mocks.MockTemplate.default_ttl_ms / 2, "milliseconds"),
)
expect(
canExtendDeadline(okDeadline, Mocks.MockWorkspace, Mocks.MockTemplate),
).toBeFalsy()
})
})
describe("canReduceDeadline", () => {
it("should be falsy if the deadline is 30 minutes or less in the future", () => {
expect(canReduceDeadline(dayjs())).toBeFalsy()
expect(canReduceDeadline(dayjs().add(1, "minutes"))).toBeFalsy()
expect(canReduceDeadline(dayjs().add(29, "minutes"))).toBeFalsy()
expect(canReduceDeadline(dayjs().add(30, "minutes"))).toBeFalsy()
})
it("should be truthy if the deadline is 30 minutes or more in the future", () => {
expect(canReduceDeadline(dayjs().add(31, "minutes"))).toBeTruthy()
expect(canReduceDeadline(dayjs().add(100, "years"))).toBeTruthy()
})
})
describe("getMaxDeadlineChange", () => {
it("should return the number of hours you can add before hitting the max deadline", () => {
const deadline = dayjs()
+4 -26
View File
@@ -7,7 +7,7 @@ import duration from "dayjs/plugin/duration"
import relativeTime from "dayjs/plugin/relativeTime"
import timezone from "dayjs/plugin/timezone"
import utc from "dayjs/plugin/utc"
import { Template, Workspace } from "../api/typesGenerated"
import { Workspace } from "../api/typesGenerated"
import { isWorkspaceOn } from "./workspace"
import { WorkspaceScheduleFormValues } from "components/WorkspaceScheduleForm/WorkspaceScheduleForm"
import { AutoStop } from "pages/WorkspaceSchedulePage/ttl"
@@ -127,27 +127,18 @@ export const deadlineExtensionMin = dayjs.duration(30, "minutes")
export const deadlineExtensionMax = dayjs.duration(24, "hours")
/**
* Depends on the time the workspace was last updated, the template config,
* and a global constant.
* Depends on the time the workspace was last updated and a global constant.
* @param ws workspace
* @param tpl template
* @returns the latest datetime at which the workspace can be automatically shut down.
*/
export function getMaxDeadline(
ws: Workspace | undefined,
tpl: Template,
): dayjs.Dayjs {
export function getMaxDeadline(ws: Workspace | undefined): dayjs.Dayjs {
// note: we count runtime from updated_at as started_at counts from the start of
// the workspace build process, which can take a while.
if (ws === undefined) {
throw Error("Cannot calculate max deadline because workspace is undefined")
}
const startedAt = dayjs(ws.latest_build.updated_at)
const maxTemplateDeadline = startedAt.add(
dayjs.duration(tpl.default_ttl_ms, "milliseconds"),
)
const maxGlobalDeadline = startedAt.add(deadlineExtensionMax)
return dayjs.min(maxTemplateDeadline, maxGlobalDeadline)
return startedAt.add(deadlineExtensionMax)
}
/**
@@ -158,18 +149,6 @@ export function getMinDeadline(): dayjs.Dayjs {
return dayjs().add(deadlineExtensionMin)
}
export function canExtendDeadline(
deadline: dayjs.Dayjs,
workspace: Workspace,
template: Template,
): boolean {
return deadline < getMaxDeadline(workspace, template)
}
export function canReduceDeadline(deadline: dayjs.Dayjs): boolean {
return deadline > getMinDeadline()
}
export const getDeadline = (workspace: Workspace): dayjs.Dayjs =>
dayjs(workspace.latest_build.deadline).utc()
@@ -177,7 +156,6 @@ export const getDeadline = (workspace: Workspace): dayjs.Dayjs =>
* Get number of hours you can add or subtract to the current deadline before hitting the max or min deadline.
* @param deadline
* @param workspace
* @param template
* @returns number, in hours
*/
export const getMaxDeadlineChange = (
@@ -470,7 +470,6 @@ export const workspaceMachine = createMachine(
src: workspaceScheduleBannerMachine,
data: {
workspace: (context: WorkspaceContext) => context.workspace,
template: (context: WorkspaceContext) => context.template,
},
},
},
@@ -3,16 +3,10 @@
* presented as an Alert/banner, for reactively updating a workspace schedule.
*/
import { getErrorMessage } from "api/errors"
import { Template, Workspace } from "api/typesGenerated"
import { Workspace } from "api/typesGenerated"
import dayjs from "dayjs"
import minMax from "dayjs/plugin/minMax"
import {
canExtendDeadline,
canReduceDeadline,
getDeadline,
getMaxDeadline,
getMinDeadline,
} from "util/schedule"
import { getDeadline, getMaxDeadline, getMinDeadline } from "util/schedule"
import { ActorRefFrom, assign, createMachine } from "xstate"
import * as API from "../../api/api"
import {
@@ -29,8 +23,6 @@ export const Language = {
export interface WorkspaceScheduleBannerContext {
workspace: Workspace
template: Template
deadline?: dayjs.Dayjs
}
export type WorkspaceScheduleBannerEvent =
@@ -60,51 +52,27 @@ export const workspaceScheduleBannerMachine = createMachine(
events: {} as WorkspaceScheduleBannerEvent,
context: {} as WorkspaceScheduleBannerContext,
},
initial: "initialize",
initial: "idle",
on: {
REFRESH_WORKSPACE: { actions: "assignWorkspace" },
},
states: {
initialize: {
always: [
{ cond: "isAtMaxDeadline", target: "atMaxDeadline" },
{ cond: "isAtMinDeadline", target: "atMinDeadline" },
{ target: "midRange" },
],
},
midRange: {
idle: {
on: {
INCREASE_DEADLINE: "increasingDeadline",
DECREASE_DEADLINE: "decreasingDeadline",
},
},
atMaxDeadline: {
on: {
DECREASE_DEADLINE: "decreasingDeadline",
},
},
atMinDeadline: {
on: {
INCREASE_DEADLINE: "increasingDeadline",
},
},
increasingDeadline: {
invoke: {
src: "increaseDeadline",
id: "increaseDeadline",
onDone: [
{
cond: "isAtMaxDeadline",
target: "atMaxDeadline",
actions: "displaySuccessMessage",
},
{
target: "midRange",
actions: "displaySuccessMessage",
},
],
onDone: {
target: "idle",
actions: "displaySuccessMessage",
},
onError: {
target: "midRange",
target: "idle",
actions: "displayFailureMessage",
},
},
@@ -114,19 +82,12 @@ export const workspaceScheduleBannerMachine = createMachine(
invoke: {
src: "decreaseDeadline",
id: "decreaseDeadline",
onDone: [
{
cond: "isAtMinDeadline",
target: "atMinDeadline",
actions: "displaySuccessMessage",
},
{
target: "midRange",
actions: "displaySuccessMessage",
},
],
onDone: {
target: "idle",
actions: "displaySuccessMessage",
},
onError: {
target: "midRange",
target: "idle",
actions: "displayFailureMessage",
},
},
@@ -135,18 +96,6 @@ export const workspaceScheduleBannerMachine = createMachine(
},
},
{
guards: {
isAtMaxDeadline: (context) =>
context.deadline
? !canExtendDeadline(
context.deadline,
context.workspace,
context.template,
)
: false,
isAtMinDeadline: (context) =>
context.deadline ? !canReduceDeadline(context.deadline) : false,
},
actions: {
// This error does not have a detail, so using the snackbar is okay
displayFailureMessage: (_, event) => {
@@ -157,27 +106,32 @@ export const workspaceScheduleBannerMachine = createMachine(
},
assignWorkspace: assign((_, event) => ({
workspace: event.workspace,
deadline: getDeadline(event.workspace),
})),
},
services: {
increaseDeadline: async (context, event) => {
if (!context.deadline) {
if (!context.workspace.latest_build.deadline) {
throw Error("Deadline is undefined.")
}
const proposedDeadline = context.deadline.add(event.hours, "hours")
const proposedDeadline = getDeadline(context.workspace).add(
event.hours,
"hours",
)
const newDeadline = dayjs.min(
proposedDeadline,
getMaxDeadline(context.workspace, context.template),
getMaxDeadline(context.workspace),
)
await API.putWorkspaceExtension(context.workspace.id, newDeadline)
},
decreaseDeadline: async (context, event) => {
if (!context.deadline) {
if (!context.workspace.latest_build.deadline) {
throw Error("Deadline is undefined.")
}
const proposedDeadline = context.deadline.subtract(event.hours, "hours")
const proposedDeadline = getDeadline(context.workspace).subtract(
event.hours,
"hours",
)
const newDeadline = dayjs.max(proposedDeadline, getMinDeadline())
await API.putWorkspaceExtension(context.workspace.id, newDeadline)
},