refactor(shared): add BLOB_PRESENTER, unlock the blob-to-browser success paths (RB-28)
Three application-layer commands ended in raw DOM calls (URL.createObjectURL,
window.open, document.createElement('a').click(), URL.revokeObjectURL) as
their last statement. jsdom cannot assert a call that is also the end of the
function, so each command's success path stayed unassertable, and
StamdataStore.download()'s two-clause guard stayed permanently dark on its
true branch (TE-006).
Add BLOB_PRESENTER (libs/shared/src/application/blob-presenter.ts), an
InjectionToken mirroring SESSION_PORT's shape: an interface with open()/
download(), a real implementation preserving the existing open()-never-
revokes vs download()-always-revokes asymmetry, provided in root. Route
StamdataStore.download(), BriefStore.previewLetter(), and
OrgTemplateStore.proefbrief() through it.
Add specs with a recording fake presenter: StamdataStore.download()'s guard
(both clauses) and its success path, asserting toJson(...)'s exact output
reaches the file; BriefStore.previewLetter()'s existing success test now
goes through the seam instead of spying on window/URL directly; a new
org-template.store.spec.ts (none existed before) covers proefbrief()'s
success and failure paths.
Verified red without the fix by editing the download() filename to the
wrong extension, watching the success-path spec fail, then restoring it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -100,41 +100,41 @@ deployed first_, not _must ship together_.
|
||||
Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authoritative
|
||||
16-row "Compliance review required" list, carries it — regardless of priority.
|
||||
|
||||
| ID | Module | Category | Description | Baseline metric improved | Effort | Risk | Priority | CD batch # | Depends on | Compliance | Status |
|
||||
| --------- | -------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------ | -------- | -------- | ---------- | ---------- | ------------ | -------- |
|
||||
| **RB-01** | backend/Program.cs + Data | security | Add an owner/capability check to `GET /uploads/{id}/content` and `/uploads/status`; 404 not 403 | §3c Data 75.5% branch vs 99.0% line (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-02** | backend/Program.cs + Data | privacy | Stop concatenating the BSN into `AuthzAudit.Resource`; assert on **values** in the test | §3c Data 75.5% branch (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-03** | backend/Contracts | privacy | `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` — both cross-owner lists inherit it | §3a bhp/behandeling 91.6%/81.5%; §7 Mapping row | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-04** | backend/Data | privacy | Mask the BSN used as `AuditEntry.Actor` on document audit rows (ownership column untouched) | §3c Data 99.0% line / 75.5% branch | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-05** | backend/Zgw | privacy | Drop the BSN-bearing query + body snippet from the `ZgwHttpClient` exception message | §3c Zgw 98.1%/85.5% (best backend branch) — a design gap, not a test gap | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-06** | backend/Program.cs | security | Delete the dead `POST /registrations` (no FE caller) — or add the `ForeignIds` guard | BL-003 (48 mappings in 940 lines, file CC 78) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-07** | backend/Program.cs | audit | Audit the **allow** path in all five authz gates + the 3 brief transitions and the besluit | §3c Program.cs 84.8% branch; BL-003 | S–M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-08** | backend/Program.cs | security | Route `DELETE /admin/uploads/{id}` through `CasesAdmin`; delete the orphaned `IsAdmin` gate | BL-003; §7 CQRS-light wrappers row | S | Low | **P1** | 2 | RB-07 | **SIGN-OFF** | **done** |
|
||||
| **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | **done** |
|
||||
| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | **done** |
|
||||
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | **done** |
|
||||
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | **done** |
|
||||
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | **done** |
|
||||
| **RB-24** | libs/shared/upload | ADR conform. | Move `upload/` into `infrastructure`/`domain`/`application`; **delete** the depcruise carve-out | BL-010; §7 "+1 adapter outside `infrastructure/`", "8 of 9 machines in `domain/`"; §3b shared/domain 0% reach | M | Med | P2 | 5 | — | **SIGN-OFF** | **done** |
|
||||
| **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** | open |
|
||||
| **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** | open |
|
||||
| **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-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 |
|
||||
| **RB-32** | libs/shared/docs | ADR conform. | Add the missing `language-switcher` row to the CIBG gap register (9 markers vs 8 rows) | §2 libs/shared 86 files / 5 194 lines; §6 layout Ca 22 | S | Low | P3 | 6 | — | — | open |
|
||||
| **RB-33** | libs/shared/testing | ADR conform. | Adopt `unwrapOk` at its one call site — **or delete it**; both satisfy ADR-0006 §3 | BL-004; §3a libs/shared/testing 3 files, 100% line | S | Low | P3 | 6 | — | — | open |
|
||||
| ID | Module | Category | Description | Baseline metric improved | Effort | Risk | Priority | CD batch # | Depends on | Compliance | Status |
|
||||
| --------- | -------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------ | -------- | -------- | ---------- | ---------- | ------------ | ----------- |
|
||||
| **RB-01** | backend/Program.cs + Data | security | Add an owner/capability check to `GET /uploads/{id}/content` and `/uploads/status`; 404 not 403 | §3c Data 75.5% branch vs 99.0% line (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-02** | backend/Program.cs + Data | privacy | Stop concatenating the BSN into `AuthzAudit.Resource`; assert on **values** in the test | §3c Data 75.5% branch (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-03** | backend/Contracts | privacy | `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` — both cross-owner lists inherit it | §3a bhp/behandeling 91.6%/81.5%; §7 Mapping row | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-04** | backend/Data | privacy | Mask the BSN used as `AuditEntry.Actor` on document audit rows (ownership column untouched) | §3c Data 99.0% line / 75.5% branch | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-05** | backend/Zgw | privacy | Drop the BSN-bearing query + body snippet from the `ZgwHttpClient` exception message | §3c Zgw 98.1%/85.5% (best backend branch) — a design gap, not a test gap | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-06** | backend/Program.cs | security | Delete the dead `POST /registrations` (no FE caller) — or add the `ForeignIds` guard | BL-003 (48 mappings in 940 lines, file CC 78) | S | Low | **P1** | 1 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-07** | backend/Program.cs | audit | Audit the **allow** path in all five authz gates + the 3 brief transitions and the besluit | §3c Program.cs 84.8% branch; BL-003 | S–M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-08** | backend/Program.cs | security | Route `DELETE /admin/uploads/{id}` through `CasesAdmin`; delete the orphaned `IsAdmin` gate | BL-003; §7 CQRS-light wrappers row | S | Low | **P1** | 2 | RB-07 | **SIGN-OFF** | **done** |
|
||||
| **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | **done** |
|
||||
| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | **done** |
|
||||
| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | **done** |
|
||||
| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | **done** |
|
||||
| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | **done** |
|
||||
| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | **done** |
|
||||
| **RB-24** | libs/shared/upload | ADR conform. | Move `upload/` into `infrastructure`/`domain`/`application`; **delete** the depcruise carve-out | BL-010; §7 "+1 adapter outside `infrastructure/`", "8 of 9 machines in `domain/`"; §3b shared/domain 0% reach | M | Med | P2 | 5 | — | **SIGN-OFF** | **done** |
|
||||
| **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** | open |
|
||||
| **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** | open |
|
||||
| **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** | implemented |
|
||||
| **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 |
|
||||
| **RB-32** | libs/shared/docs | ADR conform. | Add the missing `language-switcher` row to the CIBG gap register (9 markers vs 8 rows) | §2 libs/shared 86 files / 5 194 lines; §6 layout Ca 22 | S | Low | P3 | 6 | — | — | open |
|
||||
| **RB-33** | libs/shared/testing | ADR conform. | Adopt `unwrapOk` at its one call site — **or delete it**; both satisfy ADR-0006 §3 | BL-004; §3a libs/shared/testing 3 files, 100% line | S | Low | P3 | 6 | — | — | open |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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<BlobPresenter>('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 `<a href>`, 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<UploadAdapter>` 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.
|
||||
Reference in New Issue
Block a user