From 42f7bd651d2e487ee68bd815761b1d371cce4893 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Wed, 19 Aug 2026 16:32:07 +0200 Subject: [PATCH] test(e2e): isolate runs and identities without a new backend endpoint (WP-74) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three specs shared one mutable backend and said so in their own comments ("Restart the backend between CI runs"). WP-70 recorded the fix as a dev-only seed endpoint; it isn't needed. The DB path already routes through IConfiguration, so playwright.config's webServer hands the backend a throwaway SQLite file per invocation — the same trick TestWebApplicationFactory already uses, with zero backend change. And StubIdentityProvider already honoured X-Subject; the only gap was that nothing sent it. That matters because the backend has no IsDevelopment() gate anywhere, so a seed endpoint would have had to invent the codebase's first environment gate. subjectInterceptor mirrors the existing roleInterceptor and is wired into the same isDevMode()-only list. Interceptors alone were not enough: the raw XHR upload and the hand-written letter-preview fetch bypass Angular's chain (as CLAUDE.md documents), so both now stamp X-Subject explicitly — without that, every uploaded document still landed under DemoOwner. reuseExistingServer stays on: flipping it would break local runs for anyone already serving the docker stack. Each run gets a unique DB filename and global-setup sweeps only prior runs' leftovers — deleting a fixed path mid-run risks SQLite silently recreating an empty, unmigrated file under fullyParallel. Verified: e2e passes twice back-to-back with no backend restart, and X-Subject was observed on a real request, not merely wired. brief-v2.spec.ts keeps the shared identity for now — see the KNOWN GAP note; a backend staleness bug makes /brief/preview return a sent letter with the draft watermark for any non-DemoOwner BSN. actors.ts reserves the actor for whoever fixes it. Co-Authored-By: Claude Sonnet 5 --- apps/behandelportal/src/app/app.config.ts | 5 +- apps/ssp/src/app/app.config.ts | 7 ++- .../infrastructure/letter-preview.adapter.ts | 27 ++++++++- e2e/brief-v2.spec.ts | 20 +++++-- e2e/global-setup.ts | 39 ++++++++++++ e2e/smoke.spec.ts | 13 ++-- e2e/support/actors.ts | 49 ++++++++++++--- .../subject.interceptor.spec.ts | 60 +++++++++++++++++++ .../src/infrastructure/subject.interceptor.ts | 35 +++++++++++ libs/shared/src/infrastructure/subject.ts | 30 ++++++++++ libs/shared/src/upload/upload.adapter.ts | 8 +++ playwright.config.ts | 47 +++++++++++++++ 12 files changed, 319 insertions(+), 21 deletions(-) create mode 100644 e2e/global-setup.ts create mode 100644 libs/shared/src/infrastructure/subject.interceptor.spec.ts create mode 100644 libs/shared/src/infrastructure/subject.interceptor.ts create mode 100644 libs/shared/src/infrastructure/subject.ts diff --git a/apps/behandelportal/src/app/app.config.ts b/apps/behandelportal/src/app/app.config.ts index bb59aa6..13ee858 100644 --- a/apps/behandelportal/src/app/app.config.ts +++ b/apps/behandelportal/src/app/app.config.ts @@ -14,6 +14,7 @@ import localeEn from '@angular/common/locales/en'; import { routes } from './app.routes'; import { scenarioInterceptor } from '@shared/infrastructure/scenario.interceptor'; import { roleInterceptor } from '@shared/infrastructure/role.interceptor'; +import { subjectInterceptor } from '@shared/infrastructure/subject.interceptor'; import { medewerkerInterceptor } from '@auth/infrastructure/medewerker.interceptor'; import { provideApiClient } from '@shared/infrastructure/api-client.provider'; import { SESSION_PORT } from '@shared/application/session.port'; @@ -55,7 +56,9 @@ export const appConfig: ApplicationConfig = { // a query param could otherwise force errors on the live app. provideHttpClient( withInterceptors( - isDevMode() ? [scenarioInterceptor, roleInterceptor, medewerkerInterceptor] : [], + isDevMode() + ? [scenarioInterceptor, roleInterceptor, subjectInterceptor, medewerkerInterceptor] + : [], ), ), provideApiClient(), diff --git a/apps/ssp/src/app/app.config.ts b/apps/ssp/src/app/app.config.ts index a0a528a..0bf2794 100644 --- a/apps/ssp/src/app/app.config.ts +++ b/apps/ssp/src/app/app.config.ts @@ -14,6 +14,7 @@ import localeEn from '@angular/common/locales/en'; import { routes } from './app.routes'; import { scenarioInterceptor } from '@shared/infrastructure/scenario.interceptor'; import { roleInterceptor } from '@shared/infrastructure/role.interceptor'; +import { subjectInterceptor } from '@shared/infrastructure/subject.interceptor'; import { provideApiClient } from '@shared/infrastructure/api-client.provider'; import { SESSION_PORT } from '@shared/application/session.port'; import { SessionStore } from '@auth/application/session.store'; @@ -54,7 +55,11 @@ export const appConfig: ApplicationConfig = { ), // Dev-only: the ?scenario= toggle must never reach a production build, where // a query param could otherwise force errors on the live app. - provideHttpClient(withInterceptors(isDevMode() ? [scenarioInterceptor, roleInterceptor] : [])), + provideHttpClient( + withInterceptors( + isDevMode() ? [scenarioInterceptor, roleInterceptor, subjectInterceptor] : [], + ), + ), provideApiClient(), { provide: SESSION_PORT, useExisting: SessionStore }, // Per-bundle locale: the localize build sets `$localize.locale` ('nl'/'en'); the diff --git a/apps/ssp/src/app/brief/infrastructure/letter-preview.adapter.ts b/apps/ssp/src/app/brief/infrastructure/letter-preview.adapter.ts index 1c3db05..7fc1f31 100644 --- a/apps/ssp/src/app/brief/infrastructure/letter-preview.adapter.ts +++ b/apps/ssp/src/app/brief/infrastructure/letter-preview.adapter.ts @@ -1,6 +1,7 @@ import { Injectable } from '@angular/core'; import { Result, ok, err } from '@shared/kernel/fp'; import { currentRole } from '@shared/infrastructure/role'; +import { currentSubject } from '@shared/infrastructure/subject'; import { problemDetail } from '@shared/infrastructure/api-error'; import { environment } from '@shared/environments/environment'; @@ -12,15 +13,37 @@ export const PREVIEW_FAILED = $localize`:@@brief.preview.failed:De voorvertoning * `/brief/preview` returns `text/html`, not JSON, and is `.ExcludeFromDescription()`'d * to keep the NSwag-generated client JSON-only (same seam as uploads) — so this is a * hand-written fetch, not the `ApiClient`. That also means it bypasses `HttpClient`'s - * `roleInterceptor`, so `X-Role` is set here explicitly. + * `roleInterceptor` AND `subjectInterceptor`, so both `X-Role` and `X-Subject` are set + * here explicitly (WP-74 — without `X-Subject` this always previewed + * `DocumentStore.DemoOwner`'s letter regardless of who was actually logged in). + * + * `cache: 'no-store'` (WP-74): the endpoint has no `Cache-Control`, only a CORS-driven + * `Vary: Origin`, and its content changes at the SAME URL as the letter moves + * draft → sent. Explicitly bypassing the HTTP cache is the correct default for any + * mutable resource served under one unversioned URL — independent of WP-74's + * identity work, and not a complete fix by itself: see the KNOWN GAP note below. + * + * KNOWN GAP (WP-74, not fixed here): under a non-`DocumentStore.DemoOwner` `X-Subject`, + * this repo's own e2e run against a real backend observed this endpoint's SENT + * response still carrying the draft watermark, even though (a) the outgoing request + * carried the correct `X-Subject`, and (b) `curl` against the same backend at the + * same moment correctly returned the frozen, unwatermarked archive. `cache: 'no-store'` + * did not change the outcome, so it is very unlikely a client-side caching artifact — + * it looks like a genuine backend-side staleness/race in `BriefStore`'s SQLite-backed + * read path, reproducible for MULTIPLE distinct owners and NOT reproducible for + * `DemoOwner`, which needs backend-side investigation (out of WP-74's file scope — + * see `e2e/brief-v2.spec.ts`'s header comment, which keeps that spec on the shared + * `zorgverlener` identity until this is root-caused). */ @Injectable({ providedIn: 'root' }) export class LetterPreviewAdapter { async preview(): Promise> { let res: Response; try { + const subject = currentSubject(); res = await fetch(`${environment.apiBaseUrl}/api/v1/brief/preview`, { - headers: { 'X-Role': currentRole() }, + cache: 'no-store', + headers: { 'X-Role': currentRole(), ...(subject ? { 'X-Subject': subject } : {}) }, }); } catch { return err(PREVIEW_FAILED); diff --git a/e2e/brief-v2.spec.ts b/e2e/brief-v2.spec.ts index 74dab99..25c8742 100644 --- a/e2e/brief-v2.spec.ts +++ b/e2e/brief-v2.spec.ts @@ -7,10 +7,22 @@ import { Actors, loginAs } from './support/actors'; // Preview assertions are content-type/body-level (text/html + watermark marker), not // pixel, per WP-28's decision. // -// The backend persists to SQLite (WP-22) and is shared across runs: `/brief/reset` -// covers the letter, but org templates have no reset endpoint, so this test restores -// the org-template draft it edits (step 8) and never asserts an absolute version -// number — only that it increased by exactly one. +// This test mutates real state (a letter, keyed per-owner by `BriefStore.GetOrCreate`), +// and WP-74 gives it a fresh throwaway backend DB every `npm run e2e` run, so a +// leftover/in-progress letter from a PREVIOUS RUN is never an issue any more. It +// deliberately still logs in as the shared `Actors.zorgverlener` rather than its own +// BSN, though: giving it a distinct BSN (as `smoke.spec.ts` does) hit a real, +// reproducible bug in this repo's own e2e run — `GET /brief/preview`'s sent-letter +// response kept the DRAFT watermark under a non-`DemoOwner` `X-Subject`, even though +// the outgoing request carried the right header and a direct `curl` against the same +// backend at the same instant returned the correct, frozen archive. That points to a +// backend-side staleness/race in `BriefStore`'s SQLite read path (see +// `letter-preview.adapter.ts`'s "KNOWN GAP" note), out of WP-74's file scope to fix — +// so this spec stays on the one identity that doesn't trip it, pending that backend +// investigation. The org-template appearance is a SEPARATE, already-known gap: it's +// NOT owner-keyed (there's exactly one, shared by every caller) and has no reset +// endpoint, so this test still restores the org-template draft it edits (step 8) and +// never asserts an absolute version number — only that it increased by exactly one. test('drafter composes → approver sends; admin republishes appearance', async ({ page }) => { await loginAs(page, Actors.zorgverlener); await expect(page).toHaveURL(/\/dashboard$/); diff --git a/e2e/global-setup.ts b/e2e/global-setup.ts new file mode 100644 index 0000000..1fb7d92 --- /dev/null +++ b/e2e/global-setup.ts @@ -0,0 +1,39 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +/** + * WP-74: `playwright.config.ts` gives the backend a UNIQUE throwaway SQLite file + * per `npm run e2e` invocation (`/big-register-e2e--.db`), + * so Playwright has no webServer teardown hook to delete it once a run ends — this + * sweeps them up instead, at the START of the NEXT run. + * + * Safe regardless of run ordering: `globalSetup` always executes AFTER `webServer` + * has already started (Playwright's task order, not something a config can flip), + * so THIS run's own file (`process.env['E2E_DB_PATH']`, set by the config module — + * same node process, so the assignment is visible here) is always excluded. Every + * OTHER matching file belongs to an invocation whose `dotnet run` process has + * already exited, so deleting it can't race a live connection. + */ +export default function globalSetup(): void { + const mine = process.env['E2E_DB_PATH']; + const dir = os.tmpdir(); + let entries: string[]; + try { + entries = fs.readdirSync(dir); + } catch { + return; // best-effort cleanup — a missing/unreadable temp dir isn't this run's problem + } + + for (const name of entries) { + if (!name.startsWith('big-register-e2e-')) continue; + const base = name.replace(/(-shm|-wal)$/, ''); + if (mine && base === path.basename(mine)) continue; // never this run's own file + try { + fs.unlinkSync(path.join(dir, name)); + } catch { + // best-effort — a file another leftover process still has open, or already + // gone, is not worth failing this run's e2e suite over. + } + } +} diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts index 9c4343d..1a810f9 100644 --- a/e2e/smoke.spec.ts +++ b/e2e/smoke.spec.ts @@ -6,13 +6,14 @@ import { Actors, loginAs, SeedRefs } from './support/actors'; // with zero policy questions, so the only required upload is identiteit), submit, // and see the real confirmation. Not a full wizard-coverage suite — see WP-19. // -// The backend is in-memory and shared across runs; this test mutates real state -// (creates a registratie application for the fixed demo identity). Restart the -// backend between CI runs — a second run would see a leftover Concept/submitted -// application on the dashboard, which this test doesn't assert against, but a -// stricter future test might. +// This test mutates real state (creates+submits a registratie application), so it +// logs in as its own BSN (`registratieAanvrager`, WP-74) rather than the shared +// `zorgverlener` — a leftover Concept from a previous run lands on THAT BSN's +// dashboard, not this one's, so a rerun (or another spec) never sees it. The +// backend itself also gets a fresh throwaway SQLite file per `npm run e2e` +// invocation (`playwright.config.ts`), so even a from-scratch run starts clean. test('login → dashboard → registratie wizard → submitted', async ({ page }) => { - await loginAs(page, Actors.zorgverlener); + await loginAs(page, Actors.registratieAanvrager); await expect(page).toHaveURL(/\/dashboard$/); await expect(page.getByRole('heading', { level: 1, name: 'Mijn overzicht' })).toBeVisible(); diff --git a/e2e/support/actors.ts b/e2e/support/actors.ts index 8b9886d..33e6d17 100644 --- a/e2e/support/actors.ts +++ b/e2e/support/actors.ts @@ -11,19 +11,54 @@ export interface Actor { } /** - * The demo identities seeded by the backend. Today there is exactly one seeded - * citizen (`backend/src/BigRegister.Api/Data/SeedData.cs`'s `Person`/`Registration`, - * whose BSN also matches `DocumentStore.DemoOwner`) — named for the ROLE it plays - * in a spec, not its BSN, so a spec reads as "log in as the zorgverlener", not - * "log in as 123456782". + * The demo identities e2e specs log in as. The BRP/registration data every one of + * them sees on the dashboard comes from static `SeedData` and is identity-independent + * (only `DocumentStore.DemoOwner`'s owner-keyed Applications/Documents/Briefs differ + * per BSN — see `subject.interceptor.ts`), so any elfproef-valid BSN works here; these + * are just distinct, not otherwise special. + * + * Named for the ROLE the identity plays in a spec, not its BSN, so a spec reads as + * "log in as the zorgverlener", not "log in as 123456782". **A spec that mutates + * owner-keyed state (creates a Concept, composes a brief, …) should use its own + * BSN** (WP-74) — `subject.interceptor.ts` stamps it as `X-Subject`, so two specs + * sharing a BSN would collide on the same backend rows across the same run and + * across reruns. A read-only spec (nothing created/submitted) can keep using + * `zorgverlener`. + * + * `briefOpsteller` is the one exception, currently unused: `brief-v2.spec.ts` stays + * on `zorgverlener` despite mutating state, because giving it its own BSN tripped a + * real backend bug (a stale/watermarked `GET /brief/preview` response for the SENT + * letter under a non-`DemoOwner` owner — see that spec's header comment and + * `letter-preview.adapter.ts`'s "KNOWN GAP" note). Kept defined, not deleted, so + * whoever fixes that backend issue has the identity ready to switch the spec onto. + * + * Every BSN below passed the elfproef (`libs/shared/src/kernel/bsn.ts`'s checksum) — + * required, or `DigidAdapter.authenticate` rejects it and login never completes: + * 123456782 ✓ (9·1+8·2+7·3+6·4+5·5+4·6+3·7+2·8−1·2 = 154, 154 mod 11 = 0) + * 111222333 ✓ (9·1+8·1+7·1+6·2+5·2+4·2+3·3+2·3−1·3 = 66, 66 mod 11 = 0) + * 111111110 ✓ (9+8+7+6+5+4+3+2−0 = 44, 44 mod 11 = 0) */ export const Actors = { + /** The one seeded citizen (`SeedData.cs`'s `Person`/`Registration`) — read-only + specs, and (for now — see above) `brief-v2.spec.ts` too. */ zorgverlener: { bsn: '123456782', wachtwoord: 'demo' }, + /** `smoke.spec.ts`'s own identity — it creates+submits a registratie-aanvraag. */ + registratieAanvrager: { bsn: '111222333', wachtwoord: 'demo' }, + /** Reserved for `brief-v2.spec.ts` once the backend bug above is fixed — not + currently used by any spec. */ + briefOpsteller: { bsn: '111111110', wachtwoord: 'demo' }, } as const satisfies Record; -/** The shared DigiD-style mock login sequence every e2e spec starts from. */ +/** + * The shared DigiD-style mock login sequence every e2e spec starts from. Navigating + * to `/login?subject=` (rather than plain `/login`) primes `subject.interceptor.ts`'s + * sticky sessionStorage the same way `?role=` primes `roleInterceptor` (WP-33) — the + * BSN typed into the form and the one the interceptor stamps as `X-Subject` are the + * same value by construction, they just can't share a single read (see + * `subject.interceptor.ts`'s doc comment for why not). + */ export async function loginAs(page: Page, actor: Actor): Promise { - await page.goto('/login'); + await page.goto(`/login?subject=${actor.bsn}`); await page.getByLabel('BSN').fill(actor.bsn); await page.getByLabel('Wachtwoord').fill(actor.wachtwoord); await page.getByRole('button', { name: 'Inloggen met DigiD' }).click(); diff --git a/libs/shared/src/infrastructure/subject.interceptor.spec.ts b/libs/shared/src/infrastructure/subject.interceptor.spec.ts new file mode 100644 index 0000000..6a64352 --- /dev/null +++ b/libs/shared/src/infrastructure/subject.interceptor.spec.ts @@ -0,0 +1,60 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { subjectInterceptor } from './subject.interceptor'; + +// currentSubject() reads window.location.search; set it via the real URL rather than +// vi.mock (the Angular unit-test system forbids mocking relative imports). +afterEach(() => { + window.history.replaceState({}, '', '/'); + sessionStorage.clear(); // currentSubject() persists across calls; don't leak across tests +}); + +// Minimal stand-in for HttpRequest — the interceptor only reads `url` and calls +// `clone({ setHeaders })`. Avoids importing @angular/common/http (its XHR chunk needs +// the JIT compiler under vitest). +function fakeReq(url: string) { + const make = (headers: Map) => ({ + url, + headers, + clone(opts: { setHeaders: Record }) { + const next = new Map(headers); + for (const [k, v] of Object.entries(opts.setHeaders)) next.set(k, v); + return make(next); + }, + }); + return make(new Map()); +} + +/** Run the interceptor and return the request it forwarded to `next`. */ +function forward(url: string) { + let seen!: ReturnType; + const next = (r: ReturnType) => { + seen = r; + return undefined; + }; + // Cast: the fake matches the shape the interceptor actually touches. + (subjectInterceptor as unknown as (req: unknown, next: unknown) => unknown)(fakeReq(url), next); + return seen; +} + +describe('subjectInterceptor', () => { + it('stamps X-Subject on an /api/v1/ request once ?subject= has been seen', () => { + window.history.replaceState({}, '', '/?subject=111222333'); + expect(forward('/api/v1/registratie/concept').headers.get('X-Subject')).toBe('111222333'); + }); + + it('keeps stamping later requests on the same tab after the query param is gone (WP-33-style stickiness)', () => { + window.history.replaceState({}, '', '/?subject=111222333'); + forward('/api/v1/me'); + window.history.replaceState({}, '', '/dashboard'); // navigation drops the query param + expect(forward('/api/v1/me').headers.get('X-Subject')).toBe('111222333'); + }); + + it('leaves a non-API request untouched even when a subject is known', () => { + window.history.replaceState({}, '', '/?subject=111222333'); + expect(forward('/assets/logo.svg').headers.has('X-Subject')).toBe(false); + }); + + it('sends no header at all when no subject has ever been seen', () => { + expect(forward('/api/v1/me').headers.has('X-Subject')).toBe(false); + }); +}); diff --git a/libs/shared/src/infrastructure/subject.interceptor.ts b/libs/shared/src/infrastructure/subject.interceptor.ts new file mode 100644 index 0000000..9890495 --- /dev/null +++ b/libs/shared/src/infrastructure/subject.interceptor.ts @@ -0,0 +1,35 @@ +import { HttpInterceptorFn } from '@angular/common/http'; +import { currentSubject } from './subject'; + +/** + * Dev-only (WP-74): stamps every API request with `X-Subject`, the BSN + * `StubIdentityProvider` (backend) resolves the caller's `ZorgverlenerCaller` from — + * every owner-keyed store (`ApplicationStore`, `DocumentStore`, `BriefStore`) reads + * off that resolved identity, so this is the seam that lets e2e specs log in as + * distinct citizens and mutate independent rows instead of all colliding on + * `DocumentStore.DemoOwner`. Scoped like `medewerker.interceptor.ts` (every + * `/api/v1/*` request, not an allow-list like `roleInterceptor`) — the identity + * middleware resolves a `CallerIdentity` for every request, not just some endpoints. + * + * **BSN source — a deliberate compromise, read before changing:** the "obvious" + * source would be the authenticated `Session.bsn` held by each app's own + * `SessionStore`, but `libs/shared` may not depend on an app-local `auth` context + * (the import-direction rule), and the one sanctioned cross-context seam — + * `SessionPort` (`@shared/application/session.port`) — deliberately exposes only + * `{ naam }`: `SessionStore`'s G1 comment is explicit that the BSN (a GDPR + * special-category identifier) is never persisted or otherwise handed outward, by + * design. Extending that port (or injecting `SessionStore` itself) would undo that + * boundary just to serve a dev/e2e convenience. So instead this reuses + * `role.interceptor.ts`'s own trick (see `subject.ts`, mirroring `role.ts`'s + * `currentRole()`): a `?subject=` seen in the URL is remembered in sessionStorage + * for the tab, and every later request reuses it. `e2e/support/actors.ts`'s + * `loginAs` sets it once per spec by navigating to `/login?subject=` before + * filling in the login form. Outside e2e nothing ever sets `?subject=`, so no + * header is sent and the backend falls back to `DocumentStore.DemoOwner` exactly as + * before this WP. + */ +export const subjectInterceptor: HttpInterceptorFn = (req, next) => { + const subject = currentSubject(); + if (!subject || !req.url.includes('/api/v1/')) return next(req); + return next(req.clone({ setHeaders: { 'X-Subject': subject } })); +}; diff --git a/libs/shared/src/infrastructure/subject.ts b/libs/shared/src/infrastructure/subject.ts new file mode 100644 index 0000000..5556d87 --- /dev/null +++ b/libs/shared/src/infrastructure/subject.ts @@ -0,0 +1,30 @@ +/** + * Dev-only role stand-in's sibling (the reading MECHANISM for `X-Subject`; see + * `role.ts`'s own doc comment for the twin `X-Role` mechanism this mirrors). This + * POC has no real DigiD identity — `Session.bsn` lives only in each app's own + * in-memory `SessionStore` and is deliberately never persisted (see that store's G1 + * comment) — so `subject.interceptor.ts` can't reach it without a layering + * violation (`libs/shared` may not depend on an app-local `auth` context). Instead a + * `?subject=` query param, seen once on any navigation, is remembered for the + * tab in sessionStorage — the exact `?role=` trick `role.ts` already uses (WP-33). + * + * Two consumers read this, both dev/e2e-only: `subject.interceptor.ts` (every + * `HttpClient` request) and `letter-preview.adapter.ts` (`/brief/preview`'s + * hand-written `fetch`, which bypasses every `HttpInterceptorFn` — the same reason + * that adapter already sets `X-Role` explicitly via `currentRole()`). + * + * `undefined` (not a default BSN) when nothing has ever set `?subject=`: unlike + * `currentRole()` (a closed enum with a sensible default), there is no "default + * subject" to fall back to here — omitting the header entirely lets the backend's + * own default (`DocumentStore.DemoOwner`) apply, exactly as if this WP didn't exist. + */ +const STORAGE_KEY = 'dev-subject'; + +export function currentSubject(): string | undefined { + const fromUrl = new URLSearchParams(window.location.search).get('subject'); + if (fromUrl) { + sessionStorage.setItem(STORAGE_KEY, fromUrl); + return fromUrl; + } + return sessionStorage.getItem(STORAGE_KEY) ?? undefined; +} diff --git a/libs/shared/src/upload/upload.adapter.ts b/libs/shared/src/upload/upload.adapter.ts index 041967c..a0d1620 100644 --- a/libs/shared/src/upload/upload.adapter.ts +++ b/libs/shared/src/upload/upload.adapter.ts @@ -6,6 +6,7 @@ import { } from '@shared/infrastructure/api-client'; import { problemDetail } from '@shared/infrastructure/api-error'; import { currentScenario } from '@shared/infrastructure/scenario'; +import { currentSubject } from '@shared/infrastructure/subject'; import { environment } from '@shared/environments/environment'; import { DocumentCategory } from './upload.machine'; @@ -144,6 +145,13 @@ export class UploadAdapter { }); xhr.open('POST', `${environment.apiBaseUrl}/api/v1/uploads`); + // WP-74: this XHR bypasses `HttpClient`'s `subjectInterceptor` (the same reason + // `letter-preview.adapter.ts` sets `X-Role` explicitly) — without `X-Subject` a + // document always uploaded under `DocumentStore.DemoOwner` regardless of who was + // actually logged in, so a submission attempted under any other BSN would find + // its own required document "missing" (owned by someone else). + const subject = currentSubject(); + if (subject) xhr.setRequestHeader('X-Subject', subject); xhr.send(form); return { done, cancel: () => ((aborted = true), xhr.abort()) }; } diff --git a/playwright.config.ts b/playwright.config.ts index 0106bc7..9ced5b6 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,15 +1,48 @@ import { defineConfig } from '@playwright/test'; +import * as os from 'node:os'; +import * as path from 'node:path'; // Smoke-level e2e (WP-19): one happy path, one degraded path, against the REAL // backend — proving the FE+BE seam, not replacing component/unit tests. const baseURL = process.env['E2E_BASE_URL'] ?? 'http://localhost:4200'; +// WP-74: give the backend a THROWAWAY SQLite file per `npm run e2e` invocation +// instead of its default `bigregister.db`, so two separate runs never see each +// other's leftover Applications/Documents/Briefs (`Program.cs:44-46` already reads +// `ConnectionStrings__AppDb` from `IConfiguration` — zero backend change; the same +// trick `TestWebApplicationFactory.cs` uses per xUnit test class). `Program.cs:93` +// self-migrates a fresh file on startup, so a brand-new path is immediately usable. +// +// UNIQUE filename per invocation (pid + timestamp), not a single fixed name: a fixed +// name that gets deleted WHILE the backend still has it open only stays safe if +// Microsoft.Data.Sqlite's connection pool never needs to open a fresh native handle +// by path after the delete — under `fullyParallel: true` (several workers hitting +// the backend concurrently) a pool miss is a real, if intermittent, risk, and it +// would silently recreate an empty, unmigrated file mid-run (every query after that +// would 500 on "no such table"). A unique path sidesteps the whole question: nothing +// ever deletes the file THIS run is actively using. The tradeoff is litter in the OS +// temp dir across many runs, since Playwright has no webServer teardown hook to +// delete it once the run ends — `globalSetup` (below) sweeps prior runs' files at +// the start of each new run instead. That's safe regardless of ordering: it only +// ever touches OTHER runs' paths, whose processes have already exited. +const dbPath = path.join(os.tmpdir(), `big-register-e2e-${process.pid}-${Date.now()}.db`); +// Shared with `e2e/global-setup.ts`, which runs in this SAME node process (it's +// loaded and invoked by the Playwright runner right after `webServer` comes up, not +// spawned separately) — so this assignment is visible there without a second import. +process.env['E2E_DB_PATH'] = dbPath; + export default defineConfig({ testDir: './e2e', timeout: 30_000, fullyParallel: true, retries: process.env['CI'] ? 1 : 0, reporter: process.env['CI'] ? 'github' : 'list', + // Sweeps `big-register-e2e-*.db*` files left behind by EARLIER invocations (never + // this run's own `dbPath` — see its comment above). Runs after `webServer` is + // already up (Playwright always starts webServer before globalSetup — there's no + // config knob to reverse that), which is exactly why it must never touch the + // current run's own file. + globalSetup: './e2e/global-setup.ts', use: { baseURL, trace: 'on-first-retry', @@ -27,8 +60,22 @@ export default defineConfig({ { command: 'dotnet run --project backend/src/BigRegister.Api --urls http://localhost:5000', url: 'http://localhost:5000/swagger', + // WP-74 caveat, read before "fixing" this to `false`: this stays `!CI` (reuse + // locally) on purpose, matching the FE server below and the docker-stack note + // above it — turning reuse off would make `npm run e2e` hard-fail with + // "port already in use" for anyone who already has the docker stack (or a + // plain `dotnet run`) up on :5000, a real local-workflow regression for a POC + // convenience feature. The tradeoff this buys: `env` below (the throwaway DB) + // ONLY applies when Playwright itself spawns the process — reusing an + // already-running backend silently falls back to THAT process's own DB + // (typically the shared dev `bigregister.db`), so the isolation this WP adds + // is real in CI (`reuseExistingServer` is always `false` there) and in the + // common local case of "nothing was already running on :5000", but not if you + // deliberately point e2e at an already-running shared backend — that was + // already shared state before this WP and still is. reuseExistingServer: !process.env['CI'], timeout: 180_000, + env: { ConnectionStrings__AppDb: `Data Source=${dbPath}` }, }, { command: 'npm start',