Skip to content

Commit e61491b

Browse files
ammarioammario
committed
Fix one of the e2e tests
1 parent 51cc138 commit e61491b

File tree

9 files changed

+87
-58
lines changed

9 files changed

+87
-58
lines changed

‎site/e2e/parameters.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ export const seventhParameter: RichParameter = {
128128
order: 1,
129129
};
130130

131+
// randParamName returns a new parameter with a random name.
132+
export const randParamName = (p: RichParameter): RichParameter => {
133+
p.name += Math.random().toString(36).substring(7);
134+
return p;
135+
};
136+
131137
// Build options
132138

133139
export const firstBuildOption: RichParameter = {

‎site/e2e/tests/createWorkspace.spec.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { randParamName } from "./../parameters";
12
import { test, expect } from "@playwright/test";
23
import {
34
createTemplate,
@@ -101,10 +102,16 @@ test("create workspace with default and required parameters", async ({
101102
});
102103

103104
test("create workspace and overwrite default parameters", async ({ page }) => {
104-
const richParameters: RichParameter[] = [secondParameter, fourthParameter];
105+
// We use randParamName to prevent the new values from corrupting user_history
106+
// and thus affecting other tests.
107+
const richParameters: RichParameter[] = [
108+
randParamName(secondParameter),
109+
randParamName(fourthParameter),
110+
];
111+
105112
const buildParameters = [
106-
{ name: secondParameter.name, value: "AAAAA" },
107-
{ name: fourthParameter.name, value: "false" },
113+
{ name: richParameters[0].name, value: "AAAAA" },
114+
{ name: richParameters[1].name, value: "false" },
108115
];
109116
const template = await createTemplate(
110117
page,

‎site/src/components/RichParameterInput/RichParameterInput.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { MemoizedMarkdown } from "components/Markdown/Markdown";
1010
import { Stack } from "components/Stack/Stack";
1111
import { MultiTextField } from "./MultiTextField";
1212
import { ExternalImage } from "components/ExternalImage/ExternalImage";
13+
import { AutofillSource } from "utils/richParameters";
1314

1415
const isBoolean = (parameter: TemplateVersionParameter) => {
1516
return parameter.type === "bool";
@@ -137,16 +138,15 @@ export type RichParameterInputProps = Omit<
137138
"size" | "onChange"
138139
> & {
139140
parameter: TemplateVersionParameter;
140-
// defaultReason is commentary on how the default value was determined.
141-
defaultReason?: JSX.Element;
141+
autofillSource?: AutofillSource;
142142
onChange: (value: string) => void;
143143
size?: Size;
144144
};
145145

146146
export const RichParameterInput: FC<RichParameterInputProps> = ({
147147
parameter,
148148
size = "medium",
149-
defaultReason,
149+
autofillSource,
150150
...fieldProps
151151
}) => {
152152
return (
@@ -159,9 +159,15 @@ export const RichParameterInput: FC<RichParameterInputProps> = ({
159159
<ParameterLabel parameter={parameter} />
160160
<div css={{ display: "flex", flexDirection: "column" }}>
161161
<RichParameterField {...fieldProps} size={size} parameter={parameter} />
162-
{defaultReason && (
162+
{autofillSource && (
163163
<div css={{ marginTop: 4, fontSize: 12 }}>
164-
🪄 Autofilled: {defaultReason}
164+
🪄 Autofilled:{" "}
165+
{
166+
{
167+
["url"]: "supplied by URL.",
168+
["user_history"]: "recently used value.",
169+
}[autofillSource]
170+
}
165171
</div>
166172
)}
167173
</div>

‎site/src/components/TemplateParameters/TemplateParameters.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ import {
55
RichParameterInputProps,
66
} from "components/RichParameterInput/RichParameterInput";
77
import { ComponentProps, FC } from "react";
8+
import { AutofillSource } from "utils/richParameters";
89

910
export type TemplateParametersSectionProps = {
1011
templateParameters: TemplateVersionParameter[];
11-
defaultReasons?: Record<string, JSX.Element>;
12+
defaultReasons?: Record<string, AutofillSource>;
1213
getInputProps: (
1314
parameter: TemplateVersionParameter,
1415
index: number,
@@ -42,7 +43,7 @@ export const MutableTemplateParametersSection: FC<
4243
{...getInputProps(parameter, index)}
4344
key={parameter.name}
4445
parameter={parameter}
45-
defaultReason={
46+
autofillSource={
4647
defaultReasons && defaultReasons[parameter.name]
4748
}
4849
/>

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ import { type FC, useCallback, useState, useEffect } from "react";
99
import { Helmet } from "react-helmet-async";
1010
import { useNavigate, useParams, useSearchParams } from "react-router-dom";
1111
import { pageTitle } from "utils/page";
12-
import {
13-
CreateWorkspacePageView,
14-
DefaultBuildParameter,
15-
} from "./CreateWorkspacePageView";
12+
import { CreateWorkspacePageView } from "./CreateWorkspacePageView";
1613
import { Loader } from "components/Loader/Loader";
1714
import { ErrorAlert } from "components/Alert/ErrorAlert";
1815
import {
@@ -33,6 +30,7 @@ import { CreateWSPermissions, createWorkspaceChecks } from "./permissions";
3330
import { paramsUsedToCreateWorkspace } from "utils/workspace";
3431
import { useEffectEvent } from "hooks/hookPolyfills";
3532
import { userParameters } from "api/queries/users";
33+
import { AutofillBuildParameter } from "utils/richParameters";
3634

3735
export const createWorkspaceModes = ["form", "auto", "duplicate"] as const;
3836
export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number];
@@ -94,7 +92,7 @@ const CreateWorkspacePage: FC = () => {
9492
[navigate],
9593
);
9694

97-
const defaultBuildParameters = getDefaultBuildParameters(
95+
const autofillParameters = getAutofillParameters(
9896
searchParams,
9997
userParametersQuery.data ? userParametersQuery.data : [],
10098
);
@@ -104,7 +102,7 @@ const CreateWorkspacePage: FC = () => {
104102
const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({
105103
templateName,
106104
organizationId,
107-
defaultBuildParameters,
105+
defaultBuildParameters: autofillParameters,
108106
defaultName,
109107
versionId: realizedVersionId,
110108
});
@@ -135,7 +133,7 @@ const CreateWorkspacePage: FC = () => {
135133
mode={mode}
136134
defaultName={defaultName}
137135
defaultOwner={me}
138-
defaultBuildParameters={defaultBuildParameters}
136+
autofillParameters={autofillParameters}
139137
error={createWorkspaceMutation.error}
140138
resetMutation={createWorkspaceMutation.reset}
141139
template={templateQuery.data!}
@@ -218,35 +216,31 @@ const useExternalAuth = (versionId: string | undefined) => {
218216
};
219217
};
220218

221-
const getDefaultBuildParameters = (
219+
const getAutofillParameters = (
222220
urlSearchParams: URLSearchParams,
223221
userParameters: UserParameter[],
224-
): DefaultBuildParameter[] => {
222+
): AutofillBuildParameter[] => {
225223
const userParamMap = userParameters.reduce((acc, param) => {
226224
acc.set(param.name, param);
227225
return acc;
228226
}, new Map<string, UserParameter>());
229227

230-
const buildValues: DefaultBuildParameter[] = [];
228+
const buildValues: AutofillBuildParameter[] = [];
231229
Array.from(urlSearchParams.keys())
232230
.filter((key) => key.startsWith("param."))
233231
.forEach((key) => {
234232
const name = key.replace("param.", "");
235233
const value = urlSearchParams.get(key) ?? "";
236234
// URL should take precedence over user parameters
237235
userParamMap.delete(name);
238-
buildValues.push({ name, value, reason: <>supplied by URL</> });
236+
buildValues.push({ name, value, source: "url" });
239237
});
240238

241239
userParamMap.forEach((param) => {
242240
buildValues.push({
243241
name: param.name,
244242
value: param.value,
245-
reason: (
246-
<>
247-
recently used <code>{param.name}</code> value.
248-
</>
249-
),
243+
source: "user_history",
250244
});
251245
});
252246
return buildValues;

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const meta: Meta<typeof CreateWorkspacePageView> = {
1717
args: {
1818
defaultName: "",
1919
defaultOwner: MockUser,
20-
defaultBuildParameters: [],
20+
autofillParameters: [],
2121
template: MockTemplate,
2222
parameters: [],
2323
externalAuth: [],
@@ -86,11 +86,11 @@ export const Parameters: Story = {
8686
ephemeral: false,
8787
},
8888
],
89-
defaultBuildParameters: [
89+
autofillParameters: [
9090
{
9191
name: "first_parameter",
9292
value: "It works!",
93-
reason: <>recently used.</>,
93+
source: "user_history",
9494
},
9595
],
9696
},

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
HorizontalForm,
2121
} from "components/Form/Form";
2222
import {
23+
AutofillBuildParameter,
24+
AutofillSource,
2325
getInitialRichParameterValues,
2426
useValidationSchemaForRichParameters,
2527
} from "utils/richParameters";
@@ -43,10 +45,6 @@ export const Language = {
4345
"Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.",
4446
} as const;
4547

46-
export type DefaultBuildParameter = {
47-
reason: JSX.Element;
48-
} & TypesGen.WorkspaceBuildParameter;
49-
5048
export interface CreateWorkspacePageViewProps {
5149
mode: CreateWorkspaceMode;
5250
error: unknown;
@@ -59,7 +57,7 @@ export interface CreateWorkspacePageViewProps {
5957
externalAuthPollingState: ExternalAuthPollingState;
6058
startPollingExternalAuth: () => void;
6159
parameters: TypesGen.TemplateVersionParameter[];
62-
defaultBuildParameters: DefaultBuildParameter[];
60+
autofillParameters: AutofillBuildParameter[];
6361
permissions: CreateWSPermissions;
6462
creatingWorkspace: boolean;
6563
onCancel: () => void;
@@ -81,7 +79,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
8179
externalAuthPollingState,
8280
startPollingExternalAuth,
8381
parameters,
84-
defaultBuildParameters,
82+
autofillParameters,
8583
permissions,
8684
creatingWorkspace,
8785
onSubmit,
@@ -101,7 +99,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
10199
template_id: template.id,
102100
rich_parameter_values: getInitialRichParameterValues(
103101
parameters,
104-
defaultBuildParameters,
102+
autofillParameters,
105103
),
106104
},
107105
validationSchema: Yup.object({
@@ -129,15 +127,15 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
129127
error,
130128
);
131129

132-
const defaultReasons = useMemo(() => {
133-
return defaultBuildParameters.reduce(
130+
const defaultSources = useMemo(() => {
131+
return autofillParameters.reduce(
134132
(acc, param) => {
135-
acc[param.name] = param.reason;
133+
acc[param.name] = param.source;
136134
return acc;
137135
},
138-
{} as Record<string, JSX.Element>,
136+
{} as Record<string, AutofillSource>,
139137
);
140-
}, [defaultBuildParameters]);
138+
}, [autofillParameters]);
141139

142140
return (
143141
<FullPageHorizontalForm title="New workspace" onCancel={onCancel}>
@@ -225,7 +223,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
225223
{parameters && (
226224
<>
227225
<MutableTemplateParametersSection
228-
defaultReasons={defaultReasons}
226+
defaultReasons={defaultSources}
229227
templateParameters={parameters}
230228
getInputProps={(parameter, index) => {
231229
return {
@@ -246,7 +244,7 @@ export const CreateWorkspacePageView: FC<CreateWorkspacePageViewProps> = ({
246244
}}
247245
/>
248246
<ImmutableTemplateParametersSection
249-
defaultReasons={defaultReasons}
247+
defaultReasons={defaultSources}
250248
templateParameters={parameters}
251249
classes={{
252250
root: css`

‎site/src/utils/richParameters.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ test("getInitialRichParameterValues return default value when default build para
4646
const cpuParameter = templateParameters[0];
4747
const [cpuParameterInitialValue] = getInitialRichParameterValues(
4848
templateParameters,
49-
[{ name: cpuParameter.name, value: "100" }],
49+
[{ name: cpuParameter.name, value: "100", source: "user_history" }],
5050
);
5151

5252
expect(cpuParameterInitialValue.value).toBe(cpuParameter.default_value);

‎site/src/utils/richParameters.ts

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,42 @@ import {
44
} from "api/typesGenerated";
55
import * as Yup from "yup";
66

7+
export type AutofillSource = "user_history" | "url";
8+
9+
export type AutofillBuildParameter = {
10+
source: AutofillSource;
11+
} & WorkspaceBuildParameter;
12+
713
export const getInitialRichParameterValues = (
8-
templateParameters: TemplateVersionParameter[],
9-
buildParameters?: WorkspaceBuildParameter[],
14+
templateParams: TemplateVersionParameter[],
15+
autofillParams?: AutofillBuildParameter[],
1016
): WorkspaceBuildParameter[] => {
11-
return templateParameters.map((parameter) => {
12-
const existentBuildParameter = buildParameters?.find(
17+
return templateParams.map((parameter) => {
18+
const autofillParam = autofillParams?.find(
1319
(p) => p.name === parameter.name,
1420
);
15-
const shouldReturnTheDefaultValue =
16-
!existentBuildParameter ||
17-
!isValidValue(parameter, existentBuildParameter) ||
18-
parameter.ephemeral;
19-
if (shouldReturnTheDefaultValue) {
20-
return {
21-
name: parameter.name,
22-
value: parameter.default_value,
23-
};
21+
if (autofillParam !== undefined && isValidValue(parameter, autofillParam)) {
22+
// URL takes precedence over all other sources.
23+
if (autofillParam.source === "url") {
24+
return {
25+
name: parameter.name,
26+
value: autofillParam.value,
27+
};
28+
}
29+
30+
// Need to decide whether user_history is more important than default value.
31+
if (autofillParam.source === "user_history") {
32+
return {
33+
name: parameter.name,
34+
value: autofillParam.value,
35+
};
36+
}
2437
}
25-
return existentBuildParameter;
38+
39+
return {
40+
name: parameter.name,
41+
value: parameter.default_value,
42+
};
2643
});
2744
};
2845

0 commit comments

Comments
 (0)