From e811741b4c8b0636891d586fdc74b137377b5ff4 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 16 Mar 2023 17:01:23 +0000 Subject: [PATCH 1/2] fix(site): Do not require mutable parameters --- site/src/api/api.test.ts | 28 +++++++++++++++++++++------- site/src/api/api.ts | 4 ++-- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/site/src/api/api.test.ts b/site/src/api/api.test.ts index 3767a9ef81dd5..76e1fe7570458 100644 --- a/site/src/api/api.test.ts +++ b/site/src/api/api.test.ts @@ -2,6 +2,7 @@ import axios from "axios" import { MockTemplate, MockTemplateVersionParameter1, + MockTemplateVersionParameter2, MockWorkspace, MockWorkspaceBuild, } from "testHelpers/entities" @@ -178,16 +179,29 @@ describe("api.ts", () => { it("fails when having missing parameters", async () => { jest .spyOn(api, "postWorkspaceBuild") - .mockResolvedValueOnce(MockWorkspaceBuild) - jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate) - jest.spyOn(api, "getWorkspaceBuildParameters").mockResolvedValueOnce([]) + .mockResolvedValue(MockWorkspaceBuild) + jest.spyOn(api, "getTemplate").mockResolvedValue(MockTemplate) + jest.spyOn(api, "getWorkspaceBuildParameters").mockResolvedValue([]) jest .spyOn(api, "getTemplateVersionRichParameters") - .mockResolvedValueOnce([MockTemplateVersionParameter1]) + .mockResolvedValue([ + MockTemplateVersionParameter1, + { ...MockTemplateVersionParameter2, mutable: false }, + ]) + + let error = new Error() + try { + await api.updateWorkspace(MockWorkspace) + } catch (e) { + error = e as Error + } - await expect(api.updateWorkspace(MockWorkspace)).rejects.toThrow( - api.MissingBuildParameters, - ) + expect(error).toBeInstanceOf(api.MissingBuildParameters) + // Verify if the correct missing parameters are being passed + // It should not require immutable parameters + expect((error as api.MissingBuildParameters).parameters).toEqual([ + MockTemplateVersionParameter1, + ]) }) }) }) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 93035c19f3cf8..8f1b414d61954 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -987,8 +987,8 @@ const updateBuildParameters = ( continue } - // If there is no value and it is required, add it to the list of missing parameters - if (parameter.required) { + // If there is no value, it is required and can be changed, add it to the list of missing parameters + if (parameter.required && parameter.mutable) { missingParameters.push(parameter) } } From 9969c82ce38db53734c0a72d9b609837e85b0a6a Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 16 Mar 2023 17:43:05 +0000 Subject: [PATCH 2/2] Fix --- site/src/api/api.test.ts | 22 ++++++++++++++++++++++ site/src/api/api.ts | 23 +++++++++++------------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/site/src/api/api.test.ts b/site/src/api/api.test.ts index 76e1fe7570458..6efe74960bf9f 100644 --- a/site/src/api/api.test.ts +++ b/site/src/api/api.test.ts @@ -5,6 +5,7 @@ import { MockTemplateVersionParameter2, MockWorkspace, MockWorkspaceBuild, + MockWorkspaceBuildParameter1, } from "testHelpers/entities" import * as api from "./api" import * as TypesGen from "./typesGenerated" @@ -203,5 +204,26 @@ describe("api.ts", () => { MockTemplateVersionParameter1, ]) }) + + it("creates a build with the no parameters if it is already filled", async () => { + jest + .spyOn(api, "postWorkspaceBuild") + .mockResolvedValueOnce(MockWorkspaceBuild) + jest.spyOn(api, "getTemplate").mockResolvedValueOnce(MockTemplate) + jest + .spyOn(api, "getWorkspaceBuildParameters") + .mockResolvedValue([MockWorkspaceBuildParameter1]) + jest + .spyOn(api, "getTemplateVersionRichParameters") + .mockResolvedValue([ + { ...MockTemplateVersionParameter1, required: true, mutable: false }, + ]) + await api.updateWorkspace(MockWorkspace) + expect(api.postWorkspaceBuild).toHaveBeenCalledWith(MockWorkspace.id, { + transition: "start", + template_version_id: MockTemplate.active_version_id, + rich_parameter_values: [], + }) + }) }) }) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 8f1b414d61954..83cb3dd2e0ea2 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -946,11 +946,12 @@ export const updateWorkspace = async ( const templateParameters = await getTemplateVersionRichParameters( activeVersionId, ) - const [updatedBuildParameters, missingParameters] = updateBuildParameters( + const missingParameters = getMissingParameters( oldBuildParameters, newBuildParameters, templateParameters, ) + if (missingParameters.length > 0) { throw new MissingBuildParameters(missingParameters) } @@ -958,19 +959,21 @@ export const updateWorkspace = async ( return postWorkspaceBuild(workspace.id, { transition: "start", template_version_id: activeVersionId, - rich_parameter_values: updatedBuildParameters, + rich_parameter_values: newBuildParameters, }) } -const updateBuildParameters = ( +const getMissingParameters = ( oldBuildParameters: TypesGen.WorkspaceBuildParameter[], newBuildParameters: TypesGen.WorkspaceBuildParameter[], templateParameters: TypesGen.TemplateVersionParameter[], ) => { const missingParameters: TypesGen.TemplateVersionParameter[] = [] - const updatedBuildParameters: TypesGen.WorkspaceBuildParameter[] = [] + const requiredParameters = templateParameters.filter( + (p) => p.required && p.mutable, + ) - for (const parameter of templateParameters) { + for (const parameter of requiredParameters) { // Check if there is a new value let buildParameter = newBuildParameters.find( (p) => p.name === parameter.name, @@ -981,17 +984,13 @@ const updateBuildParameters = ( buildParameter = oldBuildParameters.find((p) => p.name === parameter.name) } - // If there is a value from the new or old one, add it to the list + // If there is a value from the new or old one, it is not missed if (buildParameter) { - updatedBuildParameters.push(buildParameter) continue } - // If there is no value, it is required and can be changed, add it to the list of missing parameters - if (parameter.required && parameter.mutable) { - missingParameters.push(parameter) - } + missingParameters.push(parameter) } - return [updatedBuildParameters, missingParameters] as const + return missingParameters }