fix(vendors): correct additive add reload, phone and contact (SH-250, SH-246)

Three review findings on the additive add path:

- Create-mode reload refreshed selectedRoster (including rowVersion) but left
  the form company fields on their pre-conflict values, so a retry diffed stale
  fields against the reloaded roster and could overwrite the concurrent company
  update that caused the 409. Reload now resets the form from the fresh roster,
  matching the select path.
- mapVendorRosterAdditivePatchToBackend copied companyFields verbatim while the
  reconcile write mapper canonicalizes companyPhone, so PATCH and PUT could send
  different phone shapes for the same input.
- emptyRosterTechnician seeded preferredContact "Phone" even though that
  control was removed from the form, so filling the first blank row submitted a
  fabricated value. append already omitted it; both paths now agree.
This commit is contained in:
Codex Review Integration 2026-08-18 17:28:48 -03:00
parent 9624c264de
commit 9a066092b3
4 changed files with 53 additions and 4 deletions

View file

@ -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;

View file

@ -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;
}

View file

@ -83,11 +83,14 @@ export const vendorCompanyRosterUpdateSchema = vendorCompanyRosterSchema.and(
export type VendorCompanyRosterUpdateValues = z.infer<typeof vendorCompanyRosterUpdateSchema>;
// 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,
};

View file

@ -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" }),