Files
atomic-design-poc/docs/project/readable-codebase/RD-25-org-template-editor.md
T
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

184 lines
9.5 KiB
Markdown

# 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`.
```bash
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):
```bash
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):
```bash
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):
```bash
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
```
```bash
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).