atomic-design-poc/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-26.md

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:

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.