feat(ui): bounce cold arrivals off archived company URLs, add Unarchive (#11302)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Archiving a company hides it from the sidebar switcher, but remembered last-visited paths, browser history, bookmarks, and restored tabs keep depositing users onto its URLs long after archiving > - Since the selection ping-pong fix (#11300) those arrivals render, but the user is stranded inside a workspace the sidebar refuses to show — and unarchiving had no UI anywhere, so the only way back was a hand-typed settings URL > - This pull request bounces cold arrivals at archived company URLs to an active company (with a toast naming why), lets deliberate visits stick, and adds an Unarchive action to the companies list > - The benefit is that stale URLs stop stranding users in retired workspaces, and archived companies become restorable from the one page that still lists them ## Linked Issues or Issue Description Follow-up to #11300. No existing issue for the remaining gap; description follows the enhancement template: **What happened?** After #11300, opening an archived company's URL (stale tab, history, bookmark, remembered path) renders that company's pages — but the sidebar switcher does not list it, so the user is stranded in a workspace they retired, and every stale URL pulls them back in. Separately, unarchiving a company has no UI: the archive button lives in company settings, which becomes unreachable through normal navigation once the company is archived. **Expected behavior** Arriving cold at an archived company's URL lands the user in an active workspace, with a toast explaining the redirect. Explicitly choosing the archived company (from the companies list) still works, so its pages remain reachable. Archived companies can be restored from the companies list. **Steps to reproduce** 1. Create two companies; archive one. 2. Open `/{archivedPrefix}/dashboard` directly — before: renders the archived workspace with no sidebar presence; after: bounces to the active company's dashboard with a toast. 3. On the companies list, open the archived company's row menu — before: no restore action anywhere; after: Unarchive. ## What Changed - `ui/src/lib/company-selection.ts`: `resolveArchivedCompanyBounce` — pure policy: bounce when the URL names an archived company that is not the current selection and an active company exists; prefer the currently selected active company as the destination. - `ui/src/components/Layout.tsx`: the route-sync effect applies the bounce (toast + selection + `replace` navigation) before syncing selection from the route. - `ui/src/pages/Companies.tsx`: Unarchive action (`PATCH status: "active"`) in the row menu for archived companies. - Tests: unit cases for the bounce policy; the e2e now drives all three behaviors (direct-load bounce with toast, re-arrival bounce, deliberate visit sticks) on top of the existing crash regression. ## Verification - `pnpm vitest run src/lib/company-selection.test.ts src/context/CompanyContext.test.tsx src/pages/Companies.test.tsx` in `ui/` — 20 tests pass. - `npx playwright test --config tests/e2e/playwright.config.ts archived-company-url` — passes, covering bounce, toast, and deliberate-visit paths. - `pnpm typecheck` in `ui/` — clean. ## Risks Low risk. The bounce only fires for archived-company URLs when the archived company is not already selected and an active company exists; all-archived instances render as before. Deliberate selection from the companies list is unaffected (selection equals the matched company, so no bounce). Unarchive reuses the existing `PATCH /api/companies/:id` status transition the server already supports. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with extended thinking and tool use (code search, edit, test execution, Playwright e2e). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
d24a79f741
commit
a09d7dcc06
|
|
@ -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);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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", () => ({
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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() {
|
|||
<Pencil className="h-3.5 w-3.5" />
|
||||
Rename
|
||||
</DropdownMenuItem>
|
||||
{company.status === "archived" && (
|
||||
<DropdownMenuItem
|
||||
disabled={unarchiveMutation.isPending}
|
||||
onClick={() => unarchiveMutation.mutate(company.id)}
|
||||
>
|
||||
<ArchiveRestore className="h-3.5 w-3.5" />
|
||||
Unarchive
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
<DropdownMenuSeparator />
|
||||
<DropdownMenuItem
|
||||
variant="destructive"
|
||||
|
|
|
|||
Loading…
Reference in New Issue