diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index bfd23d4817..6e46344101 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -677,7 +677,7 @@ const ParameterDiagnostics: FC = ({ }; export const getInitialParameterValues = ( - params: PreviewParameter[], + params: readonly PreviewParameter[], autofillParams?: AutofillBuildParameter[], ): WorkspaceBuildParameter[] => { return params.map((parameter) => { diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 0b1195cb2c..7e28018cc2 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -2,16 +2,18 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { act } from "react"; import { API } from "#/api/api"; -import type { DynamicParametersResponse, Preset } from "#/api/typesGenerated"; +import type { Preset, PreviewParameter } from "#/api/typesGenerated"; import { MockDropdownParameter, - MockDynamicParametersResponse, MockDynamicParametersResponseWithError, + MockMultiSelectParameter, MockPermissions, MockPreviewParameter1, MockPreviewParameter2, MockPreviewParameter7, MockSliderParameter, + MockSwitchParameter, + MockTagSelectParameter, MockTemplate, MockTemplateVersion, MockTemplateVersionExternalAuthGithub, @@ -25,7 +27,11 @@ import { renderWithAuth, waitForLoaderToBeRemoved, } from "#/testHelpers/renderHelpers"; -import { mockDynamicParameterWebSocket } from "#/testHelpers/websockets"; +import { + type MockWebSocket, + type MockWebSocketServer, + mockDynamicParameterWebSocket, +} from "#/testHelpers/websockets"; import CreateWorkspacePage from "./CreateWorkspacePage"; describe("CreateWorkspacePage", () => { @@ -44,25 +50,163 @@ describe("CreateWorkspacePage", () => { }); }; - const renderCreateWorkspacePageWithSocket = (route?: string) => { - mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(MockDynamicParametersResponse), + type Context = ReturnType & { + mockSocket: MockWebSocket; + mockPublisher: MockWebSocketServer; + }; + + // Mocks the required endpoints, most importantly the web socket, constructs + // the route with the required query parameters, then renders the page on that + // route. + // + // Returns the mock web socket and router context for further testing. + const renderPageWithSocket = async ( + opts: { + // route can be overridden to set additional query variables. + route?: string; + // urlfill contains template parameters auto-filled via the query string. + urlfill?: Record; + // version will be added to the query string. + version?: string; + // preset will be added to the query string and set on the preset endpoint. + preset?: Preset; + } = {}, + ): Promise => { + const [mockSocket, mockPublisher] = mockDynamicParameterWebSocket(); + + const params = new URLSearchParams(); + if (opts.urlfill) { + Object.entries(opts.urlfill).forEach(([k, v]) => { + params.set(`param.${k}`, v); + }); + } + if (opts.preset) { + vi.spyOn(API, "getTemplateVersionPresets").mockResolvedValue([ + opts.preset, + ]); + params.set("preset", opts.preset.Name); + } + if (opts.version) { + params.set("version", opts.version); + } + let route = opts?.route || `/templates/${MockTemplate.name}/workspace`; + const query = params.toString(); + if (query.length > 0) { + if (route.includes("?")) { + route += `&${query}`; + } else { + route += `?${query}`; + } + } + const router = renderCreateWorkspacePage(route); + return { mockSocket, mockPublisher, ...router }; + }; + + // Waits for the client to connect to the socket then sends the initial + // message. Then, if there are auto-fill parameters (whether from a URL or a + // preset), also waits for the client's initial message. + const expectSocketHandshake = async (opts: { + mockPublisher: MockWebSocketServer; + // parameters are template parameters to send via the initial message from + // the backend to the client. + parameters: PreviewParameter[]; + // urlfill will be expected in the client's init message. + urlfill?: Record; + // version will be asserted in the web socket API call. + version?: string; + // preset will be expected in the client's init message. + preset?: Preset; + }): Promise => { + // Wait for the web socket connection. + const version = opts.version || MockTemplate.active_version_id; + await waitFor(() => { + expect(API.templateVersionDynamicParameters).toHaveBeenCalledWith( + version, + MockUserOwner.id, + expect.objectContaining({ + onMessage: expect.any(Function), + onError: expect.any(Function), + onClose: expect.any(Function), }), ); }); - return renderCreateWorkspacePage(route); + // Open and and send the initial message. + await act(async () => { + opts.mockPublisher.publishOpen(new Event("open")); + // The initial message always has the default values. + opts.mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: -1, + parameters: opts.parameters, + diagnostics: [], + }), + }), + ); + }); + + // Wait for the client's own init message, which should include all the + // auto-filled values, including from a preset. Without any auto-fill + // values, the client does not send any init message. + const inputs = opts.urlfill ? { ...opts.urlfill } : {}; + opts.preset?.Parameters?.forEach((p) => { + inputs[p.Name] = p.Value; + }); + if (Object.keys(inputs).length > 0) { + await waitFor(() => { + expect(opts.mockPublisher.clientSentData).toHaveLength(1); + expect( + JSON.parse(opts.mockPublisher.clientSentData[0] as string), + ).toEqual( + expect.objectContaining({ + id: 0, + inputs, + }), + ); + }); + } + }; + + // Wait for the loader to be removed then asserts form fields based on + // parameters and auto-fill. Lastly asserts the submit button is enabled. + const expectFormFields = async (opts: { + parameters: PreviewParameter[]; + urlfill?: Record; + preset?: Preset; + }): Promise => { + // Add any preset to the autofill. + const autofill = opts.urlfill ? { ...opts.urlfill } : {}; + opts.preset?.Parameters?.forEach((p) => { + autofill[p.Name] = p.Value; + }); + + await waitForLoaderToBeRemoved(); + + const parameters = opts.parameters.map((p) => { + return { + ...p, + value: Object.hasOwn(autofill, p.name) + ? { valid: true, value: autofill[p.name] } + : p.value, + }; + }); + + // The page should render with the defaults plus any auto-fill. + await checkParameters(...parameters); + const form = screen.getByTestId("form"); + const submitButton = within(form).getByRole("button", { + name: /create workspace/i, + }); + await waitFor(() => expect(submitButton).toBeEnabled()); }; const mockGpuPreset: Preset = { ID: "preset-gpu", Name: "gpu-large", Parameters: [ - { Name: "instance_type", Value: "t3.medium" }, - { Name: "cpu_count", Value: "4" }, + { Name: MockDropdownParameter.name, Value: "t3.medium" }, + { Name: MockSliderParameter.name, Value: "4" }, ], Default: false, DesiredPrebuildInstances: null, @@ -87,84 +231,58 @@ describe("CreateWorkspacePage", () => { }); describe("WebSocket Integration", () => { - it("establishes WebSocket connection and receives initial parameters", async () => { - renderCreateWorkspacePageWithSocket(); - await waitForLoaderToBeRemoved(); - - expect(API.templateVersionDynamicParameters).toHaveBeenCalledWith( - MockTemplate.active_version_id, - MockUserOwner.id, - expect.objectContaining({ - onMessage: expect.any(Function), - onError: expect.any(Function), - onClose: expect.any(Function), - }), - ); - - await waitFor(() => { - expect(screen.getByText(/instance type/i)).toBeInTheDocument(); - expect(screen.getByText("CPU Count")).toBeInTheDocument(); - expect(screen.getByText("Enable Monitoring")).toBeInTheDocument(); - expect(screen.getByText("Tags")).toBeInTheDocument(); - }); + it("skips initial parameters when no auto-fill", async () => { + const parameters = [ + MockDropdownParameter, + MockSliderParameter, + MockSwitchParameter, + MockTagSelectParameter, + MockMultiSelectParameter, + ]; + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters }); + // Should render without any sending any init message. + await expectFormFields({ parameters }); + expect(mockPublisher.clientSentData).toHaveLength(0); }); - it("sends parameter updates via WebSocket when form values change", async () => { - const [mockWebSocket] = mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(MockDynamicParametersResponse), - }), - ); - }); - - renderCreateWorkspacePage(); - await waitForLoaderToBeRemoved(); - - expect(screen.getByText(/instance type/i)).toBeInTheDocument(); - - const instanceTypeField = screen.getByTestId( - "parameter-field-instance_type", - ); - const instanceTypeSelect = - within(instanceTypeField).getByRole("combobox"); - expect(instanceTypeSelect).toBeInTheDocument(); - - vi.useFakeTimers({ shouldAdvanceTime: true }); - - await userEvent.click(instanceTypeSelect); - - const mediumOption = await screen.findByRole("option", { - name: /t3\.medium/i, - }); - - await userEvent.click(mediumOption); - - await act(async () => { - await vi.runAllTimersAsync(); - }); - - expect(mockWebSocket.send).toHaveBeenCalledWith( - expect.stringContaining('"instance_type":"t3.medium"'), - ); - - vi.useRealTimers(); + it("waits for and sends initial parameters when auto-filled", async () => { + const parameters = [ + MockDropdownParameter, + MockSliderParameter, + MockSwitchParameter, + MockTagSelectParameter, + MockMultiSelectParameter, + ]; + const urlfill = { + [MockDropdownParameter.name]: "t3.micro", + [MockSliderParameter.name]: "55", + [MockSwitchParameter.name]: "false", + [MockTagSelectParameter.name]: JSON.stringify(["tag1", "tag2"]), + [MockMultiSelectParameter.name]: JSON.stringify(["goland", "vscode"]), + }; + const { mockPublisher } = await renderPageWithSocket({ urlfill }); + await expectSocketHandshake({ mockPublisher, parameters, urlfill }); + // Should still see the loader as the client wais for the response to the + // client's init message. + expect(screen.queryByTestId("loader")).toBeInTheDocument(); }); - it("handles WebSocket error gracefully", async () => { + it("handles error gracefully", async () => { const [_, mockPublisher] = mockDynamicParameterWebSocket(); - renderCreateWorkspacePage(); + // Wait for the client to open the web socket. await waitFor(() => { expect(API.templateVersionDynamicParameters).toHaveBeenCalled(); }); + // Then error the web socket. await act(async () => { mockPublisher.publishError(new Event("Connection failed")); }); + // We should see an error message. await waitFor(() => { const alert = screen.getByRole("alert"); expect( @@ -175,30 +293,18 @@ describe("CreateWorkspacePage", () => { }); }); - it("handles WebSocket close event", async () => { - const [_, mockPublisher] = mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify({ - id: -1, - parameters: [], - diagnostics: [], - }), - }), - ); - }); + it("handles close", async () => { + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters: [] }); - renderCreateWorkspacePage(); - - await waitFor(() => { - expect(API.templateVersionDynamicParameters).toHaveBeenCalled(); - }); + await waitForLoaderToBeRemoved(); + // Close the web socket. await act(async () => { mockPublisher.publishClose(new Event("close") as CloseEvent); }); + // We should see an error message. await waitFor(() => { const alert = screen.getByRole("alert"); expect( @@ -209,67 +315,56 @@ describe("CreateWorkspacePage", () => { }); }); - it("only parameters from latest response are displayed", async () => { - const [, mockPublisher] = mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify({ - id: -1, - parameters: [MockDropdownParameter], - diagnostics: [], - }), - }), - ); - }); - - renderCreateWorkspacePage(); - await waitForLoaderToBeRemoved(); - - const response1: DynamicParametersResponse = { - id: 1, - parameters: [MockDropdownParameter], - diagnostics: [], - }; - const response2: DynamicParametersResponse = { - id: 4, - parameters: [MockSliderParameter], - diagnostics: [], - }; - - await act(async () => { - mockPublisher.publishMessage( - new MessageEvent("message", { data: JSON.stringify(response1) }), - ); - - mockPublisher.publishMessage( - new MessageEvent("message", { data: JSON.stringify(response2) }), - ); - }); - - await waitFor(() => { - expect(screen.queryByText("CPU Count")).toBeInTheDocument(); - expect(screen.queryByText("Instance Type")).not.toBeInTheDocument(); - }); + it("displays no parameters if none from init message", async () => { + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters: [] }); + await expectFormFields({ parameters: [] }); }); - it("does not clobber user values", async () => { - const [, mockPublisher] = mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - // The initial message always has the default values. - publisher.publishMessage( + it("only parameters from the latest response are displayed", async () => { + const parameters = [MockDropdownParameter]; + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters }); + await expectFormFields({ parameters }); + + // Send multiple messages. + await act(async () => { + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 0, + parameters: [MockSliderParameter], + diagnostics: [], + }), + }), + ); + mockPublisher.publishMessage( new MessageEvent("message", { data: JSON.stringify({ id: -1, - parameters: [MockPreviewParameter1, MockPreviewParameter7], + parameters: [MockSwitchParameter], diagnostics: [], }), }), ); }); - renderCreateWorkspacePage(); - await waitForLoaderToBeRemoved(); + // Page should re-render with the last message only. + await checkParameters(MockSwitchParameter); + + // The submit button should still be enabled. + const form = screen.getByTestId("form"); + const submitButton = within(form).getByRole("button", { + name: /create workspace/i, + }); + await waitFor(() => expect(submitButton).toBeEnabled()); + }); + + it("does not clobber edited parameters", async () => { + const parameters = [MockPreviewParameter1, MockPreviewParameter7]; + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters }); + await expectFormFields({ parameters }); // Blank out one field and fill out another. const editedParameters = [ @@ -286,7 +381,7 @@ describe("CreateWorkspacePage", () => { ]; editParameters(...editedParameters); - // Respond with different values. + // Send a message with different values. await act(async () => { mockPublisher.publishMessage( new MessageEvent("message", { @@ -305,52 +400,34 @@ describe("CreateWorkspacePage", () => { // Should have the new field, but keep the existing user-filled values. await checkParameters(...editedParameters, MockPreviewParameter2); + + // The submit button should still be enabled. + const form = screen.getByTestId("form"); + const submitButton = within(form).getByRole("button", { + name: /create workspace/i, + }); + await waitFor(() => expect(submitButton).toBeEnabled()); }); it("does not clobber auto-filled values", async () => { - const [, mockPublisher] = mockDynamicParameterWebSocket((publisher) => { - publisher.publishOpen(new Event("open")); - // The initial message always has the default values. - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify({ - id: -1, - parameters: [MockPreviewParameter1, MockPreviewParameter7], - diagnostics: [], - }), - }), - ); - }); - + const parameters = [MockPreviewParameter1, MockPreviewParameter7]; // Blank out one field and fill out another. - const editedParameters = [ - { - name: MockPreviewParameter1.name, - value: "", - }, - { - name: MockPreviewParameter7.name, - value: "not-blank", - }, - ]; - const query = editedParameters - .map((param) => `param.${param.name}=${param.value}`) - .join("&"); - renderCreateWorkspacePage( - `/templates/${MockTemplate.name}/workspace?${query}`, - ); - await waitForLoaderToBeRemoved(); + const urlfill = { + [MockPreviewParameter1.name]: "", + [MockPreviewParameter7.name]: "not-blank", + }; + const { mockPublisher } = await renderPageWithSocket({ urlfill }); + await expectSocketHandshake({ mockPublisher, parameters, urlfill }); - // Respond with different values. + // Respond to the client's init message with different values. await act(async () => { mockPublisher.publishMessage( new MessageEvent("message", { data: JSON.stringify({ id: 2, parameters: [ - MockPreviewParameter1, + ...parameters, MockPreviewParameter2, // new field - MockPreviewParameter7, ], diagnostics: [], }), @@ -358,8 +435,10 @@ describe("CreateWorkspacePage", () => { ); }); - // Should have the new field, but keep the existing auto-filled values. - await checkParameters(...editedParameters, MockPreviewParameter2); + await expectFormFields({ + parameters: [...parameters, MockPreviewParameter2], + urlfill, + }); }); }); @@ -388,73 +467,55 @@ describe("CreateWorkspacePage", () => { }); it("displays parameter validation errors for min/max constraints", async () => { - const mockResponseInitial: DynamicParametersResponse = { - id: 1, + const { mockSocket, mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ + mockPublisher, parameters: [MockValidationParameter], - diagnostics: [], - }; + }); - const mockResponseWithError: DynamicParametersResponse = { - id: 2, - parameters: [ - { - ...MockValidationParameter, - value: { value: "200", valid: false }, - diagnostics: [ - { - severity: "error", - summary: - "Invalid parameter value according to 'validation' block", - detail: "value 200 is more than the maximum 100", - extra: { - code: "", + // Respond to the client's edit with an error. + mockSocket.send.mockImplementation((data) => { + expect(JSON.parse(data as string)).toEqual( + expect.objectContaining({ + id: 0, + inputs: { + [MockValidationParameter.name]: "200", + }, + }), + ); + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 2, + parameters: [ + { + ...MockValidationParameter, + value: { value: "200", valid: false }, + diagnostics: [ + { + severity: "error", + summary: + "Invalid parameter value according to 'validation' block", + detail: "value 200 is more than the maximum 100", + extra: { + code: "", + }, + }, + ], }, - }, - ], - }, - ], - diagnostics: [], + ], + diagnostics: [], + }), + }), + ); + }); + + const edited = { + name: MockValidationParameter.name, + display_name: MockValidationParameter.display_name, + value: "200", }; - - const [mockWebSocket, mockPublisher] = mockDynamicParameterWebSocket( - (publisher) => { - publisher.publishOpen(new Event("open")); - publisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(mockResponseInitial), - }), - ); - }, - ); - const originalSend = mockWebSocket.send; - mockWebSocket.send = vi.fn((data) => { - originalSend.call(mockWebSocket, data); - - if (typeof data === "string" && data.includes('"200"')) { - mockPublisher.publishMessage( - new MessageEvent("message", { - data: JSON.stringify(mockResponseWithError), - }), - ); - } - }); - - renderCreateWorkspacePage(); - await waitForLoaderToBeRemoved(); - - await waitFor(() => { - expect(screen.getByText("Invalid Parameter")).toBeInTheDocument(); - }); - - const numberInput = screen.getByDisplayValue("50"); - expect(numberInput).toBeInTheDocument(); - - await userEvent.clear(numberInput); - await userEvent.type(numberInput, "200"); - - await waitFor(() => { - expect(screen.getByDisplayValue("200")).toBeInTheDocument(); - }); + editParameters(edited); await waitFor(() => { expect( @@ -485,8 +546,8 @@ describe("CreateWorkspacePage", () => { MockTemplateVersionExternalAuthGithub, ]); - renderCreateWorkspacePageWithSocket(); - await waitForLoaderToBeRemoved(); + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters: [] }); await waitFor(() => { expect(screen.getByText("GitHub")).toBeInTheDocument(); @@ -501,8 +562,8 @@ describe("CreateWorkspacePage", () => { MockTemplateVersionExternalAuthGithubAuthenticated, ]); - renderCreateWorkspacePageWithSocket(); - await waitForLoaderToBeRemoved(); + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters: [] }); await waitFor(() => { expect(screen.getByText("GitHub")).toBeInTheDocument(); @@ -515,10 +576,12 @@ describe("CreateWorkspacePage", () => { MockTemplateVersionExternalAuthGithub, ]); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?mode=auto&version=${MockTemplate.id}`, - ); - await waitForLoaderToBeRemoved(); + const version = MockTemplate.id; + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?mode=auto`, + version, + }); + await expectSocketHandshake({ mockPublisher, parameters: [], version }); await waitFor(() => { expect( @@ -542,19 +605,27 @@ describe("CreateWorkspacePage", () => { new Error("Auto-creation failed"), ); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?mode=auto`, - ); + const parameters = [ + MockDropdownParameter, + MockSliderParameter, + MockSwitchParameter, + MockTagSelectParameter, + MockMultiSelectParameter, + ]; + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?mode=auto`, + }); + await expectSocketHandshake({ mockPublisher, parameters }); // Consent dialog appears for mode=auto. Confirm to proceed. - const confirmButton = await screen.findByRole("button", { - name: /confirm and create/i, + await act(async () => { + const confirmButton = await screen.findByRole("button", { + name: /confirm and create/i, + }); + await userEvent.click(confirmButton); }); - await userEvent.click(confirmButton); - await waitForLoaderToBeRemoved(); - - expect(screen.getByText(/instance type/i)).toBeInTheDocument(); + await expectFormFields({ parameters }); await waitFor(() => { expect(screen.getByText("Create workspace")).toBeInTheDocument(); @@ -567,10 +638,16 @@ describe("CreateWorkspacePage", () => { describe("Form Submission", () => { it("creates workspace with correct parameters", async () => { - renderCreateWorkspacePageWithSocket(); - await waitForLoaderToBeRemoved(); - - expect(screen.getByText(/instance type/i)).toBeInTheDocument(); + const parameters = [ + MockDropdownParameter, + MockSliderParameter, + MockSwitchParameter, + MockTagSelectParameter, + MockMultiSelectParameter, + ]; + const { mockPublisher } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters }); + await expectFormFields({ parameters }); const nameInput = screen.getByRole("textbox", { name: /workspace name/i, @@ -590,16 +667,10 @@ describe("CreateWorkspacePage", () => { name: "my-test-workspace", template_version_id: MockTemplate.active_version_id, template_id: undefined, - rich_parameter_values: [ - expect.objectContaining({ name: "instance_type", value: "" }), - expect.objectContaining({ name: "cpu_count", value: "2" }), - expect.objectContaining({ - name: "enable_monitoring", - value: "true", - }), - expect.objectContaining({ name: "tags", value: "[]" }), - expect.objectContaining({ name: "ides", value: "[]" }), - ], + rich_parameter_values: parameters.map((p) => ({ + name: p.name, + value: p.value.value, + })), }), ); }); @@ -607,38 +678,20 @@ describe("CreateWorkspacePage", () => { }); describe("URL Parameters", () => { - it("pre-fills parameters from URL", async () => { - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?param.instance_type=t3.large¶m.cpu_count=4`, - ); - await waitForLoaderToBeRemoved(); - - expect(screen.getByText(/instance type/i)).toBeInTheDocument(); - expect(screen.getByText("CPU Count")).toBeInTheDocument(); - }); - it("uses custom template version when specified", async () => { - const customVersionId = "custom-version-123"; - - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?version=${customVersionId}`, - ); - - await waitFor(() => { - expect(API.templateVersionDynamicParameters).toHaveBeenCalledWith( - customVersionId, - MockUserOwner.id, - expect.any(Object), - ); - }); + const version = "custom-version-123"; + const { mockPublisher } = await renderPageWithSocket({ version }); + await expectSocketHandshake({ mockPublisher, parameters: [], version }); }); it("pre-fills workspace name from URL", async () => { const workspaceName = "my-custom-workspace"; - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?name=${workspaceName}`, - ); + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?name=${workspaceName}`, + }); + await expectSocketHandshake({ mockPublisher, parameters: [] }); + await waitForLoaderToBeRemoved(); await waitFor(() => { @@ -651,14 +704,26 @@ describe("CreateWorkspacePage", () => { }); describe("URL Presets", () => { + const parameters = [MockDropdownParameter]; it("resolves a preset from the URL and selects it in the form", async () => { - vi.spyOn(API, "getTemplateVersionPresets").mockResolvedValue([ - mockGpuPreset, - ]); + const { mockPublisher } = await renderPageWithSocket({ + preset: mockGpuPreset, + }); + await expectSocketHandshake({ mockPublisher, parameters }); + + // Respond to the client's init message. + await act(async () => { + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 2, + parameters, + diagnostics: [], + }), + }), + ); + }); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?preset=gpu-large`, - ); await waitForLoaderToBeRemoved(); expect( @@ -667,19 +732,12 @@ describe("CreateWorkspacePage", () => { }); it("resolves a preset against the pinned template version", async () => { - const getTemplateVersionPresetsSpy = vi - .spyOn(API, "getTemplateVersionPresets") - .mockResolvedValue([mockGpuPreset]); - - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?version=custom-version&preset=gpu-large`, - ); - - await waitFor(() => { - expect(getTemplateVersionPresetsSpy).toHaveBeenCalledWith( - "custom-version", - ); + const version = "custom-version"; + const { mockPublisher } = await renderPageWithSocket({ + version, + preset: mockGpuPreset, }); + await expectSocketHandshake({ mockPublisher, parameters, version }); }); it("falls back to form mode when auto-create cannot resolve the preset", async () => { @@ -690,9 +748,11 @@ describe("CreateWorkspacePage", () => { mockGpuPreset, ]); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?mode=auto&preset=missing`, - ); + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?mode=auto&preset=missing`, + }); + await expectSocketHandshake({ mockPublisher, parameters: [] }); + await waitForLoaderToBeRemoved(); expect( @@ -717,10 +777,10 @@ describe("CreateWorkspacePage", () => { new Error("presets unavailable"), ); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?mode=auto&preset=gpu-large`, - ); - await waitForLoaderToBeRemoved(); + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?mode=auto&preset=gpu-large`, + }); + await expectSocketHandshake({ mockPublisher, parameters: [] }); expect( screen.queryByRole("button", { name: /confirm and create/i }), @@ -735,16 +795,39 @@ describe("CreateWorkspacePage", () => { }); it("uses preset parameters instead of param values", async () => { - vi.spyOn(API, "getTemplateVersionPresets").mockResolvedValue([ - mockGpuPreset, - ]); + const parameters = [MockDropdownParameter, MockSliderParameter]; + const urlfill = { + [MockDropdownParameter.name]: "t3.small", + [MockSliderParameter.name]: "99", + }; + const { mockPublisher } = await renderPageWithSocket({ + preset: mockGpuPreset, + // Will be overridden by the preset values. + urlfill, + }); + await expectSocketHandshake({ + mockPublisher, + parameters, + urlfill, + preset: mockGpuPreset, + }); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?preset=gpu-large¶m.instance_type=t3.small¶m.cpu_count=99`, - ); - await waitForLoaderToBeRemoved(); + // Respond to the client's init message. Even though this uses the + // default values, it should not clobber the preset values. + await act(async () => { + mockPublisher.publishMessage( + new MessageEvent("message", { + data: JSON.stringify({ + id: 2, + parameters, + diagnostics: [], + }), + }), + ); + }); - expect(screen.getAllByText(/param\.\*/i).length).toBeGreaterThan(0); + // No parameters show since they are under the preset section toggle. + await expectFormFields({ parameters: [] }); const nameInput = screen.getByRole("textbox", { name: /workspace name/i, @@ -760,13 +843,10 @@ describe("CreateWorkspacePage", () => { "test-user", expect.objectContaining({ template_version_preset_id: mockGpuPreset.ID, - rich_parameter_values: expect.arrayContaining([ - expect.objectContaining({ - name: "instance_type", - value: "t3.medium", - }), - expect.objectContaining({ name: "cpu_count", value: "4" }), - ]), + rich_parameter_values: mockGpuPreset.Parameters.map((p) => ({ + name: p.Name, + value: p.Value, + })), }), ); }); @@ -776,13 +856,16 @@ describe("CreateWorkspacePage", () => { vi.spyOn(API, "getTemplateVersionExternalAuth").mockResolvedValue([ MockTemplateVersionExternalAuthGithubAuthenticated, ]); - vi.spyOn(API, "getTemplateVersionPresets").mockResolvedValue([ - mockGpuPreset, - ]); - renderCreateWorkspacePageWithSocket( - `/templates/${MockTemplate.name}/workspace?mode=auto&preset=gpu-large&name=preset-workspace`, - ); + const { mockPublisher } = await renderPageWithSocket({ + route: `/templates/${MockTemplate.name}/workspace?mode=auto&name=preset-workspace`, + preset: mockGpuPreset, + }); + await expectSocketHandshake({ + mockPublisher, + parameters: [], + preset: mockGpuPreset, + }); const confirmButton = await screen.findByRole("button", { name: /confirm and create/i, @@ -804,8 +887,8 @@ describe("CreateWorkspacePage", () => { describe("Navigation", () => { it("navigates to workspace after successful creation", async () => { - const { router } = renderCreateWorkspacePageWithSocket(); - await waitForLoaderToBeRemoved(); + const { mockPublisher, router } = await renderPageWithSocket(); + await expectSocketHandshake({ mockPublisher, parameters: [] }); const nameInput = screen.getByRole("textbox", { name: /workspace name/i, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 9873339cd0..e3df74b64f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -22,13 +22,11 @@ import type { DynamicParametersRequest, DynamicParametersResponse, MinimalUser, - PreviewParameter, Workspace, } from "#/api/typesGenerated"; import { Loader } from "#/components/Loader/Loader"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { useExternalAuth } from "#/hooks/useExternalAuth"; -import { getInitialParameterValues } from "#/modules/workspaces/DynamicParameter/DynamicParameter"; import { generateWorkspaceName } from "#/modules/workspaces/generateWorkspaceName"; import { pageTitle } from "#/utils/page"; import type { AutofillBuildParameter } from "#/utils/richParameters"; @@ -51,10 +49,14 @@ const CreateWorkspacePage: FC = () => { const [latestResponse, setLatestResponse] = useState(null); + // The current expected response ID. Starts at -1 because the backend sends + // an initial message when the web socket is connected with -1. const wsResponseId = useRef(-1); const ws = useRef(null); const [wsError, setWsError] = useState(null); - const initialParamsSentRef = useRef(false); + // The expected ID of the init message, so we can wait until the initial + // parameters have gone through before rendering the form. + const [initId, setInitId] = useState(Number.NaN); const customVersionId = searchParams.get("version") ?? undefined; const defaultName = searchParams.get("name"); @@ -155,61 +157,50 @@ const CreateWorkspacePage: FC = () => { const hasIgnoredUrlParams = urlAutofillParameters.length > 0 && urlPresetResult.preset !== undefined; + // sendMessage increments the ID and sends the form values on the web socket + // and returns true. If the socket is not open, it does not increment the ID + // and returns false. const sendMessage = ( formValues: Record, ownerId?: string, - ) => { + ): boolean => { const request: DynamicParametersRequest = { id: wsResponseId.current + 1, owner_id: ownerId ?? owner.id, inputs: formValues, }; if (ws.current && ws.current.readyState === WebSocket.OPEN) { - ws.current.send(JSON.stringify(request)); wsResponseId.current = wsResponseId.current + 1; + ws.current.send(JSON.stringify(request)); + return true; } + if (ws.current) { + console.error( + "Tried to send message but the web socket state is %s", + ws.current.readyState, + request, + ); + } + return false; }; - // On page load, sends all initial parameter values to the websocket - // (including defaults and autofilled from the url) - // This ensures the backend has the complete initial state of the form, - // which is vital for correctly rendering dynamic UI elements where parameter visibility - // or options might depend on the initial values of other parameters. - const sendInitialParameters = useEffectEvent( - (parameters: PreviewParameter[]) => { - if (initialParamsSentRef.current) return; - if (parameters.length === 0) return; - - const initialFormValues = getInitialParameterValues( - parameters, - autofillParameters, - ); - if (initialFormValues.length === 0) return; - - const initialParamsToSend: Record = {}; - for (const param of initialFormValues) { - if (param.name && param.value !== undefined) { - initialParamsToSend[param.name] = param.value; - } - } - - if (Object.keys(initialParamsToSend).length === 0) return; - - sendMessage(initialParamsToSend); - initialParamsSentRef.current = true; - }, - ); - - const onMessage = useEffectEvent((response: DynamicParametersResponse) => { - if (latestResponse && latestResponse?.id >= response.id) { + // Send the initial parameters if necessary and mark the ID of the response we + // need to wait for until we can finally render the form with the right state. + const sendInitialParameters = useEffectEvent(() => { + if (!Number.isNaN(initId)) { return; } - - if (!initialParamsSentRef.current && response.parameters?.length > 0) { - sendInitialParameters([...response.parameters]); + if (autofillParameters.length > 0) { + const values = Object.fromEntries( + autofillParameters.map((afp) => [afp.name, afp.value]), + ); + if (!sendMessage(values)) { + return; + } } - - setLatestResponse(response); + // If there were no parameters to send, this will end up just using the + // response we already have. Otherwise it will wait for the next response. + setInitId(wsResponseId.current); }); // Initialize the WebSocket connection when there is a valid template version ID @@ -220,7 +211,17 @@ const CreateWorkspacePage: FC = () => { realizedVersionId, defaultOwner.id, { - onMessage, + // Send initial parameters once the web socket is open. + onOpen: () => { + sendInitialParameters(); + }, + // Record the latest message every time we get one from the web + // socket. Stale responses are discarded. + onMessage: (response: DynamicParametersResponse) => { + if (response.id >= wsResponseId.current) { + setLatestResponse(response); + } + }, onError: (error) => { if (ws.current === socket) { setWsError(error); @@ -373,12 +374,19 @@ const CreateWorkspacePage: FC = () => { return [...latestResponse.parameters].sort((a, b) => a.order - b.order); }, [latestResponse?.parameters]); + const isInitializing = + !latestResponse || + Number.isNaN(initId) || + latestResponse.id < initId || + (ws.current && ws.current.readyState === WebSocket.CONNECTING); + const shouldShowLoader = !templateQuery.data || isLoadingFormData || isLoadingExternalAuth || autoCreateReady || (!latestResponse && !wsError) || + (isInitializing && !wsError) || (effectivePresetName && !templateVersionPresetsQuery.isSuccess && !templateVersionPresetsQuery.isError); diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx index 1dfa7a8f70..6ab7ad9a72 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx @@ -67,7 +67,7 @@ const WorkspaceParametersPage: FC = () => { })) ?? []; // sendMessage increments the ID and sends the form values on the web socket - // and return true. If the socket is not open, it does not increment the ID + // and returns true. If the socket is not open, it does not increment the ID // and returns false. const sendMessage = (formValues: Record): boolean => { const request: DynamicParametersRequest = { diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 02949a91cc..92f6ac4027 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -3560,7 +3560,7 @@ export const MockDropdownParameter: TypesGen.PreviewParameter = { order: 1, }; -const MockTagSelectParameter: TypesGen.PreviewParameter = { +export const MockTagSelectParameter: TypesGen.PreviewParameter = { ...MockPreviewParameter, name: "tags", display_name: "Tags", @@ -3578,7 +3578,7 @@ const MockTagSelectParameter: TypesGen.PreviewParameter = { order: 4, }; -const MockSwitchParameter: TypesGen.PreviewParameter = { +export const MockSwitchParameter: TypesGen.PreviewParameter = { ...MockPreviewParameter, name: "enable_monitoring", display_name: "Enable Monitoring", @@ -3613,7 +3613,7 @@ export const MockSliderParameter: TypesGen.PreviewParameter = { order: 2, }; -const MockMultiSelectParameter: TypesGen.PreviewParameter = { +export const MockMultiSelectParameter: TypesGen.PreviewParameter = { ...MockPreviewParameter, name: "ides", display_name: "IDEs", @@ -3673,19 +3673,6 @@ export const MockValidationParameter: TypesGen.PreviewParameter = { order: 1, }; -export const MockDynamicParametersResponse: TypesGen.DynamicParametersResponse = - { - id: 1, - parameters: [ - MockDropdownParameter, - MockSliderParameter, - MockSwitchParameter, - MockTagSelectParameter, - MockMultiSelectParameter, - ], - diagnostics: [], - }; - export const MockDynamicParametersResponseWithError: TypesGen.DynamicParametersResponse = { id: 2, diff --git a/site/src/testHelpers/parameters.ts b/site/src/testHelpers/parameters.ts index f3699e2dec..2484381590 100644 --- a/site/src/testHelpers/parameters.ts +++ b/site/src/testHelpers/parameters.ts @@ -2,7 +2,12 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import type * as TypesGen from "#/api/typesGenerated"; -type Parameter = TypesGen.WorkspaceBuildParameter | TypesGen.PreviewParameter; +type BuildParameter = TypesGen.WorkspaceBuildParameter & { + display_name?: string; + form_type?: TypesGen.ParameterFormType; +}; + +type Parameter = BuildParameter | TypesGen.PreviewParameter; export function isBuildParameter( parameter: Parameter, @@ -11,8 +16,9 @@ export function isBuildParameter( } // checkParameters waits until all the provided parameters have the expected -// display value within the parameters form. Requires that the form and -// parameters all have test IDs (`form` and `parameter-field-$name`). +// display value within the parameters form and that there are no additional +// parameters. Requires that the form and parameters all have test IDs (`form` +// and `parameter-field-$name`). export async function checkParameters(...parameters: Parameter[]) { const form = screen.getByTestId("form"); await waitFor(() => { @@ -23,8 +29,34 @@ export async function checkParameters(...parameters: Parameter[]) { const value = isBuildParameter(parameter) ? parameter.value : parameter.value.value; - expect(within(field).getByDisplayValue(value)).toBeInTheDocument(); + const type = parameter.form_type || "input"; + switch (type) { + case "switch": + if (value === "true") { + expect(within(field).getByRole("switch")).toBeChecked(); + } else { + expect(within(field).getByRole("switch")).not.toBeChecked(); + } + break; + case "dropdown": + expect(within(field).getByRole("combobox")).toHaveTextContent( + value || "Select option", + ); + break; + case "multi-select": + case "tag-select": + // TODO: Validate these values as well, not just that they exist. + break; + case "input": + case "slider": + expect(within(field).getByDisplayValue(value)).toBeInTheDocument(); + break; + default: + throw new Error(`checking ${type} fields is not implemented`); + } } + const fields = within(form).queryAllByTestId(/^parameter-field-/); + expect(fields).toHaveLength(parameters.length); }); } @@ -35,15 +67,24 @@ export async function editParameters(...parameters: Parameter[]) { const form = screen.getByTestId("form"); for (const parameter of parameters) { const field = within(form).getByTestId(`parameter-field-${parameter.name}`); - const input = await within(field).findByRole("textbox", { - name: new RegExp(parameter.name, "i"), - }); - await userEvent.clear(input); const value = isBuildParameter(parameter) ? parameter.value : parameter.value.value; - if (value !== "") { - await userEvent.type(input, value); + const type = parameter.form_type || "input"; + const label = parameter.display_name || parameter.name; + switch (type) { + case "input": { + const input = await within(field).findByLabelText( + new RegExp(label, "i"), + ); + await userEvent.clear(input); + if (value !== "") { + await userEvent.type(input, value); + } + break; + } + default: + throw new Error(`editing ${type} fields is not implemented`); } } } diff --git a/site/src/testHelpers/websockets.ts b/site/src/testHelpers/websockets.ts index 137fafeeea..885d2e68fd 100644 --- a/site/src/testHelpers/websockets.ts +++ b/site/src/testHelpers/websockets.ts @@ -18,7 +18,7 @@ type CallbackStore = { [K in keyof WebSocketEventMap]: Set<(event: WebSocketEventMap[K]) => void>; }; -type MockWebSocket = Omit & { +export type MockWebSocket = Omit & { /** * A version of the WebSocket `send` method that has been pre-wrapped inside * a vitest mock.