From 4c9f8b7d81ca26fb9fa8e4d9f9a06aa2fa492f32 Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Thu, 20 Aug 2026 14:17:20 -0300 Subject: [PATCH] fix(work-orders): treat vendor save as a live company assignment Clear leftover primary dispatch status on vendor patch so Completed is not blocked after a new company is chosen. Keep Jira keys out of source comments. --- REVIEW_AND_PR_FRAMEWORK.md | 4 ++ .../tabs/slide-over-info-tab-edit-view.tsx | 10 +--- .../list/table/wo-table-inline-row-cells.tsx | 10 +--- .../list/table/wo-table-row-service-cells.tsx | 10 +--- .../types/work-order-board-detail.ts | 1 - .../work-orders/types/work-order-board.ts | 1 - .../work-orders/types/work-order-table-row.ts | 1 - .../enrich-detail-closability-from-board.ts | 2 +- .../utils/vendor-assignment-patch.ts | 18 ++++++ .../work-orders/utils/wo-closability.ts | 7 +-- .../status-cell-pending-uplift.test.tsx | 57 +++++++++++++++++-- .../work-order-table-row-mapper.test.ts | 22 +++++-- .../utils/vendor-assignment-patch.test.ts | 37 ++++++++++++ .../work-orders/utils/wo-closability.test.ts | 37 ++++++++---- 14 files changed, 164 insertions(+), 53 deletions(-) create mode 100644 src/domain/work-orders/utils/vendor-assignment-patch.ts create mode 100644 src/test/domain/work-orders/utils/vendor-assignment-patch.test.ts diff --git a/REVIEW_AND_PR_FRAMEWORK.md b/REVIEW_AND_PR_FRAMEWORK.md index 977582f4..3496a066 100644 --- a/REVIEW_AND_PR_FRAMEWORK.md +++ b/REVIEW_AND_PR_FRAMEWORK.md @@ -63,6 +63,10 @@ a status or mark unverified work Done. - **MUST NOT** leave comments that only restate what Prettier or ESLint already enforces (formatting, naming nits the linter catches). Style is settled by the gates; review is for behavior, correctness, security, and architecture. +- **MUST NOT** put Jira issue keys or ticket titles in source comments, JSDoc, + or test names (for example `(SH-183)`). Ticket identity belongs in the PR, + commit message, and branch — not in the code. Flag and request removal if a + diff adds them. - **MUST** make every comment actionable: tie it to a behavior, a risk, or an evidence-based convention in these docs, and offer a concrete fix or a targeted question. Use GitHub suggestion blocks when safe. diff --git a/src/app/(protected)/workorders/_components/detail/tabs/slide-over-info-tab-edit-view.tsx b/src/app/(protected)/workorders/_components/detail/tabs/slide-over-info-tab-edit-view.tsx index bdfe8af9..8b92c86f 100644 --- a/src/app/(protected)/workorders/_components/detail/tabs/slide-over-info-tab-edit-view.tsx +++ b/src/app/(protected)/workorders/_components/detail/tabs/slide-over-info-tab-edit-view.tsx @@ -15,6 +15,7 @@ import type { } from "@/domain/work-orders/types/work-order"; import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row"; import type { WizardWOStatus } from "@/domain/work-orders/types/work-order-wizard"; +import { toVendorTablePatch } from "@/domain/work-orders/utils/vendor-assignment-patch"; import { DocBadge } from "./slide-over-doc-badge"; type SlideOverInfoTabEditViewProps = { @@ -140,14 +141,7 @@ export function SlideOverInfoTabEditView({ tech={draft.tech} techPhone={draft.techPhone} vendors={vendors} - onSave={(patch) => - onDraftChange({ - vendorId: patch.vendorId, - company: patch.company, - tech: patch.tech, - techPhone: patch.techPhone, - }) - } + onSave={(patch) => onDraftChange(toVendorTablePatch(patch))} /> diff --git a/src/app/(protected)/workorders/_components/list/table/wo-table-inline-row-cells.tsx b/src/app/(protected)/workorders/_components/list/table/wo-table-inline-row-cells.tsx index f50d464d..02ee2880 100644 --- a/src/app/(protected)/workorders/_components/list/table/wo-table-inline-row-cells.tsx +++ b/src/app/(protected)/workorders/_components/list/table/wo-table-inline-row-cells.tsx @@ -16,6 +16,7 @@ import { PMTypeCell } from "./cells/pm-type-cell"; import { StatusCell } from "./cells/status-cell"; import { TypeCell } from "./cells/type-cell"; import { VendorCell } from "./cells/vendor-cell"; +import { toVendorTablePatch } from "@/domain/work-orders/utils/vendor-assignment-patch"; import { toInlineDraftRow } from "./to-inline-draft-row"; import { WoTableInlineIdentityCells } from "./wo-table-inline-identity-cells"; @@ -193,14 +194,7 @@ export function WoTableInlineRowCells({ tech={draft.tech} techPhone={draft.techPhone} vendors={vendors} - onSave={(p) => - onPatch({ - vendorId: p.vendorId, - company: p.company, - tech: p.tech, - techPhone: p.techPhone, - }) - } + onSave={(p) => onPatch(toVendorTablePatch(p))} /> diff --git a/src/app/(protected)/workorders/_components/list/table/wo-table-row-service-cells.tsx b/src/app/(protected)/workorders/_components/list/table/wo-table-row-service-cells.tsx index 7c7c2d50..26773141 100644 --- a/src/app/(protected)/workorders/_components/list/table/wo-table-row-service-cells.tsx +++ b/src/app/(protected)/workorders/_components/list/table/wo-table-row-service-cells.tsx @@ -10,6 +10,7 @@ import { UpliftCell } from "./cells/uplift-cell"; import { VendorCell } from "./cells/vendor-cell"; import { EMPTY_UPLIFT_SUMMARY } from "@/domain/work-orders/types/work-order-uplift"; import { canOpenUpliftsDialog } from "@/domain/work-orders/utils/uplift-display-utils"; +import { toVendorTablePatch } from "@/domain/work-orders/utils/vendor-assignment-patch"; import type { WoTableRowHandlers } from "./wo-table-row"; type WoTableRowServiceCellsProps = { @@ -67,14 +68,7 @@ export function WoTableRowServiceCells({ techPhone={row.techPhone} vendors={vendors} q={search} - onSave={(p) => - onPatchRow({ - vendorId: p.vendorId, - company: p.company, - tech: p.tech, - techPhone: p.techPhone, - }) - } + onSave={(p) => onPatchRow(toVendorTablePatch(p))} /> { expect(onChangeStatus).toHaveBeenCalledWith("Completed"); }); - it("disables Completed when company is missing (SH-183)", () => { + it("disables Completed when company is missing", () => { const onChangeStatus = vi.fn(); renderWithProviders( @@ -141,7 +141,7 @@ describe("StatusCell pending uplift closability", () => { expect(onChangeStatus).not.toHaveBeenCalled(); }); - it("allows Completed when vendorId is set even if company label is empty (SH-183)", () => { + it("allows Completed when vendorId is set even if company label is empty", () => { const onChangeStatus = vi.fn(); renderWithProviders( @@ -159,7 +159,7 @@ describe("StatusCell pending uplift closability", () => { expect(onChangeStatus).toHaveBeenCalledWith("Completed"); }); - it("allows Completed when vendorId has an empty company label and a live dispatch status (SH-183)", () => { + it("allows Completed when vendorId has an empty company label and a live dispatch status", () => { const onChangeStatus = vi.fn(); renderWithProviders( @@ -178,7 +178,7 @@ describe("StatusCell pending uplift closability", () => { }); it.each(["Cancelled", "Canceled", "Refused"] as const)( - "disables Completed when vendorId is leftover from a %s dispatch (SH-183)", + "disables Completed when vendorId is leftover from a %s dispatch", (primaryDispatchStatus) => { const onChangeStatus = vi.fn(); @@ -206,7 +206,54 @@ describe("StatusCell pending uplift closability", () => { }, ); - it("allows Completed when technician is empty if company is set (SH-183)", () => { + it("disables Completed when the board omits vendorId after a Refused primary", () => { + const onChangeStatus = vi.fn(); + + renderWithProviders( + , + { withAuth: false }, + ); + + fireEvent.click(screen.getByRole("button", { name: /in progress/i })); + + const completed = screen.getByRole("button", { name: /completed/i }); + expect(completed).toBeDisabled(); + expect(completed).toHaveAttribute("title", "Missing: Company"); + }); + + it("allows Completed after a vendor patch clears leftover refused status", () => { + const onChangeStatus = vi.fn(); + + renderWithProviders( + , + { withAuth: false }, + ); + + fireEvent.click(screen.getByRole("button", { name: /in progress/i })); + fireEvent.click(screen.getByRole("button", { name: /completed/i })); + + expect(onChangeStatus).toHaveBeenCalledWith("Completed"); + }); + + it("allows Completed when technician is empty if company is set", () => { const onChangeStatus = vi.fn(); renderWithProviders( diff --git a/src/test/domain/work-orders/mappers/work-order-table-row-mapper.test.ts b/src/test/domain/work-orders/mappers/work-order-table-row-mapper.test.ts index 99283be9..970eeff7 100644 --- a/src/test/domain/work-orders/mappers/work-order-table-row-mapper.test.ts +++ b/src/test/domain/work-orders/mappers/work-order-table-row-mapper.test.ts @@ -61,13 +61,27 @@ describe("mapWorkOrderTableRow primaryDispatchStatus", () => { expect(row.primaryDispatchStatus).toBe(value); }); - it("ignores undocumented dispatchStatus aliases", () => { + it("maps backend #74 omitted vendor with Refused primaryDispatchStatus", () => { + const row = mapWorkOrderTableRow({ + id: 9, + vendorId: null, + vendorName: null, + primaryDispatchStatus: "Refused", + }); + expect(row.vendorId).toBe(""); + expect(row.company).toBe(""); + expect(row.primaryDispatchStatus).toBe("Refused"); + }); + + it("maps live vendorId with empty company label and Sent status", () => { const row = mapWorkOrderTableRow({ id: 9, vendorId: 45, - dispatchStatus: "Refused", - vendorDispatchStatus: "Cancelled", + vendorName: "", + primaryDispatchStatus: "Sent", }); - expect(row.primaryDispatchStatus).toBe(""); + expect(row.vendorId).toBe("45"); + expect(row.company).toBe(""); + expect(row.primaryDispatchStatus).toBe("Sent"); }); }); diff --git a/src/test/domain/work-orders/utils/vendor-assignment-patch.test.ts b/src/test/domain/work-orders/utils/vendor-assignment-patch.test.ts new file mode 100644 index 00000000..72aaa7d2 --- /dev/null +++ b/src/test/domain/work-orders/utils/vendor-assignment-patch.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; + +import { toVendorTablePatch } from "@/domain/work-orders/utils/vendor-assignment-patch"; +import { + getClosabilityGaps, + tableRowToClosabilityInput, +} from "@/domain/work-orders/utils/wo-closability"; +import type { WorkOrderTableRow } from "@/domain/work-orders/types/work-order-table-row"; + +describe("toVendorTablePatch", () => { + it("clears leftover primaryDispatchStatus so a new vendor is a live assignment", () => { + const patch = toVendorTablePatch({ + vendorId: "9", + company: "New Co", + tech: "Pat", + techPhone: "555", + }); + expect(patch.primaryDispatchStatus).toBe(""); + expect(patch.vendorId).toBe("9"); + + const input = tableRowToClosabilityInput({ + dispatcherId: "u1", + dispatcherName: "Alex", + company: patch.company ?? "", + vendorId: patch.vendorId ?? "", + tech: patch.tech ?? "", + completedDate: "2026-07-15", + woNumber: "20260623001", + pm: "HVAC PM", + docStatus: "Yes", + mediaCount: 1, + type: "PM", + primaryDispatchStatus: patch.primaryDispatchStatus, + } as WorkOrderTableRow); + expect(getClosabilityGaps(input)).not.toContain("Company"); + }); +}); diff --git a/src/test/domain/work-orders/utils/wo-closability.test.ts b/src/test/domain/work-orders/utils/wo-closability.test.ts index c463dd5f..240ed796 100644 --- a/src/test/domain/work-orders/utils/wo-closability.test.ts +++ b/src/test/domain/work-orders/utils/wo-closability.test.ts @@ -77,7 +77,7 @@ describe("getClosabilityGaps", () => { expect(getClosabilityGaps(makeInput())).toEqual([]); }); - it("allows company without technician (SH-183)", () => { + it("allows company without technician", () => { const gaps = getClosabilityGaps(makeInput({ tech: "" })); expect(gaps).toEqual([]); expect(gaps).not.toContain("Technician"); @@ -88,7 +88,7 @@ describe("getClosabilityGaps", () => { expect(getClosabilityGaps(makeInput({ company: " ", tech: "Sam" }))).toEqual(["Company"]); }); - it("allows Completed when vendorId is set and company/technician are empty (SH-183)", () => { + it("allows Completed when vendorId is set and company/technician are empty", () => { expect(getClosabilityGaps(makeInput({ company: "", vendorId: "7", tech: "" }))).toEqual([]); }); @@ -99,7 +99,7 @@ describe("getClosabilityGaps", () => { }); it.each(["PM", "Reactive", "Emergency", "Overdue", ""] as const)( - "blocks Completed without a company for type %s (SH-183)", + "blocks Completed without a company for type %s", (type) => { const gaps = getClosabilityGaps( makeInput({ @@ -309,7 +309,7 @@ describe("detailToClosabilityInput", () => { expect(getClosabilityGaps(detailToClosabilityInput(detail))).toEqual(["At least 1 photo"]); }); - it("treats vendorId without vendorName as assigned company (SH-183)", () => { + it("treats vendorId without vendorName as assigned company", () => { const detail = { assignedTo: "Alice", completedDate: "2026-07-15", @@ -330,7 +330,7 @@ describe("detailToClosabilityInput", () => { }); it.each(["Cancelled", "Canceled", "Refused", "CANCELLED", " canceled "])( - "ignores inactive dispatch status %s when resolving assigned company (SH-183)", + "ignores inactive dispatch status %s when resolving assigned company", (status) => { const inactiveOnly = { assignedTo: "Alice", @@ -352,7 +352,7 @@ describe("detailToClosabilityInput", () => { ); it.each(["Verified", "Completed", "Sent"])( - "treats dispatch status %s as a live company assignment (SH-183)", + "treats dispatch status %s as a live company assignment", (status) => { const liveAssignment = { assignedTo: "Alice", @@ -374,7 +374,7 @@ describe("detailToClosabilityInput", () => { }, ); - it("uses the active dispatch after an inactive cancelled spelling (SH-183)", () => { + it("uses the active dispatch after an inactive cancelled spelling", () => { const supersededThenCurrent = { assignedTo: "Alice", completedDate: "2026-07-15", @@ -413,14 +413,14 @@ describe("tableRowToClosabilityInput", () => { type: "", } as WorkOrderTableRow; - it("keeps vendorId as a live assignment when primary dispatch status is absent (SH-183)", () => { + it("keeps vendorId as a live assignment when primary dispatch status is absent", () => { const input = tableRowToClosabilityInput(closableRow); expect(input.vendorId).toBe("45"); expect(getClosabilityGaps(input)).toEqual([]); }); it.each(["Cancelled", "Canceled", "Refused"] as const)( - "clears company assignment when primary dispatch status is %s (SH-183)", + "clears company assignment when primary dispatch status is %s", (primaryDispatchStatus) => { const input = tableRowToClosabilityInput({ ...closableRow, primaryDispatchStatus }); expect(input.company).toBe(""); @@ -429,11 +429,26 @@ describe("tableRowToClosabilityInput", () => { }, ); - it("raises Company when the board omits vendor assignment (inactive primary, SH-183)", () => { - const input = tableRowToClosabilityInput({ ...closableRow, vendorId: "", company: "" }); + it("raises Company when the board omits vendor assignment (inactive primary)", () => { + const input = tableRowToClosabilityInput({ + ...closableRow, + vendorId: "", + company: "", + primaryDispatchStatus: "Refused", + }); expect(input.vendorId).toBe(""); expect(getClosabilityGaps(input)).toContain("Company"); }); + + it("keeps a live vendorId after a vendor patch clears leftover refused status", () => { + const input = tableRowToClosabilityInput({ + ...closableRow, + company: "New Co", + vendorId: "9", + primaryDispatchStatus: "", + }); + expect(getClosabilityGaps(input)).toEqual([]); + }); }); describe("readDetailWoType", () => {