Skip to content

Commit 8cd5aea

Browse files
Kira-PilotKira-Pilot
authored
cleanup workspace machine (coder#4160)
* removed dead build states * removed dead code * removed guards * not calling events from actions * simplified timeline * simplify refresh template
1 parent 1214022 commit 8cd5aea

File tree

2 files changed

+78
-86
lines changed

2 files changed

+78
-86
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export const WorkspacePage: FC = () => {
3535
workspace,
3636
getWorkspaceError,
3737
template,
38-
refreshTemplateWarning,
38+
getTemplateWarning,
3939
refreshWorkspaceWarning,
4040
builds,
4141
getBuildsError,
@@ -64,7 +64,7 @@ export const WorkspacePage: FC = () => {
6464
return (
6565
<div className={styles.error}>
6666
{Boolean(getWorkspaceError) && <ErrorSummary error={getWorkspaceError} />}
67-
{Boolean(refreshTemplateWarning) && <ErrorSummary error={refreshTemplateWarning} />}
67+
{Boolean(getTemplateWarning) && <ErrorSummary error={getTemplateWarning} />}
6868
{Boolean(checkPermissionsError) && <ErrorSummary error={checkPermissionsError} />}
6969
</div>
7070
)

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

Lines changed: 76 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { assign, createMachine, send } from "xstate"
2-
import { pure } from "xstate/lib/actions"
32
import * as API from "../../api/api"
43
import * as Types from "../../api/types"
54
import * as TypesGen from "../../api/typesGenerated"
@@ -12,8 +11,31 @@ const latestBuild = (builds: TypesGen.WorkspaceBuild[]) => {
1211
})[0]
1312
}
1413

14+
const moreBuildsAvailable = (
15+
context: WorkspaceContext,
16+
event: {
17+
type: "REFRESH_TIMELINE"
18+
checkRefresh?: boolean
19+
data?: TypesGen.ServerSentEvent["data"]
20+
},
21+
) => {
22+
// No need to refresh the timeline if it is not loaded
23+
if (!context.builds) {
24+
return false
25+
}
26+
27+
if (!event.checkRefresh) {
28+
return true
29+
}
30+
31+
// After we refresh a workspace, we want to check if the latest
32+
// build was updated before refreshing the timeline so as to not over fetch the builds
33+
const latestBuildInTimeline = latestBuild(context.builds)
34+
return event.data.latest_build.updated_at !== latestBuildInTimeline.updated_at
35+
}
36+
1537
const Language = {
16-
refreshTemplateWarning: "Error updating workspace: latest template could not be fetched.",
38+
getTemplateWarning: "Error updating workspace: latest template could not be fetched.",
1739
buildError: "Workspace action failed.",
1840
}
1941

@@ -28,7 +50,7 @@ export interface WorkspaceContext {
2850
getWorkspaceError?: Error | unknown
2951
// these are labeled as warnings because they don't make the page unusable
3052
refreshWorkspaceWarning?: Error | unknown
31-
refreshTemplateWarning: Error | unknown
53+
getTemplateWarning: Error | unknown
3254
// Builds
3355
builds?: TypesGen.WorkspaceBuild[]
3456
getBuildsError?: Error | unknown
@@ -51,8 +73,7 @@ export type WorkspaceEvent =
5173
| { type: "CANCEL_DELETE" }
5274
| { type: "UPDATE" }
5375
| { type: "CANCEL" }
54-
| { type: "CHECK_REFRESH_TIMELINE"; data: TypesGen.ServerSentEvent["data"] }
55-
| { type: "REFRESH_TIMELINE" }
76+
| { type: "REFRESH_TIMELINE"; checkRefresh?: boolean; data?: TypesGen.ServerSentEvent["data"] }
5677
| { type: "EVENT_SOURCE_ERROR"; error: Error | unknown }
5778

5879
export const checks = {
@@ -139,7 +160,7 @@ export const workspaceMachine = createMachine(
139160
onDone: [
140161
{
141162
actions: "assignWorkspace",
142-
target: "refreshingTemplate",
163+
target: "gettingTemplate",
143164
},
144165
],
145166
onError: [
@@ -151,11 +172,11 @@ export const workspaceMachine = createMachine(
151172
},
152173
tags: "loading",
153174
},
154-
refreshingTemplate: {
155-
entry: "clearRefreshTemplateWarning",
175+
gettingTemplate: {
176+
entry: "clearGettingTemplateWarning",
156177
invoke: {
157178
src: "getTemplate",
158-
id: "refreshTemplate",
179+
id: "getTemplate",
159180
onDone: [
160181
{
161182
actions: "assignTemplate",
@@ -164,7 +185,7 @@ export const workspaceMachine = createMachine(
164185
],
165186
onError: [
166187
{
167-
actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
188+
actions: ["assignGetTemplateWarning", "displayGetTemplateWarning"],
168189
target: "error",
169190
},
170191
],
@@ -210,9 +231,6 @@ export const workspaceMachine = createMachine(
210231
EVENT_SOURCE_ERROR: {
211232
target: "error",
212233
},
213-
CHECK_REFRESH_TIMELINE: {
214-
actions: ["refreshTimeline"],
215-
},
216234
},
217235
},
218236
error: {
@@ -254,7 +272,7 @@ export const workspaceMachine = createMachine(
254272
src: "startWorkspaceWithLatestTemplate",
255273
onDone: {
256274
target: "idle",
257-
actions: ["assignBuild", "refreshTimeline"],
275+
actions: ["assignBuild"],
258276
},
259277
onError: {
260278
target: "idle",
@@ -269,7 +287,7 @@ export const workspaceMachine = createMachine(
269287
id: "startWorkspace",
270288
onDone: [
271289
{
272-
actions: ["assignBuild", "refreshTimeline"],
290+
actions: ["assignBuild"],
273291
target: "idle",
274292
},
275293
],
@@ -288,7 +306,7 @@ export const workspaceMachine = createMachine(
288306
id: "stopWorkspace",
289307
onDone: [
290308
{
291-
actions: ["assignBuild", "refreshTimeline"],
309+
actions: ["assignBuild"],
292310
target: "idle",
293311
},
294312
],
@@ -307,7 +325,7 @@ export const workspaceMachine = createMachine(
307325
id: "deleteWorkspace",
308326
onDone: [
309327
{
310-
actions: ["assignBuild", "refreshTimeline"],
328+
actions: ["assignBuild"],
311329
target: "idle",
312330
},
313331
],
@@ -326,11 +344,7 @@ export const workspaceMachine = createMachine(
326344
id: "cancelWorkspace",
327345
onDone: [
328346
{
329-
actions: [
330-
"assignCancellationMessage",
331-
"displayCancellationMessage",
332-
"refreshTimeline",
333-
],
347+
actions: ["assignCancellationMessage", "displayCancellationMessage"],
334348
target: "idle",
335349
},
336350
],
@@ -342,31 +356,11 @@ export const workspaceMachine = createMachine(
342356
],
343357
},
344358
},
345-
refreshingTemplate: {
346-
entry: "clearRefreshTemplateWarning",
347-
invoke: {
348-
src: "getTemplate",
349-
id: "refreshTemplate",
350-
onDone: [
351-
{
352-
actions: "assignTemplate",
353-
target: "requestingStart",
354-
},
355-
],
356-
onError: [
357-
{
358-
actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
359-
target: "idle",
360-
},
361-
],
362-
},
363-
},
364359
},
365360
},
366361
timeline: {
367362
initial: "gettingBuilds",
368363
states: {
369-
idle: {},
370364
gettingBuilds: {
371365
entry: "clearGetBuildsError",
372366
invoke: {
@@ -380,19 +374,17 @@ export const workspaceMachine = createMachine(
380374
onError: [
381375
{
382376
actions: "assignGetBuildsError",
383-
target: "idle",
377+
target: "loadedBuilds",
384378
},
385379
],
386380
},
387381
},
388382
loadedBuilds: {
389-
initial: "idle",
390-
states: {
391-
idle: {
392-
on: {
393-
REFRESH_TIMELINE: {
394-
target: "#workspaceState.ready.timeline.gettingBuilds",
395-
},
383+
on: {
384+
REFRESH_TIMELINE: {
385+
target: "#workspaceState.ready.timeline.gettingBuilds",
386+
cond: {
387+
type: "moreBuildsAvailable",
396388
},
397389
},
398390
},
@@ -483,14 +475,14 @@ export const workspaceMachine = createMachine(
483475
clearRefreshWorkspaceWarning: assign({
484476
refreshWorkspaceWarning: (_) => undefined,
485477
}),
486-
assignRefreshTemplateWarning: assign({
487-
refreshTemplateWarning: (_, event) => event.data,
478+
assignGetTemplateWarning: assign({
479+
getTemplateWarning: (_, event) => event.data,
488480
}),
489-
displayRefreshTemplateWarning: () => {
490-
displayError(Language.refreshTemplateWarning)
481+
displayGetTemplateWarning: () => {
482+
displayError(Language.getTemplateWarning)
491483
},
492-
clearRefreshTemplateWarning: assign({
493-
refreshTemplateWarning: (_) => undefined,
484+
clearGettingTemplateWarning: assign({
485+
getTemplateWarning: (_) => undefined,
494486
}),
495487
// Timeline
496488
assignBuilds: assign({
@@ -502,24 +494,9 @@ export const workspaceMachine = createMachine(
502494
clearGetBuildsError: assign({
503495
getBuildsError: (_) => undefined,
504496
}),
505-
refreshTimeline: pure((context, event) => {
506-
// No need to refresh the timeline if it is not loaded
507-
if (!context.builds) {
508-
return
509-
}
510-
511-
// When it is a CHECK_REFRESH_TIMELINE workspace event, we want to check if the latest
512-
// build was updated to not over fetch the builds
513-
if (event.type === "CHECK_REFRESH_TIMELINE") {
514-
const latestBuildInTimeline = latestBuild(context.builds)
515-
const isUpdated = event.data?.latest_build.updated_at !== latestBuildInTimeline.updated_at
516-
if (isUpdated) {
517-
return send({ type: "REFRESH_TIMELINE" })
518-
}
519-
} else {
520-
return send({ type: "REFRESH_TIMELINE" })
521-
}
522-
}),
497+
},
498+
guards: {
499+
moreBuildsAvailable,
523500
},
524501
services: {
525502
getWorkspace: async (_, event) => {
@@ -534,40 +511,55 @@ export const workspaceMachine = createMachine(
534511
throw Error("Cannot get template without workspace")
535512
}
536513
},
537-
startWorkspaceWithLatestTemplate: async (context) => {
514+
startWorkspaceWithLatestTemplate: (context) => async (send) => {
538515
if (context.workspace && context.template) {
539-
return await API.startWorkspace(context.workspace.id, context.template.active_version_id)
516+
const startWorkspacePromise = await API.startWorkspace(
517+
context.workspace.id,
518+
context.template.active_version_id,
519+
)
520+
send({ type: "REFRESH_TIMELINE" })
521+
return startWorkspacePromise
540522
} else {
541523
throw Error("Cannot start workspace without workspace id")
542524
}
543525
},
544-
startWorkspace: async (context) => {
526+
startWorkspace: (context) => async (send) => {
545527
if (context.workspace) {
546-
return await API.startWorkspace(
528+
const startWorkspacePromise = await API.startWorkspace(
547529
context.workspace.id,
548530
context.workspace.latest_build.template_version_id,
549531
)
532+
send({ type: "REFRESH_TIMELINE" })
533+
return startWorkspacePromise
550534
} else {
551535
throw Error("Cannot start workspace without workspace id")
552536
}
553537
},
554-
stopWorkspace: async (context) => {
538+
stopWorkspace: (context) => async (send) => {
555539
if (context.workspace) {
556-
return await API.stopWorkspace(context.workspace.id)
540+
const stopWorkspacePromise = await API.stopWorkspace(context.workspace.id)
541+
send({ type: "REFRESH_TIMELINE" })
542+
return stopWorkspacePromise
557543
} else {
558544
throw Error("Cannot stop workspace without workspace id")
559545
}
560546
},
561547
deleteWorkspace: async (context) => {
562548
if (context.workspace) {
563-
return await API.deleteWorkspace(context.workspace.id)
549+
const deleteWorkspacePromise = await API.deleteWorkspace(context.workspace.id)
550+
send({ type: "REFRESH_TIMELINE" })
551+
return deleteWorkspacePromise
564552
} else {
565553
throw Error("Cannot delete workspace without workspace id")
566554
}
567555
},
568-
cancelWorkspace: async (context) => {
556+
cancelWorkspace: (context) => async (send) => {
569557
if (context.workspace) {
570-
return await API.cancelWorkspaceBuild(context.workspace.latest_build.id)
558+
const cancelWorkspacePromise = await API.cancelWorkspaceBuild(
559+
context.workspace.latest_build.id,
560+
)
561+
send({ type: "REFRESH_TIMELINE" })
562+
return cancelWorkspacePromise
571563
} else {
572564
throw Error("Cannot cancel workspace without build id")
573565
}
@@ -582,7 +574,7 @@ export const workspaceMachine = createMachine(
582574
// refresh our workspace with each SSE
583575
send({ type: "REFRESH_WORKSPACE", data: JSON.parse(event.data) })
584576
// refresh our timeline
585-
send({ type: "CHECK_REFRESH_TIMELINE", data: JSON.parse(event.data) })
577+
send({ type: "REFRESH_TIMELINE", checkRefresh: true, data: JSON.parse(event.data) })
586578
})
587579

588580
// handle any error events returned by our sse

0 commit comments

Comments
 (0)