From 18ad06441df951becdbebd06f0669ce022f86d14 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:27:47 +0000 Subject: [PATCH 1/6] fix: revamp UI for batch-updating workspaces --- .../BatchUpdateConfirmation.stories.tsx | 86 --- .../BatchUpdateConfirmation.tsx | 494 -------------- .../BatchUpdateModalForm.stories.tsx | 277 ++++++++ .../WorkspacesPage/BatchUpdateModalForm.tsx | 634 ++++++++++++++++++ .../pages/WorkspacesPage/WorkspacesPage.tsx | 14 +- 5 files changed, 918 insertions(+), 587 deletions(-) delete mode 100644 site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx delete mode 100644 site/src/pages/WorkspacesPage/BatchUpdateConfirmation.tsx create mode 100644 site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx create mode 100644 site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx diff --git a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx deleted file mode 100644 index 140d433d3e860..0000000000000 --- a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.stories.tsx +++ /dev/null @@ -1,86 +0,0 @@ -import { action } from "@storybook/addon-actions"; -import type { Meta, StoryObj } from "@storybook/react"; -import type { Workspace } from "api/typesGenerated"; -import { useQueryClient } from "react-query"; -import { chromatic } from "testHelpers/chromatic"; -import { - MockDormantOutdatedWorkspace, - MockOutdatedWorkspace, - MockRunningOutdatedWorkspace, - MockTemplateVersion, - MockUserMember, - MockWorkspace, -} from "testHelpers/entities"; -import { - BatchUpdateConfirmation, - type Update, -} from "./BatchUpdateConfirmation"; - -const workspaces: Workspace[] = [ - { ...MockRunningOutdatedWorkspace, id: "1" }, - { ...MockDormantOutdatedWorkspace, id: "2" }, - { ...MockOutdatedWorkspace, id: "3" }, - { ...MockOutdatedWorkspace, id: "4" }, - { ...MockWorkspace, id: "5" }, - { - ...MockRunningOutdatedWorkspace, - id: "6", - owner_id: MockUserMember.id, - owner_name: MockUserMember.username, - }, -]; - -function getPopulatedUpdates(): Map { - type MutableUpdate = Omit & { - affected_workspaces: Workspace[]; - }; - - const updates = new Map(); - for (const it of workspaces) { - const versionId = it.template_active_version_id; - const version = updates.get(versionId); - - if (version) { - version.affected_workspaces.push(it); - continue; - } - - updates.set(versionId, { - ...MockTemplateVersion, - template_display_name: it.template_display_name, - affected_workspaces: [it], - }); - } - - return updates as Map; -} - -const updates = getPopulatedUpdates(); - -const meta: Meta = { - title: "pages/WorkspacesPage/BatchUpdateConfirmation", - parameters: { chromatic }, - component: BatchUpdateConfirmation, - decorators: [ - (Story) => { - const queryClient = useQueryClient(); - for (const [id, it] of updates) { - queryClient.setQueryData(["batchUpdate", id], it); - } - return ; - }, - ], - args: { - onClose: action("onClose"), - onConfirm: action("onConfirm"), - open: true, - checkedWorkspaces: workspaces, - }, -}; - -export default meta; -type Story = StoryObj; - -const Example: Story = {}; - -export { Example as BatchUpdateConfirmation }; diff --git a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.tsx b/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.tsx deleted file mode 100644 index a6b0a27b374f4..0000000000000 --- a/site/src/pages/WorkspacesPage/BatchUpdateConfirmation.tsx +++ /dev/null @@ -1,494 +0,0 @@ -import type { Interpolation, Theme } from "@emotion/react"; -import { API } from "api/api"; -import type { TemplateVersion, Workspace } from "api/typesGenerated"; -import { ErrorAlert } from "components/Alert/ErrorAlert"; -import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog"; -import { Loader } from "components/Loader/Loader"; -import { MemoizedInlineMarkdown } from "components/Markdown/Markdown"; -import { Stack } from "components/Stack/Stack"; -import dayjs from "dayjs"; -import relativeTime from "dayjs/plugin/relativeTime"; -import { MonitorDownIcon } from "lucide-react"; -import { ClockIcon, SettingsIcon, UserIcon } from "lucide-react"; -import { type FC, type ReactNode, useEffect, useMemo, useState } from "react"; -import { useQueries } from "react-query"; - -dayjs.extend(relativeTime); - -type BatchUpdateConfirmationProps = { - checkedWorkspaces: readonly Workspace[]; - open: boolean; - isLoading: boolean; - onClose: () => void; - onConfirm: () => void; -}; - -export interface Update extends TemplateVersion { - template_display_name: string; - affected_workspaces: readonly Workspace[]; -} - -export const BatchUpdateConfirmation: FC = ({ - checkedWorkspaces, - open, - onClose, - onConfirm, - isLoading, -}) => { - // Ignore workspaces with no pending update - const outdatedWorkspaces = useMemo( - () => checkedWorkspaces.filter((workspace) => workspace.outdated), - [checkedWorkspaces], - ); - - // Separate out dormant workspaces. You cannot update a dormant workspace without - // activate it, so notify the user that these selected workspaces will not be updated. - const [dormantWorkspaces, workspacesToUpdate] = useMemo(() => { - const dormantWorkspaces = []; - const workspacesToUpdate = []; - - for (const it of outdatedWorkspaces) { - if (it.dormant_at) { - dormantWorkspaces.push(it); - } else { - workspacesToUpdate.push(it); - } - } - - return [dormantWorkspaces, workspacesToUpdate]; - }, [outdatedWorkspaces]); - - // We need to know which workspaces are running, so we can provide more detailed - // warnings about them - const runningWorkspacesToUpdate = useMemo( - () => - workspacesToUpdate.filter( - (workspace) => workspace.latest_build.status === "running", - ), - [workspacesToUpdate], - ); - - // If there aren't any running _and_ outdated workspaces selected, we can skip - // the consequences page, since an update shouldn't have any consequences that - // the stop didn't already. If there are dormant workspaces but no running - // workspaces, start there instead. - const [stage, setStage] = useState< - "consequences" | "dormantWorkspaces" | "updates" | null - >(null); - // biome-ignore lint/correctness/useExhaustiveDependencies: consider refactoring - useEffect(() => { - if (runningWorkspacesToUpdate.length > 0) { - setStage("consequences"); - } else if (dormantWorkspaces.length > 0) { - setStage("dormantWorkspaces"); - } else { - setStage("updates"); - } - }, [runningWorkspacesToUpdate, dormantWorkspaces, checkedWorkspaces, open]); - - // Figure out which new versions everything will be updated to so that we can - // show update messages and such. - const newVersions = useMemo(() => { - type MutableUpdateInfo = { - id: string; - template_display_name: string; - affected_workspaces: Workspace[]; - }; - - const newVersions = new Map(); - for (const it of workspacesToUpdate) { - const versionId = it.template_active_version_id; - const version = newVersions.get(versionId); - - if (version) { - version.affected_workspaces.push(it); - continue; - } - - newVersions.set(versionId, { - id: versionId, - template_display_name: it.template_display_name, - affected_workspaces: [it], - }); - } - - type ReadonlyUpdateInfo = Readonly & { - affected_workspaces: readonly Workspace[]; - }; - - return newVersions as Map; - }, [workspacesToUpdate]); - - // Not all of the information we want is included in the `Workspace` type, so we - // need to query all of the versions. - const results = useQueries({ - queries: [...newVersions.values()].map((version) => ({ - queryKey: ["batchUpdate", version.id], - queryFn: async () => ({ - // ...but the query _also_ doesn't have everything we need, like the - // template display name! - ...version, - ...(await API.getTemplateVersion(version.id)), - }), - })), - }); - const { data, error } = { - data: results.every((result) => result.isSuccess && result.data) - ? results.map((result) => result.data!) - : undefined, - error: results.some((result) => result.error), - }; - - const onProceed = () => { - switch (stage) { - case "updates": - onConfirm(); - break; - case "dormantWorkspaces": - setStage("updates"); - break; - case "consequences": - setStage( - dormantWorkspaces.length > 0 ? "dormantWorkspaces" : "updates", - ); - break; - } - }; - - const workspaceCount = `${workspacesToUpdate.length} ${ - workspacesToUpdate.length === 1 ? "workspace" : "workspaces" - }`; - - let confirmText: ReactNode = <>Review updates…; - if (stage === "updates") { - confirmText = <>Update {workspaceCount}; - } - - return ( - - {stage === "consequences" && ( - - )} - {stage === "dormantWorkspaces" && ( - - )} - {stage === "updates" && ( - - )} - - } - /> - ); -}; - -interface ConsequencesProps { - runningWorkspaces: Workspace[]; -} - -const Consequences: FC = ({ runningWorkspaces }) => { - const workspaceCount = `${runningWorkspaces.length} ${ - runningWorkspaces.length === 1 ? "running workspace" : "running workspaces" - }`; - - const owners = new Set(runningWorkspaces.map((it) => it.owner_id)).size; - const ownerCount = `${owners} ${owners === 1 ? "owner" : "owners"}`; - - return ( - <> -

You are about to update {workspaceCount}.

-
    -
  • - Updating will start workspaces on their latest template versions. This - can delete non-persistent data. -
    • -
  • - Anyone connected to a running workspace will be disconnected until the - update is complete. -
    • -
  • Any unsaved data will be lost.
    • -
- - - - {ownerCount} - - - - ); -}; - -interface DormantWorkspacesProps { - workspaces: Workspace[]; -} - -const DormantWorkspaces: FC = ({ workspaces }) => { - const mostRecent = workspaces.reduce( - (latestSoFar, against) => { - if (!latestSoFar) { - return against; - } - - return new Date(against.last_used_at).getTime() > - new Date(latestSoFar.last_used_at).getTime() - ? against - : latestSoFar; - }, - undefined as Workspace | undefined, - ); - - const owners = new Set(workspaces.map((it) => it.owner_id)).size; - const ownersCount = `${owners} ${owners === 1 ? "owner" : "owners"}`; - - return ( - <> -

- {workspaces.length === 1 ? ( - <> - This selected workspace is dormant, and must be activated before it - can be updated. - - ) : ( - <> - These selected workspaces are dormant, and must be activated before - they can be updated. - - )} -

-
    - {workspaces.map((workspace) => ( -
  • - - {workspace.name} - - - - - {workspace.owner_name} - - - - - - {lastUsed(workspace.last_used_at)} - - - - -
    • - ))} -
- - - - {ownersCount} - - {mostRecent && ( - - - Last used {lastUsed(mostRecent.last_used_at)} - - )} - - - ); -}; - -interface UpdatesProps { - workspaces: Workspace[]; - updates?: Update[]; - error?: unknown; -} - -const Updates: FC = ({ workspaces, updates, error }) => { - const workspaceCount = `${workspaces.length} ${ - workspaces.length === 1 ? "outdated workspace" : "outdated workspaces" - }`; - - const updateCount = - updates && - `${updates.length} ${ - updates.length === 1 ? "new version" : "new versions" - }`; - - return ( - <> - - - - - {workspaceCount} - - {updateCount && ( - - - {updateCount} - - )} - - - ); -}; - -interface TemplateVersionMessagesProps { - error?: unknown; - updates?: Update[]; -} - -const TemplateVersionMessages: FC = ({ - error, - updates, -}) => { - if (error) { - return ; - } - - if (!updates) { - return ; - } - - return ( -
    - {updates.map((update) => ( -
  • - - - {update.template_display_name} - → {update.name} - - - {update.message ?? "No message"} - - - -
    • - ))} -
- ); -}; - -interface UsedByProps { - workspaces: readonly Workspace[]; -} - -const UsedBy: FC = ({ workspaces }) => { - const workspaceNames = workspaces.map((it) => it.name); - - return ( -

- Used by {workspaceNames.slice(0, 2).join(", ")}{" "} - {workspaceNames.length > 2 && ( - - and {workspaceNames.length - 2} more - - )} -

- ); -}; - -const lastUsed = (time: string) => { - const now = dayjs(); - const then = dayjs(time); - return then.isAfter(now.subtract(1, "hour")) ? "now" : then.fromNow(); -}; - -const PersonIcon: FC = () => { - // Using the Lucide icon with appropriate size class - return ; -}; - -const styles = { - summaryIcon: { width: 16, height: 16 }, - - consequences: { - display: "flex", - flexDirection: "column", - gap: 8, - paddingLeft: 16, - }, - - workspacesList: (theme) => ({ - listStyleType: "none", - padding: 0, - border: `1px solid ${theme.palette.divider}`, - borderRadius: 8, - overflow: "hidden auto", - maxHeight: 184, - }), - - updatesList: (theme) => ({ - listStyleType: "none", - padding: 0, - border: `1px solid ${theme.palette.divider}`, - borderRadius: 8, - overflow: "hidden auto", - maxHeight: 256, - }), - - workspace: (theme) => ({ - padding: "8px 16px", - borderBottom: `1px solid ${theme.palette.divider}`, - - "&:last-child": { - border: "none", - }, - }), - - name: (theme) => ({ - fontWeight: 500, - color: theme.experimental.l1.text, - }), - - newVersion: (theme) => ({ - fontSize: 13, - fontWeight: 500, - color: theme.roles.active.fill.solid, - }), - - message: { - fontSize: 14, - }, - - summary: { - gap: "6px 20px", - fontSize: 14, - }, -} satisfies Record>; diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx new file mode 100644 index 0000000000000..8b44f02d07fe1 --- /dev/null +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -0,0 +1,277 @@ +import type { Meta, Parameters, StoryObj } from "@storybook/react"; +import { expect, screen, userEvent, within } from "@storybook/test"; +import { templateVersionRoot } from "api/queries/templates"; +import type { + TemplateVersion, + Workspace, + WorkspaceBuild, +} from "api/typesGenerated"; +import { useQueryClient } from "react-query"; +import { MockTemplateVersion, MockWorkspace } from "testHelpers/entities"; +import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; +import { ACTIVE_BUILD_STATUSES } from "./WorkspacesPage"; + +type Writeable = { -readonly [Key in keyof T]: T[Key] }; +type MutableWorkspace = Writeable> & { + latest_build: Writeable; +}; + +const meta: Meta = { + title: "pages/WorkspacesPage/BatchUpdateModalForm", + component: BatchUpdateModalForm, + args: { + open: true, + isProcessing: false, + onSubmit: () => window.alert("Hooray! Everything has been submitted"), + // Since we're using Radix, any cancel functionality is also going to + // trigger when you click outside the component bounds, which would make + // doing an alert really annoying in the Storybook web UI + onCancel: () => console.log("Canceled"), + }, +}; + +export default meta; +type Story = StoryObj; + +type QueryEntry = NonNullable; + +type PatchedDependencies = Readonly<{ + workspaces: readonly Workspace[]; + queries: QueryEntry; +}>; +function createPatchedDependencies(size: number): PatchedDependencies { + const workspaces: Workspace[] = []; + const queries: QueryEntry = []; + + for (let i = 1; i <= size; i++) { + const patchedTemplateVersion: TemplateVersion = { + ...MockTemplateVersion, + id: `${MockTemplateVersion.id}-${i}`, + name: `${MockTemplateVersion.name}-${i}`, + }; + + const patchedWorkspace: Workspace = { + ...MockWorkspace, + outdated: true, + id: `${MockWorkspace.id}-${i}`, + template_active_version_id: patchedTemplateVersion.id, + name: `${MockWorkspace.name}-${i}`, + + latest_build: { + ...MockWorkspace.latest_build, + status: "stopped", + }, + }; + + workspaces.push(patchedWorkspace); + queries.push({ + key: [templateVersionRoot, patchedWorkspace.template_active_version_id], + data: patchedTemplateVersion, + }); + } + + return { workspaces, queries }; +} + +export const NoWorkspacesSelected: Story = { + args: { + workspacesToUpdate: [], + }, +}; + +export const OnlyReadyToUpdate: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +export const NoWorkspacesToUpdate: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + for (const ws of workspaces) { + const writable = ws as MutableWorkspace; + writable.outdated = false; + } + + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +export const CurrentlyProcessing: Story = { + args: { isProcessing: true }, + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +/** + * @todo 2025-07-15 - Need to figure out if there's a decent way to validate + * that the onCancel callback gets called when you press the "Close" button, + * without going into Jest+RTL. + */ +export const OnlyDormantWorkspaces: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + for (const ws of workspaces) { + const writable = ws as MutableWorkspace; + writable.dormant_at = new Date().toISOString(); + } + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +export const FetchError: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, + decorators: [ + (Story, ctx) => { + const queryClient = useQueryClient(); + queryClient.clear(); + + for (const ws of ctx.args.workspacesToUpdate) { + void queryClient.fetchQuery({ + queryKey: [templateVersionRoot, ws.template_active_version_id], + queryFn: () => { + throw new Error("Workspaces? Sir, this is a Wendy's."); + }, + }); + } + + return ; + }, + ], +}; + +export const TransitioningWorkspaces: Story = { + args: { isProcessing: true }, + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies( + 2 * ACTIVE_BUILD_STATUSES.length, + ); + for (const [i, ws] of workspaces.entries()) { + if (i % 2 === 0) { + continue; + } + const writable = ws.latest_build as Writeable; + writable.status = ACTIVE_BUILD_STATUSES[i % ACTIVE_BUILD_STATUSES.length]; + } + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +export const RunningWorkspaces: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + for (const ws of workspaces) { + const writable = ws.latest_build as Writeable; + writable.status = "running"; + } + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; + +export const RunningWorkspacesFailedValidation: Story = { + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(3); + for (const ws of workspaces) { + const writable = ws.latest_build as Writeable; + writable.status = "running"; + } + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, + play: async () => { + // Can't use canvasElement from the play function's context because the + // component node uses React Portals and won't be part of the main + // canvas body + const modal = within( + screen.getByRole("dialog", { name: "Review updates" }), + ); + + const updateButton = modal.getByRole("button", { name: "Update" }); + await userEvent.click(updateButton, { + /** + * @todo 2025-07-15 - Something in the test setup is causing the + * Update button to get treated as though it should opt out of + * pointer events, which causes userEvent to break. All of our code + * seems to be fine - we do have logic to disable pointer events, + * but only when the button is obviously configured wrong (e.g., + * it's configured as a link but has no URL). + * + * Disabling this check makes things work again, but shoots our + * confidence for how accessible the UI is, even if we know that at + * this point, the button exists, has the right text content, and is + * not disabled. + * + * We should aim to remove this property as soon as possible, + * opening up an issue upstream if necessary. + */ + pointerEventsCheck: 0, + }); + await modal.findByText("Please acknowledge consequences to continue."); + + const checkbox = modal.getByRole("checkbox", { + name: /I acknowledge these consequences\./, + }); + expect(checkbox).toHaveFocus(); + }, +}; + +export const MixOfWorkspaces: Story = { + args: { isProcessing: true }, + /** + * List of all workspace kinds we're trying to represent here: + * - Ready to update + stopped + * - Ready to update + running + * - Ready to update + transitioning + * - Dormant + * - Not outdated + stopped + * - Not outdated + transitioning + * + * Deliberately omitted: + * - Not outdated + running (the update logic should skip the workspace, so + * you shouldn't care whether it's running) + */ + beforeEach: (ctx) => { + const { workspaces, queries } = createPatchedDependencies(6); + + const readyToUpdateStopped = workspaces[0] as MutableWorkspace; + readyToUpdateStopped.outdated = true; + readyToUpdateStopped.latest_build.status = "stopped"; + + const readyToUpdateRunning = workspaces[1] as MutableWorkspace; + readyToUpdateRunning.outdated = true; + readyToUpdateRunning.latest_build.status = "running"; + + const readyToUpdateTransitioning = workspaces[2] as MutableWorkspace; + readyToUpdateTransitioning.outdated = true; + readyToUpdateTransitioning.latest_build.status = "starting"; + + const dormant = workspaces[3] as MutableWorkspace; + dormant.outdated = true; + dormant.latest_build.status = "stopped"; + dormant.dormant_at = new Date().toISOString(); + + const noUpdatesNeededStopped = workspaces[4] as MutableWorkspace; + noUpdatesNeededStopped.outdated = false; + dormant.latest_build.status = "stopped"; + + const noUpdatesNeededTransitioning = workspaces[5] as MutableWorkspace; + noUpdatesNeededTransitioning.outdated = false; + noUpdatesNeededTransitioning.latest_build.status = "starting"; + + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; + ctx.parameters = { ...ctx.parameters, queries }; + }, +}; diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx new file mode 100644 index 0000000000000..7e1a0f1358296 --- /dev/null +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx @@ -0,0 +1,634 @@ +import { Label } from "@radix-ui/react-label"; +import { Slot } from "@radix-ui/react-slot"; +import { templateVersion } from "api/queries/templates"; +import type { Workspace } from "api/typesGenerated"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { Avatar } from "components/Avatar/Avatar"; +import { Badge } from "components/Badge/Badge"; +import { Button } from "components/Button/Button"; +import { Checkbox } from "components/Checkbox/Checkbox"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogTitle, +} from "components/Dialog/Dialog"; +import { Spinner } from "components/Spinner/Spinner"; +import { TriangleAlert } from "lucide-react"; +import { + type FC, + type ForwardedRef, + type PropsWithChildren, + type ReactNode, + useId, + useRef, + useState, +} from "react"; +import { useQueries } from "react-query"; +import { cn } from "utils/cn"; +import { ACTIVE_BUILD_STATUSES } from "./WorkspacesPage"; + +type WorkspacePartitionByUpdateType = Readonly<{ + dormant: readonly Workspace[]; + noUpdateNeeded: readonly Workspace[]; + readyToUpdate: readonly Workspace[]; +}>; + +function separateWorkspacesByUpdateType( + workspaces: readonly Workspace[], +): WorkspacePartitionByUpdateType { + const noUpdateNeeded: Workspace[] = []; + const dormant: Workspace[] = []; + const readyToUpdate: Workspace[] = []; + + for (const ws of workspaces) { + if (!ws.outdated) { + noUpdateNeeded.push(ws); + continue; + } + if (ws.dormant_at !== null) { + dormant.push(ws); + continue; + } + readyToUpdate.push(ws); + } + + return { dormant, noUpdateNeeded, readyToUpdate }; +} + +type ReviewPanelProps = Readonly<{ + workspaceName: string; + workspaceIconUrl: string; + running: boolean; + transitioning: boolean; + label?: ReactNode; + adornment?: ReactNode; + className?: string; +}>; + +const ReviewPanel: FC = ({ + workspaceName, + label, + running, + transitioning, + workspaceIconUrl, + className, +}) => { + // Preemptively adding border to this component to help decouple the styling + // from the rest of the components in this file, and make the core parts of + // this component easier to reason about + return ( +
+
+ +
+ + {workspaceName} + {running && ( + + Running + + )} + {transitioning && ( + + Getting latest status + + )} + + + {label} + +
+
+
+ ); +}; + +type TemplateNameChangeProps = Readonly<{ + oldTemplateVersionName: string; + newTemplateVersionName: string; +}>; + +const TemplateNameChange: FC = ({ + oldTemplateVersionName: oldTemplateName, + newTemplateVersionName: newTemplateName, +}) => { + return ( + <> + + {oldTemplateName} → {newTemplateName} + + + Workspace will go from version {oldTemplateName} to version{" "} + {newTemplateName} + + + ); +}; + +type RunningWorkspacesWarningProps = Readonly<{ + acceptedConsequences: boolean; + onAcceptedConsequencesChange: (newValue: boolean) => void; + checkboxRef: ForwardedRef; + containerRef: ForwardedRef; +}>; +const RunningWorkspacesWarning: FC = ({ + acceptedConsequences, + onAcceptedConsequencesChange, + checkboxRef, + containerRef, +}) => { + return ( +
+

+ + Running workspaces detected +

+ +
    +
  • + Updating a workspace will start it on its latest template version. + This can delete non-persistent data. +
    • +
  • + Anyone connected to a running workspace will be disconnected until the + update is complete. +
    • +
  • Any unsaved data will be lost.
    • +
+ + +
+ ); +}; + +type ContainerProps = Readonly< + PropsWithChildren<{ + asChild?: boolean; + }> +>; +const Container: FC = ({ children, asChild = false }) => { + const Wrapper = asChild ? Slot : "div"; + return ( + + {children} + + ); +}; + +type ContainerBodyProps = Readonly< + PropsWithChildren<{ + headerText: ReactNode; + description: ReactNode; + showDescription?: boolean; + }> +>; +const ContainerBody: FC = ({ + children, + headerText, + description, + showDescription = false, +}) => { + return ( + // Have to subtract parent padding via margin values and then add it + // back as child padding so that there's no risk of the scrollbar + // covering up content when the container gets tall enough to overflow +
+
+ +

+ {headerText} +

+ + + + {description} + +
+ + {children} +
+ ); +}; + +type ContainerFooterProps = Readonly< + PropsWithChildren<{ + className?: string; + }> +>; +const ContainerFooter: FC = ({ children, className }) => { + return ( +
+ {children} +
+ ); +}; + +type WorkspacesListSectionProps = Readonly< + PropsWithChildren<{ + headerText: ReactNode; + description: ReactNode; + }> +>; +const WorkspacesListSection: FC = ({ + children, + headerText, + description, +}) => { + return ( +
+
+

{headerText}

+

+ {description} +

+
+ +
    + {children} +
+
+ ); +}; + +// Used to force the user to acknowledge that batch updating has risks in +// certain situations and could destroy their data +type ConsequencesStage = "notAccepted" | "accepted" | "failedValidation"; + +type ReviewFormProps = Readonly<{ + workspacesToUpdate: readonly Workspace[]; + isProcessing: boolean; + onCancel: () => void; + onSubmit: () => void; +}>; + +const ReviewForm: FC = ({ + workspacesToUpdate, + isProcessing, + onCancel, + onSubmit, +}) => { + const hookId = useId(); + const [stage, setStage] = useState("notAccepted"); + const consequencesContainerRef = useRef(null); + const consequencesCheckboxRef = useRef(null); + + // Dormant workspaces can't be activated without activating them first. For + // now, we'll only show the user that some workspaces can't be updated, and + // then skip over them for all other update logic + const { dormant, noUpdateNeeded, readyToUpdate } = + separateWorkspacesByUpdateType(workspacesToUpdate); + + // The workspaces don't have all necessary data by themselves, so we need to + // fetch the unique template versions, and massage the results + const uniqueTemplateVersionIds = new Set( + readyToUpdate.map((ws) => ws.template_active_version_id), + ); + const templateVersionQueries = useQueries({ + queries: [...uniqueTemplateVersionIds].map((id) => templateVersion(id)), + }); + + // React Query persists previous errors even if a query is no longer in the + // error state, so we need to explicitly check the isError property to see + // if any of the queries actively have an error + const error = templateVersionQueries.find((q) => q.isError)?.error; + + const hasWorkspaces = workspacesToUpdate.length > 0; + const someWorkspacesCanBeUpdated = readyToUpdate.length > 0; + + const formIsNeeded = someWorkspacesCanBeUpdated || dormant.length > 0; + if (!formIsNeeded) { + return ( + + + None of the{" "} + + {workspacesToUpdate.length} + {" "} + selected workspaces need updates. + + ) : ( + "Nothing to update." + ) + } + > + {error !== undefined && } + + + + + + + ); + } + + const runningIds = new Set( + readyToUpdate + .filter((ws) => ws.latest_build.status === "running") + .map((ws) => ws.id), + ); + + /** + * Two things: + * 1. We have to make sure that we don't let the user submit anything while + * workspaces are transitioning, or else we'll run into a race condition. + * If a user starts a workspace, and then immediately batch-updates it, + * the workspace won't be in the running state yet. We need to issue + * warnings about how updating running workspaces is a destructive + * action, but if the the user goes through the form quickly enough, + * they'll be able to update without seeing the warning. + * 2. Just to be on the safe side, we also need to derive the transitioning + * IDs from all checked workspaces, because the separation result could + * theoretically change on re-render after any workspace state + * transitions end. + */ + const transitioningIds = new Set( + workspacesToUpdate + .filter((ws) => ACTIVE_BUILD_STATUSES.includes(ws.latest_build.status)) + .map((ws) => ws.id), + ); + + const hasRunningWorkspaces = runningIds.size > 0; + const consequencesResolved = !hasRunningWorkspaces || stage === "accepted"; + const failedValidationId = + stage === "failedValidation" ? `${hookId}-failed-validation` : undefined; + + // For UX/accessibility reasons, we're splitting a lot of hairs between + // various invalid/disabled states. We do not just want to throw a blanket + // `disabled` attribute on a button and call it a day. The most important + // thing is that we need to give the user feedback on how to get unstuck if + // they fail any input validations + const safeToSubmit = transitioningIds.size === 0 && error === undefined; + const buttonIsDisabled = !safeToSubmit || isProcessing; + const submitIsValid = + consequencesResolved && error === undefined && readyToUpdate.length > 0; + + return ( + +
{ + e.preventDefault(); + if (!someWorkspacesCanBeUpdated) { + onCancel(); + return; + } + if (submitIsValid) { + onSubmit(); + return; + } + if (stage === "accepted") { + return; + } + + setStage("failedValidation"); + // Makes sure that if the modal is long enough to scroll and + // if the warning section checkbox isn't on screen anymore, + // the warning section goes back to being on screen + consequencesContainerRef.current?.scrollIntoView({ + behavior: "smooth", + }); + consequencesCheckboxRef.current?.focus(); + }} + > + +
+ {error !== undefined && } + + {hasRunningWorkspaces && ( + { + if (newChecked) { + setStage("accepted"); + } else { + setStage("notAccepted"); + } + }} + /> + )} + + {readyToUpdate.length > 0 && ( + + {readyToUpdate.map((ws) => { + const matchedQuery = templateVersionQueries.find( + (q) => q.data?.id === ws.template_active_version_id, + ); + const newTemplateName = matchedQuery?.data?.name; + + return ( +
  • + + ) + } + /> +
    • + ); + })} +     + )} + + {noUpdateNeeded.length > 0 && ( + + {noUpdateNeeded.map((ws) => ( +
  • + +
    • + ))} +     + )} + + {dormant.length > 0 && ( + + Dormant workspaces cannot be updated without first + activating the workspace. They will always be skipped during + batch updates. + + } + > + {dormant.map((ws) => ( +
  • + +
    • + ))} +     + )} +
    +     + + +
    + + +
    + + {stage === "failedValidation" && ( +

    + Please acknowledge consequences to continue. +

    + )} +     +
    +     + ); +}; + +type BatchUpdateModalFormProps = Readonly<{ + open: boolean; + isProcessing: boolean; + workspacesToUpdate: readonly Workspace[]; + onCancel: () => void; + onSubmit: () => void; +}>; + +export const BatchUpdateModalForm: FC = ({ + open, + isProcessing, + workspacesToUpdate, + onCancel, + onSubmit, +}) => { + return ( + { + if (open) { + onCancel(); + } + }} + > + + {/* + * Because of how the Dialog component works, we need to make + * sure that at least the parent stays mounted at all times. But + * if we move all the state into ReviewForm, that means that its + * state only mounts when the user actually opens up the batch + * update form. That saves us from mounting a bunch of extra + * state and firing extra queries, when realistically, the form + * will stay closed 99% of the time while the user is on the + * workspaces page. + */} + + + + ); +}; diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 18caa6aba3285..809f6a83b40a8 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -17,7 +17,7 @@ import { useQuery, useQueryClient } from "react-query"; import { useSearchParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import { BatchDeleteConfirmation } from "./BatchDeleteConfirmation"; -import { BatchUpdateConfirmation } from "./BatchUpdateConfirmation"; +import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; import { WorkspacesPageView } from "./WorkspacesPageView"; import { useBatchActions } from "./batchActions"; import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; @@ -27,7 +27,7 @@ import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; * workspace is in the middle of a transition and will eventually reach a more * stable state/status. */ -const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ +export const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ "canceling", "deleting", "pending", @@ -234,12 +234,12 @@ const WorkspacesPage: FC = () => { }} /> - setActiveBatchAction(undefined)} - onConfirm={async () => { + workspacesToUpdate={checkedWorkspaces} + isProcessing={batchActions.isProcessing} + onCancel={() => setActiveBatchAction(undefined)} + onSubmit={async () => { await batchActions.updateTemplateVersions({ workspaces: checkedWorkspaces, isDynamicParametersEnabled: false, From 5504b5bcbf6bec84112bb7bfceebd969830d7f5f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 04:26:23 +0000 Subject: [PATCH 2/6] fix: remove outdated tests --- .../WorkspacesPage/WorkspacesPage.test.tsx | 161 ------------------ 1 file changed, 161 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 678f0331331a6..a4565e2b9f61d 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -85,167 +85,6 @@ describe("WorkspacesPage", () => { expect(deleteWorkspace).toHaveBeenCalledWith(workspaces[1].id); }); - describe("batch update", () => { - it("ignores up-to-date workspaces", async () => { - const workspaces = [ - { ...MockWorkspace, id: "1" }, // running, not outdated. no warning. - { ...MockDormantWorkspace, id: "2" }, // dormant, not outdated. no warning. - { ...MockOutdatedWorkspace, id: "3" }, - { ...MockOutdatedWorkspace, id: "4" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const updateWorkspace = jest.spyOn(API, "updateWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - for (const workspace of workspaces) { - await user.click(getWorkspaceCheckbox(workspace)); - } - - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // One click: no running workspaces warning, no dormant workspaces warning. - // There is a running workspace and a dormant workspace selected, but they - // are not outdated. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - // `workspaces[0]` was up-to-date, and running - // `workspaces[1]` was dormant - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(2); - }); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); - }); - - it("warns about and updates running workspaces", async () => { - const workspaces = [ - { ...MockRunningOutdatedWorkspace, id: "1" }, - { ...MockOutdatedWorkspace, id: "2" }, - { ...MockOutdatedWorkspace, id: "3" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const updateWorkspace = jest.spyOn(API, "updateWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - for (const workspace of workspaces) { - await user.click(getWorkspaceCheckbox(workspace)); - } - - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // Two clicks: 1 running workspace, no dormant workspaces warning. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/1 running workspace/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(3); - }); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); - }); - - it("warns about and ignores dormant workspaces", async () => { - const workspaces = [ - { ...MockDormantOutdatedWorkspace, id: "1" }, - { ...MockOutdatedWorkspace, id: "2" }, - { ...MockOutdatedWorkspace, id: "3" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const updateWorkspace = jest.spyOn(API, "updateWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - for (const workspace of workspaces) { - await user.click(getWorkspaceCheckbox(workspace)); - } - - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // Two clicks: no running workspaces warning, 1 dormant workspace. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/dormant/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - // `workspaces[0]` was dormant - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(2); - }); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); - }); - - it("warns about running workspaces and then dormant workspaces", async () => { - const workspaces = [ - { ...MockRunningOutdatedWorkspace, id: "1" }, - { ...MockDormantOutdatedWorkspace, id: "2" }, - { ...MockOutdatedWorkspace, id: "3" }, - { ...MockOutdatedWorkspace, id: "4" }, - { ...MockWorkspace, id: "5" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const updateWorkspace = jest.spyOn(API, "updateWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - for (const workspace of workspaces) { - await user.click(getWorkspaceCheckbox(workspace)); - } - - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // Three clicks: 1 running workspace, 1 dormant workspace. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/1 running workspace/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/dormant/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - // `workspaces[1]` was dormant, and `workspaces[4]` was up-to-date - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(3); - }); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); - }); - }); - it("stops only the running and selected workspaces", async () => { const workspaces = [ { ...MockWorkspace, id: "1" }, From 5ec7dac7358874dcc5c60c8e81657d11edb99c0d Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 04:31:03 +0000 Subject: [PATCH 3/6] wip: add tests back --- .../WorkspacesPage/WorkspacesPage.test.tsx | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index a4565e2b9f61d..d3cd33d4d48b4 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -112,6 +112,167 @@ describe("WorkspacesPage", () => { expect(stopWorkspace).toHaveBeenCalledWith(workspaces[1].id); }); + describe("batch update", () => { + it("ignores up-to-date workspaces", async () => { + const workspaces = [ + { ...MockWorkspace, id: "1" }, // running, not outdated. no warning. + { ...MockDormantWorkspace, id: "2" }, // dormant, not outdated. no warning. + { ...MockOutdatedWorkspace, id: "3" }, + { ...MockOutdatedWorkspace, id: "4" }, + ]; + jest + .spyOn(API, "getWorkspaces") + .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); + const user = userEvent.setup(); + renderWithAuth(); + await waitForLoaderToBeRemoved(); + + for (const workspace of workspaces) { + await user.click(getWorkspaceCheckbox(workspace)); + } + + await user.click(screen.getByRole("button", { name: /bulk actions/i })); + const updateButton = await screen.findByTestId("bulk-action-update"); + await user.click(updateButton); + + // One click: no running workspaces warning, no dormant workspaces warning. + // There is a running workspace and a dormant workspace selected, but they + // are not outdated. + const confirmButton = await screen.findByTestId("confirm-button"); + const dialog = await screen.findByRole("dialog"); + expect(dialog).toHaveTextContent(/used by/i); + await user.click(confirmButton); + + // `workspaces[0]` was up-to-date, and running + // `workspaces[1]` was dormant + await waitFor(() => { + expect(updateWorkspace).toHaveBeenCalledTimes(2); + }); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); + }); + + it("warns about and updates running workspaces", async () => { + const workspaces = [ + { ...MockRunningOutdatedWorkspace, id: "1" }, + { ...MockOutdatedWorkspace, id: "2" }, + { ...MockOutdatedWorkspace, id: "3" }, + ]; + jest + .spyOn(API, "getWorkspaces") + .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); + const user = userEvent.setup(); + renderWithAuth(); + await waitForLoaderToBeRemoved(); + + for (const workspace of workspaces) { + await user.click(getWorkspaceCheckbox(workspace)); + } + + await user.click(screen.getByRole("button", { name: /bulk actions/i })); + const updateButton = await screen.findByTestId("bulk-action-update"); + await user.click(updateButton); + + // Two clicks: 1 running workspace, no dormant workspaces warning. + const confirmButton = await screen.findByTestId("confirm-button"); + const dialog = await screen.findByRole("dialog"); + expect(dialog).toHaveTextContent(/1 running workspace/i); + await user.click(confirmButton); + expect(dialog).toHaveTextContent(/used by/i); + await user.click(confirmButton); + + await waitFor(() => { + expect(updateWorkspace).toHaveBeenCalledTimes(3); + }); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); + }); + + it("warns about and ignores dormant workspaces", async () => { + const workspaces = [ + { ...MockDormantOutdatedWorkspace, id: "1" }, + { ...MockOutdatedWorkspace, id: "2" }, + { ...MockOutdatedWorkspace, id: "3" }, + ]; + jest + .spyOn(API, "getWorkspaces") + .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); + const user = userEvent.setup(); + renderWithAuth(); + await waitForLoaderToBeRemoved(); + + for (const workspace of workspaces) { + await user.click(getWorkspaceCheckbox(workspace)); + } + + await user.click(screen.getByRole("button", { name: /bulk actions/i })); + const updateButton = await screen.findByTestId("bulk-action-update"); + await user.click(updateButton); + + // Two clicks: no running workspaces warning, 1 dormant workspace. + const confirmButton = await screen.findByTestId("confirm-button"); + const dialog = await screen.findByRole("dialog"); + expect(dialog).toHaveTextContent(/dormant/i); + await user.click(confirmButton); + expect(dialog).toHaveTextContent(/used by/i); + await user.click(confirmButton); + + // `workspaces[0]` was dormant + await waitFor(() => { + expect(updateWorkspace).toHaveBeenCalledTimes(2); + }); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); + }); + + it("warns about running workspaces and then dormant workspaces", async () => { + const workspaces = [ + { ...MockRunningOutdatedWorkspace, id: "1" }, + { ...MockDormantOutdatedWorkspace, id: "2" }, + { ...MockOutdatedWorkspace, id: "3" }, + { ...MockOutdatedWorkspace, id: "4" }, + { ...MockWorkspace, id: "5" }, + ]; + jest + .spyOn(API, "getWorkspaces") + .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); + const user = userEvent.setup(); + renderWithAuth(); + await waitForLoaderToBeRemoved(); + + for (const workspace of workspaces) { + await user.click(getWorkspaceCheckbox(workspace)); + } + + await user.click(screen.getByRole("button", { name: /bulk actions/i })); + const updateButton = await screen.findByTestId("bulk-action-update"); + await user.click(updateButton); + + // Three clicks: 1 running workspace, 1 dormant workspace. + const confirmButton = await screen.findByTestId("confirm-button"); + const dialog = await screen.findByRole("dialog"); + expect(dialog).toHaveTextContent(/1 running workspace/i); + await user.click(confirmButton); + expect(dialog).toHaveTextContent(/dormant/i); + await user.click(confirmButton); + expect(dialog).toHaveTextContent(/used by/i); + await user.click(confirmButton); + + // `workspaces[1]` was dormant, and `workspaces[4]` was up-to-date + await waitFor(() => { + expect(updateWorkspace).toHaveBeenCalledTimes(3); + }); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); + expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); + }); + }); + it("starts only the stopped and selected workspaces", async () => { const workspaces = [ { ...MockStoppedWorkspace, id: "1" }, From 527104ba17e1ba5e8a851e7fa7a7c2a02bca8d73 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 05:14:21 +0000 Subject: [PATCH 4/6] chore: get first integration test working --- .../WorkspacesPage/WorkspacesPage.test.tsx | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index d3cd33d4d48b4..9d28b535d30e8 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -112,17 +112,28 @@ describe("WorkspacesPage", () => { expect(stopWorkspace).toHaveBeenCalledWith(workspaces[1].id); }); - describe("batch update", () => { - it("ignores up-to-date workspaces", async () => { - const workspaces = [ - { ...MockWorkspace, id: "1" }, // running, not outdated. no warning. - { ...MockDormantWorkspace, id: "2" }, // dormant, not outdated. no warning. + describe("batch updates", () => { + it("skips up-to-date workspaces after confirming update", async () => { + const workspaces: readonly Workspace[] = [ + // Not outdated but running; should have no warning + { ...MockWorkspace, id: "1" }, + // Dormant; no warning + { ...MockDormantWorkspace, id: "2" }, + // Out of date but not running; no warning { ...MockOutdatedWorkspace, id: "3" }, - { ...MockOutdatedWorkspace, id: "4" }, + // Out of date but running; should issue warning + { + ...MockOutdatedWorkspace, id: "4", + latest_build: { + ...MockOutdatedWorkspace.latest_build, + status: "running" + }, + }, ]; jest .spyOn(API, "getWorkspaces") .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); const user = userEvent.setup(); renderWithAuth(); @@ -133,22 +144,22 @@ describe("WorkspacesPage", () => { } await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); + const dropdownItem = await screen.findByRole("menuitem", { + name: /Update/, + }); + await user.click(dropdownItem); - // One click: no running workspaces warning, no dormant workspaces warning. - // There is a running workspace and a dormant workspace selected, but they - // are not outdated. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); + const modal = await screen.findByRole("dialog", { name: /Review Updates/i }); + const confirmCheckbox = within(modal).getByRole("checkbox", { + name: /I acknowledge these consequences\./, + }); + await user.click(confirmCheckbox); + const updateModalButton = within(modal).getByRole("button", {name: /Update/}); + await user.click(updateModalButton); // `workspaces[0]` was up-to-date, and running // `workspaces[1]` was dormant - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(2); - }); + await waitFor(() => expect(updateWorkspace).toHaveBeenCalledTimes(2)); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); }); From 15556276bd76f5f705c8c0185114ec0b27523d0b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 05:21:07 +0000 Subject: [PATCH 5/6] fix: update all integration tests --- .../WorkspacesPage/WorkspacesPage.test.tsx | 115 +++++++----------- 1 file changed, 41 insertions(+), 74 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 9d28b535d30e8..694e3524a36f1 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -123,10 +123,11 @@ describe("WorkspacesPage", () => { { ...MockOutdatedWorkspace, id: "3" }, // Out of date but running; should issue warning { - ...MockOutdatedWorkspace, id: "4", + ...MockOutdatedWorkspace, + id: "4", latest_build: { ...MockOutdatedWorkspace.latest_build, - status: "running" + status: "running", }, }, ]; @@ -149,12 +150,16 @@ describe("WorkspacesPage", () => { }); await user.click(dropdownItem); - const modal = await screen.findByRole("dialog", { name: /Review Updates/i }); + const modal = await screen.findByRole("dialog", { + name: /Review Updates/i, + }); const confirmCheckbox = within(modal).getByRole("checkbox", { name: /I acknowledge these consequences\./, }); await user.click(confirmCheckbox); - const updateModalButton = within(modal).getByRole("button", {name: /Update/}); + const updateModalButton = within(modal).getByRole("button", { + name: /Update/, + }); await user.click(updateModalButton); // `workspaces[0]` was up-to-date, and running @@ -164,8 +169,8 @@ describe("WorkspacesPage", () => { expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); }); - it("warns about and updates running workspaces", async () => { - const workspaces = [ + it("lets user update a running workspace (after user goes through warning)", async () => { + const workspaces: readonly Workspace[] = [ { ...MockRunningOutdatedWorkspace, id: "1" }, { ...MockOutdatedWorkspace, id: "2" }, { ...MockOutdatedWorkspace, id: "3" }, @@ -173,6 +178,7 @@ describe("WorkspacesPage", () => { jest .spyOn(API, "getWorkspaces") .mockResolvedValue({ workspaces, count: workspaces.length }); + const updateWorkspace = jest.spyOn(API, "updateWorkspace"); const user = userEvent.setup(); renderWithAuth(); @@ -183,20 +189,24 @@ describe("WorkspacesPage", () => { } await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // Two clicks: 1 running workspace, no dormant workspaces warning. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/1 running workspace/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(3); + const dropdownItem = await screen.findByRole("menuitem", { + name: /Update/, + }); + await user.click(dropdownItem); + + const modal = await screen.findByRole("dialog", { + name: /Review Updates/i, + }); + const confirmCheckbox = within(modal).getByRole("checkbox", { + name: /I acknowledge these consequences\./, + }); + await user.click(confirmCheckbox); + const updateModalButton = within(modal).getByRole("button", { + name: /Update/, }); + await user.click(updateModalButton); + + await waitFor(() => expect(updateWorkspace).toHaveBeenCalledTimes(3)); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); @@ -221,67 +231,24 @@ describe("WorkspacesPage", () => { } await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); + const dropdownItem = await screen.findByRole("menuitem", { + name: /Update/, + }); + await user.click(dropdownItem); - // Two clicks: no running workspaces warning, 1 dormant workspace. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/dormant/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); + const modal = await screen.findByRole("dialog", { + name: /Review Updates/i, + }); + const updateModalButton = within(modal).getByRole("button", { + name: /Update/, + }); + await user.click(updateModalButton); // `workspaces[0]` was dormant - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(2); - }); + await waitFor(() => expect(updateWorkspace).toHaveBeenCalledTimes(2)); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[1], [], false); expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); }); - - it("warns about running workspaces and then dormant workspaces", async () => { - const workspaces = [ - { ...MockRunningOutdatedWorkspace, id: "1" }, - { ...MockDormantOutdatedWorkspace, id: "2" }, - { ...MockOutdatedWorkspace, id: "3" }, - { ...MockOutdatedWorkspace, id: "4" }, - { ...MockWorkspace, id: "5" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const updateWorkspace = jest.spyOn(API, "updateWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - for (const workspace of workspaces) { - await user.click(getWorkspaceCheckbox(workspace)); - } - - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const updateButton = await screen.findByTestId("bulk-action-update"); - await user.click(updateButton); - - // Three clicks: 1 running workspace, 1 dormant workspace. - const confirmButton = await screen.findByTestId("confirm-button"); - const dialog = await screen.findByRole("dialog"); - expect(dialog).toHaveTextContent(/1 running workspace/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/dormant/i); - await user.click(confirmButton); - expect(dialog).toHaveTextContent(/used by/i); - await user.click(confirmButton); - - // `workspaces[1]` was dormant, and `workspaces[4]` was up-to-date - await waitFor(() => { - expect(updateWorkspace).toHaveBeenCalledTimes(3); - }); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[0], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[2], [], false); - expect(updateWorkspace).toHaveBeenCalledWith(workspaces[3], [], false); - }); }); it("starts only the stopped and selected workspaces", async () => { From 007efc1556624b38ca1f7b8920ca2a5913008eaf Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 05:23:01 +0000 Subject: [PATCH 6/6] fix: move test to shrink diff --- .../WorkspacesPage/WorkspacesPage.test.tsx | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 694e3524a36f1..3d03f98c26773 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -85,33 +85,6 @@ describe("WorkspacesPage", () => { expect(deleteWorkspace).toHaveBeenCalledWith(workspaces[1].id); }); - it("stops only the running and selected workspaces", async () => { - const workspaces = [ - { ...MockWorkspace, id: "1" }, - { ...MockWorkspace, id: "2" }, - { ...MockWorkspace, id: "3" }, - ]; - jest - .spyOn(API, "getWorkspaces") - .mockResolvedValue({ workspaces, count: workspaces.length }); - const stopWorkspace = jest.spyOn(API, "stopWorkspace"); - const user = userEvent.setup(); - renderWithAuth(); - await waitForLoaderToBeRemoved(); - - await user.click(getWorkspaceCheckbox(workspaces[0])); - await user.click(getWorkspaceCheckbox(workspaces[1])); - await user.click(screen.getByRole("button", { name: /bulk actions/i })); - const stopButton = await screen.findByRole("menuitem", { name: /stop/i }); - await user.click(stopButton); - - await waitFor(() => { - expect(stopWorkspace).toHaveBeenCalledTimes(2); - }); - expect(stopWorkspace).toHaveBeenCalledWith(workspaces[0].id); - expect(stopWorkspace).toHaveBeenCalledWith(workspaces[1].id); - }); - describe("batch updates", () => { it("skips up-to-date workspaces after confirming update", async () => { const workspaces: readonly Workspace[] = [ @@ -251,6 +224,33 @@ describe("WorkspacesPage", () => { }); }); + it("stops only the running and selected workspaces", async () => { + const workspaces = [ + { ...MockWorkspace, id: "1" }, + { ...MockWorkspace, id: "2" }, + { ...MockWorkspace, id: "3" }, + ]; + jest + .spyOn(API, "getWorkspaces") + .mockResolvedValue({ workspaces, count: workspaces.length }); + const stopWorkspace = jest.spyOn(API, "stopWorkspace"); + const user = userEvent.setup(); + renderWithAuth(); + await waitForLoaderToBeRemoved(); + + await user.click(getWorkspaceCheckbox(workspaces[0])); + await user.click(getWorkspaceCheckbox(workspaces[1])); + await user.click(screen.getByRole("button", { name: /bulk actions/i })); + const stopButton = await screen.findByRole("menuitem", { name: /stop/i }); + await user.click(stopButton); + + await waitFor(() => { + expect(stopWorkspace).toHaveBeenCalledTimes(2); + }); + expect(stopWorkspace).toHaveBeenCalledWith(workspaces[0].id); + expect(stopWorkspace).toHaveBeenCalledWith(workspaces[1].id); + }); + it("starts only the stopped and selected workspaces", async () => { const workspaces = [ { ...MockStoppedWorkspace, id: "1" },