Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(site): support match option for auto create workspace flow#13836

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
BrunoQuaresma merged 6 commits intomainfrombq/support-match
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
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
2 changes: 1 addition & 1 deletionsite/e2e/parameters.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,7 +2,7 @@ import type { RichParameter } from "./provisionerGenerated";

// Rich parameters

const emptyParameter: RichParameter = {
exportconst emptyParameter: RichParameter = {
name: "",
description: "",
type: "",
Expand Down
65 changes: 65 additions & 0 deletionssite/e2e/tests/workspaces/autoCreateWorkspace.spec.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
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&param.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(
Copy link
Member

Choose a reason for hiding this comment

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

I haven't worked with the E2E stuff much yet, so just to make sure I'm understanding: these testsaren't isolated, right? That's how the second test is able to have a workspace already exist, even though it didn't explicitly make two of them?

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

They are not isolated, but I ensure that a previous workspace with the right match exists in line 39.

page,
echoResponsesWithParameters(richParameters),
);
const prevWorkspace = await createWorkspace(page, template);
await page.goto(
`/templates/${template}/workspace?mode=auto&param.repo=example&name=new-name&match=name:${prevWorkspace}`,
{
waitUntil: "domcontentloaded",
},
);
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&param.repo=example&name=new-name&match=not-valid-query:${prevWorkspace}`,
{
waitUntil: "domcontentloaded",
},
);
await expect(page.getByText("Invalid match value")).toBeVisible();
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ import {
openTerminalWindow,
requireTerraformProvisioner,
verifyParameters,
} from "../helpers";
import { beforeCoderTest } from "../hooks";
} from "../../helpers";
import { beforeCoderTest } from "../../hooks";
import {
secondParameter,
fourthParameter,
Expand All@@ -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));

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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));

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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));

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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));

Expand Down
13 changes: 13 additions & 0 deletionssite/src/api/errors.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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.";
}
Expand All@@ -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);
}
}
72 changes: 56 additions & 16 deletionssite/src/api/queries/workspaces.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand DownExpand Up@@ -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;
Expand All@@ -61,19 +54,45 @@ export const createWorkspace = (queryClient: QueryClient) => {
};
};

type AutoCreateWorkspaceOptions = {
organizationId: string;
templateName: 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
* 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;
templateVersionId?: string;
buildParameters?: WorkspaceBuildParameter[];
};

export const autoCreateWorkspace = (queryClient: QueryClient) => {
return {
mutationFn: async ({
templateName,
versionId,
organizationId,
defaultBuildParameters,
defaultName,
templateName,
workspaceName,
templateVersionId,
buildParameters,
match,
}: AutoCreateWorkspaceOptions) => {
if (match) {
const matchWorkspace = await findMatchWorkspace(
`owner:me template:${templateName} ${match}`,
);
if (matchWorkspace) {
return matchWorkspace;
}
}

let templateVersionParameters;

if (versionId) {
templateVersionParameters = { template_version_id:versionId };
if (templateVersionId) {
templateVersionParameters = { template_version_id:templateVersionId };
} else {
const template = await API.getTemplateByName(
organizationId,
Expand All@@ -84,8 +103,8 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => {

return API.createWorkspace(organizationId, "me", {
...templateVersionParameters,
name:defaultName,
rich_parameter_values:defaultBuildParameters,
name:workspaceName,
rich_parameter_values:buildParameters,
});
},
onSuccess: async () => {
Expand All@@ -94,6 +113,27 @@ export const autoCreateWorkspace = (queryClient: QueryClient) => {
};
};

async function findMatchWorkspace(q: string): Promise<Workspace | undefined> {
try {
const { workspaces } = await API.getWorkspaces({ q, limit: 1 });
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;
Expand Down
21 changes: 12 additions & 9 deletionssite/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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";
Expand All@@ -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;
Expand DownExpand Up@@ -118,15 +117,15 @@ const CreateWorkspacePage: FC = () => {
const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({
templateName,
organizationId,
defaultBuildParameters: autofillParameters,
defaultName: defaultName ?? generateWorkspaceName(),
versionId: realizedVersionId,
buildParameters: autofillParameters,
workspaceName: defaultName ?? generateWorkspaceName(),
templateVersionId: realizedVersionId,
match: searchParams.get("match"),
});

onCreateWorkspace(newWorkspace);
} catch (err) {
searchParams.delete("mode");
setSearchParams(searchParams);
setMode("form");
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if it might still be good to update the search params on top of the state change, but good catch with this

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

I've thought about this and I don't think it makes much of a difference in the user experience, based on what I can see. Since no other part of the component uses it, I just removed it and decided to set the mode only in the state.

}
});

Expand DownExpand Up@@ -175,7 +174,6 @@ const CreateWorkspacePage: FC = () => {
<Helmet>
<title>{pageTitle(title)}</title>
</Helmet>
{loadFormDataError && <ErrorAlert error={loadFormDataError} />}
{isLoadingFormData || isLoadingExternalAuth || autoCreateReady ? (
<Loader />
) : (
Expand All@@ -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}
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp