# Merchandising — Read-only audit

**Branch:** `retail-audit` (no code changes)
**Scope:** the Merchandise (catalog) module — products, kinds, variants, categories,
units, barcodes, batches, price lists, label printing, CSV import.
**Companion docs:** `RETAIL_AUDIT.md`, `ZF_AUDIT.md`.

> ⚠️ **Corrigendum to main audit, item #7.**
> The main audit said: *"No variant editor on FE — `ProductVariantController` orphan."*
> That understates the problem. There **is** a variant editor in `AddProductPage.jsx`
> (lines 578-705). The UI accepts variant axes, values, pack sizes, pack pricing —
> and then **silently drops all of it on submit** because the payload built at
> `onSubmit` (lines 179-189) doesn't include `form.variants`. The
> `ProductVariantController` endpoints remain orphaned. The end state is the same
> (no variants reach the database), but the failure mode is worse than the main
> audit suggested: the user fills the form, sees "Saved", and gets a single SKU.

---

## 1. Surface map

### Frontend

`app/src/modules/merchandise/` — 14 files, ~5,110 LOC total:

| File | LOC | Wired? |
|---|---|---|
| `AddProductPage.jsx` | 990 | Partial — variants discarded (see MERCH-1) |
| `LabelPrintPage.jsx` | 547 | Real — full Code128B encoder, client-side print |
| `BatchesPage.jsx` | 518 | Real — `/catalog/products/{id}/batches` |
| `ProductsPage.jsx` | 472 | Real — list / search / bulk patch |
| `CategoriesPage.jsx` | 352 | Real — categories CRUD |
| `UnitsPage.jsx` | 309 | Real — units CRUD |
| `BarcodesPage.jsx` | 268 | Real — `/catalog/products/{id}/barcodes` |
| `PriceListsPage.jsx` | 267 | Real — `/catalog/price-lists` |
| `ImportCsvModal.jsx` | 239 | Real — async job + poll (see MERCH-5) |
| `ProductEditModal.jsx` | 209 | Real — also discards variants |
| `PricingResolverPage.jsx` | 198 | Real — read-only resolver demo |
| `ProductKindsPage.jsx` | 125 | Real — read-only |
| `PricingPage.jsx` | 120 | Real — list view |
| `CatalogShell.jsx` | 63 | Layout |

Module-local hooks: `catalog.hooks.js` (318), API: `catalog.api.js` (115).

### Backend

| Component | File |
|---|---|
| `ProductController` | `backend/app/Http/Controllers/Catalog/ProductController.php` |
| `ProductVariantController` | `backend/app/Http/Controllers/Catalog/ProductVariantController.php` |
| Category / Unit / Price list controllers | `backend/app/Http/Controllers/Catalog/*` |
| Routes | `backend/routes/api.php:497-510` and surrounding |

---

## 2. Top findings (ranked)

### MERCH-1 · HIGH · Variant UI in `AddProductPage` is non-functional

The variant builder is real UI — lines 578-705 in `AddProductPage.jsx`. It
supports `axis ∈ {size, color, flavor, pack}`, per-row labels (en/ar),
SKU suffix, price delta, and a dedicated pack-size mode where the user
enters `pack_qty` + `pack_price` and the component derives `price_delta`.

The save handler is at lines 179-207:

```js
const onSubmit = async () => {
  setError(null);
  const payload = {
    sku:        form.sku.trim(),
    name_en:    form.name_en.trim(),
    name_ar:    form.name_ar.trim() || form.name_en.trim(),
    kind:       form.kind,
    base_price: price,
    cost_price: cost,
    tax_rate:   vatPct,
  };
  if (form.barcode?.trim())  payload.barcode     = form.barcode.trim();
  if (form.category_id)      payload.category_id = form.category_id;
  try {
    const created = await create.mutateAsync(payload);
    // …barcodes posted in a follow-up…
    nav('../products');
  } catch (e) { setError(e); }
};
```

`form.variants` is never read. `payload.variants` is never set.

The backend, `ProductController::store` (lines 131-149), doesn't accept
`variants` either:

```php
$data = $request->validate([
    'sku'           => [...],
    'barcode'       => [...],
    'name_en'       => [...],
    'name_ar'       => [...],
    'category_id'   => [...],
    'unit_id'       => [...],
    'kind'          => ['nullable', Rule::in(Product::KINDS)],
    // …no 'variants' key…
]);
$product = Product::create($data + ['kind' => 'simple', 'is_active' => true, 'status' => 'active']);
```

The `kind` value is persisted on the `products` row, but no `product_variants`
rows are written. The `/catalog/products/{product}/variants` endpoints
(`ProductVariantController`, routes 507-510) have **zero FE callers** —
`catalog.api.js` exports no `createProductVariant` / `productVariants` helpers,
and a grep of the FE for the path returns no results.

The user-facing effect:

1. Operator picks "With variants", picks "pack" axis, adds "6-pack → 29.99 SAR".
2. Clicks Save. Sees a green toast.
3. POS rings the SKU at base price; reports show one product, not many; pack pricing was never persisted.

**This is the worst failure mode in the audit:** silent, looks-like-success,
hidden by the fact that variants are a fairly advanced flow most testers don't exercise.

Files:
- `app/src/modules/merchandise/AddProductPage.jsx:179-207` (drop site)
- `app/src/modules/merchandise/AddProductPage.jsx:578-705` (variant UI)
- `app/src/modules/merchandise/catalog.api.js` (no variant helpers)
- `backend/app/Http/Controllers/Catalog/ProductController.php:131-149` (no `variants` validator)
- `backend/routes/api.php:507-510` (orphan endpoints)

---

### MERCH-2 · HIGH · Catalog writes gated by `catalog.products.view`

Every state-changing method in `ProductController` uses the same gate as the
read endpoints:

```php
public function store(Request $request)    { $this->authorizeP('catalog.products.view');  /* … */ }
public function update(Request $request, Product $product)
                                           { $this->authorizeP('catalog.products.view');  /* … */ }
public function destroy(Product $product)  { $this->authorizeP('catalog.products.view');  /* … */ }
public function bulkUpdate(Request $request)
                                           { $this->authorizeP('catalog.products.view');  /* … */ }
public function importCsv(Request $request)
                                           { $this->authorizeP('catalog.products.view');  /* … */ }
```

This is the same write-with-view pattern flagged in the main audit's item #5
(`AdjustmentController:54`). Anyone with view-only catalog access can:

- Create products with arbitrary prices.
- Patch existing products' `base_price` / `cost_price` / `tax_rate`.
- Delete products.
- Bulk-update up to 200 products in one call (status, category, is_active).
- Run CSV import that dispatches `ImportProductsCsv` to insert hundreds of SKUs.

Files: `backend/app/Http/Controllers/Catalog/ProductController.php:128, 162, 191, 200, 218`

---

### MERCH-3 · HIGH · `cost_price` exposed unconditionally to every viewer

`ProductController::shape` at line 302-317:

```php
return [
    'id'          => $p->id,
    'sku'         => $p->sku,
    // …
    'price'       => (float) $p->base_price,
    'cost'        => (float) $p->cost_price,   // ← always returned
    // …
];
```

There's no role-based redaction. The `cost` field is in every product list /
detail response. Cashiers, baristas, and inventory operators all see the
margin on every SKU.

This is a confidentiality question, not an integrity one — but in a multi-
role tenant, cost is a sensitive field. Compare to `PermissionBoundary`
which is used elsewhere in the platform to scope visibility; nothing
equivalent fires here.

Files: `backend/app/Http/Controllers/Catalog/ProductController.php:302-317`

---

### MERCH-4 · MEDIUM · `AddProductPage.jsx` is a 990-LOC monolith

A single component manages:

- 4 product kinds × per-kind state shape
- Variant axis builder (4 axes, axis-specific row schema)
- Pack-size derivation (per-row price_delta computation)
- Inline parts builder (composite kind)
- Inline ingredients (recipe kind)
- Primary barcode + extra-barcodes list with pack-quantity per barcode
- Auto-SKU generator + slug fallback
- Price / cost / VAT / margin / profit / preview-VAT / preview-net derived state
- Live receipt preview (with ZATCA QR TLV encoding)
- Save + barcodes follow-up

The variant-discard bug (MERCH-1) is hard to spot because the file's so
large. A future refactor of the save handler is unlikely to notice that
`form.variants` was supposed to ship somewhere. Splitting this file into
a kind-router + per-kind sub-components would surface the missing variant
wiring on day 1.

Files: `app/src/modules/merchandise/AddProductPage.jsx` (entire — 990 LOC)

---

### MERCH-5 · LOW · CSV import template missing variant columns

`ImportCsvModal.jsx:23-27`:

```js
const TEMPLATE_COLS = [
  'name_en', 'name_ar', 'sku', 'barcode',
  'base_price', 'cost_price', 'tax_rate',
  'category_id', 'kind', 'status',
];
```

So the bulk-import path is also variant-blind. Even if the backend
`ImportProductsCsv` job were taught to materialise variants from extra
columns, the FE template doesn't surface those columns to the operator.

Files: `app/src/modules/merchandise/ImportCsvModal.jsx:23-27`

---

### MERCH-6 · LOW · `ProductEditModal.jsx` also drops variants

A spot-check of the edit modal (209 LOC) shows the same pattern as
`AddProductPage`: a payload of scalar fields with no `variants`. Worth
mentioning so MERCH-1's fix lands in both places.

Files: `app/src/modules/merchandise/ProductEditModal.jsx`

---

## 3. What works well

- **Categories, units, barcodes, batches, price lists** are all wired end-to-end
  with envelope-shaped responses, optimistic invalidation, and bilingual support.
  `catalog.api.js` (115 LOC) is a model module API — documented per-endpoint,
  consistent realRequest usage, single envelope helper.
- **`LabelPrintPage.jsx`** is a real Code128B + QR implementation with
  mm-accurate print CSS. No mock data, no stubs.
- **`ImportCsvModal`** uses the correct async-job pattern: dispatch +
  poll-with-timeout (30 attempts at 2s = 60s ceiling), with clear phase
  states (idle / uploading / polling / done / error).
- **Scan endpoint** (`/catalog/scan/{code}`) is a real universal-resolver
  used by POS, label generator, and barcode tools — single source of truth
  for "what is this code?".
- **`ProductController::shape`** correctly composes derived fields (stock,
  reorder, branches) so the FE doesn't have to N+1.
- **`Rule::unique('products')->where('company_id', $companyId)`** on
  the `sku` validator (line 132) is the right tenant-scoped uniqueness
  pattern.

---

## 4. Recommended fix order (when Merchandising work resumes)

1. **MERCH-1** — first, fix the silent data loss. Smallest path:
   - Include `form.variants` in the `payload` at `AddProductPage:179`.
   - Add a `variants` validator to `ProductController::store` accepting `axis` + `values[]`.
   - On create, persist rows via the existing `ProductVariantController::store` logic
     or inline equivalent in the same DB transaction.
   - Same fix in `ProductEditModal.jsx` (MERCH-6).
2. **MERCH-2** — split `catalog.products.create` / `.update` / `.delete` / `.bulk_update`
   / `.import` permissions out of `.view`. Update seeders to grant the new perms
   to roles that previously implicitly had write via view.
3. **MERCH-3** — redact `cost` in `shape()` based on a `viewCost` permission
   check, or in a `PermissionBoundary` filter.
4. **MERCH-4** — refactor `AddProductPage.jsx`. Optional but high value:
   each kind becomes its own sub-component with a typed payload builder,
   and the parent is just a kind-router + save coordinator.
5. **MERCH-5** — add `variant_axis` + `variant_values_json` columns to the
   CSV template (and the backend parser).

Items 1-3 are independent, low-risk, and total roughly one day of work.

---

**End of Merchandising audit. No code changes proposed in this branch.**