diff --git a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-detail.png b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-detail.png index 1beb0a15..1993c2c7 100644 Binary files a/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-detail.png and b/e2e/__screenshots__/vendors/vendors.visual.spec.ts/vendor-detail.png differ diff --git a/e2e/vendors/vendors.spec.ts b/e2e/vendors/vendors.spec.ts index ad1a89e8..d1263828 100644 --- a/e2e/vendors/vendors.spec.ts +++ b/e2e/vendors/vendors.spec.ts @@ -65,7 +65,10 @@ interface MockState { listUrls: string[]; createdBody?: Record; updatedBody?: Record; + patchedBody?: Record; + patchedCompanyId?: string; deletedId?: string; + deleteConfirmedOpenWorkOrders?: boolean; } async function fulfillJson(route: Route, body: unknown, status = 200) { @@ -175,6 +178,52 @@ async function mockVendorApi( return; } + if (request.method() === "PATCH" && pathCompanyId) { + state.patchedBody = request.postDataJSON(); + state.patchedCompanyId = pathCompanyId; + const anchor = vendorRecords.find((vendor) => String(vendor.CompanyId) === pathCompanyId); + if (!anchor) { + await fulfillJson(route, { message: "Vendor roster not found" }, 404); + return; + } + const added = Array.isArray(state.patchedBody.addTechnicians) + ? (state.patchedBody.addTechnicians as Array>) + : []; + await fulfillJson(route, { + companyId: anchor.CompanyId, + rowVersion: "rv-patched", + name: anchor.CompanyName, + companyPhone: anchor.CompanyPhone, + email: anchor.Email, + address: anchor.Address, + city: anchor.City, + state: anchor.State, + zip: anchor.Zip, + googleMapsUrl: anchor.GoogleMapsUrl, + notes: anchor.Notes, + technicians: [ + ...vendorRecords + .filter((vendor) => vendor.CompanyId === anchor.CompanyId) + .map((vendor) => ({ + id: vendor.Id, + contactName: vendor.ContactName, + phone: vendor.Phone, + email: vendor.Email, + preferredContact: vendor.PreferredContact ?? "Phone", + tradeSpecialties: vendor.TradeSpecialties, + isActive: vendor.IsActive, + totalJobs: vendor.TotalJobs, + })), + ...added.map((technician, index) => ({ + id: 900 + index, + totalJobs: 0, + ...technician, + })), + ], + }); + return; + } + if (request.method() === "PUT" && pathCompanyId) { state.updatedBody = request.postDataJSON(); const technicians = Array.isArray(state.updatedBody.technicians) @@ -259,8 +308,9 @@ async function mockVendorApi( }, }), ); - await page.route(/\/api\/vendors\/\d+$/, async (route) => { - const id = route.request().url().split("/").pop() ?? ""; + await page.route(/\/api\/vendors\/\d+(\?.*)?$/, async (route) => { + const requestUrl = new URL(route.request().url()); + const id = requestUrl.pathname.split("/").pop() ?? ""; if (route.request().method() === "PUT") { state.updatedBody = route.request().postDataJSON(); const vendor = vendorRecords.find((item) => String(item.Id) === id); @@ -273,6 +323,8 @@ async function mockVendorApi( return; } if (route.request().method() === "DELETE") { + state.deleteConfirmedOpenWorkOrders = + requestUrl.searchParams.get("confirmOpenWorkOrders") === "true"; if (options.deleteConflict) { await fulfillJson( route, @@ -409,6 +461,11 @@ test.describe("Vendor directory prototype parity", () => { "https://maps.google.com/gateway", ); await expect(page.getByLabel("Preferred Contact")).toHaveCount(0); + await expect(page.getByLabel("Technician name (optional)")).toHaveCount(1); + await expect(page.getByLabel("Technician name (optional)")).toHaveValue(""); + await expect( + page.getByRole("dialog", { name: /Add Vendor/ }).getByText("Adam Whyte"), + ).toHaveCount(0); await page.getByRole("button", { name: "Add technician" }).click(); await page.getByLabel("Technician name (optional)").last().fill("New Technician"); const tradeInput = page.getByRole("combobox", { name: "Add Trade" }).last(); @@ -420,27 +477,33 @@ test.describe("Vendor directory prototype parity", () => { await page.getByLabel("Notes (optional)").fill("Created in browser E2E"); await page.getByRole("button", { name: "Add Vendor", exact: true }).last().click(); await expect(page.getByRole("dialog", { name: /Add Vendor/ })).toHaveCount(0); - expect(state.updatedBody).toMatchObject({ - name: "Gateway Plumbing", - companyPhone: "(314) 555-0100", - notes: "Created in browser E2E", - rowVersion: "rv-1", - }); - expect(state.updatedBody?.technicians).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - contactName: "New Technician", - tradeSpecialties: "Plumbing, HVAC", - }), - ]), - ); - const submittedTechnicians = Array.isArray(state.updatedBody?.technicians) - ? (state.updatedBody.technicians as Array>) + expect(state.patchedCompanyId).toBe("101"); + expect(state.patchedBody).toMatchObject({ rowVersion: "rv-1" }); + // Only the field the user actually typed into is transmitted. Address/phone/email were + // populated by selecting the company and never edited, so they must not be sent — otherwise a + // concurrent edit by another user to any of them would be silently reverted on resubmit. + expect(state.patchedBody?.companyFields).toEqual({ notes: "Created in browser E2E" }); + expect(state.patchedBody?.companyFields).not.toHaveProperty("companyPhone"); + expect(state.patchedBody?.companyFields).not.toHaveProperty("address"); + expect(state.patchedBody?.companyFields).not.toHaveProperty("email"); + expect(state.patchedBody?.companyFields).not.toHaveProperty("name"); + expect(state.patchedBody?.addTechnicians).toEqual([ + expect.objectContaining({ + contactName: "New Technician", + tradeSpecialties: "Plumbing, HVAC", + }), + ]); + const patchedTechnicians = Array.isArray(state.patchedBody?.addTechnicians) + ? (state.patchedBody.addTechnicians as Array>) : []; - const newTechnician = submittedTechnicians.find( + const newTechnician = patchedTechnicians.find( (technician) => technician.contactName === "New Technician", ); expect(newTechnician?.preferredContact).toBeUndefined(); + expect(patchedTechnicians.some((technician) => technician.contactName === "Adam Whyte")).toBe( + false, + ); + expect(state.updatedBody).toBeUndefined(); await page.getByRole("button", { name: "View vendor Gateway Plumbing" }).click(); const detailDrawer = page.locator(".MuiDrawer-paper").last(); @@ -517,20 +580,28 @@ test.describe("Vendor directory prototype parity", () => { await expect(page.getByRole("button", { name: "Close drawer" })).toHaveCount(0); }); - test("blocks deactivation for linked work orders and preserves the vendor on a raced 409", async ({ + test("confirms deactivation past linked work orders and preserves the vendor on a raced 409", async ({ page, }) => { - const blockedState = await mockVendorApi(page, { deactivationBlocked: true }); + const confirmState = await mockVendorApi(page, { deactivationBlocked: true }); await page.goto("/vendors"); await page.getByRole("button", { name: "Edit vendor Gateway Plumbing" }).click(); await page.getByRole("switch", { name: "Active status" }).click(); - const blockedDialog = page.getByRole("dialog", { name: "Deactivate Vendor" }); - await expect(blockedDialog).toContainText("WO-501 — Emergency boiler repair"); - await expect(blockedDialog.getByRole("button", { name: /^Deactivate$/ })).toBeDisabled(); - expect(blockedState.deletedId).toBeUndefined(); + const dialog = page.getByRole("dialog", { name: "Deactivate this vendor?" }); + await expect(dialog).toContainText("It still has 1 open work order"); + await expect( + dialog.getByRole("link", { name: /WO-501 — Emergency boiler repair/ }), + ).toHaveAttribute("href", "/workorders/501"); + + // SH-254: the open work orders inform the decision, they no longer block it. + const confirm = dialog.getByRole("button", { name: "Deactivate anyway" }); + await expect(confirm).toBeEnabled(); + await confirm.click(); + + await expect.poll(() => confirmState.deletedId).toBe("1"); + expect(confirmState.deleteConfirmedOpenWorkOrders).toBe(true); - await blockedDialog.getByRole("button", { name: "Cancel" }).click(); await page.unrouteAll({ behavior: "wait" }); const racedState = await mockVendorApi(page, { deleteConflict: true }); @@ -538,13 +609,13 @@ test.describe("Vendor directory prototype parity", () => { await page.getByRole("button", { name: "Edit vendor Gateway Plumbing" }).click(); await page.getByRole("switch", { name: "Active status" }).click(); await page - .getByRole("dialog", { name: "Deactivate Vendor" }) + .getByRole("dialog", { name: "Deactivate this vendor?" }) .getByRole("button", { name: /^Deactivate$/, }) .click(); - await expect(page.getByRole("dialog", { name: "Deactivate Vendor" })).toContainText( + await expect(page.getByRole("dialog", { name: "Deactivate this vendor?" })).toContainText( /open work orders|conflict/i, ); expect(racedState.deletedId).toBeUndefined(); diff --git a/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts b/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts index b63291be..5870f7db 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-deactivation.ts @@ -38,13 +38,18 @@ export function useVendorDeactivation(onSuccess?: () => void): VendorDeactivatio const confirm = () => { if (!target || target.id == null) return; setError(null); - deleteVendor.mutate(target.id, { - onSuccess: () => { - setTarget(null); - onSuccess?.(); + // The dialog has shown whatever open work orders exist, so confirming here is + // the explicit confirmation the API requires to deactivate past them (SH-254). + deleteVendor.mutate( + { id: target.id, confirmOpenWorkOrders: (impact?.openWorkOrders.length ?? 0) > 0 }, + { + onSuccess: () => { + setTarget(null); + onSuccess?.(); + }, + onError: (err: Error) => setError(err.message || "Failed to deactivate vendor"), }, - onError: (err: Error) => setError(err.message || "Failed to deactivate vendor"), - }); + ); }; return { 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 c285f592..82ae4333 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts @@ -1,4 +1,5 @@ import { useCallback, useMemo, useState } from "react"; +import { toast } from "react-toastify"; import { useForm, useWatch, type FieldErrors } from "react-hook-form"; import { emptyVendorCompanyRosterForm, @@ -8,10 +9,38 @@ import { } from "@/domain/vendors/schemas/vendor-roster-schema"; import { useVendorCompanyRoster } from "@/domain/vendors/use-cases/use-vendor-company-roster"; import { + buildAdditiveRosterPatch, getSingleStatusOnlyChange, useSaveVendorCompanyRoster, + type EditedCompanyFields, } from "@/domain/vendors/use-cases/use-save-vendor-company-roster"; import { useVendorRosterResolver } from "./use-vendor-roster-resolver"; + +const EDITABLE_COMPANY_FIELDS = [ + "name", + "companyPhone", + "email", + "address", + "city", + "state", + "zip", + "googleMapsUrl", + "notes", +] as const; + +/** + * Company fields the user actually typed into, taken from react-hook-form dirty state. + * Selecting a company calls reset(), so loaded values are not dirty; only genuine edits are. + * The additive PATCH must send these rather than a form-vs-roster diff, which would resend + * stale values for fields another user changed while a conflict was being resolved. + */ +function pickEditedCompanyFields(dirtyFields: Record): EditedCompanyFields { + const edited: EditedCompanyFields = {}; + for (const field of EDITABLE_COMPANY_FIELDS) { + if (dirtyFields[field] === true) edited[field] = true; + } + return edited; +} import { toFormValues, useVendorRosterSelection } from "./use-vendor-roster-selection"; import { useVendorFacets } from "@/domain/vendors/use-cases/use-vendor-facets"; import { isVendorRosterConflictError } from "@/domain/vendors/lib/vendor-roster-conflict"; @@ -105,6 +134,10 @@ export function useVendorRosterForm({ mode: "onChange", }); const { control, handleSubmit, reset, 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. + const editedCompanyFields = pickEditedCompanyFields(formState.dirtyFields); const watched = useWatch({ control }); const clearConflict = useCallback(() => setConflict(null), []); @@ -130,17 +163,39 @@ export function useVendorRosterForm({ clearConflict(); }, [clearConflict, createDefaults, reset, resetSelection]); - const committedRoster = mode === "update" ? routeRoster : selection.selectedRoster; - const isUpdate = mode === "update" || selection.selectedRoster != null; - const submit = (formValues: VendorCompanyRosterFormValues) => { setConflict(null); + const values = withoutBlankNewTechnicians(formValues); + if (mode === "create" && selection.selectedRoster != null) { + const selected = selection.selectedRoster; + if (!buildAdditiveRosterPatch(selected, values, selected.rowVersion, editedCompanyFields)) { + toast.error("Enter at least one technician to add to this vendor."); + return; + } + save.mutate( + { + mode: "add", + values, + companyId: selected.companyId, + rowVersion: selected.rowVersion, + originalRoster: selected, + editedCompanyFields, + }, + { + onSuccess: (data) => onSuccess?.(data), + onError: (error) => { + if (isVendorRosterConflictError(error)) setConflict(error.conflict); + }, + }, + ); + return; + } save.mutate( { - mode: isUpdate ? "update" : "create", - values: withoutBlankNewTechnicians(formValues), - companyId: isUpdate ? (committedRoster?.companyId ?? companyId) : undefined, - rowVersion: isUpdate ? committedRoster?.rowVersion : undefined, + mode: mode === "update" ? "update" : "create", + values, + companyId: mode === "update" ? (routeRoster?.companyId ?? companyId) : undefined, + rowVersion: mode === "update" ? routeRoster?.rowVersion : undefined, originalRoster: mode === "update" ? routeRoster : undefined, }, { diff --git a/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts index cce00e18..24d51ce8 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts @@ -1,6 +1,7 @@ import { useCallback, useRef, useState } from "react"; import { useQueryClient, type UseQueryResult } from "@tanstack/react-query"; import { + emptyRosterTechnician, emptyVendorCompanyRosterForm, type VendorCompanyRosterFormValues, } from "@/domain/vendors/schemas/vendor-roster-schema"; @@ -53,6 +54,20 @@ export function toFormValues(roster: VendorCompanyRoster): VendorCompanyRosterFo }; } +export function toCompanyFormValues(roster: VendorCompanyRoster): VendorCompanyRosterFormValues { + return { ...toFormValues(roster), technicians: [] }; +} + +function withCompanyFieldsOnly( + roster: VendorCompanyRoster, +): (current: VendorCompanyRosterFormValues) => VendorCompanyRosterFormValues { + return (current) => ({ + ...toCompanyFormValues(roster), + technicians: + current.technicians.length > 0 ? current.technicians : [{ ...emptyRosterTechnician }], + }); +} + export function useVendorRosterSelection({ mode, reset, @@ -72,6 +87,13 @@ export function useVendorRosterSelection({ queryClient.fetchQuery({ queryKey: queryKeys.vendors.roster("companyId", id), queryFn: () => vendorCompanyRosterApi.get({ companyId: id }), + // SH-250: this fetch exists to obtain the freshest rowVersion, so it must not + // serve the global 5-minute staleTime cache. A conflict never invalidates this + // key (useSaveVendorCompanyRoster invalidates only on success, and its onError + // returns early for conflicts), and the Add flow has no mounted observer on it. + // Without staleTime: 0 the Reload button re-seeds the same stale rowVersion and + // the retry 409s again indefinitely. + staleTime: 0, }), [queryClient], ); @@ -91,7 +113,7 @@ export function useVendorRosterSelection({ try { const roster = await fetchRosterByCompany(company.companyId); if (requestIdRef.current !== requestId) return; - reset(toFormValues(roster)); + reset(withCompanyFieldsOnly(roster)); setSelectedRoster(roster); clearConflict(); retryTargetRef.current = null; @@ -107,7 +129,11 @@ export function useVendorRosterSelection({ (nextName = "") => { requestIdRef.current += 1; if (selectedRoster) { - reset({ ...emptyVendorCompanyRosterForm, name: nextName }); + reset((current) => ({ + ...emptyVendorCompanyRosterForm, + name: nextName, + technicians: current.technicians, + })); } setSelectedRoster(null); clearConflict(); @@ -134,7 +160,11 @@ export function useVendorRosterSelection({ void fetchRosterByCompany(selectedRoster.companyId) .then((roster) => { if (requestIdRef.current !== requestId) return; - reset(toFormValues(roster)); + // SH-250: refresh the visible company fields alongside rowVersion. + // Refreshing only selectedRoster left the form on pre-conflict values, + // so a retry diffed stale fields against the reloaded roster and could + // overwrite the concurrent company update that caused the 409. + reset(withCompanyFieldsOnly(roster)); setSelectedRoster(roster); retryTargetRef.current = null; }) diff --git a/src/app/(protected)/vendors/_components/vendor-deactivation-dialog.tsx b/src/app/(protected)/vendors/_components/vendor-deactivation-dialog.tsx index cf673b6c..a56ca3b5 100644 --- a/src/app/(protected)/vendors/_components/vendor-deactivation-dialog.tsx +++ b/src/app/(protected)/vendors/_components/vendor-deactivation-dialog.tsx @@ -8,11 +8,13 @@ import { DialogContent, DialogContentText, DialogTitle, + Link, List, ListItem, Stack, Typography, } from "@mui/material"; +import { Link as RouterLink } from "react-router"; import type { VendorDeactivationImpact, VendorListItem } from "@/domain/vendors/types/vendor"; interface VendorDeactivationDialogProps { @@ -26,6 +28,15 @@ interface VendorDeactivationDialogProps { onConfirm: () => void; } +function deactivationMessage(companyName: string | undefined, openCount: number): string { + const name = companyName ? `"${companyName}"` : "This vendor"; + if (openCount === 0) { + return `${name} will no longer be selectable for new work orders.`; + } + const plural = openCount === 1 ? "work order" : "work orders"; + return `${name} will no longer be selectable for new work orders. It still has ${openCount} open ${plural} — they'll keep it as-is unless you reassign them.`; +} + export function VendorDeactivationDialog({ target, isLoading, @@ -36,16 +47,16 @@ export function VendorDeactivationDialog({ onClose, onConfirm, }: VendorDeactivationDialogProps) { - const hasBlockingImpact = Boolean(impact && !impact.canDeactivate); + const openWorkOrders = impact?.openWorkOrders ?? []; + const hasOpenWorkOrders = openWorkOrders.length > 0; return ( - Deactivate Vendor + Deactivate this vendor? - Deactivate "{target?.companyName}"? Existing work-order and audit history will - be preserved. + {deactivationMessage(target?.companyName, openWorkOrders.length)} {isLoading && ( @@ -63,19 +74,13 @@ export function VendorDeactivationDialog({ )} - {impact != null && !isLoading && !impact.canDeactivate && ( - - This vendor cannot be deactivated because it still has open work orders. - - )} - - {impact != null && impact.openWorkOrders.length > 0 && ( + {hasOpenWorkOrders && ( - Open work orders ({impact.openWorkOrders.length}) + Open work orders ({openWorkOrders.length}) - {impact.openWorkOrders.map((wo) => ( + {openWorkOrders.map((wo) => ( - + {wo.workOrderNumber ? `${wo.workOrderNumber} — ${wo.workOrderTitle || "Untitled"}` : wo.workOrderTitle || `Work order ${wo.workOrderId}`} - + {Boolean(wo.status) && ( {wo.status} @@ -113,12 +118,12 @@ export function VendorDeactivationDialog({ Cancel diff --git a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx index 7a365a48..62d65f15 100644 --- a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx +++ b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx @@ -17,6 +17,7 @@ import { Switch, } from "@mui/material"; import { VendorRosterConflictAlert } from "./vendor-roster-conflict-alert"; +import { VendorStatusBadge } from "./vendor-status-badge"; import { VendorRosterFormFields } from "./vendor-roster-form-fields"; import { VendorRosterLoadErrorAlert } from "./vendor-roster-load-error"; import { useVendorRosterForm } from "./use-vendor-roster-form"; @@ -81,6 +82,13 @@ function DrawerHeader({ technician?: VendorRosterTechnician; onClose: () => void; }) { + const title = technician?.contactName || vendor?.contactName || roster?.name || "Vendor company"; + const company = roster?.name || vendor?.companyName || "Vendor details"; + // The company is a subtitle under the technician's name. When there is no + // technician the title already falls back to the company, and repeating it + // would render the same line twice. + const subtitle = company === title ? null : company; + return ( - {technician?.contactName || vendor?.contactName || roster?.name || "Vendor company"} - - - {roster?.name || vendor?.companyName || "Vendor details"} + {title} + {subtitle !== null && ( + + {subtitle} + + )} @@ -118,17 +128,24 @@ function CompanySection({ roster }: { roster: VendorCompanyRoster }) { - {Boolean(roster.googleMapsUrl) && ( - - - Open in Google Maps - - )} + + + Google Maps + + {roster.googleMapsUrl ? ( + + + Open in Google Maps + + ) : ( + — + )} + ); } @@ -154,23 +171,14 @@ function TechnicianSection({ technician }: { technician: VendorRosterTechnician )} - + {technician.totalJobs ?? 0} Total Jobs - - - Status - - - + @@ -254,10 +262,13 @@ function DrawerEditor({ const selectedIndex = technicians.findIndex( (technician) => technician.id != null && String(technician.id) === String(vendor.id), ); - const selectedTotalJobs = - roster?.technicians.find( - (technician) => technician.id != null && String(technician.id) === String(vendor.id), - )?.totalJobs ?? vendor.totalJobs; + const persistedTechnician = roster?.technicians.find( + (technician) => technician.id != null && String(technician.id) === String(vendor.id), + ); + const selectedTotalJobs = persistedTechnician?.totalJobs ?? vendor.totalJobs; + // Deactivation is a property of the saved vendor, not of the toggle: flipping an + // inactive vendor on and back off without saving must not ask to deactivate it. + const persistedIsActive = persistedTechnician?.isActive ?? vendor.isActive; if (form.isLoading) { return ; @@ -311,7 +322,7 @@ function DrawerEditor({ checked={Boolean(field.value)} slotProps={{ input: { "aria-label": "Active status" } }} onChange={(_event, checked) => { - if (field.value && !checked) onRequestDeactivation(vendor); + if (persistedIsActive && !checked) onRequestDeactivation(vendor); else field.onChange(checked); }} /> diff --git a/src/app/(protected)/vendors/_components/vendor-status-badge.tsx b/src/app/(protected)/vendors/_components/vendor-status-badge.tsx new file mode 100644 index 00000000..7fee8970 --- /dev/null +++ b/src/app/(protected)/vendors/_components/vendor-status-badge.tsx @@ -0,0 +1,20 @@ +import { Box, Stack } from "@mui/material"; +import { Text } from "@/components/ui/text"; + +/** Dot plus text, so the status never reads by colour alone. */ +export function VendorStatusBadge({ isActive }: { isActive: boolean }) { + return ( + + + {isActive ? "Active" : "Inactive"} + + ); +} diff --git a/src/app/(protected)/vendors/_components/vendors-table.tsx b/src/app/(protected)/vendors/_components/vendors-table.tsx index db588cff..e524630c 100644 --- a/src/app/(protected)/vendors/_components/vendors-table.tsx +++ b/src/app/(protected)/vendors/_components/vendors-table.tsx @@ -18,6 +18,7 @@ import { TableRow, Tooltip, } from "@mui/material"; +import { VendorStatusBadge } from "./vendor-status-badge"; import { Text } from "@/components/ui/text"; import type { VendorListItem } from "@/domain/vendors/types/vendor"; @@ -50,23 +51,6 @@ function stopPropagation(event: MouseEvent): void { event.stopPropagation(); } -function VendorStatus({ isActive }: { isActive: boolean }) { - return ( - - - {isActive ? "Active" : "Inactive"} - - ); -} - interface VendorTableRowProps { row: VendorListItem; onOpenDetail: (row: VendorListItem) => void; @@ -177,7 +161,9 @@ function VendorTableRow({ row, onOpenDetail, onOpenEdit }: VendorTableRowProps) {row.totalJobs ?? 0} - + + + => { + try { + const data = await apiPatch( + API_PATHS.vendorCompanyRoster.byCompany(companyId), + mapVendorRosterAdditivePatchToBackend(patch), + ); + return mapVendorCompanyRoster(handleApiResponse(data)); + } catch (error) { + if (isHTTPError(error) && error.response.status === 409) { + await throwRosterConflict(error); + } + throw error; + } + }, }; diff --git a/src/domain/vendors/api/vendors-api.ts b/src/domain/vendors/api/vendors-api.ts index 467a3a3b..5afbc13e 100644 --- a/src/domain/vendors/api/vendors-api.ts +++ b/src/domain/vendors/api/vendors-api.ts @@ -101,8 +101,11 @@ export const vendorsApi = { return mapVendor(handleApiResponse(data)); }, - delete: async (id: string | number): Promise => { - await apiDelete(`${API_PATHS.rest.vendors}/${id}`); + // SH-254: confirmOpenWorkOrders tells the API the caller has been shown the + // vendor's open work orders and chose to proceed. Without it the API still blocks. + delete: async (id: string | number, confirmOpenWorkOrders = false): Promise => { + const suffix = confirmOpenWorkOrders ? "?confirmOpenWorkOrders=true" : ""; + await apiDelete(`${API_PATHS.rest.vendors}/${id}${suffix}`); }, getDeactivationImpact: async (id: string | number): Promise => { diff --git a/src/domain/vendors/mappers/vendor-roster-mapper.ts b/src/domain/vendors/mappers/vendor-roster-mapper.ts index 8dd450c4..641042ef 100644 --- a/src/domain/vendors/mappers/vendor-roster-mapper.ts +++ b/src/domain/vendors/mappers/vendor-roster-mapper.ts @@ -134,6 +134,36 @@ export function mapVendorRosterToBackend(values: unknown): Record; +} + +export function mapVendorRosterAdditivePatchToBackend( + patch: VendorRosterAdditivePatch, +): Record { + const item = asRecord(patch); + const payload: Record = { + rowVersion: readString(item, "rowVersion", "RowVersion"), + addTechnicians: patch.addTechnicians.map((technician) => { + const mapped = mapRosterTechnicianToBackend(technician); + delete mapped.id; + return mapped; + }), + }; + if (patch.companyFields) { + // SH-250: match the reconcile write path, which canonicalizes companyPhone. + // Copying companyFields verbatim let additive patches send a different phone + // shape than PUT for the same user input. + const companyFields = { ...patch.companyFields }; + if (companyFields.companyPhone !== undefined) + companyFields.companyPhone = toCanonicalPhone(companyFields.companyPhone); + payload.companyFields = companyFields; + } + return payload; +} + function mapBlockedWorkOrder(raw: unknown): VendorRosterBlockedWorkOrder { const item = asRecord(raw); return { diff --git a/src/domain/vendors/schemas/vendor-roster-schema.ts b/src/domain/vendors/schemas/vendor-roster-schema.ts index a429d69e..9358b12b 100644 --- a/src/domain/vendors/schemas/vendor-roster-schema.ts +++ b/src/domain/vendors/schemas/vendor-roster-schema.ts @@ -83,11 +83,14 @@ export const vendorCompanyRosterUpdateSchema = vendorCompanyRosterSchema.and( export type VendorCompanyRosterUpdateValues = z.infer; +// SH-250: the preferred-contact control was removed from the form, so seeding a +// value here fabricated one on submit. `append` already omits it; the schema +// keeps it optional, so both paths now agree and the backend receives it only +// when a real value exists. export const emptyRosterTechnician: RosterTechnicianValues = { contactName: "", phone: "", email: "", - preferredContact: "Phone", tradeSpecialties: "", isActive: true, }; diff --git a/src/domain/vendors/use-cases/use-delete-vendor.ts b/src/domain/vendors/use-cases/use-delete-vendor.ts index 0457cc7e..17f8ae11 100644 --- a/src/domain/vendors/use-cases/use-delete-vendor.ts +++ b/src/domain/vendors/use-cases/use-delete-vendor.ts @@ -3,11 +3,17 @@ import { toast } from "react-toastify"; import { vendorsApi } from "@/domain/vendors/api/vendors-api"; import { queryKeys } from "@/infra/query-key/query-key"; -export function useDeleteVendor(): UseMutationResult { +export interface DeleteVendorInput { + id: string | number; + confirmOpenWorkOrders?: boolean; +} + +export function useDeleteVendor(): UseMutationResult { const queryClient = useQueryClient(); return useMutation({ - mutationFn: (id: string | number) => vendorsApi.delete(id), + mutationFn: ({ id, confirmOpenWorkOrders = false }: DeleteVendorInput) => + vendorsApi.delete(id, confirmOpenWorkOrders), onSuccess: () => { void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.all }); toast.success("Vendor deactivated"); 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..ca7fa247 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 @@ -11,13 +11,17 @@ import type { import { queryKeys } from "@/infra/query-key/query-key"; export interface SaveVendorCompanyRosterInput { - mode: "create" | "update"; + mode: "create" | "update" | "add"; values: VendorCompanyRosterFormValues; companyId?: string | number | null; rowVersion?: string; originalRoster?: VendorCompanyRoster; + /** Company fields the user actually edited. See buildAdditiveRosterPatch. */ + editedCompanyFields?: EditedCompanyFields; } +export type EditedCompanyFields = Partial>; + export interface SaveVendorCompanyRosterContext { conflict?: unknown; } @@ -65,6 +69,66 @@ export function getSingleStatusOnlyChange( return changed.length === 1 ? changed[0] : null; } +export interface VendorRosterAdditivePatchInput { + rowVersion: string; + addTechnicians: Array<{ + contactName: string; + phone: string; + email: string; + preferredContact?: string; + tradeSpecialties: string; + isActive: boolean; + }>; + companyFields?: Record; +} + +function hasTechnicianContent(technician: { + contactName: string; + phone: string; + email: string; + tradeSpecialties: string; +}): boolean { + return Boolean( + technician.contactName.trim() || + technician.phone.trim() || + technician.email.trim() || + technician.tradeSpecialties.trim(), + ); +} + +/** + * Builds the additive PATCH body for the Add flow. + * + * `editedCompanyFields` must carry the fields the user actually touched. A plain + * form-vs-roster diff is unsafe here: after a stale-409 the roster is refetched while the form + * keeps the values loaded before the conflict, so a field another user changed in that window + * would look "different" and be sent back with our stale value, silently reverting their edit. + * Only fields the user edited are ever transmitted. + */ +export function buildAdditiveRosterPatch( + roster: VendorCompanyRoster | null | undefined, + values: VendorCompanyRosterFormValues, + rowVersion: string, + editedCompanyFields: EditedCompanyFields = {}, +): VendorRosterAdditivePatchInput | null { + const addTechnicians = values.technicians.filter( + (technician) => technician.id == null && hasTechnicianContent(technician), + ); + const companyFields: Record = {}; + if (roster) { + for (const field of COMPANY_FIELDS) { + if (editedCompanyFields[field] !== true) continue; + if (values[field] !== roster[field]) companyFields[field] = values[field]; + } + } + if (addTechnicians.length === 0 && Object.keys(companyFields).length === 0) return null; + return { + rowVersion, + addTechnicians, + ...(Object.keys(companyFields).length > 0 ? { companyFields } : {}), + }; +} + export function useSaveVendorCompanyRoster(): UseMutationResult< VendorCompanyRoster, unknown, @@ -85,6 +149,7 @@ export function useSaveVendorCompanyRoster(): UseMutationResult< companyId, rowVersion, originalRoster, + editedCompanyFields, }: SaveVendorCompanyRosterInput) => { if (mode === "update") { const statusChange = originalRoster @@ -108,17 +173,31 @@ export function useSaveVendorCompanyRoster(): UseMutationResult< if (!rowVersion) throw new Error("Row version is required to update"); return vendorCompanyRosterApi.update(companyId, values, rowVersion); } + if (mode === "add") { + if (companyId === undefined || companyId === null || companyId === "") { + throw new Error("Company id is required to add technicians"); + } + if (!rowVersion) throw new Error("Row version is required to add technicians"); + const patch = buildAdditiveRosterPatch( + originalRoster, + values, + rowVersion, + editedCompanyFields, + ); + if (!patch) throw new Error("Nothing to add to this vendor company"); + return vendorCompanyRosterApi.addTechnicians(companyId, patch); + } return vendorCompanyRosterApi.create(values); }, onSuccess: (data, variables) => { void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.all }); - if (variables.mode === "update" && variables.companyId) { + if ((variables.mode === "update" || variables.mode === "add") && variables.companyId) { void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.roster("companyId", variables.companyId), }); } toast.success( - variables.mode === "update" ? "Vendor company updated" : "Vendor company created", + variables.mode === "create" ? "Vendor company created" : "Vendor company updated", ); }, onError: (error) => { 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 2e9f8f1f..b56aa714 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 @@ -12,15 +12,19 @@ vi.mock("@/domain/vendors/api/vendor-company-roster-api", () => ({ })); vi.mock("@/domain/vendors/use-cases/use-vendor-company-roster", () => ({ - useVendorCompanyRoster: () => ({ - data: undefined, - isLoading: false, - isError: false, - error: null, - refetch: vi.fn(), - }), + useVendorCompanyRoster: () => rosterQueryResult, })); +vi.mock("react-toastify", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +let rosterQueryResult: { + data: VendorCompanyRoster | undefined; + isLoading: boolean; + isError: boolean; + error: Error | null; + refetch: ReturnType; +} = { data: undefined, isLoading: false, isError: false, error: null, refetch: vi.fn() }; + vi.mock("@/domain/vendors/use-cases/use-vendor-facets", () => ({ useVendorFacets: () => ({ data: { companies: [], trades: [] } }), })); @@ -36,6 +40,7 @@ vi.mock("@/domain/vendors/use-cases/use-save-vendor-company-roster", async () => }); import { useVendorRosterForm } from "@/app/(protected)/vendors/_components/use-vendor-roster-form"; +import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; import type { VendorCompanyRoster, VendorFacetCompany } from "@/domain/vendors/types/vendor"; const company: VendorFacetCompany = { @@ -81,10 +86,87 @@ const secondRoster: VendorCompanyRoster = { email: "dispatch@metro.test", }; +const rosterWithTechnicians: VendorCompanyRoster = { + ...roster, + technicians: [ + { + id: 11, + contactName: "Ray Holt", + phone: "(314) 555-0122", + email: "", + preferredContact: "Phone", + tradeSpecialties: "Plumbing", + isActive: true, + totalJobs: 0, + }, + { + id: 12, + contactName: "Amy Santiago", + phone: "(314) 555-0133", + email: "", + preferredContact: "Text", + tradeSpecialties: "HVAC", + isActive: true, + totalJobs: 0, + }, + ], +}; + +const newTechnician = { + contactName: "Dana Kim", + phone: "(314) 555-0144", + email: "dana@test.test", + preferredContact: "Text" as const, + tradeSpecialties: "HVAC", + isActive: true, +}; + +function submitValues( + technicians: VendorCompanyRosterFormValues["technicians"], +): VendorCompanyRosterFormValues { + return { + name: "Gateway Plumbing", + companyPhone: "(314) 555-0100", + email: "dispatch@gateway.test", + address: "1 Main St", + city: "St. Louis", + state: "MO", + zip: "63101", + googleMapsUrl: "", + notes: "", + technicians, + }; +} + +function resetRosterQuery(): void { + rosterQueryResult = { + data: undefined, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }; +} + +beforeEach(() => { + resetRosterQuery(); +}); + function createClient(): QueryClient { return new QueryClient({ defaultOptions: { queries: { retry: false } } }); } +/** + * SH-250: mirrors the app's real staleTime (src/lib/query/query-client.ts). The default + * test client uses staleTime 0, which silently masks cache-staleness defects on the + * reload path — the conflict recovery must not depend on an empty cache. + */ +function createCachingClient(): QueryClient { + return new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: 5 * 60 * 1000 } }, + }); +} + function makeWrapper(client: QueryClient) { return function Wrapper({ children }: { children: ReactNode }) { return {children}; @@ -316,3 +398,288 @@ describe("useVendorRosterForm stale-selection handling", () => { expect(result.current.name.field.value).toBe("Draft Vendor"); }); }); + +describe("useVendorRosterForm additive add flow (SH-250)", () => { + beforeEach(() => { + rosterGet.mockReset(); + saveMutate.mockReset(); + }); + + it("issues an additive add — never update — when an existing company is selected", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "create", startWithTechnician: true }); + const technician = useController({ + control: form.control, + name: "technicians.0.contactName", + }); + return { form, technician }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await act(async () => { + await result.current.form.selectCompany(company); + }); + await act(async () => { + result.current.technician.field.onChange("Dana Kim"); + }); + act(() => { + result.current.form.submit(submitValues([newTechnician])); + }); + + expect(saveMutate).toHaveBeenCalledTimes(1); + const input = saveMutate.mock.calls[0]?.[0] as { mode: string }; + expect(input.mode).toBe("add"); + expect(saveMutate).toHaveBeenCalledWith( + expect.objectContaining({ + companyId: 5, + rowVersion: "rv-1", + originalRoster: roster, + values: expect.objectContaining({ technicians: [newTechnician] }), + }), + expect.any(Object), + ); + }); + + it("still creates via POST when no existing company is matched", () => { + const { result } = renderHook( + () => useVendorRosterForm({ mode: "create", startWithTechnician: true }), + { wrapper: makeWrapper(createClient()) }, + ); + + act(() => { + result.current.submit(submitValues([newTechnician])); + }); + + expect(saveMutate).toHaveBeenCalledTimes(1); + expect(saveMutate.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ mode: "create", companyId: undefined, rowVersion: undefined }), + ); + }); + + it("blocks submit when the selected company has nothing to add", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook(() => useVendorRosterForm({ mode: "create" }), { + wrapper: makeWrapper(createClient()), + }); + + await act(async () => { + await result.current.selectCompany(company); + }); + act(() => { + result.current.submit(submitValues([])); + }); + + expect(saveMutate).not.toHaveBeenCalled(); + }); + + it("keeps the entered technician and retries with the fresh rowVersion after reload", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "create", startWithTechnician: true }); + const technician = useController({ + control: form.control, + name: "technicians.0.contactName", + }); + return { form, technician }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await act(async () => { + await result.current.form.selectCompany(company); + }); + await act(async () => { + result.current.technician.field.onChange("Dana Kim"); + }); + act(() => { + result.current.form.submit(submitValues([newTechnician])); + }); + expect(saveMutate.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ rowVersion: "rv-1" })); + + rosterGet.mockResolvedValueOnce({ ...roster, rowVersion: "rv-2" }); + await act(async () => { + result.current.form.reload(); + }); + await waitFor(() => expect(result.current.form.selectedCompanyId).toBe(5)); + + act(() => { + result.current.form.submit(submitValues([newTechnician])); + }); + expect(saveMutate.mock.calls[1]?.[0]).toEqual( + expect.objectContaining({ mode: "add", rowVersion: "rv-2" }), + ); + }); + + it("refreshes the visible company fields on reload, not just rowVersion", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "create", startWithTechnician: true }); + const name = useController({ control: form.control, name: "name" }); + return { form, name }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await act(async () => { + await result.current.form.selectCompany(company); + }); + expect(result.current.name.field.value).toBe("Gateway Plumbing"); + + // Someone else renamed the company, which is what caused the 409. + rosterGet.mockResolvedValueOnce({ + ...roster, + rowVersion: "rv-2", + name: "Gateway Plumbing & Drain", + }); + await act(async () => { + result.current.form.reload(); + }); + + // Reload must not leave the form on the pre-conflict name, or the retry + // would diff stale fields and overwrite the concurrent update. + await waitFor(() => expect(result.current.name.field.value).toBe("Gateway Plumbing & Drain")); + }); + + it("reload bypasses the cache so the retry carries the fresh rowVersion", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook( + () => useVendorRosterForm({ mode: "create", startWithTechnician: true }), + { wrapper: makeWrapper(createCachingClient()) }, + ); + + await act(async () => { + await result.current.selectCompany(company); + }); + expect(rosterGet).toHaveBeenCalledTimes(1); + + act(() => { + result.current.submit(submitValues([newTechnician])); + }); + expect(saveMutate.mock.calls[0]?.[0]).toEqual(expect.objectContaining({ rowVersion: "rv-1" })); + + // The 409 never invalidates this key, and in the Add flow nothing else observes it. + // Without staleTime: 0 on the reload fetch, the cached rv-1 is served straight back + // and every retry 409s again. + rosterGet.mockResolvedValueOnce({ ...roster, rowVersion: "rv-2" }); + await act(async () => { + result.current.reload(); + }); + + await waitFor(() => expect(rosterGet).toHaveBeenCalledTimes(2)); + + act(() => { + result.current.submit(submitValues([newTechnician])); + }); + expect(saveMutate.mock.calls[1]?.[0]).toEqual( + expect.objectContaining({ mode: "add", rowVersion: "rv-2" }), + ); + }); + + it("does not regress the Edit Vendor reconcile path (mode update)", () => { + rosterQueryResult = { + data: rosterWithTechnicians, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }; + const { result } = renderHook(() => useVendorRosterForm({ mode: "update", vendorId: 7 }), { + wrapper: makeWrapper(createClient()), + }); + + act(() => { + result.current.submit(submitValues([...rosterWithTechnicians.technicians, newTechnician])); + }); + + expect(saveMutate).toHaveBeenCalledTimes(1); + expect(saveMutate.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + mode: "update", + companyId: 5, + rowVersion: "rv-1", + originalRoster: rosterWithTechnicians, + values: expect.objectContaining({ + technicians: [...rosterWithTechnicians.technicians, newTechnician], + }), + }), + ); + }); +}); + +describe("useVendorRosterForm technician autopopulation (SH-246)", () => { + beforeEach(() => { + rosterGet.mockReset(); + saveMutate.mockReset(); + }); + + it("does not autopopulate technician fields when a company is selected", async () => { + rosterGet.mockResolvedValueOnce(rosterWithTechnicians); + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "create" }); + const technicians = useController({ control: form.control, name: "technicians" }); + const name = useController({ control: form.control, name: "name" }); + return { form, technicians, name }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await act(async () => { + await result.current.form.selectCompany(company); + }); + + const technicians = result.current.technicians.field.value; + expect(technicians).toEqual([ + { + contactName: "", + phone: "", + email: "", + tradeSpecialties: "", + isActive: true, + }, + ]); + // SH-250: the blank row must not carry a fabricated preferredContact, since + // the control was removed from the form and `append` omits it too. + expect(technicians[0]).not.toHaveProperty("preferredContact"); + expect(technicians).not.toContainEqual(expect.objectContaining({ contactName: "Ray Holt" })); + expect(technicians).not.toContainEqual( + expect.objectContaining({ contactName: "Amy Santiago" }), + ); + expect(result.current.name.field.value).toBe("Gateway Plumbing"); + expect(result.current.form.selectedCompanyId).toBe(5); + }); + + it("keeps a typed technician when the company selection is cleared to free text", async () => { + rosterGet.mockResolvedValueOnce(roster); + const { result } = renderHook( + () => { + const form = useVendorRosterForm({ mode: "create", startWithTechnician: true }); + const technician = useController({ + control: form.control, + name: "technicians.0.contactName", + }); + return { form, technician }; + }, + { wrapper: makeWrapper(createClient()) }, + ); + + await act(async () => { + await result.current.form.selectCompany(company); + }); + await act(async () => { + result.current.technician.field.onChange("Dana Kim"); + }); + + act(() => { + result.current.form.clearSelectedCompany("Draft Vendor"); + }); + + expect(result.current.form.selectedCompanyId).toBeNull(); + expect(result.current.technician.field.value).toBe("Dana Kim"); + }); +}); 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 df329a4a..4e52aa7f 100644 --- a/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx +++ b/src/test/app/(protected)/vendors/vendor-detail-drawer.test.tsx @@ -1,4 +1,5 @@ import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { VendorDetailDrawer } from "@/app/(protected)/vendors/_components/vendor-detail-drawer"; import { renderWithProviders } from "@/test/test-utils"; @@ -134,3 +135,139 @@ describe("VendorDetailDrawer selected-technician display", () => { expect(screen.queryByRole("switch", { name: "Active status" })).not.toBeInTheDocument(); }); }); + +describe("VendorDetailDrawer design parity", () => { + it("renders the company once when there is no technician name to head the panel", () => { + useVendorCompanyRoster.mockReturnValue(rosterWith([])); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + const heading = screen.getByRole("heading", { level: 2, name: "Gateway Plumbing" }); + expect(heading).toBeInTheDocument(); + // The header block holds the title alone — no subtitle repeating the company. + expect(heading.parentElement?.children).toHaveLength(1); + }); + + it("keeps the company subtitle under a technician name", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByRole("heading", { level: 2, name: "Adam" })).toBeInTheDocument(); + expect(screen.getAllByText("Gateway Plumbing").length).toBeGreaterThan(0); + }); + + it("labels the Google Maps field below Address even when no URL is stored", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByText("Google Maps")).toBeInTheDocument(); + expect(screen.queryByRole("link", { name: /Open in Google Maps/ })).not.toBeInTheDocument(); + }); + + it("links out to Google Maps when a URL is stored", () => { + const roster = rosterWith([ + { id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }, + ]); + roster.data.googleMapsUrl = "https://maps.google.com/?q=1+Industrial+Pkwy"; + useVendorCompanyRoster.mockReturnValue(roster); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + expect(screen.getByRole("link", { name: /Open in Google Maps/ })).toHaveAttribute( + "href", + "https://maps.google.com/?q=1+Industrial+Pkwy", + ); + }); + + it("puts the status opposite Total Jobs and drops the redundant Status label", () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([{ id: 1, contactName: "Adam", phone: "", isActive: true, totalJobs: 5 }]), + ); + + renderWithProviders(, { + route: "/vendors", + withAuth: false, + }); + + const row = screen.getByText("Total Jobs").closest("div")?.parentElement; + expect(row).not.toBeNull(); + expect(getComputedStyle(row as Element).justifyContent).toBe("space-between"); + expect(row).toHaveTextContent("Active"); + expect(screen.queryByText("Status")).not.toBeInTheDocument(); + }); +}); + +describe("VendorDetailDrawer deactivation prompt", () => { + const inactiveVendor: VendorListItem = { ...vendor, isActive: false }; + + it("does not prompt to deactivate a saved-inactive vendor toggled on and back off", async () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([ + { id: 1, contactName: "Adam", phone: "314-555-0198", isActive: false, totalJobs: 5 }, + ]), + ); + const onRequestDeactivation = vi.fn(); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + const toggle = screen.getByRole("switch", { name: "Active status" }); + await userEvent.click(toggle); + expect(toggle).toBeChecked(); + + await userEvent.click(toggle); + + expect(toggle).not.toBeChecked(); + expect(onRequestDeactivation).not.toHaveBeenCalled(); + }); + + it("prompts to deactivate a saved-active vendor when the toggle is switched off", async () => { + useVendorCompanyRoster.mockReturnValue( + rosterWith([ + { id: 1, contactName: "Adam", phone: "314-555-0198", isActive: true, totalJobs: 5 }, + ]), + ); + const onRequestDeactivation = vi.fn(); + + renderWithProviders( + , + { route: "/vendors", withAuth: false }, + ); + + await userEvent.click(screen.getByRole("switch", { name: "Active status" })); + + expect(onRequestDeactivation).toHaveBeenCalledWith(vendor); + }); +}); diff --git a/src/test/app/(protected)/vendors/vendors-list.test.tsx b/src/test/app/(protected)/vendors/vendors-list.test.tsx index cbfa47f9..ae5a18d3 100644 --- a/src/test/app/(protected)/vendors/vendors-list.test.tsx +++ b/src/test/app/(protected)/vendors/vendors-list.test.tsx @@ -177,7 +177,7 @@ describe("VendorsListPage", () => { expect(screen.getByRole("heading", { level: 2, name: "Adam Whyte" })).toBeInTheDocument(); }); - it("blocks deactivation when the preflight reports open work orders", async () => { + it("offers deactivate-anyway when the preflight reports open work orders", async () => { setupDefaults(); useVendorCompanyRoster.mockReturnValue({ data: activeRoster, @@ -213,13 +213,52 @@ describe("VendorsListPage", () => { await userEvent.click(screen.getByRole("switch", { name: "Active status" })); expect( - screen.getByText(/cannot be deactivated because it still has open work orders/), + screen.getByText( + /will no longer be selectable for new work orders\. It still has 1 open work order —/, + ), ).toBeInTheDocument(); - expect(screen.getByText(/Boiler repair/)).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /^Deactivate$/ })).toBeDisabled(); - expect(mutate).not.toHaveBeenCalled(); + expect(screen.getByRole("link", { name: /Boiler repair/ })).toHaveAttribute( + "href", + "/workorders/101", + ); + + const confirm = screen.getByRole("button", { name: "Deactivate anyway" }); + expect(confirm).toBeEnabled(); + await userEvent.click(confirm); + + expect(mutate).toHaveBeenCalledWith({ id: 1, confirmOpenWorkOrders: true }, expect.anything()); }); + it("deactivates without the confirmation flag when nothing is linked", async () => { + setupDefaults(); + useVendorCompanyRoster.mockReturnValue({ + data: activeRoster, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + }); + useVendorDeactivationImpact.mockReturnValue({ + data: { vendorId: 1, canDeactivate: true, openWorkOrders: [] }, + isLoading: false, + error: null, + }); + useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => { + if (params.pageSize === 1) return result([], 1); + return params.isActive ? result([activeVendor], 1) : result([], 0); + }); + + renderWithProviders(, { route: "/vendors", withAuth: false }); + + await userEvent.click(screen.getByRole("button", { name: "Edit vendor Gateway Plumbing" })); + await userEvent.click(screen.getByRole("switch", { name: "Active status" })); + + expect(screen.queryByText(/It still has/)).not.toBeInTheDocument(); + await userEvent.click(screen.getByRole("button", { name: /^Deactivate$/ })); + + expect(mutate).toHaveBeenCalledWith({ id: 1, confirmOpenWorkOrders: false }, expect.anything()); + }, 10_000); + it("preserves inline edits when deactivation is cancelled", async () => { setupDefaults(); useVendorCompanyRoster.mockReturnValue({ @@ -247,7 +286,7 @@ describe("VendorsListPage", () => { await userEvent.type(company, "Draft Company Name"); await userEvent.click(screen.getByRole("switch", { name: "Active status" })); await userEvent.click( - within(screen.getByRole("dialog", { name: "Deactivate Vendor" })).getByRole("button", { + within(screen.getByRole("dialog", { name: "Deactivate this vendor?" })).getByRole("button", { name: "Cancel", }), ); diff --git a/src/test/domain/vendors/api/vendor-company-roster-api.test.ts b/src/test/domain/vendors/api/vendor-company-roster-api.test.ts index 1bad51e4..32fb5aeb 100644 --- a/src/test/domain/vendors/api/vendor-company-roster-api.test.ts +++ b/src/test/domain/vendors/api/vendor-company-roster-api.test.ts @@ -4,11 +4,13 @@ import { API_PATHS } from "@/api/api-paths"; const apiGet = vi.fn(); const apiPost = vi.fn(); const apiPut = vi.fn(); +const apiPatch = vi.fn(); vi.mock("@/api/api", () => ({ apiGet: (...args: unknown[]) => apiGet(...args), apiPost: (...args: unknown[]) => apiPost(...args), apiPut: (...args: unknown[]) => apiPut(...args), + apiPatch: (...args: unknown[]) => apiPatch(...args), })); vi.mock("ky", () => ({ @@ -38,6 +40,7 @@ describe("vendorCompanyRosterApi", () => { apiGet.mockReset(); apiPost.mockReset(); apiPut.mockReset(); + apiPatch.mockReset(); }); it("fetches by vendorId or companyId with the correct query params", async () => { @@ -121,4 +124,72 @@ describe("vendorCompanyRosterApi", () => { await expect(vendorCompanyRosterApi.create({ name: "Solo Co" })).rejects.toBe(generic); }); + + it("patches the additive payload without a technician id and without PUT", async () => { + apiPatch.mockResolvedValueOnce({ data: { companyId: 5, name: "Solo Co" } }); + + await vendorCompanyRosterApi.addTechnicians(5, { + rowVersion: "rv-1", + addTechnicians: [ + { + id: 99, + contactName: "Adam", + phone: "3145550198", + email: "", + preferredContact: "Phone", + tradeSpecialties: "Plumbing", + isActive: true, + }, + ], + }); + + expect(apiPatch).toHaveBeenCalledWith( + API_PATHS.vendorCompanyRoster.byCompany(5), + expect.objectContaining({ + rowVersion: "rv-1", + addTechnicians: [expect.objectContaining({ contactName: "Adam", phone: "(314) 555-0198" })], + }), + ); + const body = apiPatch.mock.calls[0]?.[1] as Record; + expect(body).not.toHaveProperty("companyFields"); + expect((body.addTechnicians as Array>)[0]).not.toHaveProperty("id"); + expect(apiPut).not.toHaveBeenCalled(); + expect(apiPost).not.toHaveBeenCalled(); + }); + + it("includes companyFields on the additive patch when provided", async () => { + apiPatch.mockResolvedValueOnce({ data: { companyId: 5, name: "Solo Co" } }); + + await vendorCompanyRosterApi.addTechnicians(5, { + rowVersion: "rv-2", + addTechnicians: [ + { contactName: "Adam", phone: "", email: "", tradeSpecialties: "", isActive: true }, + ], + companyFields: { notes: "Updated notes" }, + }); + + expect(apiPatch).toHaveBeenCalledWith( + API_PATHS.vendorCompanyRoster.byCompany(5), + expect.objectContaining({ + rowVersion: "rv-2", + companyFields: { notes: "Updated notes" }, + }), + ); + }); + + it("throws a stale conflict on a 409 from the additive patch", async () => { + apiPatch.mockRejectedValueOnce(httpError(409, { message: "rowversion mismatch" })); + + await expect( + vendorCompanyRosterApi.addTechnicians(5, { + rowVersion: "rv-1", + addTechnicians: [ + { contactName: "Adam", phone: "", email: "", tradeSpecialties: "", isActive: true }, + ], + }), + ).rejects.toSatisfy((error: unknown) => { + if (!isVendorRosterConflictError(error)) return false; + return error.conflict.kind === "stale"; + }); + }); }); diff --git a/src/test/domain/vendors/use-cases/use-save-vendor-company-roster-additive.test.tsx b/src/test/domain/vendors/use-cases/use-save-vendor-company-roster-additive.test.tsx new file mode 100644 index 00000000..f03c2a27 --- /dev/null +++ b/src/test/domain/vendors/use-cases/use-save-vendor-company-roster-additive.test.tsx @@ -0,0 +1,263 @@ +import { act, renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const rosterAdd = vi.fn(); +const rosterUpdate = vi.fn(); +const rosterCreate = vi.fn(); + +vi.mock("@/domain/vendors/api/vendor-company-roster-api", () => ({ + vendorCompanyRosterApi: { + addTechnicians: (...args: unknown[]) => rosterAdd(...args), + update: (...args: unknown[]) => rosterUpdate(...args), + create: (...args: unknown[]) => rosterCreate(...args), + }, +})); + +vi.mock("@/domain/vendors/api/vendors-api", () => ({ + vendorsApi: { update: vi.fn() }, +})); + +vi.mock("react-toastify", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +import { + buildAdditiveRosterPatch, + useSaveVendorCompanyRoster, +} from "@/domain/vendors/use-cases/use-save-vendor-company-roster"; +import type { VendorCompanyRoster } from "@/domain/vendors/types/vendor"; +import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; + +const roster: VendorCompanyRoster = { + companyId: 5, + rowVersion: "rv-1", + name: "Gateway Plumbing", + companyPhone: "(314) 555-0100", + email: "dispatch@gateway.test", + address: "1 Main St", + city: "St. Louis", + state: "MO", + zip: "63101", + googleMapsUrl: "", + notes: "Preferred vendor", + technicians: [ + { + id: 11, + contactName: "Ray Holt", + phone: "(314) 555-0122", + email: "", + preferredContact: "Phone", + tradeSpecialties: "Plumbing", + isActive: true, + totalJobs: 0, + }, + ], +}; + +const newTechnician = { + contactName: "Dana Kim", + phone: "(314) 555-0144", + email: "dana@test.test", + preferredContact: "Text" as const, + tradeSpecialties: "HVAC", + isActive: true, +}; + +function formValues( + overrides: Partial = {}, +): VendorCompanyRosterFormValues { + return { + name: roster.name, + companyPhone: roster.companyPhone, + email: roster.email, + address: roster.address, + city: roster.city, + state: roster.state, + zip: roster.zip, + googleMapsUrl: roster.googleMapsUrl, + notes: roster.notes, + technicians: [newTechnician], + ...overrides, + }; +} + +describe("buildAdditiveRosterPatch", () => { + it("carries only the newly entered technician, never a full roster snapshot", () => { + const patch = buildAdditiveRosterPatch(roster, formValues(), "rv-1"); + + expect(patch).not.toBeNull(); + expect(patch?.addTechnicians).toEqual([newTechnician]); + expect(patch?.addTechnicians[0]).not.toHaveProperty("id"); + }); + + it("ignores blank technician rows", () => { + const patch = buildAdditiveRosterPatch( + roster, + formValues({ + technicians: [ + { + contactName: "", + phone: "", + email: "", + preferredContact: "Phone", + tradeSpecialties: "", + isActive: true, + }, + newTechnician, + ], + }), + "rv-1", + ); + + expect(patch?.addTechnicians).toEqual([newTechnician]); + }); + + it("sends only company fields the user edited", () => { + const patch = buildAdditiveRosterPatch( + roster, + formValues({ notes: "New notes", address: "2 Oak Ave" }), + "rv-1", + { notes: true, address: true }, + ); + + expect(patch?.companyFields).toEqual({ notes: "New notes", address: "2 Oak Ave" }); + }); + + it("never sends a company field the user did not edit, even when it differs from the roster", () => { + // Regression: after a stale-409 the roster is refetched while the form keeps pre-conflict + // values. A form-vs-roster diff would resend our stale value for a field another user changed + // in that window, silently reverting their edit. Only edited fields may be transmitted. + const rosterAfterOtherUserEditedPhone = { ...roster, companyPhone: "314-555-9999" }; + + const patch = buildAdditiveRosterPatch( + rosterAfterOtherUserEditedPhone, + formValues({ companyPhone: roster.companyPhone, notes: "New notes" }), + "rv-2", + { notes: true }, + ); + + expect(patch?.companyFields).toEqual({ notes: "New notes" }); + expect(patch?.companyFields).not.toHaveProperty("companyPhone"); + }); + + it("omits companyFields when values differ but nothing was edited", () => { + const patch = buildAdditiveRosterPatch( + roster, + formValues({ notes: "Drifted under us" }), + "rv-1", + {}, + ); + + expect(patch).not.toHaveProperty("companyFields"); + }); + + it("omits companyFields entirely when nothing changed", () => { + const patch = buildAdditiveRosterPatch(roster, formValues(), "rv-1"); + + expect(patch).not.toHaveProperty("companyFields"); + }); + + it("returns null when there is nothing to add or change", () => { + const patch = buildAdditiveRosterPatch( + roster, + formValues({ + technicians: [ + { + contactName: "", + phone: "", + email: "", + preferredContact: "Phone", + tradeSpecialties: "", + isActive: true, + }, + ], + }), + "rv-1", + ); + + expect(patch).toBeNull(); + }); +}); + +function createWrapper() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +describe("useSaveVendorCompanyRoster routing", () => { + beforeEach(() => { + rosterAdd.mockReset(); + rosterUpdate.mockReset(); + rosterCreate.mockReset(); + }); + + it("add mode issues the additive patch and never the reconcile PUT", async () => { + rosterAdd.mockResolvedValueOnce(roster); + const { result } = renderHook(() => useSaveVendorCompanyRoster(), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.mutate({ + mode: "add", + values: formValues(), + companyId: 5, + rowVersion: "rv-1", + originalRoster: roster, + }); + }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(rosterAdd).toHaveBeenCalledTimes(1); + expect(rosterAdd).toHaveBeenCalledWith( + 5, + expect.objectContaining({ + rowVersion: "rv-1", + addTechnicians: [newTechnician], + }), + ); + expect(rosterUpdate).not.toHaveBeenCalled(); + expect(rosterCreate).not.toHaveBeenCalled(); + }); + + it("update mode still issues the full reconcile PUT", async () => { + rosterUpdate.mockResolvedValueOnce(roster); + const { result } = renderHook(() => useSaveVendorCompanyRoster(), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.mutate({ + mode: "update", + values: formValues({ + technicians: [{ id: 11, ...newTechnician }, newTechnician], + }), + companyId: 5, + rowVersion: "rv-1", + originalRoster: roster, + }); + }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(rosterUpdate).toHaveBeenCalledWith(5, expect.anything(), "rv-1"); + expect(rosterAdd).not.toHaveBeenCalled(); + }); + + it("create mode still issues the POST", async () => { + rosterCreate.mockResolvedValueOnce(roster); + const { result } = renderHook(() => useSaveVendorCompanyRoster(), { + wrapper: createWrapper(), + }); + + act(() => { + result.current.mutate({ mode: "create", values: formValues() }); + }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(rosterCreate).toHaveBeenCalledTimes(1); + expect(rosterAdd).not.toHaveBeenCalled(); + expect(rosterUpdate).not.toHaveBeenCalled(); + }); +});