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

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

@@ -1,5 +1,7 @@
 import { action } from "@storybook/addon-actions"
 import { Story } from "@storybook/react"
+import dayjs from "dayjs"
+import { canExtendDeadline, canReduceDeadline } from "util/schedule"
 import * as Mocks from "../../testHelpers/entities"
 import { Workspace, WorkspaceErrors, WorkspaceProps } from "./Workspace"
 
@@ -24,6 +26,16 @@ Started.args = {
     onDeadlinePlus: () => {
       // do nothing, this is just for storybook
     },
+    deadlineMinusEnabled: () => {
+      return canReduceDeadline(dayjs(Mocks.MockWorkspace.latest_build.deadline))
+    },
+    deadlinePlusEnabled: () => {
+      return canExtendDeadline(
+        dayjs(Mocks.MockWorkspace.latest_build.deadline),
+        Mocks.MockWorkspace,
+        Mocks.MockTemplate,
+      )
+    },
   },
   workspace: Mocks.MockWorkspace,
   handleStart: action("start"),

site/src/components/Workspace/Workspace.tsx

@@ -31,6 +31,8 @@ export interface WorkspaceProps {
   scheduleProps: {
     onDeadlinePlus: () => void
     onDeadlineMinus: () => void
+    deadlinePlusEnabled: () => boolean
+    deadlineMinusEnabled: () => boolean
   }
   handleStart: () => void
   handleStop: () => void
@@ -81,6 +83,8 @@ export const Workspace: FC<WorkspaceProps> = ({
               workspace={workspace}
               onDeadlineMinus={scheduleProps.onDeadlineMinus}
               onDeadlinePlus={scheduleProps.onDeadlinePlus}
+              deadlineMinusEnabled={scheduleProps.deadlineMinusEnabled}
+              deadlinePlusEnabled={scheduleProps.deadlinePlusEnabled}
               canUpdateWorkspace={canUpdateWorkspace}
             />
             <WorkspaceActions

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

@@ -2,14 +2,9 @@ import dayjs from "dayjs"
 import utc from "dayjs/plugin/utc"
 import * as TypesGen from "../../api/typesGenerated"
 import * as Mocks from "../../testHelpers/entities"
-import {
-  deadlineMinusDisabled,
-  deadlinePlusDisabled,
-  shouldDisplayPlusMinus,
-} from "./WorkspaceScheduleButton"
+import { shouldDisplayPlusMinus } from "./WorkspaceScheduleButton"
 
 dayjs.extend(utc)
-const now = dayjs()
 
 describe("WorkspaceScheduleButton", () => {
   describe("shouldDisplayPlusMinus", () => {
@@ -29,92 +24,4 @@ describe("WorkspaceScheduleButton", () => {
       expect(shouldDisplayPlusMinus(workspace)).toBeTruthy()
     })
   })
-
-  describe("deadlineMinusDisabled", () => {
-    it("should be false if the deadline is more than 30 minutes in the future", () => {
-      // Given: a workspace with a deadline set to 31 minutes in the future
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(31, "minutes").utc().format(),
-        },
-      }
-
-      // Then: deadlineMinusDisabled should be falsy
-      expect(deadlineMinusDisabled(workspace, now)).toBeFalsy()
-    })
-
-    it("should be true if the deadline is 30 minutes or less in the future", () => {
-      // Given: a workspace with a deadline set to 30 minutes in the future
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(30, "minutes").utc().format(),
-        },
-      }
-
-      // Then: deadlineMinusDisabled should be truthy
-      expect(deadlineMinusDisabled(workspace, now)).toBeTruthy()
-    })
-
-    it("should be true if the deadline is in the past", () => {
-      // Given: a workspace with a deadline set to 1 minute in the past
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(-1, "minutes").utc().format(),
-        },
-      }
-
-      // Then: deadlineMinusDisabled should be truthy
-      expect(deadlineMinusDisabled(workspace, now)).toBeTruthy()
-    })
-  })
-
-  describe("deadlinePlusDisabled", () => {
-    it("should be false if the deadline is less than 24 hours in the future", () => {
-      // Given: a workspace with a deadline set to 23 hours in the future
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(23, "hours").utc().format(),
-        },
-      }
-
-      // Then: deadlinePlusDisabled should be falsy
-      expect(deadlinePlusDisabled(workspace, now)).toBeFalsy()
-    })
-
-    it("should be true if the deadline is 24 hours or more in the future", () => {
-      // Given: a workspace with a deadline set to 25 hours in the future
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(25, "hours").utc().format(),
-        },
-      }
-
-      // Then: deadlinePlusDisabled should be truthy
-      expect(deadlinePlusDisabled(workspace, now)).toBeTruthy()
-    })
-
-    it("should be false if the deadline is in the past", () => {
-      // Given: a workspace with a deadline set to 1 minute in the past
-      const workspace: TypesGen.Workspace = {
-        ...Mocks.MockWorkspace,
-        latest_build: {
-          ...Mocks.MockWorkspaceBuild,
-          deadline: now.add(-1, "minute").utc().format(),
-        },
-      }
-
-      // Then: deadlinePlusDisabled should be falsy
-      expect(deadlinePlusDisabled(workspace, now)).toBeFalsy()
-    })
-  })
 })

site/src/components/WorkspaceScheduleButton/WorkspaceScheduleButton.tsx

@@ -40,20 +40,12 @@ export const shouldDisplayPlusMinus = (workspace: Workspace): boolean => {
   return deadline.year() > 1
 }
 
-export const deadlineMinusDisabled = (workspace: Workspace, now: dayjs.Dayjs): boolean => {
-  const delta = dayjs(workspace.latest_build.deadline).diff(now)
-  return delta <= 30 * 60 * 1000 // 30 minutes
-}
-
-export const deadlinePlusDisabled = (workspace: Workspace, now: dayjs.Dayjs): boolean => {
-  const delta = dayjs(workspace.latest_build.deadline).diff(now)
-  return delta >= 24 * 60 * 60 * 1000 // 24 hours
-}
-
 export interface WorkspaceScheduleButtonProps {
   workspace: Workspace
   onDeadlinePlus: () => void
   onDeadlineMinus: () => void
+  deadlineMinusEnabled: () => boolean
+  deadlinePlusEnabled: () => boolean
   canUpdateWorkspace: boolean
 }
 
@@ -61,6 +53,8 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
   workspace,
   onDeadlinePlus,
   onDeadlineMinus,
+  deadlinePlusEnabled,
+  deadlineMinusEnabled,
   canUpdateWorkspace,
 }) => {
   const anchorRef = useRef<HTMLButtonElement>(null)
@@ -81,7 +75,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
             <IconButton
               className={styles.iconButton}
               size="small"
-              disabled={deadlineMinusDisabled(workspace, dayjs())}
+              disabled={!deadlineMinusEnabled()}
               onClick={onDeadlineMinus}
             >
               <Tooltip title={Language.editDeadlineMinus}>
@@ -91,7 +85,7 @@ export const WorkspaceScheduleButton: React.FC<WorkspaceScheduleButtonProps> = (
             <IconButton
               className={styles.iconButton}
               size="small"
-              disabled={deadlinePlusDisabled(workspace, dayjs())}
+              disabled={!deadlinePlusEnabled()}
               onClick={onDeadlinePlus}
             >
               <Tooltip title={Language.editDeadlinePlus}>

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

@@ -27,11 +27,13 @@ import { WorkspacePage } from "./WorkspacePage"
 
 // It renders the workspace page and waits for it be loaded
 const renderWorkspacePage = async () => {
+  const getTemplateMock = jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
   renderWithAuth(<WorkspacePage />, {
     route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`,
     path: "/@:username/:workspace",
   })
   await screen.findByText(MockWorkspace.name)
+  expect(getTemplateMock).toBeCalled()
 }
 
 /**
@@ -50,10 +52,10 @@ const testButton = async (label: string, actionMock: jest.SpyInstance) => {
   expect(actionMock).toBeCalled()
 }
 
-const testStatus = async (mock: Workspace, label: string) => {
+const testStatus = async (ws: Workspace, label: string) => {
   server.use(
     rest.get(`/api/v2/users/:username/workspace/:workspaceName`, (req, res, ctx) => {
-      return res(ctx.status(200), ctx.json(mock))
+      return res(ctx.status(200), ctx.json(ws))
     }),
   )
   await renderWorkspacePage()
@@ -181,6 +183,7 @@ describe("Workspace Page", () => {
 
   describe("Resources", () => {
     it("shows the status of each agent in each resource", async () => {
+      const getTemplateMock = jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
       renderWithAuth(<WorkspacePage />, {
         route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`,
         path: "/@:username/:workspace",
@@ -197,6 +200,7 @@ describe("Workspace Page", () => {
         DisplayAgentStatusLanguage[MockWorkspaceAgentDisconnected.status],
       )
       expect(agent2Status.length).toEqual(2)
+      expect(getTemplateMock).toBeCalled()
     })
   })
 })

site/src/pages/WorkspacePage/WorkspacePage.tsx

@@ -11,6 +11,7 @@ import { FullScreenLoader } from "../../components/Loader/FullScreenLoader"
 import { Workspace, WorkspaceErrors } from "../../components/Workspace/Workspace"
 import { firstOrItem } from "../../util/array"
 import { pageTitle } from "../../util/page"
+import { canExtendDeadline, canReduceDeadline, maxDeadline, minDeadline } from "../../util/schedule"
 import { getFaviconByStatus } from "../../util/workspace"
 import { selectUser } from "../../xServices/auth/authSelectors"
 import { XServiceContext } from "../../xServices/StateContext"
@@ -35,6 +36,8 @@ export const WorkspacePage: React.FC = () => {
   const {
     workspace,
     getWorkspaceError,
+    template,
+    refreshTemplateError,
     resources,
     getResourcesError,
     builds,
@@ -63,12 +66,16 @@ export const WorkspacePage: React.FC = () => {
     return (
       <div className={styles.error}>
         {getWorkspaceError && <ErrorSummary error={getWorkspaceError} />}
+        {refreshTemplateError && <ErrorSummary error={refreshTemplateError} />}
         {checkPermissionsError && <ErrorSummary error={checkPermissionsError} />}
       </div>
     )
   } else if (!workspace) {
     return <FullScreenLoader />
+  } else if (!template) {
+    return <FullScreenLoader />
   } else {
+    const deadline = dayjs(workspace.latest_build.deadline).utc()
     const favicon = getFaviconByStatus(workspace.latest_build)
     return (
       <>
@@ -85,7 +92,7 @@ export const WorkspacePage: React.FC = () => {
               bannerSend({
                 type: "UPDATE_DEADLINE",
                 workspaceId: workspace.id,
-                newDeadline: dayjs(workspace.latest_build.deadline).utc().add(4, "hours"),
+                newDeadline: dayjs.min(deadline.add(4, "hours"), maxDeadline(workspace, template)),
               })
             },
           }}
@@ -94,22 +101,22 @@ export const WorkspacePage: React.FC = () => {
               bannerSend({
                 type: "UPDATE_DEADLINE",
                 workspaceId: workspace.id,
-                newDeadline: boundedDeadline(
-                  dayjs(workspace.latest_build.deadline).utc().add(-1, "hours"),
-                  dayjs(),
-                ),
+                newDeadline: dayjs.max(deadline.add(-1, "hours"), minDeadline()),
               })
             },
             onDeadlinePlus: () => {
               bannerSend({
                 type: "UPDATE_DEADLINE",
                 workspaceId: workspace.id,
-                newDeadline: boundedDeadline(
-                  dayjs(workspace.latest_build.deadline).utc().add(1, "hours"),
-                  dayjs(),
-                ),
+                newDeadline: dayjs.min(deadline.add(1, "hours"), maxDeadline(workspace, template)),
               })
             },
+            deadlineMinusEnabled: () => {
+              return canReduceDeadline(deadline)
+            },
+            deadlinePlusEnabled: () => {
+              return canExtendDeadline(deadline, workspace, template)
+            },
           }}
           workspace={workspace}
           handleStart={() => workspaceSend("START")}
@@ -139,12 +146,6 @@ export const WorkspacePage: React.FC = () => {
   }
 }
 
-export const boundedDeadline = (newDeadline: dayjs.Dayjs, now: dayjs.Dayjs): dayjs.Dayjs => {
-  const minDeadline = now.add(30, "minutes")
-  const maxDeadline = now.add(24, "hours")
-  return dayjs.min(dayjs.max(minDeadline, newDeadline), maxDeadline)
-}
-
 const useStyles = makeStyles((theme) => ({
   error: {
     margin: theme.spacing(2),

site/src/testHelpers/entities.ts

@@ -151,8 +151,8 @@ export const MockTemplate: TypesGen.Template = {
   active_version_id: MockTemplateVersion.id,
   workspace_owner_count: 1,
   description: "This is a test description.",
-  max_ttl_ms: 604800000,
-  min_autostart_interval_ms: 3600000,
+  max_ttl_ms: 24 * 60 * 60 * 1000,
+  min_autostart_interval_ms: 60 * 60 * 1000,
   created_by_id: "test-creator-id",
   created_by_name: "test_creator",
 }

site/src/util/schedule.test.ts

@@ -1,4 +1,20 @@
-import { extractTimezone, stripTimezone } from "./schedule"
+import dayjs from "dayjs"
+import duration from "dayjs/plugin/duration"
+import { Template, Workspace } from "../api/typesGenerated"
+import * as Mocks from "../testHelpers/entities"
+import {
+  canExtendDeadline,
+  canReduceDeadline,
+  deadlineExtensionMax,
+  deadlineExtensionMin,
+  extractTimezone,
+  maxDeadline,
+  minDeadline,
+  stripTimezone,
+} from "./schedule"
+
+dayjs.extend(duration)
+const now = dayjs()
 
 describe("util/schedule", () => {
   describe("stripTimezone", () => {
@@ -21,3 +37,80 @@ describe("util/schedule", () => {
     })
   })
 })
+
+describe("maxDeadline", () => {
+  // Given: a workspace built from a template with a max deadline equal to 25 hours which isn't really possible
+  const workspace: Workspace = {
+    ...Mocks.MockWorkspace,
+    latest_build: {
+      ...Mocks.MockWorkspaceBuild,
+      deadline: now.add(8, "hours").utc().format(),
+    },
+  }
+  describe("given a template with 25 hour max ttl", () => {
+    it("should be never be greater than global max deadline", () => {
+      const template: Template = {
+        ...Mocks.MockTemplate,
+        max_ttl_ms: 25 * 60 * 60 * 1000,
+      }
+
+      // Then: deadlineMinusDisabled should be falsy
+      const delta = maxDeadline(workspace, template).diff(now)
+      expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
+    })
+  })
+
+  describe("given a template with 4 hour max ttl", () => {
+    it("should be never be greater than global max deadline", () => {
+      const template: Template = {
+        ...Mocks.MockTemplate,
+        max_ttl_ms: 4 * 60 * 60 * 1000,
+      }
+
+      // Then: deadlineMinusDisabled should be falsy
+      const delta = maxDeadline(workspace, template).diff(now)
+      expect(delta).toBeLessThanOrEqual(deadlineExtensionMax.asMilliseconds())
+    })
+  })
+})
+
+describe("minDeadline", () => {
+  it("should never be less than 30 minutes", () => {
+    const delta = minDeadline().diff(now)
+    expect(delta).toBeGreaterThanOrEqual(deadlineExtensionMin.asMilliseconds())
+  })
+})
+
+describe("canExtendDeadline", () => {
+  it("should be falsy if the deadline is more than 24 hours in the future", () => {
+    expect(
+      canExtendDeadline(dayjs().add(25, "hours"), Mocks.MockWorkspace, Mocks.MockTemplate),
+    ).toBeFalsy()
+  })
+
+  it("should be falsy if the deadline is more than the template max_ttl", () => {
+    const tooFarAhead = dayjs().add(dayjs.duration(Mocks.MockTemplate.max_ttl_ms, "milliseconds"))
+    expect(canExtendDeadline(tooFarAhead, Mocks.MockWorkspace, Mocks.MockTemplate)).toBeFalsy()
+  })
+
+  it("should be truth if the deadline is within the template max_ttl", () => {
+    const okDeadline = dayjs().add(
+      dayjs.duration(Mocks.MockTemplate.max_ttl_ms / 2, "milliseconds"),
+    )
+    expect(canExtendDeadline(okDeadline, Mocks.MockWorkspace, Mocks.MockTemplate)).toBeFalsy()
+  })
+})
+
+describe("canReduceDeadline", () => {
+  it("should be falsy if the deadline is 30 minutes or less in the future", () => {
+    expect(canReduceDeadline(dayjs())).toBeFalsy()
+    expect(canReduceDeadline(dayjs().add(1, "minutes"))).toBeFalsy()
+    expect(canReduceDeadline(dayjs().add(29, "minutes"))).toBeFalsy()
+    expect(canReduceDeadline(dayjs().add(30, "minutes"))).toBeFalsy()
+  })
+
+  it("should be truthy if the deadline is 30 minutes or more in the future", () => {
+    expect(canReduceDeadline(dayjs().add(31, "minutes"))).toBeTruthy()
+    expect(canReduceDeadline(dayjs().add(100, "years"))).toBeTruthy()
+  })
+})

site/src/util/schedule.ts

@@ -5,7 +5,7 @@ import duration from "dayjs/plugin/duration"
 import relativeTime from "dayjs/plugin/relativeTime"
 import timezone from "dayjs/plugin/timezone"
 import utc from "dayjs/plugin/utc"
-import { Workspace } from "../api/typesGenerated"
+import { Template, Workspace } from "../api/typesGenerated"
 import { isWorkspaceOn } from "./workspace"
 
 // REMARK: some plugins depend on utc, so it's listed first. Otherwise they're
@@ -110,3 +110,31 @@ export const autoStopDisplay = (workspace: Workspace): string => {
     return `${duration.humanize()} ${Language.afterStart}`
   }
 }
+
+export const deadlineExtensionMin = dayjs.duration(30, "minutes")
+export const deadlineExtensionMax = dayjs.duration(24, "hours")
+
+export function maxDeadline(ws: Workspace, tpl: Template): dayjs.Dayjs {
+  // note: we count runtime from updated_at as started_at counts from the start of
+  // the workspace build process, which can take a while.
+  const startedAt = dayjs(ws.latest_build.updated_at)
+  const maxTemplateDeadline = startedAt.add(dayjs.duration(tpl.max_ttl_ms, "milliseconds"))
+  const maxGlobalDeadline = startedAt.add(deadlineExtensionMax)
+  return dayjs.min(maxTemplateDeadline, maxGlobalDeadline)
+}
+
+export function minDeadline(): dayjs.Dayjs {
+  return dayjs().add(deadlineExtensionMin)
+}
+
+export function canExtendDeadline(
+  deadline: dayjs.Dayjs,
+  workspace: Workspace,
+  template: Template,
+): boolean {
+  return deadline < maxDeadline(workspace, template)
+}
+
+export function canReduceDeadline(deadline: dayjs.Dayjs): boolean {
+  return deadline > minDeadline()
+}

site/src/util/workspace.ts

@@ -1,10 +1,14 @@
 import { Theme } from "@material-ui/core/styles"
 import dayjs from "dayjs"
+import duration from "dayjs/plugin/duration"
+import minMax from "dayjs/plugin/minMax"
 import utc from "dayjs/plugin/utc"
 import { WorkspaceBuildTransition } from "../api/types"
 import * as TypesGen from "../api/typesGenerated"
 
+dayjs.extend(duration)
 dayjs.extend(utc)
+dayjs.extend(minMax)
 
 // all the possible states returned by the API
 export enum WorkspaceStateEnum {

site/src/xServices/workspace/workspaceXService.ts

@@ -136,7 +136,7 @@ export const workspaceMachine = createMachine(
           src: "getWorkspace",
           id: "getWorkspace",
           onDone: {
-            target: "gettingPermissions",
+            target: "refreshingTemplate",
             actions: ["assignWorkspace"],
           },
           onError: {
@@ -146,6 +146,22 @@ export const workspaceMachine = createMachine(
         },
         tags: "loading",
       },
+      refreshingTemplate: {
+        entry: ["clearRefreshTemplateError"],
+        invoke: {
+          id: "refreshTemplate",
+          src: "getTemplate",
+          onDone: {
+            target: "gettingPermissions",
+            actions: ["assignTemplate"],
+          },
+          onError: {
+            target: "error",
+            actions: ["assignRefreshTemplateError", "displayRefreshTemplateError"],
+          },
+        },
+        tags: "loading",
+      },
       gettingPermissions: {
         entry: "clearGetPermissionsError",
         invoke: {