From bb05088f4beb299313f635a992566bbd289bf389 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 00:48:56 +0000 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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 cb92b79abe31b5c5513124cc019d466a0f0e32d7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 16 Jul 2025 03:23:38 +0000 Subject: [PATCH 12/16] 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 13/16] 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 14/16] 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 15/16] 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 ebd8b1c784df07caffb22bfc2d5d81d49ec6896b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 14 Aug 2025 03:04:36 +0000 Subject: [PATCH 16/16] chore: apply feedback --- site/src/components/Filter/Filter.tsx | 2 +- site/src/hooks/usePagination.ts | 6 ++++-- .../CreateWorkspacePageViewExperimental.tsx | 10 ++-------- site/src/pages/WorkspacesPage/batchActions.ts | 9 ++------- 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/site/src/components/Filter/Filter.tsx b/site/src/components/Filter/Filter.tsx index 2310867cb274c..ea5e60ff0a7f9 100644 --- a/site/src/components/Filter/Filter.tsx +++ b/site/src/components/Filter/Filter.tsx @@ -64,7 +64,7 @@ export const useFilter = ({ /** * @todo 2025-07-15 - We have a slightly nasty bug here, where trying to - * update state the "React way" causes our code to break. + * update state via immutable state updates 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 diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 8d94a8544492c..235a1505033e1 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -28,13 +28,15 @@ export function usePagination( offset: Math.max(0, (page - 1) * limit), goToPage: (newPage) => { const abortNavigation = - page === newPage || !Number.isFinite(page) || !Number.isInteger(page); + page === newPage || + !Number.isFinite(newPage) || + !Number.isInteger(newPage); if (abortNavigation) { return; } const copy = new URLSearchParams(searchParams); - copy.set("page", page.toString()); + copy.set("page", newPage.toString()); onSearchParamsChange(copy); }, }; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 6defee1c10e4c..99876bfdb534d 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -118,14 +118,8 @@ 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>( - (touched, parameter) => { - if (autofillByName[parameter.name] !== undefined) { - touched[parameter.name] = true; - } - return touched; - }, - {}, + const initialTouched = Object.fromEntries( + parameters.filter((p) => autofillByName[p.name]).map((p) => [p, true]), ); // 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/WorkspacesPage/batchActions.ts b/site/src/pages/WorkspacesPage/batchActions.ts index 9d3c89fbd7611..a4b6c830537d9 100644 --- a/site/src/pages/WorkspacesPage/batchActions.ts +++ b/site/src/pages/WorkspacesPage/batchActions.ts @@ -78,13 +78,8 @@ export function useBatchActions( }, }); - // 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[]) => { + mutationFn: async (workspaces: readonly Workspace[]): Promise => { await Promise.all( workspaces .filter((w) => !w.favorite) @@ -98,7 +93,7 @@ export function useBatchActions( }); const unfavoriteAllMutation = useMutation({ - mutationFn: async (workspaces: readonly Workspace[]) => { + mutationFn: async (workspaces: readonly Workspace[]): Promise => { await Promise.all( workspaces .filter((w) => w.favorite)