Skip to content

Commit 04c5f92

Browse files
johnstcnjohnstcn
authored
fix: ui: workspace bumpers now honour template max_ttl (#3532)
- chore: WorkspacePage: invert workspace schedule bumper logic for readibility - fix: make workspace bumpers honour template max_ttl - chore: refactor workspace schedule bumper logic to util/schedule.ts and unit test separately
1 parent 7599ad4 commit 04c5f92

File tree

11 files changed

+191
-128
lines changed

11 files changed

+191
-128
lines changed

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

+12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
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"
35
import * as Mocks from "../../testHelpers/entities"
46
import { Workspace, WorkspaceErrors, WorkspaceProps } from "./Workspace"
57

@@ -24,6 +26,16 @@ Started.args = {
2426
onDeadlinePlus: () => {
2527
// do nothing, this is just for storybook
2628
},
29+
deadlineMinusEnabled: () => {
30+
return canReduceDeadline(dayjs(Mocks.MockWorkspace.latest_build.deadline))
31+
},
32+
deadlinePlusEnabled: () => {
33+
return canExtendDeadline(
34+
dayjs(Mocks.MockWorkspace.latest_build.deadline),
35+
Mocks.MockWorkspace,
36+
Mocks.MockTemplate,
37+
)
38+
},
2739
},
2840
workspace: Mocks.MockWorkspace,
2941
handleStart: action("start"),

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

+4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ export interface WorkspaceProps {
3131
scheduleProps: {
3232
onDeadlinePlus: () => void
3333
onDeadlineMinus: () => void
34+
deadlinePlusEnabled: () => boolean
35+
deadlineMinusEnabled: () => boolean
3436
}
3537
handleStart: () => void
3638
handleStop: () => void
@@ -81,6 +83,8 @@ export const Workspace: FC<WorkspaceProps> = ({
8183
workspace={workspace}
8284
onDeadlineMinus={scheduleProps.onDeadlineMinus}
8385
onDeadlinePlus={scheduleProps.onDeadlinePlus}
86+
deadlineMinusEnabled={scheduleProps.deadlineMinusEnabled}
87+
deadlinePlusEnabled={scheduleProps.deadlinePlusEnabled}
8488
canUpdateWorkspace={canUpdateWorkspace}
8589
/>
8690
<WorkspaceActions

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

+1-94
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@ import dayjs from "dayjs"
22
import utc from "dayjs/plugin/utc"
33
import * as TypesGen from "../../api/typesGenerated"
44
import * as Mocks from "../../testHelpers/entities"
5-
import {
6-
deadlineMinusDisabled,
7-
deadlinePlusDisabled,
8-
shouldDisplayPlusMinus,
9-
} from "./WorkspaceScheduleButton"
5+
import { shouldDisplayPlusMinus } from "./WorkspaceScheduleButton"
106

117
dayjs.extend(utc)
12-
const now = dayjs()
138

149
describe("WorkspaceScheduleButton", () => {
1510
describe("shouldDisplayPlusMinus", () => {
@@ -29,92 +24,4 @@ describe("WorkspaceScheduleButton", () => {
2924
expect(shouldDisplayPlusMinus(workspace)).toBeTruthy()
3025
})
3126
})
32-
33-
describe("deadlineMinusDisabled", () => {
34-
it("should be false if the deadline is more than 30 minutes in the future", () => {
35-
// Given: a workspace with a deadline set to 31 minutes in the future
36-
const workspace: TypesGen.Workspace = {
37-
...Mocks.MockWorkspace,
38-
latest_build: {
39-
...Mocks.MockWorkspaceBuild,
40-
deadline: now.add(31, "minutes").utc().format(),
41-
},
42-
}
43-
44-
// Then: deadlineMinusDisabled should be falsy
45-
expect(deadlineMinusDisabled(workspace, now)).toBeFalsy()
46-
})
47-
48-
it("should be true if the deadline is 30 minutes or less in the future", () => {
49-
// Given: a workspace with a deadline set to 30 minutes in the future
50-
const workspace: TypesGen.Workspace = {
51-
...Mocks.MockWorkspace,
52-
latest_build: {
53-
...Mocks.MockWorkspaceBuild,
54-
deadline: now.add(30, "minutes").utc().format(),
55-
},
56-
}
57-
58-
// Then: deadlineMinusDisabled should be truthy
59-
expect(deadlineMinusDisabled(workspace, now)).toBeTruthy()
60-
})
61-
62-
it("should be true if the deadline is in the past", () => {
63-
// Given: a workspace with a deadline set to 1 minute in the past
64-
const workspace: TypesGen.Workspace = {
65-
...Mocks.MockWorkspace,
66-
latest_build: {
67-
...Mocks.MockWorkspaceBuild,
68-
deadline: now.add(-1, "minutes").utc().format(),
69-
},
70-
}
71-
72-
// Then: deadlineMinusDisabled should be truthy
73-
expect(deadlineMinusDisabled(workspace, now)).toBeTruthy()
74-
})
75-
})
76-
77-
describe("deadlinePlusDisabled", () => {
78-
it("should be false if the deadline is less than 24 hours in the future", () => {
79-
// Given: a workspace with a deadline set to 23 hours in the future
80-
const workspace: TypesGen.Workspace = {
81-
...Mocks.MockWorkspace,
82-
latest_build: {
83-
...Mocks.MockWorkspaceBuild,
84-
deadline: now.add(23, "hours").utc().format(),
85-
},
86-
}
87-
88-
// Then: deadlinePlusDisabled should be falsy
89-
expect(deadlinePlusDisabled(workspace, now)).toBeFalsy()
90-
})
91-
92-
it("should be true if the deadline is 24 hours or more in the future", () => {
93-
// Given: a workspace with a deadline set to 25 hours in the future
94-
const workspace: TypesGen.Workspace = {
95-
...Mocks.MockWorkspace,
96-
latest_build: {
97-
...Mocks.MockWorkspaceBuild,
98-
deadline: now.add(25, "hours").utc().format(),
99-
},
100-
}
101-
102-
// Then: deadlinePlusDisabled should be truthy
103-
expect(deadlinePlusDisabled(workspace, now)).toBeTruthy()
104-
})
105-
106-
it("should be false if the deadline is in the past", () => {
107-
// Given: a workspace with a deadline set to 1 minute in the past
108-
const workspace: TypesGen.Workspace = {
109-
...Mocks.MockWorkspace,
110-
latest_build: {
111-
...Mocks.MockWorkspaceBuild,
112-
deadline: now.add(-1, "minute").utc().format(),
113-
},
114-
}
115-
116-
// Then: deadlinePlusDisabled should be falsy
117-
expect(deadlinePlusDisabled(workspace, now)).toBeFalsy()
118-
})
119-
})
12027
})

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

+6-12
Original file line numberDiff line numberDiff line change
@@ -40,27 +40,21 @@ export const shouldDisplayPlusMinus = (workspace: Workspace): boolean => {
4040
return deadline.year() > 1
4141
}
4242

43-
export const deadlineMinusDisabled = (workspace: Workspace, now: dayjs.Dayjs): boolean => {
44-
const delta = dayjs(workspace.latest_build.deadline).diff(now)
45-
return delta <= 30 * 60 * 1000 // 30 minutes
46-
}
47-
48-
export const deadlinePlusDisabled = (workspace: Workspace, now: dayjs.Dayjs): boolean => {
49-
const delta = dayjs(workspace.latest_build.deadline).diff(now)
50-
return delta >= 24 * 60 * 60 * 1000 // 24 hours
51-
}
52-
5343
export interface WorkspaceScheduleButtonProps {
5444
workspace: Workspace
5545
onDeadlinePlus: () => void
5646
onDeadlineMinus: () => void
47+
deadlineMinusEnabled: () => boolean
48+
deadlinePlusEnabled: () => boolean
5749
canUpdateWorkspace: boolean
5850
}
5951

6052
export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = ({
6153
workspace,
6254
onDeadlinePlus,
6355
onDeadlineMinus,
56+
deadlinePlusEnabled,
57+
deadlineMinusEnabled,
6458
canUpdateWorkspace,
6559
}) => {
6660
const anchorRef = useRef<HTMLButtonElement>(null)
@@ -81,7 +75,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
8175
<IconButton
8276
className={styles.iconButton}
8377
size="small"
84-
disabled={deadlineMinusDisabled(workspace, dayjs())}
78+
disabled={!deadlineMinusEnabled()}
8579
onClick={onDeadlineMinus}
8680
>
8781
<Tooltip title={Language.editDeadlineMinus}>
@@ -91,7 +85,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
9185
<IconButton
9286
className={styles.iconButton}
9387
size="small"
94-
disabled={deadlinePlusDisabled(workspace, dayjs())}
88+
disabled={!deadlinePlusEnabled()}
9589
onClick={onDeadlinePlus}
9690
>
9791
<Tooltip title={Language.editDeadlinePlus}>

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ import { WorkspacePage } from "./WorkspacePage"
2727

2828
// It renders the workspace page and waits for it be loaded
2929
const renderWorkspacePage = async () => {
30+
const getTemplateMock = jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
3031
renderWithAuth(<WorkspacePage />, {
3132
route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`,
3233
path: "/@:username/:workspace",
3334
})
3435
await screen.findByText(MockWorkspace.name)
36+
expect(getTemplateMock).toBeCalled()
3537
}
3638

3739
/**
@@ -50,10 +52,10 @@ const testButton = async (label: string, actionMock: jest.SpyInstance) => {
5052
expect(actionMock).toBeCalled()
5153
}
5254

53-
const testStatus = async (mock: Workspace, label: string) => {
55+
const testStatus = async (ws: Workspace, label: string) => {
5456
server.use(
5557
rest.get(`/api/v2/users/:username/workspace/:workspaceName`, (req, res, ctx) => {
56-
return res(ctx.status(200), ctx.json(mock))
58+
return res(ctx.status(200), ctx.json(ws))
5759
}),
5860
)
5961
await renderWorkspacePage()
@@ -181,6 +183,7 @@ describe("Workspace Page", () => {
181183

182184
describe("Resources", () => {
183185
it("shows the status of each agent in each resource", async () => {
186+
const getTemplateMock = jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
184187
renderWithAuth(<WorkspacePage />, {
185188
route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`,
186189
path: "/@:username/:workspace",
@@ -197,6 +200,7 @@ describe("Workspace Page", () => {
197200
DisplayAgentStatusLanguage[MockWorkspaceAgentDisconnected.status],
198201
)
199202
expect(agent2Status.length).toEqual(2)
203+
expect(getTemplateMock).toBeCalled()
200204
})
201205
})
202206
})

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

+16-15
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { FullScreenLoader } from "../../components/Loader/FullScreenLoader"
1111
import { Workspace, WorkspaceErrors } from "../../components/Workspace/Workspace"
1212
import { firstOrItem } from "../../util/array"
1313
import { pageTitle } from "../../util/page"
14+
import { canExtendDeadline, canReduceDeadline, maxDeadline, minDeadline } from "../../util/schedule"
1415
import { getFaviconByStatus } from "../../util/workspace"
1516
import { selectUser } from "../../xServices/auth/authSelectors"
1617
import { XServiceContext } from "../../xServices/StateContext"
@@ -35,6 +36,8 @@ export const WorkspacePage: React.FC = () => {
3536
const {
3637
workspace,
3738
getWorkspaceError,
39+
template,
40+
refreshTemplateError,
3841
resources,
3942
getResourcesError,
4043
builds,
@@ -63,12 +66,16 @@ export const WorkspacePage: React.FC = () => {
6366
return (
6467
<div className={styles.error}>
6568
{getWorkspaceError && <ErrorSummary error={getWorkspaceError} />}
69+
{refreshTemplateError && <ErrorSummary error={refreshTemplateError} />}
6670
{checkPermissionsError && <ErrorSummary error={checkPermissionsError} />}
6771
</div>
6872
)
6973
} else if (!workspace) {
7074
return <FullScreenLoader />
75+
} else if (!template) {
76+
return <FullScreenLoader />
7177
} else {
78+
const deadline = dayjs(workspace.latest_build.deadline).utc()
7279
const favicon = getFaviconByStatus(workspace.latest_build)
7380
return (
7481
<>
@@ -85,7 +92,7 @@ export const WorkspacePage: React.FC = () => {
8592
bannerSend({
8693
type: "UPDATE_DEADLINE",
8794
workspaceId: workspace.id,
88-
newDeadline: dayjs(workspace.latest_build.deadline).utc().add(4, "hours"),
95+
newDeadline: dayjs.min(deadline.add(4, "hours"), maxDeadline(workspace, template)),
8996
})
9097
},
9198
}}
@@ -94,22 +101,22 @@ export const WorkspacePage: React.FC = () => {
94101
bannerSend({
95102
type: "UPDATE_DEADLINE",
96103
workspaceId: workspace.id,
97-
newDeadline: boundedDeadline(
98-
dayjs(workspace.latest_build.deadline).utc().add(-1, "hours"),
99-
dayjs(),
100-
),
104+
newDeadline: dayjs.max(deadline.add(-1, "hours"), minDeadline()),
101105
})
102106
},
103107
onDeadlinePlus: () => {
104108
bannerSend({
105109
type: "UPDATE_DEADLINE",
106110
workspaceId: workspace.id,
107-
newDeadline: boundedDeadline(
108-
dayjs(workspace.latest_build.deadline).utc().add(1, "hours"),
109-
dayjs(),
110-
),
111+
newDeadline: dayjs.min(deadline.add(1, "hours"), maxDeadline(workspace, template)),
111112
})
112113
},
114+
deadlineMinusEnabled: () => {
115+
return canReduceDeadline(deadline)
116+
},
117+
deadlinePlusEnabled: () => {
118+
return canExtendDeadline(deadline, workspace, template)
119+
},
113120
}}
114121
workspace={workspace}
115122
handleStart={() => workspaceSend("START")}
@@ -139,12 +146,6 @@ export const WorkspacePage: React.FC = () => {
139146
}
140147
}
141148

142-
export const boundedDeadline = (newDeadline: dayjs.Dayjs, now: dayjs.Dayjs): dayjs.Dayjs => {
143-
const minDeadline = now.add(30, "minutes")
144-
const maxDeadline = now.add(24, "hours")
145-
return dayjs.min(dayjs.max(minDeadline, newDeadline), maxDeadline)
146-
}
147-
148149
const useStyles = makeStyles((theme) => ({
149150
error: {
150151
margin: theme.spacing(2),

‎site/src/testHelpers/entities.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ export const MockTemplate: TypesGen.Template = {
151151
active_version_id: MockTemplateVersion.id,
152152
workspace_owner_count: 1,
153153
description: "This is a test description.",
154-
max_ttl_ms: 604800000,
155-
min_autostart_interval_ms: 3600000,
154+
max_ttl_ms: 24 * 60 * 60 * 1000,
155+
min_autostart_interval_ms: 60 * 60 * 1000,
156156
created_by_id: "test-creator-id",
157157
created_by_name: "test_creator",
158158
}

0 commit comments

Comments
 (0)