# WP4 report — Shared column-layout layer: two defects fixed + Production documents registered

**Status:** DONE
**Branch:** `hazemdev2` (moonui2) · **Repo:** `/home/moonui2/public_html/moon-erp` · **BE change:** none
**Build:** `npx ng build --base-href /app/` → exit 0 (only the pre-existing CommonJS/ESM bailout warnings for `jsbarcode`, `qrcode`, `file-saver`, `html2canvas`).
**Typecheck:** `npx tsc --noEmit` → exit 0, zero output.
**Not deployed** to `/app` (orchestrator deploys). **Not pushed, not merged.**

## Files changed (7)

| File | What |
|---|---|
| `src/app/shared/components/transaction-line-items/transaction-line-items.component.ts` | Defect 1 (resize in a dialog) + Defect 2 part 1 (permission gate) |
| `src/app/shared/util/tx-column-layout.ts` | Defect 2 part 1 (permission gate) — kept identical to the component |
| `src/app/core/services/doc-config.service.ts` | Defect 2 parts 2+3 (surface + roll back) · 7 Production registry entries |
| `src/app/app.config.ts` | root `MessageService` provider (see "why" below) |
| `src/app/app.ts` | one app-level `<p-toast />` bound to that root instance |
| `src/assets/i18n/en.json` | +8 keys, additive only |
| `src/assets/i18n/ar.json` | +8 keys, additive only |

---

## Defect 1 — resize silently no-ops inside a PrimeNG dialog

`onResizeStart` resolved the `<th>` through the page component's host:

```ts
const table: HTMLElement = this.elRef.nativeElement.querySelector('.tx-lines-table');
const thEl = table ? table.querySelector(`th[data-col-key="${col.key}"]`) : null;
if (!thEl) return;
```

Most of the 12 screens render the grid inside `app-form-dialog` (e.g. `sales/invoices/invoices.component.html:455`,
inside the `app-form-dialog` opened at line 242), which PrimeNG attaches **outside** the page component's host
element. `querySelector` returned `null`, the guard returned early, and the entire gesture did nothing — no error,
no log.

Replaced with the resolution `TxColumnLayout.onResizeStart` already used and documented:

```ts
const handleEl = event.target as HTMLElement;
const thEl = handleEl.closest('th') as HTMLElement | null;
if (!thEl) return;
```

`elRef`/`ElementRef` had no other use in the component and were removed.

**Everything else about the gesture is byte-identical** — verified by reading the diff line by line: Pointer Events
only (no `mouse*` co-fire → still exactly one persist per gesture), `window`-level listeners with no pointer
capture, `stopPropagation()` + `preventDefault()` so CDK reorder does not fire on the same pointerdown, RTL delta
(`isRtl ? -deltaX : deltaX`), `MIN_COL_WIDTH = 48`, the single `cleanedUp`-guarded cleanup, persist on `pointerup`
only and never on `pointercancel`.

### Why the non-dialog case cannot have changed

In the inline case both resolutions return the *same element*. The handler is bound per-column from inside the
`<th>` that carries `data-col-key="col.key"`:

```html
<th ... [attr.data-col-key]="col.key">
  ...
  <div class="tx-resize-handle" (pointerdown)="onResizeStart($event, col)"></div>
</th>
```

`event.target` is that handle `div` (it has no children, so it is always the target), and its nearest `th`
ancestor is exactly the `th` the old `querySelector` matched. So inline resize is unchanged; the dialog case goes
from "silently dead" to "works". `data-col-key` is left on the `<th>` — it is now only a debugging/CSS hook.

## Defect 2 — handles shown to users who cannot save, failure swallowed

### Part 1 — gate on the permission that authorises the write

Both mechanisms used `permissions.hasPermission('core.settings')`, which routes through
`hasAnyPermission(['core.settings'])` → `permissionMatches`, a **prefix** match at a dot boundary. A user holding
only `core.settings.view` therefore passed and was shown drag + resize handles.

Both now use the exact-match `permissions.can('core.settings.manage')`.

Confirmed against the backend catalog: `Modules/Core/app/Support/PermissionCatalog.php:166` defines
`'core.settings' => [... 'actions' => ['view','manage']]`, i.e. the real permission strings are
`core.settings.view` and `core.settings.manage`. **There is no literal `core.settings` permission**, so no
legitimate holder loses access by switching from prefix to exact match. Super-admin is unaffected — `can()`
short-circuits on `isSuperAdmin()` exactly as `hasAnyPermission()` did.

Applied identically to `transaction-line-items.component.ts` and `tx-column-layout.ts`, with a matching docblock in
both saying they must not diverge.

### Parts 2 + 3 — surface the failure and roll the state back

`DocConfigService.persist()` was:

```ts
this.settingService.update({...}).subscribe({ error: () => {} });
```

It now takes the pre-change snapshot and, on error, **restores the signal and tells the user**:

```ts
private persist(next: DocConfigState, previous: DocConfigState): void {
  this.settingService.update({ setting_key: this.KEY, value: JSON.stringify(next) }).subscribe({
    error: () => {
      this.state.set(previous);
      this.messageService.add({
        severity: 'error',
        summary: this.translate.instant('COMMON.ERROR'),
        detail: this.translate.instant('DOCCFG.PERSIST_FAILED'),
        life: 6000,
      });
    },
  });
}
```

All four mutators (`setField`, `setBehavior`, `setLineColumnOrder`, `setLineColumnWidth`) capture
`const previous = this.state();` **before** mutating and pass it through. Holding it by reference is safe: every
mutator writes to a `JSON.parse(JSON.stringify(...))` deep clone, so `previous` is never touched.

The rollback is a whole-state restore rather than a per-key undo on purpose — every mutator persists the entire
blob, so the server accepted all of a given write or none of it. Because `state` is a signal, the grid re-renders
the server's layout immediately, which is the part that makes the bug visible today (a toast alone would leave the
rejected layout on screen until reload).

`load()`'s `error: () => {}` was deliberately **left alone** — it must stay silent for the unauthenticated portal
path documented above it.

### ⚠️ One structural change this required: a root `MessageService` + one app-level `<p-toast />`

This is the only part of the WP that touches a file outside the three named in the brief, so it is called out
explicitly.

PrimeNG's `MessageService` has no `providedIn`, and **every screen in this app provides its own instance** in its
component `providers` array. There was **no root instance and no app-level `<p-toast>`** — so a root service like
`DocConfigService` had literally no way to reach the user, which is very likely why the error was swallowed in the
first place. Injecting `MessageService` into `DocConfigService` without providing it at root would have thrown
`NullInjectorError` at bootstrap.

So: `MessageService` added to `app.config.ts` providers, and `<p-toast />` added to `App`'s template.

Why this changes nothing for existing screens:

- A component-level `providers: [MessageService]` **shadows** the root instance, so every existing screen's
  messages still go to its own service and its own `<p-toast>` exactly as before.
- I checked every file that does `inject(MessageService)` without a local `providers` entry (22 files): all are
  child components/sub-dialogs that resolve to an **ancestor** component's provider (element-injector lookup wins
  over the root injector), plus `shared/services/confirm-delete.service.ts`, which is `@Injectable()` with no
  `providedIn` and is therefore component-provided too. None of them start resolving to the new root instance.
- An empty `<p-toast>` container is inert: it renders a zero-height div and PrimeNG's base toast style sets no
  blocking geometry. This is already proven in production — **all 12 transaction screens and both stock screens
  already render their own `<p-toast />`**, so this adds an identical, already-shipping element.

If the orchestrator would rather not have a global toast, the alternative is to expose an error signal on
`DocConfigService` and have each of the 14 screens subscribe — which is 14 screens of churn for the same result,
so I did not take it.

---

## Part 3 — the 7 Production registry entries

Added to `DOC_CONFIG_REGISTRY` in `doc-config.service.ts`, following the existing `LINE(...)` style, with
`wired: true` per the brief. All 52 field label keys were verified to already exist in **both** `ar.json` and
`en.json`, so **no new i18n key was needed for any field label** — only the 7 `DOCS.*` document names and the one
`DOCCFG.PERSIST_FAILED` message are new.

### Registry corrections — where the brief's table and the code disagreed, the code won

**`production.routing` — 10 of 12 keys were wrong in the brief.** The routing grid is a signal array of
`OperationRow` (`routing.component.ts:34-49`) bound with `[(ngModel)]`, not a FormArray:

| brief said | code says |
|---|---|
| `sequence` | `operation_no` |
| `production_center_id` | `work_center_id` |
| `setup_time` | `setup_time_minutes` |
| `run_time` | `run_time_per_unit` |
| `cavities` | `cavity_count` |
| `cycle_time` | `cycle_time_seconds` |
| `wait_time` | `queue_time_minutes` |
| `move_time` | `move_time_minutes` |
| `is_inspection` | `inspection_required` |
| `is_critical` | `critical_operation` |

(`name` and `tool_id` matched.)

**`production.consignment` — 2 keys wrong.** `ReceiptLineForm` (`consignment.component.ts:53-60`):

| brief said | code says |
|---|---|
| `declared_cost` | `declared_unit_cost` |
| `production_date` | `manufacturing_date` |

**`production.bom`, `production.bom_operation`, `production.mps` — the brief's table was exactly right**, verified
against `boms.component.ts` `addComponent()` / `addOperation()` (both true FormArrays) and the MPS `LineRow`
property names written by `patchLine()`.

**`production.coa` (brief said "read the screen").** True FormArray `parameters`
(`coa.component.ts` `newParameterGroup()`): `name`, `unit`, `spec_min`, `spec_max`, `result`, plus a computed
PASS/FAIL chip registered as `result_status`. The brief's "`parameter` is locked" column **is the `name` control**
— `name` and `result` are the two locked ones.

**`production.order_issue` (brief said "read the screen").** This is the **Issue Materials dialog** on
`production-orders.component.html:581-741` over the `issueLines` signal (`IssueLineRow`,
`production-orders.component.ts:77-104`) — *not* the order-detail Materials tab at lines 370-428, which is fully
read-only and has no inputs at all. Columns: `product_id`, `unit_id`, `planned_quantity`, `consumed_quantity`,
`remaining`, `quantity`, `warehouse_id`, `owner_partner_id`. The product column renders `product_name` +
`product_code` but its row identity — and therefore its column key — is `product_id`.

### Two deliberate judgement calls (please sanity-check these)

1. **`production.order_issue` locks `quantity` (Issue Qty) as well as `product_id`.** The brief only said "the
   product column is locked". `quantity` is the single editable purpose of that dialog — hiding it would leave a
   grid that cannot issue anything — and every other document in this registry locks product + quantity together.
   Easy to revert to `defaultVisible: true` if you disagree.
2. **Display-only columns are registered, with the key documented inline.** `production.consignment.line_value`
   (computed `quantity × declared_unit_cost`), `production.coa.result_status` (the PASS/FAIL chip) and
   `production.routing.operation_no` (auto-resequenced ×10) have no model field. They are still real, visible,
   user-hideable columns, and omitting them would make them permanently unhideable — the same failure mode the
   brief warns about. **WP5–WP8 must use these exact strings as those columns' keys.** Each is flagged in an
   inline comment in the registry.

### Fields deliberately NOT registered

Model-only fields with no grid column: BOM component `sort_order`/`notes`/`substitutes` (a nested sub-grid), BOM
operation `sequence`/`notes`, routing `name_ar`, COA `sort_order` (stamped from the row index on save), MPS `key`
(local row-tracking id), order-issue `product_variant_id`/`batchExpanded`/`batchAllocations` (an expander
sub-grid, not a column of this grid).

### Note on `wired: true`

The brief specified it, so it is set. `settings.component.html:677,695,713,729` uses `!doc.wired` to show a "SOON"
badge and disable every toggle — so with `wired: true` these 7 documents render as fully enabled in the Documents
settings screen, while their grids are not actually reading the config until WP5–WP8 land. If WP4 might ship
independently of WP5–WP8, flip these seven to `wired: false` and let the wiring WPs flip them back.

---

## AC #6 — how I confirmed no behavioural change for the 14 existing screens

1. **The 12 pre-existing registry entries are byte-identical.** Proven mechanically, not by eye — I extracted the
   `DOC_CONFIG_REGISTRY` array body from `git show HEAD:` and from the working tree and asserted the old body is
   an exact **string prefix** of the new one: `True` (8112 → 18003 chars, purely appended). Doc keys went from the
   same 12, in the same order, to those 12 + 7. So every existing screen's `docKey` lookup, field set, `locked`
   flags and `defaultVisible` values are untouched, and `def()`/`registryCols()` look up by key anyway.
2. **The load/save paths for order, width and visibility are untouched.** The complete list of removed lines in
   `doc-config.service.ts` is 10: the four `const next = JSON.parse(...this.state())` lines (replaced by the same
   clone taken from the `previous` snapshot), the four `this.persist(next)` calls (now `this.persist(next, previous)`),
   and the two lines of the old `persist()` signature/body. `load()`, `migrateLineOrders()`,
   `LINE_ORDER_MIGRATIONS`, `fieldVisible()`, `fieldRequired()`, `fieldDefault()`, `behavior()`,
   `lineColumnOrder()`, `lineColumnWidth()`, `lineColumnWidths()` and the mutation logic inside all four setters
   are unchanged. On the success path `persist()` behaves exactly as before (no `next` handler, same payload).
3. **The grid's column maths is untouched.** In `transaction-line-items.component.ts` the diff touches only three
   regions: the `ElementRef` import/field removal, the `canReorder` expression, and the `<th>` lookup inside
   `onResizeStart`. `cols()`, `dataCols()`, `actionCols()`, `widthFor()` (the whole %-rebalance formula) and
   `onColumnDrop()` are not modified at all. In `tx-column-layout.ts` only `canReorder` changed.
4. **The intended behaviour delta is exactly the three fixes**, and its only user-visible surface for existing
   screens is: (a) resize now works in a dialog where it previously did nothing, (b) a `core.settings.view`-only
   user no longer sees handles they could never save with, (c) a rejected save now toasts and reverts instead of
   lying. Nobody who could successfully save before loses the ability.
5. `npx tsc --noEmit` clean and `npx ng build --base-href /app/` green.

## i18n — additive only, both files

No `git checkout` / `restore` / `stash` ever ran on `ar.json` or `en.json`. Edits were made by loading the JSON,
inserting keys, and re-serialising, then verified mechanically: **0 keys removed, 8 added per file, 0 pre-existing
values changed.** (The re-serialisation initially un-escaped `—`/`›` on one unrelated
`POS.NEGATIVE_STOCK_SET_ON_WAREHOUSE` line in each file — semantically identical, but I restored the original
escaping so the diff is strictly additive. Final diff: +12/−2 lines per file, where the two `−` lines are the
lines that merely gained a trailing comma.)

New keys, in both `en.json` and `ar.json`:
`DOCS.PRODUCTION_BOM`, `DOCS.PRODUCTION_BOM_OPERATION`, `DOCS.PRODUCTION_MPS`, `DOCS.PRODUCTION_CONSIGNMENT`,
`DOCS.PRODUCTION_ROUTING`, `DOCS.PRODUCTION_COA`, `DOCS.PRODUCTION_ORDER_ISSUE`, `DOCCFG.PERSIST_FAILED`.

## Acceptance criteria

| # | Criterion | Status |
|---|---|---|
| 1 | Resize works in a PrimeNG dialog | Fixed by code; **needs a human click-through.** Use **Sales → Invoices** — its grid is at `invoices.component.html:455` inside the `app-form-dialog` opened at line 242. On `hazemdev2` before this change that resize was dead. |
| 2 | Non-dialog resize still works, RTL-correct, persists once, not on cancel | Gesture code unchanged apart from the `<th>` lookup, which resolves to the same element inline (argued above). Needs the same click-through to confirm empirically. |
| 3 | Read-only settings user sees no handles in either mechanism | Done — both gate on `can('core.settings.manage')`. **Note: a newly changed permission needs logout/login to take effect** (`cached_user` holds the permission list), so test with a fresh login. |
| 4 | Rejected persist shows an error and the layout reverts | Done — toast + `state.set(previous)`. |
| 5 | All 7 Production doc keys in the Documents settings screen with bilingual labels, keys matching the real grids | Done — keys verified against each grid's real row model; labels exist in both i18n files. |
| 6 | No behavioural change for the 14 existing screens | Done — evidence above. |
| 7 | Build green, no new tsc errors | Done — both exit 0. |

## Concerns / follow-ups for the orchestrator

1. **AC 1 and 2 are code-verified, not click-verified.** There is no test runner in this repo (`skipTests: true`)
   and this WP does not deploy. Someone should drag a Sales-Invoice column edge after the deploy.
2. **The root `MessageService` + app-level `<p-toast />`** is the one change outside the three named files.
   Reasoned safe above, but it is app-wide, so it deserves a look.
3. **`wired: true` on 7 not-yet-wired documents** — cosmetically enabled toggles until WP5–WP8 land. See above.
4. **`production.order_issue` locking `quantity`** — a deliberate deviation from the brief's "product only".
5. **WP5–WP8 must use the exact column keys registered here**, especially the three display-only ones
   (`line_value`, `result_status`, `operation_no`) — a mismatch silently produces an unhideable column.
6. **Out of scope and untouched**, as instructed: the "a drag drops currently-hidden columns from the saved order"
   issue documented in `LINE_ORDER_MIGRATIONS`, per-user layouts, and wiring any Production grid.
7. Did not conflict with WP3 (`c03c989d1`) — that commit touches only `src/app/features/production/rnd/`, which
   this WP does not read or write.