diff --git a/site/src/api/queries/workspaceBuilds.ts b/site/src/api/queries/workspaceBuilds.ts index 98049a89e7..9da020881b 100644 --- a/site/src/api/queries/workspaceBuilds.ts +++ b/site/src/api/queries/workspaceBuilds.ts @@ -1,6 +1,21 @@ -import { UseInfiniteQueryOptions } from "react-query"; +import { QueryOptions, UseInfiniteQueryOptions } from "react-query"; import * as API from "api/api"; -import { WorkspaceBuild, WorkspaceBuildsRequest } from "api/typesGenerated"; +import { + type WorkspaceBuild, + type WorkspaceBuildParameter, + type WorkspaceBuildsRequest, +} from "api/typesGenerated"; + +export function workspaceBuildParametersKey(workspaceBuildId: string) { + return ["workspaceBuilds", workspaceBuildId, "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, diff --git a/site/src/components/Alert/Alert.tsx b/site/src/components/Alert/Alert.tsx index fb3f5e07c1..bba4044c4c 100644 --- a/site/src/components/Alert/Alert.tsx +++ b/site/src/components/Alert/Alert.tsx @@ -21,8 +21,15 @@ export const Alert: FC = ({ }) => { const [open, setOpen] = useState(true); + // Can't only rely on MUI's hiding behavior inside flex layouts, because even + // though MUI will make a dismissed alert have zero height, the alert will + // still behave as a flex child and introduce extra row/column gaps + if (!open) { + return null; + } + return ( - + { ); }); }); + + it("Detects when a workspace is being created with the 'duplicate' mode", async () => { + const params = new URLSearchParams({ + mode: "duplicate", + name: MockWorkspace.name, + version: MockWorkspace.template_active_version_id, + }); + + renderWithAuth(, { + path: "/templates/:template/workspace", + route: `/templates/${MockWorkspace.name}/workspace?${params.toString()}`, + }); + + const warningMessage = await screen.findByRole("alert"); + const nameInput = await screen.findByRole("textbox", { + name: "Workspace Name", + }); + + expect(warningMessage).toHaveTextContent(Language.duplicationWarning); + expect(nameInput).toHaveValue(`${MockWorkspace.name}-copy`); + }); }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 2c6cff4807..7a0d41ce9f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -30,7 +30,8 @@ import { CreateWSPermissions, createWorkspaceChecks } from "./permissions"; 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"; @@ -41,10 +42,9 @@ const CreateWorkspacePage: FC = () => { const navigate = useNavigate(); const [searchParams, setSearchParams] = useSearchParams(); const defaultBuildParameters = getDefaultBuildParameters(searchParams); - const mode = (searchParams.get("mode") ?? "form") as CreateWorkspaceMode; + const mode = getWorkspaceMode(searchParams); const customVersionId = searchParams.get("version") ?? undefined; - const defaultName = - mode === "auto" ? generateUniqueName() : searchParams.get("name") ?? ""; + const defaultName = getDefaultName(mode, searchParams); const queryClient = useQueryClient(); const autoCreateWorkspaceMutation = useMutation( @@ -122,6 +122,7 @@ const CreateWorkspacePage: FC = () => { ) : ( { - if (!templateParameters) { - return []; - } - - const immutables = templateParameters.filter( - (parameter) => !parameter.mutable, - ); - const mutables = templateParameters.filter((parameter) => parameter.mutable); - return [...immutables, ...mutables]; -}; - const generateUniqueName = () => { const numberDictionary = NumberDictionary.generate({ min: 0, max: 99 }); return uniqueNamesGenerator({ @@ -245,3 +232,25 @@ const generateUniqueName = () => { }; export default CreateWorkspacePage; + +function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { + const paramMode = params.get("mode"); + if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { + return paramMode as 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 ?? ""; +} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index fb0546ca41..dfb43f5c31 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -19,6 +19,7 @@ const meta: Meta = { template: MockTemplate, parameters: [], externalAuth: [], + mode: "form", permissions: { createWorkspaceForUser: true, }, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index edcee19798..aa963e3029 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -30,11 +30,21 @@ import { import { ExternalAuth } from "./ExternalAuth"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Stack } from "components/Stack/Stack"; -import { type ExternalAuthPollingState } from "./CreateWorkspacePage"; +import { + CreateWorkspaceMode, + type ExternalAuthPollingState, +} from "./CreateWorkspacePage"; import { useSearchParams } from "react-router-dom"; -import type { CreateWSPermissions } from "./permissions"; +import { CreateWSPermissions } from "./permissions"; +import { Alert } from "components/Alert/Alert"; + +export const Language = { + duplicationWarning: + "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", +} as const; export interface CreateWorkspacePageViewProps { + mode: CreateWorkspaceMode; error: unknown; defaultName: string; defaultOwner: TypesGen.User; @@ -55,6 +65,7 @@ export interface CreateWorkspacePageViewProps { } export const CreateWorkspacePageView: FC = ({ + mode, error, defaultName, defaultOwner, @@ -116,6 +127,13 @@ export const CreateWorkspacePageView: FC = ({ {Boolean(error) && } + + {mode === "duplicate" && ( + + {Language.duplicationWarning} + + )} + {/* General info */} { + return useWorkspaceDuplication(workspace); + }, + { + initialProps: { workspace }, + extraRoutes: [ + { + path: "/templates/:template/workspace", + element: , + }, + ], + }, + ); +} + +type RenderResult = Awaited>; + +async function performNavigation( + result: RenderResult["result"], + router: RenderResult["router"], +) { + await waitFor(() => expect(result.current.isDuplicationReady).toBe(true)); + result.current.duplicateWorkspace(); + + return waitFor(() => { + expect(router.state.location.pathname).toEqual( + `/templates/${MockWorkspace.template_name}/workspace`, + ); + }); +} + +describe(`${useWorkspaceDuplication.name}`, () => { + it("Will never be ready when there is no workspace passed in", async () => { + const { result, rerender } = await render(undefined); + expect(result.current.isDuplicationReady).toBe(false); + + for (let i = 0; i < 10; i++) { + rerender({ workspace: undefined }); + expect(result.current.isDuplicationReady).toBe(false); + } + }); + + it("Will become ready when workspace is provided and build params are successfully fetched", async () => { + const { result } = await render(MockWorkspace); + + expect(result.current.isDuplicationReady).toBe(false); + await waitFor(() => expect(result.current.isDuplicationReady).toBe(true)); + }); + + it("Is able to navigate the user to the workspace creation page", async () => { + const { result, router } = await render(MockWorkspace); + await performNavigation(result, router); + }); + + test("Navigating populates the URL search params with the workspace's build params", async () => { + const { result, router } = await render(MockWorkspace); + await performNavigation(result, router); + + const parsedParams = new URLSearchParams(router.state.location.search); + const mockBuildParams = [ + M.MockWorkspaceBuildParameter1, + M.MockWorkspaceBuildParameter2, + M.MockWorkspaceBuildParameter3, + M.MockWorkspaceBuildParameter4, + M.MockWorkspaceBuildParameter5, + ]; + + for (const { name, value } of mockBuildParams) { + const key = `param.${name}`; + expect(parsedParams.get(key)).toEqual(value); + } + }); + + test("Navigating appends other necessary metadata to the search params", async () => { + const { result, router } = await render(MockWorkspace); + await performNavigation(result, router); + + const parsedParams = new URLSearchParams(router.state.location.search); + const extraMetadataEntries = [ + ["mode", "duplicate"], + ["name", MockWorkspace.name], + ["version", MockWorkspace.template_active_version_id], + ] as const; + + for (const [key, value] of extraMetadataEntries) { + expect(parsedParams.get(key)).toBe(value); + } + }); +}); diff --git a/site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.ts b/site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.ts new file mode 100644 index 0000000000..6e929dd6f0 --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.ts @@ -0,0 +1,71 @@ +import { useNavigate } from "react-router-dom"; +import { useQuery } from "react-query"; +import { type CreateWorkspaceMode } from "./CreateWorkspacePage"; +import { + type Workspace, + type WorkspaceBuildParameter, +} from "api/typesGenerated"; +import { workspaceBuildParameters } from "api/queries/workspaceBuilds"; +import { useCallback } from "react"; + +function getDuplicationUrlParams( + workspaceParams: readonly WorkspaceBuildParameter[], + workspace: Workspace, +): URLSearchParams { + // 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 p of workspaceParams) { + consolidatedParams[`param.${p.name}`] = p.value; + } + + return new URLSearchParams({ + ...consolidatedParams, + 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. + */ +export function useWorkspaceDuplication(workspace?: Workspace) { + const navigate = useNavigate(); + const buildParametersQuery = useQuery( + workspace !== undefined + ? workspaceBuildParameters(workspace.latest_build.id) + : { enabled: false }, + ); + + // Not using useEffectEvent for this, because useEffect isn't really an + // intended use case for this custom hook + const duplicateWorkspace = useCallback(() => { + const buildParams = buildParametersQuery.data; + if (buildParams === undefined || workspace === undefined) { + return; + } + + const newUrlParams = getDuplicationUrlParams(buildParams, workspace); + + // Necessary for giving modals/popups time to flush their state changes and + // close the popup before actually navigating. MUI does provide the + // disablePortal prop, which also side-steps this issue, but you have to + // remember to put it on any component that calls this function. Better to + // code defensively and have some redundancy in case someone forgets + void Promise.resolve().then(() => { + navigate({ + pathname: `/templates/${workspace.template_name}/workspace`, + search: newUrlParams.toString(), + }); + }); + }, [navigate, workspace, buildParametersQuery.data]); + + return { + duplicateWorkspace, + isDuplicationReady: buildParametersQuery.isSuccess, + } as const; +} diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index e36b2161bb..ead3f4c702 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -1,5 +1,6 @@ import { FC, Fragment, ReactNode } from "react"; import { Workspace, WorkspaceBuildParameter } from "api/typesGenerated"; +import { useWorkspaceDuplication } from "pages/CreateWorkspacePage/useWorkspaceDuplication"; import { ActionLoadingButton, CancelButton, @@ -15,16 +16,19 @@ import { 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 Divider from "@mui/material/Divider"; +import DuplicateIcon from "@mui/icons-material/FileCopyOutlined"; +import SettingsIcon from "@mui/icons-material/SettingsOutlined"; +import HistoryIcon from "@mui/icons-material/HistoryOutlined"; +import DeleteIcon from "@mui/icons-material/DeleteOutlined"; + import { MoreMenu, MoreMenuContent, MoreMenuItem, MoreMenuTrigger, } from "components/MoreMenu/MoreMenu"; -import Divider from "@mui/material/Divider"; export interface WorkspaceActionsProps { workspace: Workspace; @@ -68,6 +72,8 @@ export const WorkspaceActions: FC = ({ canChangeVersions, ); const canBeUpdated = workspace.outdated && canAcceptJobs; + const { duplicateWorkspace, isDuplicationReady } = + useWorkspaceDuplication(workspace); // A mapping of button type to the corresponding React component const buttonMapping: ButtonMapping = { @@ -120,11 +126,14 @@ export const WorkspaceActions: FC = ({ (isUpdating ? buttonMapping[ButtonTypesEnum.updating] : buttonMapping[ButtonTypesEnum.update])} + {isRestarting && buttonMapping[ButtonTypesEnum.restarting]} + {!isRestarting && actionsByStatus.map((action) => ( {buttonMapping[action]} ))} + {canCancel && } = ({ aria-controls="workspace-options" disabled={!canAcceptJobs} /> + - + Settings + {canChangeVersions && ( - + Change version… )} + + + + Duplicate… + + + - + Delete… diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index a4d8b80561..be31e8ee37 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -354,7 +354,16 @@ export const handlers = [ rest.get( "/api/v2/workspacebuilds/:workspaceBuildId/parameters", (_, res, ctx) => { - return res(ctx.status(200), ctx.json([M.MockWorkspaceBuildParameter1])); + return res( + ctx.status(200), + ctx.json([ + M.MockWorkspaceBuildParameter1, + M.MockWorkspaceBuildParameter2, + M.MockWorkspaceBuildParameter3, + M.MockWorkspaceBuildParameter4, + M.MockWorkspaceBuildParameter5, + ]), + ); }, ), diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index 1de1c1997b..b07d4921be 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -1,4 +1,9 @@ -import { render as tlRender, screen, waitFor } from "@testing-library/react"; +import { + render as tlRender, + screen, + waitFor, + renderHook, +} from "@testing-library/react"; import { AppProviders, ThemeProviders } from "App"; import { DashboardLayout } from "components/Dashboard/DashboardLayout"; import { TemplateSettingsLayout } from "pages/TemplateSettingsPage/TemplateSettingsLayout"; @@ -10,15 +15,13 @@ import { } from "react-router-dom"; import { RequireAuth } from "../components/RequireAuth/RequireAuth"; import { MockUser } from "./entities"; -import { ReactNode } from "react"; +import { ReactNode, useState } from "react"; import { QueryClient } from "react-query"; -export const renderWithRouter = ( - router: ReturnType, -) => { - // Create one query client for each render isolate it avoid other - // tests to be affected - const queryClient = new QueryClient({ +function createTestQueryClient() { + // Helps create one query client for each test case, to make sure that tests + // are isolated and can't affect each other + return new QueryClient({ defaultOptions: { queries: { retry: false, @@ -28,6 +31,12 @@ export const renderWithRouter = ( }, }, }); +} + +export const renderWithRouter = ( + router: ReturnType, +) => { + const queryClient = createTestQueryClient(); return { ...tlRender( @@ -53,7 +62,7 @@ export const render = (element: ReactNode) => { ); }; -type RenderWithAuthOptions = { +export type RenderWithAuthOptions = { // The current URL, /workspaces/123 route?: string; // The route path, /workspaces/:workspaceId @@ -95,6 +104,82 @@ export function renderWithAuth( }; } +type RenderHookWithAuthOptions = Partial< + Readonly< + Omit & { + initialProps: Props; + } + > +>; + +/** + * Custom version of renderHook that is aware of all our App providers. + * + * Had to do some nasty, cursed things in the implementation to make sure that + * the tests using this function remained simple. + * + * @see {@link https://github.com/coder/coder/pull/10362#discussion_r1380852725} + */ +export async function renderHookWithAuth( + render: (initialProps: Props) => Result, + { + initialProps, + path = "/", + extraRoutes = [], + }: RenderHookWithAuthOptions = {}, +) { + const queryClient = createTestQueryClient(); + + // Easy to miss – there's an evil definite assignment via the ! + let escapedRouter!: ReturnType; + + const { result, rerender, unmount } = renderHook(render, { + initialProps, + wrapper: ({ children }) => { + /** + * Unfortunately, there isn't a way to define the router outside the + * wrapper while keeping it aware of children, meaning that we need to + * define the router as readonly state in the component instance. This + * ensures the value remains stable across all re-renders + */ + // eslint-disable-next-line react-hooks/rules-of-hooks -- This is actually processed as a component; the linter just isn't aware of that + const [readonlyStatefulRouter] = useState(() => { + return createMemoryRouter([ + { path, element: <>{children} }, + ...extraRoutes, + ]); + }); + + /** + * Leaks the wrapper component's state outside React's render cycles. + */ + escapedRouter = readonlyStatefulRouter; + + return ( + + + + ); + }, + }); + + /** + * This is necessary to get around some providers in AppProviders having + * conditional rendering and not always rendering their children immediately. + * + * The hook result won't actually exist until the children defined via wrapper + * render in full. + */ + await waitFor(() => expect(result.current).not.toBe(null)); + + return { + result, + rerender, + unmount, + router: escapedRouter, + } as const; +} + export function renderWithTemplateSettingsLayout( element: JSX.Element, {