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 4eb141b1..4bd467f1 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-selection.ts @@ -153,6 +153,11 @@ export function useVendorRosterSelection({ void fetchRosterByCompany(selectedRoster.companyId) .then((roster) => { if (requestIdRef.current !== requestId) return; + // 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; }) @@ -161,7 +166,7 @@ export function useVendorRosterSelection({ setLoadErrorMessage(RELOAD_ERROR_MESSAGE); }); } - }, [clearConflict, fetchRosterByCompany, mode, query, selectedRoster]); + }, [clearConflict, fetchRosterByCompany, mode, query, reset, selectedRoster]); const retryLoad = useCallback(() => { const target = retryTargetRef.current; diff --git a/src/domain/vendors/mappers/vendor-roster-mapper.ts b/src/domain/vendors/mappers/vendor-roster-mapper.ts index e4de67dd..641042ef 100644 --- a/src/domain/vendors/mappers/vendor-roster-mapper.ts +++ b/src/domain/vendors/mappers/vendor-roster-mapper.ts @@ -152,7 +152,15 @@ export function mapVendorRosterAdditivePatchToBackend( return mapped; }), }; - if (patch.companyFields) payload.companyFields = { ...patch.companyFields }; + 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; } 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/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx b/src/test/app/(protected)/vendors/use-vendor-roster-form.test.tsx index 80091636..777363fd 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 @@ -503,6 +503,37 @@ describe("useVendorRosterForm additive add flow (SH-250)", () => { ); }); + 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("does not regress the Edit Vendor reconcile path (mode update)", () => { rosterQueryResult = { data: rosterWithTechnicians, @@ -562,11 +593,13 @@ describe("useVendorRosterForm technician autopopulation (SH-246)", () => { contactName: "", phone: "", email: "", - preferredContact: "Phone", 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" }),