fix(site): Do not require immutable parameters (#6637)

BrunoQuaresma · web-flow · commit 01a6af98b4bf · 2023-03-17T10:09:10.000-03:00
diff --git a/site/src/api/api.test.ts b/site/src/api/api.test.ts
@@ -2,8 +2,10 @@ import axios from "axios"
 import {
   MockTemplate,
   MockTemplateVersionParameter1,
+  MockTemplateVersionParameter2,
   MockWorkspace,
   MockWorkspaceBuild,
+  MockWorkspaceBuildParameter1,
 } from "testHelpers/entities"
 import * as api from "./api"
 import * as TypesGen from "./typesGenerated"
@@ -176,18 +178,52 @@ describe("api.ts", () => {
     })
 
     it("fails when having missing parameters", async () => {
+      jest
+        .spyOn(api, "postWorkspaceBuild")
+        .mockResolvedValue(MockWorkspaceBuild)
+      jest.spyOn(api, "getTemplate").mockResolvedValue(MockTemplate)
+      jest.spyOn(api, "getWorkspaceBuildParameters").mockResolvedValue([])
+      jest
+        .spyOn(api, "getTemplateVersionRichParameters")
+        .mockResolvedValue([
+          MockTemplateVersionParameter1,
+          { ...MockTemplateVersionParameter2, mutable: false },
+        ])
+
+      let error = new Error()
+      try {
+        await api.updateWorkspace(MockWorkspace)
+      } catch (e) {
+        error = e as Error
+      }
+
+      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,
+      ])
+    })
+
+    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").mockResolvedValueOnce([])
+      jest
+        .spyOn(api, "getWorkspaceBuildParameters")
+        .mockResolvedValue([MockWorkspaceBuildParameter1])
       jest
         .spyOn(api, "getTemplateVersionRichParameters")
-        .mockResolvedValueOnce([MockTemplateVersionParameter1])
-
-      await expect(api.updateWorkspace(MockWorkspace)).rejects.toThrow(
-        api.MissingBuildParameters,
-      )
+        .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
@@ -946,31 +946,34 @@ 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)
   }
 
   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 and it is required, add it to the list of missing parameters
-    if (parameter.required) {
-      missingParameters.push(parameter)
-    }
+    missingParameters.push(parameter)
   }
 
-  return [updatedBuildParameters, missingParameters] as const
+  return missingParameters
 }
