diff --git a/Inventory-and-Catalogue.md b/Inventory-and-Catalogue.md index 3cb36c7..be1af92 100644 --- a/Inventory-and-Catalogue.md +++ b/Inventory-and-Catalogue.md @@ -72,9 +72,11 @@ surfaces are hidden; the vendor/product catalogue and cart remain available. - **Sites** (`/admin/sites`) — ports/depots/offices that hold stock; geocoded from pincode; vessels can be associated. -- **`ItemInventory`** — quantity per `(product, site)`. **Incremented at PO - approval** (not on close) for the ordered quantities, when the PO has a - `siteId`. +- **`ItemInventory`** — quantity per `(product, site)`. **Designed to be + incremented at PO approval** (not on close) for the ordered quantities, when + the PO has a `siteId`. ⚠️ In production this rarely fires — POs are raised + against a Vessel and carry no `siteId`, and the write isn't gated by the flag. + See [Inventory on Approval](Inventory-on-Approval) and [Tech Debt](Tech-Debt). - **`ItemConsumption`** — daily draw-down per `(product, site, date)`, recorded via the "Log Consumption" form on the site detail page (with `recordedBy`). diff --git a/Inventory-on-Approval.md b/Inventory-on-Approval.md new file mode 100644 index 0000000..8d20ce2 --- /dev/null +++ b/Inventory-on-Approval.md @@ -0,0 +1,116 @@ +# Inventory on Approval + +How site inventory is (meant to be) updated when a PO is approved, the feature +flag that governs the inventory surfaces, and **how it actually behaves in +production today** — which diverges from the intended design. + +> **TL;DR** — The design is "approving a PO adds its ordered items to the +> delivery site's stock immediately." In production this path is effectively +> dormant: POs no longer carry a `siteId` (cost centre is a **Vessel**), so the +> `if (po.siteId)` guard in the approve action is almost never true, and the +> write isn't gated by the inventory feature flag anyway. See +> [Tech Debt](Tech-Debt) for the open items. + +## 1. What the design is meant to do + +When a Manager approves a PO, the ordered line items should be added to the +**delivery site's** on-hand stock **right away** — so inventory reflects +orders-in-flight rather than waiting for delivery/closure. + +- Inventory lives in `ItemInventory`, keyed by `(productId, siteId)`. +- On approval, for each line item the ordered quantity is **incremented** onto + that product's stock at the PO's site. +- This replaced an earlier design where inventory was only updated at receipt + confirmation (close). The intent of moving it to approval was to surface + committed demand sooner on the site dashboards. + +Implementation lives in `approvePo()` — +`app/(portal)/approvals/[id]/actions.ts`: + +```ts +// after the PO is set to MGR_APPROVED … +const siteId = po.siteId ?? null; +if (siteId) { + for (const li of po.lineItems) { + if (!li.productId) continue; // unlinked items are skipped + await db.itemInventory.upsert({ + where: { productId_siteId: { productId: li.productId, siteId } }, + update: { quantity: { increment: Number(li.quantity) } }, + create: { productId: li.productId, siteId, quantity: Number(li.quantity) }, + }); + } + revalidatePath(`/admin/sites/${siteId}`); +} +``` + +## 2. The feature flag + +`NEXT_PUBLIC_INVENTORY_ENABLED` — read once in `lib/feature-flags.ts`: + +```ts +export const INVENTORY_ENABLED = + process.env.NEXT_PUBLIC_INVENTORY_ENABLED !== "false"; +``` + +- **On unless explicitly `"false"`.** Unset ⇒ enabled. +- The flag **only controls UI visibility** of the inventory *tracking* surfaces: + the `Sites` admin area, per-site stock tables, the consumption charts/form, and + the Sites sidebar link. The vendor list, product catalogue, and cart used for + PO creation are always available, regardless of the flag. +- **The flag does _not_ gate the data write.** The `ItemInventory` upsert in + `approvePo()` (section 1) does **not** check `INVENTORY_ENABLED`. It runs + purely on the `po.siteId` condition. So "inventory disabled" hides the screens + but does **not** stop the approval-time mutation. + +## 3. How it functions in production right now + +Production does **not** set `NEXT_PUBLIC_INVENTORY_ENABLED`, so +`INVENTORY_ENABLED` is `true` and the inventory screens are visible. Despite +that, the auto-increment is effectively a no-op: + +1. **POs don't carry a `siteId`.** The cost centre is a **Vessel** — PO + create/edit/import forms set `vesselId`, never `siteId` (`siteId` remains an + optional legacy column on `PurchaseOrder`). The `if (siteId)` guard is + therefore almost always false, so nothing is written. +2. **Even with a `siteId`, line items may be skipped.** Items are counted only + when they already have a `productId`. Products are auto-linked/created at + **payment** (`syncProductCatalog` in `payments/actions.ts`), so line items + typed by hand have no `productId` at approval and are skipped. + +**Net effect in prod:** the Sites / inventory pages render (flag is on) but stay +empty, because the on-approve write rarely — in practice, never — fires for a +normally-created PO. Inventory is *visible but unpopulated*. + +> The slightly different statement in +> [Inventory and Catalogue](Inventory-and-Catalogue) ("incremented at PO +> approval … when the PO has a `siteId`") is technically correct but reads as if +> the path is live; in the current vessel-only model the `siteId` precondition +> is the reason it isn't. + +## 4. Why it diverges (root cause) + +Inventory is modelled **per Site**, but POs are now raised against a **Vessel** +(the Vessel-or-Site cost-centre model was collapsed to Vessel-only). Nothing in +the current flow maps a PO to a Site, so the per-site write has no site to target. + +## 5. Options to make it match the design (or retire it) + +Pick one direction and apply consistently — tracked in [Tech Debt](Tech-Debt): + +- **Give POs a site.** Derive a Site from the chosen Vessel (e.g. a vessel→home-site + link) or add an explicit "delivery site" field, so `po.siteId` is populated. +- **Link products at approval.** If inventory-on-approve is to mean anything, + ensure line items are product-linked (or create products) before the upsert, + rather than only at payment. +- **Gate the write on the flag.** Make the `ItemInventory` upsert honour + `INVENTORY_ENABLED` so "off" has no data side effects — or remove the + approval-time write entirely if the feature is being parked. +- **Retire it.** If inventory tracking isn't a near-term priority, set + `NEXT_PUBLIC_INVENTORY_ENABLED=false` in prod to hide the empty screens, and + delete/flag-gate the dormant write. + +## Related + +- [Inventory and Catalogue](Inventory-and-Catalogue) — products, pricing, cart, sites. +- [PO Lifecycle](PO-Lifecycle) — where approval sits in the flow. +- [Tech Debt](Tech-Debt) — the open items captured above. diff --git a/Tech-Debt.md b/Tech-Debt.md new file mode 100644 index 0000000..59c9883 --- /dev/null +++ b/Tech-Debt.md @@ -0,0 +1,66 @@ +# Tech Debt + +A running register of known shortcuts, dormant code paths, and +design/implementation mismatches that we've consciously accepted for now. The +goal is visibility: each item records **what**, **why it matters**, and a +**suggested direction** — not a commitment to fix on any timeline. + +> Add new items at the top of the "Open" list with a short, honest description. +> Move resolved items to "Resolved" with the commit/PR that closed them. + +## Open + +### TD-1 · Inventory-on-approval is dormant in production + +**What.** Approving a PO is meant to add its ordered items to the delivery +site's stock (`ItemInventory`, keyed by `(productId, siteId)`). In production the +write almost never fires, and it isn't governed by the inventory feature flag. + +**Why it matters.** + +- POs are raised against a **Vessel** (cost centre), and PO forms set `vesselId`, + never `siteId`. The `if (po.siteId)` guard in `approvePo()` is therefore + almost always false → no inventory is written. Inventory screens render + (the flag defaults on) but stay empty. +- The `ItemInventory` upsert is **not gated by `NEXT_PUBLIC_INVENTORY_ENABLED`** — + the flag only hides UI. So "inventory off" would still mutate data if a PO ever + did carry a `siteId`. +- Line items are counted only when they already have a `productId`, but products + are linked/created at **payment**, not approval — so hand-typed items would be + skipped even with a site. +- Root cause: inventory is modelled per **Site**, but POs are per **Vessel**, and + nothing maps a PO to a Site. + +**Suggested direction.** Decide one: (a) give POs a Site (vessel→home-site link +or an explicit delivery-site field) and link products at approval; or (b) gate +the write on the flag / remove it and set `NEXT_PUBLIC_INVENTORY_ENABLED=false` +in prod to hide the empty screens. Full write-up: +[Inventory on Approval](Inventory-on-Approval). + +**Touch points.** `app/(portal)/approvals/[id]/actions.ts` (the upsert), +`lib/feature-flags.ts`, `PurchaseOrder.siteId`, `payments/actions.ts` +(`syncProductCatalog`). + +--- + +### TD-2 · Migrations are not coupled to the build + +**What.** `pnpm build` runs `prisma generate` (TypeScript client) but **not** +`prisma migrate deploy`. Shipping code whose client expects a new column before +the DB has it throws `P2022 … column does not exist` at runtime. + +**Why it matters.** This already caused a production incident (the `paymentDate` +column). The deploy workflow (`.forgejo/workflows/deploy.yml`) does run +`migrate deploy`, but **manual** deploys can skip it. + +**Suggested direction.** Treat "apply migrations before the new build serves +traffic" as the invariant. Options: fold `prisma migrate deploy` into the +`start`/release script, or add a pre-deploy check that fails if migrations are +pending. Documented in the App `README.md`. + +**Touch points.** `App/package.json` scripts, `.forgejo/workflows/deploy.yml`, +`App/README.md`. + +## Resolved + +_None yet._ diff --git a/_Sidebar.md b/_Sidebar.md index 4b7848f..b6bd9cf 100644 --- a/_Sidebar.md +++ b/_Sidebar.md @@ -24,6 +24,7 @@ - [Purchase Orders](Purchase-Orders) - [Vendors and GST Lookup](Vendors-and-GST-Lookup) - [Inventory and Catalogue](Inventory-and-Catalogue) +- [Inventory on Approval](Inventory-on-Approval) - [Notifications](Notifications) - [File Storage](File-Storage) - [Design System](Design-System) @@ -42,3 +43,6 @@ **Ops** - [Deployment and Operations](Deployment-and-Operations) - [Issue-to-Deploy Pipeline](Issue-to-Deploy-Pipeline) + +**Engineering** +- [Tech Debt](Tech-Debt)