diff --git a/ui/src/context/CompanyContext.test.tsx b/ui/src/context/CompanyContext.test.tsx index 7faad85a4f..cd45f94531 100644 --- a/ui/src/context/CompanyContext.test.tsx +++ b/ui/src/context/CompanyContext.test.tsx @@ -137,6 +137,7 @@ describe("shouldClearStoredCompanySelection", () => { companies: [], isLoading: false, unauthorized: true, + errored: false, })).toBe(false); }); @@ -145,8 +146,23 @@ describe("shouldClearStoredCompanySelection", () => { companies: [], isLoading: false, unauthorized: false, + errored: false, })).toBe(true); }); + + it("does not clear the stored company selection when the request failed", () => { + // `companiesListQueryOptions` sets `retry: false`, and a failure before any + // success leaves `data` undefined — which the provider defaults to an empty + // list. That is indistinguishable from "asked, and owns nothing", so + // without this a single failed request on a cold load would drop the + // customer's stored company. + expect(shouldClearStoredCompanySelection({ + companies: [], + isLoading: false, + unauthorized: false, + errored: true, + })).toBe(false); + }); }); describe("CompanyProvider", () => { @@ -176,6 +192,30 @@ describe("CompanyProvider", () => { vi.clearAllMocks(); }); + it("keeps the stored company when the company request fails", async () => { + // The seam the predicate test above cannot reach: the provider defaults a + // failed request to an empty list, so the effect has to be told the + // request errored or it reads that as "asked, and owns nothing" and clears + // the customer's stored company. + localStorage.setItem("paperclip.selectedCompanyId", "company-a"); + mockCompaniesApi.list.mockRejectedValue(new Error("companies unavailable")); + + await act(async () => { + root.render( + + + {}} /> + + , + ); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + + expect(localStorage.getItem("paperclip.selectedCompanyId")).toBe("company-a"); + }); + it("does not expose a stale stored company id before companies load", async () => { localStorage.setItem("paperclip.selectedCompanyId", "stale-company"); mockCompaniesApi.list.mockImplementation(() => new Promise(() => {})); diff --git a/ui/src/context/CompanyContext.tsx b/ui/src/context/CompanyContext.tsx index c653fd11af..5a0a3f7d8f 100644 --- a/ui/src/context/CompanyContext.tsx +++ b/ui/src/context/CompanyContext.tsx @@ -67,7 +67,24 @@ export function shouldClearStoredCompanySelection(input: { companies: Array>; isLoading: boolean; unauthorized: boolean; + /** + * Whether the company request failed. An error is not an answer, and this + * branch is destructive. + * + * `companiesListQueryOptions` sets `retry: false`, and a request that fails + * before ever succeeding leaves `data` undefined - which the provider + * defaults to `{ companies: [], unauthorized: false }`. That is + * indistinguishable from "this account was asked, and owns nothing", so a + * single failed request on a cold load would clear the customer's stored + * company and drop them onto whichever company sorts first next time. + * + * Not clearing costs nothing: a stored id that no longer resolves is + * ignored by {@link resolveBootstrapCompanySelection}, which checks it + * against the current list before using it. + */ + errored: boolean; }) { + if (input.errored) return false; return !input.isLoading && !input.unauthorized && input.companies.length === 0; } @@ -89,7 +106,12 @@ export function CompanyProvider({ children }: { children: ReactNode }) { useEffect(() => { if (isLoading) return; if (companies.length === 0) { - if (shouldClearStoredCompanySelection({ companies, isLoading: false, unauthorized: companyListUnauthorized })) { + if (shouldClearStoredCompanySelection({ + companies, + isLoading: false, + unauthorized: companyListUnauthorized, + errored: error !== null, + })) { if (selectedCompanyId !== null) { setSelectedCompanyIdState(null); } @@ -108,7 +130,7 @@ export function CompanyProvider({ children }: { children: ReactNode }) { setSelectedCompanyIdState(next); setSelectionSource("bootstrap"); localStorage.setItem(STORAGE_KEY, next); - }, [companies, companyListUnauthorized, isLoading, selectedCompanyId, sidebarCompanies]); + }, [companies, companyListUnauthorized, error, isLoading, selectedCompanyId, sidebarCompanies]); const setSelectedCompanyId = useCallback((companyId: string, options?: CompanySelectionOptions) => { setSelectedCompanyIdState(companyId);