cleanup workspace machine (#4160)

* removed dead build states

* removed dead code

* removed guards

* not calling events from actions

* simplified timeline

* simplify refresh template

site/src/pages/WorkspacePage/WorkspacePage.tsx

@@ -35,7 +35,7 @@ export const WorkspacePage: FC = () => {
     workspace,
     getWorkspaceError,
     template,
-    refreshTemplateWarning,
+    getTemplateWarning,
     refreshWorkspaceWarning,
     builds,
     getBuildsError,
@@ -64,7 +64,7 @@ export const WorkspacePage: FC = () => {
     return (
       <div className={styles.error}>
         {Boolean(getWorkspaceError) && <ErrorSummary error={getWorkspaceError} />}
-        {Boolean(refreshTemplateWarning) && <ErrorSummary error={refreshTemplateWarning} />}
+        {Boolean(getTemplateWarning) && <ErrorSummary error={getTemplateWarning} />}
         {Boolean(checkPermissionsError) && <ErrorSummary error={checkPermissionsError} />}
       </div>
     )

site/src/xServices/workspace/workspaceXService.ts

@@ -1,5 +1,4 @@
 import { assign, createMachine, send } from "xstate"
-import { pure } from "xstate/lib/actions"
 import * as API from "../../api/api"
 import * as Types from "../../api/types"
 import * as TypesGen from "../../api/typesGenerated"
@@ -12,8 +11,31 @@ const latestBuild = (builds: TypesGen.WorkspaceBuild[]) => {
   })[0]
 }
 
+const moreBuildsAvailable = (
+  context: WorkspaceContext,
+  event: {
+    type: "REFRESH_TIMELINE"
+    checkRefresh?: boolean
+    data?: TypesGen.ServerSentEvent["data"]
+  },
+) => {
+  // No need to refresh the timeline if it is not loaded
+  if (!context.builds) {
+    return false
+  }
+
+  if (!event.checkRefresh) {
+    return true
+  }
+
+  // After we refresh a workspace, we want to check if the latest
+  // build was updated before refreshing the timeline so as to not over fetch the builds
+  const latestBuildInTimeline = latestBuild(context.builds)
+  return event.data.latest_build.updated_at !== latestBuildInTimeline.updated_at
+}
+
 const Language = {
-  refreshTemplateWarning: "Error updating workspace: latest template could not be fetched.",
+  getTemplateWarning: "Error updating workspace: latest template could not be fetched.",
   buildError: "Workspace action failed.",
 }
 
@@ -28,7 +50,7 @@ export interface WorkspaceContext {
   getWorkspaceError?: Error | unknown
   // these are labeled as warnings because they don't make the page unusable
   refreshWorkspaceWarning?: Error | unknown
-  refreshTemplateWarning: Error | unknown
+  getTemplateWarning: Error | unknown
   // Builds
   builds?: TypesGen.WorkspaceBuild[]
   getBuildsError?: Error | unknown
@@ -51,8 +73,7 @@ export type WorkspaceEvent =
   | { type: "CANCEL_DELETE" }
   | { type: "UPDATE" }
   | { type: "CANCEL" }
-  | { type: "CHECK_REFRESH_TIMELINE"; data: TypesGen.ServerSentEvent["data"] }
-  | { type: "REFRESH_TIMELINE" }
+  | { type: "REFRESH_TIMELINE"; checkRefresh?: boolean; data?: TypesGen.ServerSentEvent["data"] }
   | { type: "EVENT_SOURCE_ERROR"; error: Error | unknown }
 
 export const checks = {
@@ -139,7 +160,7 @@ export const workspaceMachine = createMachine(
           onDone: [
             {
               actions: "assignWorkspace",
-              target: "refreshingTemplate",
+              target: "gettingTemplate",
             },
           ],
           onError: [
@@ -151,11 +172,11 @@ export const workspaceMachine = createMachine(
         },
         tags: "loading",
       },
-      refreshingTemplate: {
-        entry: "clearRefreshTemplateWarning",
+      gettingTemplate: {
+        entry: "clearGettingTemplateWarning",
         invoke: {
           src: "getTemplate",
-          id: "refreshTemplate",
+          id: "getTemplate",
           onDone: [
             {
               actions: "assignTemplate",
@@ -164,7 +185,7 @@ export const workspaceMachine = createMachine(
           ],
           onError: [
             {
-              actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
+              actions: ["assignGetTemplateWarning", "displayGetTemplateWarning"],
               target: "error",
             },
           ],
@@ -210,9 +231,6 @@ export const workspaceMachine = createMachine(
                   EVENT_SOURCE_ERROR: {
                     target: "error",
                   },
-                  CHECK_REFRESH_TIMELINE: {
-                    actions: ["refreshTimeline"],
-                  },
                 },
               },
               error: {
@@ -254,7 +272,7 @@ export const workspaceMachine = createMachine(
                   src: "startWorkspaceWithLatestTemplate",
                   onDone: {
                     target: "idle",
-                    actions: ["assignBuild", "refreshTimeline"],
+                    actions: ["assignBuild"],
                   },
                   onError: {
                     target: "idle",
@@ -269,7 +287,7 @@ export const workspaceMachine = createMachine(
                   id: "startWorkspace",
                   onDone: [
                     {
-                      actions: ["assignBuild", "refreshTimeline"],
+                      actions: ["assignBuild"],
                       target: "idle",
                     },
                   ],
@@ -288,7 +306,7 @@ export const workspaceMachine = createMachine(
                   id: "stopWorkspace",
                   onDone: [
                     {
-                      actions: ["assignBuild", "refreshTimeline"],
+                      actions: ["assignBuild"],
                       target: "idle",
                     },
                   ],
@@ -307,7 +325,7 @@ export const workspaceMachine = createMachine(
                   id: "deleteWorkspace",
                   onDone: [
                     {
-                      actions: ["assignBuild", "refreshTimeline"],
+                      actions: ["assignBuild"],
                       target: "idle",
                     },
                   ],
@@ -326,11 +344,7 @@ export const workspaceMachine = createMachine(
                   id: "cancelWorkspace",
                   onDone: [
                     {
-                      actions: [
-                        "assignCancellationMessage",
-                        "displayCancellationMessage",
-                        "refreshTimeline",
-                      ],
+                      actions: ["assignCancellationMessage", "displayCancellationMessage"],
                       target: "idle",
                     },
                   ],
@@ -342,31 +356,11 @@ export const workspaceMachine = createMachine(
                   ],
                 },
               },
-              refreshingTemplate: {
-                entry: "clearRefreshTemplateWarning",
-                invoke: {
-                  src: "getTemplate",
-                  id: "refreshTemplate",
-                  onDone: [
-                    {
-                      actions: "assignTemplate",
-                      target: "requestingStart",
-                    },
-                  ],
-                  onError: [
-                    {
-                      actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"],
-                      target: "idle",
-                    },
-                  ],
-                },
-              },
             },
           },
           timeline: {
             initial: "gettingBuilds",
             states: {
-              idle: {},
               gettingBuilds: {
                 entry: "clearGetBuildsError",
                 invoke: {
@@ -380,19 +374,17 @@ export const workspaceMachine = createMachine(
                   onError: [
                     {
                       actions: "assignGetBuildsError",
-                      target: "idle",
+                      target: "loadedBuilds",
                     },
                   ],
                 },
               },
               loadedBuilds: {
-                initial: "idle",
-                states: {
-                  idle: {
-                    on: {
-                      REFRESH_TIMELINE: {
-                        target: "#workspaceState.ready.timeline.gettingBuilds",
-                      },
+                on: {
+                  REFRESH_TIMELINE: {
+                    target: "#workspaceState.ready.timeline.gettingBuilds",
+                    cond: {
+                      type: "moreBuildsAvailable",
                     },
                   },
                 },
@@ -483,14 +475,14 @@ export const workspaceMachine = createMachine(
       clearRefreshWorkspaceWarning: assign({
         refreshWorkspaceWarning: (_) => undefined,
       }),
-      assignRefreshTemplateWarning: assign({
-        refreshTemplateWarning: (_, event) => event.data,
+      assignGetTemplateWarning: assign({
+        getTemplateWarning: (_, event) => event.data,
       }),
-      displayRefreshTemplateWarning: () => {
-        displayError(Language.refreshTemplateWarning)
+      displayGetTemplateWarning: () => {
+        displayError(Language.getTemplateWarning)
       },
-      clearRefreshTemplateWarning: assign({
-        refreshTemplateWarning: (_) => undefined,
+      clearGettingTemplateWarning: assign({
+        getTemplateWarning: (_) => undefined,
       }),
       // Timeline
       assignBuilds: assign({
@@ -502,24 +494,9 @@ export const workspaceMachine = createMachine(
       clearGetBuildsError: assign({
         getBuildsError: (_) => undefined,
       }),
-      refreshTimeline: pure((context, event) => {
-        // No need to refresh the timeline if it is not loaded
-        if (!context.builds) {
-          return
-        }
-
-        // When it is a CHECK_REFRESH_TIMELINE workspace event, we want to check if the latest
-        // build was updated to not over fetch the builds
-        if (event.type === "CHECK_REFRESH_TIMELINE") {
-          const latestBuildInTimeline = latestBuild(context.builds)
-          const isUpdated = event.data?.latest_build.updated_at !== latestBuildInTimeline.updated_at
-          if (isUpdated) {
-            return send({ type: "REFRESH_TIMELINE" })
-          }
-        } else {
-          return send({ type: "REFRESH_TIMELINE" })
-        }
-      }),
+    },
+    guards: {
+      moreBuildsAvailable,
     },
     services: {
       getWorkspace: async (_, event) => {
@@ -534,40 +511,55 @@ export const workspaceMachine = createMachine(
           throw Error("Cannot get template without workspace")
         }
       },
-      startWorkspaceWithLatestTemplate: async (context) => {
+      startWorkspaceWithLatestTemplate: (context) => async (send) => {
         if (context.workspace && context.template) {
-          return await API.startWorkspace(context.workspace.id, context.template.active_version_id)
+          const startWorkspacePromise = await API.startWorkspace(
+            context.workspace.id,
+            context.template.active_version_id,
+          )
+          send({ type: "REFRESH_TIMELINE" })
+          return startWorkspacePromise
         } else {
           throw Error("Cannot start workspace without workspace id")
         }
       },
-      startWorkspace: async (context) => {
+      startWorkspace: (context) => async (send) => {
         if (context.workspace) {
-          return await API.startWorkspace(
+          const startWorkspacePromise = await API.startWorkspace(
             context.workspace.id,
             context.workspace.latest_build.template_version_id,
           )
+          send({ type: "REFRESH_TIMELINE" })
+          return startWorkspacePromise
         } else {
           throw Error("Cannot start workspace without workspace id")
         }
       },
-      stopWorkspace: async (context) => {
+      stopWorkspace: (context) => async (send) => {
         if (context.workspace) {
-          return await API.stopWorkspace(context.workspace.id)
+          const stopWorkspacePromise = await API.stopWorkspace(context.workspace.id)
+          send({ type: "REFRESH_TIMELINE" })
+          return stopWorkspacePromise
         } else {
           throw Error("Cannot stop workspace without workspace id")
         }
       },
       deleteWorkspace: async (context) => {
         if (context.workspace) {
-          return await API.deleteWorkspace(context.workspace.id)
+          const deleteWorkspacePromise = await API.deleteWorkspace(context.workspace.id)
+          send({ type: "REFRESH_TIMELINE" })
+          return deleteWorkspacePromise
         } else {
           throw Error("Cannot delete workspace without workspace id")
         }
       },
-      cancelWorkspace: async (context) => {
+      cancelWorkspace: (context) => async (send) => {
         if (context.workspace) {
-          return await API.cancelWorkspaceBuild(context.workspace.latest_build.id)
+          const cancelWorkspacePromise = await API.cancelWorkspaceBuild(
+            context.workspace.latest_build.id,
+          )
+          send({ type: "REFRESH_TIMELINE" })
+          return cancelWorkspacePromise
         } else {
           throw Error("Cannot cancel workspace without build id")
         }
@@ -582,7 +574,7 @@ export const workspaceMachine = createMachine(
           // refresh our workspace with each SSE
           send({ type: "REFRESH_WORKSPACE", data: JSON.parse(event.data) })
           // refresh our timeline
-          send({ type: "CHECK_REFRESH_TIMELINE", data: JSON.parse(event.data) })
+          send({ type: "REFRESH_TIMELINE", checkRefresh: true, data: JSON.parse(event.data) })
         })
 
         // handle any error events returned by our sse