Skip to content

Commit 0eb2530

Browse files
presleyppresleyp
authored
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
1 parent 8d95285 commit 0eb2530

File tree

9 files changed

+54
-211
lines changed

9 files changed

+54
-211
lines changed

‎site/src/components/Workspace/Workspace.stories.tsx

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import { action } from "@storybook/addon-actions"
22
import { Story } from "@storybook/react"
3-
import dayjs from "dayjs"
4-
import { canExtendDeadline, canReduceDeadline } from "util/schedule"
53
import * as Mocks from "../../testHelpers/entities"
64
import { Workspace, WorkspaceErrors, WorkspaceProps } from "./Workspace"
75

@@ -22,18 +20,8 @@ Running.args = {
2220
onDeadlinePlus: () => {
2321
// do nothing, this is just for storybook
2422
},
25-
deadlineMinusEnabled: () => {
26-
return canReduceDeadline(dayjs(Mocks.MockWorkspace.latest_build.deadline))
27-
},
28-
deadlinePlusEnabled: () => {
29-
return canExtendDeadline(
30-
dayjs(Mocks.MockWorkspace.latest_build.deadline),
31-
Mocks.MockWorkspace,
32-
Mocks.MockTemplate,
33-
)
34-
},
35-
maxDeadlineDecrease: 1000,
36-
maxDeadlineIncrease: 1000,
23+
maxDeadlineDecrease: 0,
24+
maxDeadlineIncrease: 24,
3725
},
3826
workspace: Mocks.MockWorkspace,
3927
handleStart: action("start"),

‎site/src/components/Workspace/Workspace.tsx

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ export interface WorkspaceProps {
3535
scheduleProps: {
3636
onDeadlinePlus: (hours: number) => void
3737
onDeadlineMinus: (hours: number) => void
38-
deadlinePlusEnabled: () => boolean
39-
deadlineMinusEnabled: () => boolean
4038
maxDeadlineIncrease: number
4139
maxDeadlineDecrease: number
4240
}
@@ -131,8 +129,6 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
131129
workspace={workspace}
132130
onDeadlineMinus={scheduleProps.onDeadlineMinus}
133131
onDeadlinePlus={scheduleProps.onDeadlinePlus}
134-
deadlineMinusEnabled={scheduleProps.deadlineMinusEnabled}
135-
deadlinePlusEnabled={scheduleProps.deadlinePlusEnabled}
136132
maxDeadlineDecrease={scheduleProps.maxDeadlineDecrease}
137133
maxDeadlineIncrease={scheduleProps.maxDeadlineIncrease}
138134
canUpdateWorkspace={canUpdateWorkspace}

‎site/src/components/WorkspaceScheduleButton/EditHours.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ export const EditHours = ({
1919
const styles = useStyles()
2020

2121
return (
22-
<form onSubmit={() => handleSubmit(hours)}>
22+
// hours is NaN when user deletes the value, so treat it as 0
23+
<form onSubmit={() => handleSubmit(Number.isNaN(hours) ? 0 : hours)}>
2324
<Stack direction="row" alignItems="baseline" spacing={1}>
2425
<TextField
2526
className={styles.inputField}
2627
inputProps={{ min: 0, max, step: 1 }}
2728
label={t("workspaceScheduleButton.hours")}
28-
value={hours}
29+
value={hours.toString()}
2930
onChange={(e) => setHours(parseInt(e.target.value))}
3031
type="number"
3132
/>

‎site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@ export interface WorkspaceScheduleButtonProps {
4848
workspace: Workspace
4949
onDeadlinePlus: (hours: number) => void
5050
onDeadlineMinus: (hours: number) => void
51-
deadlineMinusEnabled: () => boolean
52-
deadlinePlusEnabled: () => boolean
5351
maxDeadlineIncrease: number
5452
maxDeadlineDecrease: number
5553
canUpdateWorkspace: boolean
@@ -63,8 +61,6 @@ export const WorkspaceScheduleButton: React.FC<
6361
workspace,
6462
onDeadlinePlus,
6563
onDeadlineMinus,
66-
deadlinePlusEnabled,
67-
deadlineMinusEnabled,
6864
maxDeadlineDecrease,
6965
maxDeadlineIncrease,
7066
canUpdateWorkspace,
@@ -75,6 +71,8 @@ export const WorkspaceScheduleButton: React.FC<
7571
const [editMode, setEditMode] = useState<EditMode>("off")
7672
const id = isOpen ? "schedule-popover" : undefined
7773
const styles = useStyles({ editMode })
74+
const deadlinePlusEnabled = maxDeadlineIncrease >= 1
75+
const deadlineMinusEnabled = maxDeadlineDecrease >= 1
7876

7977
const onClose = () => {
8078
setIsOpen(false)
@@ -107,8 +105,9 @@ export const WorkspaceScheduleButton: React.FC<
107105
<span className={styles.actions}>
108106
<IconButton
109107
className={styles.subtractButton}
108+
aria-label="Subtract hours from deadline"
110109
size="small"
111-
disabled={!deadlineMinusEnabled()}
110+
disabled={!deadlineMinusEnabled}
112111
onClick={() => {
113112
setEditMode("subtract")
114113
}}
@@ -121,8 +120,9 @@ export const WorkspaceScheduleButton: React.FC<
121120
</IconButton>
122121
<IconButton
123122
className={styles.addButton}
123+
aria-label="Add hours to deadline"
124124
size="small"
125-
disabled={!deadlinePlusEnabled()}
125+
disabled={!deadlinePlusEnabled}
126126
onClick={() => {
127127
setEditMode("add")
128128
}}

‎site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Helmet } from "react-helmet-async"
66
import { useTranslation } from "react-i18next"
77
import { useNavigate } from "react-router-dom"
88
import {
9+
getDeadline,
910
getMaxDeadline,
1011
getMaxDeadlineChange,
1112
getMinDeadline,
@@ -37,10 +38,9 @@ export const WorkspaceReadyPage = ({
3738
quotaState,
3839
workspaceSend,
3940
}: WorkspaceReadyPageProps): JSX.Element => {
40-
const [bannerState, bannerSend] = useActor(
41+
const [_, bannerSend] = useActor(
4142
workspaceState.children["scheduleBannerMachine"],
4243
)
43-
const deadline = bannerState.context.deadline
4444
const xServices = useContext(XServiceContext)
4545
const featureVisibility = useSelector(
4646
xServices.entitlementsXService,
@@ -61,6 +61,7 @@ export const WorkspaceReadyPage = ({
6161
if (workspace === undefined) {
6262
throw Error("Workspace is undefined")
6363
}
64+
const deadline = getDeadline(workspace)
6465
const canUpdateWorkspace = Boolean(permissions?.updateWorkspace)
6566
const { t } = useTranslation("workspacePage")
6667
const favicon = getFaviconByStatus(workspace.latest_build)
@@ -101,18 +102,11 @@ export const WorkspaceReadyPage = ({
101102
hours,
102103
})
103104
},
104-
deadlineMinusEnabled: () => !bannerState.matches("atMinDeadline"),
105-
deadlinePlusEnabled: () => !bannerState.matches("atMaxDeadline"),
106-
maxDeadlineDecrease: deadline
107-
? getMaxDeadlineChange(deadline, getMinDeadline())
108-
: 0,
109-
maxDeadlineIncrease:
110-
deadline && template
111-
? getMaxDeadlineChange(
112-
getMaxDeadline(workspace, template),
113-
deadline,
114-
)
115-
: 0,
105+
maxDeadlineDecrease: getMaxDeadlineChange(deadline, getMinDeadline()),
106+
maxDeadlineIncrease: getMaxDeadlineChange(
107+
getMaxDeadline(workspace),
108+
deadline,
109+
),
116110
}}
117111
isUpdating={workspaceState.hasTag("updating")}
118112
workspace={workspace}

‎site/src/util/schedule.test.ts

Lines changed: 6 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ import dayjs from "dayjs"
22
import duration from "dayjs/plugin/duration"
33
import { emptySchedule } from "pages/WorkspaceSchedulePage/schedule"
44
import { emptyTTL } from "pages/WorkspaceSchedulePage/ttl"
5-
import { Template, Workspace } from "../api/typesGenerated"
5+
import { Workspace } from "../api/typesGenerated"
66
import * as Mocks from "../testHelpers/entities"
77
import {
8-
canExtendDeadline,
9-
canReduceDeadline,
108
deadlineExtensionMax,
119
deadlineExtensionMin,
1210
extractTimezone,
@@ -19,6 +17,7 @@ import {
1917

2018
dayjs.extend(duration)
2119
const now = dayjs()
20+
const startTime = dayjs(Mocks.MockWorkspaceBuild.updated_at)
2221

2322
describe("util/schedule", () => {
2423
describe("stripTimezone", () => {
@@ -43,38 +42,16 @@ describe("util/schedule", () => {
4342
})
4443

4544
describe("maxDeadline", () => {
46-
// Given: a workspace built from a template with a max deadline equal to 25 hours which isn't really possible
4745
const workspace: Workspace = {
4846
...Mocks.MockWorkspace,
4947
latest_build: {
5048
...Mocks.MockWorkspaceBuild,
51-
deadline: now.add(8, "hours").utc().format(),
49+
deadline: startTime.add(8, "hours").utc().format(),
5250
},
5351
}
54-
describe("given a template with 25 hour max ttl", () => {
55-
it("should be never be greater than global max deadline", () => {
56-
const template: Template = {
57-
...Mocks.MockTemplate,
58-
default_ttl_ms: 25 * 60 * 60 * 1000,
59-
}
60-
61-
// Then: deadlineMinusDisabled should be falsy
62-
const delta = getMaxDeadline(workspace, template).diff(now)
63-
expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
64-
})
65-
})
66-
67-
describe("given a template with 4 hour max ttl", () => {
68-
it("should be never be greater than global max deadline", () => {
69-
const template: Template = {
70-
...Mocks.MockTemplate,
71-
default_ttl_ms: 4 * 60 * 60 * 1000,
72-
}
73-
74-
// Then: deadlineMinusDisabled should be falsy
75-
const delta = getMaxDeadline(workspace, template).diff(now)
76-
expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
77-
})
52+
it("should be 24 hours from the workspace start time", () => {
53+
const delta = getMaxDeadline(workspace).diff(startTime)
54+
expect(delta).toEqual(deadlineExtensionMax.asMilliseconds())
7855
})
7956
})
8057

@@ -85,50 +62,6 @@ describe("minDeadline", () => {
8562
})
8663
})
8764

88-
describe("canExtendDeadline", () => {
89-
it("should be falsy if the deadline is more than 24 hours in the future", () => {
90-
expect(
91-
canExtendDeadline(
92-
dayjs().add(25, "hours"),
93-
Mocks.MockWorkspace,
94-
Mocks.MockTemplate,
95-
),
96-
).toBeFalsy()
97-
})
98-
99-
it("should be falsy if the deadline is more than the template max_ttl", () => {
100-
const tooFarAhead = dayjs().add(
101-
dayjs.duration(Mocks.MockTemplate.default_ttl_ms, "milliseconds"),
102-
)
103-
expect(
104-
canExtendDeadline(tooFarAhead, Mocks.MockWorkspace, Mocks.MockTemplate),
105-
).toBeFalsy()
106-
})
107-
108-
it("should be truth if the deadline is within the template max_ttl", () => {
109-
const okDeadline = dayjs().add(
110-
dayjs.duration(Mocks.MockTemplate.default_ttl_ms / 2, "milliseconds"),
111-
)
112-
expect(
113-
canExtendDeadline(okDeadline, Mocks.MockWorkspace, Mocks.MockTemplate),
114-
).toBeFalsy()
115-
})
116-
})
117-
118-
describe("canReduceDeadline", () => {
119-
it("should be falsy if the deadline is 30 minutes or less in the future", () => {
120-
expect(canReduceDeadline(dayjs())).toBeFalsy()
121-
expect(canReduceDeadline(dayjs().add(1, "minutes"))).toBeFalsy()
122-
expect(canReduceDeadline(dayjs().add(29, "minutes"))).toBeFalsy()
123-
expect(canReduceDeadline(dayjs().add(30, "minutes"))).toBeFalsy()
124-
})
125-
126-
it("should be truthy if the deadline is 30 minutes or more in the future", () => {
127-
expect(canReduceDeadline(dayjs().add(31, "minutes"))).toBeTruthy()
128-
expect(canReduceDeadline(dayjs().add(100, "years"))).toBeTruthy()
129-
})
130-
})
131-
13265
describe("getMaxDeadlineChange", () => {
13366
it("should return the number of hours you can add before hitting the max deadline", () => {
13467
const deadline = dayjs()

‎site/src/util/schedule.ts

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import duration from "dayjs/plugin/duration"
77
import relativeTime from "dayjs/plugin/relativeTime"
88
import timezone from "dayjs/plugin/timezone"
99
import utc from "dayjs/plugin/utc"
10-
import { Template, Workspace } from "../api/typesGenerated"
10+
import { Workspace } from "../api/typesGenerated"
1111
import { isWorkspaceOn } from "./workspace"
1212
import { WorkspaceScheduleFormValues } from "components/WorkspaceScheduleForm/WorkspaceScheduleForm"
1313
import { AutoStop } from "pages/WorkspaceSchedulePage/ttl"
@@ -127,27 +127,18 @@ export const deadlineExtensionMin = dayjs.duration(30, "minutes")
127127
export const deadlineExtensionMax = dayjs.duration(24, "hours")
128128

129129
/**
130-
* Depends on the time the workspace was last updated, the template config,
131-
* and a global constant.
130+
* Depends on the time the workspace was last updated and a global constant.
132131
* @param ws workspace
133-
* @param tpl template
134132
* @returns the latest datetime at which the workspace can be automatically shut down.
135133
*/
136-
export function getMaxDeadline(
137-
ws: Workspace | undefined,
138-
tpl: Template,
139-
): dayjs.Dayjs {
134+
export function getMaxDeadline(ws: Workspace | undefined): dayjs.Dayjs {
140135
// note: we count runtime from updated_at as started_at counts from the start of
141136
// the workspace build process, which can take a while.
142137
if (ws === undefined) {
143138
throw Error("Cannot calculate max deadline because workspace is undefined")
144139
}
145140
const startedAt = dayjs(ws.latest_build.updated_at)
146-
const maxTemplateDeadline = startedAt.add(
147-
dayjs.duration(tpl.default_ttl_ms, "milliseconds"),
148-
)
149-
const maxGlobalDeadline = startedAt.add(deadlineExtensionMax)
150-
return dayjs.min(maxTemplateDeadline, maxGlobalDeadline)
141+
return startedAt.add(deadlineExtensionMax)
151142
}
152143

153144
/**
@@ -158,26 +149,13 @@ export function getMinDeadline(): dayjs.Dayjs {
158149
return dayjs().add(deadlineExtensionMin)
159150
}
160151

161-
export function canExtendDeadline(
162-
deadline: dayjs.Dayjs,
163-
workspace: Workspace,
164-
template: Template,
165-
): boolean {
166-
return deadline < getMaxDeadline(workspace, template)
167-
}
168-
169-
export function canReduceDeadline(deadline: dayjs.Dayjs): boolean {
170-
return deadline > getMinDeadline()
171-
}
172-
173152
export const getDeadline = (workspace: Workspace): dayjs.Dayjs =>
174153
dayjs(workspace.latest_build.deadline).utc()
175154

176155
/**
177156
* Get number of hours you can add or subtract to the current deadline before hitting the max or min deadline.
178157
* @param deadline
179158
* @param workspace
180-
* @param template
181159
* @returns number, in hours
182160
*/
183161
export const getMaxDeadlineChange = (

‎site/src/xServices/workspace/workspaceXService.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,6 @@ export const workspaceMachine = createMachine(
470470
src: workspaceScheduleBannerMachine,
471471
data: {
472472
workspace: (context: WorkspaceContext) => context.workspace,
473-
template: (context: WorkspaceContext) => context.template,
474473
},
475474
},
476475
},

0 commit comments

Comments
 (0)