From 7ec522e8eae265b1349961602b3db26bdf76c51d Mon Sep 17 00:00:00 2001 From: Alexandre Brandizzi Date: Thu, 27 Aug 2026 15:03:36 -0300 Subject: [PATCH] fix: complete SH-283 vendor QA paths (#148) Co-authored-by: Codex Review Integration --- e2e/vendors/vendors.spec.ts | 22 +++++++++++++++++++ .../vendor-roster-conflict-alert.tsx | 9 ++++++++ .../_components/vendor-roster-form-fields.tsx | 6 ++--- .../vendors/mappers/vendor-roster-mapper.ts | 7 +++++- src/domain/vendors/types/vendor.ts | 2 +- .../vendors/vendor-create-modal.test.tsx | 10 +++++++-- .../vendor-roster-conflict-alert.test.tsx | 22 +++++++++++++++++++ .../api/vendor-company-roster-api.test.ts | 19 ++++++++++++++++ .../mappers/vendor-roster-mapper.test.ts | 14 ++++++++++++ 9 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 src/test/app/(protected)/vendors/vendor-roster-conflict-alert.test.tsx diff --git a/e2e/vendors/vendors.spec.ts b/e2e/vendors/vendors.spec.ts index 0a4dca8d..988e0a7f 100644 --- a/e2e/vendors/vendors.spec.ts +++ b/e2e/vendors/vendors.spec.ts @@ -475,6 +475,28 @@ test.describe("Vendor directory prototype parity", () => { await page.getByRole("combobox", { name: "Company (required)" }).click(); await page.getByRole("option", { name: "Gateway Plumbing" }).click(); + const floatedCompanyLabel = page + .getByRole("dialog", { name: /Add Vendor/ }) + .locator("label") + .filter({ hasText: "Company (required)" }); + const floatedCompanyField = floatedCompanyLabel.locator(".."); + await expect(floatedCompanyLabel).toHaveText("Company (required)"); + await expect(floatedCompanyField.locator("legend")).toHaveText("Company (required)"); + const floatedMetrics = await floatedCompanyLabel.evaluate((element) => { + const labelRect = element.getBoundingClientRect(); + const legendRect = element + .closest(".MuiFormControl-root") + ?.querySelector("legend") + ?.getBoundingClientRect(); + return { + labelClientWidth: element.clientWidth, + labelScrollWidth: element.scrollWidth, + legendWidth: legendRect?.width ?? 0, + labelWidth: labelRect.width, + }; + }); + expect(floatedMetrics.labelClientWidth).toBeGreaterThanOrEqual(floatedMetrics.labelScrollWidth); + expect(floatedMetrics.legendWidth).toBeGreaterThanOrEqual(floatedMetrics.labelWidth); await expect(page.getByLabel("Company Phone (optional)")).toHaveValue("314-555-0100"); await expect(page.getByRole("textbox", { name: "Email (optional)", exact: true })).toHaveValue( "dispatch@gateway.test", diff --git a/src/app/(protected)/vendors/_components/vendor-roster-conflict-alert.tsx b/src/app/(protected)/vendors/_components/vendor-roster-conflict-alert.tsx index 5a1ff44e..30878dc3 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-conflict-alert.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-conflict-alert.tsx @@ -8,6 +8,15 @@ interface VendorRosterConflictAlertProps { } export function VendorRosterConflictAlert({ conflict, onReload }: VendorRosterConflictAlertProps) { + if (conflict.kind === "duplicate") { + return ( + + Company name already exists + {conflict.message} + + ); + } + if (conflict.kind === "stale") { return ( )} diff --git a/src/domain/vendors/mappers/vendor-roster-mapper.ts b/src/domain/vendors/mappers/vendor-roster-mapper.ts index 641042ef..d3fb7ceb 100644 --- a/src/domain/vendors/mappers/vendor-roster-mapper.ts +++ b/src/domain/vendors/mappers/vendor-roster-mapper.ts @@ -179,8 +179,13 @@ export function mapRosterConflict(raw: unknown, fallbackMessage: string): Vendor const item = asRecord(raw); const blockedRaw = item.blockedWorkOrders ?? item.BlockedWorkOrders ?? item.openWorkOrders; const blockedWorkOrders = Array.isArray(blockedRaw) ? blockedRaw.map(mapBlockedWorkOrder) : []; + const code = readString(item, "code", "Code"); const kind: VendorRosterConflictKind = - blockedWorkOrders.length > 0 ? "open-work-orders" : "stale"; + code === "duplicate_vendor_company_name" + ? "duplicate" + : blockedWorkOrders.length > 0 + ? "open-work-orders" + : "stale"; const message = readString(item, "message", "Message") || fallbackMessage; return { kind, message, blockedWorkOrders }; } diff --git a/src/domain/vendors/types/vendor.ts b/src/domain/vendors/types/vendor.ts index 74e6519e..b6ee694d 100644 --- a/src/domain/vendors/types/vendor.ts +++ b/src/domain/vendors/types/vendor.ts @@ -145,7 +145,7 @@ export interface VendorRosterBlockedWorkOrder { scheduledDate?: string; } -export type VendorRosterConflictKind = "open-work-orders" | "stale"; +export type VendorRosterConflictKind = "duplicate" | "open-work-orders" | "stale"; export interface VendorRosterConflict { kind: VendorRosterConflictKind; diff --git a/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx index c543abb7..214fecea 100644 --- a/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx +++ b/src/test/app/(protected)/vendors/vendor-create-modal.test.tsx @@ -67,9 +67,12 @@ describe("VendorCreateModal validation", () => { const company = screen.getByRole("combobox", { name: "Company (required)" }); expect(company).toBeRequired(); - expect(screen.getByText("Company (required)")).toBeInTheDocument(); + expect(screen.getAllByText("Company (required)")).toHaveLength(2); expect(screen.queryByText("*")).not.toBeInTheDocument(); expect(document.querySelector(".MuiFormLabel-asterisk")).toBeNull(); + expect(company.closest(".MuiFormControl-root")?.querySelector("legend")).toHaveTextContent( + /^Company \(required\)$/, + ); }); it("labels the plain company field exactly, required, with no generated asterisk", () => { @@ -77,9 +80,12 @@ describe("VendorCreateModal validation", () => { const company = screen.getByRole("textbox", { name: "Company (required)" }); expect(company).toBeRequired(); - expect(screen.getByText("Company (required)")).toBeInTheDocument(); + expect(screen.getAllByText("Company (required)")).toHaveLength(2); expect(screen.queryByText("*")).not.toBeInTheDocument(); expect(document.querySelector(".MuiFormLabel-asterisk")).toBeNull(); + expect(company.closest(".MuiFormControl-root")?.querySelector("legend")).toHaveTextContent( + /^Company \(required\)$/, + ); }); it("shows the phone-or-email error once without marking either input individually invalid", async () => { diff --git a/src/test/app/(protected)/vendors/vendor-roster-conflict-alert.test.tsx b/src/test/app/(protected)/vendors/vendor-roster-conflict-alert.test.tsx new file mode 100644 index 00000000..25f61538 --- /dev/null +++ b/src/test/app/(protected)/vendors/vendor-roster-conflict-alert.test.tsx @@ -0,0 +1,22 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import { VendorRosterConflictAlert } from "@/app/(protected)/vendors/_components/vendor-roster-conflict-alert"; + +describe("VendorRosterConflictAlert", () => { + it("explains a duplicate company without offering a stale-data reload", () => { + render( + , + ); + + expect(screen.getByText("Company name already exists")).toBeInTheDocument(); + expect(screen.getByText("Another vendor company already uses that name.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Reload" })).not.toBeInTheDocument(); + }); +}); 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 32fb5aeb..f8242703 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 @@ -118,6 +118,25 @@ describe("vendorCompanyRosterApi", () => { ); }); + it("throws a duplicate conflict for the stable create response", async () => { + apiPost.mockRejectedValueOnce( + httpError(409, { + code: "duplicate_vendor_company_name", + message: "Another vendor company already uses that name.", + }), + ); + + await expect(vendorCompanyRosterApi.create({ name: "Solo Co" })).rejects.toSatisfy( + (error: unknown) => { + if (!isVendorRosterConflictError(error)) return false; + return ( + error.conflict.kind === "duplicate" && + error.conflict.message === "Another vendor company already uses that name." + ); + }, + ); + }); + it("rethrows non-conflict errors untouched", async () => { const generic = new Error("boom"); apiPost.mockRejectedValueOnce(generic); diff --git a/src/test/domain/vendors/mappers/vendor-roster-mapper.test.ts b/src/test/domain/vendors/mappers/vendor-roster-mapper.test.ts index 66e8d28b..65494d15 100644 --- a/src/test/domain/vendors/mappers/vendor-roster-mapper.test.ts +++ b/src/test/domain/vendors/mappers/vendor-roster-mapper.test.ts @@ -113,6 +113,20 @@ describe("vendor roster mapper", () => { expect(conflict.message).toBe("stale rowversion"); }); + it("classifies the stable duplicate-company response", () => { + const conflict = mapRosterConflict( + { + code: "duplicate_vendor_company_name", + message: "Another vendor company already uses that name.", + }, + "fallback", + ); + + expect(conflict.kind).toBe("duplicate"); + expect(conflict.message).toBe("Another vendor company already uses that name."); + expect(conflict.blockedWorkOrders).toEqual([]); + }); + it("preserves an explicit Phone/Email/Text preferred contact on read", () => { expect( mapRosterTechnician({ contactName: "A", preferredContact: "Phone" }).preferredContact,