# WP3 report — R&D product pickers → the shared product search

**Status:** DONE
**Commit:** `c03c989d1` on `hazemdev2` (not pushed, not merged)
**Repo:** FE only (`/home/moonui2/public_html/moon-erp`). No backend change.

---

## What was there

`src/app/features/production/rnd/` had **exactly two** product pickers — the brief's count is
confirmed by reading both the component and the template:

| # | Where | Signal | Required? |
|---|-------|--------|-----------|
| 1 | Project dialog → "المنتج" | `pfProductId` | optional (`[showClear]="true"`) |
| 2 | Schedule dialog → "المنتج" | `sfProductId` | required |

Both were `<p-select [options]="productOptions()" optionValue="value" optionLabel="label" [filter]="true">`.

`productOptions` was filled by `loadProducts()` → `ProductService.listAll()`, i.e. the **entire**
product catalogue paginated 25 rows per request (the API caps `per_page` at 25), on every entry to
the screen, filtered client-side by PrimeNG. Its error branch was literally:

```ts
error: () => { /* product picker stays empty — non-fatal */ }
```

so a failed load left both pickers empty with no toast, no retry, no trace.

## What changed

### `rnd.component.html`
- Both `<p-select>` product fields → `<app-product-search [ngModel]="…()" (ngModelChange)="….set($event)" [placeholder]="'RND.SELECT_PRODUCT' | translate" />`.
- The two field wrappers changed from `<label class="rnd-field">` to `<div class="rnd-field">`.
  Reason: the search panel is a DOM descendant of the field, and a `<label>` re-focuses its
  labelable control whenever a descendant is clicked — that would fire *after* the picker's own
  `focusNext()` on pick and yank focus back into the input. `.rnd-field` is a class selector with
  `display:flex; flex-direction:column`, so the swap is visually inert. Every other field on the
  screen is still a `<label>`.
- Table cells: `productLabel(...)` → `productDisplay(...)`.

### `rnd.component.ts`
- Removed `ProductService` import + injection, the `productOptions` signal, `loadProducts()`, the
  `loadProducts()` call in `ngOnInit`, and `productLabel()`.
- Added `ProductSearchComponent` (from `../../../shared`) to the component `imports`.
- Added `productDisplay(id)` — a two-line `#id`/`—` fallback, documented in place.
- `SelectModule` stays: the status, cost-type and **project** dropdowns still use it. The project
  dropdown is a legitimate `p-select` — it is fed by `projectOptions()`, derived from the already
  loaded `projects()` signal, not by a catalogue fetch. Out of scope and correctly left alone.

## Binding style — why `[ngModel]` and not `formControlName`

The brief points at `boms.component.html` as the reference caller, and that is what I read. BOMs
uses reactive forms, so it binds `formControlName="product_id"`. **R&D has no reactive form at all**
— it is signal-driven with `[ngModel]`/`(ngModelChange)` on every field, and there is no `<form>`
element, so `NgModel` runs standalone. I therefore kept R&D's own established binding style rather
than importing BOMs' form infrastructure into a screen that has none. This is the same contract:
`ProductSearchComponent` is a `ControlValueAccessor`, and `FormsModule` (already imported here)
drives a CVA identically to `ReactiveFormsModule`. No other caller in the codebase binds it via
`ngModel` today, so this is the first — hence the explicit verification below.

## ✅ Value-shape verification (the silent-break check)

Checked against the actual submit payload construction, not assumed.

**Before** — `p-select` with `optionValue="value"` over `{ value: number; label: string }` emitted a
plain `number` (or `null` on clear).

**After** — `ProductSearchComponent`'s CVA is explicitly typed on `number | null` end to end:

```ts
value: number | null = null;
private onChange: (v: number | null) => void = () => {};
pick(p) { this.value = this.selected.id; … this.onChange(this.value); }   // id, not the object
clear() { this.value = null; … this.onChange(null); }
writeValue(value: number | null): void { … }
```

**Payloads are unchanged:**

| Payload | Field | Source | Type |
|---|---|---|---|
| `RndProjectRequest` | `product_id` | `this.pfProductId()` | `number \| null` |
| `RndAmortScheduleRequest` | `product_id` | `this.sfProductId() as number` | `number` |

`saveSchedule()`'s guard (`this.sfProductId() == null → toast RND.SCHEDULE_INCOMPLETE`) still fires
correctly, because clearing the picker sets the signal to `null` exactly as `[showClear]` did.

Round-trip also verified: `openEditProject()` sets `pfProductId` from `p.product_id`, the one-way
`[ngModel]` pushes it into `writeValue()`, which resolves the label via `ProductService.getById(id)`
— one request for one product, versus the whole catalogue. `onScheduleProjectPicked()` still
auto-fills `sfProductId` from the picked project and the field now displays that product's label.

## The preload: removed, and why nothing else needed it

The brief said to keep the load if another part of the screen reads the array. One other reader
existed — the tables' name fallback, `p.product_name || productLabel(p.product_id)` in the Projects
and Amortization tabs. I traced it rather than guessing, and it was already dead weight:

- `RndProjectController@index` → `->with('product')`; `RndAmortizationController@index` → `->with(['project', 'product'])`.
- `RndProjectResource` / `RndAmortizationScheduleResource` both emit
  `'product_name' => $this->whenLoaded('product', fn () => $this->product?->getLocalizedName())`.
- `RndCostService.mapProject/mapSchedule` read `product_name`, falling back to the nested `product`
  relation's name.

So a row with a product always arrives with a `product_name` and the fallback is only reached when
there is no product at all — where the old code returned `#id` anyway (the id was not in
`productOptions`). The fallback now returns `#id` directly, matching the std-cost tab, which has
always rendered `r.product_name || ('#' + r.product_id)`. **No behaviour change, catalogue preload
gone.**

## Error-swallow

Resolved by deletion rather than by a new pattern: the swallowing `error: () => {}` belonged to
`loadProducts()`, and `loadProducts()` no longer exists. Nothing was added — the remaining loads
(`loadProjects`, `loadSchedules`, `loadReport`) keep their existing inline error banner + retry,
and every mutation keeps `toastError()` on the `MessageService`. The picker surfaces its own search
failures inline ("no results") the same way it does on every other Production screen.

## i18n

**No new keys.** `RND.FIELD_PRODUCT` and `RND.SELECT_PRODUCT` already existed and are reused
verbatim as label and placeholder. Neither `ar.json` nor `en.json` was opened, edited, or touched by
any git command.

## Verification

| Check | Result |
|---|---|
| `npx tsc --noEmit` | exit 0 — no errors, none new |
| `npx ng build --base-href /app/` | exit 0 — green; only the pre-existing CommonJS bailout warnings (`qrcode`, `file-saver`, `html2canvas`) |
| `grep productOptions\|productLabel\|loadProducts src/app/features/production/rnd/` | no matches — nothing dangling |
| File ownership | both edited files `moonui2:moonui2` |
| Deploy to `/app` | **not done** — the orchestrator deploys |
| Push / merge | **not done** — commit only, on `hazemdev2` |

## Notes for the orchestrator

1. **Worth a click-through on the R&D screen after deploy.** This is the first place in the codebase
   that binds `<app-product-search>` through `ngModel` instead of `formControlName`. It is the same
   CVA contract and it compiles and type-checks clean, but it has not been exercised in a browser by
   me. The two things to eyeball: the label appears when you open **Edit** on a project that already
   has a product, and picking in the **schedule** dialog after choosing a project does not get
   overwritten by the auto-fill.
2. The two product fields are now `<div class="rnd-field">` while their siblings remain `<label>` —
   intentional, explained above and commented in the template so nobody "fixes" it back.
3. No `docs/moonstack/CHANGELOG.md` exists in the FE repo (it lives in the BE repo), so no
   release-note entry was added here despite the post-edit hook's reminder.
