Skip to content

Commit 3cf235c

Browse files
BrunoQuaresmaBrunoQuaresma
authored
feat(site): Ask for parameter values when updating a workspace (#6586)
1 parent f91b3ac commit 3cf235c

File tree

15 files changed

+689
-278
lines changed

15 files changed

+689
-278
lines changed

‎site/src/api/api.test.ts

+45-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import axios from "axios"
2-
import { getApiKey, getURLWithSearchParams, login, logout } from "./api"
2+
import {
3+
MockTemplate,
4+
MockTemplateVersionParameter1,
5+
MockWorkspace,
6+
MockWorkspaceBuild,
7+
} from "testHelpers/entities"
8+
import * as api from "./api"
39
import * as TypesGen from "./typesGenerated"
410

511
describe("api.ts", () => {
@@ -12,7 +18,7 @@ describe("api.ts", () => {
1218
jest.spyOn(axios, "post").mockResolvedValueOnce({ data: loginResponse })
1319

1420
// when
15-
const result = await login("test", "123")
21+
const result = await api.login("test", "123")
1622

1723
// then
1824
expect(axios.post).toHaveBeenCalled()
@@ -33,7 +39,7 @@ describe("api.ts", () => {
3339
axios.post = axiosMockPost
3440

3541
try {
36-
await login("test", "123")
42+
await api.login("test", "123")
3743
} catch (error) {
3844
expect(error).toStrictEqual(expectedError)
3945
}
@@ -49,7 +55,7 @@ describe("api.ts", () => {
4955
axios.post = axiosMockPost
5056

5157
// when
52-
await logout()
58+
await api.logout()
5359

5460
// then
5561
expect(axiosMockPost).toHaveBeenCalled()
@@ -68,7 +74,7 @@ describe("api.ts", () => {
6874
axios.post = axiosMockPost
6975

7076
try {
71-
await logout()
77+
await api.logout()
7278
} catch (error) {
7379
expect(error).toStrictEqual(expectedError)
7480
}
@@ -87,7 +93,7 @@ describe("api.ts", () => {
8793
axios.post = axiosMockPost
8894

8995
// when
90-
const result = await getApiKey()
96+
const result = await api.getApiKey()
9197

9298
// then
9399
expect(axiosMockPost).toHaveBeenCalled()
@@ -107,7 +113,7 @@ describe("api.ts", () => {
107113
axios.post = axiosMockPost
108114

109115
try {
110-
await getApiKey()
116+
await api.getApiKey()
111117
} catch (error) {
112118
expect(error).toStrictEqual(expectedError)
113119
}
@@ -133,7 +139,7 @@ describe("api.ts", () => {
133139
])(
134140
`Workspaces - getURLWithSearchParams(%p, %p) returns %p`,
135141
(basePath, filter, expected) => {
136-
expect(getURLWithSearchParams(basePath, filter)).toBe(expected)
142+
expect(api.getURLWithSearchParams(basePath, filter)).toBe(expected)
137143
},
138144
)
139145
})
@@ -150,8 +156,38 @@ describe("api.ts", () => {
150156
])(
151157
`Users - getURLWithSearchParams(%p, %p) returns %p`,
152158
(basePath, filter, expected) => {
153-
expect(getURLWithSearchParams(basePath, filter)).toBe(expected)
159+
expect(api.getURLWithSearchParams(basePath, filter)).toBe(expected)
154160
},
155161
)
156162
})
163+
164+
describe("update", () => {
165+
it("creates a build with start and the latest template", async () => {
166+
jest
167+
.spyOn(api, "postWorkspaceBuild")
168+
.mockResolvedValueOnce(MockWorkspaceBuild)
169+
jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
170+
await api.updateWorkspace(MockWorkspace)
171+
expect(api.postWorkspaceBuild).toHaveBeenCalledWith(MockWorkspace.id, {
172+
transition: "start",
173+
template_version_id: MockTemplate.active_version_id,
174+
rich_parameter_values: [],
175+
})
176+
})
177+
178+
it("fails when having missing parameters", async () => {
179+
jest
180+
.spyOn(api, "postWorkspaceBuild")
181+
.mockResolvedValueOnce(MockWorkspaceBuild)
182+
jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate)
183+
jest.spyOn(api, "getWorkspaceBuildParameters").mockResolvedValueOnce([])
184+
jest
185+
.spyOn(api, "getTemplateVersionRichParameters")
186+
.mockResolvedValueOnce([MockTemplateVersionParameter1])
187+
188+
await expect(api.updateWorkspace(MockWorkspace)).rejects.toThrow(
189+
api.MissingBuildParameters,
190+
)
191+
})
192+
})
157193
})

‎site/src/api/api.ts

+79
Original file line numberDiff line numberDiff line change
@@ -897,3 +897,82 @@ export const getWorkspaceBuildParameters = async (
897897
)
898898
return response.data
899899
}
900+
901+
export class MissingBuildParameters extends Error {
902+
parameters: TypesGen.TemplateVersionParameter[] = []
903+
904+
constructor(parameters: TypesGen.TemplateVersionParameter[]) {
905+
super("Missing build parameters.")
906+
this.parameters = parameters
907+
}
908+
}
909+
910+
/** Steps to update the workspace
911+
* - Get the latest template to access the latest active version
912+
* - Get the current build parameters
913+
* - Get the template parameters
914+
* - Update the build parameters and check if there are missed parameters for the newest version
915+
* - If there are missing parameters raise an error
916+
* - Create a build with the latest version and updated build parameters
917+
*/
918+
export const updateWorkspace = async (
919+
workspace: TypesGen.Workspace,
920+
newBuildParameters: TypesGen.WorkspaceBuildParameter[] = [],
921+
): Promise<TypesGen.WorkspaceBuild> => {
922+
const [template, oldBuildParameters] = await Promise.all([
923+
getTemplate(workspace.template_id),
924+
getWorkspaceBuildParameters(workspace.latest_build.id),
925+
])
926+
const activeVersionId = template.active_version_id
927+
const templateParameters = await getTemplateVersionRichParameters(
928+
activeVersionId,
929+
)
930+
const [updatedBuildParameters, missingParameters] = updateBuildParameters(
931+
oldBuildParameters,
932+
newBuildParameters,
933+
templateParameters,
934+
)
935+
if (missingParameters.length > 0) {
936+
throw new MissingBuildParameters(missingParameters)
937+
}
938+
939+
return postWorkspaceBuild(workspace.id, {
940+
transition: "start",
941+
template_version_id: activeVersionId,
942+
rich_parameter_values: updatedBuildParameters,
943+
})
944+
}
945+
946+
const updateBuildParameters = (
947+
oldBuildParameters: TypesGen.WorkspaceBuildParameter[],
948+
newBuildParameters: TypesGen.WorkspaceBuildParameter[],
949+
templateParameters: TypesGen.TemplateVersionParameter[],
950+
) => {
951+
const missingParameters: TypesGen.TemplateVersionParameter[] = []
952+
const updatedBuildParameters: TypesGen.WorkspaceBuildParameter[] = []
953+
954+
for (const parameter of templateParameters) {
955+
// Check if there is a new value
956+
let buildParameter = newBuildParameters.find(
957+
(p) => p.name === parameter.name,
958+
)
959+
960+
// If not, get the old one
961+
if (!buildParameter) {
962+
buildParameter = oldBuildParameters.find((p) => p.name === parameter.name)
963+
}
964+
965+
// If there is a value from the new or old one, add it to the list
966+
if (buildParameter) {
967+
updatedBuildParameters.push(buildParameter)
968+
continue
969+
}
970+
971+
// If there is no value and it is required, add it to the list of missing parameters
972+
if (parameter.required) {
973+
missingParameters.push(parameter)
974+
}
975+
}
976+
977+
return [updatedBuildParameters, missingParameters] as const
978+
}

‎site/src/components/HorizontalForm/HorizontalForm.tsx renamed to ‎site/src/components/Form/Form.tsx

+77-19
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,84 @@ import {
44
FormFooter as BaseFormFooter,
55
} from "components/FormFooter/FormFooter"
66
import { Stack } from "components/Stack/Stack"
7-
import { FC, HTMLProps, PropsWithChildren } from "react"
7+
import {
8+
createContext,
9+
FC,
10+
HTMLProps,
11+
PropsWithChildren,
12+
useContext,
13+
} from "react"
814
import { combineClasses } from "util/combineClasses"
915

10-
export const HorizontalForm: FC<
11-
PropsWithChildren & HTMLProps<HTMLFormElement>
12-
> = ({ children, ...formProps }) => {
16+
type FormContextValue = { direction?: "horizontal" | "vertical" }
17+
18+
const FormContext = createContext<FormContextValue>({
19+
direction: "horizontal",
20+
})
21+
22+
type FormProps = HTMLProps<HTMLFormElement> & {
23+
direction?: FormContextValue["direction"]
24+
}
25+
26+
export const Form: FC<FormProps> = ({ direction, className, ...formProps }) => {
1327
const styles = useStyles()
1428

1529
return (
16-
<form {...formProps}>
17-
<Stack direction="column" spacing={10} className={styles.formSections}>
18-
{children}
19-
</Stack>
20-
</form>
30+
<FormContext.Provider value={{ direction }}>
31+
<form
32+
{...formProps}
33+
className={combineClasses([styles.form, className])}
34+
/>
35+
</FormContext.Provider>
36+
)
37+
}
38+
39+
export const HorizontalForm: FC<HTMLProps<HTMLFormElement>> = ({
40+
children,
41+
...formProps
42+
}) => {
43+
return (
44+
<Form direction="horizontal" {...formProps}>
45+
{children}
46+
</Form>
47+
)
48+
}
49+
50+
export const VerticalForm: FC<HTMLProps<HTMLFormElement>> = ({
51+
children,
52+
...formProps
53+
}) => {
54+
return (
55+
<Form direction="vertical" {...formProps}>
56+
{children}
57+
</Form>
2158
)
2259
}
2360

2461
export const FormSection: FC<
2562
PropsWithChildren & {
26-
title: string
63+
title: string | JSX.Element
2764
description: string | JSX.Element
28-
className?: string
65+
classes?: {
66+
root?: string
67+
infoTitle?: string
68+
}
2969
}
30-
> = ({ children, title, description, className }) => {
31-
const styles = useStyles()
70+
> = ({ children, title, description, classes = {} }) => {
71+
const formContext = useContext(FormContext)
72+
const styles = useStyles(formContext)
3273

3374
return (
34-
<div className={combineClasses([styles.formSection, className])}>
75+
<div className={combineClasses([styles.formSection, classes.root])}>
3576
<div className={styles.formSectionInfo}>
36-
<h2 className={styles.formSectionInfoTitle}>{title}</h2>
77+
<h2
78+
className={combineClasses([
79+
styles.formSectionInfoTitle,
80+
classes.infoTitle,
81+
])}
82+
>
83+
{title}
84+
</h2>
3785
<div className={styles.formSectionInfoDescription}>{description}</div>
3886
</div>
3987

@@ -62,7 +110,12 @@ export const FormFooter: FC<BaseFormFooterProps> = (props) => {
62110
}
63111

64112
const useStyles = makeStyles((theme) => ({
65-
formSections: {
113+
form: {
114+
display: "flex",
115+
flexDirection: "column",
116+
gap: ({ direction }: FormContextValue = {}) =>
117+
direction === "horizontal" ? theme.spacing(10) : theme.spacing(5),
118+
66119
[theme.breakpoints.down("sm")]: {
67120
gap: theme.spacing(8),
68121
},
@@ -71,7 +124,10 @@ const useStyles = makeStyles((theme) => ({
71124
formSection: {
72125
display: "flex",
73126
alignItems: "flex-start",
74-
gap: theme.spacing(15),
127+
gap: ({ direction }: FormContextValue = {}) =>
128+
direction === "horizontal" ? theme.spacing(15) : theme.spacing(3),
129+
flexDirection: ({ direction }: FormContextValue = {}) =>
130+
direction === "horizontal" ? "row" : "column",
75131

76132
[theme.breakpoints.down("sm")]: {
77133
flexDirection: "column",
@@ -80,9 +136,11 @@ const useStyles = makeStyles((theme) => ({
80136
},
81137

82138
formSectionInfo: {
83-
width: 312,
139+
maxWidth: ({ direction }: FormContextValue = {}) =>
140+
direction === "horizontal" ? 312 : undefined,
84141
flexShrink: 0,
85-
position: "sticky",
142+
position: ({ direction }: FormContextValue = {}) =>
143+
direction === "horizontal" ? "sticky" : undefined,
86144
top: theme.spacing(3),
87145

88146
[theme.breakpoints.down("sm")]: {

‎site/src/i18n/en/workspacePage.json

+3
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,8 @@
6565
"agentVersionLabel": "Agent version",
6666
"serverVersionLabel": "Server version",
6767
"updateWorkspaceLabel": "Update workspace"
68+
},
69+
"askParametersDialog": {
70+
"message": "It looks like the new version has new parameters that need to be filled in to update the workspace."
6871
}
6972
}

‎site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ import { LazyIconField } from "components/IconField/LazyIconField"
3131
import { Maybe } from "components/Conditionals/Maybe"
3232
import i18next from "i18next"
3333
import Link from "@material-ui/core/Link"
34-
import { FormFooter } from "components/FormFooter/FormFooter"
3534
import {
3635
HorizontalForm,
3736
FormSection,
3837
FormFields,
39-
} from "components/HorizontalForm/HorizontalForm"
38+
FormFooter,
39+
} from "components/Form/Form"
4040
import camelCase from "lodash/camelCase"
4141
import capitalize from "lodash/capitalize"
4242
import { VariableInput } from "./VariableInput"

0 commit comments

Comments
 (0)