# WP2 — Production roles must hold `core.products.view` — REPORT

**Status:** ✅ **DONE**
**Repo:** BE `/home/moonui2/moon-erp-be` · **Branch:** `hazemdev2` · **Install:** moonui2
**Commit:** `27519dbe6` — `fix(production): grant Production roles core.products.view so factory pickers work`
**Not pushed, not merged** (as instructed).
**Date:** 2026-08-04

---

## 1. What was wrong, restated from the evidence

`Modules/Core/app/Http/Controllers/ProductController.php:36` gates `index` and `show` on
`permission:core.products.view`. Every product picker in the factory is the shared
`<app-product-search>` component, whose only data source is
`ProductService` → `GET /api/core/products`
(`moon-erp/src/app/shared/components/product-search/product-search.component.ts` →
`core/services/product.service.ts:75`).

No seeded Production role held the key. The Angular route guard checks the **Production**
permission, so the screen opens normally; the picker's request is then refused 403, the
interceptor does not surface a 403, and the dropdown simply stays empty with no message.

**Screens that mount `<app-product-search>` and were therefore dead for a Production user**
(grepped, not assumed):

| Area | File |
|---|---|
| BOM components | `features/production/boms/boms.component.html` |
| Production orders | `features/production/orders/production-orders.component.html` |
| MPS | `features/production/mps/mps.component.html` |
| Routing | `features/production/routing/routing.component.html` |
| Tools | `features/production/tools/production-tools.component.html` |
| Toll contracts | `features/production/toll/toll-contracts.component.html` |
| Consignment | `features/production/consignment/consignment.component.html` |
| Batches | `features/production/batches/batches.component.html` |
| R&D | `features/production/rnd/rnd.component.html` (WP3 converted these to the shared picker) |
| Stock receipts | `features/stock-receipts/stock-receipts.component.html` |
| Stock issues | `features/stock-issues/stock-issues.component.html` |

That last pair is what makes `warehouse_storekeeper` **not** a judgement call — see §3.

---

## 2. Red → green (criterion 1)

Test: `Modules/Production/tests/Feature/ProductionProductViewPermissionTest.php`,
`it lets a production_supervisor list products for the picker` — seeds `ProductionRoleSeeder`,
creates a user whose **only** role is `production_supervisor` (`syncRoles` strips the factory's
automatic `owner`, which would otherwise pass every gate), and calls `GET /api/core/products`.

### RED — before the change

```
$ cd /home/moonui2/moon-erp-be && /opt/cpanel/ea-php82/root/usr/bin/php -d memory_limit=1G \
    vendor/bin/pest Modules/Production/tests/Feature/ProductionProductViewPermissionTest.php \
    --filter='lets a production_supervisor list products for the picker'

   FAIL  Modules\Production\tests\Feature\ProductionProductViewPermissionTest
  ⨯ it lets a production_supervisor list products for the picker         6.24s
  ────────────────────────────────────────────────────────────────────────────
   FAILED  Modules\Production\tests\Feature\ProductionProductViewPermissionT…
  Expected response status code [200] but received 403.
Failed asserting that 403 is identical to 200.

  at Modules/Production/tests/Feature/ProductionProductViewPermissionTest.php:100
     96▕     $user = productionProductViewUserWithRole('production_supervisor');
     97▕
     98▕     $this->actingAs($user)
     99▕         ->getJson('/api/core/products')
  ➜ 100▕         ->assertStatus(200);
    101▕ });

  Tests:    1 failed (1 assertions)
  Duration: 6.27s
```

Full-file red state at the same moment: **7 failed, 3 passed (36 assertions)** — the 403, plus
the 6 tests that instantiate the not-yet-written `GrantProductionProductViewSeeder`.

### GREEN — after the change

```
$ cd /home/moonui2/moon-erp-be && /opt/cpanel/ea-php82/root/usr/bin/php -d memory_limit=1G \
    vendor/bin/pest Modules/Production/tests/Feature/ProductionProductViewPermissionTest.php

   PASS  Modules\Production\tests\Feature\ProductionProductViewPermissionTest
  ✓ it lets a production_supervisor list products for the picker         6.36s
  ✓ it grants core.products.view to every Production role that reaches…  1.14s
  ✓ it does NOT grant core.products.view to machine_operator (terminal…  1.15s
  ✓ it gives Production roles view only — no create/update/delete on th… 1.26s
  ✓ it still refuses a production_supervisor POST /api/core/products     1.72s
  ✓ it ships a bridge migration that carries the grant to installed cli… 1.18s
  ✓ it gives an existing client whose Production roles never held the k… 2.55s
  ✓ it is idempotent — running the grant twice changes nothing and thro… 1.15s
  ✓ it is additive — a client custom permission on a Production role su… 1.20s
  ✓ it does not fail when the sibling-owned permission row is absent     1.20s

  Tests:    10 passed (65 assertions)
  Duration: 18.94s
```

---

## 3. Which roles were granted, and why

`core.products.view` **only**. No Production role gains `core.products.create` / `.update` /
`.delete` — asserted for all ten roles by
`it gives Production roles view only — no create/update/delete on the catalogue`, and asserted
over HTTP by `it still refuses a production_supervisor POST /api/core/products` (403).

| Role | Granted | Reasoning |
|---|:--:|---|
| `production_supervisor` | ✅ | Owns the order lifecycle and holds `boms.view` / `routings.view` — the BOM component picker and the order product picker are its daily tools. This is the exact role in the reported defect. |
| `production_manager` | ✅ | Creates/edits orders and owns the **toll contracts** screen; both mount the picker. |
| `production_planner` | ✅ | Authors BOMs and routings, creates orders, plans **MPS/MRP**. The BOM editor *is* the picker — this role is unusable without it. |
| `production_cost_accountant` | ✅ | Standard costs, pricing, **consignment** receive/settle and **R&D** are all product-keyed and all mount the picker. |
| `production_qc_manager` | ✅ | Owns the COA lifecycle and reads batches/genealogy (`BatchController` gates on `production.boms.view`); the batches screen filters by product with the shared picker. |
| `production_auditor` | ✅ | Read-only across every production view surface, including consignment, R&D and tools. Granting a **view** key does not weaken its "zero writes" contract — the existing `ProductionRoleSeederTest` assertion that every `production.*` key ends in `.view` still passes, and the explicit forbidden list is untouched. |
| `production_materials` | ✅ | Holds `inventory.receipts.create` / `inventory.issues.create` → the **stock-receipt and stock-issue line grids**, which mount `<app-product-search>`. Also reads orders. |
| `warehouse_storekeeper` | ✅ | **The brief flagged this as a judgement call; the code makes it clear-cut.** Its "2 Production permissions" undersells it: it also carries `inventory.receipts.view/create`, `inventory.issues.view/create`, `inventory.transfers.*` and `inventory.stock.view`. `stock-receipts.component.html` and `stock-issues.component.html` both mount `<app-product-search>`, so without the grant a storekeeper cannot add a single line to a receipt or an issue. Granting it is not generosity — withholding it leaves the role's primary screens non-functional. |
| `quality_inspector` | ✅ | Holds `production.boms.view`, which is the gate on batches/genealogy — the batches screen filters by product with the shared picker. |
| `cost_accountant` (legacy Phase-5 role) | ✅ | Holds `production.orders.view`; the production-orders screen mounts the picker, and its cost reports are filtered by product. Excluding it would leave the legacy role broken where its Phase-2 counterpart works. |
| `machine_operator` | ❌ **excluded** | Its three permissions (`shopfloor.operate`, `confirmations.view`, `confirmations.create`) open the shop-floor terminal only, which has **no** product picker. Least privilege: it would gain read access to the whole catalogue for nothing. This is also load-bearing for an existing test — `ProductionRoleSeederTest` asserts `machine_operator` holds **exactly 3** permissions, so granting it would have been a regression. |

---

## 4. What was built

### a) `Modules/Production/database/seeders/ProductionRoleSeeder.php` (modified — fresh installs)

Added a `CORE_PRODUCTS_VIEW` class constant and referenced it in the ten role permission arrays.
Followed the seeder's own documented convention exactly:

- **Not** added to `SELF_OWNED_PERMISSIONS`, so it is **never** `firstOrCreate`'d here.
  `core.products.view` is owned by `Modules\Core\Database\Seeders\RolePermissionSeeder`
  (line 610). The seeder's existing `$existing` snapshot + `array_filter` existence gate attaches
  it only if the row is already on the `web` guard and skips-with-log otherwise, so seed order
  between modules stays irrelevant.
- The seeder still `syncPermissions()` on every run — correct for a fresh install (it corrects
  drift), which is precisely why it must **not** be what runs on an update.

### b) `Modules/Production/database/seeders/GrantProductionProductViewSeeder.php` (new — the fleet)

A dedicated **additive** grant. It is deliberately *not* `ProductionRoleSeeder`:

> `ProductionRoleSeeder` `syncPermissions()`. Running it on an existing client would wipe every
> permission the client hand-added to a seeded Production role from the Roles screen. The bridge
> must be additive, so it needs its own seeder.

- `givePermissionTo`, never `sync`.
- Looks the permission up; if the row does not exist it `Log::warning`s and returns — it never
  `firstOrCreate`s a permission this module does not own.
- Skips a role it cannot find (a client may have renamed or deleted one).
- Skips a role that already holds the key, so re-runs write nothing to the pivot or the activity
  log.
- The role list and the permission name are `public const` so the test asserts against the same
  source of truth the migration uses.

### c) `Modules/Production/database/migrations/2026_08_04_100000_production_roles_products_view_bridge.php` (new)

Follows the three precedent bridges (`2026_07_04_120000_grant_new_role_permissions_bridge`,
`2026_07_12_120000_wp8a_action_permissions_bridge`, `2026_08_01_620000_pos_refunds_permission_bridge`)
part for part:

| Load-bearing part | Present |
|---|:--:|
| `if (app()->runningUnitTests()) return;` | ✅ — `RefreshDatabase` re-runs `migrate` per test class; the test calls the seeder explicitly instead |
| try/catch + guarded `Log::warning`, best-effort | ✅ — a grant hiccup must not roll back an update |
| `givePermissionTo`, never `sync` | ✅ (in the seeder it calls) |
| Idempotent | ✅ — proven in tests *and* on the dev DB |
| `down()` a documented no-op | ✅ |
| Timestamp sorts after the previous newest | ✅ — previous newest anywhere in the tree was `2026_08_02_110000_add_is_implemented_to_setting_definitions`; this is `2026_08_04_100000` |

**Why the bridge is mandatory** (re-verified, not taken on trust): `config/moonstack.php` →
`moonstack.updater.seeders` contains `Modules\Core\Database\Seeders\GrantRolePermissionsSeeder`
and the setting-definition seeders — **`ProductionRoleSeeder` is absent**. Seeders outside that
list run on a fresh install and on nothing else, so a seeder-only change would have been dead code
for every client already live.

### d) `Modules/Production/tests/Feature/ProductionProductViewPermissionTest.php` (new, 10 tests)

Pest single-process hazard respected: **every** top-level helper is prefixed with the file's
subject — `productionProductViewExpectedRoles`, `productionProductViewFlush`,
`productionProductViewUserWithRole`, `productionProductViewPermsOf`. Checked against the existing
`permsOf()` in `ProductionRoleSeederTest.php` and `posRefundBridge*()` in
`POSRefundPermissionBridgeTest.php` — no collision.

Coverage against the acceptance criteria:

| Criterion | Test |
|---|---|
| 1 — red→green over HTTP | `it lets a production_supervisor list products for the picker` |
| 2 — idempotent | `it is idempotent — running the grant twice changes nothing and throws nothing` (also asserts the pivot row count stays 1) |
| 3 — additive, not `sync` | `it is additive — a client custom permission on a Production role survives the grant` — gives the role a custom `core.partners.view`, runs the grant, asserts the custom key **and** the role's own Production keys survive |
| 4 — view only | `it gives Production roles view only…` (all 10 roles) + `it still refuses a production_supervisor POST /api/core/products` (403 over HTTP) |
| bridge ships | `it ships a bridge migration that carries the grant to installed clients` — globs every module's migrations for the seeder reference and pins the timestamp ≥ `2026_08_02` |
| fleet path | `it gives an existing client whose Production roles never held the key the grant` — revokes the key from all 10 roles to simulate a pre-fix client, then runs *only* what the migration runs |
| order-independence | `it does not fail when the sibling-owned permission row is absent` — deletes the Core-owned permission row and asserts the grant neither throws nor re-creates it |
| least privilege | `it does NOT grant core.products.view to machine_operator (terminal only)` |

---

## 5. Scoped test results (criterion 5)

| Suite | Result | Verdict |
|---|---|---|
| New file alone | **10 passed (65 assertions)** | — |
| `pest Modules/Production` | **632 passed / 10 failed (3689 assertions), 1456s** | Baseline is **622 passed / 10 failed**. 622 + my 10 new tests = 632. Failure count **unchanged at 10**. **Zero new failures.** |
| `pest Modules/Core` | **719 passed / 10 failed (4758 assertions), 860s** | **Zero new failures** — proven, not assumed (see below) |

The 10 pre-existing Production failures are the known set: ConsignmentFoundation ×1,
CostAiAnomalyVariance ×3, ProductionVariance ×6.

**Core failures proven pre-existing.** The brief gave no Core baseline, so rather than assert it I
`git stash push -u`'d the whole change and re-ran the eight affected files. All ten reproduce
identically with the change removed:

```
AdminApiTest            > import test data auto-populates sales account settings by code
BackupSchedulingTest    > 🔑 the FTP connection never asks the server to switch to UTF-8 mode
CountryApiTest          > can search countries
CountryApiTest          > cannot create country with duplicate code
ProductCategorySlugTest > storefront category listing includes slug
RoleLifecycleTest       > C2: another company role is invisible in both role indexes
RoleTenantScopingTest   > lab role index hides another companys roles
RoleTenantScopingTest   > assigning another companys role to a new user is rejected
SessionApiTest          > login captures ip and user_agent on the issued token
SettingsServiceTest     > set method validates allowed values for enum type

Tests:    10 failed, 76 passed (265 assertions)   ← with the change STASHED
```

That was worth doing because three of them (`RoleLifecycleTest`, `RoleTenantScopingTest` ×2) are
role tests, i.e. exactly where a false negative would have hidden a real regression. They fail
identically without the change.

`ProductionRoleSeederTest` (the file most exposed to this edit) was also run on its own:
**14 passed (129 assertions)** — including `machine_operator EXACTLY the 3 perms` and
`production_auditor … ZERO write permissions`.

Test command used throughout, as mandated:
```
cd /home/moonui2/moon-erp-be && /opt/cpanel/ea-php82/root/usr/bin/php -d memory_limit=1G vendor/bin/pest --filter='…'
```

---

## 6. Migration ran on `moonui2_dev_be` (criterion 6)

```
$ cd /home/moonui2/moon-erp-be && bash local-deploy.sh
…
[٣/٣] تشغيل ملفات الهجرة الجديدة...

   INFO  Running migrations.

  2026_08_04_100000_production_roles_products_view_bridge ...... 209.41ms DONE
```

`php artisan migrate --force` (via `local-deploy.sh`) only. **No `migrate:fresh`, no
`migrate:refresh`, no `db:wipe` was run at any point.**

State of `moonui2_dev_be` after the migration, read back from the database:

```
production_supervisor          view=YES  total=21   writes=0
production_manager             view=YES  total=21   writes=0
production_planner             view=YES  total=12   writes=0
production_cost_accountant     view=YES  total=35   writes=0
production_qc_manager          view=YES  total=12   writes=0
production_auditor             view=YES  total=21   writes=0
production_materials           view=YES  total=25   writes=0
warehouse_storekeeper          view=YES  total=24   writes=0
quality_inspector              view=YES  total=6    writes=0
cost_accountant                view=YES  total=5    writes=0
machine_operator               view=no   total=3    writes=0
```

`writes` = count of `core.products.create|update|delete` held — **zero everywhere**.

Cross-check against the brief's table: `production_cost_accountant` reads 35 total, and the brief
recorded 32. Verified rather than waved through — it holds exactly **32** `production.*` keys
(matching the brief), plus two pre-existing `inventory.reports.*` keys the brief's
production-only column did not count, plus the one key this WP added. Nothing unexpected was
granted.

Idempotency confirmed against the live dev DB as well (not only in tests):

```
dev idempotency: before=21 after=21     (GrantProductionProductViewSeeder run twice more)
```

---

## 7. Housekeeping

- `./vendor/bin/pint` on the four touched files → `{"result":"pass"}`.
- `chown moonui2:moonui2` on all five edited/created files (incl. `docs/moonstack/CHANGELOG.md`).
- `bash local-deploy.sh` run after the edits (dump-autoload → `optimize:clear` → `migrate`).
- Bilingual `[Unreleased]` bullet added at the top of `docs/moonstack/CHANGELOG.md` (EN + `{{ar}}`
  AR, tagged `(Production)`) — user-facing, and it names the roles that changed, states the grant is
  read-only, states that `machine_operator` is deliberately excluded, and reassures the client that
  the grant is **added** to what its roles already hold so its own permission tweaks survive.
- Committed on `hazemdev2` as `27519dbe6`, conventional. **Not pushed. Not merged.**
- `513cc5697` (WP1, BOM update persists components/operations) is untouched — this commit sits
  directly on top of it and shares no file with it.

---

## 8. Concerns / notes for the orchestrator

1. **`config/moonstack.php` was deliberately NOT edited.** Adding
   `GrantProductionProductViewSeeder` to `moonstack.updater.seeders` would make it run on every
   future update as well as this one. It is idempotent, so it would be harmless — but the bridge
   migration is the mechanism the brief mandated and it is sufficient, and touching the shipped
   updater seeder list is a wider blast radius than this WP owns. Flagging it as a **considered and
   rejected** option, not an oversight. If Production ever needs a *second* cross-module grant, the
   registry entry becomes the better answer than a second bridge.

2. **`core.products.view` is the gate this WP was asked to fix, and it is the only one verified.**
   The picker itself is now unblocked end-to-end (proven over HTTP). If any factory screen also
   resolves units, categories or brands through a *separate* gated endpoint, that would be a
   distinct 403 this WP neither caused nor covers. Worth one click-through on `/app/factory/boms`
   as a production user after the FE work deploys — the acceptance test the owner named.

3. **A client who renamed a seeded Production role will not receive the grant.** The precedent POS
   bridge has a `backfillPosRefundCapability()` for exactly that case, keyed on the cashier role.
   Building the equivalent here would mean guessing which custom role is "the supervisor", so the
   seeder skips a role it cannot find by name rather than guessing. Such a client grants the
   permission from the Roles screen, which is the documented fallback for every bridge in this
   repo. Recording it, not hiding it.

4. **The Production suite takes ~24 minutes and Core ~14.** Not a defect; noted so the next BE WP
   budgets for it.

5. **`baseline.md` does not exist** at `knowledge-base/plans/production-grid-and-search/` — the file
   there is `baseline-production.txt`, and it is itself truncated (no `Tests:` summary line). The
   authoritative 622/10 figure came from the LEDGER's WP1 entry. Worth reconciling before the next
   WP is told to diff against `../baseline.md`. There is **no** Core baseline file at all, which is
   why this WP established one by stash-and-re-run (§5).