From d6eaf48cf98ca7ddf9b2737e9ef745908644683d Mon Sep 17 00:00:00 2001 From: Jeremy Ruppel Date: Wed, 12 Aug 2026 19:37:51 +0000 Subject: [PATCH] fix(site/src/pages/TemplateBuilder): validate customizations form before submit Convert the Template Builder customizations step to a Formik form with a Yup schema and surface aggregated validation errors in an Alert at the top of the step. This avoids inline field errors that would disrupt the horizontal two-column layout, and stops empty organization/name submissions from hitting the API with an unhelpful backend error. The wizard's Create Template button submits the step form via the form attribute. Only valid submits reach the create mutation; a genuinely invalid organization still surfaces the backend message via the existing error alert. --- .../TemplateBuilder/TemplateBuilderPage.tsx | 13 +- .../TemplateBuilderPageView.tsx | 50 ++++-- .../TemplateCustomizationsStep.stories.tsx | 147 ++++++++++++++++++ .../TemplateCustomizationsStep.tsx | 103 +++++++++--- .../pages/TemplateBuilder/wizardState.test.ts | 31 ++-- site/src/pages/TemplateBuilder/wizardState.ts | 38 +++-- 6 files changed, 314 insertions(+), 68 deletions(-) create mode 100644 site/src/pages/TemplateBuilder/TemplateCustomizationsStep.stories.tsx diff --git a/site/src/pages/TemplateBuilder/TemplateBuilderPage.tsx b/site/src/pages/TemplateBuilder/TemplateBuilderPage.tsx index e1a443b6580..e67d9fef927 100644 --- a/site/src/pages/TemplateBuilder/TemplateBuilderPage.tsx +++ b/site/src/pages/TemplateBuilder/TemplateBuilderPage.tsx @@ -17,7 +17,11 @@ import type { SelectedBaseMeta, TemplateBuilderWizardState, } from "./wizardState"; -import { toCreateTemplateRequest, toSelectedBaseMeta } from "./wizardState"; +import { + type CustomizationsFormValues, + toCreateTemplateRequest, + toSelectedBaseMeta, +} from "./wizardState"; const TemplateBuilderPage: FC = () => { const navigate = useNavigate(); @@ -109,8 +113,11 @@ const TemplateBuilderPage: FC = () => { return ; } - const handleCreate = (state: TemplateBuilderWizardState) => { - const req = toCreateTemplateRequest(state); + const handleCreate = ( + state: TemplateBuilderWizardState, + customizations: CustomizationsFormValues, + ) => { + const req = toCreateTemplateRequest(state, customizations); const durationSeconds = (Date.now() - state.enteredAt) / 1000; createMutation.mutate(req, { diff --git a/site/src/pages/TemplateBuilder/TemplateBuilderPageView.tsx b/site/src/pages/TemplateBuilder/TemplateBuilderPageView.tsx index 53fbacfb471..db5ceb74276 100644 --- a/site/src/pages/TemplateBuilder/TemplateBuilderPageView.tsx +++ b/site/src/pages/TemplateBuilder/TemplateBuilderPageView.tsx @@ -46,8 +46,12 @@ import { WIZARD_STEPS, } from "./steps"; import { TemplateAlternatives } from "./TemplateAlternatives"; -import { TemplateCustomizationsStep } from "./TemplateCustomizationsStep"; import { + TEMPLATE_CUSTOMIZATIONS_FORM_ID, + TemplateCustomizationsStep, +} from "./TemplateCustomizationsStep"; +import { + type CustomizationsFormValues, initWizardState, type SelectedBaseMeta, type TemplateBuilderWizardState, @@ -59,7 +63,10 @@ interface TemplateBuilderPageViewProps { error: unknown; basesData: TemplateBuilderBasesResponse | undefined; preselectedBase?: SelectedBaseMeta; - onCreateTemplate: (state: TemplateBuilderWizardState) => void; + onCreateTemplate: ( + state: TemplateBuilderWizardState, + customizations: CustomizationsFormValues, + ) => void; createError: Error | null; isCreating: boolean; onClearCreateError?: () => void; @@ -154,13 +161,16 @@ export const TemplateBuilderPageView: FC = ({ }; const handleNext = () => { - if (isLastStep) { - onCreateTemplate(state); - return; - } navigateToStep(nextIndex); }; + const handleCreate = useCallback( + (values: CustomizationsFormValues) => { + onCreateTemplate(state, values); + }, + [onCreateTemplate, state], + ); + const handleProvisionerStatusChange = useCallback( (value: boolean | undefined) => { dispatch({ type: "SET_HAS_PROVISIONERS", value }); @@ -278,6 +288,7 @@ export const TemplateBuilderPageView: FC = ({ handleProvisionerStatusChange, handleDeselectModule, registerModuleRef, + handleCreate, )} @@ -290,9 +301,19 @@ export const TemplateBuilderPageView: FC = ({ Back )} - + {isLastStep ? ( + + ) : ( + + )} {currentStep.id === "base-infra" && } @@ -332,6 +353,7 @@ function renderStepContent( onProvisionerStatusChange: (value: boolean | undefined) => void, onRemoveModule: (moduleId: string) => void, registerModuleRef: (moduleId: string, node: HTMLDivElement | null) => void, + onCreate: (values: CustomizationsFormValues) => void, ): ReactNode { switch (stepId) { case "base-infra": @@ -387,13 +409,7 @@ function renderStepContent( {createError != null && } - dispatch({ - type: "SET_CUSTOMIZATION", - field, - value, - }) - } + onCreate={onCreate} onProvisionerStatusChange={onProvisionerStatusChange} /> @@ -426,7 +442,7 @@ function computeCanContinue( moduleVarMap, ); case "customizations": - return state.name.trim() !== "" && state.hasProvisioners !== false; + return state.hasProvisioners !== false; default: return true; } diff --git a/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.stories.tsx b/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.stories.tsx new file mode 100644 index 00000000000..be9d3711a81 --- /dev/null +++ b/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.stories.tsx @@ -0,0 +1,147 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, fn, userEvent, within } from "storybook/test"; +import { + getProvisionerDaemonsKey, + permittedOrganizations, +} from "#/api/queries/organizations"; +import { Button } from "#/components/Button/Button"; +import { + MockDefaultOrganization, + MockOrganization, + MockOrganization2, + MockProvisioner, +} from "#/testHelpers/entities"; +import { + TEMPLATE_CUSTOMIZATIONS_FORM_ID, + TemplateCustomizationsStep, +} from "./TemplateCustomizationsStep"; +import { + initialWizardState, + type TemplateBuilderWizardState, +} from "./wizardState"; + +const baseState: TemplateBuilderWizardState = { + ...initialWizardState, + baseTemplateId: "docker", + selectedBase: { + id: "docker", + name: "Docker Containers", + iconUrl: "/icon/docker.svg", + hasParameters: false, + hasPrerequisites: false, + }, + name: "docker", + displayName: "Docker Containers", + description: "Run workspaces as Docker containers", + icon: "/icon/docker.svg", +}; + +const permittedOrgsKey = permittedOrganizations({ + object: { resource_type: "template" }, + action: "create", +}).queryKey; + +const provisionersKey = (organizationId: string) => + getProvisionerDaemonsKey(organizationId); + +const meta: Meta = { + title: "pages/TemplateBuilder/TemplateCustomizationsStep", + component: TemplateCustomizationsStep, + args: { + state: baseState, + onCreate: fn(), + onProvisionerStatusChange: fn(), + }, + parameters: { + queries: [ + { key: permittedOrgsKey, data: [MockOrganization, MockOrganization2] }, + ], + }, + // The "Create Template" submit button lives in the wizard's shared nav bar, + // outside this component. It is associated with the form via the `form` + // attribute, so the stories render an equivalent button to exercise submit. + decorators: [ + (Story) => ( +
+ + +
+ ), + ], +}; + +export default meta; +type Story = StoryObj; + +export const MultipleOrganizations: Story = {}; + +// Submitting without choosing an organization surfaces an aggregated error at +// the top of the step instead of an inline field error, and does not call +// onCreate. +export const MissingOrganizationError: Story = { + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + await canvas.findByTestId("organization-autocomplete"); + await userEvent.click( + canvas.getByRole("button", { name: "Create Template" }), + ); + await canvas.findByText("Select an organization to continue."); + await expect(args.onCreate).not.toHaveBeenCalled(); + }, +}; + +// A single permitted organization is auto-selected, so a valid form submits and +// forwards the selected organization id to onCreate. +export const SingleOrganizationSubmits: Story = { + parameters: { + queries: [ + { key: permittedOrgsKey, data: [MockDefaultOrganization] }, + { + key: provisionersKey(MockDefaultOrganization.id), + data: [MockProvisioner], + }, + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + // Wait for the auto-selected org to render in the autocomplete. + await canvas.findByText(MockDefaultOrganization.display_name); + await userEvent.click( + canvas.getByRole("button", { name: "Create Template" }), + ); + await expect(args.onCreate).toHaveBeenCalledWith( + expect.objectContaining({ + organization_id: MockDefaultOrganization.id, + name: "docker", + }), + ); + }, +}; + +// A missing template ID surfaces the name validation message at the top of the +// step and blocks submission. +export const MissingNameError: Story = { + args: { + state: { ...baseState, name: "" }, + }, + parameters: { + queries: [ + { key: permittedOrgsKey, data: [MockDefaultOrganization] }, + { + key: provisionersKey(MockDefaultOrganization.id), + data: [MockProvisioner], + }, + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + await canvas.findByText(MockDefaultOrganization.display_name); + await userEvent.click( + canvas.getByRole("button", { name: "Create Template" }), + ); + await canvas.findByText("Please enter a template id."); + await expect(args.onCreate).not.toHaveBeenCalled(); + }, +}; diff --git a/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.tsx b/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.tsx index 22885bef390..0fc68bd9b61 100644 --- a/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.tsx +++ b/site/src/pages/TemplateBuilder/TemplateCustomizationsStep.tsx @@ -1,5 +1,7 @@ +import { useFormik } from "formik"; import { type FC, useEffect, useState } from "react"; import { useQuery } from "react-query"; +import * as Yup from "yup"; import { permittedOrganizations, provisionerDaemons, @@ -18,23 +20,44 @@ import { TemplateBuilderTitle, } from "#/pages/TemplateBuilder/TemplateBuilderHeader"; import { docs } from "#/utils/docs"; +import { + displayNameValidator, + iconValidator, + nameValidator, +} from "#/utils/formUtils"; import type { + CustomizationsFormValues, SelectedBaseMeta, TemplateBuilderWizardState, } from "./wizardState"; +export const TEMPLATE_CUSTOMIZATIONS_FORM_ID = "template-customizations-form"; + +const MAX_DESCRIPTION_CHAR_LIMIT = 128; + +const validationSchema = Yup.object({ + name: nameValidator("Template ID"), + display_name: displayNameValidator("Display name"), + description: Yup.string().max( + MAX_DESCRIPTION_CHAR_LIMIT, + "Please enter a description that is less than or equal to 128 characters.", + ), + icon: iconValidator, + // An organization is always required: the page is gated on the create- + // template permission, so there is always at least one permitted org, and + // it is auto-selected when only one is available. + organization_id: Yup.string().required("Select an organization to continue."), +}); + interface TemplateCustomizationsStepProps { state: TemplateBuilderWizardState; - onChangeField: ( - field: "organizationId" | "name" | "displayName" | "description" | "icon", - value: string, - ) => void; + onCreate: (values: CustomizationsFormValues) => void; onProvisionerStatusChange: (hasProvisioners: boolean | undefined) => void; } export const TemplateCustomizationsStep: FC< TemplateCustomizationsStepProps -> = ({ state, onChangeField, onProvisionerStatusChange }) => { +> = ({ state, onCreate, onProvisionerStatusChange }) => { const permittedOrgsQuery = useQuery( permittedOrganizations({ object: { resource_type: "template" }, @@ -43,6 +66,19 @@ export const TemplateCustomizationsStep: FC< ); const orgOptions = permittedOrgsQuery.data ?? []; + const form = useFormik({ + initialValues: { + organization_id: "", + name: state.name, + display_name: state.displayName, + description: state.description, + icon: state.icon, + }, + validationSchema, + onSubmit: (values) => onCreate(values), + }); + + // Display object for the autocomplete; the Formik field only stores the id. const [selectedOrg, setSelectedOrg] = useState(null); const { data: provisioners } = useQuery({ @@ -59,25 +95,52 @@ export const TemplateCustomizationsStep: FC< }, [hasProvisioners, onProvisionerStatusChange]); // Auto-select when exactly one org is available. + // biome-ignore lint/correctness/useExhaustiveDependencies: form.setFieldValue is stable useEffect(() => { if (orgOptions.length === 1 && !selectedOrg) { setSelectedOrg(orgOptions[0]); - onChangeField("organizationId", orgOptions[0].id); + void form.setFieldValue("organization_id", orgOptions[0].id); } - }, [orgOptions, selectedOrg, onChangeField]); + }, [orgOptions, selectedOrg]); const handleOrgChange = (org: Organization | null) => { setSelectedOrg(org); - onChangeField("organizationId", org?.id ?? ""); + void form.setFieldValue("organization_id", org?.id ?? ""); }; + // Aggregate validation messages and show them at the top of the step so the + // horizontal two-column layout is not disturbed by inline field errors. + const errorMessages = Object.values(form.errors).filter( + (message): message is string => Boolean(message), + ); + const showErrorSummary = form.submitCount > 0 && errorMessages.length > 0; + return ( -
+
Customizations Add additional configurations. + {showErrorSummary && ( + + {errorMessages.length === 1 ? ( + errorMessages[0] + ) : ( +
    + {errorMessages.map((message) => ( +
  • {message}
    • + ))} +
+ )} + + )} + {showProvisionerWarning && }
@@ -90,10 +153,10 @@ export const TemplateCustomizationsStep: FC<
onChangeField("displayName", e.target.value)} placeholder="My Template" + aria-invalid={Boolean(form.errors.display_name)} />
@@ -120,23 +183,25 @@ export const TemplateCustomizationsStep: FC<