From d52df52de674b66ff9fb7696be7c42e7c3698436 Mon Sep 17 00:00:00 2001 From: "arthur.bassi" Date: Mon, 10 Aug 2026 19:08:01 -0300 Subject: [PATCH] fix(vendors): restore blank-tech filter and deactivation gate Omit empty new technician rows on save and route Active to Inactive through VendorDeactivationDialog, blocking sparse isActive:false updates. Co-authored-by: Cursor --- .../_components/use-vendor-deactivation.ts | 9 +- .../_components/use-vendor-roster-form.ts | 20 +++- .../_components/vendor-roster-form-fields.tsx | 33 +++++- .../_components/vendor-roster-page.tsx | 47 ++++++++ .../use-save-vendor-company-roster.ts | 5 + .../vendors/use-vendor-roster-form.test.tsx | 101 +++++++++++++++++- .../use-save-vendor-company-roster.test.ts | 75 ++++++++++++- 7 files changed, 280 insertions(+), 10 deletions(-) diff --git a/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts b/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts index 9b6f3b0d..2409814f 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts @@ -15,7 +15,9 @@ export interface VendorDeactivationState { confirm: () => void; } -export function useVendorDeactivation(): VendorDeactivationState { +export function useVendorDeactivation(options?: { + onSuccess?: () => void; +}): VendorDeactivationState { const [target, setTarget] = useState(null); const [error, setError] = useState(null); const deleteVendor = useDeleteVendor(); @@ -39,7 +41,10 @@ export function useVendorDeactivation(): VendorDeactivationState { if (!target || target.id == null) return; setError(null); deleteVendor.mutate(target.id, { - onSuccess: () => setTarget(null), + onSuccess: () => { + setTarget(null); + options?.onSuccess?.(); + }, onError: (err: Error) => setError(err.message || "Failed to deactivate vendor"), }); }; 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 f6ea637a..b4c3a51f 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts @@ -20,6 +20,24 @@ import type { VendorRosterConflict, } from "@/domain/vendors/types/vendor"; +function withoutBlankNewTechnicians( + values: VendorCompanyRosterFormValues, +): VendorCompanyRosterFormValues { + return { + ...values, + technicians: values.technicians.filter( + (technician) => + technician.id != null || + Boolean( + technician.contactName.trim() || + technician.phone.trim() || + technician.email.trim() || + technician.tradeSpecialties.trim(), + ), + ), + }; +} + export interface VendorRosterFormProps { mode: "create" | "update"; vendorId?: string | number; @@ -109,7 +127,7 @@ export function useVendorRosterForm({ save.mutate( { mode: isUpdate ? "update" : "create", - values: formValues, + values: withoutBlankNewTechnicians(formValues), companyId: isUpdate ? (committedRoster?.companyId ?? companyId) : undefined, rowVersion: isUpdate ? committedRoster?.rowVersion : undefined, originalRoster: mode === "update" ? routeRoster : undefined, 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 32f91675..d54c3016 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 { @@ -28,6 +34,7 @@ interface VendorRosterFormFieldsProps { selectedCompanyId?: string | number | null; onSelectCompany?: (company: VendorFacetCompany | null) => Promise; onClearSelectedCompany?: (nextName?: string) => void; + onRequestDeactivate?: (technician: RosterTechnicianValues) => void; } function CompanyNameField({ @@ -221,6 +228,7 @@ interface TechnicianRowProps { onRemove: () => void; canRemove: boolean; tradeOptions: string[]; + onRequestDeactivate?: (technician: RosterTechnicianValues) => void; } function TechnicianRow({ @@ -230,7 +238,10 @@ function TechnicianRow({ onRemove, canRemove, tradeOptions, + onRequestDeactivate, }: TechnicianRowProps) { + const technician = useWatch({ control, name: `technicians.${index}` }); + return ( @@ -305,7 +316,13 @@ function TechnicianRow({ control={ field.onChange(checked)} + onChange={(_event, checked) => { + if (!checked && technician?.id != null && onRequestDeactivate != null) { + onRequestDeactivate(technician); + return; + } + field.onChange(checked); + }} /> } label={field.value ? "Active" : "Inactive"} @@ -320,10 +337,12 @@ function TechniciansFieldArray({ control, errors, tradeOptions, + onRequestDeactivate, }: { control: Control; errors: FieldErrors; tradeOptions: string[]; + onRequestDeactivate?: (technician: RosterTechnicianValues) => void; }) { const { fields, append, remove } = useFieldArray({ control, name: "technicians" }); @@ -366,6 +385,7 @@ function TechniciansFieldArray({ onRemove={() => remove(index)} canRemove tradeOptions={tradeOptions} + onRequestDeactivate={onRequestDeactivate} /> )) )} @@ -375,12 +395,17 @@ function TechniciansFieldArray({ } export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) { - const { control, errors, tradeOptions = [] } = props; + const { control, errors, tradeOptions = [], onRequestDeactivate } = props; return ( - + ); } diff --git a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx index 15f8ff9a..cf4e6daf 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx @@ -1,17 +1,44 @@ import type { ReactNode } from "react"; import { useNavigate } from "react-router"; import { Alert, Box, Button, CircularProgress, Paper, Stack, Typography } from "@mui/material"; +import { VendorDeactivationDialog } from "./vendor-deactivation-dialog"; import { VendorPortalTokenPanel } from "./vendor-portal-token-panel"; import { VendorRosterConflictAlert } from "./vendor-roster-conflict-alert"; import { VendorRosterFormFields } from "./vendor-roster-form-fields"; import { VendorRosterLoadErrorAlert } from "./vendor-roster-load-error"; +import { useVendorDeactivation } from "./use-vendor-deactivation"; import { useVendorRosterForm } from "./use-vendor-roster-form"; +import type { RosterTechnicianValues } from "@/domain/vendors/schemas/vendor-roster-schema"; +import type { VendorCompanyRoster, VendorListItem } from "@/domain/vendors/types/vendor"; interface VendorRosterPageProps { vendorId?: string; companyId?: string; } +function toDeactivationListItem( + roster: VendorCompanyRoster, + technician: RosterTechnicianValues, +): VendorListItem { + return { + id: technician.id ?? null, + companyId: roster.companyId, + companyName: roster.name, + contactName: technician.contactName, + email: technician.email, + phone: technician.phone, + companyPhone: roster.companyPhone, + googleMapsUrl: roster.googleMapsUrl, + notes: roster.notes, + totalJobs: 0, + city: roster.city, + state: roster.state, + tradeSpecialties: technician.tradeSpecialties, + isActive: technician.isActive, + preferredContact: technician.preferredContact ?? "Phone", + }; +} + function PageShell({ title, subtitle, @@ -43,6 +70,9 @@ function PageShell({ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPageProps) { const navigate = useNavigate(); const isEdit = vendorId !== undefined || companyId !== undefined; + const deactivation = useVendorDeactivation({ + onSuccess: () => navigate("/vendors"), + }); const form = useVendorRosterForm({ mode: isEdit ? "update" : "create", vendorId, @@ -50,6 +80,11 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa onSuccess: () => navigate("/vendors"), }); + const handleRequestDeactivate = (technician: RosterTechnicianValues) => { + if (!form.roster || technician.id == null) return; + deactivation.open(toDeactivationListItem(form.roster, technician)); + }; + if (isEdit && form.isLoading) { return ( @@ -104,6 +139,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa control={form.control} errors={form.errors} tradeOptions={form.trades} + onRequestDeactivate={isEdit ? handleRequestDeactivate : undefined} {...companySelectionProps} /> @@ -125,6 +161,17 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa + + ); } diff --git a/src/domain/vendors/use-cases/use-save-vendor-company-roster.ts b/src/domain/vendors/use-cases/use-save-vendor-company-roster.ts index 37532220..cdc4b010 100644 --- a/src/domain/vendors/use-cases/use-save-vendor-company-roster.ts +++ b/src/domain/vendors/use-cases/use-save-vendor-company-roster.ts @@ -91,6 +91,11 @@ export function useSaveVendorCompanyRoster(): UseMutationResult< ? getSingleStatusOnlyChange(originalRoster, values) : null; if (statusChange) { + if (statusChange.isActive === false) { + throw new Error( + "Use Deactivate to check open work orders before inactivating a technician.", + ); + } const baseRoster = originalRoster as VendorCompanyRoster; await vendorsApi.update(statusChange.id, { isActive: statusChange.isActive }); return { 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 e083f352..d2e8433b 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 @@ -24,11 +24,16 @@ vi.mock("@/domain/vendors/use-cases/use-vendor-facets", () => ({ useVendorFacets: () => ({ data: { companies: [], trades: [] } }), })); +const saveMutate = vi.fn(); + 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 }) }; + return { + ...actual, + useSaveVendorCompanyRoster: () => ({ mutate: saveMutate, isPending: false }), + }; }); import { useVendorRosterForm } from "@/app/(protected)/vendors/_components/use-vendor-roster-form"; @@ -273,3 +278,97 @@ describe("useVendorRosterForm stale-selection handling", () => { expect(result.current.name.field.value).toBe("Draft Vendor"); }); }); + +describe("useVendorRosterForm blank technician filtering", () => { + beforeEach(() => { + rosterGet.mockReset(); + saveMutate.mockReset(); + }); + + it("omits blank new technician rows from the save payload", () => { + const { result } = renderHook(() => useVendorRosterForm({ mode: "create" }), { + wrapper: makeWrapper(createClient()), + }); + + act(() => { + result.current.submit({ + name: "Gateway Plumbing", + companyPhone: "(314) 555-0100", + email: "dispatch@gateway.test", + address: "", + city: "", + state: "", + zip: "", + googleMapsUrl: "", + notes: "", + technicians: [ + { + contactName: "Taylor", + phone: "(314) 555-0199", + email: "taylor@gateway.test", + tradeSpecialties: "Plumbing", + isActive: true, + }, + { + contactName: "", + phone: "", + email: "", + tradeSpecialties: "", + isActive: true, + }, + ], + }); + }); + + expect(saveMutate).toHaveBeenCalledTimes(1); + const payload = saveMutate.mock.calls[0]?.[0] as { + values: { technicians: Array<{ contactName: string }> }; + }; + expect(payload.values.technicians).toHaveLength(1); + expect(payload.values.technicians[0]?.contactName).toBe("Taylor"); + }); + + it("keeps existing technicians with an id even when contact fields are blank", () => { + const { result } = renderHook(() => useVendorRosterForm({ mode: "create" }), { + wrapper: makeWrapper(createClient()), + }); + + act(() => { + result.current.submit({ + name: "Gateway Plumbing", + companyPhone: "(314) 555-0100", + email: "dispatch@gateway.test", + address: "", + city: "", + state: "", + zip: "", + googleMapsUrl: "", + notes: "", + technicians: [ + { + id: 9, + contactName: "", + phone: "", + email: "", + tradeSpecialties: "", + isActive: true, + }, + ], + }); + }); + + const payload = saveMutate.mock.calls[0]?.[0] as { + values: { technicians: Array<{ id?: number }> }; + }; + expect(payload.values.technicians).toEqual([ + { + id: 9, + contactName: "", + phone: "", + email: "", + tradeSpecialties: "", + isActive: true, + }, + ]); + }); +}); diff --git a/src/test/domain/vendors/use-cases/use-save-vendor-company-roster.test.ts b/src/test/domain/vendors/use-cases/use-save-vendor-company-roster.test.ts index f0547378..9c61e64f 100644 --- a/src/test/domain/vendors/use-cases/use-save-vendor-company-roster.test.ts +++ b/src/test/domain/vendors/use-cases/use-save-vendor-company-roster.test.ts @@ -1,7 +1,32 @@ -import { describe, expect, it } from "vitest"; -import { getSingleStatusOnlyChange } from "@/domain/vendors/use-cases/use-save-vendor-company-roster"; +import { createElement, type ReactNode } from "react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + getSingleStatusOnlyChange, + useSaveVendorCompanyRoster, +} from "@/domain/vendors/use-cases/use-save-vendor-company-roster"; import type { VendorCompanyRoster } from "@/domain/vendors/types/vendor"; +const updateVendor = vi.fn(); + +vi.mock("@/domain/vendors/api/vendors-api", () => ({ + vendorsApi: { + update: (...args: unknown[]) => updateVendor(...args), + }, +})); + +vi.mock("@/domain/vendors/api/vendor-company-roster-api", () => ({ + vendorCompanyRosterApi: { + update: vi.fn(), + create: vi.fn(), + }, +})); + +vi.mock("react-toastify", () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + const roster: VendorCompanyRoster = { companyId: 10, rowVersion: "rv-1", @@ -69,3 +94,49 @@ describe("getSingleStatusOnlyChange", () => { ).toBeNull(); }); }); + +describe("useSaveVendorCompanyRoster status-only deactivation gate", () => { + beforeEach(() => { + updateVendor.mockReset(); + updateVendor.mockResolvedValue(undefined); + }); + + it("rejects sparse Active→Inactive updates and still allows reactivation", async () => { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const wrapper = ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client }, children); + + const { result } = renderHook(() => useSaveVendorCompanyRoster(), { wrapper }); + + const activeRoster: VendorCompanyRoster = { + ...roster, + technicians: [{ ...roster.technicians[0], isActive: true }], + }; + + await expect( + result.current.mutateAsync({ + mode: "update", + values: { + ...values, + technicians: [{ ...values.technicians[0], isActive: false }], + }, + companyId: 10, + rowVersion: "rv-1", + originalRoster: activeRoster, + }), + ).rejects.toThrow("Use Deactivate to check open work orders before inactivating a technician."); + expect(updateVendor).not.toHaveBeenCalled(); + + await result.current.mutateAsync({ + mode: "update", + values: { + ...values, + technicians: [{ ...values.technicians[0], isActive: true }], + }, + companyId: 10, + rowVersion: "rv-1", + originalRoster: roster, + }); + expect(updateVendor).toHaveBeenCalledWith(7, { isActive: true }); + }); +});