# WP5a report — Backend: the toll customer on stock balances **Status:** DONE · **Repo:** `/home/moonui2/moon-erp-be` · **Branch:** `hazemdev2` · **Migration:** none · **FE:** untouched --- ## 🔑 The contract WP5b must code against | Thing | Exact name | Notes | |---|---|---| | Filter query param | **`toll_customer_id`** | a `business_partner` id, **or** the literal string **`own`** (= `products.toll_customer_id IS NULL`, the company's own materials). Absent / empty string → no filtering (unchanged behaviour). | | Resource field — id | **`toll_customer_id`** | `int|null`. Null for the company's own items. | | Resource field — name | **`toll_customer_name`** | `string|null`. Arabic-first (`name_ar ?: name`), identical to `ProductResource`. | | Search | the **existing `search` param** | now also matches the toll customer's `name` / `name_ar`, in addition to product `name`, `name_ar`, `code`, `sku`. No new param. | Endpoints carrying the two fields: `GET /api/inventory/stock-balances`, `GET /api/inventory/stock-balances/product/{id}`, `GET /api/inventory/stock-balances/warehouse/{id}`. The `toll_customer_id` **filter** exists on `index()` only (the other two are already product-/warehouse-scoped). **No picker endpoint was added.** WP5b must NOT feed this filter from `/api/inventory/stock-balances/lot-owners` — that endpoint lists consignment *custody* owners, a different set. Source the options from the products/partners side (the same place the products screen's toll-customer filter gets them). --- ## The naming trap — how it was honoured Two unrelated "owner" concepts now live on the same screen and were kept **completely separate**: | | source | meaning | param | fields | |---|---|---|---|---| | pre-existing | `inventory_lot_balances.owner_partner_id` | consignment **custody** of the physical quantity | `owner` (`all` / `own` / id) | `owner_quantity`, `owner_declared_value`, `nearest_expiry_owner_*` | | **new (WP5a)** | `products.toll_customer_id` | who owns the **catalogue definition** | `toll_customer_id` (id / `own`) | `toll_customer_id`, `toll_customer_name` | - `owner` was not extended, no `owner_*` field was reused, `lotOwners()` was not touched. - Both filters compose (AND) — a dedicated test seeds one row where the catalogue owner is ORGANIX and the custody owner is a different partner, and asserts `?toll_customer_id=&owner=` returns exactly that row. --- ## What changed ### `Modules/Inventory/app/Http/Controllers/StockBalanceController.php` 1. **Eager load** — `product.tollCustomer` added to `index()`, `byProduct()` **and** `byWarehouse()`. All three were done because `StockBalanceResource` emits the field unconditionally; leaving it off `byProduct`/`byWarehouse` would have made the resource lazy-load the partner once per row (the repo does not enable `preventLazyLoading`, so it would have been a *silent* N+1, not an error). 2. **Filter** — `$tollCustomerId = request()->filled('toll_customer_id') ? request('toll_customer_id') : null;` applied as `whereHas('product', …)`, with the `own` sentinel → `whereNull('toll_customer_id')`, matching `ProductService::search()` lines ~214-218. Read with `filled()` rather than a truthy check, so an empty string is "no filter" and the string `own` survives. 3. **Search** — the customer arm is an `orWhereHas('tollCustomer', …)` **nested inside the existing `whereHas('product', …)` closure**, i.e. a fifth alternative *within the product subquery*, not a sibling of the outer wheres. 4. **`companyTotals()`** — gained a `?string $tollCustomerId` parameter. - The *physical* leg inherits the filter for free: `$totalsBase` is cloned from `$query` **after** the new filter is applied. - The *consignment* leg is a separate raw `products as p` join and needed the filter mirrored explicitly — plus the toll-customer arm of the `search` clause, expressed there as `orWhereExists(business_partners as tc … whereColumn tc.id = p.toll_customer_id)` since that leg has no Eloquent relation to lean on. 5. Scribe `@queryParam` docs updated for `toll_customer_id` and the widened `search`. ### `Modules/Inventory/app/Http/Resources/StockBalanceResource.php` `toll_customer_id` + `toll_customer_name` added, both behind `whenLoaded('product')`, and the name additionally guarded by `$this->product?->relationLoaded('tollCustomer')` so a caller that loads `product` without the partner gets `null` instead of triggering a lazy load. ### `Modules/Inventory/tests/Feature/StockBalanceTollCustomerTest.php` (new, 19 tests) Every top-level helper is prefixed `tollCust…` (`tollCustSetupTenant`, `tollCustSeedBalance`, `tollCustPartner`) — Pest loads all test files into one process, so an unprefixed `setupTenant` would have been a fatal redeclare. --- ## The dangerous line — proved, not assumed The brief's warning was verified empirically rather than by reading. The clause was temporarily **hoisted one level up** (as `->orWhereHas('product.tollCustomer', …)` chained on `$query` instead of on `$q`) and the suite re-run: ``` Tests: 2 failed ← 'search and warehouse still apply TOGETHER (the nested-closure trap)' 'search still composes with category_id and hide_zero' ``` The warehouse test returned 3 product ids instead of 1 — the whole `where` had become an OR, exactly as predicted. The controller was then restored from a byte-identical backup and the tests pass again. So the guard tests are **known to fail on the wrong implementation**, which is the only thing that makes them worth having. (Worth recording for the next person: the *existing* four-column search was already safe from this, because Laravel's `Builder::callScope` wraps whatever a `whereHas` closure adds into its own nested group. The danger is entirely about which builder you chain onto — `$q` (the product subquery) versus `$query` (the outer stock-balance query).) --- ## Acceptance criteria | # | Criterion | Evidence | |---|---|---| | 1 | id + name present; null for an item with no toll customer | `a stock-balance row exposes toll_customer_id and toll_customer_name; an own item exposes neither`; plus a latin-name-fallback test and a `byProduct`/`byWarehouse` test | | 2 | search by customer name works **and other filters still apply** | `search by the toll customer name returns that customer items` (latin + Arabic), `search and warehouse still apply TOGETHER`, `search still composes with category_id and hide_zero`, `the product columns still search after the customer clause was added` | | 3 | filter narrows; `own` returns only items with no toll customer | `toll_customer_id narrows the list to that customer items`, `toll_customer_id=own returns only items with no toll customer`, `an absent or empty toll_customer_id changes nothing`, `toll_customer_id composes with warehouse_id`, `the toll customer filter is company-scoped` | | 4 | **totals card == the filtered rows** | `meta.totals matches the sum over the toll_customer_id-filtered rows` (asserts `meta.totals.owned_quantity/owned_value` equal the sum over `data`), `meta.totals consignment memo is scoped by toll_customer_id too` (40 / 200, not 340 / 2900), `meta.totals matches the rows when the filter is the customer NAME search` | | 5 | **query count bounded** | `a full page of 25 rows with toll customers issues a bounded number of queries` — **measured 23 queries** for 25 rows carrying **25 distinct** toll customers. Asserted `<= 25`. Without the eager load this would be 25 additional queries (one per row). | | 6 | `owner` lens + `below_reorder` unchanged | `the owner lens is unaffected by the toll customer work`, `the owner lens and the toll customer filter are independent and compose`, `below_reorder still works and composes with the toll customer filter` (also asserts `toll_customer_name` still resolves alongside the raw `products` join — i.e. the `select('inventory_stock_balances.*')` reset did not break eager loading) | | 7 | zero NEW failures vs baseline | see below | ### Measured query count — the number, and where it goes **23** queries for one page of 25 rows / 25 distinct toll customers. Roughly: session + auth + permissions, the `count(*)` and the page `select`, then one query each for the eager loads (`products`, `units`, **`business_partners` ← the toll customers**, `product_variants`, `warehouses`), the nearest-expiry map (serials / lot ledger / receipt batches / movements), the consignment portion map, the settings read, and the two `companyTotals` aggregates. The toll-customer addition costs **exactly one** of those 23, independent of row count. ## Tests - New file: **19 passed** (77 assertions). - `pest Modules/Inventory`: **779 passed / 4 failed** (1303s). 760 + 19 new = 779, so every pre-existing test still passes. The 4 failures were re-run in isolation and are byte-for-byte the baseline set: 2 × `LotAllocationTest` (FEFO allocation, quantity), 1 × `OpeningBalanceApiTest`, 1 × `TransferLotPreservationTest` (ErrorException at line 192). **Zero new failures.** Run as required: `/opt/cpanel/ea-php82/root/usr/bin/php -d memory_limit=1G vendor/bin/pest …` (bare `php` on this host is php-cgi → "Undefined constant STDOUT"). ## Housekeeping - `./vendor/bin/pint` on the three touched PHP files → `pass`. - `chown moonui2:moonui2` on all four edited files. - `bash local-deploy.sh` → success, no pending migrations. - Bilingual `[Unreleased]` bullet added to `docs/moonstack/CHANGELOG.md`, EN-first with the `{{ar}}` separator, matching the house format. It closes by saying the on-screen column arrives with the next update — WP5b should not need to add another bullet for the column, only adjust that sentence if the wording no longer fits. - Committed on `hazemdev2` as **`008da5d27`**. **Not pushed, not merged.** - The param is read as a scalar (`is_string || is_int`), so `?toll_customer_id[]=1` yields "no filter" instead of a TypeError against the `?string` signature of `companyTotals()`. ## Concerns / notes for WP5b 1. **The picker is the remaining trap.** There is no `toll-customers` option endpoint on the inventory side. Do not wire the new filter to `lot-owners`. If the FE needs a list of partners that actually appear as toll customers on stock, that endpoint does not exist yet and would be a small BE follow-up. 2. **No validation was added** on `toll_customer_id`, matching the brief (the endpoint has no FormRequest and none of the existing params are validated). A non-numeric, non-`own` value simply matches nothing — it is bound as a parameter, so there is no injection surface. 3. **`toll_customer_name` is Arabic-first** (`name_ar ?: name`), not locale-aware — this copies `ProductResource` deliberately so the two screens never disagree on a label. `business_partners.name_ar` is `NOT NULL`, so "no Arabic name" is the empty string, and the `?:` fallback handles it. 4. `byProduct` / `byWarehouse` carry the fields but have **no** toll-customer filter — they are already scoped to one product / one warehouse, so a catalogue-owner filter there has no caller. Add it only if a screen actually asks.