diff --git a/.vscode/settings.json b/.vscode/settings.json index f2cf72b7d8ae0..3667afcdc18ed 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -60,5 +60,8 @@ "typos.config": ".github/workflows/typos.toml", "[markdown]": { "editor.defaultFormatter": "DavidAnson.vscode-markdownlint" + }, + "[typescriptreact]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" } } diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 5135f2304426e..fe42d4aa6854b 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), }; }; 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..a9c4703b88ae5 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -1,25 +1,38 @@ 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; +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 page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; const limit = DEFAULT_RECORDS_PER_PAGE; - const offset = page <= 0 ? 0 : (page - 1) * limit; - - const goToPage = (page: number) => { - searchParams.set("page", page.toString()); - setSearchParams(searchParams); - }; return { page, limit, - goToPage, - offset, + offset: page <= 0 ? 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/BatchUpdateModalForm.stories.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx new file mode 100644 index 0000000000000..0dd4853b9f9c3 --- /dev/null +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.stories.tsx @@ -0,0 +1,34 @@ +import { Meta, StoryObj } from "@storybook/react"; +import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; + +const meta: Meta = { + title: "pages/WorkspacesPage/BatchUpdateModalForm", + component: BatchUpdateModalForm, + args: { + // Not terribly useful to represent any stories without the modal being + // open + open: true, + }, +}; + +export default meta; +type Story = StoryObj; + +export const MixOfWorkspaces: Story = {}; + +export const ProcessingWithWorkspaces: Story = {}; + +export const OnlyDormant: Story = {}; + +export const WorkspacesWitFetchError: Story = {}; + +export const OnlyReadyToUpdate: Story = {}; + +export const TransitioningWorkspaces: Story = {}; + +// Be sure to add an action for failing to accept consequences +export const RunningWorkspaces: Story = {}; + +export const TriggeredVerticalOverflow: Story = {}; + +export const NoWorkspacesToUpdate: Story = {}; diff --git a/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx new file mode 100644 index 0000000000000..d460cad6a58f2 --- /dev/null +++ b/site/src/pages/WorkspacesPage/BatchUpdateModalForm.tsx @@ -0,0 +1,605 @@ +import { Label } from "@radix-ui/react-label"; +import { Slot } from "@radix-ui/react-slot"; +import { templateVersion } from "api/queries/templates"; +import type { Workspace, WorkspaceStatus } 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, + PropsWithChildren, + type ReactNode, + useId, + useRef, + useState, +} from "react"; +import { useQueries } from "react-query"; +import { cn } from "utils/cn"; + +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"; + +// 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 user goes +// through the form quickly enough, they'll be able to update without seeing the +// warning. +const transitioningStatuses: readonly WorkspaceStatus[] = [ + "canceling", + "deleting", + "pending", + "starting", + "stopping", +]; + +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 formIsNeeded = readyToUpdate.length > 0 || dormant.length > 0; + if (!formIsNeeded) { + return ( + + + {error !== undefined && } + + + + + + + ); + } + + const runningIds = new Set( + readyToUpdate + .filter((ws) => ws.latest_build.status === "running") + .map((ws) => ws.id), + ); + + // Just to be on the safe side, we need to derive the 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) => transitioningStatuses.includes(ws.latest_build.status)) + .map((ws) => ws.id), + ); + + const failedValidationId = `${hookId}-failed-validation`; + const hasRunningWorkspaces = runningIds.size > 0; + const consequencesResolved = !hasRunningWorkspaces || stage === "accepted"; + + // For UX/accessibility reasons, we're splitting hairs between whether a + // form submission seems valid, versus whether clicking the button will give + // the user useful results/feedback. If we do a blanket disable for the + // button, there's many cases where there's no way to give the user feedback + // on how to get themselves unstuck. + const submitButtonDisabled = isProcessing || transitioningIds.size > 0; + const submitIsValid = + consequencesResolved && error === undefined && readyToUpdate.length > 0; + + return ( + +
{ + e.preventDefault(); + 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 be skipped during the + batch update. + + } + > + {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 22ba0d15f1f9a..bb1d9a38b91d9 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,8 +1,11 @@ import { getErrorDetail, getErrorMessage } from "api/errors"; import { workspacePermissionsByOrganization } from "api/queries/organizations"; -import { templates } from "api/queries/templates"; +import { + templates, + templateVersion, + templateVersionRoot, +} from "api/queries/templates"; import { workspaces } from "api/queries/workspaces"; -import type { Workspace } from "api/typesGenerated"; import { useFilter } from "components/Filter/Filter"; import { useUserFilterMenu } from "components/Filter/UserFilter"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -11,18 +14,18 @@ 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"; import { pageTitle } from "utils/page"; import { BatchDeleteConfirmation } from "./BatchDeleteConfirmation"; -import { BatchUpdateConfirmation } from "./BatchUpdateConfirmation"; import { WorkspacesPageView } from "./WorkspacesPageView"; import { useBatchActions } from "./batchActions"; import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; +import { BatchUpdateModalForm } from "./BatchUpdateModalForm"; -function useSafeSearchParams() { +function useSafeSearchParams(): ReturnType { // Have to wrap setSearchParams because React Router doesn't make sure that // the function's memory reference stays stable on each render, even though // its logic never changes, and even though it has function update support @@ -36,13 +39,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 = () => { + setCheckedWorkspaceIds((current) => { + return current.size === 0 ? current : new Set(); + }); + }; + + const pagination = usePagination({ + searchParams, + onSearchParamsChange: (newParams) => { + setSearchParams(newParams); + resetChecked(); + }, + }); const { permissions, user: me } = useAuthenticated(); const { entitlements } = useDashboard(); const templatesQuery = useQuery(templates()); @@ -66,14 +91,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, @@ -82,28 +111,18 @@ const WorkspacesPage: FC = () => { }, }); - 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 ( <> @@ -115,7 +134,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} @@ -125,12 +155,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, @@ -145,31 +194,34 @@ const WorkspacesPage: FC = () => { /> setActiveBatchAction(undefined)} onConfirm={async () => { - await batchActions.deleteAll(checkedWorkspaces); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + await batchActions.delete(checkedWorkspaces); + setActiveBatchAction(undefined); }} /> - { - await batchActions.updateAll({ - workspaces: checkedWorkspaces, - isDynamicParametersEnabled: false, - }); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + setActiveBatchAction(undefined)} + onSubmit={async () => { + window.alert("Hooray!"); + /** + * @todo Make sure this gets added back in once more of the + * component has been fleshed out + */ + if (false) { + await batchActions.updateTemplateVersions({ + workspaces: checkedWorkspaces, + isDynamicParametersEnabled: false, + }); + } + setActiveBatchAction(undefined); }} /> @@ -179,17 +231,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..1921b8f501a2c 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, + onBatchUpdateTransition, + onBatchStopTransition, + onBatchStartTransition, 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 ( @@ -155,7 +155,7 @@ export const WorkspacesPageView: FC = ({ !mustUpdateWorkspace(w, canChangeVersions), ) } - onClick={onStartAll} + onClick={onBatchStartTransition} > Start @@ -165,12 +165,12 @@ export const WorkspacesPageView: FC = ({ (w) => w.latest_build.status === "running", ) } - onClick={onStopAll} + onClick={onBatchStopTransition} > Stop - + = ({ Delete… @@ -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.tsx b/site/src/pages/WorkspacesPage/batchActions.tsx index 806c7a03afddb..3e342b38ee132 100644 --- a/site/src/pages/WorkspacesPage/batchActions.tsx +++ b/site/src/pages/WorkspacesPage/batchActions.tsx @@ -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 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 ||