From d126314ef351adc50a5d1c04da6469186b4be387 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 9 Oct 2023 21:29:09 +0000 Subject: [PATCH 01/21] refactor: clean up CreateWorkspacePage --- .../CreateWorkspacePage.tsx | 81 ++++++++----------- 1 file changed, 32 insertions(+), 49 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index a0ad1e3502deb..55be99644b830 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -1,11 +1,8 @@ import { useMachine } from "@xstate/react"; -import { - TemplateVersionParameter, - WorkspaceBuildParameter, -} from "api/typesGenerated"; +import { WorkspaceBuildParameter } from "api/typesGenerated"; import { useMe } from "hooks/useMe"; import { useOrganizationId } from "hooks/useOrganizationId"; -import { type FC, useCallback, useState, useEffect } from "react"; +import { type FC, useState, useEffect } from "react"; import { Helmet } from "react-helmet-async"; import { useNavigate, useParams, useSearchParams } from "react-router-dom"; import { pageTitle } from "utils/page"; @@ -30,10 +27,11 @@ export type ExternalAuthPollingState = "idle" | "polling" | "abandoned"; const CreateWorkspacePage: FC = () => { const organizationId = useOrganizationId(); + const [searchParams] = useSearchParams(); const { template: templateName } = useParams() as { template: string }; const me = useMe(); const navigate = useNavigate(); - const [searchParams] = useSearchParams(); + const defaultBuildParameters = getDefaultBuildParameters(searchParams); const mode = (searchParams.get("mode") ?? "form") as CreateWorkspaceMode; const [createWorkspaceState, send] = useMachine(createWorkspaceMachine, { @@ -52,19 +50,13 @@ const CreateWorkspacePage: FC = () => { }, }, }); + const { template, parameters, permissions, defaultName, versionId } = createWorkspaceState.context; - const title = createWorkspaceState.matches("autoCreating") - ? "Creating workspace..." - : "Create workspace"; const [externalAuthPollingState, setExternalAuthPollingState] = useState("idle"); - const startPollingExternalAuth = useCallback(() => { - setExternalAuthPollingState("polling"); - }, []); - const { data: externalAuth, error } = useQuery( versionId ? { @@ -75,40 +67,47 @@ const CreateWorkspacePage: FC = () => { : { enabled: false }, ); - const allSignedIn = externalAuth?.every((it) => it.authenticated); - useEffect(() => { - if (allSignedIn) { - setExternalAuthPollingState("idle"); - return; - } - if (externalAuthPollingState !== "polling") { return; } - // Poll for a maximum of one minute - const quitPolling = setTimeout( + const pollingTimeoutId = window.setTimeout( () => setExternalAuthPollingState("abandoned"), 60_000, ); - return () => { - clearTimeout(quitPolling); - }; - }, [externalAuthPollingState, allSignedIn]); + + return () => window.clearTimeout(pollingTimeoutId); + }, [externalAuthPollingState]); + + if (externalAuthPollingState !== "idle") { + const allSignedIn = externalAuth?.every((it) => it.authenticated) ?? false; + if (allSignedIn) { + setExternalAuthPollingState("idle"); + } + } return ( <> - {pageTitle(title)} + + {pageTitle( + createWorkspaceState.matches("autoCreating") + ? "Creating workspace..." + : "Create workspace", + )} + + {Boolean( createWorkspaceState.matches("loadingFormData") || createWorkspaceState.matches("autoCreating"), ) && } + {createWorkspaceState.matches("loadError") && ( )} + {createWorkspaceState.matches("idle") && ( { versionId={versionId} externalAuth={externalAuth ?? []} externalAuthPollingState={externalAuthPollingState} - startPollingExternalAuth={startPollingExternalAuth} permissions={permissions as CreateWSPermissions} parameters={parameters!} creatingWorkspace={createWorkspaceState.matches("creatingWorkspace")} - onCancel={() => { - navigate(-1); + startPollingExternalAuth={() => { + setExternalAuthPollingState("polling"); }} + onCancel={() => navigate(-1)} onSubmit={(request, owner) => { send({ type: "CREATE_WORKSPACE", @@ -144,29 +143,13 @@ export default CreateWorkspacePage; const getDefaultBuildParameters = ( urlSearchParams: URLSearchParams, ): WorkspaceBuildParameter[] => { - const buildValues: WorkspaceBuildParameter[] = []; - Array.from(urlSearchParams.keys()) + return Array.from(urlSearchParams.keys()) .filter((key) => key.startsWith("param.")) - .forEach((key) => { + .map((key) => { const name = key.replace("param.", ""); const value = urlSearchParams.get(key) ?? ""; - buildValues.push({ name, value }); + return { name, value }; }); - return buildValues; -}; - -export const orderedTemplateParameters = ( - templateParameters?: TemplateVersionParameter[], -): TemplateVersionParameter[] => { - if (!templateParameters) { - return []; - } - - const immutables = templateParameters.filter( - (parameter) => !parameter.mutable, - ); - const mutables = templateParameters.filter((parameter) => parameter.mutable); - return [...immutables, ...mutables]; }; const generateUniqueName = () => { From 881d4a3e8940aa0778b8ebbc700e06af5e84e7dc Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 9 Oct 2023 22:23:18 +0000 Subject: [PATCH 02/21] fix: remove unnecessary state --- .../CreateWorkspacePageView.tsx | 50 ++++++------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 2883f28bf53ae..c7a0271dea2f8 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -71,11 +71,10 @@ export const CreateWorkspacePageView: FC = ({ }) => { const styles = useStyles(); const [owner, setOwner] = useState(defaultOwner); - const { verifyExternalAuth, externalAuthErrors } = - useExternalAuthVerification(externalAuth); const [searchParams] = useSearchParams(); const disabledParamsList = searchParams?.get("disable_params")?.split(","); + const { authErrors, errorCount } = getAuthErrors(externalAuth); const form: FormikContextType = useFormik({ initialValues: { @@ -92,7 +91,7 @@ export const CreateWorkspacePageView: FC = ({ }), enableReinitialize: true, onSubmit: (request) => { - if (!verifyExternalAuth()) { + if (errorCount > 0) { form.setSubmitting(false); return; } @@ -180,7 +179,7 @@ export const CreateWorkspacePageView: FC = ({ startPollingExternalAuth={startPollingExternalAuth} displayName={auth.display_name} displayIcon={auth.display_icon} - error={externalAuthErrors[auth.id]} + error={authErrors[auth.id]} /> ))} @@ -245,40 +244,21 @@ export const CreateWorkspacePageView: FC = ({ type ExternalAuthErrors = Record; -const useExternalAuthVerification = ( - externalAuth: TypesGen.TemplateVersionExternalAuth[], -) => { - const [externalAuthErrors, setExternalAuthErrors] = - useState({}); +function getAuthErrors( + externalAuth: readonly TypesGen.TemplateVersionExternalAuth[], +) { + const authErrors: ExternalAuthErrors = {}; + let errorCount = 0; - // Clear errors when externalAuth is refreshed - useEffect(() => { - setExternalAuthErrors({}); - }, [externalAuth]); - - const verifyExternalAuth = () => { - const errors: ExternalAuthErrors = {}; - - for (let i = 0; i < externalAuth.length; i++) { - const auth = externalAuth.at(i); - if (!auth) { - continue; - } - if (!auth.authenticated) { - errors[auth.id] = "You must authenticate to create a workspace!"; - } + for (const auth of externalAuth) { + if (!auth.authenticated) { + authErrors[auth.id] = "You must authenticate to create a workspace!"; + errorCount++; } + } - setExternalAuthErrors(errors); - const isValid = Object.keys(errors).length === 0; - return isValid; - }; - - return { - externalAuthErrors, - verifyExternalAuth, - }; -}; + return { authErrors, errorCount } as const; +} const useStyles = makeStyles((theme) => ({ hasDescription: { From f793fe4164d62980ceae6ff6a61a7c1d16a9e7a5 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 10 Oct 2023 12:21:10 +0000 Subject: [PATCH 03/21] refactor: clean up auth logic more --- .../CreateWorkspacePageView.tsx | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index c7a0271dea2f8..9656658a092bb 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -74,7 +74,7 @@ export const CreateWorkspacePageView: FC = ({ const [searchParams] = useSearchParams(); const disabledParamsList = searchParams?.get("disable_params")?.split(","); - const { authErrors, errorCount } = getAuthErrors(externalAuth); + const authErrors = getAuthErrors(externalAuth); const form: FormikContextType = useFormik({ initialValues: { @@ -91,6 +91,7 @@ export const CreateWorkspacePageView: FC = ({ }), enableReinitialize: true, onSubmit: (request) => { + const errorCount = Object.keys(authErrors).length; if (errorCount > 0) { form.setSubmitting(false); return; @@ -242,22 +243,19 @@ export const CreateWorkspacePageView: FC = ({ ); }; -type ExternalAuthErrors = Record; - +type VerifiableAuth = Readonly<{ authenticated: boolean; id: string }>; function getAuthErrors( - externalAuth: readonly TypesGen.TemplateVersionExternalAuth[], -) { - const authErrors: ExternalAuthErrors = {}; - let errorCount = 0; + authList: readonly VerifiableAuth[], +): Readonly> { + const authErrors: Record = {}; - for (const auth of externalAuth) { + for (const auth of authList) { if (!auth.authenticated) { authErrors[auth.id] = "You must authenticate to create a workspace!"; - errorCount++; } } - return { authErrors, errorCount } as const; + return authErrors; } const useStyles = makeStyles((theme) => ({ From 24169583da7de3f0d7d22b22b3dbcaf2d2a98ec0 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 10 Oct 2023 13:21:03 +0000 Subject: [PATCH 04/21] refactor: remove enums from workspace actions --- .../WorkspaceActions/WorkspaceActions.tsx | 62 +++++++---------- .../WorkspaceActions/constants.ts | 66 +++++++++---------- 2 files changed, 54 insertions(+), 74 deletions(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index 04d2744530b90..f399fbdb280a8 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -14,14 +14,10 @@ import { UpdateButton, ActivateButton, } from "./Buttons"; -import { - ButtonMapping, - ButtonTypesEnum, - actionsByWorkspaceStatus, -} from "./constants"; -import SettingsOutlined from "@mui/icons-material/SettingsOutlined"; -import HistoryOutlined from "@mui/icons-material/HistoryOutlined"; -import DeleteOutlined from "@mui/icons-material/DeleteOutlined"; +import { ButtonMapping, actionsByWorkspaceStatus } from "./constants"; +import SettingsIcon from "@mui/icons-material/SettingsOutlined"; +import HistoryIcon from "@mui/icons-material/HistoryOutlined"; +import DeleteIcon from "@mui/icons-material/DeleteOutlined"; import IconButton from "@mui/material/IconButton"; export interface WorkspaceActionsProps { @@ -68,40 +64,30 @@ export const WorkspaceActions: FC = ({ // A mapping of button type to the corresponding React component const buttonMapping: ButtonMapping = { - [ButtonTypesEnum.update]: , - [ButtonTypesEnum.updating]: ( - - ), - [ButtonTypesEnum.start]: ( - - ), - [ButtonTypesEnum.starting]: ( + update: , + updating: , + start: , + starting: ( ), - [ButtonTypesEnum.stop]: , - [ButtonTypesEnum.stopping]: ( - - ), - [ButtonTypesEnum.restart]: ( + stop: , + stopping: , + restart: ( ), - [ButtonTypesEnum.restarting]: ( + restarting: ( ), - [ButtonTypesEnum.deleting]: , - [ButtonTypesEnum.canceling]: , - [ButtonTypesEnum.deleted]: , - [ButtonTypesEnum.pending]: , - [ButtonTypesEnum.activate]: ( - - ), - [ButtonTypesEnum.activating]: ( - - ), + deleting: , + canceling: , + deleted: , + pending: , + activate: , + activating: , }; // Returns a function that will execute the action and close the menu @@ -113,10 +99,8 @@ export const WorkspaceActions: FC = ({ return (
{canBeUpdated && - (isUpdating - ? buttonMapping[ButtonTypesEnum.updating] - : buttonMapping[ButtonTypesEnum.update])} - {isRestarting && buttonMapping[ButtonTypesEnum.restarting]} + (isUpdating ? buttonMapping.updating : buttonMapping.update)} + {isRestarting && buttonMapping.restarting} {!isRestarting && actionsByStatus.map((action) => ( {buttonMapping[action]} @@ -142,12 +126,12 @@ export const WorkspaceActions: FC = ({ onClose={() => setIsMenuOpen(false)} > - + Settings {canChangeVersions && ( - + Change version… )} @@ -155,7 +139,7 @@ export const WorkspaceActions: FC = ({ onClick={onMenuItemClick(handleDelete)} data-testid="delete-button" > - + Delete… diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts index d6f2704a18f80..da63d2234f913 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts +++ b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts @@ -1,31 +1,27 @@ -import { Workspace, WorkspaceStatus } from "api/typesGenerated"; -import { ReactNode } from "react"; +import { type Workspace, type WorkspaceStatus } from "api/typesGenerated"; +import { type ReactElement } from "react"; -// the button types we have -export enum ButtonTypesEnum { - start = "start", - starting = "starting", - stop = "stop", - stopping = "stopping", - restart = "restart", - restarting = "restarting", - deleting = "deleting", - update = "update", - updating = "updating", - activate = "activate", - activating = "activating", - // disabled buttons - canceling = "canceling", - deleted = "deleted", - pending = "pending", -} +/** + * Buttons supported by workspace actions. Canceling, Deleted, and Pending + * should all be associated with disabled states + */ +type ButtonType = + | Exclude + | "start" + | "stop" + | "restart" + | "restarting" + | "update" + | "updating" + | "activate" + | "activating"; export type ButtonMapping = { - [key in ButtonTypesEnum]: ReactNode; + [key in ButtonType]: ReactElement; }; interface WorkspaceAbilities { - actions: ButtonTypesEnum[]; + actions: readonly ButtonType[]; canCancel: boolean; canAcceptJobs: boolean; } @@ -36,7 +32,7 @@ export const actionsByWorkspaceStatus = ( ): WorkspaceAbilities => { if (workspace.dormant_at) { return { - actions: [ButtonTypesEnum.activate], + actions: ["activate"], canCancel: false, canAcceptJobs: false, }; @@ -44,35 +40,35 @@ export const actionsByWorkspaceStatus = ( return statusToActions[status]; }; -const statusToActions: Record = { +const statusToActions = { starting: { - actions: [ButtonTypesEnum.starting], + actions: ["starting"], canCancel: true, canAcceptJobs: false, }, running: { - actions: [ButtonTypesEnum.stop, ButtonTypesEnum.restart], + actions: ["stop", "restart"], canCancel: false, canAcceptJobs: true, }, stopping: { - actions: [ButtonTypesEnum.stopping], + actions: ["stopping"], canCancel: true, canAcceptJobs: false, }, stopped: { - actions: [ButtonTypesEnum.start], + actions: ["start"], canCancel: false, canAcceptJobs: true, }, canceled: { - actions: [ButtonTypesEnum.start, ButtonTypesEnum.stop], + actions: ["start", "stop"], canCancel: false, canAcceptJobs: true, }, // in the case of an error failed: { - actions: [ButtonTypesEnum.start, ButtonTypesEnum.stop], + actions: ["start", "stop"], canCancel: false, canAcceptJobs: true, }, @@ -80,23 +76,23 @@ const statusToActions: Record = { * disabled states */ canceling: { - actions: [ButtonTypesEnum.canceling], + actions: ["canceling"], canCancel: false, canAcceptJobs: false, }, deleting: { - actions: [ButtonTypesEnum.deleting], + actions: ["deleting"], canCancel: true, canAcceptJobs: false, }, deleted: { - actions: [ButtonTypesEnum.deleted], + actions: ["deleted"], canCancel: false, canAcceptJobs: false, }, pending: { - actions: [ButtonTypesEnum.pending], + actions: ["pending"], canCancel: false, canAcceptJobs: false, }, -}; +} as const satisfies Record; From e97cdd84d5e63c6a4a95a8793766a98216a1d443 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 10 Oct 2023 13:23:49 +0000 Subject: [PATCH 05/21] fix: export ButtonType type --- site/src/pages/WorkspacePage/WorkspaceActions/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts index da63d2234f913..19274346d539a 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts +++ b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts @@ -5,7 +5,7 @@ import { type ReactElement } from "react"; * Buttons supported by workspace actions. Canceling, Deleted, and Pending * should all be associated with disabled states */ -type ButtonType = +export type ButtonType = | Exclude | "start" | "stop" From 0af0352598a7274339195004b73d9c20c7389389 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 10 Oct 2023 13:44:36 +0000 Subject: [PATCH 06/21] refactor: remove workspace error enums --- .../pages/WorkspacePage/Workspace.stories.tsx | 8 +-- site/src/pages/WorkspacePage/Workspace.tsx | 34 ++++------- .../WorkspaceActions/WorkspaceActions.tsx | 57 ++++++++++++------- .../WorkspacePage/WorkspaceReadyPage.tsx | 8 +-- 4 files changed, 58 insertions(+), 49 deletions(-) diff --git a/site/src/pages/WorkspacePage/Workspace.stories.tsx b/site/src/pages/WorkspacePage/Workspace.stories.tsx index d77bff78c5875..7ef67ac0911eb 100644 --- a/site/src/pages/WorkspacePage/Workspace.stories.tsx +++ b/site/src/pages/WorkspacePage/Workspace.stories.tsx @@ -3,7 +3,7 @@ import { Meta, StoryObj } from "@storybook/react"; import { WatchAgentMetadataContext } from "components/Resources/AgentMetadata"; import { ProvisionerJobLog } from "api/typesGenerated"; import * as Mocks from "testHelpers/entities"; -import { Workspace, WorkspaceErrors } from "./Workspace"; +import { Workspace } from "./Workspace"; import { withReactContext } from "storybook-react-context"; import EventSource from "eventsourcemock"; import { ProxyContext, getPreferredProxy } from "contexts/ProxyContext"; @@ -133,7 +133,7 @@ export const Failed: Story = { ...Running.args, workspace: Mocks.MockFailedWorkspace, workspaceErrors: { - [WorkspaceErrors.BUILD_ERROR]: Mocks.mockApiError({ + buildError: Mocks.mockApiError({ message: "A workspace build is already active.", }), }, @@ -216,7 +216,7 @@ export const GetBuildsError: Story = { args: { ...Running.args, workspaceErrors: { - [WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.mockApiError({ + getBuildsError: Mocks.mockApiError({ message: "There is a problem fetching builds.", }), }, @@ -227,7 +227,7 @@ export const CancellationError: Story = { args: { ...Failed.args, workspaceErrors: { - [WorkspaceErrors.CANCELLATION_ERROR]: Mocks.mockApiError({ + cancellationError: Mocks.mockApiError({ message: "Job could not be canceled.", }), }, diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index ebfab404172d4..4bdb96b8dc557 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -30,11 +30,11 @@ import { useLocalStorage } from "hooks"; import AlertTitle from "@mui/material/AlertTitle"; import dayjs from "dayjs"; -export enum WorkspaceErrors { - GET_BUILDS_ERROR = "getBuildsError", - BUILD_ERROR = "buildError", - CANCELLATION_ERROR = "cancellationError", -} +export type WorkspaceError = + | "getBuildsError" + | "buildError" + | "cancellationError"; + export interface WorkspaceProps { scheduleProps: { onDeadlinePlus: (hours: number) => void; @@ -62,7 +62,7 @@ export interface WorkspaceProps { canChangeVersions: boolean; hideSSHButton?: boolean; hideVSCodeDesktopButton?: boolean; - workspaceErrors: Partial>; + workspaceErrors: Partial>; buildInfo?: TypesGen.BuildInfoResponse; sshPrefix?: string; template?: TypesGen.Template; @@ -117,20 +117,12 @@ export const Workspace: FC> = ({ const serverVersion = buildInfo?.version || ""; const { saveLocal, getLocal } = useLocalStorage(); - const buildError = Boolean(workspaceErrors[WorkspaceErrors.BUILD_ERROR]) && ( - + const buildError = Boolean(workspaceErrors.buildError) && ( + ); - const cancellationError = Boolean( - workspaceErrors[WorkspaceErrors.CANCELLATION_ERROR], - ) && ( - + const cancellationError = Boolean(workspaceErrors.cancellationError) && ( + ); let transitionStats: TypesGen.TransitionStats | undefined = undefined; @@ -346,10 +338,8 @@ export const Workspace: FC> = ({ /> )} - {workspaceErrors[WorkspaceErrors.GET_BUILDS_ERROR] ? ( - + {workspaceErrors.getBuildsError ? ( + ) : ( = ({ isRestarting, canChangeVersions, }) => { - const styles = useStyles(); - const { - canCancel, - canAcceptJobs, - actions: actionsByStatus, - } = actionsByWorkspaceStatus(workspace, workspace.latest_build.status); - const canBeUpdated = workspace.outdated && canAcceptJobs; - const menuTriggerRef = useRef(null); - const [isMenuOpen, setIsMenuOpen] = useState(false); - // A mapping of button type to the corresponding React component const buttonMapping: ButtonMapping = { update: , @@ -90,21 +87,35 @@ export const WorkspaceActions: FC = ({ activating: , }; + const styles = useStyles(); + const menuTriggerRef = useRef(null); + const [isMenuOpen, setIsMenuOpen] = useState(false); + // Returns a function that will execute the action and close the menu const onMenuItemClick = (actionFn: () => void) => () => { setIsMenuOpen(false); actionFn(); }; + const { + canCancel, + canAcceptJobs, + actions: actionsByStatus, + } = actionsByWorkspaceStatus(workspace, workspace.latest_build.status); + const canBeUpdated = workspace.outdated && canAcceptJobs; + return (
{canBeUpdated && (isUpdating ? buttonMapping.updating : buttonMapping.update)} + {isRestarting && buttonMapping.restarting} + {!isRestarting && actionsByStatus.map((action) => ( {buttonMapping[action]} ))} + {canCancel && }
= ({ ref={menuTriggerRef} onClick={() => setIsMenuOpen(true)} > - + + = ({ Settings + {canChangeVersions && ( Change version… )} + + + + Clone… + + Date: Tue, 10 Oct 2023 21:00:19 +0000 Subject: [PATCH 07/21] chore: get basic clone functionality done for UI --- .../CreateWorkspacePage.tsx | 23 +- .../CreateWorkspacePageView.tsx | 274 ++++++++++-------- site/src/pages/WorkspacePage/Workspace.tsx | 3 + .../WorkspaceActions/WorkspaceActions.tsx | 10 +- .../WorkspacePage/WorkspaceReadyPage.tsx | 106 ++++--- .../createWorkspaceXService.ts | 3 +- 6 files changed, 257 insertions(+), 162 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 55be99644b830..eaa4971b82fd5 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -10,6 +10,7 @@ import { CreateWSPermissions, CreateWorkspaceMode, createWorkspaceMachine, + createWorkspaceModes, } from "xServices/createWorkspace/createWorkspaceXService"; import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; import { Loader } from "components/Loader/Loader"; @@ -33,7 +34,7 @@ const CreateWorkspacePage: FC = () => { const navigate = useNavigate(); const defaultBuildParameters = getDefaultBuildParameters(searchParams); - const mode = (searchParams.get("mode") ?? "form") as CreateWorkspaceMode; + const mode = getWorkspaceMode(searchParams); const [createWorkspaceState, send] = useMachine(createWorkspaceMachine, { context: { organizationId, @@ -87,6 +88,11 @@ const CreateWorkspacePage: FC = () => { } } + const isFormLoading = Boolean( + createWorkspaceState.matches("loadingFormData") || + createWorkspaceState.matches("autoCreating"), + ); + return ( <> @@ -99,10 +105,7 @@ const CreateWorkspacePage: FC = () => { - {Boolean( - createWorkspaceState.matches("loadingFormData") || - createWorkspaceState.matches("autoCreating"), - ) && } + {isFormLoading && } {createWorkspaceState.matches("loadError") && ( @@ -121,6 +124,7 @@ const CreateWorkspacePage: FC = () => { permissions={permissions as CreateWSPermissions} parameters={parameters!} creatingWorkspace={createWorkspaceState.matches("creatingWorkspace")} + mode={mode} startPollingExternalAuth={() => { setExternalAuthPollingState("polling"); }} @@ -152,6 +156,15 @@ const getDefaultBuildParameters = ( }); }; +function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { + const paramMode = params.get("mode"); + if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { + return paramMode as CreateWorkspaceMode; + } + + return "form"; +} + const generateUniqueName = () => { const numberDictionary = NumberDictionary.generate({ min: 0, max: 99 }); return uniqueNamesGenerator({ diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 9656658a092bb..f4230c3cfa886 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -2,7 +2,7 @@ import TextField from "@mui/material/TextField"; import * as TypesGen from "api/typesGenerated"; import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; import { FormikContextType, useFormik } from "formik"; -import { FC, useEffect, useState } from "react"; +import { type FC, useEffect, useState, useReducer } from "react"; import { getFormHelpers, nameValidator, @@ -26,12 +26,18 @@ import { ImmutableTemplateParametersSection, MutableTemplateParametersSection, } from "components/TemplateParameters/TemplateParameters"; -import { CreateWSPermissions } from "xServices/createWorkspace/createWorkspaceXService"; +import { + CreateWSPermissions, + CreateWorkspaceMode, +} from "xServices/createWorkspace/createWorkspaceXService"; import { ExternalAuth } from "./ExternalAuth"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Stack } from "components/Stack/Stack"; import { type ExternalAuthPollingState } from "./CreateWorkspacePage"; import { useSearchParams } from "react-router-dom"; +import { Alert } from "components/Alert/Alert"; +import { Margins } from "components/Margins/Margins"; +import { useTheme } from "@emotion/react"; export interface CreateWorkspacePageViewProps { error: unknown; @@ -47,6 +53,7 @@ export interface CreateWorkspacePageViewProps { permissions: CreateWSPermissions; creatingWorkspace: boolean; onCancel: () => void; + mode: CreateWorkspaceMode; onSubmit: ( req: TypesGen.CreateWorkspaceRequest, owner: TypesGen.User, @@ -68,6 +75,7 @@ export const CreateWorkspacePageView: FC = ({ creatingWorkspace, onSubmit, onCancel, + mode, }) => { const styles = useStyles(); const [owner, setOwner] = useState(defaultOwner); @@ -113,133 +121,143 @@ export const CreateWorkspacePageView: FC = ({ ); return ( - - - {Boolean(error) && } - {/* General info */} - - - - {versionId && versionId !== template.active_version_id && ( - - - - This parameter has been preset, and cannot be modified. - - - )} - - - + <> + {mode === "duplicate" && } - {permissions.createWorkspaceForUser && ( + + + {Boolean(error) && } + {/* General info */} - { - setOwner(user ?? defaultOwner); - }} - label="Owner" - size="medium" + + {versionId && versionId !== template.active_version_id && ( + + + + This parameter has been preset, and cannot be modified. + + + )} + - )} - {externalAuth && externalAuth.length > 0 && ( - - - {externalAuth.map((auth) => ( - + + { + setOwner(user ?? defaultOwner); + }} + label="Owner" + size="medium" /> - ))} - - - )} + + + )} - {parameters && ( - <> - { - return { - ...getFieldHelpers( - "rich_parameter_values[" + index + "].value", - ), - onChange: async (value) => { - await form.setFieldValue("rich_parameter_values." + index, { - name: parameter.name, - value: value, - }); - }, - disabled: - disabledParamsList?.includes( - parameter.name.toLowerCase().replace(/ /g, "_"), - ) || form.isSubmitting, - }; - }} - /> - { - return { - ...getFieldHelpers( - "rich_parameter_values[" + index + "].value", - ), - onChange: async (value) => { - await form.setFieldValue("rich_parameter_values." + index, { - name: parameter.name, - value: value, - }); - }, - disabled: - disabledParamsList?.includes( - parameter.name.toLowerCase().replace(/ /g, "_"), - ) || form.isSubmitting, - }; - }} - /> - - )} + {externalAuth && externalAuth.length > 0 && ( + + + {externalAuth.map((auth) => ( + + ))} + + + )} + + {parameters && ( + <> + { + return { + ...getFieldHelpers( + "rich_parameter_values[" + index + "].value", + ), + onChange: async (value) => { + await form.setFieldValue( + "rich_parameter_values." + index, + { + name: parameter.name, + value: value, + }, + ); + }, + disabled: + disabledParamsList?.includes( + parameter.name.toLowerCase().replace(/ /g, "_"), + ) || form.isSubmitting, + }; + }} + /> + { + return { + ...getFieldHelpers( + "rich_parameter_values[" + index + "].value", + ), + onChange: async (value) => { + await form.setFieldValue( + "rich_parameter_values." + index, + { + name: parameter.name, + value: value, + }, + ); + }, + disabled: + disabledParamsList?.includes( + parameter.name.toLowerCase().replace(/ /g, "_"), + ) || form.isSubmitting, + }; + }} + /> + + )} - - - + + + + ); }; @@ -258,6 +276,26 @@ function getAuthErrors( return authErrors; } +function DuplicateWarningMessage() { + const [isDismissed, dismiss] = useReducer(() => true, false); + const theme = useTheme(); + + if (isDismissed) { + return null; + } + + return ( +
+ + + Duplicating a workspace only copies its parameters. No state from the + old workspace is copied over. + + +
+ ); +} + const useStyles = makeStyles((theme) => ({ hasDescription: { paddingBottom: theme.spacing(2), diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index 4bdb96b8dc557..0d6e3dc10d994 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -49,6 +49,7 @@ export interface WorkspaceProps { handleUpdate: () => void; handleCancel: () => void; handleSettings: () => void; + handleClone: () => void; handleChangeVersion: () => void; handleDormantActivate: () => void; isUpdating: boolean; @@ -87,6 +88,7 @@ export const Workspace: FC> = ({ handleUpdate, handleCancel, handleSettings, + handleClone, handleChangeVersion, handleDormantActivate: handleDormantActivate, workspace, @@ -201,6 +203,7 @@ export const Workspace: FC> = ({ handleUpdate={handleUpdate} handleCancel={handleCancel} handleSettings={handleSettings} + handleClone={handleClone} handleChangeVersion={handleChangeVersion} handleDormantActivate={handleDormantActivate} canChangeVersions={canChangeVersions} diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index 6cfaaba5adf04..2e1850fab656a 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -25,7 +25,7 @@ import SettingsIcon from "@mui/icons-material/SettingsOutlined"; import HistoryIcon from "@mui/icons-material/HistoryOutlined"; import DeleteIcon from "@mui/icons-material/DeleteOutlined"; import MoreOptionsIcon from "@mui/icons-material/MoreVertOutlined"; -import CloneIcon from "@mui/icons-material/ContentCopyOutlined"; +import DuplicateIcon from "@mui/icons-material/FileCopyOutlined"; export interface WorkspaceActionsProps { workspace: Workspace; @@ -35,6 +35,7 @@ export interface WorkspaceActionsProps { handleDelete: () => void; handleUpdate: () => void; handleCancel: () => void; + handleClone: () => void; handleSettings: () => void; handleChangeVersion: () => void; handleDormantActivate: () => void; @@ -53,6 +54,7 @@ export const WorkspaceActions: FC = ({ handleUpdate, handleCancel, handleSettings, + handleClone, handleChangeVersion, handleDormantActivate: handleDormantActivate, isUpdating, @@ -149,9 +151,9 @@ export const WorkspaceActions: FC = ({ )} - - - Clone… + + + Duplicate… ; @@ -56,11 +57,6 @@ export const WorkspaceReadyPage = ({ isLoadingMoreBuilds, hasMoreBuilds, }: WorkspaceReadyPageProps): JSX.Element => { - const [_, bannerSend] = useActor( - workspaceState.children["scheduleBannerMachine"], - ); - const { buildInfo } = useDashboard(); - const featureVisibility = useFeatureVisibility(); const { workspace, template, @@ -72,17 +68,22 @@ export const WorkspaceReadyPage = ({ permissions, missedParameters, } = workspaceState.context; + + // Breaks the rules of hooks, but if we're throwing an error, the rules won't + // even have a chance to matter. Best to get the check out of the way early + // for better type narrowing if (workspace === undefined) { throw Error("Workspace is undefined"); } - const deadline = getDeadline(workspace); - const canUpdateWorkspace = Boolean(permissions?.updateWorkspace); - const canUpdateTemplate = Boolean(permissions?.updateTemplate); - const canRetryDebugMode = - Boolean(permissions?.viewDeploymentValues) && - Boolean(deploymentValues?.enable_terraform_debug_mode); - const favicon = getFaviconByStatus(workspace.latest_build); + + const { buildInfo } = useDashboard(); + const featureVisibility = useFeatureVisibility(); const navigate = useNavigate(); + + const [_, bannerSend] = useActor( + workspaceState.children["scheduleBannerMachine"], + ); + const [changeVersionDialogOpen, setChangeVersionDialogOpen] = useState(false); const [isConfirmingUpdate, setIsConfirmingUpdate] = useState(false); const [confirmingRestart, setConfirmingRestart] = useState<{ @@ -90,29 +91,19 @@ export const WorkspaceReadyPage = ({ buildParameters?: TypesGen.WorkspaceBuildParameter[]; }>({ open: false }); - const { data: allVersions } = useQuery({ + const { data: allTemplateVersions } = useQuery({ ...templateVersions(workspace.template_id), enabled: changeVersionDialogOpen, }); - const { data: latestVersion } = useQuery({ + + const { data: latestTemplateVersion } = useQuery({ ...templateVersion(workspace.template_active_version_id), enabled: workspace.outdated, }); - const [faviconTheme, setFaviconTheme] = useState<"light" | "dark">("dark"); - useEffect(() => { - if (typeof window === "undefined" || !window.matchMedia) { - return; - } - const isDark = window.matchMedia("(prefers-color-scheme: dark)"); - // We want the favicon the opposite of the theme. - setFaviconTheme(isDark ? "light" : "dark"); - }, []); + const buildLogs = useWorkspaceBuildLogs(workspace.latest_build.id); - const shouldDisplayBuildLogs = - hasJobError(workspace) || - ["canceling", "deleting", "pending", "starting", "stopping"].includes( - workspace.latest_build.status, - ); + const faviconTheme = useFaviconTheme(); + const { mutate: mutateRestartWorkspace, error: restartBuildError, @@ -120,11 +111,42 @@ export const WorkspaceReadyPage = ({ } = useMutation({ mutationFn: restartWorkspace, }); + // keep banner machine in sync with workspace useEffect(() => { bannerSend({ type: "REFRESH_WORKSPACE", workspace }); }, [bannerSend, workspace]); + const handleWorkspaceCloning = () => { + if (template?.name === undefined) { + return; + } + + const workspaceCreationParams = new URLSearchParams({ + mode: "duplicate" satisfies CreateWorkspaceMode, + name: workspace.name, + }); + + navigate({ + pathname: `/templates/${template.name}/workspace`, + search: workspaceCreationParams.toString(), + }); + }; + + const favicon = getFaviconByStatus(workspace.latest_build); + const deadline = getDeadline(workspace); + const canUpdateWorkspace = Boolean(permissions?.updateWorkspace); + const canUpdateTemplate = Boolean(permissions?.updateTemplate); + const canRetryDebugMode = + Boolean(permissions?.viewDeploymentValues) && + Boolean(deploymentValues?.enable_terraform_debug_mode); + + const shouldDisplayBuildLogs = + hasJobError(workspace) || + ["canceling", "deleting", "pending", "starting", "stopping"].includes( + workspace.latest_build.status, + ); + return ( <> @@ -177,6 +199,7 @@ export const WorkspaceReadyPage = ({ }} handleCancel={() => workspaceSend({ type: "CANCEL" })} handleSettings={() => navigate("settings")} + handleClone={handleWorkspaceCloning} handleBuildRetry={() => workspaceSend({ type: "RETRY_BUILD" })} handleChangeVersion={() => { setChangeVersionDialogOpen(true); @@ -188,7 +211,7 @@ export const WorkspaceReadyPage = ({ isLoadingMoreBuilds={isLoadingMoreBuilds} hasMoreBuilds={hasMoreBuilds} canUpdateWorkspace={canUpdateWorkspace} - updateMessage={latestVersion?.message} + updateMessage={latestTemplateVersion?.message} canRetryDebugMode={canRetryDebugMode} canChangeVersions={canUpdateTemplate} hideSSHButton={featureVisibility["browser_only"]} @@ -234,9 +257,9 @@ export const WorkspaceReadyPage = ({ }} /> workspace.latest_build.template_version_id === v.id, )} open={changeVersionDialogOpen} @@ -266,8 +289,8 @@ export const WorkspaceReadyPage = ({ Restarting your workspace will stop all running processes and{" "} delete non-persistent data.

- {latestVersion?.message && ( - {latestVersion.message} + {latestTemplateVersion?.message && ( + {latestTemplateVersion.message} )} } @@ -296,6 +319,21 @@ export const WorkspaceReadyPage = ({ ); }; +function useFaviconTheme() { + const [faviconTheme, setFaviconTheme] = useState<"light" | "dark">("dark"); + + useEffect(() => { + if (typeof window === "undefined" || !window.matchMedia) { + return; + } + const isDark = window.matchMedia("(prefers-color-scheme: dark)"); + // We want the favicon the opposite of the theme. + setFaviconTheme(isDark ? "light" : "dark"); + }, []); + + return faviconTheme; +} + const WarningDialog: FC< Pick< ConfirmDialogProps, diff --git a/site/src/xServices/createWorkspace/createWorkspaceXService.ts b/site/src/xServices/createWorkspace/createWorkspaceXService.ts index 79dc44701430f..769399b63c6e6 100644 --- a/site/src/xServices/createWorkspace/createWorkspaceXService.ts +++ b/site/src/xServices/createWorkspace/createWorkspaceXService.ts @@ -15,7 +15,8 @@ import { import { assign, createMachine } from "xstate"; import { paramsUsedToCreateWorkspace } from "utils/workspace"; -export type CreateWorkspaceMode = "form" | "auto"; +export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; +export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; type CreateWorkspaceContext = { organizationId: string; From f0e120bdfecb486b6015caf647c49277bf07bf31 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Wed, 11 Oct 2023 12:33:32 +0000 Subject: [PATCH 08/21] fix: clean up Create page view again --- .../pages/CreateWorkspacePage/CreateWorkspacePageView.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index f4230c3cfa886..e06aa4dd41dd3 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -261,9 +261,8 @@ export const CreateWorkspacePageView: FC = ({ ); }; -type VerifiableAuth = Readonly<{ authenticated: boolean; id: string }>; function getAuthErrors( - authList: readonly VerifiableAuth[], + authList: readonly TypesGen.TemplateVersionExternalAuth[], ): Readonly> { const authErrors: Record = {}; @@ -284,6 +283,11 @@ function DuplicateWarningMessage() { return null; } + // Set up looks a little hokey (having an Alert already fully configured to + // listen to dismissals, on top of more dismissal state), but relying solely + // on the Alert API wouldn't get rid of the div and horizontal margin helper + // after the dismiss happens. Not using CSS margins because those can be a + // style maintenance nightmare over time return (
From 53a9cae76363bcf1606c6a79e9b6b38e9f639d19 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Wed, 11 Oct 2023 22:43:37 +0000 Subject: [PATCH 09/21] refactor/fix: reconcile merge changes from new PR --- site/src/api/queries/workspaces.ts | 2 +- .../CreateWorkspacePage.tsx | 238 ++++++++++-------- .../CreateWorkspacePageView.tsx | 8 +- .../CreateWorkspacePage/ExternalAuth.tsx | 4 +- .../WorkspacePage/WorkspaceReadyPage.tsx | 2 +- 5 files changed, 139 insertions(+), 115 deletions(-) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index bd5b76f951838..ff233f33c3d1e 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -26,7 +26,7 @@ export const workspaceByOwnerAndName = ( }; }; -type AutoCreateWorkspaceOptions = { +export type AutoCreateWorkspaceOptions = { templateName: string; versionId?: string; organizationId: string; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 666a513d3d6c3..8ae9c5e57b0e4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -23,102 +23,87 @@ import { templateByName, templateVersionExternalAuth, } from "api/queries/templates"; -import { autoCreateWorkspace, createWorkspace } from "api/queries/workspaces"; +import { + AutoCreateWorkspaceOptions, + autoCreateWorkspace, + createWorkspace, +} from "api/queries/workspaces"; import { checkAuthorization } from "api/queries/authCheck"; import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; import { richParameters } from "api/queries/templateVersions"; -import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { useEffectEvent } from "hooks/hookPolyfills"; -type CreateWorkspaceMode = "form" | "auto"; +export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; +export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; -export type ExternalAuthPollingState = "idle" | "polling" | "abandoned"; +export type ExternalAuthPollingStatus = "idle" | "polling" | "abandoned"; const CreateWorkspacePage: FC = () => { const organizationId = useOrganizationId(); + const [searchParams] = useSearchParams(); const { template: templateName } = useParams() as { template: string }; - const me = useMe(); const navigate = useNavigate(); - const [searchParams, setSearchParams] = useSearchParams(); - const defaultBuildParameters = getDefaultBuildParameters(searchParams); - const mode = (searchParams.get("mode") ?? "form") as CreateWorkspaceMode; - const customVersionId = searchParams.get("version") ?? undefined; - const defaultName = - mode === "auto" ? generateUniqueName() : searchParams.get("name") ?? ""; + const me = useMe(); const queryClient = useQueryClient(); - const autoCreateWorkspaceMutation = useMutation( - autoCreateWorkspace(queryClient), - ); const createWorkspaceMutation = useMutation(createWorkspace(queryClient)); const templateQuery = useQuery(templateByName(organizationId, templateName)); const permissionsQuery = useQuery( - checkAuthorization({ - checks: createWorkspaceChecks(organizationId), - }), + checkAuthorization({ checks: createWorkspaceChecks(organizationId) }), ); - const realizedVersionId = - customVersionId ?? templateQuery.data?.active_version_id; + + const versionId = + searchParams.get("version") ?? templateQuery.data?.active_version_id; + + const { authList, pollingStatus, startPolling } = useExternalAuth(versionId); const richParametersQuery = useQuery({ - ...richParameters(realizedVersionId ?? ""), - enabled: realizedVersionId !== undefined, + ...richParameters(versionId ?? ""), + enabled: versionId !== undefined, }); - const realizedParameters = richParametersQuery.data - ? richParametersQuery.data.filter(paramsUsedToCreateWorkspace) - : undefined; - const { externalAuth, externalAuthPollingState, startPollingExternalAuth } = - useExternalAuth(realizedVersionId); + const defaultBuildParameters = getDefaultBuildParameters(searchParams); + const mode = getWorkspaceMode(searchParams); + const defaultName = + mode === "auto" ? generateUniqueName() : searchParams.get("name") ?? ""; + + const onCreateWorkspace = (workspace: Workspace) => { + navigate(`/@${workspace.owner_name}/${workspace.name}`); + }; + + const isAutoCreating = useAutomatedWorkspaceCreation({ + auto: mode === "auto", + onSuccess: onCreateWorkspace, + payload: { + templateName, + organizationId, + defaultBuildParameters, + defaultName, + versionId, + }, + }); const isLoadingFormData = templateQuery.isLoading || permissionsQuery.isLoading || richParametersQuery.isLoading; + const loadFormDataError = templateQuery.error ?? permissionsQuery.error ?? richParametersQuery.error; - const title = autoCreateWorkspaceMutation.isLoading - ? "Creating workspace..." - : "Create workspace"; - - const onCreateWorkspace = useCallback( - (workspace: Workspace) => { - navigate(`/@${workspace.owner_name}/${workspace.name}`); - }, - [navigate], - ); - - const automateWorkspaceCreation = useEffectEvent(async () => { - try { - const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({ - templateName, - organizationId, - defaultBuildParameters, - defaultName, - versionId: realizedVersionId, - }); - - onCreateWorkspace(newWorkspace); - } catch (err) { - searchParams.delete("mode"); - setSearchParams(searchParams); - } - }); - - useEffect(() => { - if (mode === "auto") { - void automateWorkspaceCreation(); - } - }, [automateWorkspaceCreation, mode]); - return ( <> - {pageTitle(title)} + + {pageTitle( + isAutoCreating ? "Creating workspace..." : "Create workspace", + )} + + {loadFormDataError && } - {isLoadingFormData || autoCreateWorkspaceMutation.isLoading ? ( + + {isLoadingFormData || isAutoCreating ? ( ) : ( { defaultBuildParameters={defaultBuildParameters} error={createWorkspaceMutation.error} template={templateQuery.data!} - versionId={realizedVersionId} - externalAuth={externalAuth ?? []} - externalAuthPollingState={externalAuthPollingState} - startPollingExternalAuth={startPollingExternalAuth} + versionId={versionId} + externalAuth={authList ?? []} + externalAuthPollingStatus={pollingStatus} + startPollingExternalAuth={startPolling} permissions={permissionsQuery.data as CreateWSPermissions} - parameters={realizedParameters as TemplateVersionParameter[]} creatingWorkspace={createWorkspaceMutation.isLoading} - onCancel={() => { - navigate(-1); - }} + onCancel={() => navigate(-1)} + parameters={richParametersQuery.data!.filter( + (param) => !param.ephemeral, + )} onSubmit={async (request, owner) => { - if (realizedVersionId) { + if (versionId) { request = { ...request, template_id: undefined, - template_version_id: realizedVersionId, + template_version_id: versionId, }; } @@ -161,64 +146,103 @@ const CreateWorkspacePage: FC = () => { }; const useExternalAuth = (versionId: string | undefined) => { - const [externalAuthPollingState, setExternalAuthPollingState] = - useState("idle"); + const [pollingStatus, setPollingStatus] = + useState("idle"); - const startPollingExternalAuth = useCallback(() => { - setExternalAuthPollingState("polling"); + const startPolling = useCallback(() => { + setPollingStatus("polling"); }, []); - const { data: externalAuth } = useQuery( + const { data: authList } = useQuery( versionId ? { ...templateVersionExternalAuth(versionId), - refetchInterval: - externalAuthPollingState === "polling" ? 1000 : false, + refetchInterval: pollingStatus === "polling" ? 1000 : false, } : { enabled: false }, ); - const allSignedIn = externalAuth?.every((it) => it.authenticated); - useEffect(() => { - if (allSignedIn) { - setExternalAuthPollingState("idle"); - return; - } - - if (externalAuthPollingState !== "polling") { + if (pollingStatus !== "polling") { return; } - // Poll for a maximum of one minute - const quitPolling = setTimeout( - () => setExternalAuthPollingState("abandoned"), + const timeoutId = window.setTimeout( + () => setPollingStatus("abandoned"), 60_000, ); - return () => { - clearTimeout(quitPolling); - }; - }, [externalAuthPollingState, allSignedIn]); - - return { - startPollingExternalAuth, - externalAuth, - externalAuthPollingState, - }; + + return () => clearTimeout(timeoutId); + }, [pollingStatus]); + + const isAllSignedIn = authList?.every((it) => it.authenticated) ?? false; + + // Doing inline state sync to minimize extra re-renders that useEffect + // approach would involve + if (isAllSignedIn && pollingStatus === "polling") { + setPollingStatus("idle"); + } + + return { authList, isAllSignedIn, pollingStatus, startPolling } as const; }; +function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { + const paramMode = params.get("mode"); + if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { + return paramMode as CreateWorkspaceMode; + } + + return "form"; +} + +type AutomatedWorkspaceConfig = { + auto: boolean; + payload: AutoCreateWorkspaceOptions; + onSuccess: (newWorkspace: Workspace) => void; +}; + +function useAutomatedWorkspaceCreation(config: AutomatedWorkspaceConfig) { + // Duplicates some of the hook calls from the parent, but that was preferable + // to having the function arguments balloon in complexity + const [searchParams, setSearchParams] = useSearchParams(); + const queryClient = useQueryClient(); + const autoCreateWorkspaceMutation = useMutation( + autoCreateWorkspace(queryClient), + ); + + const automateWorkspaceCreation = useEffectEvent(async () => { + try { + const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync( + config.payload, + ); + + config.onSuccess(newWorkspace); + } catch (err) { + searchParams.delete("mode"); + setSearchParams(searchParams); + } + }); + + useEffect(() => { + if (config.auto) { + void automateWorkspaceCreation(); + } + }, [automateWorkspaceCreation, config.auto]); + + return autoCreateWorkspaceMutation.isLoading; +} + const getDefaultBuildParameters = ( urlSearchParams: URLSearchParams, ): WorkspaceBuildParameter[] => { - const buildValues: WorkspaceBuildParameter[] = []; - Array.from(urlSearchParams.keys()) + return [...urlSearchParams.keys()] .filter((key) => key.startsWith("param.")) - .forEach((key) => { - const name = key.replace("param.", ""); - const value = urlSearchParams.get(key) ?? ""; - buildValues.push({ name, value }); + .map((key) => { + return { + name: key.replace("param.", ""), + value: urlSearchParams.get(key) ?? "", + }; }); - return buildValues; }; export const orderedTemplateParameters = ( diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 3c79667a94e0b..b9fe003e7990c 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -35,7 +35,7 @@ import { useTheme } from "@emotion/react"; import { Alert } from "components/Alert/Alert"; import { Margins } from "components/Margins/Margins"; import { - type ExternalAuthPollingState, + type ExternalAuthPollingStatus, type CreateWorkspaceMode, } from "./CreateWorkspacePage"; @@ -46,7 +46,7 @@ export interface CreateWorkspacePageViewProps { template: TypesGen.Template; versionId?: string; externalAuth: TypesGen.TemplateVersionExternalAuth[]; - externalAuthPollingState: ExternalAuthPollingState; + externalAuthPollingStatus: ExternalAuthPollingStatus; startPollingExternalAuth: () => void; parameters: TypesGen.TemplateVersionParameter[]; defaultBuildParameters: TypesGen.WorkspaceBuildParameter[]; @@ -67,7 +67,7 @@ export const CreateWorkspacePageView: FC = ({ template, versionId, externalAuth, - externalAuthPollingState, + externalAuthPollingStatus, startPollingExternalAuth, parameters, defaultBuildParameters, @@ -187,7 +187,7 @@ export const CreateWorkspacePageView: FC = ({ key={auth.id} authenticateURL={auth.authenticate_url} authenticated={auth.authenticated} - externalAuthPollingState={externalAuthPollingState} + externalAuthPollingState={externalAuthPollingStatus} startPollingExternalAuth={startPollingExternalAuth} displayName={auth.display_name} displayIcon={auth.display_icon} diff --git a/site/src/pages/CreateWorkspacePage/ExternalAuth.tsx b/site/src/pages/CreateWorkspacePage/ExternalAuth.tsx index 41dde1b8420ab..e9d85d17311cb 100644 --- a/site/src/pages/CreateWorkspacePage/ExternalAuth.tsx +++ b/site/src/pages/CreateWorkspacePage/ExternalAuth.tsx @@ -5,14 +5,14 @@ import Tooltip from "@mui/material/Tooltip"; import { type FC } from "react"; import { LoadingButton } from "components/LoadingButton/LoadingButton"; import { Stack } from "components/Stack/Stack"; -import { type ExternalAuthPollingState } from "./CreateWorkspacePage"; +import { type ExternalAuthPollingStatus } from "./CreateWorkspacePage"; export interface ExternalAuthProps { displayName: string; displayIcon: string; authenticated: boolean; authenticateURL: string; - externalAuthPollingState: ExternalAuthPollingState; + externalAuthPollingState: ExternalAuthPollingStatus; startPollingExternalAuth: () => void; error?: string; } diff --git a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx index f5d3bddf5fffb..7cb465d0988dd 100644 --- a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx @@ -34,7 +34,7 @@ import { templateVersion, templateVersions } from "api/queries/templates"; import { Alert } from "components/Alert/Alert"; import { Stack } from "components/Stack/Stack"; import { useWorkspaceBuildLogs } from "hooks/useWorkspaceBuildLogs"; -import { type CreateWorkspaceMode } from "xServices/createWorkspace/createWorkspaceXService"; +import { type CreateWorkspaceMode } from "../CreateWorkspacePage/CreateWorkspacePage"; interface WorkspaceReadyPageProps { workspaceState: StateFrom; From d98a5e0e293bb7472a0da1a55b3ef09fc018230a Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 12 Oct 2023 22:14:01 +0000 Subject: [PATCH 10/21] refactor: centralize workspace duplication logic --- .../CreateWorkspacePage.tsx | 44 +++++++++++++++++-- site/src/pages/WorkspacePage/Workspace.tsx | 3 -- .../WorkspaceActions/WorkspaceActions.tsx | 8 ++-- .../WorkspacePage/WorkspaceReadyPage.tsx | 18 -------- 4 files changed, 45 insertions(+), 28 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 8ae9c5e57b0e4..3a922d4e066b0 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -64,8 +64,7 @@ const CreateWorkspacePage: FC = () => { const defaultBuildParameters = getDefaultBuildParameters(searchParams); const mode = getWorkspaceMode(searchParams); - const defaultName = - mode === "auto" ? generateUniqueName() : searchParams.get("name") ?? ""; + const defaultName = getDefaultName(mode, searchParams); const onCreateWorkspace = (workspace: Workspace) => { navigate(`/@${workspace.owner_name}/${workspace.name}`); @@ -177,8 +176,8 @@ const useExternalAuth = (versionId: string | undefined) => { const isAllSignedIn = authList?.every((it) => it.authenticated) ?? false; - // Doing inline state sync to minimize extra re-renders that useEffect - // approach would involve + // Doing state sync inline, because doing it inside a useEffect call would add + // unnecessary renders and re-painting. if (isAllSignedIn && pollingStatus === "polling") { setPollingStatus("idle"); } @@ -186,6 +185,30 @@ const useExternalAuth = (versionId: string | undefined) => { return { authList, isAllSignedIn, pollingStatus, startPolling } as const; }; +/** + * Returns a function that takes a workspace, and then navigates the user to the + * workspace creation page, with the form pre-filled with the provided + * workspace's parameters. + */ +export function useWorkspaceDuplication() { + const navigate = useNavigate(); + + return useCallback( + (workspace: Workspace) => { + const workspaceCreationParams = new URLSearchParams({ + mode: "duplicate" satisfies CreateWorkspaceMode, + name: workspace.name, + }); + + navigate({ + pathname: `/templates/${workspace.template_name}/workspace`, + search: workspaceCreationParams.toString(), + }); + }, + [navigate], + ); +} + function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { const paramMode = params.get("mode"); if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { @@ -195,6 +218,19 @@ function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { return "form"; } +function getDefaultName(mode: CreateWorkspaceMode, params: URLSearchParams) { + if (mode === "auto") { + return generateUniqueName(); + } + + const paramsName = params.get("name"); + if (mode === "duplicate" && paramsName) { + return `${paramsName}-copy`; + } + + return paramsName ?? ""; +} + type AutomatedWorkspaceConfig = { auto: boolean; payload: AutoCreateWorkspaceOptions; diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index 0d6e3dc10d994..4bdb96b8dc557 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -49,7 +49,6 @@ export interface WorkspaceProps { handleUpdate: () => void; handleCancel: () => void; handleSettings: () => void; - handleClone: () => void; handleChangeVersion: () => void; handleDormantActivate: () => void; isUpdating: boolean; @@ -88,7 +87,6 @@ export const Workspace: FC> = ({ handleUpdate, handleCancel, handleSettings, - handleClone, handleChangeVersion, handleDormantActivate: handleDormantActivate, workspace, @@ -203,7 +201,6 @@ export const Workspace: FC> = ({ handleUpdate={handleUpdate} handleCancel={handleCancel} handleSettings={handleSettings} - handleClone={handleClone} handleChangeVersion={handleChangeVersion} handleDormantActivate={handleDormantActivate} canChangeVersions={canChangeVersions} diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index 2e1850fab656a..3cb8e0d91cd5f 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -26,6 +26,7 @@ import HistoryIcon from "@mui/icons-material/HistoryOutlined"; import DeleteIcon from "@mui/icons-material/DeleteOutlined"; import MoreOptionsIcon from "@mui/icons-material/MoreVertOutlined"; import DuplicateIcon from "@mui/icons-material/FileCopyOutlined"; +import { useWorkspaceDuplication } from "pages/CreateWorkspacePage/CreateWorkspacePage"; export interface WorkspaceActionsProps { workspace: Workspace; @@ -35,7 +36,6 @@ export interface WorkspaceActionsProps { handleDelete: () => void; handleUpdate: () => void; handleCancel: () => void; - handleClone: () => void; handleSettings: () => void; handleChangeVersion: () => void; handleDormantActivate: () => void; @@ -54,7 +54,6 @@ export const WorkspaceActions: FC = ({ handleUpdate, handleCancel, handleSettings, - handleClone, handleChangeVersion, handleDormantActivate: handleDormantActivate, isUpdating, @@ -92,6 +91,7 @@ export const WorkspaceActions: FC = ({ const styles = useStyles(); const menuTriggerRef = useRef(null); const [isMenuOpen, setIsMenuOpen] = useState(false); + const duplicateWorkspace = useWorkspaceDuplication(); // Returns a function that will execute the action and close the menu const onMenuItemClick = (actionFn: () => void) => () => { @@ -151,7 +151,9 @@ export const WorkspaceActions: FC = ({ )} - + duplicateWorkspace(workspace))} + > Duplicate… diff --git a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx index 7cb465d0988dd..e7fb62eb47591 100644 --- a/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx @@ -34,7 +34,6 @@ import { templateVersion, templateVersions } from "api/queries/templates"; import { Alert } from "components/Alert/Alert"; import { Stack } from "components/Stack/Stack"; import { useWorkspaceBuildLogs } from "hooks/useWorkspaceBuildLogs"; -import { type CreateWorkspaceMode } from "../CreateWorkspacePage/CreateWorkspacePage"; interface WorkspaceReadyPageProps { workspaceState: StateFrom; @@ -117,22 +116,6 @@ export const WorkspaceReadyPage = ({ bannerSend({ type: "REFRESH_WORKSPACE", workspace }); }, [bannerSend, workspace]); - const handleWorkspaceCloning = () => { - if (template?.name === undefined) { - return; - } - - const workspaceCreationParams = new URLSearchParams({ - mode: "duplicate" satisfies CreateWorkspaceMode, - name: workspace.name, - }); - - navigate({ - pathname: `/templates/${template.name}/workspace`, - search: workspaceCreationParams.toString(), - }); - }; - const favicon = getFaviconByStatus(workspace.latest_build); const deadline = getDeadline(workspace); const canUpdateWorkspace = Boolean(permissions?.updateWorkspace); @@ -199,7 +182,6 @@ export const WorkspaceReadyPage = ({ }} handleCancel={() => workspaceSend({ type: "CANCEL" })} handleSettings={() => navigate("settings")} - handleClone={handleWorkspaceCloning} handleBuildRetry={() => workspaceSend({ type: "RETRY_BUILD" })} handleChangeVersion={() => { setChangeVersionDialogOpen(true); From ac86d94aee46c378e45cbc565f2cd60848da0efb Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Thu, 12 Oct 2023 23:14:22 +0000 Subject: [PATCH 11/21] refactor: rename rich params validator for clarity --- .../pages/CreateWorkspacePage/CreateWorkspacePageView.tsx | 6 +++--- site/src/utils/richParameters.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index b9fe003e7990c..9b857724f61cd 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -20,7 +20,7 @@ import { import { makeStyles } from "@mui/styles"; import { getInitialRichParameterValues, - useValidationSchemaForRichParameters, + validateRichParameters, } from "utils/richParameters"; import { ImmutableTemplateParametersSection, @@ -95,7 +95,7 @@ export const CreateWorkspacePageView: FC = ({ }, validationSchema: Yup.object({ name: nameValidator("Workspace Name"), - rich_parameter_values: useValidationSchemaForRichParameters(parameters), + rich_parameter_values: validateRichParameters(parameters), }), enableReinitialize: true, onSubmit: (request) => { @@ -283,7 +283,7 @@ function DuplicateWarningMessage() { return null; } - // Set up looks a little hokey (having an Alert already fully configured to + // Setup looks a little hokey (having an Alert already fully configured to // listen to dismissals, on top of more dismissal state), but relying solely // on the Alert API wouldn't get rid of the div and horizontal margin helper // after the dismiss happens. Not using CSS margins because those can be a diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index d63da7c4ee530..634f49496b020 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -38,7 +38,7 @@ const isValidValue = ( return true; }; -export const useValidationSchemaForRichParameters = ( +export const validateRichParameters = ( templateParameters?: TemplateVersionParameter[], lastBuildParameters?: WorkspaceBuildParameter[], ): Yup.AnySchema => { From e290ece56c5f951273f427ae5e8129d5a21ca815 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 14:43:49 +0000 Subject: [PATCH 12/21] refactor: clean up getInitialRichParameterValues for clarity --- site/src/utils/richParameters.ts | 33 +++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index 634f49496b020..64a9e0d5638bd 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -5,24 +5,27 @@ import { import * as Yup from "yup"; export const getInitialRichParameterValues = ( - templateParameters: TemplateVersionParameter[], - buildParameters?: WorkspaceBuildParameter[], + templateParameters: readonly TemplateVersionParameter[], + buildParameters?: readonly WorkspaceBuildParameter[], ): WorkspaceBuildParameter[] => { - return templateParameters.map((parameter) => { - const existentBuildParameter = buildParameters?.find( - (p) => p.name === parameter.name, + return templateParameters.map((templateParam) => { + const matchedBuildParam = buildParameters?.find( + (buildParam) => buildParam.name === templateParam.name, ); - const shouldReturnTheDefaultValue = - !existentBuildParameter || - !isValidValue(parameter, existentBuildParameter) || - parameter.ephemeral; - if (shouldReturnTheDefaultValue) { - return { - name: parameter.name, - value: parameter.default_value, - }; + + const shouldOverrideTemplate = + matchedBuildParam !== undefined && + isValidValue(templateParam, matchedBuildParam) && + !templateParam.ephemeral; + + if (shouldOverrideTemplate) { + return matchedBuildParam; } - return existentBuildParameter; + + return { + name: templateParam.name, + value: templateParam.default_value, + }; }); }; From 49b4ad67b9be8a4e30f70e726c1d19b3929bda7c Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 15:08:08 +0000 Subject: [PATCH 13/21] refactor: clean up TemplateParameters components --- .../TemplateParameters/TemplateParameters.tsx | 98 +++++++++---------- 1 file changed, 45 insertions(+), 53 deletions(-) diff --git a/site/src/components/TemplateParameters/TemplateParameters.tsx b/site/src/components/TemplateParameters/TemplateParameters.tsx index 9befc7c74051f..2213166b3ae71 100644 --- a/site/src/components/TemplateParameters/TemplateParameters.tsx +++ b/site/src/components/TemplateParameters/TemplateParameters.tsx @@ -17,68 +17,60 @@ export type TemplateParametersSectionProps = { export const MutableTemplateParametersSection: FC< TemplateParametersSectionProps > = ({ templateParameters, getInputProps, ...formSectionProps }) => { - const hasMutableParameters = - templateParameters.filter((p) => p.mutable).length > 0; + const mutableParameters = templateParameters.filter((p) => p.mutable); + + if (mutableParameters.length === 0) { + return null; + } return ( - <> - {hasMutableParameters && ( - - - {templateParameters.map( - (parameter, index) => - parameter.mutable && ( - - ), - )} - - - )} - + + + {mutableParameters.map((parameter, index) => ( + + ))} + + ); }; export const ImmutableTemplateParametersSection: FC< TemplateParametersSectionProps > = ({ templateParameters, getInputProps, ...formSectionProps }) => { - const hasImmutableParameters = - templateParameters.filter((p) => !p.mutable).length > 0; + const immutableParams = templateParameters.filter((p) => !p.mutable); + + if (immutableParams.length === 0) { + return null; + } return ( - <> - {hasImmutableParameters && ( - - These settings cannot be changed after creating - the workspace. - - } - > - - {templateParameters.map( - (parameter, index) => - !parameter.mutable && ( - - ), - )} - - - )} - + + These settings cannot be changed after creating the + workspace. + + } + > + + {immutableParams.map((parameter, index) => ( + + ))} + + ); }; From f6b9285ac3924d23ba49b811ef9a18e1ccccee6f Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 15:48:21 +0000 Subject: [PATCH 14/21] fix: update imports for validator name change --- site/src/pages/WorkspacePage/UpdateBuildParametersDialog.tsx | 5 ++--- .../WorkspaceParametersPage/WorkspaceParametersForm.tsx | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/site/src/pages/WorkspacePage/UpdateBuildParametersDialog.tsx b/site/src/pages/WorkspacePage/UpdateBuildParametersDialog.tsx index fadc9a8a94b72..cd2e278f4caba 100644 --- a/site/src/pages/WorkspacePage/UpdateBuildParametersDialog.tsx +++ b/site/src/pages/WorkspacePage/UpdateBuildParametersDialog.tsx @@ -15,7 +15,7 @@ import { RichParameterInput } from "components/RichParameterInput/RichParameterI import { useFormik } from "formik"; import { getInitialRichParameterValues, - useValidationSchemaForRichParameters, + validateRichParameters, } from "utils/richParameters"; import * as Yup from "yup"; import DialogActions from "@mui/material/DialogActions"; @@ -36,8 +36,7 @@ export const UpdateBuildParametersDialog: FC< rich_parameter_values: getInitialRichParameterValues(missedParameters), }, validationSchema: Yup.object({ - rich_parameter_values: - useValidationSchemaForRichParameters(missedParameters), + rich_parameter_values: validateRichParameters(missedParameters), }), onSubmit: (values) => { onUpdate(values.rich_parameter_values); diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx index e281a740ed9ce..ebb284e29608f 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx @@ -9,7 +9,7 @@ import { useFormik } from "formik"; import { FC } from "react"; import { getInitialRichParameterValues, - useValidationSchemaForRichParameters, + validateRichParameters, } from "utils/richParameters"; import * as Yup from "yup"; import { getFormHelpers } from "utils/formUtils"; @@ -46,7 +46,7 @@ export const WorkspaceParametersForm: FC<{ ), }, validationSchema: Yup.object({ - rich_parameter_values: useValidationSchemaForRichParameters( + rich_parameter_values: validateRichParameters( templateVersionRichParameters, ), }), From ba6e20c245ab3b5512901d57b9dce3feaa7b4fc9 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 19:02:52 +0000 Subject: [PATCH 15/21] wip: commit progress for creating workspaces --- .../CreateWorkspacePage.tsx | 97 ++++++++++++++----- .../WorkspaceActions/WorkspaceActions.tsx | 7 +- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 3a922d4e066b0..1b3d95ad66ea4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -2,6 +2,7 @@ import { TemplateVersionParameter, Workspace, WorkspaceBuildParameter, + WorkspaceResourceMetadata, } from "api/typesGenerated"; import { useMe } from "hooks/useMe"; import { useOrganizationId } from "hooks/useOrganizationId"; @@ -185,30 +186,6 @@ const useExternalAuth = (versionId: string | undefined) => { return { authList, isAllSignedIn, pollingStatus, startPolling } as const; }; -/** - * Returns a function that takes a workspace, and then navigates the user to the - * workspace creation page, with the form pre-filled with the provided - * workspace's parameters. - */ -export function useWorkspaceDuplication() { - const navigate = useNavigate(); - - return useCallback( - (workspace: Workspace) => { - const workspaceCreationParams = new URLSearchParams({ - mode: "duplicate" satisfies CreateWorkspaceMode, - name: workspace.name, - }); - - navigate({ - pathname: `/templates/${workspace.template_name}/workspace`, - search: workspaceCreationParams.toString(), - }); - }, - [navigate], - ); -} - function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { const paramMode = params.get("mode"); if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { @@ -281,6 +258,78 @@ const getDefaultBuildParameters = ( }); }; +function getDuplicationUrlParams( + templateParams: readonly TemplateVersionParameter[], + workspace: Workspace, +): URLSearchParams { + // Record type makes sure that every property key added starts with "param." + const buildParams: Record<`param.${string}`, string> = {}; + + const allMetadata: WorkspaceResourceMetadata[] = []; + for (const resource of workspace.latest_build.resources) { + const metadata = resource.metadata; + if (metadata !== undefined) { + allMetadata.push(...metadata); + } + } + + for (const param of templateParams) { + /** + * @todo Pretty sure the last piece of the puzzle is figuring out how to + * cross-reference the resources from the build with the template params + */ + buildParams[`param.${param.name}`] = String(param["default_value"]); + } + + return new URLSearchParams({ + ...buildParams, + mode: "duplicate" satisfies CreateWorkspaceMode, + name: workspace.name, + version: workspace.template_active_version_id, + }); +} + +/** + * Takes a workspace, and returns out a function that will navigate the user to + * the 'Create Workspace' page, pre-filling the form with as much information + * about the workspace as possible. + */ +// Meant to be consumed by components outside of this file +export function useWorkspaceDuplication(workspace: Workspace) { + const navigate = useNavigate(); + + // Sadly, there isn't a good way to derive this data from the workspace itself + const templateParametersQuery = useQuery( + richParameters(workspace.template_active_version_id), + ); + + // Not using useEffectEvent for this, even with the slightly more complicated + // dependency array, because useEffect isn't really an intended use case + const duplicateWorkspace = useCallback(() => { + const templateParams = templateParametersQuery.data; + if (templateParams === undefined) { + return; + } + + const newUrlParams = getDuplicationUrlParams(templateParams, workspace); + + // Necessary for giving modals/popups time to flush their state changes and + // close the popup before actually navigating. Otherwise, you risk the modal + // awkwardly hanging there during the page transition + void Promise.resolve().then(() => { + navigate({ + pathname: `/templates/${workspace.template_name}/workspace`, + search: newUrlParams.toString(), + }); + }); + }, [navigate, workspace, templateParametersQuery.data]); + + return { + duplicateWorkspace, + duplicationReady: !templateParametersQuery.isLoading, + } as const; +} + export const orderedTemplateParameters = ( templateParameters?: TemplateVersionParameter[], ): TemplateVersionParameter[] => { diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index 3cb8e0d91cd5f..ead67790309d3 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -91,7 +91,9 @@ export const WorkspaceActions: FC = ({ const styles = useStyles(); const menuTriggerRef = useRef(null); const [isMenuOpen, setIsMenuOpen] = useState(false); - const duplicateWorkspace = useWorkspaceDuplication(); + + const { duplicateWorkspace, duplicationReady } = + useWorkspaceDuplication(workspace); // Returns a function that will execute the action and close the menu const onMenuItemClick = (actionFn: () => void) => () => { @@ -152,7 +154,8 @@ export const WorkspaceActions: FC = ({ )} duplicateWorkspace(workspace))} + onClick={onMenuItemClick(duplicateWorkspace)} + disabled={!duplicationReady} > Duplicate… From 2a7f9e0f05e4682af9119498aecbd37283605554 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 19:24:39 +0000 Subject: [PATCH 16/21] refactor: centralize out workspace parameter fetching logic --- site/src/api/queries/workspaces.ts | 11 +++++++++++ .../WorkspaceParametersPage.tsx | 7 +++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index ff233f33c3d1e..5c3cfdd34607d 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -84,6 +84,17 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { }; }; +export function workspaceParametersKey(workspaceId: string) { + return ["workspaces", workspaceId, "parameters"] as const; +} + +export function workspaceParameters(workspace: Workspace) { + return { + queryKey: workspaceParametersKey(workspace.id), + queryFn: () => API.getWorkspaceParameters(workspace), + } as const; +} + export function workspacesKey(config: WorkspacesRequest = {}) { const { q, limit } = config; return ["workspaces", { q, limit }] as const; diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx index e92c220585396..5d94625abcec0 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx @@ -14,14 +14,13 @@ import { FC } from "react"; import { isApiValidationError } from "api/errors"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { WorkspaceBuildParameter } from "api/typesGenerated"; +import { workspaceParameters } from "api/queries/workspaces"; const WorkspaceParametersPage = () => { const workspace = useWorkspaceSettings(); - const parameters = useQuery({ - queryKey: ["workspace", workspace.id, "parameters"], - queryFn: () => getWorkspaceParameters(workspace), - }); + const parameters = useQuery(workspaceParameters(workspace)); const navigate = useNavigate(); + const updateParameters = useMutation({ mutationFn: (buildParameters: WorkspaceBuildParameter[]) => postWorkspaceBuild(workspace.id, { From bbe44a16bc335a484b72c2da8834c2558351abc2 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 19:34:58 +0000 Subject: [PATCH 17/21] refactor: centralize workspace parameter mutation logic --- site/src/api/queries/workspaces.ts | 10 ++++++++++ .../WorkspaceParametersPage.tsx | 14 ++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index 5c3cfdd34607d..4607464621e2d 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -95,6 +95,16 @@ export function workspaceParameters(workspace: Workspace) { } as const; } +export function updateWorkspaceParameters(workspaceId: string) { + return { + mutationFn: (buildParameters: WorkspaceBuildParameter[]) => + API.postWorkspaceBuild(workspaceId, { + transition: "start", + rich_parameter_values: buildParameters, + }), + } as const; +} + export function workspacesKey(config: WorkspacesRequest = {}) { const { q, limit } = config; return ["workspaces", { q, limit }] as const; diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx index 5d94625abcec0..75575f2f6814d 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx @@ -1,4 +1,4 @@ -import { getWorkspaceParameters, postWorkspaceBuild } from "api/api"; +import { getWorkspaceParameters } from "api/api"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { useWorkspaceSettings } from "../WorkspaceSettingsLayout"; @@ -13,8 +13,10 @@ import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader"; import { FC } from "react"; import { isApiValidationError } from "api/errors"; import { ErrorAlert } from "components/Alert/ErrorAlert"; -import { WorkspaceBuildParameter } from "api/typesGenerated"; -import { workspaceParameters } from "api/queries/workspaces"; +import { + updateWorkspaceParameters, + workspaceParameters, +} from "api/queries/workspaces"; const WorkspaceParametersPage = () => { const workspace = useWorkspaceSettings(); @@ -22,11 +24,7 @@ const WorkspaceParametersPage = () => { const navigate = useNavigate(); const updateParameters = useMutation({ - mutationFn: (buildParameters: WorkspaceBuildParameter[]) => - postWorkspaceBuild(workspace.id, { - transition: "start", - rich_parameter_values: buildParameters, - }), + ...updateWorkspaceParameters(workspace.id), onSuccess: () => { navigate(`/${workspace.owner_name}/${workspace.name}`); }, From 1fb22051adf15daf971b260c8eed96b44c60d917 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 13 Oct 2023 20:21:37 +0000 Subject: [PATCH 18/21] feat: get cloning functionality finished --- site/src/api/queries/workspaceBuilds.ts | 13 +++++- .../CreateWorkspacePage.tsx | 43 +++++++------------ 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/site/src/api/queries/workspaceBuilds.ts b/site/src/api/queries/workspaceBuilds.ts index fb35a735ab3ef..abcef99c882c0 100644 --- a/site/src/api/queries/workspaceBuilds.ts +++ b/site/src/api/queries/workspaceBuilds.ts @@ -1,7 +1,18 @@ -import { UseInfiniteQueryOptions } from "react-query"; +import { QueryOptions, UseInfiniteQueryOptions } from "react-query"; import * as API from "api/api"; import { WorkspaceBuild, WorkspaceBuildsRequest } from "api/typesGenerated"; +export function workspaceBuildParametersKey(workspaceId: string) { + return ["workspaceBuilds", workspaceId, "parameters"] as const; +} + +export function workspaceBuildParameters(workspaceBuildId: string) { + return { + queryKey: workspaceBuildParametersKey(workspaceBuildId), + queryFn: () => API.getWorkspaceBuildParameters(workspaceBuildId), + } as const satisfies QueryOptions; +} + export const workspaceBuildByNumber = ( username: string, workspaceName: string, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 1b3d95ad66ea4..05a38707ea066 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -2,7 +2,6 @@ import { TemplateVersionParameter, Workspace, WorkspaceBuildParameter, - WorkspaceResourceMetadata, } from "api/typesGenerated"; import { useMe } from "hooks/useMe"; import { useOrganizationId } from "hooks/useOrganizationId"; @@ -33,6 +32,7 @@ import { checkAuthorization } from "api/queries/authCheck"; import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; import { richParameters } from "api/queries/templateVersions"; import { useEffectEvent } from "hooks/hookPolyfills"; +import { workspaceBuildParameters } from "api/queries/workspaceBuilds"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; @@ -259,30 +259,19 @@ const getDefaultBuildParameters = ( }; function getDuplicationUrlParams( - templateParams: readonly TemplateVersionParameter[], + workspaceParams: readonly WorkspaceBuildParameter[], workspace: Workspace, ): URLSearchParams { - // Record type makes sure that every property key added starts with "param." - const buildParams: Record<`param.${string}`, string> = {}; - - const allMetadata: WorkspaceResourceMetadata[] = []; - for (const resource of workspace.latest_build.resources) { - const metadata = resource.metadata; - if (metadata !== undefined) { - allMetadata.push(...metadata); - } - } + // Record type makes sure that every property key added starts with "param."; + // page is also set up to parse params with this prefix for auto mode + const consolidatedParams: Record<`param.${string}`, string> = {}; - for (const param of templateParams) { - /** - * @todo Pretty sure the last piece of the puzzle is figuring out how to - * cross-reference the resources from the build with the template params - */ - buildParams[`param.${param.name}`] = String(param["default_value"]); + for (const p of workspaceParams) { + consolidatedParams[`param.${p.name}`] = p.value; } return new URLSearchParams({ - ...buildParams, + ...consolidatedParams, mode: "duplicate" satisfies CreateWorkspaceMode, name: workspace.name, version: workspace.template_active_version_id, @@ -297,21 +286,19 @@ function getDuplicationUrlParams( // Meant to be consumed by components outside of this file export function useWorkspaceDuplication(workspace: Workspace) { const navigate = useNavigate(); - - // Sadly, there isn't a good way to derive this data from the workspace itself - const templateParametersQuery = useQuery( - richParameters(workspace.template_active_version_id), + const buildParametersQuery = useQuery( + workspaceBuildParameters(workspace.latest_build.id), ); // Not using useEffectEvent for this, even with the slightly more complicated // dependency array, because useEffect isn't really an intended use case const duplicateWorkspace = useCallback(() => { - const templateParams = templateParametersQuery.data; - if (templateParams === undefined) { + const buildParams = buildParametersQuery.data; + if (buildParams === undefined) { return; } - const newUrlParams = getDuplicationUrlParams(templateParams, workspace); + const newUrlParams = getDuplicationUrlParams(buildParams, workspace); // Necessary for giving modals/popups time to flush their state changes and // close the popup before actually navigating. Otherwise, you risk the modal @@ -322,11 +309,11 @@ export function useWorkspaceDuplication(workspace: Workspace) { search: newUrlParams.toString(), }); }); - }, [navigate, workspace, templateParametersQuery.data]); + }, [navigate, workspace, buildParametersQuery.data]); return { duplicateWorkspace, - duplicationReady: !templateParametersQuery.isLoading, + duplicationReady: !buildParametersQuery.isLoading, } as const; } From 239bddc0cb8b83facb0494650f1c802f69c520e6 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 16 Oct 2023 13:19:42 +0000 Subject: [PATCH 19/21] fix: make sure button does not auto-submit connected forms --- site/src/pages/TemplatesPage/TemplatesPageView.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 795ac844e1e73..97858da90eaa4 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -120,6 +120,7 @@ const TemplateRow: FC<{ template: Template }> = ({ template }) => {