From cacf802b8ddf07a718426cb645cd04127921785f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 15:14:35 +0200 Subject: [PATCH] test(e2e): the sharing journey, and an honest account of the stale paint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two browser contexts, two seeded accounts, one database: the owner invites, the delegate accepts and opens the record, the owner takes it back. Every step through the shipped UI, because the half of this feature that can go wrong is the half a person touches. The stale-paint check is a negative one and deliberately not "the new record's data arrives" — a check that waits for the new data says nothing about what was on screen while it arrived. An init script arms a MutationObserver before any page script on every navigation, including the reload the switch performs, and latches the delegate's own marker weight. The observer's positive control runs first on a page that definitely shows it, so its later silence is not the silence of a broken observer. Then it was broken on purpose, and the result is written into the spec rather than assumed. Removing the switch-time cache wipe, the persister's restore guard and the record-scoped hash — all three — leaves the spec GREEN: the restore already refuses to clobber a query the live cache holds, and the dashboard's query is created during render, so the snapshot never reaches that route's screen at all. Removing the reload turns it red on the banner rather than on the marker. So this suite is a regression net over the journey, not a proof of any one control; the controls are proven where they can be broken and observed, in the unit suites, and the spec's header says so. The nav assertion turned up a real gap while it was failing: the avatar block still linked into the account settings from inside a shared record. Those routes refuse under a switch, so the link led only to an explanation of why it did not work. It renders as a plain identity block now, and the mobile user menu drops the same utilities. Setup gains a second account, a marker weight in each record, a grant reset, and a clear of the login bucket — this setup signs in twice now, and the ceiling is five attempts a quarter-hour, so three local runs used to end with a 429 from the fixture rather than a failure from the product. --- e2e/account-sharing.spec.ts | 71 ++++++++++++++-- e2e/setup/global-setup.ts | 7 ++ src/components/layout/sidebar-nav.tsx | 112 ++++++++++++++++++-------- src/components/layout/top-bar.tsx | 49 ++++++----- 4 files changed, 182 insertions(+), 57 deletions(-) diff --git a/e2e/account-sharing.spec.ts b/e2e/account-sharing.spec.ts index 437f9346d..4f98006e3 100644 --- a/e2e/account-sharing.spec.ts +++ b/e2e/account-sharing.spec.ts @@ -24,6 +24,30 @@ * the delegate's own measurements page must trip the latch. Without that, "the * marker never appeared" and "the observer never worked" are the same sentence. * + * ### How much this assertion can be made to fail, measured rather than assumed + * + * Three controls stand behind it, and they were removed one at a time and + * together against a real build to see which of them this spec can actually + * detect. The honest answer is: none of them individually, and that is worth + * writing down rather than leaving as an implication. + * + * - Delete the switch-time cache wipe, the persister's restore guard and the + * record-scoped query hash — all three at once — and this spec stays + * GREEN. The reason is a rule that predates this feature: the restore + * refuses to clobber a query the live cache already holds, and the + * dashboard's own query is created during render, before any effect runs. + * So the IndexedDB snapshot never reaches the screen on this route at all. + * - Delete the full reload and the spec goes RED, but on the banner line + * rather than the marker line: without a reload the account payload never + * re-resolves and the banner never paints, so the run stops before the + * latch is read. + * + * What that means, plainly: this spec is a regression net over the whole + * journey, not a proof of any single control. The controls are proven where + * they can be broken and observed — `query-persister-shared-record.test.ts` + * for the two disk guards, `record-scope.test.ts` for the cache dimension — + * and each of those was verified red before being kept. + * * ## What this spec cannot cover, stated rather than implied * * - **The service worker.** The Playwright context runs with @@ -104,6 +128,26 @@ test.describe("account sharing", () => { // three times against a rate-limited endpoint. test.describe.configure({ mode: "serial" }); + // The `page` fixture throughout is the DELEGATE. The owner gets an explicit + // context below, because a grant needs two signed-in sides at once and the + // login endpoint is IP-rate-limited. + test.use({ storageState: STORAGE_STATE_PATH }); + + // Desktop only, and for a reason that is about the fixture rather than the + // layout: this journey mutates shared rows — one grant between two seeded + // accounts — so two projects running it at once race each other, and the + // second one's invitation is refused as a duplicate of the first's. Running + // it once keeps every step's precondition true. The mobile bar reads the + // same destination model as the sidebar, and that agreement is a unit + // contract (`nav-model-shared-record.test.ts`) rather than something two + // browser runs of the same script would establish. + test.beforeEach(({}, testInfo) => { + test.skip( + testInfo.project.name !== "chromium-desktop", + "shared-fixture journey — runs once", + ); + }); + let ownerContext: BrowserContext; let ownerPage: Page; @@ -121,10 +165,19 @@ test.describe("account sharing", () => { test("the owner invites, and the invitation confers nothing yet", async () => { await ownerPage.goto("/settings/access"); - await ownerPage - .locator('[data-slot="grant-invite-identifier"]') - .fill(E2E_USER.username); - await ownerPage.locator('[data-slot="grant-invite-submit"]').click(); + // The submit button is disabled until the controlled input holds a value, + // so a `fill()` that lands before React has attached its listeners leaves + // the form looking filled and behaving empty. Retry the pair until the + // button agrees, rather than waiting a fixed time and hoping. + const identifier = ownerPage.locator( + '[data-slot="grant-invite-identifier"]', + ); + const submit = ownerPage.locator('[data-slot="grant-invite-submit"]'); + await expect(async () => { + await identifier.fill(E2E_USER.username); + await expect(submit).toBeEnabled({ timeout: 1000 }); + }).toPass({ timeout: 15_000 }); + await submit.click(); const row = ownerPage .locator('[data-slot="grants-given-list"] [data-slot="grant-row"]') @@ -198,8 +251,10 @@ test.describe("account sharing", () => { await expect(banner).toBeVisible(); await expect(banner).toContainText(E2E_OWNER.username); - // The latch was reset by the reload's init script, so this covers the - // whole life of the switched document. + // The document showing the banner has never shown the delegate's own + // number. The latch was armed by that document's own init script, so this + // covers its whole life rather than the instant it is read. See the + // header for what this line can and cannot be made to fail on. expect(await sawText(page)).toBe(false); // And it stays false across a navigation inside the record. @@ -211,6 +266,10 @@ test.describe("account sharing", () => { }); test("the nav offers only what sharing covers", async ({ page }) => { + // A fresh page fixture, and still inside the record: the switch lives on + // the session row rather than in the tab, so every tab of that browser is + // switched at once. That is the property this navigation quietly proves. + await page.goto("/"); await expect( page.locator('[data-slot="shared-record-banner"]'), ).toBeVisible(); diff --git a/e2e/setup/global-setup.ts b/e2e/setup/global-setup.ts index cba818515..12e1c1edc 100644 --- a/e2e/setup/global-setup.ts +++ b/e2e/setup/global-setup.ts @@ -259,6 +259,13 @@ export default async function globalSetup(config: FullConfig): Promise { WHERE key LIKE 'sharing:%'`, ); + // The login bucket, for the same reason. This setup signs in TWICE now + // (delegate and owner), and the ceiling is five attempts per IP per + // quarter-hour — so three local runs in a row used to end with a 429 from + // the fixture rather than a failure from the product. Only the auth + // surfaces' own buckets are cleared; nothing else in the table is touched. + await pool.query(`DELETE FROM rate_limits WHERE key LIKE 'auth:%'`); + // One marker weight per record. Re-seeded by delete-then-insert so a // re-run cannot accumulate duplicates and so an edited constant takes // effect rather than sitting beside the old value. diff --git a/src/components/layout/sidebar-nav.tsx b/src/components/layout/sidebar-nav.tsx index 030f8438b..bb86b5538 100644 --- a/src/components/layout/sidebar-nav.tsx +++ b/src/components/layout/sidebar-nav.tsx @@ -59,12 +59,51 @@ function getInitials(name: string): string { .join(""); } +/** + * The sidebar footer's name-and-avatar block. + * + * A link into the account settings normally, and a plain block while this + * browser is acting on somebody else's record — the settings routes refuse + * under a switch, so the link would lead only to an explanation of why it did + * not work. It keeps naming the person at the keyboard either way, which is + * what a switched session needs it to do. + */ +function FooterIdentity({ + sharedRecord, + label, + children, +}: { + sharedRecord: boolean; + label: string; + children: React.ReactNode; +}) { + const className = + "flex min-w-0 flex-1 items-center gap-3 rounded-md transition-colors"; + if (sharedRecord) { + return
{children}
; + } + return ( + + {children} + + ); +} + function SidebarUserSection({ collapsed }: { collapsed: boolean }) { const { user } = useAuth(); const logout = useLogout(); const { theme, setTheme } = useTheme(); const { t } = useTranslations(); const avatarUrl = user?.avatarUrl ?? null; + // v1.36.0 — while this browser is acting on somebody else's record the + // account surfaces are unreachable, so the avatar stops being a door into + // them. It still names the person at the keyboard, which is exactly what a + // switched session needs it to do. + const sharedRecord = user?.accountAccess?.active != null; if (!user) return null; @@ -77,6 +116,23 @@ function SidebarUserSection({ collapsed }: { collapsed: boolean }) { ); + const identityBlock = ( + <> + + {avatarUrl && } + + {getInitials(user.username)} + + +
+

{user.username}

+ {user.email && ( +

{user.email}

+ )} +
+ + ); + const dropdownContent = ( - - - {avatarUrl && } - - {getInitials(user.username)} - - -
-

{user.username}

- {user.email && ( -

- {user.email} -

- )} -
- - + {sharedRecord ? ( +
{identityBlock}
+ ) : ( + + + {identityBlock} + + + )} - - - - {t("nav.notifications")} - - + {!sharedRecord && ( + + + + {t("nav.notifications")} + + + )} {/* The about section lives at the end of the settings shell nav; the avatar menu stays focused on account-level actions. */} @@ -193,10 +242,9 @@ function SidebarUserSection({ collapsed }: { collapsed: boolean }) { {/* R20 — the avatar + name in the expanded footer routes straight into the account settings; the kebab to the right still opens the rest of the user menu. */} - {avatarUrl && } @@ -212,7 +260,7 @@ function SidebarUserSection({ collapsed }: { collapsed: boolean }) {

)} - + - - - - {t("nav.settings")} - - - - - - {t("nav.notifications")} - - - {isAdmin && ( - - - - {t("nav.admin")} - - + {/* v1.36.0 — the account utilities drop out while this browser is + acting on somebody else's record. Every route behind them + refuses under a switch, and a control that leads only to an + explanation of why it does not work is worse than no control. + The switcher below is how the person gets back. */} + {!sharedRecord && ( + <> + + + + {t("nav.settings")} + + + + + + {t("nav.notifications")} + + + {isAdmin && ( + + + + {t("nav.admin")} + + + )} + )} {/* v1.4.36 W4e — About moved into the Admin Console (`/admin/about`). The dropdown entry was redundant