fix(work-orders): keep wizard vendor notes bound to the selected company

Late roster data no longer overwrites notes typed for the current selection.
The notes baseline is keyed by vendor, so a save never patches a previous
company. Changing the primary service clears leftover notes, and a save
with a missing baseline reads the current roster first. Also drops ticket
keys from shipped comments.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codex Review Integration 2026-09-16 15:18:06 -03:00
parent 7c090571c5
commit cd5d4ad1d8
8 changed files with 193 additions and 30 deletions

View file

@ -65,8 +65,9 @@ export function useNewWoWizardController({
};
const createMutation = useCreateWorkOrderFromWizard((createdDraft) => {
saveCompanyNotes(createdDraft);
onOpenChange(false);
// Close only after the company-notes save attempt settles; its failures are surfaced
// as note-save warnings and must never report the work-order creation as failed.
void saveCompanyNotes(createdDraft).finally(() => onOpenChange(false));
});
const { duplicateRow, setDuplicateRow, handleDuplicateFound, handleCreate, isCreating } =
useWizardDuplicateActions({

View file

@ -1,7 +1,14 @@
import { useEffect, useState } from "react";
import { useQueryClient } from "@tanstack/react-query";
import { toast } from "react-toastify";
import type { WorkOrderWizardDraft } from "@/domain/work-orders/types/work-order-wizard";
import { useVendorCompanyRoster } from "@/domain/vendors/use-cases/use-vendor-company-roster";
import { useSaveVendorCompanyNotes } from "@/domain/vendors/use-cases/use-save-vendor-company-notes";
import {
NOTES_SAVE_FAILED_MESSAGE,
useSaveVendorCompanyNotes,
} from "@/domain/vendors/use-cases/use-save-vendor-company-notes";
import { vendorCompanyRosterApi } from "@/domain/vendors/api/vendor-company-roster-api";
import { queryKeys } from "@/infra/query-key/query-key";
import {
buildCompanyNotesUpdate,
type VendorCompanyNotesBaseline,
@ -9,15 +16,21 @@ import {
type SetDraft = (updater: (current: WorkOrderWizardDraft) => WorkOrderWizardDraft) => void;
type VendorNotesBaseline = VendorCompanyNotesBaseline & { vendorId: string };
/**
* SH-321: the wizard's Vendor Notes field is the vendor company's notes (same field as the
* Vendors page). Picking a technician pre-fills the company's current notes once per company,
* so switching technicians within the same company keeps the dispatcher's edits.
* The wizard's Vendor Notes field is the vendor company's notes (same field as the Vendors
* page). Picking a technician pre-fills the company's current notes once per company, so
* switching technicians within the same company keeps the dispatcher's edits. Notes typed
* for the current selection always win over roster data that arrives late, and a save whose
* baseline is missing or belongs to another selection reads the current roster before
* building the PATCH.
*/
export function useWizardVendorCompanyNotes(open: boolean, vendorId: string, setDraft: SetDraft) {
const queryClient = useQueryClient();
const { data: roster } = useVendorCompanyRoster(vendorId || undefined);
const saveNotes = useSaveVendorCompanyNotes();
const [baseline, setBaseline] = useState<VendorCompanyNotesBaseline | null>(null);
const [baseline, setBaseline] = useState<VendorNotesBaseline | null>(null);
useEffect(() => {
if (open) {
@ -34,24 +47,72 @@ export function useWizardVendorCompanyNotes(open: boolean, vendorId: string, set
if (!roster || roster.companyId === null) {
return;
}
if (baseline && String(baseline.companyId) === String(roster.companyId)) {
if (
baseline &&
baseline.vendorId === vendorId &&
String(baseline.companyId) === String(roster.companyId)
) {
return;
}
setBaseline({
vendorId,
companyId: roster.companyId,
rowVersion: roster.rowVersion,
notes: roster.notes,
});
setDraft((current) => ({ ...current, vendorNotes: roster.notes }));
setDraft((current) => {
if (current.vendorId !== vendorId) {
return current;
}
// Seed only empty content or content still holding the previous company's unedited
// notes; anything else is the dispatcher's typing and must never be overwritten.
if (current.vendorNotes !== "" && current.vendorNotes !== baseline?.notes) {
return current;
}
return { ...current, vendorNotes: roster.notes };
});
}, [vendorId, roster, baseline, setDraft]);
function saveCompanyNotes(draft: WorkOrderWizardDraft) {
async function fetchCurrentBaseline(vendorId: string): Promise<VendorNotesBaseline | null> {
try {
const current = await queryClient.fetchQuery({
queryKey: queryKeys.vendors.roster("vendorId", vendorId),
queryFn: () => vendorCompanyRosterApi.get({ vendorId }),
});
if (!current || current.companyId === null) {
return null;
}
return {
vendorId,
companyId: current.companyId,
rowVersion: current.rowVersion,
notes: current.notes,
};
} catch {
toast.error(NOTES_SAVE_FAILED_MESSAGE);
return null;
}
}
async function saveCompanyNotes(draft: WorkOrderWizardDraft): Promise<void> {
if (!draft.vendorId) {
return;
}
const update = buildCompanyNotesUpdate(baseline, draft.vendorNotes);
if (update) {
saveNotes.mutate(update);
const active =
baseline && baseline.vendorId === draft.vendorId
? baseline
: await fetchCurrentBaseline(draft.vendorId);
if (!active) {
return;
}
const update = buildCompanyNotesUpdate(active, draft.vendorNotes);
if (!update) {
return;
}
try {
await saveNotes.mutateAsync(update);
} catch {
// The mutation already surfaces the note-save failure toast.
}
}

View file

@ -29,7 +29,7 @@ export function WizardStepLocationServiceSelect({
onPatch,
}: WizardStepLocationServiceSelectProps) {
const clearVendorIfPrimaryChanged = (nextPm: string): Partial<WorkOrderWizardDraft> =>
nextPm !== draft.pm ? { vendorId: "", vendorName: "", techPhone: "" } : {};
nextPm !== draft.pm ? { vendorId: "", vendorName: "", techPhone: "", vendorNotes: "" } : {};
const selectSvc = (p: string) => {
onPatch({

View file

@ -5,7 +5,10 @@ import type { VendorCompanyRoster } from "@/domain/vendors/types/vendor";
import type { VendorCompanyNotesUpdate } from "@/domain/vendors/utils/build-company-notes-patch";
import { queryKeys } from "@/infra/query-key/query-key";
/** Writes company-level vendor notes (SH-321) and refreshes every vendor view that shows them. */
export const NOTES_SAVE_FAILED_MESSAGE =
"Work order created, but the vendor notes could not be saved.";
/** Writes company-level vendor notes and refreshes every vendor view that shows them. */
export function useSaveVendorCompanyNotes(): UseMutationResult<
VendorCompanyRoster,
unknown,
@ -20,7 +23,7 @@ export function useSaveVendorCompanyNotes(): UseMutationResult<
void queryClient.invalidateQueries({ queryKey: queryKeys.vendors.all });
},
onError: () => {
toast.error("Work order created, but the vendor notes could not be saved.");
toast.error(NOTES_SAVE_FAILED_MESSAGE);
},
});
}

View file

@ -12,7 +12,7 @@ export interface VendorCompanyNotesUpdate {
}
/**
* SH-321: company-level notes edited outside the Vendors page (e.g. the Create WO wizard)
* Company-level notes edited outside the Vendors page (e.g. the Create WO wizard)
* are written through the additive roster PATCH, which only touches the fields sent.
* Returns null when nothing changed. Clearing notes is not expressible on that PATCH
* (blank company fields mean "unchanged"), so an emptied field is not sent.

View file

@ -3,10 +3,12 @@ import { QueryClientProvider } from "@tanstack/react-query";
import { useState, type ReactNode } from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useWizardVendorCompanyNotes } from "@/app/(protected)/workorders/_components/wizard/use-wizard-vendor-company-notes";
import { NOTES_SAVE_FAILED_MESSAGE } from "@/domain/vendors/use-cases/use-save-vendor-company-notes";
import {
EMPTY_WIZARD_DRAFT,
type WorkOrderWizardDraft,
} from "@/domain/work-orders/types/work-order-wizard";
import { toast } from "react-toastify";
import { createTestQueryClient } from "@/test/test-utils";
const rosterGet = vi.fn();
@ -21,6 +23,8 @@ vi.mock("@/domain/vendors/api/vendor-company-roster-api", () => ({
vi.mock("react-toastify", () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
type Roster = ReturnType<typeof roster>;
function roster(companyId: number, notes: string) {
return {
companyId,
@ -38,6 +42,17 @@ function roster(companyId: number, notes: string) {
};
}
function deferredRoster() {
let resolve!: (value: Roster) => void;
const pending = new Promise<Roster>((res) => {
resolve = res;
});
return {
promise: () => pending,
resolveAll: (value: Roster) => resolve(value),
};
}
function useHarness() {
const [draft, setDraft] = useState<WorkOrderWizardDraft>({ ...EMPTY_WIZARD_DRAFT });
const notes = useWizardVendorCompanyNotes(true, draft.vendorId, setDraft);
@ -52,10 +67,11 @@ function renderHarness() {
return renderHook(() => useHarness(), { wrapper });
}
describe("useWizardVendorCompanyNotes (SH-321)", () => {
describe("useWizardVendorCompanyNotes", () => {
beforeEach(() => {
rosterGet.mockReset();
addTechnicians.mockReset();
vi.mocked(toast.error).mockClear();
rosterGet.mockImplementation(({ vendorId }: { vendorId: string }) =>
Promise.resolve(
vendorId === "31" ? roster(9, "Other company notes") : roster(7, "Call before arrival"),
@ -98,22 +114,98 @@ describe("useWizardVendorCompanyNotes (SH-321)", () => {
await waitFor(() => expect(result.current.draft.vendorNotes).toBe("Other company notes"));
});
it("keeps notes typed before the roster resolves and saves them for that company", async () => {
const lateRoster = deferredRoster();
rosterGet.mockImplementation(({ vendorId }: { vendorId: string }) =>
vendorId === "21" ? lateRoster.promise() : Promise.resolve(roster(7, "Call before arrival")),
);
const { result } = renderHarness();
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "21" })));
act(() => result.current.setDraft((d) => ({ ...d, vendorNotes: "Gate code 4411" })));
await act(async () => lateRoster.resolveAll(roster(7, "Call before arrival")));
expect(result.current.draft.vendorNotes).toBe("Gate code 4411");
await act(async () => {
await result.current.saveCompanyNotes({
...result.current.draft,
vendorNotes: "Gate code 4411",
});
});
expect(addTechnicians).toHaveBeenCalledWith(7, {
rowVersion: "rv-7",
addTechnicians: [],
companyFields: { notes: "Gate code 4411" },
});
});
it("targets the newly selected company at save time while its roster is still loading", async () => {
const companyB = deferredRoster();
rosterGet.mockImplementation(({ vendorId }: { vendorId: string }) =>
vendorId === "21" ? Promise.resolve(roster(7, "Call before arrival")) : companyB.promise(),
);
const { result } = renderHarness();
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "21" })));
await waitFor(() => expect(result.current.draft.vendorNotes).toBe("Call before arrival"));
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "31" })));
act(() => result.current.setDraft((d) => ({ ...d, vendorNotes: "Gate code 4411" })));
let savePromise!: Promise<void>;
act(() => {
savePromise = result.current
.saveCompanyNotes({ ...result.current.draft, vendorNotes: "Gate code 4411" })
.then(() => undefined);
});
await act(async () => companyB.resolveAll(roster(9, "Other company notes")));
await act(async () => {
await savePromise;
});
expect(addTechnicians).toHaveBeenCalledWith(9, {
rowVersion: "rv-9",
addTechnicians: [],
companyFields: { notes: "Gate code 4411" },
});
expect(addTechnicians).not.toHaveBeenCalledWith(7, expect.anything());
expect(result.current.draft.vendorNotes).toBe("Gate code 4411");
});
it("warns without writing when the save-time roster read fails", async () => {
rosterGet.mockRejectedValue(new Error("roster down"));
const { result } = renderHarness();
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "21" })));
act(() => result.current.setDraft((d) => ({ ...d, vendorNotes: "Gate code 4411" })));
await act(async () => {
await result.current.saveCompanyNotes({
...result.current.draft,
vendorNotes: "Gate code 4411",
});
});
expect(toast.error).toHaveBeenCalledWith(NOTES_SAVE_FAILED_MESSAGE);
expect(addTechnicians).not.toHaveBeenCalled();
});
it("writes edited notes to the vendor company when the work order is created", async () => {
const { result } = renderHarness();
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "21" })));
await waitFor(() => expect(result.current.draft.vendorNotes).toBe("Call before arrival"));
act(() =>
result.current.saveCompanyNotes({ ...result.current.draft, vendorNotes: "Gate code 4411" }),
);
await act(async () => {
await result.current.saveCompanyNotes({
...result.current.draft,
vendorNotes: "Gate code 4411",
});
});
await waitFor(() =>
expect(addTechnicians).toHaveBeenCalledWith(7, {
rowVersion: "rv-7",
addTechnicians: [],
companyFields: { notes: "Gate code 4411" },
}),
);
expect(addTechnicians).toHaveBeenCalledWith(7, {
rowVersion: "rv-7",
addTechnicians: [],
companyFields: { notes: "Gate code 4411" },
});
});
it("does not write when the notes were left as the company's existing notes", async () => {
@ -121,7 +213,9 @@ describe("useWizardVendorCompanyNotes (SH-321)", () => {
act(() => result.current.setDraft((d) => ({ ...d, vendorId: "21" })));
await waitFor(() => expect(result.current.draft.vendorNotes).toBe("Call before arrival"));
act(() => result.current.saveCompanyNotes(result.current.draft));
await act(async () => {
await result.current.saveCompanyNotes(result.current.draft);
});
expect(addTechnicians).not.toHaveBeenCalled();
});

View file

@ -15,6 +15,7 @@ describe("WizardStepLocationServiceSelect — vendor clear on service change", (
vendorId: "9",
vendorName: "Old Vendor",
techPhone: "555-0100",
vendorNotes: "Old company notes",
};
render(
@ -43,6 +44,7 @@ describe("WizardStepLocationServiceSelect — vendor clear on service change", (
vendorId: "",
vendorName: "",
techPhone: "",
vendorNotes: "",
}),
);
});
@ -56,6 +58,7 @@ describe("WizardStepLocationServiceSelect — vendor clear on service change", (
vendorId: "9",
vendorName: "Old Vendor",
techPhone: "555-0100",
vendorNotes: "Old company notes",
};
render(
@ -83,6 +86,7 @@ describe("WizardStepLocationServiceSelect — vendor clear on service change", (
vendorId: "",
vendorName: "",
techPhone: "",
vendorNotes: "",
});
});
});

View file

@ -3,7 +3,7 @@ import { buildCompanyNotesUpdate } from "@/domain/vendors/utils/build-company-no
const baseline = { companyId: 12, rowVersion: "AAAAAAAAB9E=", notes: "Call before arrival" };
describe("buildCompanyNotesUpdate (SH-321)", () => {
describe("buildCompanyNotesUpdate", () => {
it("builds a notes-only additive patch when the notes changed", () => {
expect(buildCompanyNotesUpdate(baseline, " Gate code 4411 ")).toEqual({
companyId: 12,