refactor(shared): move the accept/reject decision into planFileSelection (RB-26)
createUploadController required inject(), an effect(), and a window listener
before a test could reach it. The file-selection policy trapped behind that
cost now lives in a pure function, planFileSelection, in upload.machine.ts.
planFileSelection takes plain { name, type, size } objects, not File, and
decides per file whether to reject it or accept it, with no I/O. The
controller executes the plan: it dispatches a rejection as-is, and starts the
upload for an accepted file (the one step that needs crypto.randomUUID()).
A new spec covers the three outcomes: the 'multiple' batch rejection, a
rejectReason-based rejection, and the accept case, plus order in a mixed
batch. Verified red-then-green with a temporary stub, undone by a second edit.
No change to the controller's public surface or to the calling organism.
previewUrlFor (added by RB-24) is untouched.
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 | — | — | open |
|
||||
| **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** | open |
|
||||
| **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** | **implemented** |
|
||||
| **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 | — | — | open |
|
||||
| **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** | open |
|
||||
| **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,109 @@
|
||||
# RB-26 — move the accept/reject decision into `planFileSelection` (`upload.machine.ts`)
|
||||
|
||||
Status: **implemented** · 2026-08-28 · Source finding: `02-testability.md` TE-004 ·
|
||||
`99-backlog.md` RB-26 · Depends on `implementation/rb-24.md` (moved the upload files into
|
||||
`infrastructure`/`domain`/`application`)
|
||||
|
||||
## What was wrong
|
||||
|
||||
TE-004: `createUploadController` does three `inject()` calls, registers an `effect()`, and
|
||||
adds a `window` focus listener, all before it returns. A spec must run inside a `TestBed`
|
||||
injection context with `UploadAdapter`, `UploadShellService`, and `DestroyRef` all
|
||||
satisfied to reach anything inside it. What sits behind that cost is real policy:
|
||||
`onFileSelected` decides, per file, whether to reject it with reason `'multiple'`, reject
|
||||
it with a `rejectReason` result, or start its upload — a decision over
|
||||
`(categories, categoryId, files)` with no I/O in it. `rejectReason`, the predicate that
|
||||
decision calls, was already exported and spec'd; the decision that calls it was not.
|
||||
|
||||
## What changed
|
||||
|
||||
`libs/shared/src/domain/upload.machine.ts` gains one pure export:
|
||||
|
||||
```ts
|
||||
export function planFileSelection(
|
||||
state: UploadState,
|
||||
categoryId: string,
|
||||
files: { name: string; type: string; size: number }[],
|
||||
): UploadMsg[];
|
||||
```
|
||||
|
||||
It takes plain `{ name, type, size }` objects, not `File` — a spec needs no DOM. The body
|
||||
is the old `onFileSelected` decision, moved: an unknown category plans nothing; too many
|
||||
files for a single-file category plans one `FileRejected` with reason `'multiple'` and
|
||||
skips the per-file checks; otherwise each file is judged by `rejectReason` and plans
|
||||
either a `FileRejected` or a `FileSelected` entry, one entry per input file, in order.
|
||||
|
||||
`libs/shared/src/application/upload-controller.ts`'s `onFileSelected` now maps `selected:
|
||||
File[]` to plain candidates, calls `planFileSelection`, and executes the result: a
|
||||
`FileRejected` entry dispatches as-is; anything else starts the upload for the file at
|
||||
that same array index (`crypto.randomUUID()`, `files.set()`, `shell.upload()` — the three
|
||||
things that must stay impure and stay in the controller). No other method changed.
|
||||
`previewUrlFor` (added by RB-24) is untouched.
|
||||
|
||||
## The `localId` placeholder — a deliberate, contained choice
|
||||
|
||||
An accepted file's planned `FileSelected` entry carries `localId: ''`. A real id needs
|
||||
`crypto.randomUUID()`, and the ticket is explicit that call stays in the controller, not
|
||||
the domain. The controller reads only each entry's `.type` to route it — it dispatches a
|
||||
`FileRejected` entry verbatim, but for a `FileSelected` entry it discards the entry and
|
||||
calls `start(categoryId, selected[i])`, which builds its own message with a real id.
|
||||
The placeholder is therefore never dispatched. This was the only way found to keep the
|
||||
return type exactly `UploadMsg[]` (as the ticket's own code sketch specifies) while still
|
||||
letting the plan carry a per-file, order-preserving "start this one" signal — the
|
||||
`FileRejected` variant carries no file identity (state keys rejections by category only),
|
||||
so position in the returned array is what the controller uses to find the matching
|
||||
original `File`. A discriminated `{ kind: 'reject' | 'start'; msg? }` return would avoid
|
||||
the placeholder but was not built, since the ticket's signature is explicit and the
|
||||
placeholder design meets it without changing behaviour.
|
||||
|
||||
## Behaviour
|
||||
|
||||
Same messages, same order, for the same inputs. Tracing all three original branches:
|
||||
|
||||
- Unknown category: original returns without dispatching; new code calls `planFileSelection`
|
||||
(returns `[]`), then `forEach` over an empty array — no dispatch, no start.
|
||||
- Too many files for a single-file category: original dispatches one `FileRejected`
|
||||
('multiple') and returns; new code gets a one-entry plan and dispatches that one entry —
|
||||
`forEach` never reaches indices past the plan's length, so no file starts.
|
||||
- Per-file loop: original dispatches `FileRejected` or calls `start` for each file, in
|
||||
order; new code's plan has one entry per file, in the same order, and the controller
|
||||
dispatches or starts at each index identically.
|
||||
|
||||
## Testing
|
||||
|
||||
`libs/shared/src/domain/upload.machine.spec.ts` gained a `planFileSelection` describe
|
||||
block: unknown category (plans nothing), the `'multiple'` batch rejection, a passing
|
||||
single file against a single-file category, `rejectReason`'s two reject cases (`'type'`,
|
||||
`'size'`) reached through the plan, the accept case's exact `FileSelected` shape
|
||||
(including the `localId: ''` placeholder), and a mixed multiple-file case asserting
|
||||
order (`['FileSelected', 'FileRejected', 'FileSelected']`).
|
||||
|
||||
**Proved red before green**, per the ticket's instruction not to use `git checkout`:
|
||||
temporarily replaced the function body with a stub returning `[]` unconditionally (an
|
||||
edit, not a revert), ran `ng test shared`, and got:
|
||||
|
||||
```
|
||||
Test Files 1 failed | 23 passed (24)
|
||||
Tests 6 failed | 139 passed (145)
|
||||
```
|
||||
|
||||
The 6 failures were the `'multiple'` rejection, both `rejectReason` cases, the accepted-
|
||||
file shape, and the mixed-order case — every outcome that depends on the real branching,
|
||||
each failing with `expected [] to deeply equal [...]`. The unknown-category case passed
|
||||
even against the stub, since both the stub and the real implementation return `[]` there
|
||||
— expected, not a gap, since that branch has no policy to exercise. A second edit restored
|
||||
the real body; the same run returned to `24 passed / 145 passed`.
|
||||
|
||||
## Scope held
|
||||
|
||||
No change to `createUploadController`'s construction, the `effect()`, or the `window`
|
||||
listener — those are RB-25/RB-27's targets (RB-25 is `UploadShellService`, running
|
||||
concurrently in the same commit window; RB-27 is `upload.adapter.ts`'s XHR closure).
|
||||
Neither file was touched. The controller's public surface (`previewUrlFor`,
|
||||
`onFileSelected`, `onRemove`, `onRetry`, `onDelete`, `onChannelChange`) is unchanged in
|
||||
name and signature, and the organism that calls it (`<app-document-upload>`) needed no
|
||||
change.
|
||||
|
||||
## `npm run ci`
|
||||
|
||||
Result and step count reported in the closing message.
|
||||
@@ -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. 467 frontend behaviours across
|
||||
**is** the suite, reshaped for a business reader. 474 frontend behaviours across
|
||||
9 contexts; 238 backend behaviours across 41 test
|
||||
classes.
|
||||
|
||||
@@ -798,6 +798,16 @@ classes.
|
||||
- maskTail keeps the requested tail length
|
||||
- masks the whole value when it is not longer than the kept tail
|
||||
|
||||
#### planFileSelection
|
||||
|
||||
- plans nothing for an unknown category
|
||||
- rejects the whole batch with reason "multiple" for a single-file category
|
||||
- does not reject a single file against a single-file category
|
||||
- rejects one file via rejectReason (wrong type)
|
||||
- rejects one file via rejectReason (too large)
|
||||
- plans a FileSelected entry for a file that passes format validation
|
||||
- judges each file independently and preserves order for a mixed multiple-file category
|
||||
|
||||
#### problemDetail
|
||||
|
||||
- extracts the detail from an RFC-7807 ProblemDetails
|
||||
|
||||
@@ -12,7 +12,7 @@ import {
|
||||
UploadMsg,
|
||||
UploadState,
|
||||
inFlight,
|
||||
rejectReason,
|
||||
planFileSelection,
|
||||
} from '@shared/domain/upload.machine';
|
||||
|
||||
export interface UploadControllerDeps {
|
||||
@@ -75,17 +75,16 @@ export function createUploadController(deps: UploadControllerDeps) {
|
||||
return documentId.startsWith('demo-') ? undefined : uploadContentUrl(documentId);
|
||||
},
|
||||
onFileSelected(categoryId: string, selected: File[]) {
|
||||
const cat = deps.getUpload().categories.find((c) => c.categoryId === categoryId);
|
||||
if (!cat) return;
|
||||
if (!cat.multiple && selected.length > 1) {
|
||||
deps.dispatch({ type: 'FileRejected', categoryId, reason: 'multiple' });
|
||||
return;
|
||||
}
|
||||
for (const file of selected) {
|
||||
const reason = rejectReason(cat, { type: file.type, sizeMb: file.size / 1e6 });
|
||||
if (reason) deps.dispatch({ type: 'FileRejected', categoryId, reason });
|
||||
else start(categoryId, file);
|
||||
}
|
||||
// The accept/reject decision lives in upload.machine.ts (planFileSelection),
|
||||
// so it's testable without a DOM File. Each plan entry lines up by index with
|
||||
// `selected`: a rejection dispatches as-is; anything else means "start", so the
|
||||
// controller runs the one impure step the plan can't (crypto.randomUUID()).
|
||||
const candidates = selected.map((f) => ({ name: f.name, type: f.type, size: f.size }));
|
||||
const plan = planFileSelection(deps.getUpload(), categoryId, candidates);
|
||||
plan.forEach((msg, i) => {
|
||||
if (msg.type === 'FileRejected') deps.dispatch(msg);
|
||||
else start(categoryId, selected[i]);
|
||||
});
|
||||
},
|
||||
onRemove(localId: string) {
|
||||
shell.cancel([localId]);
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
deliveryRefs,
|
||||
inFlight,
|
||||
rejectReason,
|
||||
planFileSelection,
|
||||
} from './upload.machine';
|
||||
|
||||
const cat = (over: Partial<DocumentCategory> = {}): DocumentCategory => ({
|
||||
@@ -309,6 +310,86 @@ describe('rejectReason', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('planFileSelection', () => {
|
||||
const file = (over: Partial<{ name: string; type: string; size: number }> = {}) => ({
|
||||
name: 'diploma.pdf',
|
||||
type: 'application/pdf',
|
||||
size: 1_000_000,
|
||||
...over,
|
||||
});
|
||||
|
||||
it('plans nothing for an unknown category', () => {
|
||||
const s = stateWith([cat({ categoryId: 'diploma' })]);
|
||||
expect(planFileSelection(s, 'unknown', [file()])).toEqual([]);
|
||||
});
|
||||
|
||||
it('rejects the whole batch with reason "multiple" for a single-file category', () => {
|
||||
const s = stateWith([cat({ categoryId: 'diploma', multiple: false })]);
|
||||
const plan = planFileSelection(s, 'diploma', [file(), file({ name: 'second.pdf' })]);
|
||||
expect(plan).toEqual([{ type: 'FileRejected', categoryId: 'diploma', reason: 'multiple' }]);
|
||||
});
|
||||
|
||||
it('does not reject a single file against a single-file category', () => {
|
||||
const s = stateWith([cat({ categoryId: 'diploma', multiple: false })]);
|
||||
const plan = planFileSelection(s, 'diploma', [file()]);
|
||||
expect(plan).toHaveLength(1);
|
||||
expect(plan[0].type).toBe('FileSelected');
|
||||
});
|
||||
|
||||
it('rejects one file via rejectReason (wrong type)', () => {
|
||||
const s = stateWith([
|
||||
cat({ categoryId: 'diploma', multiple: true, acceptedTypes: ['application/pdf'] }),
|
||||
]);
|
||||
const plan = planFileSelection(s, 'diploma', [file({ type: 'image/png' })]);
|
||||
expect(plan).toEqual([{ type: 'FileRejected', categoryId: 'diploma', reason: 'type' }]);
|
||||
});
|
||||
|
||||
it('rejects one file via rejectReason (too large)', () => {
|
||||
const s = stateWith([cat({ categoryId: 'diploma', multiple: true, maxSizeMb: 1 })]);
|
||||
const plan = planFileSelection(s, 'diploma', [file({ size: 2_000_000 })]);
|
||||
expect(plan).toEqual([{ type: 'FileRejected', categoryId: 'diploma', reason: 'size' }]);
|
||||
});
|
||||
|
||||
it('plans a FileSelected entry for a file that passes format validation', () => {
|
||||
const s = stateWith([
|
||||
cat({
|
||||
categoryId: 'diploma',
|
||||
multiple: true,
|
||||
acceptedTypes: ['application/pdf'],
|
||||
maxSizeMb: 10,
|
||||
}),
|
||||
]);
|
||||
const plan = planFileSelection(s, 'diploma', [file()]);
|
||||
expect(plan).toEqual([
|
||||
{
|
||||
type: 'FileSelected',
|
||||
categoryId: 'diploma',
|
||||
localId: '',
|
||||
fileName: 'diploma.pdf',
|
||||
fileSizeMb: 1,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('judges each file independently and preserves order for a mixed multiple-file category', () => {
|
||||
const s = stateWith([
|
||||
cat({
|
||||
categoryId: 'diploma',
|
||||
multiple: true,
|
||||
acceptedTypes: ['application/pdf'],
|
||||
maxSizeMb: 10,
|
||||
}),
|
||||
]);
|
||||
const plan = planFileSelection(s, 'diploma', [
|
||||
file({ name: 'a.pdf' }),
|
||||
file({ name: 'b.png', type: 'image/png' }),
|
||||
file({ name: 'c.pdf' }),
|
||||
]);
|
||||
expect(plan.map((m) => m.type)).toEqual(['FileSelected', 'FileRejected', 'FileSelected']);
|
||||
expect((plan[1] as { reason: string }).reason).toBe('type');
|
||||
});
|
||||
});
|
||||
|
||||
describe('inFlight', () => {
|
||||
it('returns only queued/uploading uploads', () => {
|
||||
let s = select(stateWith([cat({ multiple: true })]), 'diploma', 'u1');
|
||||
|
||||
@@ -119,6 +119,43 @@ export function rejectReason(
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The file-selection policy behind `<app-document-upload>`: for each candidate file,
|
||||
* decide whether to reject it or accept it, with no I/O. An unknown category plans
|
||||
* nothing (mirrors the controller's own "unknown category" guard). Choosing more than
|
||||
* one file for a single-file category rejects the whole batch with reason `'multiple'`
|
||||
* and skips the per-file checks; otherwise each file is judged by `rejectReason`.
|
||||
*
|
||||
* An accepted file plans a `'FileSelected'` entry with a placeholder `localId: ''` —
|
||||
* a real id needs `crypto.randomUUID()`, an impure call that stays in
|
||||
* `createUploadController`. The controller reads only each entry's `type` (never its
|
||||
* fields) to decide, at that same array index, whether to dispatch the rejection as-is
|
||||
* or start the upload for the original file — so the placeholder is never dispatched.
|
||||
*/
|
||||
export function planFileSelection(
|
||||
state: UploadState,
|
||||
categoryId: string,
|
||||
files: { name: string; type: string; size: number }[],
|
||||
): UploadMsg[] {
|
||||
const cat = state.categories.find((c) => c.categoryId === categoryId);
|
||||
if (!cat) return [];
|
||||
if (!cat.multiple && files.length > 1) {
|
||||
return [{ type: 'FileRejected', categoryId, reason: 'multiple' }];
|
||||
}
|
||||
return files.map((file): UploadMsg => {
|
||||
const reason = rejectReason(cat, { type: file.type, sizeMb: file.size / 1e6 });
|
||||
return reason
|
||||
? { type: 'FileRejected', categoryId, reason }
|
||||
: {
|
||||
type: 'FileSelected',
|
||||
categoryId,
|
||||
localId: '',
|
||||
fileName: file.name,
|
||||
fileSizeMb: file.size / 1e6,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/** Map one upload's status, leaving the rest of the list untouched. */
|
||||
function mapUpload(s: UploadState, localId: string, f: (u: Upload) => Upload): UploadState {
|
||||
return { ...s, uploads: s.uploads.map((u) => (u.localId === localId ? f(u) : u)) };
|
||||
|
||||
Reference in New Issue
Block a user