# WP6 report — MPS lines get column control

**Status:** DONE_WITH_CONCERNS (delivered via the brief's sanctioned fallback, not the full conversion)
**Branch:** `hazemdev2` · **Commit:** `433347abe`
**Build:** `npx ng build --base-href /app/` → *Application bundle generation complete (39.974 s)*, zero errors.
`npx tsc --noEmit` → clean, no output, no new errors.
**Not pushed, not merged, not deployed to `/app`.**

---

## Mechanism used: `TxColumnLayout`, not `TransactionLineItemsComponent`

I took the escape hatch the brief offers, deliberately and on the first blocker I hit.

### The blocker

`TxCellType` (`src/app/shared/components/transaction-line-items/transaction-line.types.ts:10`) is:

```ts
export type TxCellType = 'product' | 'select' | 'number' | 'percent' | 'computed' | 'action' | 'text';
```

There is **no date cell.** MPS's `period` column is a native `<input type="date">`, and in the
`production.mps` registry it is **`locked: true`** — always visible, never hideable. So the shared
grid cannot render the one column that must always be on screen.

The two ways out were both bad:

- **Render `period` as a `text` cell.** `@case ('text')` is a plain `pInputText`. That drops the
  native date picker and, worse, lets arbitrary free text reach `period` in the `UpsertMpsRequest`
  payload. That fails acceptance criteria 1 (*"works exactly as before"*) and 5 (*payload identical*)
  on a locked column.
- **Add `@case ('date')` to the shared template.** Explicitly forbidden — 12 live screens render
  that component.

So I stopped and used `TxColumnLayout` over the existing cells — the `a8c20cad5` (BOM) approach.
Same mechanism, same `core.document_settings` persistence, same `core.settings.manage` gate, same
RTL/resize math. Column control is delivered; only the vehicle differs.

### Secondary friction (not blockers, but they raise the cost of any future full conversion)

Recorded so the next person judging a conversion has the real picture:

1. **`source` select.** Today it renders through `<ng-template pTemplate="item">` with the
   `| translate` pipe. The shared grid's `@case ('select')` binds `[optionLabel]` to a raw field
   name, so the host would have to pre-translate the options into a `computed()` keyed off the
   language signal. Workable, but it is real work, and the shared select also force-enables
   `[filter]="true"` — a filter box over two options.
2. **Read-only mode.** An approved schedule freezes the dialog; today every cell carries
   `[disabled]="isReadOnly()"`. The shared grid has no disabled concept at the descriptor level, so
   this would have to become `formArray.disable()` plus `removeDisabled: () => this.isReadOnly()`.
   That *does* work — `ProductSearchComponent` implements `setDisabledState`
   (`product-search.component.ts:437`) — but it is a second behavioural rewrite riding on the
   signals→FormArray move.
3. The signals→`FormArray` conversion itself, which the brief already priced.

**Verdict on the proof-of-concept question the brief asked:** for MPS the full conversion was not
the cheap one. Any Production grid with a date column is blocked outright at the type level until
someone decides to grow `TxCellType` — which is a shared-layer decision, not a per-screen one.

---

## What changed

| File | +/- | What |
|---|---|---|
| `src/app/features/production/mps/mps.component.ts` | +42 / −1 | inject `DocConfigService` / `PermissionService` / `ElementRef`; `docKey = 'production.mps'`; construct `TxColumnLayout`; import `DragDropModule`; `docConfig.load()` in `ngOnInit` |
| `src/app/features/production/mps/mps.component.html` | +118 / −68 | CSS-grid of `<div>`s → `<table class="mps-lines-table">`; header + body both loop `layout.orderedColumns()`; cells moved verbatim into `@switch (col.key)` |
| `src/app/features/production/mps/mps.component.scss` | +114 / −25 | table styles (`table-layout: fixed`), drag/resize handle styles, horizontal-scroll wrapper; dropped the two now-meaningless CSS-grid rules |
| `src/app/core/services/doc-config.service.ts` | +5 / −4 | **comment only** — the `production.mps` block described the grid as a CSS-grid of divs, which WP6 made false. **No field key, label, lock flag or behavior changed.** |

Structural columns stay outside the ordered loop, so the CDK index math never sees them: there is
no row-# column here, and the actions `<th>`/`<td>` are pinned last.

`table-layout: fixed` is load-bearing, not cosmetic: with auto layout the inline controls' intrinsic
widths win and dragging the edge does nothing. `defaultWidths` (`product_id: 260px`, `period: 144px`,
`quantity: 112px`, `source: 144px`) mirror the old `grid-template-columns: 1fr 9rem 7rem 9rem` tracks
so an untouched grid renders as before; under fixed layout an unsized column would collapse.

---

## Every reader and writer of the lines signal

Grepped repo-wide for `lineRows|patchLine|removeLine|addLine|blankLine|lineKeySeq` before starting.
**All 18 references live in the two MPS files.** No other component, service, effect, reducer or
route reads `lineRows` — `MpsComponent` is referenced only by its own lazy route
(`production-standalone.routes.ts:386`). The only external mention is the descriptive comment in
`doc-config.service.ts`, which I updated.

| Site | Kind | Touched? |
|---|---|---|
| `.ts:78` `readonly lineRows = signal<LineRow[]>([])` | declaration | no |
| `.ts:150` `openNew` → `set([blankLine()])` | write | no |
| `.ts:159,167,171` `openEdit` + detail success/error → `set(...)` | write | no |
| `.ts:187–189` `addLine` | write | no |
| `.ts:192–194` `removeLine` | write | no |
| `.ts:197–201` `patchLine` | read+write | no |
| `.ts:206,209` `save()` filter + completeness guard | **read (payload)** | no |
| `.ts:394,404` `blankLine` / `lineRowsFrom` | factories | no |
| `.html` `@for (line of lineRows(); track line.key)` | read | moved into `<tbody>`, same expression |
| `.html` ×4 `[ngModel]="line.<field>"` | read | moved into `@case`, verbatim |
| `.html` ×4 `patchLine(line.key, '<field>', $event)` | write | moved into `@case`, verbatim |
| `.html` `removeLine(line.key)` | write | moved into pinned actions `<td>`, verbatim |

Nothing reads a stale array, because nothing about the array changed — this WP never converted it.

---

## Payload equivalence — how I verified it

**Evidence A — the whole state + payload path is byte-identical.** I extracted, from `HEAD` and from
the working tree, the union of: `interface LineRow`, `addLine`, `removeLine`, `patchLine`, **`save()`
in full** (its validation guard *and* the `UpsertMpsRequest` builder), `blankLine` and `lineRowsFrom`.
106 lines each side:

```
diff before after  →  IDENTICAL (zero lines differ)
sha256 before: 30eeccb580b24ce256810e8bf0acd08ec2db392520bf266cfb55dff017d2f36f
sha256 after : 30eeccb580b24ce256810e8bf0acd08ec2db392520bf266cfb55dff017d2f36f
```

`git diff --numstat` corroborates the shape of the change: `mps.component.ts` is `+42 / −1`, and the
single deleted line is `import { Component, OnInit, OnDestroy, inject, … }` replaced by the same
import plus `ElementRef`. Not one line of the save path was deleted or rewritten.

**Evidence B — control-name comparison (the WP5 bar).** Extracted the model-binding surface from the
template before and after and diffed it:

```
-- reads --   [ngModel]="line.period" | line.product_id | line.quantity | line.source
-- writes --  patchLine(line.key,'period') | 'product_id' | 'quantity' | 'source'
-- controls -- 1 app-product-search, 1 p-inputNumber, 1 p-select
-- type="date" -- 3 before / 3 after
-- [disabled]="isReadOnly()" -- 7 before / 7 after
```

Identical on every line. (The raw grep showed 8 `isReadOnly()` hits after; hit #7 is line 247, a
prose line inside my explanatory comment. Real guards: 7 = 7.)

**Evidence C — `save()` reads the signal, never the DOM.** So reordering or hiding a column cannot
reach the payload at all. Specifically, hiding `source` does **not** truncate it: the row model still
carries the value loaded by `lineRowsFrom`, or `'forecast'` from `blankLine`, and `save()` maps all
four fields unconditionally.

Conclusion: an unchanged schedule submits identical JSON.

---

## Acceptance criteria

| # | Criterion | Status |
|---|---|---|
| 1 | Add / edit / delete a line works exactly as before, incl. validation | ✅ every cell verbatim; `save()` byte-identical, so the `VALIDATION_LINES` warn path is untouched |
| 2 | Product cell is the shared product search | ✅ already was `app-product-search`; unchanged, so it inherits `a4df728bc`'s `name — customer — code` |
| 3 | Header drag reorders, edge drag resizes, persist company-wide, survive reload; works in a dialog | ✅ code path — `TxColumnLayout` + `docConfig.load()`; the grid *is* in a `p-dialog` and resize resolves the `<th>` via `closest('th')`, the dialog-safe resolution from `1574011b8`. **Not browser-verified — see concerns.** |
| 4 | Non-locked columns hide from Settings > Documents; `product_id`/`period`/`quantity` stay locked | ✅ `orderedColumns()` filters on `docConfig.fieldVisible(...)`, which short-circuits to `true` for `locked` fields (`doc-config.service.ts:481`); only `source` is hideable |
| 5 | Unchanged schedule → payload identical | ✅ evidence A/B/C above |
| 6 | Build green, no new type errors | ✅ both |

**Registry:** no correction needed. All four keys (`product_id`, `period`, `quantity`, `source`)
match the real `LineRow` property names that `patchLine` writes, exactly as WP4 verified. Lock flags
match the brief. Only the stale shape comment was updated.

---

## Concerns

1. **Not browser-verified.** I was told not to deploy to `/app`, so criteria 1–4 are verified by code
   path and by build, not by clicking. The reorder/resize gesture in particular is worth one manual
   pass after the orchestrator deploys — it is the first time this mechanism runs against a grid
   whose cells are `ngModel` signal rows rather than a `FormArray`. (The mechanism does not care —
   it only drives order/width/visibility — but the combination is new.)
2. **Narrow-screen behaviour changed.** Below 760 px the old CSS grid reflowed the row to two
   columns and hid the header. A `<table>` cannot do that, so it now keeps its header and scrolls
   horizontally inside `.mps-lines-scroll`. This is the same trade the BOM grids made in
   `a8c20cad5`, and it is a real (small) difference on phones.
3. **No CHANGELOG entry written.** This *is* user-facing, but none of the three sibling commits on
   this plan (`c03c989d1`, `1574011b8`, `a8c20cad5`) wrote one either — the convention on this plan
   appears to be one bilingual entry per finished capability, written by the orchestrator. Flagging
   it rather than breaking the pattern. The pre-push hook will demand it before this branch is pushed.
4. **The strategic finding.** The brief framed MPS as the cheapest conversion and the template for
   judging the rest. It was blocked by a missing cell type, not by the signals→`FormArray` cost the
   brief anticipated. Before commissioning further full conversions, someone should decide whether
   `TxCellType` grows a `date` case — that one shared-layer change is what unblocks this whole class
   of screen, and doing it once is far cheaper than the per-screen workarounds.

## Untouched

`TransactionLineItemsComponent` and `transaction-line.types.ts` — not modified, not read at runtime
by this screen. `src/assets/i18n/ar.json` and `en.json` — **not opened, not edited**; every label the
grid renders (`PRODUCTION.MPS.LINE_PRODUCT|LINE_PERIOD|LINE_QTY|LINE_SOURCE`, `DOCS.PRODUCTION_MPS`)
already exists in both files, verified. No backend change. No other Production screen.