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>
184 lines
9.5 KiB
Markdown
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).
|