From bb05088f4beb299313f635a992566bbd289bf389 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 00:48:56 +0000 Subject: [PATCH 01/26] refactor: centralize storybook queries key --- site/.storybook/preview.jsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx index 8d8a37ecd2fbf..d64e85fa03de3 100644 --- a/site/.storybook/preview.jsx +++ b/site/.storybook/preview.jsx @@ -101,6 +101,13 @@ function withHelmet(Story) { ); } +/** + * This JSX file isn't part of the main project, so it doesn't get to use the + * ambient types defined in `storybook.d.ts` to provide extra type-safety. + * Extracting main key to avoid typos. + */ +const queryParametersKey = "queries"; + /** @type {Decorator} */ function withQuery(Story, { parameters }) { const queryClient = new QueryClient({ @@ -112,8 +119,8 @@ function withQuery(Story, { parameters }) { }, }); - if (parameters.queries) { - for (const query of parameters.queries) { + if (parameters[queryParametersKey]) { + for (const query of parameters[queryParametersKey]) { queryClient.setQueryData(query.key, query.data); } } From a119d70db091e8c76b05f20277afb5a6c9e32ad0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 00:53:15 +0000 Subject: [PATCH 02/26] refactor: centralize template version key prefix --- site/src/api/queries/templates.ts | 35 ++++++++++--------------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 5135f2304426e..a9c0b64f94c56 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -48,21 +48,6 @@ export const templates = ( }; }; -const getTemplatesByOrganizationQueryKey = ( - organization: string, - options?: GetTemplatesOptions, -) => [organization, "templates", options?.deprecated]; - -const templatesByOrganization = ( - organization: string, - options: GetTemplatesOptions = {}, -) => { - return { - queryKey: getTemplatesByOrganizationQueryKey(organization, options), - queryFn: () => API.getTemplatesByOrganization(organization, options), - }; -}; - export const templateACL = (templateId: string) => { return { queryKey: ["templateAcl", templateId], @@ -121,9 +106,11 @@ export const templateExamples = () => { }; }; +export const templateVersionRoot: string = "templateVersion" + export const templateVersion = (versionId: string) => { return { - queryKey: ["templateVersion", versionId], + queryKey: [templateVersionRoot, versionId], queryFn: () => API.getTemplateVersion(versionId), }; }; @@ -134,7 +121,7 @@ export const templateVersionByName = ( versionName: string, ) => { return { - queryKey: ["templateVersion", organizationId, templateName, versionName], + queryKey: [templateVersionRoot, organizationId, templateName, versionName], queryFn: () => API.getTemplateVersionByName(organizationId, templateName, versionName), }; @@ -153,7 +140,7 @@ export const templateVersions = (templateId: string) => { }; export const templateVersionVariablesKey = (versionId: string) => [ - "templateVersion", + templateVersionRoot, versionId, "variables", ]; @@ -216,7 +203,7 @@ export const templaceACLAvailable = ( }; const templateVersionExternalAuthKey = (versionId: string) => [ - "templateVersion", + templateVersionRoot, versionId, "externalAuth", ]; @@ -257,21 +244,21 @@ const createTemplateFn = async (options: CreateTemplateOptions) => { export const templateVersionLogs = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "logs"], + queryKey: [templateVersionRoot, versionId, "logs"], queryFn: () => API.getTemplateVersionLogs(versionId), }; }; export const richParameters = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "richParameters"], + queryKey: [templateVersionRoot, versionId, "richParameters"], queryFn: () => API.getTemplateVersionRichParameters(versionId), }; }; export const resources = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "resources"], + queryKey: [templateVersionRoot, versionId, "resources"], queryFn: () => API.getTemplateVersionResources(versionId), }; }; @@ -293,7 +280,7 @@ export const previousTemplateVersion = ( ) => { return { queryKey: [ - "templateVersion", + templateVersionRoot, organizationId, templateName, versionName, @@ -313,7 +300,7 @@ export const previousTemplateVersion = ( export const templateVersionPresets = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "presets"], + queryKey: [templateVersionRoot, versionId, "presets"], queryFn: () => API.getTemplateVersionPresets(versionId), }; }; From 5dc67cf4635a5762e45a6ef05b3bbce82091f0be Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 01:11:13 +0000 Subject: [PATCH 03/26] refactor: remove JSX extension from batch actions --- .../pages/WorkspacesPage/{batchActions.tsx => batchActions.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename site/src/pages/WorkspacesPage/{batchActions.tsx => batchActions.ts} (100%) diff --git a/site/src/pages/WorkspacesPage/batchActions.tsx b/site/src/pages/WorkspacesPage/batchActions.ts similarity index 100% rename from site/src/pages/WorkspacesPage/batchActions.tsx rename to site/src/pages/WorkspacesPage/batchActions.ts From 4c77bffc5810d18d3610a029e3f29a095c62b835 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 01:21:05 +0000 Subject: [PATCH 04/26] fix: update API design of various utility hooks --- site/src/api/queries/templates.ts | 2 +- site/src/components/Filter/Filter.tsx | 40 +++-- site/src/hooks/usePagination.ts | 50 ++++-- site/src/pages/AuditPage/AuditPage.tsx | 3 +- .../CreateWorkspacePageViewExperimental.tsx | 8 +- .../src/pages/TemplatesPage/TemplatesPage.tsx | 5 +- site/src/pages/UsersPage/UsersPage.tsx | 8 +- .../pages/WorkspacesPage/WorkspacesPage.tsx | 157 ++++++++++++------ .../WorkspacesPage/WorkspacesPageView.tsx | 28 ++-- site/src/pages/WorkspacesPage/batchActions.ts | 56 +++++-- 10 files changed, 226 insertions(+), 131 deletions(-) diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index a9c0b64f94c56..9057179a8740c 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -106,7 +106,7 @@ export const templateExamples = () => { }; }; -export const templateVersionRoot: string = "templateVersion" +export const templateVersionRoot: string = "templateVersion"; export const templateVersion = (versionId: string) => { return { diff --git a/site/src/components/Filter/Filter.tsx b/site/src/components/Filter/Filter.tsx index 736592116730d..1cdff25547c30 100644 --- a/site/src/components/Filter/Filter.tsx +++ b/site/src/components/Filter/Filter.tsx @@ -16,7 +16,6 @@ import { useDebouncedFunction } from "hooks/debounce"; import { ExternalLinkIcon } from "lucide-react"; import { ChevronDownIcon } from "lucide-react"; import { type FC, type ReactNode, useEffect, useRef, useState } from "react"; -import type { useSearchParams } from "react-router-dom"; type PresetFilter = { name: string; @@ -27,35 +26,46 @@ type FilterValues = Record; type UseFilterConfig = { /** - * The fallback value to use in the event that no filter params can be parsed - * from the search params object. This value is allowed to change on - * re-renders. + * The fallback value to use in the event that no filter params can be + * parsed from the search params object. */ fallbackFilter?: string; - searchParamsResult: ReturnType; + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; onUpdate?: (newValue: string) => void; }; +export type UseFilterResult = Readonly<{ + query: string; + values: FilterValues; + used: boolean; + update: (newValues: string | FilterValues) => void; + debounceUpdate: (newValues: string | FilterValues) => void; + cancelDebounce: () => void; +}>; + export const useFilterParamsKey = "filter"; export const useFilter = ({ fallbackFilter = "", - searchParamsResult, + searchParams, + onSearchParamsChange, onUpdate, -}: UseFilterConfig) => { - const [searchParams, setSearchParams] = searchParamsResult; +}: UseFilterConfig): UseFilterResult => { const query = searchParams.get(useFilterParamsKey) ?? fallbackFilter; const update = (newValues: string | FilterValues) => { const serialized = typeof newValues === "string" ? newValues : stringifyFilter(newValues); - - searchParams.set(useFilterParamsKey, serialized); - setSearchParams(searchParams); - - if (onUpdate !== undefined) { - onUpdate(serialized); + const noUpdateNeeded = searchParams.get(useFilterParamsKey) === serialized; + if (noUpdateNeeded) { + return; } + + const copy = new URLSearchParams(searchParams); + copy.set(useFilterParamsKey, serialized); + onSearchParamsChange(copy); + onUpdate?.(serialized); }; const { debounced: debounceUpdate, cancelDebounce } = useDebouncedFunction( @@ -73,8 +83,6 @@ export const useFilter = ({ }; }; -export type UseFilterResult = ReturnType; - const parseFilterQuery = (filterQuery: string): FilterValues => { if (filterQuery === "") { return {}; diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 72ea70868fb30..0bc81d7325626 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -1,25 +1,41 @@ import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; -import type { useSearchParams } from "react-router-dom"; -export const usePagination = ({ - searchParamsResult, -}: { - searchParamsResult: ReturnType; -}) => { - const [searchParams, setSearchParams] = searchParamsResult; - const page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; - const limit = DEFAULT_RECORDS_PER_PAGE; - const offset = page <= 0 ? 0 : (page - 1) * limit; +const paginationKey = "page"; - const goToPage = (page: number) => { - searchParams.set("page", page.toString()); - setSearchParams(searchParams); - }; +type UsePaginationOptions = Readonly<{ + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; +}>; + +type UsePaginationResult = Readonly<{ + page: number; + limit: number; + offset: number; + goToPage: (page: number) => void; +}>; + +export function usePagination( + options: UsePaginationOptions, +): UsePaginationResult { + const { searchParams, onSearchParamsChange } = options; + const limit = DEFAULT_RECORDS_PER_PAGE; + const rawPage = Number.parseInt(searchParams.get(paginationKey) || "0", 10); + const page = Number.isNaN(rawPage) ? 1 : rawPage; return { page, limit, - goToPage, - offset, + offset: Math.max(0, (page - 1) * limit), + goToPage: (newPage) => { + const abortNavigation = + page === newPage || !Number.isFinite(page) || !Number.isInteger(page); + if (abortNavigation) { + return; + } + + const copy = new URLSearchParams(searchParams); + copy.set("page", page.toString()); + onSearchParamsChange(copy); + }, }; -}; +} diff --git a/site/src/pages/AuditPage/AuditPage.tsx b/site/src/pages/AuditPage/AuditPage.tsx index f63adbcd4136b..669fbbdb14a27 100644 --- a/site/src/pages/AuditPage/AuditPage.tsx +++ b/site/src/pages/AuditPage/AuditPage.tsx @@ -33,7 +33,8 @@ const AuditPage: FC = () => { const [searchParams, setSearchParams] = useSearchParams(); const auditsQuery = usePaginatedQuery(paginatedAudits(searchParams)); const filter = useFilter({ - searchParamsResult: [searchParams, setSearchParams], + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: auditsQuery.goToFirstPage, }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 8cb6c4acb6e49..5b4a966ae6965 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -108,9 +108,7 @@ export const CreateWorkspacePageViewExperimental: FC< owner, setOwner, }) => { - const [suggestedName, setSuggestedName] = useState(() => - generateWorkspaceName(), - ); + const [suggestedName, setSuggestedName] = useState(generateWorkspaceName); const [showPresetParameters, setShowPresetParameters] = useState(false); const id = useId(); const workspaceNameInputRef = useRef(null); @@ -124,14 +122,14 @@ export const CreateWorkspacePageViewExperimental: FC< // Only touched fields are sent to the websocket // Autofilled parameters are marked as touched since they have been modified - const initialTouched = parameters.reduce( + const initialTouched = parameters.reduce>( (touched, parameter) => { if (autofillByName[parameter.name] !== undefined) { touched[parameter.name] = true; } return touched; }, - {} as Record, + {}, ); // The form parameters values hold the working state of the parameters that will be submitted when creating a workspace diff --git a/site/src/pages/TemplatesPage/TemplatesPage.tsx b/site/src/pages/TemplatesPage/TemplatesPage.tsx index bef25e17bb755..832c5b43c1c9c 100644 --- a/site/src/pages/TemplatesPage/TemplatesPage.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPage.tsx @@ -14,10 +14,11 @@ const TemplatesPage: FC = () => { const { permissions, user: me } = useAuthenticated(); const { showOrganizations } = useDashboard(); - const searchParamsResult = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const filter = useFilter({ fallbackFilter: "deprecated:false", - searchParamsResult, + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: () => {}, // reset pagination }); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 581a9166bce3d..bb176e9150c4b 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -39,9 +39,8 @@ type UserPageProps = { const UsersPage: FC = ({ defaultNewPassword }) => { const queryClient = useQueryClient(); const navigate = useNavigate(); - const searchParamsResult = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const { entitlements } = useDashboard(); - const [searchParams] = searchParamsResult; const groupsByUserIdQuery = useQuery(groupsByUserId()); const authMethodsQuery = useQuery(authMethods()); @@ -58,9 +57,10 @@ const UsersPage: FC = ({ defaultNewPassword }) => { enabled: viewDeploymentConfig, }); - const usersQuery = usePaginatedQuery(paginatedUsers(searchParamsResult[0])); + const usersQuery = usePaginatedQuery(paginatedUsers(searchParams)); const useFilterResult = useFilter({ - searchParamsResult, + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: usersQuery.goToFirstPage, }); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index fa96191501379..070732df9980f 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,8 +1,8 @@ import { getErrorDetail, getErrorMessage } from "api/errors"; import { workspacePermissionsByOrganization } from "api/queries/organizations"; -import { templates } from "api/queries/templates"; +import { templateVersionRoot, templates } from "api/queries/templates"; import { workspaces } from "api/queries/workspaces"; -import type { Workspace, WorkspaceStatus } from "api/typesGenerated"; +import type { WorkspaceStatus } from "api/typesGenerated"; import { useFilter } from "components/Filter/Filter"; import { useUserFilterMenu } from "components/Filter/UserFilter"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -11,7 +11,7 @@ import { useEffectEvent } from "hooks/hookPolyfills"; import { usePagination } from "hooks/usePagination"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useOrganizationsFilterMenu } from "modules/tableFiltering/options"; -import { type FC, useEffect, useMemo, useState } from "react"; +import { type FC, useMemo, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery, useQueryClient } from "react-query"; import { useSearchParams } from "react-router-dom"; @@ -22,15 +22,21 @@ import { WorkspacesPageView } from "./WorkspacesPageView"; import { useBatchActions } from "./batchActions"; import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; -// To reduce the number of fetches, we reduce the fetch interval if there are no -// active workspace builds. -const ACTIVE_BUILD_STATUSES: WorkspaceStatus[] = [ +/** + * The set of all workspace statuses that indicate that the state for a + * workspace is in the middle of a transition and will eventually reach a more + * stable state/status. + */ +export const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ "canceling", "deleting", "pending", "starting", "stopping", ]; + +// To reduce the number of fetches, we reduce the fetch interval if there are no +// active workspace builds. const ACTIVE_BUILDS_REFRESH_INTERVAL = 5_000; const NO_ACTIVE_BUILDS_REFRESH_INTERVAL = 30_000; @@ -48,13 +54,35 @@ function useSafeSearchParams() { >; } +type BatchAction = "delete" | "update"; + const WorkspacesPage: FC = () => { const queryClient = useQueryClient(); - // If we use a useSearchParams for each hook, the values will not be in sync. - // So we have to use a single one, centralizing the values, and pass it to - // each hook. - const searchParamsResult = useSafeSearchParams(); - const pagination = usePagination({ searchParamsResult }); + // We have to be careful with how we use useSearchParams or any other + // derived hooks. The URL is global state, but each call to useSearchParams + // creates a different, contradictory source of truth for what the URL + // should look like. We need to make sure that we only mount the hook once + // per page + const [searchParams, setSearchParams] = useSafeSearchParams(); + // Always need to make sure that we reset the checked workspaces each time + // the filtering or pagination changes, as that will almost always change + // which workspaces are shown on screen and which can be interacted with + const [checkedWorkspaceIds, setCheckedWorkspaceIds] = useState( + new Set(), + ); + const resetChecked = () => { + if (checkedWorkspaceIds.size !== 0) { + setCheckedWorkspaceIds(new Set()); + } + }; + + const pagination = usePagination({ + searchParams, + onSearchParamsChange: (newParams) => { + setSearchParams(newParams); + resetChecked(); + }, + }); const { permissions, user: me } = useAuthenticated(); const { entitlements } = useDashboard(); const templatesQuery = useQuery(templates()); @@ -78,14 +106,18 @@ const WorkspacesPage: FC = () => { }); }, [templatesQuery.data, workspacePermissionsQuery.data]); - const filterProps = useWorkspacesFilter({ - searchParamsResult, - onFilterChange: () => pagination.goToPage(1), + const filterState = useWorkspacesFilter({ + searchParams, + onSearchParamsChange: setSearchParams, + onFilterChange: () => { + pagination.goToPage(1); + resetChecked(); + }, }); const workspacesQueryOptions = workspaces({ ...pagination, - q: filterProps.filter.query, + q: filterState.filter.query, }); const { data, error, refetch } = useQuery({ ...workspacesQueryOptions, @@ -109,28 +141,18 @@ const WorkspacesPage: FC = () => { refetchOnWindowFocus: "always", }); - const [checkedWorkspaces, setCheckedWorkspaces] = useState< - readonly Workspace[] - >([]); - const [confirmingBatchAction, setConfirmingBatchAction] = useState< - "delete" | "update" | null - >(null); - const [urlSearchParams] = searchParamsResult; + const [activeBatchAction, setActiveBatchAction] = useState(); const canCheckWorkspaces = entitlements.features.workspace_batch_actions.enabled; const batchActions = useBatchActions({ onSuccess: async () => { await refetch(); - setCheckedWorkspaces([]); + resetChecked(); }, }); - // We want to uncheck the selected workspaces always when the url changes - // because of filtering or pagination - // biome-ignore lint/correctness/useExhaustiveDependencies: consider refactoring - useEffect(() => { - setCheckedWorkspaces([]); - }, [urlSearchParams]); + const checkedWorkspaces = + data?.workspaces.filter((w) => checkedWorkspaceIds.has(w.id)) ?? []; return ( <> @@ -142,7 +164,18 @@ const WorkspacesPage: FC = () => { canCreateTemplate={permissions.createTemplates} canChangeVersions={permissions.updateTemplates} checkedWorkspaces={checkedWorkspaces} - onCheckChange={setCheckedWorkspaces} + onCheckChange={(newWorkspaces) => { + setCheckedWorkspaceIds((current) => { + const newIds = newWorkspaces.map((ws) => ws.id); + const sameContent = + current.size === newIds.length && + newIds.every((id) => current.has(id)); + if (sameContent) { + return current; + } + return new Set(newIds); + }); + }} canCheckWorkspaces={canCheckWorkspaces} templates={filteredTemplates} templatesFetchStatus={templatesQuery.status} @@ -152,12 +185,31 @@ const WorkspacesPage: FC = () => { page={pagination.page} limit={pagination.limit} onPageChange={pagination.goToPage} - filterProps={filterProps} - isRunningBatchAction={batchActions.isLoading} - onDeleteAll={() => setConfirmingBatchAction("delete")} - onUpdateAll={() => setConfirmingBatchAction("update")} - onStartAll={() => batchActions.startAll(checkedWorkspaces)} - onStopAll={() => batchActions.stopAll(checkedWorkspaces)} + filterProps={filterState} + isRunningBatchAction={batchActions.isProcessing} + onBatchDeleteTransition={() => setActiveBatchAction("delete")} + onBatchStartTransition={() => batchActions.start(checkedWorkspaces)} + onBatchStopTransition={() => batchActions.stop(checkedWorkspaces)} + onBatchUpdateTransition={() => { + // Just because batch-updating can be really dangerous + // action for running workspaces, we're going to invalidate + // all relevant queries as a prefetch strategy before the + // modal content is even allowed to mount. + for (const ws of checkedWorkspaces) { + // Our data layer is a little messy right now, so + // there's no great way to invalidate a bunch of + // template version queries with a single function call, + // while also avoiding all other tangentially connected + // resources that use the same key pattern. Have to be + // super granular and make one call per workspace. + queryClient.invalidateQueries({ + queryKey: [templateVersionRoot, ws.template_active_version_id], + exact: true, + type: "all", + }); + } + setActiveBatchAction("update"); + }} onActionSuccess={async () => { await queryClient.invalidateQueries({ queryKey: workspacesQueryOptions.queryKey, @@ -172,31 +224,27 @@ const WorkspacesPage: FC = () => { /> setActiveBatchAction(undefined)} onConfirm={async () => { - await batchActions.deleteAll(checkedWorkspaces); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + await batchActions.delete(checkedWorkspaces); + setActiveBatchAction(undefined); }} /> setActiveBatchAction(undefined)} onConfirm={async () => { - await batchActions.updateAll({ + await batchActions.updateTemplateVersions({ workspaces: checkedWorkspaces, isDynamicParametersEnabled: false, }); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + setActiveBatchAction(undefined); }} /> @@ -206,17 +254,20 @@ const WorkspacesPage: FC = () => { export default WorkspacesPage; type UseWorkspacesFilterOptions = { - searchParamsResult: ReturnType; + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; onFilterChange: () => void; }; const useWorkspacesFilter = ({ - searchParamsResult, + searchParams, + onSearchParamsChange, onFilterChange, }: UseWorkspacesFilterOptions) => { const filter = useFilter({ fallbackFilter: "owner:me", - searchParamsResult, + searchParams, + onSearchParamsChange, onUpdate: onFilterChange, }); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 6563533bc43da..1de4534b25f22 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -51,10 +51,10 @@ interface WorkspacesPageViewProps { onPageChange: (page: number) => void; onCheckChange: (checkedWorkspaces: readonly Workspace[]) => void; isRunningBatchAction: boolean; - onDeleteAll: () => void; - onUpdateAll: () => void; - onStartAll: () => void; - onStopAll: () => void; + onBatchDeleteTransition: () => void; + onBatchUpdateTransition: () => void; + onBatchStartTransition: () => void; + onBatchStopTransition: () => void; canCheckWorkspaces: boolean; templatesFetchStatus: TemplateQuery["status"]; templates: TemplateQuery["data"]; @@ -74,10 +74,10 @@ export const WorkspacesPageView: FC = ({ page, checkedWorkspaces, onCheckChange, - onDeleteAll, - onUpdateAll, - onStopAll, - onStartAll, + onBatchDeleteTransition: onDeleteAll, + onBatchUpdateTransition: onUpdateAll, + onBatchStopTransition: onStopAll, + onBatchStartTransition: onStartAll, isRunningBatchAction, canCheckWorkspaces, templates, @@ -87,10 +87,10 @@ export const WorkspacesPageView: FC = ({ onActionSuccess, onActionError, }) => { - // Let's say the user has 5 workspaces, but tried to hit page 100, which does - // not exist. In this case, the page is not valid and we want to show a better - // error message. - const invalidPageNumber = page !== 1 && workspaces?.length === 0; + // Let's say the user has 5 workspaces, but tried to hit page 100, which + // does not exist. In this case, the page is not valid and we want to show a + // better error message. + const pageNumberIsInvalid = page !== 1 && workspaces?.length === 0; return ( @@ -187,7 +187,7 @@ export const WorkspacesPageView: FC = ({ ) : ( - !invalidPageNumber && ( + !pageNumberIsInvalid && ( = ({ )} - {invalidPageNumber ? ( + {pageNumberIsInvalid ? ( ({ border: `1px solid ${theme.palette.divider}`, diff --git a/site/src/pages/WorkspacesPage/batchActions.ts b/site/src/pages/WorkspacesPage/batchActions.ts index 806c7a03afddb..613dec0790265 100644 --- a/site/src/pages/WorkspacesPage/batchActions.ts +++ b/site/src/pages/WorkspacesPage/batchActions.ts @@ -1,13 +1,32 @@ import { API } from "api/api"; -import type { Workspace } from "api/typesGenerated"; +import type { Workspace, WorkspaceBuild } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { useMutation } from "react-query"; -interface UseBatchActionsProps { +interface UseBatchActionsOptions { onSuccess: () => Promise; } -export function useBatchActions(options: UseBatchActionsProps) { +type UpdateAllPayload = Readonly<{ + workspaces: readonly Workspace[]; + isDynamicParametersEnabled: boolean; +}>; + +type UseBatchActionsResult = Readonly<{ + isProcessing: boolean; + start: (workspaces: readonly Workspace[]) => Promise; + stop: (workspaces: readonly Workspace[]) => Promise; + delete: (workspaces: readonly Workspace[]) => Promise; + updateTemplateVersions: ( + payload: UpdateAllPayload, + ) => Promise; + favorite: (payload: readonly Workspace[]) => Promise; + unfavorite: (payload: readonly Workspace[]) => Promise; +}>; + +export function useBatchActions( + options: UseBatchActionsOptions, +): UseBatchActionsResult { const { onSuccess } = options; const startAllMutation = useMutation({ @@ -45,10 +64,7 @@ export function useBatchActions(options: UseBatchActionsProps) { }); const updateAllMutation = useMutation({ - mutationFn: (payload: { - workspaces: readonly Workspace[]; - isDynamicParametersEnabled: boolean; - }) => { + mutationFn: (payload: UpdateAllPayload) => { const { workspaces, isDynamicParametersEnabled } = payload; return Promise.all( workspaces @@ -62,9 +78,13 @@ export function useBatchActions(options: UseBatchActionsProps) { }, }); + // We have to explicitly make the mutation functions for the + // favorite/unfavorite functionality be async and then void out the + // Promise.all result because otherwise the return type becomes a void + // array, which doesn't ever make sense with TypeScript's type system const favoriteAllMutation = useMutation({ - mutationFn: (workspaces: readonly Workspace[]) => { - return Promise.all( + mutationFn: async (workspaces: readonly Workspace[]) => { + void Promise.all( workspaces .filter((w) => !w.favorite) .map((w) => API.putFavoriteWorkspace(w.id)), @@ -77,8 +97,8 @@ export function useBatchActions(options: UseBatchActionsProps) { }); const unfavoriteAllMutation = useMutation({ - mutationFn: (workspaces: readonly Workspace[]) => { - return Promise.all( + mutationFn: async (workspaces: readonly Workspace[]) => { + void Promise.all( workspaces .filter((w) => w.favorite) .map((w) => API.deleteFavoriteWorkspace(w.id)), @@ -91,13 +111,13 @@ export function useBatchActions(options: UseBatchActionsProps) { }); return { - favoriteAll: favoriteAllMutation.mutateAsync, - unfavoriteAll: unfavoriteAllMutation.mutateAsync, - startAll: startAllMutation.mutateAsync, - stopAll: stopAllMutation.mutateAsync, - deleteAll: deleteAllMutation.mutateAsync, - updateAll: updateAllMutation.mutateAsync, - isLoading: + favorite: favoriteAllMutation.mutateAsync, + unfavorite: unfavoriteAllMutation.mutateAsync, + start: startAllMutation.mutateAsync, + stop: stopAllMutation.mutateAsync, + delete: deleteAllMutation.mutateAsync, + updateTemplateVersions: updateAllMutation.mutateAsync, + isProcessing: favoriteAllMutation.isPending || unfavoriteAllMutation.isPending || startAllMutation.isPending || From 4833ae1b831e87ddc70bfd2955418c8a0f2b47a7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 01:40:21 +0000 Subject: [PATCH 05/26] fix: remove promise race conditions for batch utilities --- site/src/pages/WorkspacesPage/batchActions.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/site/src/pages/WorkspacesPage/batchActions.ts b/site/src/pages/WorkspacesPage/batchActions.ts index 613dec0790265..9d3c89fbd7611 100644 --- a/site/src/pages/WorkspacesPage/batchActions.ts +++ b/site/src/pages/WorkspacesPage/batchActions.ts @@ -78,13 +78,14 @@ export function useBatchActions( }, }); - // We have to explicitly make the mutation functions for the - // favorite/unfavorite functionality be async and then void out the - // Promise.all result because otherwise the return type becomes a void - // array, which doesn't ever make sense with TypeScript's type system + // Not a great idea to return the promises from the Promise.all calls below + // because that then gives you a void array, which doesn't make sense with + // TypeScript's type system. Best to await them, and then have the wrapper + // mutation function return its own void promise + const favoriteAllMutation = useMutation({ mutationFn: async (workspaces: readonly Workspace[]) => { - void Promise.all( + await Promise.all( workspaces .filter((w) => !w.favorite) .map((w) => API.putFavoriteWorkspace(w.id)), @@ -98,7 +99,7 @@ export function useBatchActions( const unfavoriteAllMutation = useMutation({ mutationFn: async (workspaces: readonly Workspace[]) => { - void Promise.all( + await Promise.all( workspaces .filter((w) => w.favorite) .map((w) => API.deleteFavoriteWorkspace(w.id)), From b8d92fa781de3d71819d434762db87ecec874ad0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 01:59:28 +0000 Subject: [PATCH 06/26] fix: decouple component props from one another --- .../pages/WorkspacesPage/WorkspacesPage.tsx | 2 +- .../WorkspacesPageView.stories.tsx | 6 +-- .../WorkspacesPage/WorkspacesPageView.tsx | 31 ++++++++------- .../filter/WorkspacesFilter.tsx | 38 +++++++++++++------ 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 070732df9980f..08779f01813ae 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -185,7 +185,7 @@ const WorkspacesPage: FC = () => { page={pagination.page} limit={pagination.limit} onPageChange={pagination.goToPage} - filterProps={filterState} + filterState={filterState} isRunningBatchAction={batchActions.isProcessing} onBatchDeleteTransition={() => setActiveBatchAction("delete")} onBatchStartTransition={() => batchActions.start(checkedWorkspaces)} diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index e0178dea06c09..7b639c087ee87 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -134,7 +134,7 @@ const allWorkspaces = [ ...Object.values(additionalWorkspaces), ]; -type FilterProps = ComponentProps["filterProps"]; +type FilterProps = ComponentProps["filterState"]; const defaultFilterProps = getDefaultFilterProps({ query: "owner:me", @@ -169,7 +169,7 @@ const meta: Meta = { component: WorkspacesPageView, args: { limit: DEFAULT_RECORDS_PER_PAGE, - filterProps: defaultFilterProps, + filterState: defaultFilterProps, checkedWorkspaces: [], canCheckWorkspaces: true, templates: mockTemplates, @@ -266,7 +266,7 @@ export const UserHasNoWorkspacesAndNoTemplates: Story = { export const NoSearchResults: Story = { args: { workspaces: [], - filterProps: { + filterState: { ...defaultFilterProps, filter: { ...defaultFilterProps.filter, diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 1de4534b25f22..4476211387871 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -26,7 +26,7 @@ import { mustUpdateWorkspace } from "utils/workspace"; import { WorkspaceHelpTooltip } from "./WorkspaceHelpTooltip"; import { WorkspacesButton } from "./WorkspacesButton"; import { - type WorkspaceFilterProps, + type WorkspaceFilterState, WorkspacesFilter, } from "./filter/WorkspacesFilter"; @@ -45,7 +45,7 @@ interface WorkspacesPageViewProps { workspaces?: readonly Workspace[]; checkedWorkspaces: readonly Workspace[]; count?: number; - filterProps: WorkspaceFilterProps; + filterState: WorkspaceFilterState; page: number; limit: number; onPageChange: (page: number) => void; @@ -69,15 +69,15 @@ export const WorkspacesPageView: FC = ({ error, limit, count, - filterProps, + filterState, onPageChange, page, checkedWorkspaces, onCheckChange, - onBatchDeleteTransition: onDeleteAll, - onBatchUpdateTransition: onUpdateAll, - onBatchStopTransition: onStopAll, - onBatchStartTransition: onStartAll, + onBatchDeleteTransition, + onBatchUpdateTransition, + onBatchStopTransition, + onBatchStartTransition, isRunningBatchAction, canCheckWorkspaces, templates, @@ -117,9 +117,12 @@ export const WorkspacesPageView: FC = ({ )} @@ -155,7 +158,7 @@ export const WorkspacesPageView: FC = ({ !mustUpdateWorkspace(w, canChangeVersions), ) } - onClick={onStartAll} + onClick={onBatchStartTransition} > Start @@ -165,12 +168,12 @@ export const WorkspacesPageView: FC = ({ (w) => w.latest_build.status === "running", ) } - onClick={onStopAll} + onClick={onBatchStopTransition} > Stop - + = ({ Delete… @@ -221,7 +224,7 @@ export const WorkspacesPageView: FC = ({ ; +export type WorkspaceFilterState = { + filter: UseFilterResult; error?: unknown; menus: { user?: UserFilterMenu; @@ -73,21 +73,35 @@ export type WorkspaceFilterProps = { }; }; +type WorkspaceFilterProps = Readonly<{ + filter: UseFilterResult; + error: unknown; + templateMenu: TemplateFilterMenu; + statusMenu: StatusFilterMenu; + + userMenu?: UserFilterMenu; + organizationsMenu?: OrganizationsFilterMenu +}> + export const WorkspacesFilter: FC = ({ filter, error, - menus, + templateMenu, + statusMenu, + userMenu, + organizationsMenu, }) => { const { entitlements, showOrganizations } = useDashboard(); const width = showOrganizations ? 175 : undefined; const presets = entitlements.features.advanced_template_scheduling.enabled ? PRESETS_WITH_DORMANT : PRESET_FILTERS; + const organizationsActive = showOrganizations && organizationsMenu !== undefined return ( = ({ )} options={ <> - {menus.user && } - - - {showOrganizations && menus.organizations !== undefined && ( - + {userMenu && } + + + {organizationsActive && ( + )} } optionsSkeleton={ <> - {menus.user && } + {userMenu && } - {showOrganizations && } + {organizationsActive && } } /> From 1662a55d513a00564b44ea9feb068e4ea1b5efef Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:06:06 +0000 Subject: [PATCH 07/26] fix: apply biome fixes --- site/package.json | 6 +++++- site/src/pages/WorkspacesPage/WorkspacesPage.tsx | 2 +- site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/site/package.json b/site/package.json index 8d688b45c928b..ca55bfde68cf9 100644 --- a/site/package.json +++ b/site/package.json @@ -191,7 +191,11 @@ "vite-plugin-checker": "0.9.3", "vite-plugin-turbosnap": "1.0.3" }, - "browserslist": ["chrome 110", "firefox 111", "safari 16.0"], + "browserslist": [ + "chrome 110", + "firefox 111", + "safari 16.0" + ], "resolutions": { "optionator": "0.9.3", "semver": "7.6.2" diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 08779f01813ae..18caa6aba3285 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -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. */ -export const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ +const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ "canceling", "deleting", "pending", diff --git a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx index b684807ffb06b..5b00fa495634d 100644 --- a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx +++ b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx @@ -1,4 +1,4 @@ -import { Filter, MenuSkeleton, UseFilterResult, type useFilter } from "components/Filter/Filter"; +import { Filter, MenuSkeleton, type UseFilterResult } from "components/Filter/Filter"; import { type UserFilterMenu, UserMenu } from "components/Filter/UserFilter"; import { useDashboard } from "modules/dashboard/useDashboard"; import { From e3264144fdf2e68d6ba6363b25da01ad7c96badd Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:08:47 +0000 Subject: [PATCH 08/26] refactor: remove more bad coupling --- site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index 7b639c087ee87..d24ae72bd8e0f 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -31,6 +31,7 @@ import { withProxyProvider, } from "testHelpers/storybook"; import { WorkspacesPageView } from "./WorkspacesPageView"; +import { WorkspaceFilterState } from "./filter/WorkspacesFilter"; const createWorkspace = ( name: string, @@ -134,9 +135,7 @@ const allWorkspaces = [ ...Object.values(additionalWorkspaces), ]; -type FilterProps = ComponentProps["filterState"]; - -const defaultFilterProps = getDefaultFilterProps({ +const defaultFilterProps = getDefaultFilterProps({ query: "owner:me", menus: { user: MockMenu, From cc0010614f2237a6be16f5acf916bcba5028f0be Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:12:52 +0000 Subject: [PATCH 09/26] fix: format --- site/package.json | 6 +----- .../WorkspacesPage/filter/WorkspacesFilter.tsx | 13 +++++++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/site/package.json b/site/package.json index ca55bfde68cf9..8d688b45c928b 100644 --- a/site/package.json +++ b/site/package.json @@ -191,11 +191,7 @@ "vite-plugin-checker": "0.9.3", "vite-plugin-turbosnap": "1.0.3" }, - "browserslist": [ - "chrome 110", - "firefox 111", - "safari 16.0" - ], + "browserslist": ["chrome 110", "firefox 111", "safari 16.0"], "resolutions": { "optionator": "0.9.3", "semver": "7.6.2" diff --git a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx index 5b00fa495634d..caebfd04526d4 100644 --- a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx +++ b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx @@ -1,4 +1,8 @@ -import { Filter, MenuSkeleton, type UseFilterResult } from "components/Filter/Filter"; +import { + Filter, + MenuSkeleton, + type UseFilterResult, +} from "components/Filter/Filter"; import { type UserFilterMenu, UserMenu } from "components/Filter/UserFilter"; import { useDashboard } from "modules/dashboard/useDashboard"; import { @@ -80,8 +84,8 @@ type WorkspaceFilterProps = Readonly<{ statusMenu: StatusFilterMenu; userMenu?: UserFilterMenu; - organizationsMenu?: OrganizationsFilterMenu -}> + organizationsMenu?: OrganizationsFilterMenu; +}>; export const WorkspacesFilter: FC = ({ filter, @@ -96,7 +100,8 @@ export const WorkspacesFilter: FC = ({ const presets = entitlements.features.advanced_template_scheduling.enabled ? PRESETS_WITH_DORMANT : PRESET_FILTERS; - const organizationsActive = showOrganizations && organizationsMenu !== undefined + const organizationsActive = + showOrganizations && organizationsMenu !== undefined; return ( Date: Wed, 16 Jul 2025 02:14:30 +0000 Subject: [PATCH 10/26] fix: update call site mismatch --- site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx b/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx index 9cd27bac95bf4..40fac8ef3fb48 100644 --- a/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx +++ b/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx @@ -29,7 +29,8 @@ const ConnectionLogPage: FC = () => { paginatedConnectionLogs(searchParams), ); const filter = useFilter({ - searchParamsResult: [searchParams, setSearchParams], + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: connectionlogsQuery.goToFirstPage, }); From b512e18ae9fb85c0f6b90c61bcb64e1b8b7ee385 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:22:11 +0000 Subject: [PATCH 11/26] fix: update import --- site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index d24ae72bd8e0f..eea516eae2976 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -12,7 +12,6 @@ import { import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; import dayjs from "dayjs"; import uniqueId from "lodash/uniqueId"; -import type { ComponentProps } from "react"; import { MockBuildInfo, MockOrganization, @@ -31,7 +30,7 @@ import { withProxyProvider, } from "testHelpers/storybook"; import { WorkspacesPageView } from "./WorkspacesPageView"; -import { WorkspaceFilterState } from "./filter/WorkspacesFilter"; +import type { WorkspaceFilterState } from "./filter/WorkspacesFilter"; const createWorkspace = ( name: string, From 18ad06441df951becdbebd06f0669ce022f86d14 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 02:27:47 +0000 Subject: [PATCH 12/26] 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 cb92b79abe31b5c5513124cc019d466a0f0e32d7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 03:23:38 +0000 Subject: [PATCH 13/26] fix: update Filter logic to account for fallback filter --- site/src/components/Filter/Filter.tsx | 7 +++---- site/src/pages/AuditPage/AuditPage.test.tsx | 3 ++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/site/src/components/Filter/Filter.tsx b/site/src/components/Filter/Filter.tsx index 1cdff25547c30..75adb465e2c06 100644 --- a/site/src/components/Filter/Filter.tsx +++ b/site/src/components/Filter/Filter.tsx @@ -57,14 +57,13 @@ export const useFilter = ({ const update = (newValues: string | FilterValues) => { const serialized = typeof newValues === "string" ? newValues : stringifyFilter(newValues); - const noUpdateNeeded = searchParams.get(useFilterParamsKey) === serialized; + const noUpdateNeeded = query === serialized; if (noUpdateNeeded) { return; } - const copy = new URLSearchParams(searchParams); - copy.set(useFilterParamsKey, serialized); - onSearchParamsChange(copy); + searchParams.set(useFilterParamsKey, serialized); + onSearchParamsChange(searchParams); onUpdate?.(serialized); }; diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index bcbc40da8af5c..949c5127911c1 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -15,6 +15,7 @@ import { import { server } from "testHelpers/server"; import * as CreateDayString from "utils/createDayString"; import AuditPage from "./AuditPage"; +import type { AuditLogsRequest } from "api/typesGenerated"; interface RenderPageOptions { filter?: string; @@ -106,7 +107,7 @@ describe("AuditPage", () => { await userEvent.type(filterField, query); await waitFor(() => - expect(getAuditLogsSpy).toBeCalledWith({ + expect(getAuditLogsSpy).toHaveBeenCalledWith<[AuditLogsRequest]>({ limit: DEFAULT_RECORDS_PER_PAGE, offset: 0, q: query, From 86d19b8d7399fb179e7d2c09c5fdc2abe06c3588 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 03:32:48 +0000 Subject: [PATCH 14/26] docs: add comment about wonky code --- site/src/components/Filter/Filter.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/site/src/components/Filter/Filter.tsx b/site/src/components/Filter/Filter.tsx index 75adb465e2c06..2310867cb274c 100644 --- a/site/src/components/Filter/Filter.tsx +++ b/site/src/components/Filter/Filter.tsx @@ -62,6 +62,16 @@ export const useFilter = ({ return; } + /** + * @todo 2025-07-15 - We have a slightly nasty bug here, where trying to + * update state the "React way" causes our code to break. + * + * In theory, it would be better to make a copy of the search params. We + * can then mutate and dispatch the copy instead of the original. Doing + * that causes other parts of our existing logic to break, though. + * That's a sign that our other code is slightly broken, and only just + * happens to work by chance right now. + */ searchParams.set(useFilterParamsKey, serialized); onSearchParamsChange(searchParams); onUpdate?.(serialized); From 330d91db0ec57870d0f10ff466e6a474129ba93f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 04:07:38 +0000 Subject: [PATCH 15/26] fix: get workspace tests passing --- site/src/hooks/usePagination.ts | 4 ++-- site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 0bc81d7325626..8d94a8544492c 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -19,8 +19,8 @@ export function usePagination( ): UsePaginationResult { const { searchParams, onSearchParamsChange } = options; const limit = DEFAULT_RECORDS_PER_PAGE; - const rawPage = Number.parseInt(searchParams.get(paginationKey) || "0", 10); - const page = Number.isNaN(rawPage) ? 1 : rawPage; + const rawPage = Number.parseInt(searchParams.get(paginationKey) || "1", 10); + const page = Number.isNaN(rawPage) || rawPage <= 0 ? 1 : rawPage; return { page, diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 56f8fb34a32e8..bebc3f47dd50f 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -1,7 +1,7 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; -import type { Workspace } from "api/typesGenerated"; +import type { Template, WorkspacesResponse, Workspace } from "api/typesGenerated"; import { http, HttpResponse } from "msw"; import { MockDormantOutdatedWorkspace, @@ -28,18 +28,14 @@ describe("WorkspacesPage", () => { }); it("renders an empty workspaces page", async () => { - // Given server.use( http.get("/api/v2/workspaces", async () => { - return HttpResponse.json({ workspaces: [], count: 0 }); + return HttpResponse.json({ workspaces: [], count: 0 }); }), ); - // When renderWithAuth(); - - // Then - await screen.findByText("Create a workspace"); + await screen.findByRole("heading", { name: /Create a workspace/ }); }); it("renders a filled workspaces page", async () => { From fb9d8f2642848c25977a2b295b70d1ea2e950438 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 04:10:52 +0000 Subject: [PATCH 16/26] fix: update biome again --- site/src/pages/AuditPage/AuditPage.test.tsx | 2 +- site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index 949c5127911c1..80f6e74ae1a26 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -1,6 +1,7 @@ import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; +import type { AuditLogsRequest } from "api/typesGenerated"; import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; import { http, HttpResponse } from "msw"; import { @@ -15,7 +16,6 @@ import { import { server } from "testHelpers/server"; import * as CreateDayString from "utils/createDayString"; import AuditPage from "./AuditPage"; -import type { AuditLogsRequest } from "api/typesGenerated"; interface RenderPageOptions { filter?: string; diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index bebc3f47dd50f..678f0331331a6 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -1,7 +1,7 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; -import type { Template, WorkspacesResponse, Workspace } from "api/typesGenerated"; +import type { Workspace, WorkspacesResponse } from "api/typesGenerated"; import { http, HttpResponse } from "msw"; import { MockDormantOutdatedWorkspace, @@ -30,7 +30,10 @@ describe("WorkspacesPage", () => { it("renders an empty workspaces page", async () => { server.use( http.get("/api/v2/workspaces", async () => { - return HttpResponse.json({ workspaces: [], count: 0 }); + return HttpResponse.json({ + workspaces: [], + count: 0, + }); }), ); From 5504b5bcbf6bec84112bb7bfceebd969830d7f5f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 04:26:23 +0000 Subject: [PATCH 17/26] 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 18/26] 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 19/26] 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 20/26] 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 21/26] 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" }, From 6a9a37b191e9673453b1fc1244cfaa3a4b1d7cb7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 04:54:47 +0000 Subject: [PATCH 22/26] chore: swap in action callbacks --- .../WorkspacesPage/BatchUpdateModalForm.stories.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx index 8b44f02d07fe1..68ab530dbaa7d 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -1,5 +1,5 @@ -import type { Meta, Parameters, StoryObj } from "@storybook/react"; -import { expect, screen, userEvent, within } from "@storybook/test"; +import type { Meta, Parameters, StoryObj } from "@storybook/react-vite"; +import { expect, screen, userEvent, within } from "storybook/test"; import { templateVersionRoot } from "api/queries/templates"; import type { TemplateVersion, @@ -10,6 +10,7 @@ import { useQueryClient } from "react-query"; import { MockTemplateVersion, MockWorkspace } from "testHelpers/entities"; import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; import { ACTIVE_BUILD_STATUSES } from "./WorkspacesPage"; +import { action } from "storybook/internal/actions"; type Writeable = { -readonly [Key in keyof T]: T[Key] }; type MutableWorkspace = Writeable> & { @@ -22,11 +23,8 @@ const meta: Meta = { 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"), + onSubmit: action("All selected workspaces have been updated"), + onCancel: action("Update canceled"), }, }; From 4c371647c8e885422cac4e9721b72dfbe170be94 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 05:26:06 +0000 Subject: [PATCH 23/26] chore: start applying feedback --- .../BatchUpdateModalForm.stories.tsx | 69 ++++++++++--------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx index 68ab530dbaa7d..c352b97c94770 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -1,5 +1,4 @@ import type { Meta, Parameters, StoryObj } from "@storybook/react-vite"; -import { expect, screen, userEvent, within } from "storybook/test"; import { templateVersionRoot } from "api/queries/templates"; import type { TemplateVersion, @@ -7,10 +6,11 @@ import type { WorkspaceBuild, } from "api/typesGenerated"; import { useQueryClient } from "react-query"; +import { action } from "storybook/internal/actions"; +import { expect, screen, userEvent, within } from "storybook/test"; import { MockTemplateVersion, MockWorkspace } from "testHelpers/entities"; import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; import { ACTIVE_BUILD_STATUSES } from "./WorkspacesPage"; -import { action } from "storybook/internal/actions"; type Writeable = { -readonly [Key in keyof T]: T[Key] }; type MutableWorkspace = Writeable> & { @@ -81,20 +81,19 @@ export const OnlyReadyToUpdate: Story = { beforeEach: (ctx) => { const { workspaces, queries } = createPatchedDependencies(3); ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.parameters.queries = 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; - } + const notOutdated = workspaces.map((ws) => { + return { ...ws, outdated: false }; + }); - ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.args = { ...ctx.args, workspacesToUpdate: notOutdated }; + ctx.parameters.queries = queries; }, }; @@ -103,7 +102,7 @@ export const CurrentlyProcessing: Story = { beforeEach: (ctx) => { const { workspaces, queries } = createPatchedDependencies(3); ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.parameters.queries = queries; }, }; @@ -120,7 +119,7 @@ export const OnlyDormantWorkspaces: Story = { writable.dormant_at = new Date().toISOString(); } ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.parameters.queries = queries; }, }; @@ -128,7 +127,7 @@ export const FetchError: Story = { beforeEach: (ctx) => { const { workspaces, queries } = createPatchedDependencies(3); ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.parameters.queries = queries; }, decorators: [ (Story, ctx) => { @@ -155,39 +154,45 @@ export const TransitioningWorkspaces: Story = { const { workspaces, queries } = createPatchedDependencies( 2 * ACTIVE_BUILD_STATUSES.length, ); - for (const [i, ws] of workspaces.entries()) { + const withUpdatedStatuses = workspaces.map((ws, i) => { if (i % 2 === 0) { - continue; + return ws; } - 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 }; + return { + ...ws, + latest_build: { + ...ws.latest_build, + status: ACTIVE_BUILD_STATUSES[i % ACTIVE_BUILD_STATUSES.length], + }, + }; + }); + + ctx.args = { ...ctx.args, workspacesToUpdate: withUpdatedStatuses }; + ctx.parameters.queries = 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 }; + const allRunning = workspaces.map((ws) => { + return { ...ws, status: "running" }; + }); + + ctx.args = { ...ctx.args, workspacesToUpdate: allRunning }; + ctx.parameters.queries = 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 }; + const allRunning = workspaces.map((ws) => { + return { ...ws, status: "running" }; + }); + + ctx.args = { ...ctx.args, workspacesToUpdate: allRunning }; + ctx.parameters.queries = queries; }, play: async () => { // Can't use canvasElement from the play function's context because the @@ -270,6 +275,6 @@ export const MixOfWorkspaces: Story = { noUpdatesNeededTransitioning.latest_build.status = "starting"; ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; - ctx.parameters = { ...ctx.parameters, queries }; + ctx.parameters.queries = queries; }, }; From 82e3d71c69457e669763f757579a6065a189ae9b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 05:34:03 +0000 Subject: [PATCH 24/26] refactor: split off li elements --- .../WorkspacesPage/BatchUpdateModalForm.tsx | 88 +++++++++---------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx index 7e1a0f1358296..0bcdcfca765c8 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx @@ -28,6 +28,34 @@ import { useQueries } from "react-query"; import { cn } from "utils/cn"; import { ACTIVE_BUILD_STATUSES } from "./WorkspacesPage"; +export const BatchUpdateModalForm: FC = ({ + open, + isProcessing, + workspacesToUpdate, + onCancel, + onSubmit, +}) => { + return ( + { + if (open) { + onCancel(); + } + }} + > + + + + + ); +}; + type WorkspacePartitionByUpdateType = Readonly<{ dormant: readonly Workspace[]; noUpdateNeeded: readonly Workspace[]; @@ -109,6 +137,14 @@ const ReviewPanel: FC = ({ ); }; +const PanelListItem: FC = ({ children }) => { + return ( +
  • + {children} +
    • + ) +} + type TemplateNameChangeProps = Readonly<{ oldTemplateVersionName: string; newTemplateVersionName: string; @@ -460,10 +496,7 @@ const ReviewForm: FC = ({ const newTemplateName = matchedQuery?.data?.name; return ( -
  • + = ({ ) } /> -
    • + ); })} @@ -493,10 +526,7 @@ const ReviewForm: FC = ({ description="These workspaces are already updated and will be skipped." > {noUpdateNeeded.map((ws) => ( -
  • + = ({ workspaceName={ws.name} workspaceIconUrl={ws.template_icon} /> -
    • + ))} )} @@ -595,40 +625,4 @@ type BatchUpdateModalFormProps = Readonly<{ 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. - */} - - - - ); -}; + From 269dda00f99d58026910cc7381393c3e14aeebc8 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 05:45:53 +0000 Subject: [PATCH 25/26] chore: update one story --- .../BatchUpdateModalForm.stories.tsx | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx index c352b97c94770..08e760b9f2227 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -152,22 +152,17 @@ export const TransitioningWorkspaces: Story = { args: { isProcessing: true }, beforeEach: (ctx) => { const { workspaces, queries } = createPatchedDependencies( - 2 * ACTIVE_BUILD_STATUSES.length, + // Adding one extra so that we still have a stopped workspace at the + // end of the list + 1 + ACTIVE_BUILD_STATUSES.length, ); - const withUpdatedStatuses = workspaces.map((ws, i) => { - if (i % 2 === 0) { - return ws; - } - return { - ...ws, - latest_build: { - ...ws.latest_build, - status: ACTIVE_BUILD_STATUSES[i % ACTIVE_BUILD_STATUSES.length], - }, - }; - }); - ctx.args = { ...ctx.args, workspacesToUpdate: withUpdatedStatuses }; + for (const [i, status] of ACTIVE_BUILD_STATUSES.entries() ) { + const mutable = workspaces[i] as MutableWorkspace; + mutable.latest_build.status = status; + } + + ctx.args = { ...ctx.args, workspacesToUpdate: workspaces }; ctx.parameters.queries = queries; }, }; From af87b63edd65ffc57f297135dbabe658268c2b6c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 20:11:41 +0000 Subject: [PATCH 26/26] fix: format --- .../pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx | 2 +- site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx index 08e760b9f2227..d3aea5a6e0f52 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -157,7 +157,7 @@ export const TransitioningWorkspaces: Story = { 1 + ACTIVE_BUILD_STATUSES.length, ); - for (const [i, status] of ACTIVE_BUILD_STATUSES.entries() ) { + for (const [i, status] of ACTIVE_BUILD_STATUSES.entries()) { const mutable = workspaces[i] as MutableWorkspace; mutable.latest_build.status = status; } diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx index 0bcdcfca765c8..0beb61efd4f3c 100644 --- a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx @@ -142,8 +142,8 @@ const PanelListItem: FC = ({ children }) => {
  • {children}
    • - ) -} + ); +}; type TemplateNameChangeProps = Readonly<{ oldTemplateVersionName: string; @@ -624,5 +624,3 @@ type BatchUpdateModalFormProps = Readonly<{ onCancel: () => void; onSubmit: () => void; }>; - -