feat: allow users to duplicate workspaces by parameters (#10362)

* chore: add queries for workspace build info

* refactor: clean up logic for CreateWorkspacePage to support multiple modes

* chore: add custom workspace duplication hook

* chore: integrate mode into CreateWorkspacePageView

* fix: add mode to CreateWorkspacePageView stories

* refactor: extract workspace duplication outside CreateWorkspacePage file

* chore: integrate useWorkspaceDuplication into WorkspaceActions

* chore: delete unnecessary function

* refactor: swap useReducer for useState

* fix: swap warning alert for info alert

* refactor: move info alert message

* refactor: simplify UI logic for mode alerts

* fix: prevent dismissed Alerts from affecting layouts

* fix: remove unnecessary prop binding

* docs: reword comment for clarity

* chore: update msw build params to return multiple params

* chore: rename duplicationReady to isDuplicationReady

* chore: expose root component for testing/re-rendering

* chore: get tests in place (still have act warnings)

* refactor: move stuff around for clarity

* chore: finish tests

* chore: revamp tests
This commit is contained in:
Michael Smith
2023-11-03 18:23:09 -04:00
committed by GitHub
parent 23f02651f9
commit 744c73394a
11 changed files with 397 additions and 40 deletions

View File

@ -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<WorkspaceBuildParameter[]>;
}
export const workspaceBuildByNumber = (
username: string,

View File

@ -21,8 +21,15 @@ export const Alert: FC<AlertProps> = ({
}) => {
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 (
<Collapse in={open}>
<Collapse in>
<MuiAlert
{...alertProps}
sx={{ textAlign: "left", ...alertProps.sx }}

View File

@ -20,6 +20,7 @@ import {
waitForLoaderToBeRemoved,
} from "testHelpers/renderHelpers";
import CreateWorkspacePage from "./CreateWorkspacePage";
import { Language } from "./CreateWorkspacePageView";
const nameLabelText = "Workspace Name";
const createWorkspaceText = "Create Workspace";
@ -270,4 +271,25 @@ describe("CreateWorkspacePage", () => {
);
});
});
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(<CreateWorkspacePage />, {
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`);
});
});

View File

@ -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 = () => {
<Loader />
) : (
<CreateWorkspacePageView
mode={mode}
defaultName={defaultName}
defaultOwner={me}
defaultBuildParameters={defaultBuildParameters}
@ -220,20 +221,6 @@ const getDefaultBuildParameters = (
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 = () => {
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 ?? "";
}

View File

@ -19,6 +19,7 @@ const meta: Meta<typeof CreateWorkspacePageView> = {
template: MockTemplate,
parameters: [],
externalAuth: [],
mode: "form",
permissions: {
createWorkspaceForUser: true,
},

View File

@ -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<CreateWorkspacePageViewProps> = ({
mode,
error,
defaultName,
defaultOwner,
@ -116,6 +127,13 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
<FullPageHorizontalForm title="New workspace" onCancel={onCancel}>
<HorizontalForm onSubmit={form.handleSubmit}>
{Boolean(error) && <ErrorAlert error={error} />}
{mode === "duplicate" && (
<Alert severity="info" dismissible>
{Language.duplicationWarning}
</Alert>
)}
{/* General info */}
<FormSection
title="General"

View File

@ -0,0 +1,99 @@
import { waitFor } from "@testing-library/react";
import * as M from "../../testHelpers/entities";
import { type Workspace } from "api/typesGenerated";
import { useWorkspaceDuplication } from "./useWorkspaceDuplication";
import { MockWorkspace } from "testHelpers/entities";
import CreateWorkspacePage from "./CreateWorkspacePage";
import { renderHookWithAuth } from "testHelpers/renderHelpers";
function render(workspace?: Workspace) {
return renderHookWithAuth(
({ workspace }: { workspace?: Workspace }) => {
return useWorkspaceDuplication(workspace);
},
{
initialProps: { workspace },
extraRoutes: [
{
path: "/templates/:template/workspace",
element: <CreateWorkspacePage />,
},
],
},
);
}
type RenderResult = Awaited<ReturnType<typeof render>>;
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);
}
});
});

View File

@ -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;
}

View File

@ -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<WorkspaceActionsProps> = ({
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<WorkspaceActionsProps> = ({
(isUpdating
? buttonMapping[ButtonTypesEnum.updating]
: buttonMapping[ButtonTypesEnum.update])}
{isRestarting && buttonMapping[ButtonTypesEnum.restarting]}
{!isRestarting &&
actionsByStatus.map((action) => (
<Fragment key={action}>{buttonMapping[action]}</Fragment>
))}
{canCancel && <CancelButton handleAction={handleCancel} />}
<MoreMenu>
<MoreMenuTrigger
@ -134,24 +143,36 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
aria-controls="workspace-options"
disabled={!canAcceptJobs}
/>
<MoreMenuContent id="workspace-options">
<MoreMenuItem onClick={handleSettings}>
<SettingsOutlined />
<SettingsIcon />
Settings
</MoreMenuItem>
{canChangeVersions && (
<MoreMenuItem onClick={handleChangeVersion}>
<HistoryOutlined />
<HistoryIcon />
Change version&hellip;
</MoreMenuItem>
)}
<MoreMenuItem
onClick={duplicateWorkspace}
disabled={!isDuplicationReady}
>
<DuplicateIcon />
Duplicate&hellip;
</MoreMenuItem>
<Divider />
<MoreMenuItem
danger
onClick={handleDelete}
data-testid="delete-button"
>
<DeleteOutlined />
<DeleteIcon />
Delete&hellip;
</MoreMenuItem>
</MoreMenuContent>

View File

@ -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,
]),
);
},
),

View File

@ -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<typeof createMemoryRouter>,
) => {
// 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<typeof createMemoryRouter>,
) => {
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<Props> = Partial<
Readonly<
Omit<RenderWithAuthOptions, "children"> & {
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<Result, Props>(
render: (initialProps: Props) => Result,
{
initialProps,
path = "/",
extraRoutes = [],
}: RenderHookWithAuthOptions<Props> = {},
) {
const queryClient = createTestQueryClient();
// Easy to miss there's an evil definite assignment via the !
let escapedRouter!: ReturnType<typeof createMemoryRouter>;
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 (
<AppProviders queryClient={queryClient}>
<RouterProvider router={readonlyStatefulRouter} />
</AppProviders>
);
},
});
/**
* 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,
{