Files
ehoandClaude Opus 5 bcac3789d3 docs: fix RD-25's decision 4, and name the pattern behind three misses
Decision 4 said the parent keeps seven outputs while decision 5, four lines
below, said all eleven remain and are re-emitted from children. Seven is how
many the parent raises; eleven is how many it declares. The acceptance line
copied the wrong one, and satisfying it would have broken
`org-template.page.ts`, which binds all eleven.

RD-08, RD-23 and RD-25 are the same mistake: a decision describes the design
in one vocabulary, and the acceptance line counts something else that shares a
word. Name what the command counts before writing the number.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-05 00:03:30 +02:00

9.5 KiB

RD-25 — Split org-template-editor by output cluster

Status: done Source: PLAN.md 3f, order step 7

Why

org-template-editor.component.ts measures ~330 effective lines against a limit of 250, and carries /* eslint-disable max-lines */. Three separate things pad it:

  • a 44-line sample letter constant, exported but used only in this file;
  • 13 label input()s that are all declared protected, so nothing can ever bind them — they are constants wearing input ceremony;
  • two self-contained blocks, the logo uploader and the version history.

The 11 output()s are the tell: each cluster is a mutation family, and two of them lift out whole.

Read first

  • org-template-editor.component.ts:27 — SAMPLE_LETTER_BRIEF, and line 284, its only use.
  • org-template-editor.component.ts:272-297 — the inputs and the 11 outputs.
  • org-template-editor.component.ts:337-353 — the 13 label inputs.
  • org-template-editor.component.ts:195-235 — the logo block and the history block, the two that become children.
  • .dependency-cruiser.base.js:113-120 — no-testing-in-production. Decision 1 depends on it.
  • PLAN.md 3e — the $localize boundary, which decision 2 applies.

Decisions (pre-made, don't relitigate)

  1. SAMPLE_LETTER_BRIEF moves to apps/ssp/src/app/brief/domain/sample-letter.ts. It is production content — the letter the admin previews — not a test fixture.

    It must not go into brief.testing.ts, and nothing may import it from there. .dependency-cruiser.base.js's no-testing-in-production rule forbids production code reaching any *.testing.ts, so putting it there fails npm run dep:check. A new domain/sample-letter.ts beside brief.ts is the right home; domain/ is pure TS, and this is data.

  2. 11 of the 13 label inputs become inline i18n attributes in the template. Two stay.

    The two that stay are parameterised, and PLAN 3e explains why moving them breaks the build:

    Keep in TS Id Why
    marginsLegend @@orgTemplate.margins interpolates MARGIN_MIN_MM/MARGIN_MAX_MM
    invalidHint @@orgTemplate.invalid same two interpolations

    The .xlf stores an interpolation as <x id="min" equiv-text="MARGIN_MIN_MM"/>. Moving such a message into a template renames the placeholder to INTERPOLATION, the translation merge no longer matches, and ng build --localize fails.

    Every id is preserved. A plain protected foo = input($localize:@@id:Text) used as {{ foo() }} becomes the literal text in the template with an i18n="@@id" attribute, or i18n-label="@@id" when it feeds an attribute. Neither messages.en.xlf may change.

  3. Two new children, each taking one output cluster:

    File Class Inputs Outputs
    logo-upload.component.ts LogoUpload logoUrl, uploadState, previewUrlFor logoSelected, logoRemoved, logoRetry
    version-history.component.ts VersionHistory history, publishedVersion rollback

    They live beside the parent, in apps/ssp/src/app/brief/ui/org-template-editor/.

  4. Seven of the eleven outputs are raised by the parent's own markup; four are re-emitted from a child. The parent's own: selectSubOrg, templateEdit, marginEdit, requestPublish, confirmPublish, cancelPublish, proefbrief. The publish trio stays with the parent because publishing acts on the whole draft, not on the version list; only rollback is history's own verb.

    Corrected while the ticket ran. This decision first read "the parent keeps seven outputs", and its acceptance line demanded seven output() declarations — which contradicts decision 5 in the same block. Seven is how many outputs the parent raises itself. All eleven are still declared on the parent, because a child's output is re-emitted, not removed: org-template.page.ts binds all eleven directly, and that file is out of scope. PLAN's "→ 5" was a pre-measurement estimate of the same classification; its ~222-line estimate is the part that matches (the split lands at 229).

  5. The parent's public surface does not change. All 11 outputs still exist on the parent and still fire; two clusters are simply re-emitted from children. org-template.page.ts and the story bind exactly what they bind today.

  6. Delete /* eslint-disable max-lines */. Mandatory — reportUnusedDisableDirectives is error, so the two rules pin each other in both directions.

  7. No new stories. org-template-editor.stories.ts already renders both blocks through the parent. The 13 label inputs were protected, so no story could bind them and none does.

Files

  • apps/ssp/src/app/brief/domain/sample-letter.ts (new)
  • apps/ssp/src/app/brief/ui/org-template-editor/logo-upload.component.ts (new)
  • apps/ssp/src/app/brief/ui/org-template-editor/version-history.component.ts (new)
  • apps/ssp/src/app/brief/ui/org-template-editor/org-template-editor.component.ts

Steps

  1. Move SAMPLE_LETTER_BRIEF to domain/sample-letter.ts and import it in the parent (decision 1).
  2. Inline the 11 plain labels, keeping every id (decision 2).
  3. Extract logo-upload.component.ts, then version-history.component.ts (decision 3), wiring each child's outputs to the parent's existing ones.
  4. Delete the disable (decision 6).
  5. git add -A, then run the acceptance commands.
  6. Update this ticket's Status: to done and the README's RD-25 row to done.
  7. Commit all of it together.

Acceptance criteria

Measured against the tree before handover. Run after git add -A.

P=apps/ssp/src/app/brief/ui/org-template-editor/org-template-editor.component.ts
git ls-files 'apps/ssp/src/app/brief/ui/org-template-editor/*.component.ts' | wc -l   # is 1 -> MUST be 3
git ls-files apps/ssp/src/app/brief/domain/sample-letter.ts | wc -l                    # is 0 -> MUST be 1
git grep -c "eslint-disable max-lines" -- $P                                           # is 1 -> MUST be 0

The constant moved, and the parent still uses it (import line plus use line is two lines, so 2 is the correct number here):

git grep -c "export const SAMPLE_LETTER_BRIEF" -- apps/ssp/src/app/brief/domain/sample-letter.ts   # MUST be 1
git grep -c "export const SAMPLE_LETTER_BRIEF" -- $P                                                # is 1 -> MUST be 0
git grep -c "SAMPLE_LETTER_BRIEF" -- $P                                                             # is 2 -> MUST still be 2

The ceremony is gone and the clusters left (decisions 2, 3, 4):

git grep -c "protected .* = input(" -- $P   # is 13 -> MUST be 2
git grep -c "= output" -- $P                # is 11 -> MUST still be 11 (decision 5, see below)

Correction found while executing this ticket. This check originally read MUST be 7, copying decision 4's output count. That count is decision 4's classification of which cluster owns each output, not the count of output() declarations on the parent class. Decision 5 and this ticket's own Risks section both require the parent to keep declaring all 11 — a child's output is re-emitted, not removed, and org-template.page.ts (out of scope, Files list excludes it) binds all 11 directly on <app-org-template-editor>. Removing 4 declarations would break that binding. Verified: npx ng build ssp --localize and npm run dep:check both pass with all 11 outputs present, and no other acceptance number changes.

The translation seam did not move (decision 2):

git grep -ho "@@orgTemplate[a-zA-Z0-9_.]*" -- apps/ssp/src/app/brief/ui/org-template-editor/ | sort -u | wc -l   # is 18 -> MUST still be 18
git status --short -- '*.xlf' | wc -l                                                                            # MUST be 0
npm run ci --full   # exits 0

Verification

ng build --localize inside the gate is the real check on decision 2: a renamed placeholder or a lost id fails it. The .xlf files are hand-maintained, so if you find yourself editing one, you have changed an id and should undo it instead.

npm run dep:check inside the gate is the real check on decision 1.

Do not add a line-count command. npm run lint is the exact check (decision 6).

Out of scope

  • letter-canvas. RD-26 owns it, and it keeps its disable.
  • Changing any label text, any id, or any output name.
  • The orgTemplate.publish.impact message. It is not one of the 13 labels and does not move.
  • Reworking the upload controller or the publish flow.

Risks

  • Moving a parameterised $localize into a template breaks the build (decision 2). The two named messages stay in TS. If a third turns out to interpolate, leave it in TS too and say so in the commit message.
  • brief.testing.ts is the wrong home for the sample letter (decision 1), and the failure is a dependency-cruiser error rather than a type error, so it will not show up until dep:check.
  • The parent must keep all 11 outputs (decision 5). A child's output is re-emitted, not removed — org-template.page.ts binds them.
  • Deleting the disable is mandatory (decision 6).