mirror of
https://github.com/Sea-Haven-Industries/shoc-frontend-new.git
synced 2026-10-01 20:13:12 +00:00
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.
This commit is contained in:
parent
63e37dd0bb
commit
64caab1832
7 changed files with 66 additions and 7 deletions
|
|
@ -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<void>;
|
||||
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<VendorRosterConflict | null>(null);
|
||||
|
||||
|
|
@ -168,6 +169,7 @@ export function useVendorRosterForm({
|
|||
roster: routeRoster,
|
||||
companies,
|
||||
trades,
|
||||
tradesLoading: facetsLoading,
|
||||
selectedCompanyId: selection.selectedCompanyId,
|
||||
selectCompany: selection.selectCompany,
|
||||
clearSelectedCompany: selection.clearSelectedCompany,
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -288,6 +288,7 @@ function DrawerEditor({
|
|||
control={form.control}
|
||||
errors={form.errors}
|
||||
tradeOptions={form.trades}
|
||||
tradeOptionsLoading={form.tradesLoading}
|
||||
showTechnicianStatus={false}
|
||||
/>
|
||||
{selectedIndex >= 0 && (
|
||||
|
|
|
|||
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -124,6 +124,7 @@ export default function VendorRosterPage({ vendorId, companyId }: VendorRosterPa
|
|||
control={form.control}
|
||||
errors={form.errors}
|
||||
tradeOptions={form.trades}
|
||||
tradeOptionsLoading={form.tradesLoading}
|
||||
{...companySelectionProps}
|
||||
/>
|
||||
|
||||
|
|
|
|||
|
|
@ -81,16 +81,26 @@ interface VendorTradeSpecialtiesFieldProps {
|
|||
control: Control<VendorCompanyRosterFormValues>;
|
||||
index: number;
|
||||
tradeOptions: string[];
|
||||
tradeOptionsLoading?: boolean;
|
||||
}
|
||||
|
||||
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());
|
||||
// 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 (
|
||||
<Box>
|
||||
|
|
@ -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. */}
|
||||
<TradeFeedbackRegions listUnavailable={tradeOptions.length === 0} message={feedbackMessage} />
|
||||
<TradeFeedbackRegions listUnavailable={tradeListUnavailable} message={feedbackMessage} />
|
||||
</Box>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<>
|
||||
<VendorTradeSpecialtiesField control={control} index={0} tradeOptions={tradeOptions} />
|
||||
<VendorTradeSpecialtiesField
|
||||
control={control}
|
||||
index={0}
|
||||
tradeOptions={tradeOptions}
|
||||
tradeOptionsLoading={tradeOptionsLoading}
|
||||
/>
|
||||
<TradeSpecialtiesSpy control={control} valuesRef={valuesRef} />
|
||||
</>
|
||||
);
|
||||
|
|
@ -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(
|
||||
<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} />);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue