From df00e56d6efe5ba829873c8cc3d1b823be54dae3 Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Tue, 18 Aug 2026 13:12:10 -0300 Subject: [PATCH] fix(vendors): gate trades to canonical list, preserve legacy values (SH-249) --- .../vendor-trade-specialties-field.tsx | 103 +++++++---- .../vendor-trade-specialties-field.test.tsx | 168 ++++++++++++++++++ 2 files changed, 239 insertions(+), 32 deletions(-) create mode 100644 src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx diff --git a/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx b/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx index ff63ef4c..90329e41 100644 --- a/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx +++ b/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx @@ -1,19 +1,11 @@ -import { useState } from "react"; +import { useRef, useState } from "react"; import { Controller, type Control } from "react-hook-form"; import AddIcon from "@mui/icons-material/Add"; import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; import ArrowUpwardIcon from "@mui/icons-material/ArrowUpward"; import CloseIcon from "@mui/icons-material/Close"; -import { - Autocomplete, - Box, - Button, - Chip, - FormLabel, - Stack, - TextField, - Typography, -} from "@mui/material"; +import { Autocomplete, Box, Button, Chip, FormLabel, Stack, TextField } from "@mui/material"; +import { Text } from "@/components/ui/text"; import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; function splitTrades(value: string | undefined): string[] { @@ -23,6 +15,25 @@ function splitTrades(value: string | undefined): string[] { .filter(Boolean); } +function dedupeTrades(options: string[], current: string[]): string[] { + const seen = new Set(); + const merged: string[] = []; + for (const trade of [...options, ...current]) { + const key = trade.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + merged.push(trade); + } + return merged; +} + +function matchTradeOption(input: string, options: string[]): string | null { + const exact = options.find((option) => option === input); + if (exact) return exact; + const normalized = input.toLowerCase(); + return options.find((option) => option.toLowerCase() === normalized) ?? null; +} + interface VendorTradeSpecialtiesFieldProps { control: Control; index: number; @@ -35,22 +46,44 @@ export function VendorTradeSpecialtiesField({ tradeOptions, }: VendorTradeSpecialtiesFieldProps) { const [tradeInput, setTradeInput] = useState(""); + const [feedbackMessage, setFeedbackMessage] = useState(null); + const knownTradesRef = useRef>(new Set()); return ( Trade Specialties - + First trade is primary. Reorder with the arrows. - + { const trades = splitTrades(field.value); + for (const trade of trades) knownTradesRef.current.add(trade); + const selectableTrades = dedupeTrades(tradeOptions, [...knownTradesRef.current]); const commit = (next: string[]) => field.onChange(next.join(", ")); const add = (value: string) => { const normalized = value.trim(); - if (normalized && !trades.includes(normalized)) commit([...trades, normalized]); + setFeedbackMessage(null); + if (!normalized) { + setTradeInput(""); + return; + } + const canonical = matchTradeOption(normalized, selectableTrades); + if (tradeOptions.length > 0 && !canonical) { + setFeedbackMessage( + `"${normalized}" is not in the trades list. Pick a trade from the list.`, + ); + return; + } + const trade = canonical ?? normalized; + if (trades.some((existing) => existing.toLowerCase() === trade.toLowerCase())) { + setFeedbackMessage(`${trade} is already on this technician.`); + setTradeInput(""); + return; + } + commit([...trades, trade]); setTradeInput(""); }; const move = (tradeIndex: number, direction: -1 | 1) => { @@ -64,31 +97,31 @@ export function VendorTradeSpecialtiesField({ return ( - {trades.length === 0 ? ( - - No trades selected. - - ) : ( - trades.map((trade, tradeIndex) => ( - - commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex)) - } - deleteIcon={} - /> - )) - )} + + No trades selected. + + {trades.map((trade, tradeIndex) => ( + + commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex)) + } + deleteIcon={} + /> + ))} { - if (reason === "input") setTradeInput(value); + if (reason === "input") { + setTradeInput(value); + setFeedbackMessage(null); + } }} onChange={(_event, value) => { if (typeof value === "string") add(value); @@ -117,6 +150,12 @@ export function VendorTradeSpecialtiesField({ + + Trade list is unavailable right now — you can still type a trade manually. + + + {feedbackMessage} + {trades.length > 1 && ( {trades.map((trade, tradeIndex) => ( diff --git a/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx b/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx new file mode 100644 index 00000000..bff2a587 --- /dev/null +++ b/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx @@ -0,0 +1,168 @@ +import { fireEvent, screen } from "@testing-library/react"; +import { useEffect } from "react"; +import { useForm, useWatch, type Control } from "react-hook-form"; +import { describe, expect, it } from "vitest"; +import { VendorTradeSpecialtiesField } from "@/app/(protected)/vendors/_components/vendor-trade-specialties-field"; +import { + emptyRosterTechnician, + emptyVendorCompanyRosterForm, + type VendorCompanyRosterFormValues, +} from "@/domain/vendors/schemas/vendor-roster-schema"; +import { renderWithProviders } from "@/test/test-utils"; + +function TradeSpecialtiesSpy({ + control, + valuesRef, +}: { + control: Control; + valuesRef: { current: string }; +}) { + const value = useWatch({ control, name: "technicians.0.tradeSpecialties" }) ?? ""; + useEffect(() => { + valuesRef.current = value; + }, [value, valuesRef]); + return null; +} + +function TradeSpecialtiesHarness({ + tradeOptions, + initialTradeSpecialties = "", + valuesRef, +}: { + tradeOptions: string[]; + initialTradeSpecialties?: string; + valuesRef: { current: string }; +}) { + const { control } = useForm({ + defaultValues: { + ...emptyVendorCompanyRosterForm, + technicians: [{ ...emptyRosterTechnician, tradeSpecialties: initialTradeSpecialties }], + }, + }); + return ( + <> + + + + ); +} + +function getAddTradeInput(): HTMLElement { + return screen.getByRole("combobox", { name: "Add Trade" }); +} + +function typeAndCommitTrade(input: HTMLElement, value: string, commit: "enter" | "button") { + fireEvent.change(input, { target: { value } }); + if (commit === "enter") { + fireEvent.keyDown(input, { key: "Enter" }); + } else { + fireEvent.click(screen.getByRole("button", { name: "Add trade" })); + } +} + +describe("VendorTradeSpecialtiesField", () => { + it("offers facet trades as selectable options when the facets query is populated", async () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "H" } }); + + fireEvent.click(await screen.findByRole("option", { name: "HVAC" })); + + expect(await screen.findByText("HVAC (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC"); + }); + + it("has no selectable options when the trades facet is empty, warns, and still accepts typed trades", () => { + const valuesRef = { current: "" }; + renderWithProviders(); + + expect(screen.getByText(/trade list is unavailable right now/i)).toBeInTheDocument(); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "P" } }); + expect(screen.queryByRole("listbox")).toBeNull(); + expect(screen.queryAllByRole("option")).toHaveLength(0); + + typeAndCommitTrade(input, "Plumbing", "enter"); + + expect(screen.getByText("Plumbing (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("Plumbing"); + }); + + it("rejects trades outside the canonical list with visible feedback and keeps the draft", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "Sprinkler Fitting", "button"); + + expect(screen.getByText(/is not in the trades list/i)).toBeInTheDocument(); + expect(screen.queryByText("Sprinkler Fitting (primary)")).toBeNull(); + expect(valuesRef.current).toBe(""); + expect(input).toHaveValue("Sprinkler Fitting"); + }); + + it("commits the canonical casing when a typed trade matches case-insensitively", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "hvac", "enter"); + + expect(screen.getByText("HVAC (primary)")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC"); + }); + + it("keeps legacy free-text trades visible, removable, and re-selectable", async () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + expect(screen.getByText("Backflow Testing (primary)")).toBeInTheDocument(); + expect(screen.getByText("HVAC")).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("remove-trade-Backflow Testing")); + expect(screen.queryByText(/Backflow Testing/)).toBeNull(); + expect(valuesRef.current).toBe("HVAC"); + + const input = getAddTradeInput(); + fireEvent.change(input, { target: { value: "Back" } }); + fireEvent.click(await screen.findByRole("option", { name: "Backflow Testing" })); + + expect(await screen.findByText("Backflow Testing")).toBeInTheDocument(); + expect(valuesRef.current).toBe("HVAC, Backflow Testing"); + }); + + it("flags duplicate trades instead of silently doing nothing", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + const input = getAddTradeInput(); + typeAndCommitTrade(input, "plumbing", "enter"); + + expect(screen.getByText(/is already on this technician/i)).toBeInTheDocument(); + expect(valuesRef.current).toBe("Plumbing"); + }); +});