From 48d0e39c625d322f977be532b097b133c8ba5a17 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 8 Jul 2024 19:46:25 +0000 Subject: [PATCH 1/6] Add support for match in the auto create flow --- site/src/api/errors.ts | 13 ++++ site/src/api/queries/workspaces.ts | 66 +++++++++++++++---- .../CreateWorkspacePage.tsx | 19 +++--- 3 files changed, 77 insertions(+), 21 deletions(-) diff --git a/site/src/api/errors.ts b/site/src/api/errors.ts index 621b19856601b..8f69e06fc4dc0 100644 --- a/site/src/api/errors.ts +++ b/site/src/api/errors.ts @@ -111,6 +111,10 @@ export const getValidationErrorMessage = (error: unknown): string => { }; export const getErrorDetail = (error: unknown): string | undefined => { + if (error instanceof DetailedError) { + return error.detail; + } + if (error instanceof Error) { return "Please check the developer console for more details."; } @@ -125,3 +129,12 @@ export const getErrorDetail = (error: unknown): string | undefined => { return undefined; }; + +export class DetailedError extends Error { + constructor( + message: string, + public detail?: string, + ) { + super(message); + } +} diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index aa5f8f29a9783..ccb817dffec68 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -5,6 +5,7 @@ import type { UseMutationOptions, } from "react-query"; import { type DeleteWorkspaceOptions, API } from "api/api"; +import { DetailedError, isApiValidationError } from "api/errors"; import type { CreateWorkspaceRequest, ProvisionerLogLevel, @@ -36,14 +37,6 @@ export const workspaceByOwnerAndName = (owner: string, name: string) => { }; }; -type AutoCreateWorkspaceOptions = { - templateName: string; - versionId?: string; - organizationId: string; - defaultBuildParameters?: WorkspaceBuildParameter[]; - defaultName: string; -}; - type CreateWorkspaceMutationVariables = CreateWorkspaceRequest & { userId: string; organizationId: string; @@ -61,15 +54,41 @@ export const createWorkspace = (queryClient: QueryClient) => { }; }; +type AutoCreateWorkspaceOptions = { + organizationId: string; + templateName: string; + name: string; + /** + * If provided, the auto-create workspace feature will attempt to find a + * matching workspace. If found, it will return the existing workspace instead + * of creating a new one. Its value supports [advanced filtering queries for + * workspaces](https://coder.com/docs/workspaces#workspace-filtering). If + * multiple values are returned, the first one will be returned. + */ + match: string | null; + versionId?: string; + buildParameters?: WorkspaceBuildParameter[]; +}; + export const autoCreateWorkspace = (queryClient: QueryClient) => { return { mutationFn: async ({ + organizationId, templateName, + name, versionId, - organizationId, - defaultBuildParameters, - defaultName, + buildParameters, + match, }: AutoCreateWorkspaceOptions) => { + if (match) { + const matchWorkspace = await findMatchWorkspace( + `owner:me template:${templateName} ${match}`, + ); + if (matchWorkspace) { + return matchWorkspace; + } + } + let templateVersionParameters; if (versionId) { @@ -84,8 +103,8 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { return API.createWorkspace(organizationId, "me", { ...templateVersionParameters, - name: defaultName, - rich_parameter_values: defaultBuildParameters, + name, + rich_parameter_values: buildParameters, }); }, onSuccess: async () => { @@ -94,6 +113,27 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { }; }; +async function findMatchWorkspace(q: string): Promise { + try { + const { workspaces } = await API.getWorkspaces({ q }); + const matchWorkspace = workspaces.at(0); + if (matchWorkspace) { + return matchWorkspace; + } + } catch (err) { + if (isApiValidationError(err)) { + const firstValidationErrorDetail = + err.response.data.validations?.[0].detail; + throw new DetailedError( + "Invalid match value", + firstValidationErrorDetail, + ); + } + + throw err; + } +} + export function workspacesKey(config: WorkspacesRequest = {}) { const { q, limit } = config; return ["workspaces", { q, limit }] as const; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index c2cd9ab9da3ae..49a9248876760 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -16,7 +16,6 @@ import type { UserParameter, Workspace, } from "api/typesGenerated"; -import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Loader } from "components/Loader/Loader"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { useEffectEvent } from "hooks/hookPolyfills"; @@ -37,7 +36,7 @@ const CreateWorkspacePage: FC = () => { const { template: templateName } = useParams() as { template: string }; const { user: me } = useAuthenticated(); const navigate = useNavigate(); - const [searchParams, setSearchParams] = useSearchParams(); + const [searchParams] = useSearchParams(); const { experiments, organizationId } = useDashboard(); const customVersionId = searchParams.get("version") ?? undefined; @@ -118,15 +117,15 @@ const CreateWorkspacePage: FC = () => { const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({ templateName, organizationId, - defaultBuildParameters: autofillParameters, - defaultName: defaultName ?? generateWorkspaceName(), + buildParameters: autofillParameters, + name: defaultName ?? generateWorkspaceName(), versionId: realizedVersionId, + match: searchParams.get("match"), }); onCreateWorkspace(newWorkspace); } catch (err) { - searchParams.delete("mode"); - setSearchParams(searchParams); + setMode("form"); } }); @@ -175,7 +174,6 @@ const CreateWorkspacePage: FC = () => { {pageTitle(title)} - {loadFormDataError && } {isLoadingFormData || isLoadingExternalAuth || autoCreateReady ? ( ) : ( @@ -185,7 +183,12 @@ const CreateWorkspacePage: FC = () => { disabledParams={disabledParams} defaultOwner={me} autofillParameters={autofillParameters} - error={createWorkspaceMutation.error || autoCreateError} + error={ + createWorkspaceMutation.error || + autoCreateError || + loadFormDataError || + autoCreateWorkspaceMutation.error + } resetMutation={createWorkspaceMutation.reset} template={templateQuery.data!} versionId={realizedVersionId} From 6b6b32c5861c8d358caa01128b5712ca90fd4cb4 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 8 Jul 2024 20:23:32 +0000 Subject: [PATCH 2/6] Add e2e tests --- site/e2e/parameters.ts | 2 +- .../workspaces/autoCreateWorkspace.spec.ts | 47 +++++++++++++++++++ .../{ => workspaces}/createWorkspace.spec.ts | 8 ++-- .../{ => workspaces}/restartWorkspace.spec.ts | 8 ++-- .../{ => workspaces}/startWorkspace.spec.ts | 8 ++-- .../{ => workspaces}/updateWorkspace.spec.ts | 8 ++-- 6 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts rename site/e2e/tests/{ => workspaces}/createWorkspace.spec.ts (96%) rename site/e2e/tests/{ => workspaces}/restartWorkspace.spec.ts (87%) rename site/e2e/tests/{ => workspaces}/startWorkspace.spec.ts (87%) rename site/e2e/tests/{ => workspaces}/updateWorkspace.spec.ts (96%) diff --git a/site/e2e/parameters.ts b/site/e2e/parameters.ts index f7e02ad20f1ee..23f953a49e2a8 100644 --- a/site/e2e/parameters.ts +++ b/site/e2e/parameters.ts @@ -2,7 +2,7 @@ import type { RichParameter } from "./provisionerGenerated"; // Rich parameters -const emptyParameter: RichParameter = { +export const emptyParameter: RichParameter = { name: "", description: "", type: "", diff --git a/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts b/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts new file mode 100644 index 0000000000000..77eebb4c1f296 --- /dev/null +++ b/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts @@ -0,0 +1,47 @@ +import { test, expect } from "@playwright/test"; +import { username } from "../../constants"; +import { + createTemplate, + createWorkspace, + echoResponsesWithParameters, +} from "../../helpers"; +import { emptyParameter } from "../../parameters"; +import type { RichParameter } from "../../provisionerGenerated"; + +test("create workspace in auto mode", async ({ page }) => { + const richParameters: RichParameter[] = [ + { ...emptyParameter, name: "repo", type: "string" }, + ]; + const template = await createTemplate( + page, + echoResponsesWithParameters(richParameters), + ); + const name = "test-workspace"; + await page.goto( + `/templates/${template}/workspace?mode=auto¶m.repo=example&name=${name}`, + { + waitUntil: "domcontentloaded", + }, + ); + await expect(page).toHaveTitle(`${username}/${name} - Coder`); +}); + +test("use an existing workspace that matches the `match` parameter instead of creating a new one", async ({ + page, +}) => { + const richParameters: RichParameter[] = [ + { ...emptyParameter, name: "repo", type: "string" }, + ]; + const template = await createTemplate( + page, + echoResponsesWithParameters(richParameters), + ); + const prevWorkspace = await createWorkspace(page, template); + await page.goto( + `/templates/${template}/workspace?mode=auto¶m.repo=example&name=new-name&match=name:${prevWorkspace}`, + { + waitUntil: "domcontentloaded", + }, + ); + await expect(page).toHaveTitle(`${username}/${prevWorkspace} - Coder`); +}); diff --git a/site/e2e/tests/createWorkspace.spec.ts b/site/e2e/tests/workspaces/createWorkspace.spec.ts similarity index 96% rename from site/e2e/tests/createWorkspace.spec.ts rename to site/e2e/tests/workspaces/createWorkspace.spec.ts index 5f1713b60aaa7..affec154add06 100644 --- a/site/e2e/tests/createWorkspace.spec.ts +++ b/site/e2e/tests/workspaces/createWorkspace.spec.ts @@ -7,8 +7,8 @@ import { openTerminalWindow, requireTerraformProvisioner, verifyParameters, -} from "../helpers"; -import { beforeCoderTest } from "../hooks"; +} from "../../helpers"; +import { beforeCoderTest } from "../../hooks"; import { secondParameter, fourthParameter, @@ -18,8 +18,8 @@ import { seventhParameter, sixthParameter, randParamName, -} from "../parameters"; -import type { RichParameter } from "../provisionerGenerated"; +} from "../../parameters"; +import type { RichParameter } from "../../provisionerGenerated"; test.beforeEach(({ page }) => beforeCoderTest(page)); diff --git a/site/e2e/tests/restartWorkspace.spec.ts b/site/e2e/tests/workspaces/restartWorkspace.spec.ts similarity index 87% rename from site/e2e/tests/restartWorkspace.spec.ts rename to site/e2e/tests/workspaces/restartWorkspace.spec.ts index 0da42b1257d6a..9b45ffe3371a5 100644 --- a/site/e2e/tests/restartWorkspace.spec.ts +++ b/site/e2e/tests/workspaces/restartWorkspace.spec.ts @@ -5,10 +5,10 @@ import { createWorkspace, echoResponsesWithParameters, verifyParameters, -} from "../helpers"; -import { beforeCoderTest } from "../hooks"; -import { firstBuildOption, secondBuildOption } from "../parameters"; -import type { RichParameter } from "../provisionerGenerated"; +} from "../../helpers"; +import { beforeCoderTest } from "../../hooks"; +import { firstBuildOption, secondBuildOption } from "../../parameters"; +import type { RichParameter } from "../../provisionerGenerated"; test.beforeEach(({ page }) => beforeCoderTest(page)); diff --git a/site/e2e/tests/startWorkspace.spec.ts b/site/e2e/tests/workspaces/startWorkspace.spec.ts similarity index 87% rename from site/e2e/tests/startWorkspace.spec.ts rename to site/e2e/tests/workspaces/startWorkspace.spec.ts index eb180c3df4ff9..37f4766558e10 100644 --- a/site/e2e/tests/startWorkspace.spec.ts +++ b/site/e2e/tests/workspaces/startWorkspace.spec.ts @@ -6,10 +6,10 @@ import { echoResponsesWithParameters, stopWorkspace, verifyParameters, -} from "../helpers"; -import { beforeCoderTest } from "../hooks"; -import { firstBuildOption, secondBuildOption } from "../parameters"; -import type { RichParameter } from "../provisionerGenerated"; +} from "../../helpers"; +import { beforeCoderTest } from "../../hooks"; +import { firstBuildOption, secondBuildOption } from "../../parameters"; +import type { RichParameter } from "../../provisionerGenerated"; test.beforeEach(({ page }) => beforeCoderTest(page)); diff --git a/site/e2e/tests/updateWorkspace.spec.ts b/site/e2e/tests/workspaces/updateWorkspace.spec.ts similarity index 96% rename from site/e2e/tests/updateWorkspace.spec.ts rename to site/e2e/tests/workspaces/updateWorkspace.spec.ts index 40b62aa1fc091..5d7957e29a9ea 100644 --- a/site/e2e/tests/updateWorkspace.spec.ts +++ b/site/e2e/tests/workspaces/updateWorkspace.spec.ts @@ -7,16 +7,16 @@ import { updateWorkspace, updateWorkspaceParameters, verifyParameters, -} from "../helpers"; -import { beforeCoderTest } from "../hooks"; +} from "../../helpers"; +import { beforeCoderTest } from "../../hooks"; import { fifthParameter, firstParameter, secondParameter, sixthParameter, secondBuildOption, -} from "../parameters"; -import type { RichParameter } from "../provisionerGenerated"; +} from "../../parameters"; +import type { RichParameter } from "../../provisionerGenerated"; test.beforeEach(({ page }) => beforeCoderTest(page)); From d9cb7ba2a29cda0f9f46d73cd11f2e024a05b6eb Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 8 Jul 2024 20:26:19 +0000 Subject: [PATCH 3/6] Add test to check if invalid match erros are displayed --- .../workspaces/autoCreateWorkspace.spec.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts b/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts index 77eebb4c1f296..3a9aaee2eeb3c 100644 --- a/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts +++ b/site/e2e/tests/workspaces/autoCreateWorkspace.spec.ts @@ -45,3 +45,21 @@ test("use an existing workspace that matches the `match` parameter instead of cr ); await expect(page).toHaveTitle(`${username}/${prevWorkspace} - Coder`); }); + +test("show error if `match` parameter is invalid", async ({ page }) => { + const richParameters: RichParameter[] = [ + { ...emptyParameter, name: "repo", type: "string" }, + ]; + const template = await createTemplate( + page, + echoResponsesWithParameters(richParameters), + ); + const prevWorkspace = await createWorkspace(page, template); + await page.goto( + `/templates/${template}/workspace?mode=auto¶m.repo=example&name=new-name&match=not-valid-query:${prevWorkspace}`, + { + waitUntil: "domcontentloaded", + }, + ); + await expect(page.getByText("Invalid match value")).toBeVisible(); +}); From a64cdcdce01e408050f72608bf8148ef0b55fb17 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 8 Jul 2024 20:29:46 +0000 Subject: [PATCH 4/6] Optimize match query --- site/src/api/queries/workspaces.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index ccb817dffec68..dc81e54dc9dc6 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -115,7 +115,7 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { async function findMatchWorkspace(q: string): Promise { try { - const { workspaces } = await API.getWorkspaces({ q }); + const { workspaces } = await API.getWorkspaces({ q, limit: 1 }); const matchWorkspace = workspaces.at(0); if (matchWorkspace) { return matchWorkspace; From dac8ada11786e5afb03d61b57cbe295a4f78f9a2 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 9 Jul 2024 16:02:17 +0000 Subject: [PATCH 5/6] Improve parameters name --- site/src/api/queries/workspaces.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index dc81e54dc9dc6..71ac8c055f64f 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -57,7 +57,7 @@ export const createWorkspace = (queryClient: QueryClient) => { type AutoCreateWorkspaceOptions = { organizationId: string; templateName: string; - name: string; + workspaceName: string; /** * If provided, the auto-create workspace feature will attempt to find a * matching workspace. If found, it will return the existing workspace instead @@ -66,7 +66,7 @@ type AutoCreateWorkspaceOptions = { * multiple values are returned, the first one will be returned. */ match: string | null; - versionId?: string; + templateVersionId?: string; buildParameters?: WorkspaceBuildParameter[]; }; @@ -75,8 +75,8 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { mutationFn: async ({ organizationId, templateName, - name, - versionId, + workspaceName, + templateVersionId, buildParameters, match, }: AutoCreateWorkspaceOptions) => { @@ -91,8 +91,8 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { let templateVersionParameters; - if (versionId) { - templateVersionParameters = { template_version_id: versionId }; + if (templateVersionId) { + templateVersionParameters = { template_version_id: templateVersionId }; } else { const template = await API.getTemplateByName( organizationId, @@ -103,7 +103,7 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => { return API.createWorkspace(organizationId, "me", { ...templateVersionParameters, - name, + name: workspaceName, rich_parameter_values: buildParameters, }); }, From 15846959faf691b5706ae5f3ca9fdf82fc0b0eca Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 9 Jul 2024 16:06:22 +0000 Subject: [PATCH 6/6] Fix wrong usage --- site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 49a9248876760..fd7182fe6fbb6 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -118,8 +118,8 @@ const CreateWorkspacePage: FC = () => { templateName, organizationId, buildParameters: autofillParameters, - name: defaultName ?? generateWorkspaceName(), - versionId: realizedVersionId, + workspaceName: defaultName ?? generateWorkspaceName(), + templateVersionId: realizedVersionId, match: searchParams.get("match"), });