Merge pull request #117 from Sea-Haven-Industries/feat/sh-249-fe-trades
Some checks are pending
CI / ci (push) Waiting to run
CI / governance (push) Waiting to run
CI / vendor-visual-regression (push) Waiting to run
Deploy / deploy (push) Waiting to run

fix(vendors): gate trades to canonical list, preserve legacy values (SH-249)
This commit is contained in:
Alexandre Brandizzi 2026-08-20 13:38:23 -03:00 • committed by GitHub
commit 58bdd6bffa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 335 additions and 23 deletions

View file

@ -93,6 +93,7 @@ export interface VendorRosterForm {
roster: VendorCompanyRoster | undefined; roster: VendorCompanyRoster | undefined;
companies: VendorFacetCompany[]; companies: VendorFacetCompany[];
trades: string[]; trades: string[];
tradesLoading: boolean;
selectedCompanyId: string | number | null; selectedCompanyId: string | number | null;
selectCompany: (company: VendorFacetCompany | null) => Promise<void>; selectCompany: (company: VendorFacetCompany | null) => Promise<void>;
clearSelectedCompany: (nextName?: string) => void; clearSelectedCompany: (nextName?: string) => void;
@ -110,7 +111,7 @@ export function useVendorRosterForm({
mode === "update" ? vendorId : undefined, mode === "update" ? vendorId : undefined,
mode === "update" ? companyId : undefined, mode === "update" ? companyId : undefined,
); );
const { data: facets } = useVendorFacets(); const { data: facets, isLoading: facetsLoading } = useVendorFacets();
const save = useSaveVendorCompanyRoster(); const save = useSaveVendorCompanyRoster();
const [conflict, setConflict] = useState<VendorRosterConflict | null>(null); const [conflict, setConflict] = useState<VendorRosterConflict | null>(null);
@ -223,6 +224,7 @@ export function useVendorRosterForm({
roster: routeRoster, roster: routeRoster,
companies, companies,
trades, trades,
tradesLoading: facetsLoading,
selectedCompanyId: selection.selectedCompanyId, selectedCompanyId: selection.selectedCompanyId,
selectCompany: selection.selectCompany, selectCompany: selection.selectCompany,
clearSelectedCompany: selection.clearSelectedCompany, clearSelectedCompany: selection.clearSelectedCompany,

View file

@ -68,6 +68,7 @@ export function VendorCreateModal({ open, onClose }: VendorCreateModalProps) {
errors={form.errors} errors={form.errors}
companies={form.companies} companies={form.companies}
tradeOptions={form.trades} tradeOptions={form.trades}
tradeOptionsLoading={form.tradesLoading}
selectedCompanyId={form.selectedCompanyId} selectedCompanyId={form.selectedCompanyId}
onSelectCompany={form.selectCompany} onSelectCompany={form.selectCompany}
onClearSelectedCompany={form.clearSelectedCompany} onClearSelectedCompany={form.clearSelectedCompany}

View file

@ -299,6 +299,7 @@ function DrawerEditor({
control={form.control} control={form.control}
errors={form.errors} errors={form.errors}
tradeOptions={form.trades} tradeOptions={form.trades}
tradeOptionsLoading={form.tradesLoading}
showTechnicianStatus={false} showTechnicianStatus={false}
/> />
{selectedIndex >= 0 && ( {selectedIndex >= 0 && (

View file

@ -25,6 +25,7 @@ interface VendorRosterFormFieldsProps {
errors: FieldErrors<VendorCompanyRosterFormValues>; errors: FieldErrors<VendorCompanyRosterFormValues>;
companies?: VendorFacetCompany[]; companies?: VendorFacetCompany[];
tradeOptions?: string[]; tradeOptions?: string[];
tradeOptionsLoading?: boolean;
selectedCompanyId?: string | number | null; selectedCompanyId?: string | number | null;
onSelectCompany?: (company: VendorFacetCompany | null) => Promise<void>; onSelectCompany?: (company: VendorFacetCompany | null) => Promise<void>;
onClearSelectedCompany?: (nextName?: string) => void; onClearSelectedCompany?: (nextName?: string) => void;
@ -236,6 +237,7 @@ interface TechnicianRowProps {
onRemove: () => void; onRemove: () => void;
canRemove: boolean; canRemove: boolean;
tradeOptions: string[]; tradeOptions: string[];
tradeOptionsLoading?: boolean;
showStatus: boolean; showStatus: boolean;
} }
@ -246,6 +248,7 @@ function TechnicianRow({
onRemove, onRemove,
canRemove, canRemove,
tradeOptions, tradeOptions,
tradeOptionsLoading,
showStatus, showStatus,
}: TechnicianRowProps) { }: TechnicianRowProps) {
return ( return (
@ -315,7 +318,12 @@ function TechnicianRow({
)} )}
/> />
</Stack> </Stack>
<VendorTradeSpecialtiesField control={control} index={index} tradeOptions={tradeOptions} /> <VendorTradeSpecialtiesField
control={control}
index={index}
tradeOptions={tradeOptions}
tradeOptionsLoading={tradeOptionsLoading}
/>
{showStatus && ( {showStatus && (
<Controller <Controller
control={control} control={control}
@ -341,11 +349,13 @@ function TechniciansFieldArray({
control, control,
errors, errors,
tradeOptions, tradeOptions,
tradeOptionsLoading,
showStatus, showStatus,
}: { }: {
control: Control<VendorCompanyRosterFormValues>; control: Control<VendorCompanyRosterFormValues>;
errors: FieldErrors<VendorCompanyRosterFormValues>; errors: FieldErrors<VendorCompanyRosterFormValues>;
tradeOptions: string[]; tradeOptions: string[];
tradeOptionsLoading?: boolean;
showStatus: boolean; showStatus: boolean;
}) { }) {
const { fields, append, remove } = useFieldArray({ control, name: "technicians" }); const { fields, append, remove } = useFieldArray({ control, name: "technicians" });
@ -389,6 +399,7 @@ function TechniciansFieldArray({
onRemove={() => remove(index)} onRemove={() => remove(index)}
canRemove canRemove
tradeOptions={tradeOptions} tradeOptions={tradeOptions}
tradeOptionsLoading={tradeOptionsLoading}
showStatus={showStatus} showStatus={showStatus}
/> />
)) ))
@ -399,7 +410,13 @@ function TechniciansFieldArray({
} }
export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) { export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) {
const { control, errors, tradeOptions = [], showTechnicianStatus = true } = props; const {
control,
errors,
tradeOptions = [],
tradeOptionsLoading = false,
showTechnicianStatus = true,
} = props;
return ( return (
<Stack spacing={3}> <Stack spacing={3}>
<CompanyFields {...props} /> <CompanyFields {...props} />
@ -408,6 +425,7 @@ export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) {
control={control} control={control}
errors={errors} errors={errors}
tradeOptions={tradeOptions} tradeOptions={tradeOptions}
tradeOptionsLoading={tradeOptionsLoading}
showStatus={showTechnicianStatus} showStatus={showTechnicianStatus}
/> />
</Stack> </Stack>

View file

@ -124,6 +124,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa
control={form.control} control={form.control}
errors={form.errors} errors={form.errors}
tradeOptions={form.trades} tradeOptions={form.trades}
tradeOptionsLoading={form.tradesLoading}
{...companySelectionProps} {...companySelectionProps}
/> />

View file

@ -1,4 +1,4 @@
import { useState } from "react"; import { useRef, useState } from "react";
import { Controller, type Control } from "react-hook-form"; import { Controller, type Control } from "react-hook-form";
import AddIcon from "@mui/icons-material/Add"; import AddIcon from "@mui/icons-material/Add";
import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward";
@ -14,8 +14,43 @@ import {
TextField, TextField,
Typography, Typography,
} from "@mui/material"; } from "@mui/material";
import { Text } from "@/components/ui/text";
import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema"; import type { VendorCompanyRosterFormValues } from "@/domain/vendors/schemas/vendor-roster-schema";
/**
* `Text variant="feedback"` stays mounted when silent so the aria-live region can announce later.
* An empty paragraph still claims a line box, which would add a permanent gap under the trades
* input and shift the approved drawer layout. Collapse the box while keeping the node in the DOM
* (never `display: none`, which would stop announcements).
*/
function collapsedWhenSilent(visible: boolean) {
return visible ? undefined : { height: 0, margin: 0, overflow: "hidden" as const };
}
function TradeFeedbackRegions({
listUnavailable,
message,
}: {
listUnavailable: boolean;
message: string | null;
}) {
return (
<>
<Text
variant="feedback"
tone="warning"
when={listUnavailable}
sx={collapsedWhenSilent(listUnavailable)}
>
Trade list is unavailable right now — you can still type a trade manually.
</Text>
<Text variant="feedback" when={Boolean(message)} sx={collapsedWhenSilent(Boolean(message))}>
{message}
</Text>
</>
);
}
function splitTrades(value: string | undefined): string[] { function splitTrades(value: string | undefined): string[] {
return (value ?? "") return (value ?? "")
.split(",") .split(",")
@ -23,18 +58,59 @@ function splitTrades(value: string | undefined): string[] {
.filter(Boolean); .filter(Boolean);
} }
function dedupeTrades(options: string[], current: string[]): string[] {
const seen = new Set<string>();
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 { interface VendorTradeSpecialtiesFieldProps {
control: Control<VendorCompanyRosterFormValues>; control: Control<VendorCompanyRosterFormValues>;
index: number; index: number;
tradeOptions: string[]; tradeOptions: string[];
tradeOptionsLoading?: boolean;
}
/**
* SH-249: an empty tradeOptions array means two different things. While the facets
* query is in flight it means "not loaded yet"; only once it settles does it mean
* "vocabulary genuinely unavailable". Failing open during the cold-fetch window let
* free text through the canonical gate permanently, because anything committed there
* is added to knownTradesRef and stays selectable after the real list arrives.
*
* SH-249: why a typed trade was refused. Extracted to module scope so the field
* component stays inside the maintainability gate's function-length cap.
*/
function resolveRejectionMessage(value: string, loading: boolean): string {
return loading
? "Trades are still loading. Wait for the list, then pick a trade."
: `"${value}" is not in the trades list. Pick a trade from the list.`;
} }
export function VendorTradeSpecialtiesField({ export function VendorTradeSpecialtiesField({
control, control,
index, index,
tradeOptions, tradeOptions,
tradeOptionsLoading = false,
}: VendorTradeSpecialtiesFieldProps) { }: VendorTradeSpecialtiesFieldProps) {
const [tradeInput, setTradeInput] = useState(""); const [tradeInput, setTradeInput] = useState("");
const [feedbackMessage, setFeedbackMessage] = useState<string | null>(null);
const knownTradesRef = useRef<Set<string>>(new Set());
const tradeListUnavailable = !tradeOptionsLoading && tradeOptions.length === 0;
const gateActive = tradeOptions.length > 0 || tradeOptionsLoading;
return ( return (
<Box> <Box>
@ -47,10 +123,28 @@ export function VendorTradeSpecialtiesField({
name={`technicians.${index}.tradeSpecialties`} name={`technicians.${index}.tradeSpecialties`}
render={({ field }) => { render={({ field }) => {
const trades = splitTrades(field.value); 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 commit = (next: string[]) => field.onChange(next.join(", "));
const add = (value: string) => { const add = (value: string) => {
const normalized = value.trim(); const normalized = value.trim();
if (normalized && !trades.includes(normalized)) commit([...trades, normalized]); setFeedbackMessage(null);
if (!normalized) {
setTradeInput("");
return;
}
const canonical = matchTradeOption(normalized, selectableTrades);
if (gateActive && !canonical) {
setFeedbackMessage(resolveRejectionMessage(normalized, tradeOptionsLoading));
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(""); setTradeInput("");
}; };
const move = (tradeIndex: number, direction: -1 | 1) => { const move = (tradeIndex: number, direction: -1 | 1) => {
@ -64,31 +158,31 @@ export function VendorTradeSpecialtiesField({
return ( return (
<Stack spacing={1} className="mt-2"> <Stack spacing={1} className="mt-2">
<Box className="flex flex-wrap gap-1.5"> <Box className="flex flex-wrap gap-1.5">
{trades.length === 0 ? ( <Text variant="description" tone="muted" when={trades.length === 0}>
<Typography variant="body2" sx={{ color: "text.secondary" }}> No trades selected.
No trades selected. </Text>
</Typography> {trades.map((trade, tradeIndex) => (
) : ( <Chip
trades.map((trade, tradeIndex) => ( key={`${trade}-${tradeIndex}`}
<Chip label={tradeIndex === 0 ? `${trade} (primary)` : trade}
key={`${trade}-${tradeIndex}`} onDelete={() =>
label={tradeIndex === 0 ? `${trade} (primary)` : trade} commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex))
onDelete={() => }
commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex)) deleteIcon={<CloseIcon data-testid={`remove-trade-${trade}`} />}
} />
deleteIcon={<CloseIcon data-testid={`remove-trade-${trade}`} />} ))}
/>
))
)}
</Box> </Box>
<Stack direction="row" spacing={1} sx={{ alignItems: "center" }}> <Stack direction="row" spacing={1} sx={{ alignItems: "center" }}>
<Autocomplete <Autocomplete
freeSolo freeSolo
options={tradeOptions} options={selectableTrades}
value={null} value={null}
inputValue={tradeInput} inputValue={tradeInput}
onInputChange={(_event, value, reason) => { onInputChange={(_event, value, reason) => {
if (reason === "input") setTradeInput(value); if (reason === "input") {
setTradeInput(value);
setFeedbackMessage(null);
}
}} }}
onChange={(_event, value) => { onChange={(_event, value) => {
if (typeof value === "string") add(value); if (typeof value === "string") add(value);
@ -147,6 +241,9 @@ export function VendorTradeSpecialtiesField({
); );
}} }}
/> />
{/* Outside the spaced Stack: an always-mounted live region would otherwise inherit Stack
spacing and add a permanent gap under the trades input, shifting the approved layout. */}
<TradeFeedbackRegions listUnavailable={tradeListUnavailable} message={feedbackMessage} />
</Box> </Box>
); );
} }

View file

@ -0,0 +1,192 @@
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<VendorCompanyRosterFormValues>;
valuesRef: { current: string };
}) {
const value = useWatch({ control, name: "technicians.0.tradeSpecialties" }) ?? "";
useEffect(() => {
valuesRef.current = value;
}, [value, valuesRef]);
return null;
}
function TradeSpecialtiesHarness({
tradeOptions,
tradeOptionsLoading = false,
initialTradeSpecialties = "",
valuesRef,
}: {
tradeOptions: string[];
tradeOptionsLoading?: boolean;
initialTradeSpecialties?: string;
valuesRef: { current: string };
}) {
const { control } = useForm<VendorCompanyRosterFormValues>({
defaultValues: {
...emptyVendorCompanyRosterForm,
technicians: [{ ...emptyRosterTechnician, tradeSpecialties: initialTradeSpecialties }],
},
});
return (
<>
<VendorTradeSpecialtiesField
control={control}
index={0}
tradeOptions={tradeOptions}
tradeOptionsLoading={tradeOptionsLoading}
/>
<TradeSpecialtiesSpy control={control} valuesRef={valuesRef} />
</>
);
}
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(
<TradeSpecialtiesHarness
tradeOptions={["Plumbing", "HVAC", "Electrical"]}
valuesRef={valuesRef}
/>,
);
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("does not fail open while the trades facet is still loading", () => {
const valuesRef = { current: "" };
renderWithProviders(
<TradeSpecialtiesHarness tradeOptions={[]} tradeOptionsLoading valuesRef={valuesRef} />,
);
// A cold fetch must not look like an outage.
expect(screen.queryByText(/trade list is unavailable right now/i)).toBeNull();
const input = getAddTradeInput();
typeAndCommitTrade(input, "Plumbing", "enter");
expect(screen.getByText(/trades are still loading/i)).toBeInTheDocument();
expect(screen.queryByText("Plumbing (primary)")).toBeNull();
expect(valuesRef.current).toBe("");
});
it("has no selectable options when the trades facet is empty, warns, and still accepts typed trades", () => {
const valuesRef = { current: "" };
renderWithProviders(<TradeSpecialtiesHarness tradeOptions={[]} valuesRef={valuesRef} />);
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(
<TradeSpecialtiesHarness tradeOptions={["Plumbing", "HVAC"]} valuesRef={valuesRef} />,
);
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(
<TradeSpecialtiesHarness tradeOptions={["Plumbing", "HVAC"]} valuesRef={valuesRef} />,
);
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(
<TradeSpecialtiesHarness
tradeOptions={["Plumbing", "HVAC"]}
initialTradeSpecialties="Backflow Testing, HVAC"
valuesRef={valuesRef}
/>,
);
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(
<TradeSpecialtiesHarness
tradeOptions={["Plumbing"]}
initialTradeSpecialties="Plumbing"
valuesRef={valuesRef}
/>,
);
const input = getAddTradeInput();
typeAndCommitTrade(input, "plumbing", "enter");
expect(screen.getByText(/is already on this technician/i)).toBeInTheDocument();
expect(valuesRef.current).toBe("Plumbing");
});
});