From 64caab18326eb34e81d793b7e837f1f1ec01f19e Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Tue, 18 Aug 2026 17:25:44 -0300 Subject: [PATCH] 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. --- .../_components/use-vendor-roster-form.ts | 4 ++- .../_components/vendor-create-modal.tsx | 1 + .../_components/vendor-detail-drawer.tsx | 1 + .../_components/vendor-roster-form-fields.tsx | 22 ++++++++++++++-- .../_components/vendor-roster-page.tsx | 1 + .../vendor-trade-specialties-field.tsx | 18 ++++++++++--- .../vendor-trade-specialties-field.test.tsx | 26 ++++++++++++++++++- 7 files changed, 66 insertions(+), 7 deletions(-) diff --git a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts index 6bf89b1f..c285f592 100644 --- a/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts +++ b/src/app/(protected)/vendors/_components/use-vendor-roster-form.ts @@ -64,6 +64,7 @@ export interface VendorRosterForm { roster: VendorCompanyRoster | undefined; companies: VendorFacetCompany[]; trades: string[]; + tradesLoading: boolean; selectedCompanyId: string | number | null; selectCompany: (company: VendorFacetCompany | null) => Promise; clearSelectedCompany: (nextName?: string) => void; @@ -81,7 +82,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(null); @@ -168,6 +169,7 @@ export function useVendorRosterForm({ roster: routeRoster, companies, trades, + tradesLoading: facetsLoading, selectedCompanyId: selection.selectedCompanyId, selectCompany: selection.selectCompany, clearSelectedCompany: selection.clearSelectedCompany, diff --git a/src/app/(protected)/vendors/_components/vendor-create-modal.tsx b/src/app/(protected)/vendors/_components/vendor-create-modal.tsx index 31804a3c..002a81bf 100644 --- a/src/app/(protected)/vendors/_components/vendor-create-modal.tsx +++ b/src/app/(protected)/vendors/_components/vendor-create-modal.tsx @@ -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} diff --git a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx index 74d6ef02..7a365a48 100644 --- a/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx +++ b/src/app/(protected)/vendors/_components/vendor-detail-drawer.tsx @@ -288,6 +288,7 @@ function DrawerEditor({ control={form.control} errors={form.errors} tradeOptions={form.trades} + tradeOptionsLoading={form.tradesLoading} showTechnicianStatus={false} /> {selectedIndex >= 0 && ( diff --git a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx index d6d5cbf7..93a10fd9 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-form-fields.tsx @@ -25,6 +25,7 @@ interface VendorRosterFormFieldsProps { errors: FieldErrors; companies?: VendorFacetCompany[]; tradeOptions?: string[]; + tradeOptionsLoading?: boolean; selectedCompanyId?: string | number | null; onSelectCompany?: (company: VendorFacetCompany | null) => Promise; 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({ )} /> - + {showStatus && ( ; errors: FieldErrors; 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 ( @@ -408,6 +425,7 @@ export function VendorRosterFormFields(props: VendorRosterFormFieldsProps) { control={control} errors={errors} tradeOptions={tradeOptions} + tradeOptionsLoading={tradeOptionsLoading} showStatus={showTechnicianStatus} /> diff --git a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx index f0316957..45f1bb19 100644 --- a/src/app/(protected)/vendors/_components/vendor-roster-page.tsx +++ b/src/app/(protected)/vendors/_components/vendor-roster-page.tsx @@ -124,6 +124,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa control={form.control} errors={form.errors} tradeOptions={form.trades} + tradeOptionsLoading={form.tradesLoading} {...companySelectionProps} /> 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 490f554d..ef7b0043 100644 --- a/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx +++ b/src/app/(protected)/vendors/_components/vendor-trade-specialties-field.tsx @@ -81,16 +81,26 @@ interface VendorTradeSpecialtiesFieldProps { control: Control; index: number; tradeOptions: string[]; + tradeOptionsLoading?: boolean; } export function VendorTradeSpecialtiesField({ control, index, tradeOptions, + tradeOptionsLoading = false, }: VendorTradeSpecialtiesFieldProps) { const [tradeInput, setTradeInput] = useState(""); const [feedbackMessage, setFeedbackMessage] = useState(null); const knownTradesRef = useRef>(new Set()); + // 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. + const tradeListUnavailable = !tradeOptionsLoading && tradeOptions.length === 0; + const gateActive = tradeOptions.length > 0 || tradeOptionsLoading; return ( @@ -114,9 +124,11 @@ export function VendorTradeSpecialtiesField({ return; } const canonical = matchTradeOption(normalized, selectableTrades); - if (tradeOptions.length > 0 && !canonical) { + if (gateActive && !canonical) { setFeedbackMessage( - `"${normalized}" is not in the trades list. Pick a trade from the list.`, + tradeOptionsLoading + ? "Trades are still loading. Wait for the list, then pick a trade." + : `"${normalized}" is not in the trades list. Pick a trade from the list.`, ); return; } @@ -225,7 +237,7 @@ 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. */} - + ); } 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 index bff2a587..03990707 100644 --- a/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx +++ b/src/test/app/(protected)/vendors/vendor-trade-specialties-field.test.tsx @@ -26,10 +26,12 @@ function TradeSpecialtiesSpy({ function TradeSpecialtiesHarness({ tradeOptions, + tradeOptionsLoading = false, initialTradeSpecialties = "", valuesRef, }: { tradeOptions: string[]; + tradeOptionsLoading?: boolean; initialTradeSpecialties?: string; valuesRef: { current: string }; }) { @@ -41,7 +43,12 @@ function TradeSpecialtiesHarness({ }); return ( <> - + ); @@ -79,6 +86,23 @@ describe("VendorTradeSpecialtiesField", () => { expect(valuesRef.current).toBe("HVAC"); }); + it("does not fail open while the trades facet is still loading", () => { + const valuesRef = { current: "" }; + renderWithProviders( + , + ); + + // 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();