From 7f3cd9004a3f8c415cab694d0308d83096e94cb1 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 21 Sep 2022 20:19:53 +0000 Subject: [PATCH 1/6] removed dead build states --- .../xServices/workspace/workspaceXService.ts | 54 +------------------ 1 file changed, 1 insertion(+), 53 deletions(-) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index 144f128670779..39ef77094116e 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -32,7 +32,7 @@ export interface WorkspaceContext { // Builds builds?: TypesGen.WorkspaceBuild[] getBuildsError?: Error | unknown - loadMoreBuildsError?: Error | unknown + // loadMoreBuildsError?: Error | unknown // error creating a new WorkspaceBuild buildError?: Error | unknown cancellationMessage?: Types.Message @@ -53,7 +53,6 @@ export type WorkspaceEvent = | { type: "CANCEL_DELETE" } | { type: "UPDATE" } | { type: "CANCEL" } - | { type: "LOAD_MORE_BUILDS" } | { type: "CHECK_REFRESH_TIMELINE"; data: TypesGen.ServerSentEvent["data"] } | { type: "REFRESH_TIMELINE" } | { type: "EVENT_SOURCE_ERROR"; error: Error | unknown } @@ -118,9 +117,6 @@ export const workspaceMachine = createMachine( getBuilds: { data: TypesGen.WorkspaceBuild[] } - loadMoreBuilds: { - data: TypesGen.WorkspaceBuild[] - } checkPermissions: { data: TypesGen.UserAuthorizationResponse } @@ -396,33 +392,11 @@ export const workspaceMachine = createMachine( states: { idle: { on: { - LOAD_MORE_BUILDS: { - cond: "hasMoreBuilds", - target: "loadingMoreBuilds", - }, REFRESH_TIMELINE: { target: "#workspaceState.ready.timeline.gettingBuilds", }, }, }, - loadingMoreBuilds: { - entry: "clearLoadMoreBuildsError", - invoke: { - src: "loadMoreBuilds", - onDone: [ - { - actions: "assignNewBuilds", - target: "idle", - }, - ], - onError: [ - { - actions: "assignLoadMoreBuildsError", - target: "idle", - }, - ], - }, - }, }, }, }, @@ -530,24 +504,6 @@ export const workspaceMachine = createMachine( clearGetBuildsError: assign({ getBuildsError: (_) => undefined, }), - assignNewBuilds: assign({ - builds: (context, event) => { - const oldBuilds = context.builds - - if (!oldBuilds) { - // This state is theoretically impossible, but helps TS - throw new Error("workspaceXService: failed to load workspace builds") - } - - return [...oldBuilds, ...event.data] - }, - }), - assignLoadMoreBuildsError: assign({ - loadMoreBuildsError: (_, event) => event.data, - }), - clearLoadMoreBuildsError: assign({ - loadMoreBuildsError: (_) => undefined, - }), refreshTimeline: pure((context, event) => { // No need to refresh the timeline if it is not loaded if (!context.builds) { @@ -568,7 +524,6 @@ export const workspaceMachine = createMachine( }), }, guards: { - hasMoreBuilds: (_) => false, }, services: { getWorkspace: async (_, event) => { @@ -651,13 +606,6 @@ export const workspaceMachine = createMachine( throw Error("Cannot get builds without id") } }, - loadMoreBuilds: async (context) => { - if (context.workspace) { - return await API.getWorkspaceBuilds(context.workspace.id) - } else { - throw Error("Cannot load more builds without id") - } - }, checkPermissions: async (context) => { if (context.workspace && context.userId) { return await API.checkUserPermissions(context.userId, { From 8740c82eeb6933c4dfb84cdca1a916b9cbc58e16 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 21 Sep 2022 20:21:09 +0000 Subject: [PATCH 2/6] removed dead code --- site/src/xServices/workspace/workspaceXService.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index 39ef77094116e..390028194f21c 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -32,7 +32,6 @@ export interface WorkspaceContext { // Builds builds?: TypesGen.WorkspaceBuild[] getBuildsError?: Error | unknown - // loadMoreBuildsError?: Error | unknown // error creating a new WorkspaceBuild buildError?: Error | unknown cancellationMessage?: Types.Message From 0cd5889b3633a34bd41f50d1a0ca3dc22ccf6a99 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 21 Sep 2022 21:19:27 +0000 Subject: [PATCH 3/6] removed guards --- site/src/xServices/workspace/workspaceXService.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index 390028194f21c..0db51aa54fcb5 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -522,8 +522,6 @@ export const workspaceMachine = createMachine( } }), }, - guards: { - }, services: { getWorkspace: async (_, event) => { return await API.getWorkspaceByOwnerAndName(event.username, event.workspaceName, { From 463708b5dbf8c7416743104d91dcb24061d0c3e1 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Thu, 22 Sep 2022 19:53:25 +0000 Subject: [PATCH 4/6] not calling events from actions --- .../xServices/workspace/workspaceXService.ts | 88 ++++++++++--------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index 0db51aa54fcb5..a04a02366121a 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/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,6 +11,22 @@ 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.", buildError: "Workspace action failed.", @@ -52,8 +67,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 = { @@ -211,9 +225,6 @@ export const workspaceMachine = createMachine( EVENT_SOURCE_ERROR: { target: "error", }, - CHECK_REFRESH_TIMELINE: { - actions: ["refreshTimeline"], - }, }, }, error: { @@ -255,7 +266,7 @@ export const workspaceMachine = createMachine( src: "startWorkspaceWithLatestTemplate", onDone: { target: "idle", - actions: ["assignBuild", "refreshTimeline"], + actions: ["assignBuild"], }, onError: { target: "idle", @@ -270,7 +281,7 @@ export const workspaceMachine = createMachine( id: "startWorkspace", onDone: [ { - actions: ["assignBuild", "refreshTimeline"], + actions: ["assignBuild"], target: "idle", }, ], @@ -289,7 +300,7 @@ export const workspaceMachine = createMachine( id: "stopWorkspace", onDone: [ { - actions: ["assignBuild", "refreshTimeline"], + actions: ["assignBuild"], target: "idle", }, ], @@ -308,7 +319,7 @@ export const workspaceMachine = createMachine( id: "deleteWorkspace", onDone: [ { - actions: ["assignBuild", "refreshTimeline"], + actions: ["assignBuild"], target: "idle", }, ], @@ -330,7 +341,6 @@ export const workspaceMachine = createMachine( actions: [ "assignCancellationMessage", "displayCancellationMessage", - "refreshTimeline", ], target: "idle", }, @@ -393,6 +403,9 @@ export const workspaceMachine = createMachine( on: { REFRESH_TIMELINE: { target: "#workspaceState.ready.timeline.gettingBuilds", + cond: { + type: 'moreBuildsAvailable' + } }, }, }, @@ -503,24 +516,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) => { @@ -535,40 +533,50 @@ 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") } @@ -583,7 +591,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 @@ -591,7 +599,7 @@ export const workspaceMachine = createMachine( send({ type: "EVENT_SOURCE_ERROR", error: event }) }) - // handle any sse implementation exceptions + // handle any sse implementation exceptions context.eventSource.onerror = () => { send({ type: "EVENT_SOURCE_ERROR", error: "sse error" }) } From ee657f280d6c29efd9ebcd209c1edaaa863757e8 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Fri, 23 Sep 2022 14:33:27 +0000 Subject: [PATCH 5/6] simplified timeline --- .../xServices/workspace/workspaceXService.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index dba5c4118f91a..e2466fb277753 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -381,7 +381,6 @@ export const workspaceMachine = createMachine( timeline: { initial: "gettingBuilds", states: { - idle: {}, gettingBuilds: { entry: "clearGetBuildsError", invoke: { @@ -395,22 +394,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", - cond: { - type: "moreBuildsAvailable", - }, - }, + on: { + REFRESH_TIMELINE: { + target: "#workspaceState.ready.timeline.gettingBuilds", + cond: { + type: "moreBuildsAvailable", }, }, }, From d515a698795c20dc5b956d438bdeefa2219cafc1 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Fri, 23 Sep 2022 15:24:25 +0000 Subject: [PATCH 6/6] simplify refresh template --- .../src/pages/WorkspacePage/WorkspacePage.tsx | 4 +- .../xServices/workspace/workspaceXService.ts | 45 ++++++------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/site/src/pages/WorkspacePage/WorkspacePage.tsx b/site/src/pages/WorkspacePage/WorkspacePage.tsx index 6b243aaa500a2..3f37593a8b707 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.tsx @@ -43,7 +43,7 @@ export const WorkspacePage: FC = () => { workspace, getWorkspaceError, template, - refreshTemplateWarning, + getTemplateWarning, refreshWorkspaceWarning, builds, getBuildsError, @@ -72,7 +72,7 @@ export const WorkspacePage: FC = () => { return (
{Boolean(getWorkspaceError) && } - {Boolean(refreshTemplateWarning) && } + {Boolean(getTemplateWarning) && } {Boolean(checkPermissionsError) && }
) diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts index e2466fb277753..5a1318e3a5abf 100644 --- a/site/src/xServices/workspace/workspaceXService.ts +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -35,7 +35,7 @@ const moreBuildsAvailable = ( } 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.", } @@ -50,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 @@ -161,7 +161,7 @@ export const workspaceMachine = createMachine( onDone: [ { actions: "assignWorkspace", - target: "refreshingTemplate", + target: "gettingTemplate", }, ], onError: [ @@ -173,11 +173,11 @@ export const workspaceMachine = createMachine( }, tags: "loading", }, - refreshingTemplate: { - entry: "clearRefreshTemplateWarning", + gettingTemplate: { + entry: "clearGettingTemplateWarning", invoke: { src: "getTemplate", - id: "refreshTemplate", + id: "getTemplate", onDone: [ { actions: "assignTemplate", @@ -186,7 +186,7 @@ export const workspaceMachine = createMachine( ], onError: [ { - actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"], + actions: ["assignGetTemplateWarning", "displayGetTemplateWarning"], target: "error", }, ], @@ -357,25 +357,6 @@ export const workspaceMachine = createMachine( ], }, }, - refreshingTemplate: { - entry: "clearRefreshTemplateWarning", - invoke: { - src: "getTemplate", - id: "refreshTemplate", - onDone: [ - { - actions: "assignTemplate", - target: "requestingStart", - }, - ], - onError: [ - { - actions: ["assignRefreshTemplateWarning", "displayRefreshTemplateWarning"], - target: "idle", - }, - ], - }, - }, }, }, timeline: { @@ -495,14 +476,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({