diff --git a/apps/ssp/src/app/brief/application/brief.store.spec.ts b/apps/ssp/src/app/brief/application/brief.store.spec.ts index b96941a..2a493f8 100644 --- a/apps/ssp/src/app/brief/application/brief.store.spec.ts +++ b/apps/ssp/src/app/brief/application/brief.store.spec.ts @@ -1,6 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { describe, it, expect, vi, afterEach } from 'vitest'; import { Result } from '@shared/kernel/fp'; +import { BLOB_PRESENTER, BlobPresenter } from '@shared/application/blob-presenter'; import { Brief, BriefDecisions, CaseContext, LetterBlock } from '@brief/domain/brief'; import { OrgTemplate } from '@brief/domain/org-template'; import { @@ -53,8 +54,26 @@ const caseContext: CaseContext = { const view: BriefView = { brief, availablePassages: [], decisions, orgTemplate, caseContext }; -function setup(adapter: Partial): BriefStore { - TestBed.configureTestingModule({ providers: [{ provide: BriefAdapter, useValue: adapter }] }); +/** A recording fake of BLOB_PRESENTER (RB-28/TE-006) — records every call instead of + touching the DOM, so a spec can assert a command's success path directly. */ +function fakeBlobPresenter() { + const opened: Blob[] = []; + const presenter: BlobPresenter = { + open: (blob) => opened.push(blob), + download: () => { + throw new Error('not used by BriefStore'); + }, + }; + return { presenter, opened }; +} + +function setup(adapter: Partial, blobPresenter?: BlobPresenter): BriefStore { + TestBed.configureTestingModule({ + providers: [ + { provide: BriefAdapter, useValue: adapter }, + ...(blobPresenter ? [{ provide: BLOB_PRESENTER, useValue: blobPresenter }] : []), + ], + }); return TestBed.inject(BriefStore); } @@ -287,43 +306,46 @@ describe('BriefStore rejection diff', () => { }); describe('BriefStore.previewLetter', () => { - // vi.spyOn reuses an existing spy (and its call history) if one is already on - // the property — window.open/URL.createObjectURL must be restored between tests. afterEach(() => vi.restoreAllMocks()); - it('opens the composed letter in a new tab on success', async () => { - const store = setup({ - load: (): Promise> => - Promise.resolve({ ok: true, value: view }), - }); + it('opens the composed letter via BLOB_PRESENTER on success (RB-28)', async () => { + const { presenter, opened } = fakeBlobPresenter(); + const store = setup( + { + load: (): Promise> => + Promise.resolve({ ok: true, value: view }), + }, + presenter, + ); await store.load(); const blob = new Blob([''], { type: 'text/html' }); - vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:mock'); - const open = vi.spyOn(window, 'open').mockImplementation(() => null); vi.spyOn(TestBed.inject(LetterPreviewAdapter), 'preview').mockResolvedValue({ ok: true, value: blob, }); await store.previewLetter(); - expect(open).toHaveBeenCalledWith('blob:mock', '_blank'); + expect(opened).toEqual([blob]); expect(store.lastError()).toBeNull(); }); it('surfaces the error without opening a tab on failure', async () => { - const store = setup({ - load: (): Promise> => - Promise.resolve({ ok: true, value: view }), - }); + const { presenter, opened } = fakeBlobPresenter(); + const store = setup( + { + load: (): Promise> => + Promise.resolve({ ok: true, value: view }), + }, + presenter, + ); await store.load(); - const open = vi.spyOn(window, 'open').mockImplementation(() => null); vi.spyOn(TestBed.inject(LetterPreviewAdapter), 'preview').mockResolvedValue({ ok: false, error: PREVIEW_FAILED, }); await store.previewLetter(); - expect(open).not.toHaveBeenCalled(); + expect(opened).toHaveLength(0); expect(store.lastError()).toBe(PREVIEW_FAILED); }); }); diff --git a/apps/ssp/src/app/brief/application/brief.store.ts b/apps/ssp/src/app/brief/application/brief.store.ts index 623535a..80ab564 100644 --- a/apps/ssp/src/app/brief/application/brief.store.ts +++ b/apps/ssp/src/app/brief/application/brief.store.ts @@ -21,6 +21,7 @@ import { LetterPreviewAdapter } from '@brief/infrastructure/letter-preview.adapt import { RevealBigNummerAdapter } from '@brief/infrastructure/reveal-bignummer.adapter'; import { uploadContentUrl } from '@shared/infrastructure/upload.adapter'; import { PendingSave, registerPendingSave } from '@shared/application/pending-saves'; +import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; /** * Root singleton for the letter: the Elm store (Model + dispatch), the derived @@ -35,6 +36,7 @@ export class BriefStore implements PendingSave { private adapter = inject(BriefAdapter); private previewAdapter = inject(LetterPreviewAdapter); private revealAdapter = inject(RevealBigNummerAdapter); + private blobPresenter = inject(BLOB_PRESENTER); private store = createStore(initial, reduce); readonly model = this.store.model; @@ -244,8 +246,8 @@ export class BriefStore implements PendingSave { send = () => this.transition(() => this.adapter.send()); /** Explicit action, never a live re-render (PRD §8): opens the server-composed - letter in a new tab. ponytail: the blob URL is never revoked — it's cheap and - the tab outlives this call; not worth a teardown hook for a POC. */ + letter in a new tab via `BLOB_PRESENTER.open` — see its doc comment for why the + object URL is never revoked. */ async previewLetter() { this.actionState.set({ tag: 'Busy' }); const r = await this.previewAdapter.preview(); @@ -254,7 +256,7 @@ export class BriefStore implements PendingSave { return; } this.actionState.set({ tag: 'Idle' }); - window.open(URL.createObjectURL(r.value), '_blank'); + this.blobPresenter.open(r.value); } /** Reveal the masked case BIG-nummer (PRD-0002 §5c). Server re-checks the capability diff --git a/apps/ssp/src/app/brief/application/org-template.store.spec.ts b/apps/ssp/src/app/brief/application/org-template.store.spec.ts new file mode 100644 index 0000000..f5decef --- /dev/null +++ b/apps/ssp/src/app/brief/application/org-template.store.spec.ts @@ -0,0 +1,120 @@ +import { TestBed } from '@angular/core/testing'; +import { describe, it, expect } from 'vitest'; +import { Result, ok } from '@shared/kernel/fp'; +import { BLOB_PRESENTER, BlobPresenter } from '@shared/application/blob-presenter'; +import { UploadAdapter } from '@shared/infrastructure/upload.adapter'; +import { UploadShellService } from '@shared/application/upload-shell.service'; +import { OrgTemplate, OrgTemplateAdminView, SubOrgSummary } from '@brief/domain/org-template'; +import { OrgTemplateAdapter } from '@brief/infrastructure/org-template.adapter'; +import { OrgTemplateStore } from './org-template.store'; + +const template: OrgTemplate = { + subOrgId: 'cibg-registers', + orgName: 'CIBG — Registers', + returnAddress: 'Postbus 00000\n2500 AA Den Haag', + footerContact: 'info@voorbeeld.example', + footerLegal: 'KvK 00000000', + signatureName: 'A. de Vries', + signatureRole: 'Hoofd Registratie', + signatureClosing: 'Met vriendelijke groet,', + margins: { topMm: 25, rightMm: 25, bottomMm: 25, leftMm: 25 }, + version: 1, +}; + +const view: OrgTemplateAdminView = { + draft: template, + publishedVersion: 1, + history: [], + unsentBriefs: 0, +}; + +const subOrgs: SubOrgSummary[] = [ + { subOrgId: 'cibg-registers', orgName: 'CIBG', publishedVersion: 1 }, +]; + +/** A recording fake of BLOB_PRESENTER (RB-28/TE-006) — records every call instead of + touching the DOM, so a spec can assert a command's success path directly. */ +function fakeBlobPresenter() { + const opened: Blob[] = []; + const presenter: BlobPresenter = { + open: (blob) => opened.push(blob), + download: () => { + throw new Error('not used by OrgTemplateStore'); + }, + }; + return { presenter, opened }; +} + +/** A no-op categories resource: the logo-upload sub-state is untouched by these + tests, so 'idle' (never resolved) keeps the constructor effect from dispatching. */ +function fakeCategoriesResource(): ReturnType { + const fake = { status: () => 'idle' as const, value: () => undefined }; + return fake as unknown as ReturnType; +} + +function setup( + adapter: Partial, + blobPresenter: BlobPresenter, +): OrgTemplateStore { + const uploadAdapter: Partial = { + categoriesResource: () => fakeCategoriesResource(), + }; + TestBed.configureTestingModule({ + providers: [ + { provide: OrgTemplateAdapter, useValue: adapter }, + { provide: UploadAdapter, useValue: uploadAdapter }, + { provide: UploadShellService, useValue: {} }, + { provide: BLOB_PRESENTER, useValue: blobPresenter }, + ], + }); + return TestBed.inject(OrgTemplateStore); +} + +// --- RB-28 (TE-006): proefbrief() ends in BLOB_PRESENTER.open, not a raw +// window.open(URL.createObjectURL(...)) call, so both outcomes are assertable. --- + +describe('OrgTemplateStore.proefbrief (RB-28)', () => { + it('opens the rendered proefbrief via BLOB_PRESENTER on success', async () => { + // Given a loaded sub-org template. + const { presenter, opened } = fakeBlobPresenter(); + const blob = new Blob([''], { type: 'text/html' }); + const store = setup( + { + list: (): Promise> => Promise.resolve(ok(subOrgs)), + load: (): Promise> => Promise.resolve(ok(view)), + proefbrief: (): Promise> => Promise.resolve(ok(blob)), + }, + presenter, + ); + await store.load(); + + // When proefbrief() is called... + await store.proefbrief(); + + // Then the presenter receives exactly the rendered blob, and no error surfaces. + expect(opened).toEqual([blob]); + expect(store.lastError()).toBeNull(); + }); + + it('surfaces the error without opening a tab on failure', async () => { + // Given a loaded sub-org template whose proefbrief call fails server-side. + const { presenter, opened } = fakeBlobPresenter(); + const store = setup( + { + list: (): Promise> => Promise.resolve(ok(subOrgs)), + load: (): Promise> => Promise.resolve(ok(view)), + proefbrief: (): Promise> => + Promise.resolve({ ok: false, error: 'mislukt' }), + }, + presenter, + ); + await store.load(); + + // When proefbrief() is called... + await store.proefbrief(); + + // Then the presenter is never reached and the error is surfaced. + expect(opened).toHaveLength(0); + expect(store.lastError()).toBe('mislukt'); + }); +}); diff --git a/apps/ssp/src/app/brief/application/org-template.store.ts b/apps/ssp/src/app/brief/application/org-template.store.ts index c78213f..67bb1fe 100644 --- a/apps/ssp/src/app/brief/application/org-template.store.ts +++ b/apps/ssp/src/app/brief/application/org-template.store.ts @@ -20,6 +20,7 @@ import { } from '@brief/domain/org-template.machine'; import { OrgTemplateAdapter } from '@brief/infrastructure/org-template.adapter'; import { PendingSave, registerPendingSave } from '@shared/application/pending-saves'; +import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; type LoadedState = Extract; @@ -38,6 +39,7 @@ export class OrgTemplateStore implements PendingSave { private adapter = inject(OrgTemplateAdapter); private uploadAdapter = inject(UploadAdapter); private shell = inject(UploadShellService); + private blobPresenter = inject(BLOB_PRESENTER); private store = createStore(initial, reduce); readonly model = this.store.model; @@ -217,7 +219,7 @@ export class OrgTemplateStore implements PendingSave { return; } this.actionState.set({ tag: 'Idle' }); - window.open(URL.createObjectURL(r.value), '_blank'); + this.blobPresenter.open(r.value); } // --- logo upload (reuses the shared upload transport; single `org-logo` file) --- diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md index c888748..265fa41 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md @@ -129,7 +129,7 @@ Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authorita | **RB-25** | libs/shared/upload | testability | `UPLOAD_TRANSPORT` injection token (the `SESSION_PORT` shape) instead of `inject(KeepaliveTransport)` | §3a upload 52.0%/50.0%; §3b file unreached, non-`ui/` | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | **done** | | **RB-26** | libs/shared/upload | testability | Move the accept/reject decision to `planFileSelection` in `upload.machine.ts` | §3a upload 52.0%/50.0%; §4a module max CC 27 | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | **done** | | **RB-27** | libs/shared/upload | testability | Extract `uploadOutcome(status, responseText)` out of the XHR closure | file LH 5/64 (**7.8% line**), BRH 3/57 (**5.3% branch**) | S–M | Low | P2 | 5 | RB-25 | **SIGN-OFF** | open | -| **RB-28** | libs/beheer + ssp/brief | testability | `BLOB_PRESENTER` token; the 3 commands' success paths become assertable | §3a beheer/application **40.5% branch — worst FE**; brief.store BRH 32/64 | S–M | Low | P2 | 5 | — | **SIGN-OFF** | open | +| **RB-28** | libs/beheer + ssp/brief | testability | `BLOB_PRESENTER` token; the 3 commands' success paths become assertable | §3a beheer/application **40.5% branch — worst FE**; brief.store BRH 32/64 | S–M | Low | P2 | 5 | — | **SIGN-OFF** | **done** | | **RB-29** | backend/Domain | testability | Thread the existing `at` through `LetterHtml.ResolveAuto` instead of reading `UtcNow` | §3c Domain 82.0% branch; §4b `LetterHtml.cs` CC 21 | S | Low | P2 | 5 | — | — | **done** | | **RB-30** | backend/Data + Domain | testability | Extract 5 brief guards into `Domain/Letters/BriefRules.cs`; add `tests/Domain/BriefRuleTests.cs` | §3c Data **75.5% branch** (BL-005); §4b `BriefStore.cs` CC 17, `ToDto` CC 16 | M | Med | P2 | 5 | — | **SIGN-OFF** | **done** | | **RB-31** | 4 app contexts (specs only) | ADR conform. | Replace hand-rolled state literals with `given(reduce, initial)` replays in 4 machine specs | §7 Elm machines 9 (1 has a `*.testing.ts`); §3a herreg 67.8% / brief 68.8% branch | M | Low | P2 | 6 | — | — | open | diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-28.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-28.md new file mode 100644 index 0000000..a915435 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-28.md @@ -0,0 +1,180 @@ +# RB-28 — `BLOB_PRESENTER` token unlocks the three blob-to-browser success paths + +Status: **implemented** · 2026-08-28 · Source finding: `02-testability.md` TE-006 · +`99-backlog.md` RB-28 + +## What was wrong + +Three application-layer commands each ended in raw DOM/browser calls that jsdom cannot +meaningfully execute: `StamdataStore.download()` +(`libs/beheer/src/application/stamdata.store.ts`) did `URL.createObjectURL` → +`document.createElement('a')` → `a.click()` → `URL.revokeObjectURL`; +`BriefStore.previewLetter()` (`apps/ssp/src/app/brief/application/brief.store.ts`) and +`OrgTemplateStore.proefbrief()` (`apps/ssp/src/app/brief/application/org-template.store.ts`) +both did `window.open(URL.createObjectURL(blob), '_blank')`. Because the call was the +last statement of each command, TE-006 recorded the whole success path as effectively +unassertable, and `download()`'s two-clause guard (`if (!s || !this.canDownload()) +return;`) as permanently dark on its true branch. + +## What changed + +One new file, `libs/shared/src/application/blob-presenter.ts`, mirroring the +`SESSION_PORT` token already in that folder — an interface, a production +implementation, and an `InjectionToken`: + +```ts +export interface BlobPresenter { + open(blob: Blob): void; + download(blob: Blob, filename: string): void; +} + +const realBlobPresenter: BlobPresenter = { + open(blob) { + window.open(URL.createObjectURL(blob), '_blank'); + }, + download(blob, filename) { + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = filename; + a.click(); + URL.revokeObjectURL(url); + }, +}; + +export const BLOB_PRESENTER = new InjectionToken('BLOB_PRESENTER', { + providedIn: 'root', + factory: () => realBlobPresenter, +}); +``` + +`open()` never revokes the object URL (the tab it opens outlives the call — +`BriefStore.previewLetter`'s original comment already said so and is preserved, +moved onto the token's own doc comment); `download()` does revoke, once the click has +fired. This asymmetry is preserved deliberately, not unified — the two call sites +behaved differently before this ticket and still do. + +Each of the three commands now injects `BLOB_PRESENTER` and calls it instead of the DOM +directly: + +| File | Before (last statement) | After | +| ----------------------- | ---------------------------------------------------------------------------------- | ------------------------------------------------------------ | +| `stamdata.store.ts` | `createObjectURL` → `createElement('a')` → `click()` → `revokeObjectURL` (6 lines) | `this.blobPresenter.download(blob, \`${s.table.id}.json\`);` | +| `brief.store.ts` | `window.open(URL.createObjectURL(r.value), '_blank')` | `this.blobPresenter.open(r.value);` | +| `org-template.store.ts` | `window.open(URL.createObjectURL(r.value), '_blank')` | `this.blobPresenter.open(r.value);` | + +`OrgTemplateStore.previewUrlFor` (added by RB-24, a different seam — a document +content URL for an ``, not a blob handoff) is untouched. + +## Tests added + +**`libs/beheer/src/application/stamdata.store.spec.ts`** — a new +`StamdataStore.download (RB-28)` describe block with a recording fake `BlobPresenter`: + +1. Does not call the presenter while `canDownload()` is false because nothing is dirty + yet — the guard's previously-dark true branch, first clause. +2. Does not call the presenter while previewing a date, even with a real edit present — + the guard's true branch, second clause. +3. **The success path**, asserting `toJson(...)`'s exact output reaches the file: reads + the recorded blob's text and compares it byte-for-byte against a direct call to + `toJson(store.table()!, store.rows())`, and asserts the filename is + `professions.json`. + +**`apps/ssp/src/app/brief/application/brief.store.spec.ts`** — the existing +`BriefStore.previewLetter` describe block's success test previously spied directly on +`window.open`/`URL.createObjectURL` (both already jsdom-spyable, since the properties +exist even though calling them for real throws "not implemented"). It now provides the +recording fake via `BLOB_PRESENTER` and asserts `opened` holds exactly the resolved +blob — the same outcome, reached through the new seam instead of monkey-patching two +global browser objects. + +**`apps/ssp/src/app/brief/application/org-template.store.spec.ts`** (new file — +`OrgTemplateStore` had no spec at all before this ticket) — a +`OrgTemplateStore.proefbrief (RB-28)` describe block: the success path (presenter +receives the resolved blob, no error) and the failure path (presenter never reached, +error surfaced). A `Partial` stub with a no-op `categoriesResource` +(status `'idle'`) satisfies the store's constructor effect without touching the +logo-upload sub-state, which these tests do not exercise. + +## Verified red without the fix + +Broke `StamdataStore.download()` with an `Edit` (not `git checkout`): changed the +filename from `` `${s.table.id}.json` `` to `` `${s.table.id}.csv` ``. Ran the new +success-path spec: + +``` +AssertionError: expected 'professions.csv' to be 'professions.json' // Object.is equality + +Expected: "professions.json" +Received: "professions.csv" + ❯ libs/beheer/src/application/stamdata.store.spec.ts:134:36 +``` + +Re-applied the correct filename with a second `Edit`; the full `stamdata.store.spec.ts` +file (6 tests) went green again. + +## Verification + +- **`grep` for remaining DOM blob calls** in all three stores — + `grep -nE "window\.open|createObjectURL|revokeObjectURL|createElement\('a'\)|\.click\(\)"` — + zero matches. The only occurrences of those calls anywhere in `apps`/`libs` are inside + `blob-presenter.ts` itself (checked with a second, unscoped grep — no fourth inlined + handoff exists). +- `npm run lint`: clean. +- `npm run dep:check`: unaffected (no new import direction — `libs/shared` still does not + depend on `libs/beheer`; both `libs/beheer` and `apps/ssp/brief` import the new token + from `libs/shared`, never the reverse). +- `npm test` (all four projects): all pass — ssp 276, behandelportal 37, shared 138, + beheer 26 (up from 23; +3 for the new `download()` describe block). +- Coverage, `npm run test:coverage` narrowed per project: + - `libs/beheer/src/application/stamdata.store.ts` — **before** BRH 15 / BRF 37 + (40.5% branch, confirmed against the current tree, matching TE-006's citation + exactly); **after** BRH 25 / BRF 37 (**67.6% branch**). `libs/beheer/src/application` + has exactly this one file, so the module figure moves the same way. + - `apps/ssp/src/app/brief/application/brief.store.ts` — **before** BRH 39 / BRF 72 + (54.2% branch). This is higher than TE-006's cited 32/64 (50%) because RB-22/RB-23 + already added branches (the 404-tolerance path) since the finding was written — see + "What TE-006 got wrong" below. **After**: BRH 39 / BRF 72, unchanged — swapping the + global-spy assertions for the injected fake changes how the success branch is + reached in the spec, not whether it is reached; it was already covered before this + ticket (see below). + - `apps/ssp/src/app/brief/application/org-template.store.ts` — no spec existed before + this ticket, so there is no meaningful "before" branch figure for it specifically. + **After**: BRH 17 / BRF 77, including both `proefbrief()` branches newly covered. +- `npm run ci` (foreground, `timeout: 600000`, no background/Monitor): result reported + in the implementing agent's final answer. + +## What TE-006 got wrong + +TE-006 states: "`brief.store.spec.ts` demonstrates this exactly: it tests +`previewLetter`'s failure case ... and cannot test the success case." This is not +accurate for the code as it stood at the start of this ticket. The spec already had an +`'opens the composed letter in a new tab on success'` test that used +`vi.spyOn(URL, 'createObjectURL')` and `vi.spyOn(window, 'open')` to assert the success +path — jsdom defines both properties (as functions that throw "not implemented" if +actually invoked), so `vi.spyOn` can already replace them, and the pre-existing test +did. That test passed both before and after this ticket's change; this ticket did not +newly unlock `previewLetter`'s success path, it moved an already-passing assertion off +two hand-spied global browser objects and onto the new injectable seam. `git log +--follow -p` on the spec file shows this test dates to the WP-67 monorepo merge, not to +any of RB-22/23/24. + +The seam is still worth having: `StamdataStore.download()`'s success path (five +DOM/API calls in a row: `createObjectURL`, `createElement`, `.href`, `.download`, +`.click()`, `revokeObjectURL`) is a materially harder thing to spy on faithfully than a +single `window.open` call, and was in fact still dark before this ticket (no +`download()` test of any kind existed). `OrgTemplateStore.proefbrief()` also had no +spec at all. TE-006's diagnosis (three commands share the same class of problem, one +token fixes all three) is sound; only the specific "cannot test" claim about +`previewLetter` overstates what was true for that one call site. Scope was not reduced +because of this — all three call sites are migrated per the ticket's own instruction to +ship them together rather than half-adopt the seam. + +## What this ticket did not touch + +`OrgTemplateStore.previewUrlFor` (RB-24) — confirmed present and unchanged at +`org-template.store.ts:78`. `libs/shared/src/application/upload-shell.service.ts` +(RB-25) and `upload-controller.ts` (RB-26) — not read beyond what RB-24's own note +already described, not edited. `libs/shared/docs/behaviour-spec.mdx` — regenerated by +`npm run gen:behaviour-spec` (part of `npm run ci`) to reflect the new/renamed test +names; never hand-edited. diff --git a/libs/beheer/src/application/stamdata.store.spec.ts b/libs/beheer/src/application/stamdata.store.spec.ts index d81c350..14958ec 100644 --- a/libs/beheer/src/application/stamdata.store.spec.ts +++ b/libs/beheer/src/application/stamdata.store.spec.ts @@ -1,7 +1,8 @@ import { TestBed } from '@angular/core/testing'; import { describe, it, expect } from 'vitest'; import { Result, ok } from '@shared/kernel/fp'; -import { StamRow, StamTable } from '@beheer/domain/stamdata'; +import { BLOB_PRESENTER, BlobPresenter } from '@shared/application/blob-presenter'; +import { StamRow, StamTable, toJson } from '@beheer/domain/stamdata'; import { StamdataAdapter } from '@beheer/infrastructure/stamdata.adapter'; import { StamdataStore } from './stamdata.store'; @@ -16,13 +17,30 @@ const table: StamTable = { }; const rows: StamRow[] = [{ program: 'geneeskunde', beroep: 'Arts' }]; -function setup(): StamdataStore { +/** A recording fake of BLOB_PRESENTER — records every call instead of touching the DOM, + which is what TE-006's seam is for: the store's success path becomes assertable. */ +function fakeBlobPresenter() { + const opened: Blob[] = []; + const downloaded: { blob: Blob; filename: string }[] = []; + const presenter: BlobPresenter = { + open: (blob) => opened.push(blob), + download: (blob, filename) => downloaded.push({ blob, filename }), + }; + return { presenter, opened, downloaded }; +} + +function setup(blobPresenter?: BlobPresenter): StamdataStore { const adapter: Partial = { list: (): Promise> => Promise.resolve(ok([table])), load: (): Promise> => Promise.resolve(ok({ table, rows: rows.map((r) => ({ ...r })) })), }; - TestBed.configureTestingModule({ providers: [{ provide: StamdataAdapter, useValue: adapter }] }); + TestBed.configureTestingModule({ + providers: [ + { provide: StamdataAdapter, useValue: adapter }, + ...(blobPresenter ? [{ provide: BLOB_PRESENTER, useValue: blobPresenter }] : []), + ], + }); return TestBed.inject(StamdataStore); } @@ -63,3 +81,57 @@ describe('StamdataStore undo/redo (WP-32)', () => { expect(store.canUndo()).toBe(false); }); }); + +// --- RB-28 (TE-006): download() ends in BLOB_PRESENTER.download, not raw DOM calls, +// so the seam makes both the guard's branches and the success path assertable. --- + +describe('StamdataStore.download (RB-28)', () => { + it('does not call the presenter while the two-clause guard blocks (nothing dirty yet)', async () => { + // Given a freshly loaded table with no edits — canDownload() is false. + const { presenter, downloaded } = fakeBlobPresenter(); + const store = setup(presenter); + await store.load(); + expect(store.canDownload()).toBe(false); + + // When download() is called... + store.download(); + + // Then the guard's true branch fires and the presenter is never reached. + expect(downloaded).toHaveLength(0); + }); + + it('does not call the presenter while previewing a date, even with edits', async () => { + // Given a loaded table with a real edit, but a preview date filter active. + const { presenter, downloaded } = fakeBlobPresenter(); + const store = setup(presenter); + await store.load(); + store.editCell(0, 'beroep', 'Chirurg'); + store.setPreviewDate('2024-01-01'); + expect(store.canDownload()).toBe(false); + + // When download() is called... + store.download(); + + // Then the guard still blocks it. + expect(downloaded).toHaveLength(0); + }); + + it("passes toJson(...)'s exact output and the table id as the filename (success path)", async () => { + // Given a loaded table with a valid, dirty edit — canDownload() is true. + const { presenter, downloaded } = fakeBlobPresenter(); + const store = setup(presenter); + await store.load(); + store.editCell(0, 'beroep', 'Chirurg'); + expect(store.canDownload()).toBe(true); + const expectedJson = toJson(store.table()!, store.rows()); + + // When download() is called... + store.download(); + + // Then the presenter receives exactly one call, with toJson's output reaching the + // file byte-for-byte and the table id as the file name. + expect(downloaded).toHaveLength(1); + expect(downloaded[0].filename).toBe('professions.json'); + await expect(downloaded[0].blob.text()).resolves.toBe(expectedJson); + }); +}); diff --git a/libs/beheer/src/application/stamdata.store.ts b/libs/beheer/src/application/stamdata.store.ts index c3f9154..0574b13 100644 --- a/libs/beheer/src/application/stamdata.store.ts +++ b/libs/beheer/src/application/stamdata.store.ts @@ -18,6 +18,7 @@ import { reduce, } from '@beheer/domain/stamdata-editor.machine'; import { StamdataAdapter } from '@beheer/infrastructure/stamdata.adapter'; +import { BLOB_PRESENTER } from '@shared/application/blob-presenter'; type LoadedState = Extract; @@ -30,6 +31,7 @@ type LoadedState = Extract; @Injectable({ providedIn: 'root' }) export class StamdataStore { private adapter = inject(StamdataAdapter); + private blobPresenter = inject(BLOB_PRESENTER); private store = createStore(initial, reduce); readonly model = this.store.model; @@ -138,12 +140,7 @@ export class StamdataStore { const s = this.loaded(); if (!s || !this.canDownload()) return; const blob = new Blob([toJson(s.table, s.rows)], { type: 'application/json' }); - const url = URL.createObjectURL(blob); - const a = document.createElement('a'); - a.href = url; - a.download = `${s.table.id}.json`; - a.click(); - URL.revokeObjectURL(url); + this.blobPresenter.download(blob, `${s.table.id}.json`); } } diff --git a/libs/shared/docs/behaviour-spec.mdx b/libs/shared/docs/behaviour-spec.mdx index 9b27688..d80a96a 100644 --- a/libs/shared/docs/behaviour-spec.mdx +++ b/libs/shared/docs/behaviour-spec.mdx @@ -20,7 +20,7 @@ tested where._ Every bullet below is a real test name from the suite — an `it()` title (frontend) or a test method name (backend), read as a sentence. Nothing here is hand-written prose: this page -**is** the suite, reshaped for a business reader. 487 frontend behaviours across +**is** the suite, reshaped for a business reader. 492 frontend behaviours across 9 contexts; 261 backend behaviours across 42 test classes. @@ -114,6 +114,12 @@ classes. - records addRow and undoes it - clears history when switching table +#### StamdataStore.download (RB-28) + +- does not call the presenter while the two-clause guard blocks (nothing dirty yet) +- does not call the presenter while previewing a date, even with edits +- passes toJson(...)'s exact output and the table id as the filename (success path) + #### activeOn (valid-time, half-open [van, tot)) - includes a row whose window covers the date @@ -187,7 +193,7 @@ classes. #### BriefStore.previewLetter -- opens the composed letter in a new tab on success +- opens the composed letter via BLOB_PRESENTER on success (RB-28) - surfaces the error without opening a tab on failure #### BriefStore.revealBigNummer (PRD-0002 §5c) @@ -200,6 +206,11 @@ classes. - sends no X-Role/X-Subject headers outside isDevMode() - sends X-Role (and X-Subject when known) under isDevMode() +#### OrgTemplateStore.proefbrief (RB-28) + +- opens the rendered proefbrief via BLOB_PRESENTER on success +- surfaces the error without opening a tab on failure + #### RevealBigNummerAdapter.reveal (BIO-006a + BIO-012) - sends X-Step-Up only when the caller passes stepUp: true diff --git a/libs/shared/src/application/blob-presenter.ts b/libs/shared/src/application/blob-presenter.ts new file mode 100644 index 0000000..a2b841d --- /dev/null +++ b/libs/shared/src/application/blob-presenter.ts @@ -0,0 +1,37 @@ +import { InjectionToken } from '@angular/core'; + +/** + * A shared seam for handing a generated `Blob` to the browser, WITHOUT the calling + * command inlining `URL.createObjectURL`/`window.open`/`document.createElement('a')` + * as its own last statement (TE-006) — those calls are unassertable in jsdom because + * they are the end of the command, not a value the spec can intercept. A recording + * fake satisfies this shape in specs; `realBlobPresenter` is the production default. + */ +export interface BlobPresenter { + /** Open a blob in a new tab (e.g. a rendered letter preview). Never revokes the + object URL — the tab outlives this call, and the POC treats the leak as cheap + (see `BriefStore.previewLetter`'s original comment). */ + open(blob: Blob): void; + /** Trigger a browser download of a blob under the given file name, then revoke the + object URL once the click has been dispatched. */ + download(blob: Blob, filename: string): void; +} + +const realBlobPresenter: BlobPresenter = { + open(blob: Blob) { + window.open(URL.createObjectURL(blob), '_blank'); + }, + download(blob: Blob, filename: string) { + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = filename; + a.click(); + URL.revokeObjectURL(url); + }, +}; + +export const BLOB_PRESENTER = new InjectionToken('BLOB_PRESENTER', { + providedIn: 'root', + factory: () => realBlobPresenter, +});