diff --git a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-add.png b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-add.png index d39596dc..87c3a5be 100644 Binary files a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-add.png and b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-add.png differ diff --git a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-edit.png b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-edit.png index 33356fdc..4e9cf8d2 100644 Binary files a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-edit.png and b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-edit.png differ diff --git a/e2e/vendors/vendors.spec.ts b/e2e/vendors/vendors.spec.ts index f242b7cf..0a4dca8d 100644 --- a/e2e/vendors/vendors.spec.ts +++ b/e2e/vendors/vendors.spec.ts @@ -450,7 +450,30 @@ test.describe("Vendor directory prototype parity", () => { page.getByRole("button", { name: "Add Vendor", exact: true }).last(), ).toBeEnabled(); - await page.getByRole("combobox", { name: "Company" }).click(); + const companyLabel = page + .getByRole("dialog", { name: /Add Vendor/ }) + .locator("label") + .filter({ hasText: "Company (required)" }); + await expect(companyLabel).toBeVisible(); + const companyLabelMetrics = await companyLabel.evaluate((element) => { + const field = element.closest(".MuiFormControl-root"); + const labelRect = element.getBoundingClientRect(); + const fieldRect = field?.getBoundingClientRect(); + return { + clientWidth: element.clientWidth, + scrollWidth: element.scrollWidth, + withinFieldGeometry: fieldRect + ? labelRect.left >= fieldRect.left && + labelRect.right <= fieldRect.right && + labelRect.top >= fieldRect.top && + labelRect.bottom <= fieldRect.bottom + : false, + }; + }); + expect(companyLabelMetrics.clientWidth).toBeGreaterThanOrEqual(companyLabelMetrics.scrollWidth); + expect(companyLabelMetrics.withinFieldGeometry).toBe(true); + + await page.getByRole("combobox", { name: "Company (required)" }).click(); await page.getByRole("option", { name: "Gateway Plumbing" }).click(); await expect(page.getByLabel("Company Phone (optional)")).toHaveValue("314-555-0100"); await expect(page.getByRole("textbox", { name: "Email (optional)", exact: true })).toHaveValue( @@ -560,7 +583,7 @@ test.describe("Vendor directory prototype parity", () => { await expect( page.getByRole("button", { name: "Add Vendor", exact: true }).last(), ).toBeEnabled(); - await page.getByRole("combobox", { name: "Company" }).fill("Independent Vendor LLC"); + await page.getByRole("combobox", { name: "Company (required)" }).fill("Independent Vendor LLC"); await page.getByLabel("Company Phone (optional)").fill("3145550199"); await page.getByRole("button", { name: "Add Vendor", exact: true }).last().click(); diff --git a/src/app/(protected)/vendors/_components/vendor-company-contact-fields.tsx b/src/app/(protected)/vendors/_components/vendor-company-contact-fields.tsx new file mode 100644 index 00000000..e379223f --- /dev/null +++ b/src/app/(protected)/vendors/_components/vendor-company-contact-fields.tsx @@ -0,0 +1,89 @@ +import { Controller, useWatch, type Control, type FieldErrors } from "react-hook-form"; +import { Stack, TextField } from "@mui/material"; +import { Text } from "@/components/ui/text"; +import { + VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, + type VendorCompanyRosterFormValues, +} from "@/domain/vendors/schemas/vendor-roster-schema"; +import { formatNorthAmericanPhone } from "@/lib/format/na-phone"; + +const COMPANY_CONTACT_ERROR_ID = "vendor-company-contact-error"; +const COMPANY_PHONE_HELPER_ID = "vendor-company-phone-helper"; +const COMPANY_EMAIL_HELPER_ID = "vendor-company-email-helper"; + +interface VendorCompanyContactFieldsProps { + control: Control; + errors: FieldErrors; +} + +export function VendorCompanyContactFields({ control, errors }: VendorCompanyContactFieldsProps) { + const companyPhone = useWatch({ control, name: "companyPhone" }) ?? ""; + const companyEmail = useWatch({ control, name: "email" }) ?? ""; + const showCompanyContactError = Boolean( + errors.companyContact && !companyPhone.trim() && !companyEmail.trim(), + ); + const contactDescribedBy = (helperId: string, hasHelper: boolean) => + [hasHelper ? helperId : null, showCompanyContactError ? COMPANY_CONTACT_ERROR_ID : null] + .filter(Boolean) + .join(" ") || undefined; + + return ( + <> + + ( + field.onChange(formatNorthAmericanPhone(event.target.value))} + error={Boolean(errors.companyPhone)} + helperText={errors.companyPhone?.message} + slotProps={{ + formHelperText: { id: COMPANY_PHONE_HELPER_ID }, + htmlInput: { + "aria-describedby": contactDescribedBy( + COMPANY_PHONE_HELPER_ID, + Boolean(errors.companyPhone?.message), + ), + }, + }} + fullWidth + /> + )} + /> + ( + + )} + /> + + + {VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE} + + + ); +} diff --git a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx index df32ae32..8ac61297 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx @@ -28,6 +28,7 @@ import { type VendorCompanyRosterFormValues, } from "@/domain/vendors/schemas/vendor-roster-schema"; import type { VendorFacetCompany } from "@/domain/vendors/types/vendor"; +import { VendorCompanyContactFields } from "./vendor-company-contact-fields"; interface VendorRosterFormFieldsProps { control: Control; @@ -64,9 +65,10 @@ function CompanyNameField({ render={({ field }) => ( ( )} /> @@ -143,40 +149,7 @@ function CompanyFields({ onSelectCompany={onSelectCompany} onClearSelectedCompany={onClearSelectedCompany} /> - - ( - field.onChange(formatNorthAmericanPhone(event.target.value))} - error={Boolean(errors.companyPhone)} - helperText={errors.companyPhone?.message} - fullWidth - /> - )} - /> - ( - - )} - /> - + ; const baseCompanyFields = { name: z.string().trim().min(1, "Company is required"), + // Validation-only identity for the phone-or-email group. It is never + // registered as an input and Zod omits it from parsed values when absent, + // so it cannot enter the vendor payload. + companyContact: z.never().optional(), companyPhone: northAmericanPhone, email: optionalEmail, address: z.string(), @@ -64,14 +68,20 @@ const baseCompanyFields = { technicians: z.array(rosterTechnicianSchema), }; +export const VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE = + "Provide a company phone or email (at least one required)"; + +// The phone-or-email rule spans two optional inputs, so it is reported at a +// validation-only path instead of being attributed to `companyPhone`, which +// would mark that input individually invalid. export const vendorCompanyRosterSchema = z.object(baseCompanyFields).superRefine((data, ctx) => { const hasPhone = Boolean(data.companyPhone && data.companyPhone.trim()); const hasEmail = Boolean(data.email && data.email.trim()); if (!hasPhone && !hasEmail) { ctx.addIssue({ code: z.ZodIssueCode.custom, - path: ["companyPhone"], - message: "Provide a company phone or email (at least one required)", + path: ["companyContact"], + message: VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, }); } }); diff --git a/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx index 661f00d7..c543abb7 100644 --- a/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx +++ b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx @@ -1,7 +1,16 @@ import { fireEvent, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { useForm } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; import { describe, expect, it, vi } from "vitest"; import { VendorCreateModal } from "@/app/(protected)/vendors/_components/vendor-create-modal"; +import { VendorRosterFormFields } from "@/app/(protected)/vendors/_components/vendor-roster-form-fields"; +import { + VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, + emptyVendorCompanyRosterForm, + vendorCompanyRosterSchema, + type VendorCompanyRosterFormValues, +} from "@/domain/vendors/schemas/vendor-roster-schema"; import { renderWithProviders } from "@/test/test-utils"; vi.mock("@/domain/vendors/use-cases/use-vendor-facets", () => ({ @@ -18,28 +27,122 @@ vi.mock("@/domain/vendors/use-cases/use-save-vendor-company-roster", async () => }; }); +function PlainCompanyFieldsHarness() { + const { control, handleSubmit, formState } = useForm({ + defaultValues: emptyVendorCompanyRosterForm, + resolver: zodResolver(vendorCompanyRosterSchema), + mode: "onChange", + }); + return ( +
{})}> + + + ); +} + +function renderCreateModal() { + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); +} + +async function fillCompanyNameAndSubmit(name = "Gateway Plumbing") { + fireEvent.change(screen.getByRole("combobox", { name: "Company (required)" }), { + target: { value: name }, + }); + await userEvent.click(screen.getByRole("button", { name: "Add Vendor" })); +} + describe("VendorCreateModal validation", () => { it("explains the company phone-or-email requirement after submission", async () => { - renderWithProviders(, { - route: "/vendors", - withAuth: false, - }); + renderCreateModal(); + await fillCompanyNameAndSubmit(); - fireEvent.change(screen.getByRole("combobox", { name: /Company/ }), { - target: { value: "Gateway Plumbing" }, - }); + expect(await screen.findByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).toBeInTheDocument(); + }); + + it("labels the autocomplete company field exactly, required, with no generated asterisk", () => { + renderCreateModal(); + + const company = screen.getByRole("combobox", { name: "Company (required)" }); + expect(company).toBeRequired(); + expect(screen.getByText("Company (required)")).toBeInTheDocument(); + expect(screen.queryByText("*")).not.toBeInTheDocument(); + expect(document.querySelector(".MuiFormLabel-asterisk")).toBeNull(); + }); + + it("labels the plain company field exactly, required, with no generated asterisk", () => { + renderWithProviders(, { route: "/vendors", withAuth: false }); + + const company = screen.getByRole("textbox", { name: "Company (required)" }); + expect(company).toBeRequired(); + expect(screen.getByText("Company (required)")).toBeInTheDocument(); + expect(screen.queryByText("*")).not.toBeInTheDocument(); + expect(document.querySelector(".MuiFormLabel-asterisk")).toBeNull(); + }); + + it("shows the phone-or-email error once without marking either input individually invalid", async () => { + renderCreateModal(); + await fillCompanyNameAndSubmit(); + + const message = await screen.findByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE); + expect(message).toHaveAttribute("id"); + expect(screen.getAllByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).toHaveLength(1); + + const phone = screen.getByRole("textbox", { name: "Company Phone (optional)" }); + const email = screen.getByRole("textbox", { name: "Email (optional)" }); + expect(phone).not.toHaveAttribute("aria-invalid", "true"); + expect(email).not.toHaveAttribute("aria-invalid", "true"); + expect(phone.getAttribute("aria-describedby")).toContain(message.getAttribute("id")); + expect(email.getAttribute("aria-describedby")).toContain(message.getAttribute("id")); + }); + + it("marks only the phone input invalid when the phone is malformed", async () => { + renderCreateModal(); + await fillCompanyNameAndSubmit(); + const phone = screen.getByRole("textbox", { name: "Company Phone (optional)" }); + const email = screen.getByRole("textbox", { name: "Email (optional)" }); + + await userEvent.type(phone, "314"); await userEvent.click(screen.getByRole("button", { name: "Add Vendor" })); - expect( - await screen.findByText("Provide a company phone or email (at least one required)"), - ).toBeInTheDocument(); + expect(await screen.findByText("Enter a 10-digit phone number")).toBeInTheDocument(); + expect(phone).toHaveAttribute("aria-invalid", "true"); + expect(email).not.toHaveAttribute("aria-invalid", "true"); + expect(screen.queryByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).not.toBeInTheDocument(); + }); + + it("marks only the email input invalid when the email is malformed", async () => { + renderCreateModal(); + await fillCompanyNameAndSubmit(); + const phone = screen.getByRole("textbox", { name: "Company Phone (optional)" }); + const email = screen.getByRole("textbox", { name: "Email (optional)" }); + + await userEvent.type(email, "not-an-email"); + await userEvent.click(screen.getByRole("button", { name: "Add Vendor" })); + + expect(await screen.findByText("Invalid email")).toBeInTheDocument(); + expect(email).toHaveAttribute("aria-invalid", "true"); + expect(phone).not.toHaveAttribute("aria-invalid", "true"); + expect(screen.queryByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).not.toBeInTheDocument(); + }); + + it("clears the phone-or-email error once a valid phone is entered", async () => { + renderCreateModal(); + await fillCompanyNameAndSubmit(); + const phone = screen.getByRole("textbox", { name: "Company Phone (optional)" }); + + expect(await screen.findByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).toBeInTheDocument(); + + await userEvent.type(phone, "3145550100"); + await waitFor(() => + expect(screen.queryByText(VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE)).not.toBeInTheDocument(), + ); }); it("shows the notes limit and live character counter", async () => { - renderWithProviders(, { - route: "/vendors", - withAuth: false, - }); + renderCreateModal(); const notes = screen.getByRole("textbox", { name: "Notes (optional)" }); fireEvent.change(notes, { target: { value: "abc" } }); diff --git a/src/test/app/(protected)/vendors/vendors-list.test.tsx b/src/test/app/(protected)/vendors/vendors-list.test.tsx index cb7164e1..8d51d3cf 100644 --- a/src/test/app/(protected)/vendors/vendors-list.test.tsx +++ b/src/test/app/(protected)/vendors/vendors-list.test.tsx @@ -329,7 +329,7 @@ describe("VendorsListPage", () => { renderWithProviders(, { route: "/vendors", withAuth: false }); await userEvent.click(screen.getByRole("button", { name: "Edit vendor Gateway Plumbing" })); - const company = screen.getByRole("textbox", { name: "Company" }); + const company = screen.getByRole("textbox", { name: "Company (required)" }); await userEvent.clear(company); await userEvent.type(company, "Draft Company Name"); await userEvent.click(screen.getByRole("switch", { name: "Active status" })); @@ -362,7 +362,9 @@ describe("VendorsListPage", () => { await userEvent.click(screen.getByRole("button", { name: "Edit vendor Gateway Plumbing" })); expect(screen.getByRole("button", { name: "Save changes" })).toBeInTheDocument(); - expect(screen.getByRole("textbox", { name: "Company" })).toHaveValue("Gateway Plumbing"); + expect(screen.getByRole("textbox", { name: "Company (required)" })).toHaveValue( + "Gateway Plumbing", + ); }); it("opens inline company edit when the row has no vendor id", async () => { @@ -385,6 +387,8 @@ describe("VendorsListPage", () => { await userEvent.click(screen.getByRole("button", { name: "Edit vendor Gateway Plumbing" })); expect(screen.getByRole("button", { name: "Save changes" })).toBeInTheDocument(); - expect(screen.getByRole("textbox", { name: "Company" })).toHaveValue("Gateway Plumbing"); + expect(screen.getByRole("textbox", { name: "Company (required)" })).toHaveValue( + "Gateway Plumbing", + ); }); }); diff --git a/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts b/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts index 646f8e73..d5772d1a 100644 --- a/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts +++ b/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, emptyRosterTechnician, emptyVendorCompanyRosterForm, isAbsoluteHttpsUrl, @@ -30,12 +31,29 @@ describe("vendorCompanyRosterSchema", () => { if (!result.success) { expect( result.error.issues.some( - (issue) => issue.message === "Provide a company phone or email (at least one required)", + (issue) => issue.message === VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, ), ).toBe(true); } }); + it("reports the phone-or-email rule at the companyContact path, not on companyPhone", () => { + const result = vendorCompanyRosterSchema.safeParse({ + ...emptyVendorCompanyRosterForm, + name: "Solo Co", + }); + expect(result.success).toBe(false); + if (!result.success) { + const issue = result.error.issues.find( + (candidate) => candidate.message === VENDOR_COMPANY_CONTACT_REQUIRED_MESSAGE, + ); + expect(issue?.path).toEqual(["companyContact"]); + expect(result.error.issues.some((candidate) => candidate.path.includes("companyPhone"))).toBe( + false, + ); + } + }); + it("accepts a company with zero technicians when phone is present", () => { const result = vendorCompanyRosterSchema.safeParse({ ...emptyVendorCompanyRosterForm, @@ -43,6 +61,7 @@ describe("vendorCompanyRosterSchema", () => { companyPhone: "(314) 555-0100", }); expect(result.success).toBe(true); + if (result.success) expect(result.data).not.toHaveProperty("companyContact"); }); it("accepts a company with email only and multiple technicians", () => {