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 6e091c72..d39596dc 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 a80af4db..93fd2ef9 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/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts index 82ae4333..ea04d62d 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts @@ -97,6 +97,7 @@ export interface VendorRosterForm { selectedCompanyId: string | number | null; selectCompany: (company: VendorFacetCompany | null) => Promise; clearSelectedCompany: (nextName?: string) => void; + setTechnicianActive: (index: number, isActive: boolean) => void; resetForm: () => void; } @@ -133,7 +134,7 @@ export function useVendorRosterForm({ values: mode === "update" && routeRoster ? toFormValues(routeRoster) : undefined, mode: "onChange", }); - const { control, handleSubmit, reset, formState } = form; + const { control, handleSubmit, reset, setValue, formState } = form; // react-hook-form exposes formState through a Proxy that only tracks properties read during // render. Reading dirtyFields inside the submit callback would not subscribe and would come // back empty, so it is resolved here on every render. @@ -163,6 +164,16 @@ export function useVendorRosterForm({ clearConflict(); }, [clearConflict, createDefaults, reset, resetSelection]); + const setTechnicianActive = useCallback( + (index: number, isActive: boolean) => { + setValue(`technicians.${index}.isActive`, isActive, { + shouldDirty: true, + shouldValidate: true, + }); + }, + [setValue], + ); + const submit = (formValues: VendorCompanyRosterFormValues) => { setConflict(null); const values = withoutBlankNewTechnicians(formValues); @@ -228,6 +239,7 @@ export function useVendorRosterForm({ selectedCompanyId: selection.selectedCompanyId, selectCompany: selection.selectCompany, clearSelectedCompany: selection.clearSelectedCompany, + setTechnicianActive, resetForm, }; } diff --git a/src/app/(protected)/vendors/_components/use-vendor-roster-resolver.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-resolver.ts index 12e1810a..864bdb4a 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-resolver.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-resolver.ts @@ -8,19 +8,35 @@ import { import { getSingleStatusOnlyChange } from "@/domain/vendors/use-cases/use-save-vendor-company-roster"; import type { VendorCompanyRoster } from "@/domain/vendors/types/vendor"; +function withoutTechnicianStubs( + values: VendorCompanyRosterFormValues, +): VendorCompanyRosterFormValues { + return { + ...values, + technicians: values.technicians.filter((technician) => { + if (technician === null || typeof technician !== "object" || Array.isArray(technician)) { + return true; + } + const candidate = technician as unknown as Record; + const keys = Object.keys(candidate); + const isGeneratedStatusStub = + keys.length === 1 && keys[0] === "isActive" && typeof candidate.isActive === "boolean"; + return !isGeneratedStatusStub; + }), + }; +} + export function useVendorRosterResolver( routeRoster: VendorCompanyRoster | undefined, ): Resolver { const strictResolver = useMemo(() => zodResolver(vendorCompanyRosterSchema), []); return useMemo>( () => async (values, context, options) => { - if ( - routeRoster && - getSingleStatusOnlyChange(routeRoster, values as VendorCompanyRosterFormValues) - ) { - return { values, errors: {} }; + const normalizedValues = withoutTechnicianStubs(values); + if (routeRoster && getSingleStatusOnlyChange(routeRoster, normalizedValues)) { + return { values: normalizedValues, errors: {} }; } - return strictResolver(values, context, options); + return strictResolver(normalizedValues, context, options); }, [routeRoster, strictResolver], ); diff --git a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx index 62d65f15..52b328be 100644 --- a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx +++ b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx @@ -1,5 +1,5 @@ import { useEffect, useState, type ReactNode } from "react"; -import { Controller, useWatch } from "react-hook-form"; +import { useWatch } from "react-hook-form"; import CloseIcon from "@mui/icons-material/Close"; import EditOutlinedIcon from "@mui/icons-material/EditOutlined"; import LaunchIcon from "@mui/icons-material/Launch"; @@ -211,7 +211,7 @@ function DrawerBody({ Notes - + {roster.notes} @@ -240,6 +240,32 @@ function DrawerActions({ onEdit }: { onEdit: () => void }) { ); } +function TechnicianStatusControl({ + isActive, + persistedIsActive, + onRequestDeactivation, + onChange, +}: { + isActive: boolean; + persistedIsActive: boolean; + onRequestDeactivation: () => void; + onChange: (checked: boolean) => void; +}) { + return ( + + {isActive ? "Active" : "Inactive"} + { + if (persistedIsActive && !checked) onRequestDeactivation(); + else onChange(checked); + }} + /> + + ); +} + function DrawerEditor({ vendor, onClose, @@ -312,22 +338,11 @@ function DrawerEditor({ Total Jobs - ( - - {field.value ? "Active" : "Inactive"} - { - if (persistedIsActive && !checked) onRequestDeactivation(vendor); - else field.onChange(checked); - }} - /> - - )} + onRequestDeactivation(vendor)} + onChange={(checked) => form.setTechnicianActive(selectedIndex, checked)} /> @@ -341,7 +356,7 @@ function DrawerEditor({ - 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 93a10fd9..df32ae32 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx @@ -1,4 +1,10 @@ -import { Controller, useFieldArray, type Control, type FieldErrors } from "react-hook-form"; +import { + Controller, + useFieldArray, + useWatch, + type Control, + type FieldErrors, +} from "react-hook-form"; import AddIcon from "@mui/icons-material/Add"; import DeleteOutlineIcon from "@mui/icons-material/DeleteOutlined"; import { @@ -16,8 +22,11 @@ import { } from "@mui/material"; import { VendorTradeSpecialtiesField } from "./vendor-trade-specialties-field"; import { formatNorthAmericanPhone } from "@/lib/format/na-phone"; -import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; -import type { RosterTechnicianValues } from "@/domain/vendors/schemas/vendor-roster-schema"; +import { + VENDOR_NOTES_MAX_LENGTH, + type RosterTechnicianValues, + type VendorCompanyRosterFormValues, +} from "@/domain/vendors/schemas/vendor-roster-schema"; import type { VendorFacetCompany } from "@/domain/vendors/types/vendor"; interface VendorRosterFormFieldsProps { @@ -121,6 +130,8 @@ function CompanyFields({ onSelectCompany, onClearSelectedCompany, }: VendorRosterFormFieldsProps) { + const notes = useWatch({ control, name: "notes" }) ?? ""; + return ( @@ -222,6 +233,19 @@ function CompanyFields({ multiline minRows={2} fullWidth + error={Boolean(errors.notes)} + helperText={`${notes.length}/${VENDOR_NOTES_MAX_LENGTH} characters · ${ + errors.notes?.message ?? `Maximum ${VENDOR_NOTES_MAX_LENGTH} characters` + }`} + slotProps={{ + htmlInput: { + maxLength: VENDOR_NOTES_MAX_LENGTH, + style: { + overflowWrap: "anywhere", + whiteSpace: "pre-wrap", + }, + }, + }} /> )} /> @@ -324,23 +348,22 @@ function TechnicianRow({ tradeOptions={tradeOptions} tradeOptionsLoading={tradeOptionsLoading} /> - {showStatus && ( - ( - field.onChange(checked)} - /> - } - label={field.value ? "Active" : "Inactive"} - /> - )} - /> - )} + ( + field.onChange(checked)} + /> + } + label={field.value ? "Active" : "Inactive"} + /> + )} + /> ); } diff --git a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx index 45f1bb19..45a1c823 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx @@ -140,7 +140,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa > Cancel - diff --git a/src/domain/vendors/schemas/vendor-roster-schema.ts b/src/domain/vendors/schemas/vendor-roster-schema.ts index 9358b12b..744a3581 100644 --- a/src/domain/vendors/schemas/vendor-roster-schema.ts +++ b/src/domain/vendors/schemas/vendor-roster-schema.ts @@ -35,6 +35,8 @@ const northAmericanPhone = z const optionalEmail = z.union([z.string().email("Invalid email"), z.literal("")]); +export const VENDOR_NOTES_MAX_LENGTH = 500; + export const rosterTechnicianSchema = z.object({ id: z.union([z.string(), z.number()]).optional(), contactName: z.string(), @@ -56,7 +58,9 @@ const baseCompanyFields = { state: z.string(), zip: z.string(), googleMapsUrl: httpsUrl, - notes: z.string(), + notes: z + .string() + .max(VENDOR_NOTES_MAX_LENGTH, `Notes must be ${VENDOR_NOTES_MAX_LENGTH} characters or fewer`), technicians: z.array(rosterTechnicianSchema), }; @@ -67,7 +71,7 @@ export const vendorCompanyRosterSchema = z.object(baseCompanyFields).superRefine ctx.addIssue({ code: z.ZodIssueCode.custom, path: ["companyPhone"], - message: "Company phone or email is required", + message: "Provide a company phone or email (at least one required)", }); } }); diff --git a/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx b/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx index b56aa714..538aaf4b 100644 --- a/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx +++ b/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { ReactNode } from "react"; -import { useController } from "react-hook-form"; +import { useController, useFieldArray } from "react-hook-form"; import { beforeEach, describe, expect, it, vi } from "vitest"; const rosterGet = vi.fn(); @@ -321,6 +321,31 @@ describe("useVendorRosterForm prototype defaults", () => { }); }); +describe("useVendorRosterForm technician removal", () => { + it("keeps a valid roster saveable after an existing technician is removed", async () => { + rosterQueryResult = { + data: rosterWithTechnicians, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }; + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "update", companyId: 5 }); + const technicians = useFieldArray({ control: form.control, name: "technicians" }); + return { form, technicians }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await waitFor(() => expect(result.current.form.isValid).toBe(true)); + act(() => result.current.technicians.remove(1)); + + await waitFor(() => expect(result.current.form.isValid).toBe(true)); + }); +}); + describe("useVendorRosterForm stale-selection handling", () => { beforeEach(() => { rosterGet.mockReset(); diff --git a/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx new file mode 100644 index 00000000..661f00d7 --- /dev/null +++ b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx @@ -0,0 +1,52 @@ +import { fireEvent, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import { VendorCreateModal } from "@/app/(protected)/vendors/_components/vendor-create-modal"; +import { renderWithProviders } from "@/test/test-utils"; + +vi.mock("@/domain/vendors/use-cases/use-vendor-facets", () => ({ + useVendorFacets: () => ({ data: { companies: [], trades: [] }, isLoading: false }), +})); + +vi.mock("@/domain/vendors/use-cases/use-save-vendor-company-roster", async () => { + const actual = await vi.importActual< + typeof import("@/domain/vendors/use-cases/use-save-vendor-company-roster") + >("@/domain/vendors/use-cases/use-save-vendor-company-roster"); + return { + ...actual, + useSaveVendorCompanyRoster: () => ({ mutate: vi.fn(), isPending: false }), + }; +}); + +describe("VendorCreateModal validation", () => { + it("explains the company phone-or-email requirement after submission", async () => { + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + fireEvent.change(screen.getByRole("combobox", { name: /Company/ }), { + target: { value: "Gateway Plumbing" }, + }); + await userEvent.click(screen.getByRole("button", { name: "Add Vendor" })); + + expect( + await screen.findByText("Provide a company phone or email (at least one required)"), + ).toBeInTheDocument(); + }); + + it("shows the notes limit and live character counter", async () => { + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + const notes = screen.getByRole("textbox", { name: "Notes (optional)" }); + fireEvent.change(notes, { target: { value: "abc" } }); + + expect(notes).toHaveAttribute("maxlength", "500"); + await waitFor(() => + expect(notes).toHaveAccessibleDescription(/3\/500 characters · Maximum 500 characters/), + ); + }); +}); diff --git a/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx b/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx index 4e52aa7f..a882b91f 100644 --- a/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx +++ b/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx @@ -1,16 +1,27 @@ -import { screen } from "@testing-library/react"; +import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { VendorDetailDrawer } from "@/app/(protected)/vendors/_components/vendor-detail-drawer"; import { renderWithProviders } from "@/test/test-utils"; import type { VendorListItem } from "@/domain/vendors/types/vendor"; const useVendorCompanyRoster = vi.fn(); +const saveMutate = vi.fn(); vi.mock("@/domain/vendors/use-cases/use-vendor-company-roster", () => ({ useVendorCompanyRoster: (...args: unknown[]) => useVendorCompanyRoster(...args), })); +vi.mock("@/domain/vendors/use-cases/use-save-vendor-company-roster", async () => { + const actual = await vi.importActual< + typeof import("@/domain/vendors/use-cases/use-save-vendor-company-roster") + >("@/domain/vendors/use-cases/use-save-vendor-company-roster"); + return { + ...actual, + useSaveVendorCompanyRoster: () => ({ mutate: saveMutate, isPending: false }), + }; +}); + const vendor: VendorListItem = { id: 1, companyId: "co-1", @@ -50,7 +61,111 @@ function rosterWith(technicians: Array>) { }; } +beforeEach(() => { + saveMutate.mockReset(); +}); + describe("VendorDetailDrawer selected-technician display", () => { + it.each([ + { removedPosition: 1, remainingId: 2, remainingName: "Beth" }, + { removedPosition: 2, remainingId: 1, remainingName: "Adam" }, + ])( + "reconciles the remaining roster after removing technician $removedPosition", + async ({ removedPosition, remainingId, remainingName }) => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([ + { + id: 1, + contactName: "Adam", + phone: "314-555-0198", + email: "", + tradeSpecialties: "", + isActive: removedPosition === 1, + totalJobs: 5, + }, + { + id: 2, + contactName: "Beth", + phone: "314-555-0199", + email: "", + tradeSpecialties: "", + isActive: removedPosition === 2, + totalJobs: 2, + }, + ]), + ); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + await userEvent.click( + screen.getByRole("button", { name: `Remove technician ${removedPosition}` }), + ); + await userEvent.click(screen.getByRole("button", { name: "Save changes" })); + + await waitFor(() => expect(saveMutate).toHaveBeenCalledTimes(1)); + expect(saveMutate).toHaveBeenCalledWith( + expect.objectContaining({ + mode: "update", + values: expect.objectContaining({ + technicians: [ + expect.objectContaining({ + id: remainingId, + contactName: remainingName, + isActive: false, + }), + ], + }), + }), + expect.any(Object), + ); + }, + ); + + it("keeps Save changes available after removing a technician from a legacy roster", async () => { + const roster = rosterWith([ + { + id: 1, + contactName: "Adam", + phone: "314-555-0198", + email: "", + tradeSpecialties: "", + isActive: true, + totalJobs: 5, + }, + { + id: 2, + contactName: "Beth", + phone: "314-555-0199", + email: "", + tradeSpecialties: "", + isActive: true, + totalJobs: 2, + }, + ]); + roster.data.companyPhone = ""; + roster.data.email = ""; + useVendorCompanyRoster.mockReturnValue(roster); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + await userEvent.click(screen.getByRole("button", { name: "Remove technician 2" })); + + const save = screen.getByRole("button", { name: "Save changes" }); + expect(save).toBeEnabled(); + + await userEvent.click(save); + + expect( + await screen.findByText("Provide a company phone or email (at least one required)"), + ).toBeInTheDocument(); + }); + it("renders no preference label when preferredContact is absent", () => { useVendorCompanyRoster.mockReturnValue( rosterWith([ @@ -137,6 +252,38 @@ describe("VendorDetailDrawer selected-technician display", () => { }); describe("VendorDetailDrawer design parity", () => { + it("wraps long unbroken company notes inside the drawer", () => { + const roster = rosterWith([]); + roster.data.notes = "x".repeat(5000); + useVendorCompanyRoster.mockReturnValue(roster); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + const notes = screen.getByText("x".repeat(5000)); + expect(notes).toHaveStyle({ overflowWrap: "anywhere", whiteSpace: "pre-wrap" }); + }); + + it("preserves an oversized legacy note in edit mode while showing the new limit", async () => { + const roster = rosterWith([]); + const legacyNotes = "x".repeat(501); + roster.data.notes = legacyNotes; + useVendorCompanyRoster.mockReturnValue(roster); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + const notes = await screen.findByRole("textbox", { name: "Notes (optional)" }); + expect(notes).toHaveValue(legacyNotes); + expect(notes).toHaveAttribute("maxlength", "500"); + expect(notes).toHaveStyle({ overflowWrap: "anywhere", whiteSpace: "pre-wrap" }); + expect(screen.getByText(/501\/500 characters/)).toBeInTheDocument(); + }); + it("renders the company once when there is no technician name to head the panel", () => { useVendorCompanyRoster.mockReturnValue(rosterWith([])); 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 6b94dd90..646f8e73 100644 --- a/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts +++ b/src/test/domain/vendors/schemas/vendor-roster-schema.test.ts @@ -3,6 +3,7 @@ import { emptyRosterTechnician, emptyVendorCompanyRosterForm, isAbsoluteHttpsUrl, + VENDOR_NOTES_MAX_LENGTH, vendorCompanyRosterSchema, vendorCompanyRosterUpdateSchema, } from "@/domain/vendors/schemas/vendor-roster-schema"; @@ -28,7 +29,9 @@ describe("vendorCompanyRosterSchema", () => { expect(result.success).toBe(false); if (!result.success) { expect( - result.error.issues.some((issue) => issue.message === "Company phone or email is required"), + result.error.issues.some( + (issue) => issue.message === "Provide a company phone or email (at least one required)", + ), ).toBe(true); } }); @@ -54,6 +57,20 @@ describe("vendorCompanyRosterSchema", () => { expect(result.success).toBe(true); }); + it("accepts exactly 500 note characters and rejects 501", () => { + const atLimit = vendorCompanyRosterSchema.safeParse({ + ...baseCompany, + notes: "x".repeat(VENDOR_NOTES_MAX_LENGTH), + }); + const overLimit = vendorCompanyRosterSchema.safeParse({ + ...baseCompany, + notes: "x".repeat(VENDOR_NOTES_MAX_LENGTH + 1), + }); + + expect(atLimit.success).toBe(true); + expect(overLimit.success).toBe(false); + }); + it("accepts a technician row without a name or phone", () => { const result = vendorCompanyRosterSchema.safeParse({ ...baseCompany,