From 9614d55400bc899ddf500fec19565c317e6d652b Mon Sep 17 00:00:00 2001 From: Rowan Smith Date: Sat, 30 May 2026 09:44:44 +1000 Subject: [PATCH] fix: do not clobber dynamic parameters (backport #24645 to 2.32) (#25827) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backport of https://github.com/coder/coder/pull/24645 to `release/2.32`. Once a user has touched a field, it is better to leave it alone and display explicit validation errors over silently overwriting their inputs. Same for auto-filled values (whether from query parameters or a previous build). Original PR: #24645 — fix: do not clobber dynamic parameters Merge commit: d958d89b6f8d2356bd597acd9df5bd6030e589eb Closes #23418
Cherry-pick conflict resolution Two conflicts resolved: 1. **`site/src/testHelpers/websockets.ts`**: File was empty on release/2.32. Took the incoming version with the new `mockDynamicParameterWebSocket` helper. 2. **`site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx`**: File is `.jest.tsx` on the release branch (`.test.tsx` on main). Applied the incoming content (refactored websocket mocking, modernized test calls) to the existing `.jest.tsx` filename.
> [!NOTE] > Generated with [Coder Agents](https://coder.com) by @rowansmithau --------- Co-authored-by: Asher Co-authored-by: Garrett Delfosse --- .../modules/hooks/useSyncFormParameters.ts | 24 +- .../CreateWorkspacePage.jest.tsx | 308 ++++++++---------- .../CreateWorkspacePageView.tsx | 6 +- ...rkspaceParametersPageExperimental.jest.tsx | 172 ++++++++++ ...orkspaceParametersPageViewExperimental.tsx | 11 +- site/src/testHelpers/websockets.ts | 43 +++ 6 files changed, 381 insertions(+), 183 deletions(-) create mode 100644 site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.jest.tsx diff --git a/site/src/modules/hooks/useSyncFormParameters.ts b/site/src/modules/hooks/useSyncFormParameters.ts index 3843b1139b..74edd2a815 100644 --- a/site/src/modules/hooks/useSyncFormParameters.ts +++ b/site/src/modules/hooks/useSyncFormParameters.ts @@ -1,3 +1,4 @@ +import type { FormikTouched } from "formik"; import { useEffect, useRef } from "react"; import type * as TypesGen from "#/api/typesGenerated"; import type { PreviewParameter } from "#/api/typesGenerated"; @@ -5,6 +6,9 @@ import type { PreviewParameter } from "#/api/typesGenerated"; type UseSyncFormParametersProps = { parameters: readonly PreviewParameter[]; formValues: readonly TypesGen.WorkspaceBuildParameter[]; + touched: FormikTouched<{ + rich_parameter_values?: readonly TypesGen.WorkspaceBuildParameter[]; + }>; setFieldValue: ( field: string, value: TypesGen.WorkspaceBuildParameter[], @@ -14,6 +18,7 @@ type UseSyncFormParametersProps = { export function useSyncFormParameters({ parameters, formValues, + touched, setFieldValue, }: UseSyncFormParametersProps) { // Form values only needs to be updated when parameters change @@ -30,13 +35,16 @@ export function useSyncFormParameters({ ); const newParameterValues = parameters.map((param) => { - // When the server value is not valid (e.g., the initial - // WebSocket response before any user input is sent), - // preserve the current form value. This prevents the sync - // hook from overwriting autofilled values (from the - // previous build) with empty strings before the server - // has had a chance to process them. - if (!param.value.valid) { + // Do not mess with values the user has changed (or were auto-filled). + // Otherwise based on timing web socket responses can undo changes, and it + // seems bad to change a user's inputs from under them anyway. + if ( + touched[ + param.name as keyof { + rich_parameter_values?: readonly TypesGen.WorkspaceBuildParameter[]; + } + ] + ) { const existingValue = currentFormValuesMap.get(param.name); if (existingValue !== undefined) { return { name: param.name, value: existingValue }; @@ -60,5 +68,5 @@ export function useSyncFormParameters({ if (isChanged) { setFieldValue("rich_parameter_values", newParameterValues); } - }, [parameters, setFieldValue]); + }, [parameters, touched, setFieldValue]); } diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx index ee0135fc9f..2f128b1b26 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.jest.tsx @@ -8,6 +8,7 @@ import { MockDynamicParametersResponse, MockDynamicParametersResponseWithError, MockPermissions, + MockPreviewParameter, MockSliderParameter, MockTemplate, MockTemplateVersionExternalAuthGithub, @@ -20,7 +21,7 @@ import { renderWithAuth, waitForLoaderToBeRemoved, } from "#/testHelpers/renderHelpers"; -import { createMockWebSocket } from "#/testHelpers/websockets"; +import { mockDynamicParameterWebSocket } from "#/testHelpers/websockets"; import CreateWorkspacePage from "./CreateWorkspacePage"; describe("CreateWorkspacePage", () => { @@ -47,36 +48,11 @@ describe("CreateWorkspacePage", () => { jest.spyOn(API, "getTemplateVersionPresets").mockResolvedValue([]); jest.spyOn(API, "createWorkspace").mockResolvedValue(MockWorkspace); jest.spyOn(API, "checkAuthorization").mockResolvedValue(MockPermissions); - - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - const [mockWebSocket, publisher] = createMockWebSocket("ws://test"); - - mockWebSocket.addEventListener("message", (event) => { - callbacks.onMessage(JSON.parse(event.data)); - }); - mockWebSocket.addEventListener("error", () => { - callbacks.onError( - new Error("Connection for dynamic parameters failed."), - ); - }); - mockWebSocket.addEventListener("close", () => { - callbacks.onClose(); - }); - - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(MockDynamicParametersResponse), - }), - ); - - return mockWebSocket; - }); + mockDynamicParameterWebSocket(MockDynamicParametersResponse); }); afterEach(() => { + jest.useRealTimers(); jest.restoreAllMocks(); }); @@ -105,32 +81,9 @@ describe("CreateWorkspacePage", () => { }); it("sends parameter updates via WebSocket when form values change", async () => { - const [mockWebSocket, publisher] = createMockWebSocket("ws://test"); - - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - mockWebSocket.addEventListener("message", (event) => { - callbacks.onMessage(JSON.parse(event.data)); - }); - mockWebSocket.addEventListener("error", () => { - callbacks.onError( - new Error("Connection for dynamic parameters failed."), - ); - }); - mockWebSocket.addEventListener("close", () => { - callbacks.onClose(); - }); - - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(MockDynamicParametersResponse), - }), - ); - - return mockWebSocket; - }); + const [mockWebSocket] = mockDynamicParameterWebSocket( + MockDynamicParametersResponse, + ); renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); @@ -150,18 +103,16 @@ describe("CreateWorkspacePage", () => { await userEvent.click(instanceTypeSelect); }); - let mediumOption: Element | null = null; - await waitFor(() => { - mediumOption = screen.queryByRole("option", { name: /t3\.medium/i }); - expect(mediumOption).toBeTruthy(); + const mediumOption = await screen.findByRole("option", { + name: /t3\.medium/i, }); await waitFor(async () => { - await userEvent.click(mediumOption!); + await userEvent.click(mediumOption); }); - act(() => { - jest.runAllTimers(); + await act(async () => { + await jest.runAllTimersAsync(); }); expect(mockWebSocket.send).toHaveBeenCalledWith( @@ -172,48 +123,42 @@ describe("CreateWorkspacePage", () => { }); it("handles WebSocket error gracefully", async () => { - const [mockWebSocket, mockPublisher] = createMockWebSocket("ws://test"); - - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - mockWebSocket.addEventListener("error", () => { - callbacks.onError(new Error("Connection failed")); - }); - - return mockWebSocket; - }); + const [, mockPublisher] = mockDynamicParameterWebSocket([]); renderCreateWorkspacePage(); await waitFor(() => { - expect(mockPublisher).toBeDefined(); + expect(API.templateVersionDynamicParameters).toHaveBeenCalled(); + }); + + await act(async () => { mockPublisher.publishError(new Event("Connection failed")); + }); + + await waitFor(() => { const alert = screen.getByRole("alert"); expect( - within(alert).getByRole("heading", { name: /connection failed/i }), + within(alert).getByRole("heading", { + name: /connection for dynamic parameters failed/i, + }), ).toBeInTheDocument(); }); }); it("handles WebSocket close event", async () => { - const [mockWebSocket, mockPublisher] = createMockWebSocket("ws://test"); - - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - mockWebSocket.addEventListener("close", () => { - callbacks.onClose(); - }); - - return mockWebSocket; - }); + const [, mockPublisher] = mockDynamicParameterWebSocket([]); renderCreateWorkspacePage(); await waitFor(() => { - expect(mockPublisher).toBeDefined(); + expect(API.templateVersionDynamicParameters).toHaveBeenCalled(); + }); + + await act(async () => { mockPublisher.publishClose(new Event("close") as CloseEvent); + }); + + await waitFor(() => { const alert = screen.getByRole("alert"); expect( within(alert).getByRole("heading", { @@ -224,27 +169,9 @@ describe("CreateWorkspacePage", () => { }); it("only parameters from latest response are displayed", async () => { - const [mockWebSocket, mockPublisher] = createMockWebSocket("ws://test"); - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - mockWebSocket.addEventListener("message", (event) => { - callbacks.onMessage(JSON.parse(event.data)); - }); - - mockPublisher.publishOpen(new Event("open")); - mockPublisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify({ - id: 0, - parameters: [MockDropdownParameter], - diagnostics: [], - }), - }), - ); - - return mockWebSocket; - }); + const [, mockPublisher] = mockDynamicParameterWebSocket([ + MockDropdownParameter, + ]); renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); @@ -260,7 +187,7 @@ describe("CreateWorkspacePage", () => { diagnostics: [], }; - await waitFor(() => { + await act(async () => { mockPublisher.publishMessage( new MessageEvent("message", { data: JSON.stringify(response1) }), ); @@ -270,30 +197,94 @@ describe("CreateWorkspacePage", () => { ); }); - expect(screen.queryByText("CPU Count")).toBeInTheDocument(); - expect(screen.queryByText("Instance Type")).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByText("CPU Count")).toBeInTheDocument(); + expect(screen.queryByText("Instance Type")).not.toBeInTheDocument(); + }); + }); + + it("does not clobber user values", async () => { + const [, mockPublisher] = mockDynamicParameterWebSocket([ + MockPreviewParameter, + ]); + + renderCreateWorkspacePage(); + await waitForLoaderToBeRemoved(); + + const form = screen.getByTestId("form"); + const input = await within(form).findByRole("textbox", { + name: /parameter 1/i, + }); + await userEvent.clear(input); + await userEvent.type(input, "hi there hello"); + + await waitFor(() => { + expect( + within(form).getByDisplayValue("hi there hello"), + ).toBeInTheDocument(); + }); + + // Simulate a stale response. + await act(async () => { + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 1, + parameters: [MockPreviewParameter, MockValidationParameter], + }), + }), + ); + }); + + // Should have the new field, but keep the existing user-filled values. + await waitFor(() => { + expect(within(form).getByDisplayValue("50")).toBeInTheDocument(); + expect( + within(form).getByDisplayValue("hi there hello"), + ).toBeInTheDocument(); + }); + }); + + it("does not clobber auto-filled values", async () => { + const [, mockPublisher] = mockDynamicParameterWebSocket([ + MockPreviewParameter, + MockSliderParameter, + ]); + + renderCreateWorkspacePage( + `/templates/${MockTemplate.name}/workspace?param.cpu_count=44¶m.parameter1=auto`, + ); + await waitForLoaderToBeRemoved(); + + // Simulate a stale response. + await act(async () => { + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 2, + parameters: [ + MockPreviewParameter, + MockSliderParameter, + MockValidationParameter, + ], + }), + }), + ); + }); + + // Should have the new field, but keep the existing auto-filled values. + const form = screen.getByTestId("form"); + await waitFor(() => { + expect(within(form).getByDisplayValue("50")).toBeInTheDocument(); + expect(within(form).getByDisplayValue("44")).toBeInTheDocument(); + expect(within(form).getByDisplayValue("auto")).toBeInTheDocument(); + }); }); }); describe("Dynamic Parameter Types", () => { it("displays parameter validation errors", async () => { - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - const [mockWebSocket, publisher] = createMockWebSocket("ws://test"); - - mockWebSocket.addEventListener("message", (event) => { - callbacks.onMessage(JSON.parse(event.data)); - }); - - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(MockDynamicParametersResponseWithError), - }), - ); - - return mockWebSocket; - }); + mockDynamicParameterWebSocket(MockDynamicParametersResponseWithError); renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); @@ -337,38 +328,20 @@ describe("CreateWorkspacePage", () => { diagnostics: [], }; - jest - .spyOn(API, "templateVersionDynamicParameters") - .mockImplementation((_versionId, _ownerId, callbacks) => { - const [mockWebSocket, publisher] = createMockWebSocket("ws://test"); - - mockWebSocket.addEventListener("message", (event) => { - callbacks.onMessage(JSON.parse(event.data)); - }); - - publisher.publishOpen(new Event("open")); + const [mockWebSocket, publisher] = + mockDynamicParameterWebSocket(mockResponseInitial); + const originalSend = mockWebSocket.send; + mockWebSocket.send = jest.fn((data) => { + originalSend.call(mockWebSocket, data); + if (typeof data === "string" && data.includes('"200"')) { publisher.publishMessage( new MessageEvent("message", { - data: JSON.stringify(mockResponseInitial), + data: JSON.stringify(mockResponseWithError), }), ); - - const originalSend = mockWebSocket.send; - mockWebSocket.send = jest.fn((data) => { - originalSend.call(mockWebSocket, data); - - if (typeof data === "string" && data.includes('"200"')) { - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(mockResponseWithError), - }), - ); - } - }); - - return mockWebSocket; - }); + } + }); renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); @@ -380,10 +353,8 @@ describe("CreateWorkspacePage", () => { const numberInput = screen.getByDisplayValue("50"); expect(numberInput).toBeInTheDocument(); - await waitFor(async () => { - await userEvent.clear(numberInput); - await userEvent.type(numberInput, "200"); - }); + await userEvent.clear(numberInput); + await userEvent.type(numberInput, "200"); await waitFor(() => { expect(screen.getByDisplayValue("200")).toBeInTheDocument(); @@ -512,17 +483,13 @@ describe("CreateWorkspacePage", () => { const nameInput = screen.getByRole("textbox", { name: /workspace name/i, }); - await waitFor(async () => { - await userEvent.clear(nameInput); - await userEvent.type(nameInput, "my-test-workspace"); - }); + await userEvent.clear(nameInput); + await userEvent.type(nameInput, "my-test-workspace"); const createButton = screen.getByRole("button", { name: /create workspace/i, }); - await waitFor(async () => { - await userEvent.click(createButton); - }); + await userEvent.click(createButton); await waitFor(() => { expect(API.createWorkspace).toHaveBeenCalledWith( @@ -600,18 +567,13 @@ describe("CreateWorkspacePage", () => { name: /workspace name/i, }); - await waitFor(async () => { - await userEvent.clear(nameInput); - await userEvent.type(nameInput, "my-test-workspace"); - }); + await userEvent.clear(nameInput); + await userEvent.type(nameInput, "my-test-workspace"); - // Submit form const createButton = screen.getByRole("button", { name: /create workspace/i, }); - await waitFor(async () => { - await userEvent.click(createButton); - }); + await userEvent.click(createButton); await waitFor(() => { expect(router.state.location.pathname).toBe( diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index a52d4c53db..eacb779052 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -138,7 +138,9 @@ export const CreateWorkspacePageView: FC = ({ // 1. The form parameter values are initialized from the websocket response when the form is mounted // 2. Only touched form fields are sent to the websocket, a field is touched if edited by the user or set by autofill // 3. The websocket response may add or remove parameters, these are added or removed from the form values in the useSyncFormParameters hook - // 4. All existing form parameters are updated to match the websocket response in the useSyncFormParameters hook + // 4. All existing form parameters are updated to match the websocket response + // in the useSyncFormParameters hook, unless they have been touched by the + // user or auto-filled. const form: FormikContextType = useFormik({ initialValues: { @@ -360,6 +362,7 @@ export const CreateWorkspacePageView: FC = ({ useSyncFormParameters({ parameters, formValues: form.values.rich_parameter_values ?? [], + touched: form.touched, setFieldValue: form.setFieldValue, }); @@ -445,6 +448,7 @@ export const CreateWorkspacePageView: FC = ({ onSubmit={form.handleSubmit} aria-label="Create workspace form" className="flex flex-col gap-10 w-full border border-border-default border-solid rounded-lg p-6" + data-testid="form" > {Boolean(error) && } diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.jest.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.jest.tsx new file mode 100644 index 0000000000..6274c78b30 --- /dev/null +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.jest.tsx @@ -0,0 +1,172 @@ +import { screen, waitFor, within } from "@testing-library/react"; +import { act } from "react"; +import { API } from "#/api/api"; +import type { DynamicParametersResponse } from "#/api/typesGenerated"; +import { + MockPreviewParameter, + MockTemplateVersionParameter1, + MockTemplateVersionParameter2, + MockTemplateVersionParameter4, + MockValidationParameter, + MockWorkspace, + MockWorkspaceBuildParameter1, + MockWorkspaceBuildParameter2, + MockWorkspaceBuildParameter4, +} from "#/testHelpers/entities"; +import { + renderWithWorkspaceSettingsLayout, + waitForLoaderToBeRemoved, +} from "#/testHelpers/renderHelpers"; +import WorkspaceParametersPageExperimental from "./WorkspaceParametersPageExperimental"; + +// Minimal inline publisher for vitest. The shared websockets.ts helper +// uses jest globals and cannot be imported from vitest on this branch. +type Publisher = { + publishMessage: (event: MessageEvent) => void; + publishOpen: (event: Event) => void; +}; + +function setupDynamicParameterWebSocket( + response: DynamicParametersResponse, +): Publisher { + const subs: Record void>> = { + message: new Set(), + error: new Set(), + close: new Set(), + open: new Set(), + }; + + const fakeSocket = { + addEventListener: (type: string, cb: (e: unknown) => void) => { + subs[type]?.add(cb); + }, + removeEventListener: (type: string, cb: (e: unknown) => void) => { + subs[type]?.delete(cb); + }, + close: jest.fn(), + }; + + const publisher: Publisher = { + publishOpen: (event) => { + for (const sub of subs.open) sub(event); + }, + publishMessage: (event) => { + for (const sub of subs.message) sub(event); + }, + }; + + jest + .spyOn(API, "templateVersionDynamicParameters") + .mockImplementation((_versionId, _ownerId, callbacks) => { + fakeSocket.addEventListener("message", (event) => { + callbacks.onMessage(JSON.parse((event as MessageEvent).data)); + }); + fakeSocket.addEventListener("error", () => { + callbacks.onError( + new Error("Connection for dynamic parameters failed."), + ); + }); + fakeSocket.addEventListener("close", () => { + callbacks.onClose(); + }); + publisher.publishOpen(new Event("open")); + publisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify(response), + }), + ); + return fakeSocket as unknown as WebSocket; + }); + + return publisher; +} + +describe("WorkspaceParametersPageExperimental", () => { + const renderWorkspaceParametersPageExperimental = ( + route = `/@${MockWorkspace.owner_name}/${MockWorkspace.name}/settings`, + ) => { + return renderWithWorkspaceSettingsLayout( + , + { + route, + path: "/:username/:workspace/settings", + extraRoutes: [ + { + // Need this because after submit the user is redirected. + path: "/:username/:workspace", + element:
Workspace Page
, + }, + ], + }, + ); + }; + + beforeEach(() => { + jest.clearAllMocks(); + jest + .spyOn(API, "getWorkspaceByOwnerAndName") + .mockResolvedValueOnce(MockWorkspace); + jest + .spyOn(API, "getTemplateVersionRichParameters") + .mockResolvedValueOnce([ + MockTemplateVersionParameter1, + MockTemplateVersionParameter2, + MockTemplateVersionParameter4, + ]); + jest + .spyOn(API, "getWorkspaceBuildParameters") + .mockResolvedValueOnce([ + MockWorkspaceBuildParameter1, + MockWorkspaceBuildParameter2, + MockWorkspaceBuildParameter4, + ]); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + it("does not clobber touched parameters", async () => { + const publisher = setupDynamicParameterWebSocket({ + id: 0, + parameters: [ + { + ...MockPreviewParameter, + name: MockWorkspaceBuildParameter1.name, + }, + ], + diagnostics: [], + }); + + renderWorkspaceParametersPageExperimental(); + await waitForLoaderToBeRemoved(); + + // Simulate a stale response. + await act(async () => { + publisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 2, + parameters: [ + { + ...MockPreviewParameter, + name: MockWorkspaceBuildParameter1.name, + }, + MockValidationParameter, + ], + }), + }), + ); + }); + + // Should have the new field, but keep the existing auto-filled values. + const form = screen.getByTestId("form"); + await waitFor(() => { + expect(within(form).getByDisplayValue("50")).toBeInTheDocument(); + expect( + within(form).getByDisplayValue(MockWorkspaceBuildParameter1.value), + ).toBeInTheDocument(); + }); + }); +}); diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx index 768db8d6c2..7f2e7d706b 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageViewExperimental.tsx @@ -60,6 +60,9 @@ export const WorkspaceParametersPageViewExperimental: FC< autofillParameters, ), }, + initialTouched: Object.fromEntries( + autofillParameters.map((p) => [p.name, true]), + ), validationSchema: useValidationSchemaForDynamicParameters(parameters), enableReinitialize: false, validateOnChange: true, @@ -97,12 +100,14 @@ export const WorkspaceParametersPageViewExperimental: FC< name: parameter.name, value, }); + form.setFieldTouched(parameter.name, true); sendDynamicParamsRequest(parameter, value); }; useSyncFormParameters({ parameters, formValues: form.values.rich_parameter_values ?? [], + touched: form.touched, setFieldValue: form.setFieldValue, }); @@ -201,7 +206,11 @@ export const WorkspaceParametersPageViewExperimental: FC< )} -
+ {parameters.length > 0 && (
diff --git a/site/src/testHelpers/websockets.ts b/site/src/testHelpers/websockets.ts index cc1f6023a1..abf36e1442 100644 --- a/site/src/testHelpers/websockets.ts +++ b/site/src/testHelpers/websockets.ts @@ -1,3 +1,8 @@ +import { API } from "#/api/api"; +import type { + DynamicParametersResponse, + PreviewParameter, +} from "#/api/typesGenerated"; import type { WebSocketEventType } from "#/utils/OneWayWebSocket"; type SocketSendData = Parameters[0]; @@ -160,3 +165,41 @@ export function createMockWebSocket( return [mockSocket, publisher] as const; } + +export function mockDynamicParameterWebSocket( + response: DynamicParametersResponse | readonly PreviewParameter[], +): readonly [MockWebSocket, MockWebSocketServer] { + let message: DynamicParametersResponse; + if (Array.isArray(response)) { + message = { + id: 0, + parameters: response, + diagnostics: [], + }; + } else { + message = response as DynamicParametersResponse; + } + const [mockWebSocket, mockPublisher] = createMockWebSocket("ws://test"); + jest + .spyOn(API, "templateVersionDynamicParameters") + .mockImplementation((_versionId, _ownerId, callbacks) => { + mockWebSocket.addEventListener("message", (event) => { + callbacks.onMessage(JSON.parse(event.data)); + }); + mockWebSocket.addEventListener("error", () => { + callbacks.onError( + new Error("Connection for dynamic parameters failed."), + ); + }); + mockWebSocket.addEventListener("close", () => { + callbacks.onClose(); + }); + mockPublisher.publishOpen(new Event("open")); + mockPublisher.publishMessage( + new MessageEvent("message", { data: JSON.stringify(message) }), + ); + + return mockWebSocket; + }); + return [mockWebSocket, mockPublisher]; +}