Skip to content

feat: allow users to duplicate workspaces by parameters #10362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9e4f999
chore: add queries for workspace build info
Parkreiner Oct 20, 2023
112bc95
refactor: clean up logic for CreateWorkspacePage to support multiple …
Parkreiner Oct 20, 2023
15fdfbf
chore: add custom workspace duplication hook
Parkreiner Oct 20, 2023
d007b86
chore: integrate mode into CreateWorkspacePageView
Parkreiner Oct 20, 2023
294156e
fix: add mode to CreateWorkspacePageView stories
Parkreiner Oct 20, 2023
4554895
refactor: extract workspace duplication outside CreateWorkspacePage file
Parkreiner Oct 20, 2023
25bacf2
chore: integrate useWorkspaceDuplication into WorkspaceActions
Parkreiner Oct 20, 2023
0947031
chore: delete unnecessary function
Parkreiner Oct 20, 2023
1d4d4d7
Merge branch 'main' into mes/workspace-clone-feat
Parkreiner Oct 31, 2023
d71acf6
refactor: swap useReducer for useState
Parkreiner Oct 31, 2023
c0a8c56
fix: swap warning alert for info alert
Parkreiner Oct 31, 2023
0b3e954
refactor: move info alert message
Parkreiner Oct 31, 2023
7a763a9
refactor: simplify UI logic for mode alerts
Parkreiner Oct 31, 2023
da488fa
fix: prevent dismissed Alerts from affecting layouts
Parkreiner Oct 31, 2023
5c7242f
fix: remove unnecessary prop binding
Parkreiner Oct 31, 2023
98d1b1b
docs: reword comment for clarity
Parkreiner Oct 31, 2023
aeacda5
chore: update msw build params to return multiple params
Parkreiner Oct 31, 2023
230a4f1
chore: rename duplicationReady to isDuplicationReady
Parkreiner Oct 31, 2023
75b1839
chore: expose root component for testing/re-rendering
Parkreiner Nov 1, 2023
7cf446f
chore: get tests in place (still have act warnings)
Parkreiner Nov 1, 2023
bf21656
refactor: move stuff around for clarity
Parkreiner Nov 1, 2023
38ba3b2
chore: finish tests
Parkreiner Nov 1, 2023
923d080
chore: revamp tests
Parkreiner Nov 3, 2023
8b3d4dd
chore: merge main into branch
Parkreiner Nov 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions site/src/api/queries/workspaceBuilds.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
45 changes: 27 additions & 18 deletions site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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(
Expand Down Expand Up @@ -122,6 +122,7 @@ const CreateWorkspacePage: FC = () => {
<Loader />
) : (
<CreateWorkspacePageView
mode={mode}
defaultName={defaultName}
defaultOwner={me}
defaultBuildParameters={defaultBuildParameters}
Expand Down Expand Up @@ -220,20 +221,6 @@ const getDefaultBuildParameters = (
return buildValues;
};

export const orderedTemplateParameters = (
Copy link
Member Author

@Parkreiner Parkreiner Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't see this function get used anywhere, but also, one concern I have with the function is that even though it groups the mutable/immutable params, they're still all in one array, so you have to iterate through it to see where one group starts, and the other stops

I feel like, if this does need to get added back down the line, I'd consider these changes:

  1. Returning this as an object, so that you know which one is which from a glance
    type Return = {
      immutable: TemplateVersionParameter[];
      mutable: TemplateVersionParameter[];
    }
  2. Maybe use some custom type predicates under the hood, so that TypeScript is able to say that the mutable property is always true inside the mutable array, and always false inside the immutable array

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({
Expand All @@ -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 ?? "";
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const meta: Meta<typeof CreateWorkspacePageView> = {
template: MockTemplate,
parameters: [],
externalAuth: [],
mode: "form",
permissions: {
createWorkspaceForUser: true,
},
Expand Down
Loading