diff --git a/tests/e2e/archived-company-url.spec.ts b/tests/e2e/archived-company-url.spec.ts index 87d362a737..52cd5ea116 100644 --- a/tests/e2e/archived-company-url.spec.ts +++ b/tests/e2e/archived-company-url.spec.ts @@ -1,16 +1,18 @@ import { expect, test, type Page } from "@playwright/test"; /** - * Regression: landing on an archived company's URL crashed the app with React - * error #185. The Layout route-sync matched the URL against the full company - * list and selected the archived company; the CompanyProvider bootstrap - * resolver only accepted non-archived companies and immediately re-selected - * an active one; the two effects ping-ponged the selection until React blew - * the nested-update limit and unmounted the tree. + * Archived-company URL handling. * - * Field shape: a workspace whose seeded primary company was archived — every - * revisit of its remembered `/PREFIX/...` URL (first load and back/forward - * navigations alike) produced a blank page. + * Regression (React #185): landing on an archived company's URL crashed the + * app — the Layout route-sync selected the archived company while the + * CompanyProvider bootstrap resolver rejected it, and the two effects + * ping-ponged the selection until React blew the nested-update limit. + * + * Policy: stale state (remembered paths, browser history, bookmarks, + * restored tabs) keeps depositing users into archived companies long after + * archiving, so a cold arrival bounces to an active company with a toast. + * A deliberate visit — selecting the archived company from the companies + * list — sticks, so its pages stay reachable. */ async function createCompany(page: Page, name: string): Promise<{ id: string; prefix: string }> { @@ -25,7 +27,7 @@ function collectFatalErrors(page: Page): string[] { page.on("pageerror", (err) => { fatal.push(`PAGEERROR: ${err.message}`); }); - page.on("console", async (msg) => { + page.on("console", (msg) => { if (msg.type() !== "error") return; const text = msg.text(); if (!/App shell crashed|Page render failed|Minified React error #185|Maximum update depth/.test(text)) { @@ -36,7 +38,7 @@ function collectFatalErrors(page: Page): string[] { return fatal; } -test("landing on an archived company's URL does not crash the app", async ({ page }) => { +test("archived company URLs bounce cold arrivals and honor deliberate visits", async ({ page }) => { const fatal = collectFatalErrors(page); const active = await createCompany(page, "Archived Loop Active"); @@ -46,22 +48,37 @@ test("landing on an archived company's URL does not crash the app", async ({ pag }); expect(archiveRes.ok(), `archive failed ${archiveRes.status()}: ${await archiveRes.text()}`).toBe(true); - // Field repro #1: a fresh load straight onto the archived company's - // remembered URL (the first-open blank screen). + // The e2e server is shared across specs, so other companies exist and the + // bounce may pick any active one; the contract is only "not the archived + // company's routes anymore". + const awayFromArchived = (url: URL) => + url.pathname.endsWith("/dashboard") && !url.pathname.startsWith(`/${archived.prefix}/`); + + // Cold arrival #1: a fresh load straight onto the archived company's + // remembered URL (previously the first-open blank-screen crash) bounces + // to an active company. await page.goto(`/${archived.prefix}/dashboard`); - await page.waitForTimeout(2_500); + await page.waitForURL(awayFromArchived, { timeout: 15_000 }); + await expect(page.getByText("Archived Loop Archived is archived")).toBeVisible(); expect(fatal, `crash on direct load of archived company URL:\n${fatal.join("\n")}`).toEqual([]); - // Field repro #2: visit the active company, then return to the archived - // company's URL with client-side history navigation (the "switched back" - // crash). - await page.goto(`/${active.prefix}/dashboard`); - await page.waitForLoadState("networkidle"); - await page.goBack(); - await page.waitForTimeout(2_500); - expect(fatal, `crash on back-navigation to archived company URL:\n${fatal.join("\n")}`).toEqual([]); + // Cold arrival #2: re-arrival at the archived URL (previously the + // "switched back" crash) bounces the same way. + await page.goto(`/${archived.prefix}/issues`); + await page.waitForURL(awayFromArchived, { timeout: 15_000 }); + expect(fatal, `crash on re-arrival at archived company URL:\n${fatal.join("\n")}`).toEqual([]); + + // Deliberate visit: selecting the archived company from the companies list + // sticks — no bounce — so its pages (and the way back to unarchiving) stay + // reachable. + await page.goto(`/${active.prefix}/companies`); + await page.waitForLoadState("networkidle"); + await page.getByText("Archived Loop Archived").first().click(); + await page.waitForURL(new RegExp(`/${archived.prefix}/`), { timeout: 15_000 }); + await page.waitForTimeout(1_500); + expect(page.url()).toMatch(new RegExp(`/${archived.prefix}/`)); + expect(fatal, `crash during deliberate archived visit:\n${fatal.join("\n")}`).toEqual([]); - // The app must have landed somewhere real, not a blank unmounted root. const rootContent = await page.locator("#root").innerText().catch(() => ""); expect(rootContent.length).toBeGreaterThan(0); }); diff --git a/ui/src/components/Layout.test.tsx b/ui/src/components/Layout.test.tsx index 34272d6e00..416bc7b659 100644 --- a/ui/src/components/Layout.test.tsx +++ b/ui/src/components/Layout.test.tsx @@ -217,6 +217,10 @@ vi.mock("../api/instanceSettings", () => ({ vi.mock("../lib/company-selection", () => ({ shouldSyncCompanySelectionFromRoute: () => false, + // No bounce in the shared harness: these tests exercise layout chrome, not + // archived-company routing (covered by company-selection unit tests and the + // archived-company-url e2e). + resolveArchivedCompanyBounce: () => null, })); vi.mock("../lib/main-content-focus", () => ({ diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index a838fea1ec..e4f62b6e21 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -33,7 +33,8 @@ import { useAppsEnabled } from "../hooks/useAppsEnabled"; import { useCompanyPageMemory } from "../hooks/useCompanyPageMemory"; import { healthApi } from "../api/health"; import { instanceSettingsApi } from "../api/instanceSettings"; -import { shouldSyncCompanySelectionFromRoute } from "../lib/company-selection"; +import { resolveArchivedCompanyBounce, shouldSyncCompanySelectionFromRoute } from "../lib/company-selection"; +import { useOptionalToastActions } from "../context/ToastContext"; import { applyMainContentScrollTop, NavigationScrollMemory, @@ -94,6 +95,8 @@ export function Layout() { } = useSidebar(); const { openNewIssue, openOnboarding } = useDialogActions(); const { togglePanelVisible } = usePanel(); + // Optional: Layout also renders in harnesses without a ToastProvider. + const pushToast = useOptionalToastActions()?.pushToast ?? null; const { companies, loading: companiesLoading, @@ -240,6 +243,27 @@ export function Layout() { return; } + // Stale state (remembered paths, history, bookmarks, restored tabs) + // deposits users into archived companies long after archiving; a cold + // arrival bounces to an active company instead of dwelling there. + // Deliberate visits (the company is already the selection) stay put. + const bounce = resolveArchivedCompanyBounce({ + matchedCompany, + selectedCompanyId, + companies, + }); + if (bounce) { + pushToast?.({ + title: `${matchedCompany.name} is archived`, + body: `Switched to ${bounce.name}.`, + tone: "info", + dedupeKey: `archived-company-bounce:${matchedCompany.id}`, + }); + setSelectedCompanyId(bounce.id, { source: "route_sync" }); + navigate(`/${bounce.issuePrefix}/dashboard`, { replace: true }); + return; + } + if ( shouldSyncCompanySelectionFromRoute({ selectionSource, @@ -257,6 +281,7 @@ export function Layout() { location.pathname, location.search, navigate, + pushToast, selectionSource, selectedCompanyId, setSelectedCompanyId, diff --git a/ui/src/lib/company-selection.test.ts b/ui/src/lib/company-selection.test.ts index a8533a4b08..43f4deb8c5 100644 --- a/ui/src/lib/company-selection.test.ts +++ b/ui/src/lib/company-selection.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { shouldSyncCompanySelectionFromRoute } from "./company-selection"; +import { resolveArchivedCompanyBounce, shouldSyncCompanySelectionFromRoute } from "./company-selection"; describe("shouldSyncCompanySelectionFromRoute", () => { it("does not resync when selection already matches the route", () => { @@ -32,3 +32,56 @@ describe("shouldSyncCompanySelectionFromRoute", () => { ).toBe(true); }); }); + +describe("resolveArchivedCompanyBounce", () => { + const archived = { id: "old", name: "Old Co", issuePrefix: "OLD", status: "archived" }; + const active = { id: "pap", name: "Paperclip", issuePrefix: "PAP", status: "active" }; + const other = { id: "ret", name: "Retail", issuePrefix: "RET", status: "active" }; + + it("bounces a cold arrival on an archived company's URL to the active selection", () => { + expect( + resolveArchivedCompanyBounce({ + matchedCompany: archived, + selectedCompanyId: "pap", + companies: [archived, active, other], + }), + ).toEqual(active); + }); + + it("bounces to the first active company when nothing is selected", () => { + expect( + resolveArchivedCompanyBounce({ + matchedCompany: archived, + selectedCompanyId: null, + companies: [archived, other], + }), + ).toEqual(other); + }); + + it("does not bounce a deliberate visit where the archived company is already selected", () => { + expect( + resolveArchivedCompanyBounce({ + matchedCompany: archived, + selectedCompanyId: "old", + companies: [archived, active], + }), + ).toBeNull(); + }); + + it("does not bounce active companies or when every company is archived", () => { + expect( + resolveArchivedCompanyBounce({ + matchedCompany: active, + selectedCompanyId: null, + companies: [archived, active], + }), + ).toBeNull(); + expect( + resolveArchivedCompanyBounce({ + matchedCompany: archived, + selectedCompanyId: null, + companies: [archived], + }), + ).toBeNull(); + }); +}); diff --git a/ui/src/lib/company-selection.ts b/ui/src/lib/company-selection.ts index ce02cb4ddc..43236de27e 100644 --- a/ui/src/lib/company-selection.ts +++ b/ui/src/lib/company-selection.ts @@ -1,5 +1,43 @@ export type CompanySelectionSource = "manual" | "route_sync" | "bootstrap"; +interface BounceCandidateCompany { + id: string; + name: string; + issuePrefix: string; + status: string; +} + +/** + * Decides whether a navigation that landed on an archived company's URL + * should bounce to an active company instead of dwelling in the archive. + * + * Stale state deposits users into archived companies long after archiving: + * remembered last-visited paths, browser history, bookmarks, and restored + * tabs all outlive the archive. Rendering those pages is safe, but it is + * never where the user wants to *be* — the sidebar does not even list the + * company. Cold arrivals therefore bounce to an active company. + * + * Deliberate visits still work: when the archived company is already the + * selection (the user chose it from the companies list), there is no + * bounce, so its settings and unarchive flows stay reachable. When no + * active company exists there is nowhere better to go, so the archive + * renders rather than bouncing into a dead end. + */ +export function resolveArchivedCompanyBounce(params: { + matchedCompany: BounceCandidateCompany | null; + selectedCompanyId: string | null; + companies: BounceCandidateCompany[]; +}): BounceCandidateCompany | null { + const { matchedCompany, selectedCompanyId, companies } = params; + if (!matchedCompany || matchedCompany.status !== "archived") return null; + if (selectedCompanyId === matchedCompany.id) return null; + + const selectedActive = companies.find( + (company) => company.id === selectedCompanyId && company.status !== "archived", + ); + return selectedActive ?? companies.find((company) => company.status !== "archived") ?? null; +} + export function shouldSyncCompanySelectionFromRoute(params: { selectionSource: CompanySelectionSource; selectedCompanyId: string | null; diff --git a/ui/src/pages/Companies.tsx b/ui/src/pages/Companies.tsx index a1f42297c8..8e307201c4 100644 --- a/ui/src/pages/Companies.tsx +++ b/ui/src/pages/Companies.tsx @@ -29,6 +29,7 @@ import { CircleDot, DollarSign, Calendar, + ArchiveRestore, } from "lucide-react"; export function Companies() { @@ -74,6 +75,19 @@ export function Companies() { }, }); + // Unarchiving previously had no UI at all: archiving happens in company + // settings, but an archived company disappears from the sidebar switcher, + // so its settings page — and with it any way back — was only reachable by + // hand-typed URL. This list is the one place that still shows archived + // companies, so restoration lives here. + const unarchiveMutation = useMutation({ + mutationFn: (id: string) => companiesApi.update(id, { status: "active" }), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: queryKeys.companies.all }); + queryClient.invalidateQueries({ queryKey: queryKeys.companies.stats }); + }, + }); + useEffect(() => { setBreadcrumbs([{ label: "Companies" }]); }, [setBreadcrumbs]); @@ -224,6 +238,15 @@ export function Companies() { Rename + {company.status === "archived" && ( + unarchiveMutation.mutate(company.id)} + > + + Unarchive + + )}