Compare commits

...

10 commits

Author SHA1 Message Date
Alexandre Brandizzi
58bdd6bffa
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)
2026-08-20 13:38:23 -03:00
Alexandre Brandizzi
ef04008fa2
Merge branch 'dev' into feat/sh-249-fe-trades 2026-08-20 13:25:09 -03:00
Alexandre Brandizzi
4d8e86b264
Merge branch 'dev' into feat/sh-249-fe-trades 2026-08-20 11:48:07 -03:00
Alexandre Brandizzi
22bd392794
Merge branch 'dev' into feat/sh-249-fe-trades 2026-08-20 10:58:29 -03:00
Alexandre Brandizzi
13d9aa2034
Merge branch 'dev' into feat/sh-249-fe-trades 2026-08-19 10:28:01 -03:00
Codex Review Integration
cccb0e4ca7 refactor(vendors): extract trade rejection message to satisfy length gate (SH-249)
The loading-aware gate pushed VendorTradeSpecialtiesField to 151 lines, one over
the changed-file maintainability cap. Move the rejection-message resolution and
the contract explanation to module scope; no behavior change.
2026-08-18 18:09:23 -03:00
Alexandre Brandizzi
5d18b76143
Merge branch 'dev' into feat/sh-249-fe-trades 2026-08-18 17:46:46 -03:00
Codex Review Integration
64caab1832 fix(vendors): do not fail open while the trades facet is loading (SH-249)
An empty tradeOptions array meant two different things: "facets query still in
flight" and "vocabulary genuinely unavailable". Both enabled fail-open free
text and showed the outage warning, so a normal cold fetch looked like an
outage. Anything committed in that window was added to knownTradesRef and stayed
selectable after the canonical list arrived, permanently undermining the gate.

Expose the facets query isLoading as tradesLoading, thread it to the field as
tradeOptionsLoading, and keep the canonical gate active while loading. The
outage warning now shows only once the query has settled empty.
2026-08-18 17:25:44 -03:00
Codex Review Integration
63e37dd0bb fix(vendors): keep trades live region out of Stack spacing (SH-249)
The always-mounted aria-live feedback region inherited MUI Stack spacing,
adding a permanent gap under the trades input and shifting the approved
Vendor edit drawer layout. vendor-visual-regression caught it.

Move the region outside the spaced Stack and collapse it while silent, so
the live region stays in the DOM for announcements without claiming layout.
Restores the caption to MUI Typography; that swap was cosmetic and out of
scope for SH-249.
2026-08-18 14:27:42 -03:00
Codex Review Integration
df00e56d6e fix(vendors): gate trades to canonical list, preserve legacy values (SH-249) 2026-08-18 13:12:10 -03:00
7 changed files with 335 additions and 23 deletions

View file

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

View file

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

View file

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

View file

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

View file

@ -124,6 +124,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa
control={form.control}
errors={form.errors}
tradeOptions={form.trades}
tradeOptionsLoading={form.tradesLoading}
{...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 AddIcon from "@mui/icons-material/Add";
import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward";
@ -14,8 +14,43 @@ import {
TextField,
Typography,
} from "@mui/material";
import { Text } from "@/components/ui/text";
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[] {
return (value ?? "")
.split(",")
@ -23,18 +58,59 @@ function splitTrades(value: string | undefined): string[] {
.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 {
control: Control<VendorCompanyRosterFormValues>;
index: number;
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({
control,
index,
tradeOptions,
tradeOptionsLoading = false,
}: VendorTradeSpecialtiesFieldProps) {
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 (
<Box>
@ -47,10 +123,28 @@ export function VendorTradeSpecialtiesField({
name={`technicians.${index}.tradeSpecialties`}
render={({ field }) => {
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 (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("");
};
const move = (tradeIndex: number, direction: -1 | 1) => {
@ -64,31 +158,31 @@ export function VendorTradeSpecialtiesField({
return (
<Stack spacing={1} className="mt-2">
<Box className="flex flex-wrap gap-1.5">
{trades.length === 0 ? (
<Typography variant="body2" sx={{ color: "text.secondary" }}>
No trades selected.
</Typography>
) : (
trades.map((trade, tradeIndex) => (
<Chip
key={`${trade}-${tradeIndex}`}
label={tradeIndex === 0 ? `${trade} (primary)` : trade}
onDelete={() =>
commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex))
}
deleteIcon={<CloseIcon data-testid={`remove-trade-${trade}`} />}
/>
))
)}
<Text variant="description" tone="muted" when={trades.length === 0}>
No trades selected.
</Text>
{trades.map((trade, tradeIndex) => (
<Chip
key={`${trade}-${tradeIndex}`}
label={tradeIndex === 0 ? `${trade} (primary)` : trade}
onDelete={() =>
commit(trades.filter((_, itemIndex) => itemIndex !== tradeIndex))
}
deleteIcon={<CloseIcon data-testid={`remove-trade-${trade}`} />}
/>
))}
</Box>
<Stack direction="row" spacing={1} sx={{ alignItems: "center" }}>
<Autocomplete
freeSolo
options={tradeOptions}
options={selectableTrades}
value={null}
inputValue={tradeInput}
onInputChange={(_event, value, reason) => {
if (reason === "input") setTradeInput(value);
if (reason === "input") {
setTradeInput(value);
setFeedbackMessage(null);
}
}}
onChange={(_event, 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>
);
}

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");
});
});