Commit e37c143e3c378c48984aba20946379bf7586d2f3
1 parent
577d9214
feat(inventory+orders): movement ledger + sales-order shipping (end-to-end demo)
The killer demo finally works: place a sales order, ship it, watch
inventory drop. This chunk lands the two pieces that close the loop:
the inventory movement ledger (the audit-grade history of every
stock change) and the sales-order /ship endpoint that calls
InventoryApi.recordMovement to atomically debit stock for every line.
This is the framework's FIRST cross-PBC WRITE flow. Every earlier
cross-PBC call was a read (CatalogApi.findItemByCode,
PartnersApi.findPartnerByCode, InventoryApi.findStockBalance).
Shipping inverts that: pbc-orders-sales synchronously writes to
inventory's tables (via the api.v1 facade) as a side effect of
changing its own state, all in ONE Spring transaction.
What landed
-----------
* New `inventory__stock_movement` table — append-only ledger
(id, item_code, location_id FK, signed delta, reason enum,
reference, occurred_at, audit cols). CHECK constraint
`delta <> 0` rejects no-op rows. Indexes on item_code,
location_id, the (item, location) composite, reference, and
occurred_at. Migration is in its own changelog file
(002-inventory-movement-ledger.xml) per the project convention
that each new schema cut is a new file.
* New `StockMovement` JPA entity + repository + `MovementReason`
enum (RECEIPT, ISSUE, ADJUSTMENT, SALES_SHIPMENT, PURCHASE_RECEIPT,
TRANSFER_OUT, TRANSFER_IN). Each value carries a documented sign
convention; the service rejects mismatches (a SALES_SHIPMENT
with positive delta is a caller bug, not silently coerced).
* New `StockMovementService.record(...)` — the ONE entry point for
changing inventory. Cross-PBC item validation via CatalogApi,
local location validation, sign-vs-reason enforcement, and
negative-balance rejection all happen BEFORE the write. The
ledger row insert AND the balance row update happen in the
SAME database transaction so the two cannot drift.
* `StockBalanceService.adjust` refactored to delegate: it computes
delta = newQty - oldQty and calls record(... ADJUSTMENT). The
REST endpoint keeps its absolute-quantity semantics — operators
type "the shelf has 47" not "decrease by 3" — but every
adjustment now writes a ledger row too. A no-op adjustment
(re-saving the same value) does NOT write a row, so the audit
log doesn't fill with noise from operator clicks that didn't
change anything.
* New `StockMovementController` at `/api/v1/inventory/movements`:
GET filters by itemCode, locationId, or reference (for "all
movements caused by SO-2026-0001"); POST records a manual
movement. Both protected by `inventory.stock.adjust`.
* `InventoryApi` facade extended with `recordMovement(itemCode,
locationCode, delta, reason: String, reference)`. The reason is
a String in the api.v1 surface (not the local enum) so plug-ins
don't import inventory's internal types — the closed set is
documented on the interface. The adapter parses the string with
a meaningful error on unknown values.
* New `SHIPPED` status on `SalesOrderStatus`. Transitions:
DRAFT → CONFIRMED → SHIPPED (terminal). Cancelling a SHIPPED
order is rejected with "issue a return / refund flow instead".
* New `SalesOrderService.ship(id, shippingLocationCode)`: walks
every line, calls `inventoryApi.recordMovement(... -line.quantity
reason="SALES_SHIPMENT" reference="SO:{order_code}")`, flips
status to SHIPPED. The whole operation runs in ONE transaction
so a failure on any line — bad item, bad location, would push
balance negative — rolls back the order status change AND every
other line's already-written movement. The customer never ends
up with "5 of 7 lines shipped, status still CONFIRMED, ledger
half-written".
* New `POST /api/v1/orders/sales-orders/{id}/ship` endpoint with
body `{"shippingLocationCode": "WH-MAIN"}`, gated by the new
`orders.sales.ship` permission key.
* `ShipSalesOrderRequest` is a single-arg Kotlin data class — same
Jackson deserialization trap as `RefreshRequest`. Fixed with
`@JsonCreator(mode = PROPERTIES) + @param:JsonProperty`. The
trap is documented in the class KDoc.
End-to-end smoke test (the killer demo)
---------------------------------------
Reset Postgres, booted the app, ran:
* Login as admin
* POST /catalog/items → PAPER-A4
* POST /partners → CUST-ACME
* POST /inventory/locations → WH-MAIN
* POST /inventory/balances/adjust → quantity=1000
(now writes a ledger row via the new path)
* GET /inventory/movements?itemCode=PAPER-A4 →
ADJUSTMENT delta=1000 ref=null
* POST /orders/sales-orders → SO-2026-0001 (50 units of PAPER-A4)
* POST /sales-orders/{id}/confirm → status CONFIRMED
* POST /sales-orders/{id}/ship body={"shippingLocationCode":"WH-MAIN"}
→ status SHIPPED
* GET /inventory/balances?itemCode=PAPER-A4 → quantity=950
(1000 - 50)
* GET /inventory/movements?itemCode=PAPER-A4 →
ADJUSTMENT delta=1000 ref=null
SALES_SHIPMENT delta=-50 ref=SO:SO-2026-0001
Failure paths verified:
* Re-ship a SHIPPED order → 400 "only CONFIRMED orders can be shipped"
* Cancel a SHIPPED order → 400 "issue a return / refund flow instead"
* Place a 10000-unit order, confirm, try to ship from a 950-stock
warehouse → 400 "stock movement would push balance for 'PAPER-A4'
at location ... below zero (current=950.0000, delta=-10000.0000)";
balance unchanged after the rollback (transaction integrity
verified)
Regression: catalog uoms, identity users, inventory locations,
printing-shop plates with i18n, metadata entities — all still
HTTP 2xx.
Build
-----
* `./gradlew build`: 15 subprojects, 175 unit tests (was 163),
all green. The 12 new tests cover:
- StockMovementServiceTest (8): zero-delta rejection, positive
SALES_SHIPMENT rejection, negative RECEIPT rejection, both
signs allowed on ADJUSTMENT, unknown item via CatalogApi seam,
unknown location, would-push-balance-negative rejection,
new-row + existing-row balance update.
- StockBalanceServiceTest, rewritten (5): negative-quantity
early reject, delegation with computed positive delta,
delegation with computed negative delta, no-op adjustment
short-circuit (NO ledger row written), no-op on missing row
creates an empty row at zero.
- SalesOrderServiceTest, additions (3): ship rejects non-CONFIRMED,
ship walks lines and calls recordMovement with negated quantity
+ correct reference, cancel rejects SHIPPED.
What was deferred
-----------------
* **Event publication.** A `StockMovementRecorded` event would
let pbc-finance and pbc-production react to ledger writes
without polling. The event bus has been wired since P1.7 but
no real cross-PBC flow uses it yet — that's the natural next
chunk and the chunk after this commit.
* **Multi-leg transfers.** TRANSFER_OUT and TRANSFER_IN are in
the enum but no service operation atomically writes both legs
yet (both legs in one transaction is required to keep total
on-hand invariant).
* **Reservation / pick lists.** "Reserve 50 of PAPER-A4 for an
unconfirmed order" is its own concept that lands later.
* **Shipped-order returns / refunds.** The cancel-SHIPPED rule
points the user at "use a return flow" — that flow doesn't
exist yet. v1 says shipments are terminal.
Showing
19 changed files
with
1250 additions
and
84 deletions
CLAUDE.md
| @@ -96,9 +96,9 @@ plugins (incl. ref) depend on: api/api-v1 only | @@ -96,9 +96,9 @@ plugins (incl. ref) depend on: api/api-v1 only | ||
| 96 | **Foundation complete; first business surface in place.** As of the latest commit: | 96 | **Foundation complete; first business surface in place.** As of the latest commit: |
| 97 | 97 | ||
| 98 | - **15 Gradle subprojects** built and tested. The dependency rule (PBCs never import each other; plug-ins only see `api.v1`) is enforced at configuration time by the root `build.gradle.kts`. | 98 | - **15 Gradle subprojects** built and tested. The dependency rule (PBCs never import each other; plug-ins only see `api.v1`) is enforced at configuration time by the root `build.gradle.kts`. |
| 99 | -- **163 unit tests across 15 modules**, all green. `./gradlew build` is the canonical full build. | 99 | +- **175 unit tests across 15 modules**, all green. `./gradlew build` is the canonical full build. |
| 100 | - **All 9 cross-cutting platform services live and smoke-tested end-to-end** against real Postgres: auth (P4.1), authorization with `@RequirePermission` + JWT roles claim + AOP enforcement (P4.3), plug-in HTTP endpoints (P1.3), plug-in linter (P1.2), plug-in-owned Liquibase + typed SQL (P1.4), event bus + transactional outbox (P1.7), metadata loader + custom-field validation (P1.5 + P3.4), ICU4J translator with per-plug-in locale chain (P1.6), plus the audit/principal context bridge. | 100 | - **All 9 cross-cutting platform services live and smoke-tested end-to-end** against real Postgres: auth (P4.1), authorization with `@RequirePermission` + JWT roles claim + AOP enforcement (P4.3), plug-in HTTP endpoints (P1.3), plug-in linter (P1.2), plug-in-owned Liquibase + typed SQL (P1.4), event bus + transactional outbox (P1.7), metadata loader + custom-field validation (P1.5 + P3.4), ICU4J translator with per-plug-in locale chain (P1.6), plus the audit/principal context bridge. |
| 101 | -- **5 of 10 core PBCs implemented end-to-end** (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`). **pbc-inventory** is the first PBC to consume one cross-PBC facade (`CatalogApi`); **pbc-orders-sales** is the first to consume *two simultaneously* (`PartnersApi` + `CatalogApi`) in one transaction. The Gradle build still refuses any direct dependency between PBCs — Spring DI wires the interfaces to their concrete adapters at runtime. | 101 | +- **5 of 10 core PBCs implemented end-to-end** (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`). **pbc-inventory** has an append-only stock movement ledger; **pbc-orders-sales** can ship orders, atomically debiting stock via `InventoryApi.recordMovement` — the framework's first cross-PBC WRITE flow. The Gradle build still refuses any direct dependency between PBCs. |
| 102 | - **Reference printing-shop plug-in** owns its own DB schema, CRUDs plates and ink recipes via REST through `context.jdbc`, ships its own metadata YAML, and registers 7 HTTP endpoints. It is the executable acceptance test for the framework. | 102 | - **Reference printing-shop plug-in** owns its own DB schema, CRUDs plates and ink recipes via REST through `context.jdbc`, ships its own metadata YAML, and registers 7 HTTP endpoints. It is the executable acceptance test for the framework. |
| 103 | - **Package root** is `org.vibeerp`. | 103 | - **Package root** is `org.vibeerp`. |
| 104 | - **Build commands:** `./gradlew build` (full), `./gradlew test` (unit tests), `./gradlew :distribution:bootRun` (run dev profile, stages plug-in JAR automatically), `docker compose up -d db` (Postgres). The bootstrap admin password is printed to the application logs on first boot. | 104 | - **Build commands:** `./gradlew build` (full), `./gradlew test` (unit tests), `./gradlew :distribution:bootRun` (run dev profile, stages plug-in JAR automatically), `docker compose up -d db` (Postgres). The bootstrap admin password is printed to the application logs on first boot. |
PROGRESS.md
| @@ -10,21 +10,21 @@ | @@ -10,21 +10,21 @@ | ||
| 10 | 10 | ||
| 11 | | | | | 11 | | | | |
| 12 | |---|---| | 12 | |---|---| |
| 13 | -| **Latest version** | v0.12 (post-P4.3) | | ||
| 14 | -| **Latest commit** | `75bf870 feat(security): P4.3 — @RequirePermission + JWT roles claim + AOP enforcement` | | 13 | +| **Latest version** | v0.13 (post-ledger + ship) | |
| 14 | +| **Latest commit** | `feat(inventory+orders): movement ledger + sales-order shipping (end-to-end demo)` | | ||
| 15 | | **Repo** | https://github.com/reporkey/vibe-erp | | 15 | | **Repo** | https://github.com/reporkey/vibe-erp | |
| 16 | | **Modules** | 15 | | 16 | | **Modules** | 15 | |
| 17 | -| **Unit tests** | 163, all green | | ||
| 18 | -| **End-to-end smoke runs** | All cross-cutting services + all 5 PBCs verified against real Postgres; reference plug-in returns localised strings in 3 locales; Partner + Location accept custom-field `ext`; sales orders validate via three cross-PBC seams; **`@RequirePermission` enforces 403 on non-admin users hitting protected endpoints** | | 17 | +| **Unit tests** | 175, all green | |
| 18 | +| **End-to-end smoke runs** | The killer demo works: create catalog item + partner + location, set stock to 1000, place an order for 50, confirm, **ship**, watch the balance drop to 950 and a `SALES_SHIPMENT` row appear in the ledger tagged `SO:SO-2026-0001`. Over-shipping rolls back atomically with a meaningful 400. | | ||
| 19 | | **Real PBCs implemented** | 5 of 10 (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`) | | 19 | | **Real PBCs implemented** | 5 of 10 (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`) | |
| 20 | | **Plug-ins serving HTTP** | 1 (reference printing-shop, 7 endpoints + own DB schema + own metadata + own i18n bundles) | | 20 | | **Plug-ins serving HTTP** | 1 (reference printing-shop, 7 endpoints + own DB schema + own metadata + own i18n bundles) | |
| 21 | | **Production-ready?** | **No.** Foundation is solid; many capabilities still deferred. | | 21 | | **Production-ready?** | **No.** Foundation is solid; many capabilities still deferred. | |
| 22 | 22 | ||
| 23 | ## Current stage | 23 | ## Current stage |
| 24 | 24 | ||
| 25 | -**Foundation complete; Tier 1 customization live; authorization enforced.** All eight cross-cutting platform services are live, plus the **authorization layer (P4.3)**: `@RequirePermission` annotations on controller methods are enforced by a Spring AOP aspect that consults a `PermissionEvaluator` reading the JWT's `roles` claim from a per-request `AuthorizationContext`. The framework can now distinguish "authenticated but not authorized" (403) from "not authenticated" (401). Five real PBCs validate the modular-monolith template; pbc-orders-sales remains the most rigorous test (consumes two cross-PBC facades in one transaction). State machine DRAFT → CONFIRMED → CANCELLED is enforced. | 25 | +**Foundation complete; Tier 1 customization live; authorization enforced; end-to-end order-to-shipment loop closed.** All eight cross-cutting platform services are live plus the authorization layer. The biggest leap in this version: the **inventory movement ledger** is live (`inventory__stock_movement` append-only table; the framework's first append-only ledger), and **sales orders can now ship** (`POST /sales-orders/{id}/ship`) via a cross-PBC write — pbc-orders-sales injects the new `InventoryApi.recordMovement` to atomically debit stock for every line and update the order status, all in one transaction. Either the whole shipment commits or none of it does. This is the framework's **first cross-PBC WRITE flow** (every earlier cross-PBC call was a read). The sales-order state machine grows: DRAFT → CONFIRMED → SHIPPED (terminal). Cancelling a SHIPPED order is rejected with a meaningful "use a return / refund flow" message. |
| 26 | 26 | ||
| 27 | -The next phase continues **building business surface area**: pbc-orders-purchase, pbc-production, the stock movement ledger that pairs with sales-order shipping, the workflow engine (Flowable), and eventually the React SPA. | 27 | +The next phase continues **building business surface area**: pbc-orders-purchase, pbc-production, the workflow engine (Flowable), event-driven cross-PBC integration (the event bus has been wired since P1.7 but no flow uses it yet), and eventually the React SPA. |
| 28 | 28 | ||
| 29 | ## Total scope (the v1.0 cut line) | 29 | ## Total scope (the v1.0 cut line) |
| 30 | 30 | ||
| @@ -171,8 +171,6 @@ This is **real**: a JAR file dropped into a directory, loaded by the framework, | @@ -171,8 +171,6 @@ This is **real**: a JAR file dropped into a directory, loaded by the framework, | ||
| 171 | - **Job scheduler.** No Quartz. Periodic jobs don't have a home. | 171 | - **Job scheduler.** No Quartz. Periodic jobs don't have a home. |
| 172 | - **OIDC.** Built-in JWT only. OIDC client (Keycloak-compatible) is P4.2. | 172 | - **OIDC.** Built-in JWT only. OIDC client (Keycloak-compatible) is P4.2. |
| 173 | - **More PBCs.** Identity, catalog, partners, inventory and orders-sales exist. Warehousing, orders-purchase, production, quality, finance are all pending. | 173 | - **More PBCs.** Identity, catalog, partners, inventory and orders-sales exist. Warehousing, orders-purchase, production, quality, finance are all pending. |
| 174 | -- **Sales-order shipping flow.** Sales orders can be created and confirmed but cannot yet *ship* — that requires the inventory movement ledger (deferred from P5.3) so the framework can atomically debit stock when an order ships. | ||
| 175 | -- **Stock movement ledger.** Inventory has balances but not the append-only `inventory__stock_movement` history yet — the v1 cut is "set the quantity"; the audit-grade ledger lands in a follow-up that will also unlock sales-order shipping. | ||
| 176 | - **Web SPA.** No React app. The framework is API-only today. | 174 | - **Web SPA.** No React app. The framework is API-only today. |
| 177 | - **MCP server.** The architecture leaves room for it; the implementation is v1.1. | 175 | - **MCP server.** The architecture leaves room for it; the implementation is v1.1. |
| 178 | - **Mobile.** v2. | 176 | - **Mobile.** v2. |
README.md
| @@ -77,7 +77,7 @@ vibe-erp/ | @@ -77,7 +77,7 @@ vibe-erp/ | ||
| 77 | ## Building | 77 | ## Building |
| 78 | 78 | ||
| 79 | ```bash | 79 | ```bash |
| 80 | -# Build everything (compiles 15 modules, runs 163 unit tests) | 80 | +# Build everything (compiles 15 modules, runs 175 unit tests) |
| 81 | ./gradlew build | 81 | ./gradlew build |
| 82 | 82 | ||
| 83 | # Bring up Postgres + the reference plug-in JAR | 83 | # Bring up Postgres + the reference plug-in JAR |
| @@ -97,7 +97,7 @@ The bootstrap admin password is printed to the application logs on first boot. A | @@ -97,7 +97,7 @@ The bootstrap admin password is printed to the application logs on first boot. A | ||
| 97 | | | | | 97 | | | | |
| 98 | |---|---| | 98 | |---|---| |
| 99 | | Modules | 15 | | 99 | | Modules | 15 | |
| 100 | -| Unit tests | 163, all green | | 100 | +| Unit tests | 175, all green | |
| 101 | | Real PBCs | 5 of 10 | | 101 | | Real PBCs | 5 of 10 | |
| 102 | | Cross-cutting services live | 9 | | 102 | | Cross-cutting services live | 9 | |
| 103 | | Plug-ins serving HTTP | 1 (reference printing-shop) | | 103 | | Plug-ins serving HTTP | 1 (reference printing-shop) | |
api/api-v1/src/main/kotlin/org/vibeerp/api/v1/ext/inventory/InventoryApi.kt
| @@ -59,6 +59,61 @@ interface InventoryApi { | @@ -59,6 +59,61 @@ interface InventoryApi { | ||
| 59 | * state because that is most items most of the time. | 59 | * state because that is most items most of the time. |
| 60 | */ | 60 | */ |
| 61 | fun totalOnHand(itemCode: String): BigDecimal | 61 | fun totalOnHand(itemCode: String): BigDecimal |
| 62 | + | ||
| 63 | + /** | ||
| 64 | + * Record a stock movement and update the matching balance, | ||
| 65 | + * atomically. **The cross-PBC entry point for changing inventory.** | ||
| 66 | + * | ||
| 67 | + * Other PBCs (and plug-ins) call this to debit or credit stock as | ||
| 68 | + * a side effect of their own business operations: | ||
| 69 | + * - `pbc-orders-sales` calls it on `POST /sales-orders/{id}/ship` | ||
| 70 | + * with `delta=-quantity reason="SALES_SHIPMENT"` per line. | ||
| 71 | + * - `pbc-orders-purchase` (future) calls it on receipt with | ||
| 72 | + * `reason="PURCHASE_RECEIPT"` per line. | ||
| 73 | + * - The reference printing-shop plug-in (future) calls it from | ||
| 74 | + * work-order-completion handlers. | ||
| 75 | + * | ||
| 76 | + * **Why the reason is a String** instead of a typed enum: | ||
| 77 | + * plug-ins must not import inventory's internal `MovementReason` | ||
| 78 | + * enum (that would force them onto pbc-inventory's compile | ||
| 79 | + * classpath, violating guardrail #9). The accepted values are | ||
| 80 | + * documented as a closed set: `RECEIPT`, `ISSUE`, `ADJUSTMENT`, | ||
| 81 | + * `SALES_SHIPMENT`, `PURCHASE_RECEIPT`, `TRANSFER_OUT`, | ||
| 82 | + * `TRANSFER_IN`. Each carries a documented sign convention which | ||
| 83 | + * the framework enforces — a `SALES_SHIPMENT` with positive delta | ||
| 84 | + * is rejected with a meaningful error. | ||
| 85 | + * | ||
| 86 | + * **Why a single method** instead of separate `debit` / `credit`: | ||
| 87 | + * the ledger's natural shape is a signed delta. Callers think | ||
| 88 | + * "I'm shipping 5 units" → delta=-5, reason=SALES_SHIPMENT. The | ||
| 89 | + * sign carries the semantic; making the caller pick a method | ||
| 90 | + * name based on the sign duplicates information that is already | ||
| 91 | + * in the delta. | ||
| 92 | + * | ||
| 93 | + * @param itemCode the catalog item code (validated cross-PBC via | ||
| 94 | + * CatalogApi inside the implementation). | ||
| 95 | + * @param locationCode the inventory location code (NOT id, by | ||
| 96 | + * convention every cross-PBC reference uses codes). | ||
| 97 | + * @param delta the signed quantity to add (positive) or subtract | ||
| 98 | + * (negative). Zero is rejected. | ||
| 99 | + * @param reason one of the closed set documented above. | ||
| 100 | + * @param reference free-form caller-supplied tag, recorded on the | ||
| 101 | + * ledger row. Convention: `<source>:<code>` (e.g. `SO:SO-2026-0001`, | ||
| 102 | + * `PR:PR-2026-0042`). May be null for unattached movements. | ||
| 103 | + * @return the resulting [StockBalanceRef] AFTER the movement. | ||
| 104 | + * @throws IllegalArgumentException when the item is unknown, the | ||
| 105 | + * location code is unknown, the delta is zero, the delta sign | ||
| 106 | + * disagrees with the reason, or the resulting balance would be | ||
| 107 | + * negative. Caller-side `try { } catch (IllegalArgumentException)` | ||
| 108 | + * is the right shape for plug-in code. | ||
| 109 | + */ | ||
| 110 | + fun recordMovement( | ||
| 111 | + itemCode: String, | ||
| 112 | + locationCode: String, | ||
| 113 | + delta: BigDecimal, | ||
| 114 | + reason: String, | ||
| 115 | + reference: String? = null, | ||
| 116 | + ): StockBalanceRef | ||
| 62 | } | 117 | } |
| 63 | 118 | ||
| 64 | /** | 119 | /** |
distribution/src/main/resources/db/changelog/master.xml
| @@ -17,5 +17,6 @@ | @@ -17,5 +17,6 @@ | ||
| 17 | <include file="classpath:db/changelog/pbc-catalog/001-catalog-init.xml"/> | 17 | <include file="classpath:db/changelog/pbc-catalog/001-catalog-init.xml"/> |
| 18 | <include file="classpath:db/changelog/pbc-partners/001-partners-init.xml"/> | 18 | <include file="classpath:db/changelog/pbc-partners/001-partners-init.xml"/> |
| 19 | <include file="classpath:db/changelog/pbc-inventory/001-inventory-init.xml"/> | 19 | <include file="classpath:db/changelog/pbc-inventory/001-inventory-init.xml"/> |
| 20 | + <include file="classpath:db/changelog/pbc-inventory/002-inventory-movement-ledger.xml"/> | ||
| 20 | <include file="classpath:db/changelog/pbc-orders-sales/001-orders-sales-init.xml"/> | 21 | <include file="classpath:db/changelog/pbc-orders-sales/001-orders-sales-init.xml"/> |
| 21 | </databaseChangeLog> | 22 | </databaseChangeLog> |
distribution/src/main/resources/db/changelog/pbc-inventory/002-inventory-movement-ledger.xml
0 → 100644
| 1 | +<?xml version="1.0" encoding="UTF-8"?> | ||
| 2 | +<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" | ||
| 3 | + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| 4 | + xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog | ||
| 5 | + https://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.27.xsd"> | ||
| 6 | + | ||
| 7 | + <!-- | ||
| 8 | + pbc-inventory movement ledger (P5.3 follow-up). | ||
| 9 | + | ||
| 10 | + Adds: inventory__stock_movement. | ||
| 11 | + | ||
| 12 | + The ledger is the source of truth: inventory__stock_balance is | ||
| 13 | + a materialised view whose `quantity` equals the SUM of every | ||
| 14 | + `delta` for the same (item_code, location_id). The framework | ||
| 15 | + writes the movement row and the balance update in the SAME | ||
| 16 | + database transaction so the two cannot drift. | ||
| 17 | + | ||
| 18 | + This changeset is in its own file (002-...) instead of being | ||
| 19 | + appended to 001-inventory-init.xml because Liquibase tracks | ||
| 20 | + applied changesets by (file, id, author) tuple — once an | ||
| 21 | + environment has run 001 it will not pick up new changesets | ||
| 22 | + added inside that file (unless you increment the runOnChange | ||
| 23 | + attribute, which is fragile). The convention this codebase | ||
| 24 | + follows: each new schema cut goes in a new XML file in the | ||
| 25 | + PBC's changelog directory, registered in master.xml. | ||
| 26 | + --> | ||
| 27 | + | ||
| 28 | + <changeSet id="inventory-movement-ledger-001" author="vibe_erp"> | ||
| 29 | + <comment>Create inventory__stock_movement append-only ledger table</comment> | ||
| 30 | + <sql> | ||
| 31 | + CREATE TABLE inventory__stock_movement ( | ||
| 32 | + id uuid PRIMARY KEY, | ||
| 33 | + item_code varchar(64) NOT NULL, | ||
| 34 | + location_id uuid NOT NULL REFERENCES inventory__location(id), | ||
| 35 | + delta numeric(18,4) NOT NULL, | ||
| 36 | + reason varchar(32) NOT NULL, | ||
| 37 | + reference varchar(128), | ||
| 38 | + occurred_at timestamptz NOT NULL, | ||
| 39 | + created_at timestamptz NOT NULL, | ||
| 40 | + created_by varchar(128) NOT NULL, | ||
| 41 | + updated_at timestamptz NOT NULL, | ||
| 42 | + updated_by varchar(128) NOT NULL, | ||
| 43 | + version bigint NOT NULL DEFAULT 0, | ||
| 44 | + CONSTRAINT inventory__stock_movement_nonzero CHECK (delta <> 0) | ||
| 45 | + ); | ||
| 46 | + CREATE INDEX inventory__stock_movement_item_idx | ||
| 47 | + ON inventory__stock_movement (item_code); | ||
| 48 | + CREATE INDEX inventory__stock_movement_loc_idx | ||
| 49 | + ON inventory__stock_movement (location_id); | ||
| 50 | + CREATE INDEX inventory__stock_movement_item_loc_idx | ||
| 51 | + ON inventory__stock_movement (item_code, location_id); | ||
| 52 | + CREATE INDEX inventory__stock_movement_reference_idx | ||
| 53 | + ON inventory__stock_movement (reference); | ||
| 54 | + CREATE INDEX inventory__stock_movement_occurred_idx | ||
| 55 | + ON inventory__stock_movement (occurred_at); | ||
| 56 | + </sql> | ||
| 57 | + <rollback> | ||
| 58 | + DROP TABLE inventory__stock_movement; | ||
| 59 | + </rollback> | ||
| 60 | + </changeSet> | ||
| 61 | + | ||
| 62 | +</databaseChangeLog> |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/StockBalanceService.kt
| @@ -3,6 +3,7 @@ package org.vibeerp.pbc.inventory.application | @@ -3,6 +3,7 @@ package org.vibeerp.pbc.inventory.application | ||
| 3 | import org.springframework.stereotype.Service | 3 | import org.springframework.stereotype.Service |
| 4 | import org.springframework.transaction.annotation.Transactional | 4 | import org.springframework.transaction.annotation.Transactional |
| 5 | import org.vibeerp.api.v1.ext.catalog.CatalogApi | 5 | import org.vibeerp.api.v1.ext.catalog.CatalogApi |
| 6 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 6 | import org.vibeerp.pbc.inventory.domain.StockBalance | 7 | import org.vibeerp.pbc.inventory.domain.StockBalance |
| 7 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository | 8 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository |
| 8 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository | 9 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository |
| @@ -52,6 +53,7 @@ class StockBalanceService( | @@ -52,6 +53,7 @@ class StockBalanceService( | ||
| 52 | private val balances: StockBalanceJpaRepository, | 53 | private val balances: StockBalanceJpaRepository, |
| 53 | private val locations: LocationJpaRepository, | 54 | private val locations: LocationJpaRepository, |
| 54 | private val catalogApi: CatalogApi, | 55 | private val catalogApi: CatalogApi, |
| 56 | + private val stockMovementService: StockMovementService, | ||
| 55 | ) { | 57 | ) { |
| 56 | 58 | ||
| 57 | @Transactional(readOnly = true) | 59 | @Transactional(readOnly = true) |
| @@ -68,36 +70,49 @@ class StockBalanceService( | @@ -68,36 +70,49 @@ class StockBalanceService( | ||
| 68 | /** | 70 | /** |
| 69 | * Set the on-hand quantity for [itemCode] at [locationId] to | 71 | * Set the on-hand quantity for [itemCode] at [locationId] to |
| 70 | * [quantity], creating the balance row if it does not yet exist. | 72 | * [quantity], creating the balance row if it does not yet exist. |
| 73 | + * | ||
| 74 | + * **As of the movement-ledger landing**, this method is a thin | ||
| 75 | + * wrapper that delegates to [StockMovementService.record] with a | ||
| 76 | + * computed delta and reason=ADJUSTMENT. The wrapper exists so the | ||
| 77 | + * `POST /inventory/balances/adjust` REST endpoint keeps its | ||
| 78 | + * absolute-quantity semantics — operators want to type "the | ||
| 79 | + * shelf has 47 of these" not "decrease by 3" — while the | ||
| 80 | + * underlying ledger row still captures the change. The cross-PBC | ||
| 81 | + * item validation, the location validation, and the | ||
| 82 | + * negative-balance rejection now happen inside the movement | ||
| 83 | + * service; the ADJUSTMENT path inherits all of them. | ||
| 71 | */ | 84 | */ |
| 72 | fun adjust(itemCode: String, locationId: UUID, quantity: BigDecimal): StockBalance { | 85 | fun adjust(itemCode: String, locationId: UUID, quantity: BigDecimal): StockBalance { |
| 73 | require(quantity.signum() >= 0) { | 86 | require(quantity.signum() >= 0) { |
| 74 | "stock quantity must be non-negative (got $quantity)" | 87 | "stock quantity must be non-negative (got $quantity)" |
| 75 | } | 88 | } |
| 76 | 89 | ||
| 77 | - // Cross-PBC validation #1: the item must exist in the catalog | ||
| 78 | - // AND be active. CatalogApi returns null for inactive items — | ||
| 79 | - // see the rationale on CatalogApi.findItemByCode. | ||
| 80 | - catalogApi.findItemByCode(itemCode) | ||
| 81 | - ?: throw IllegalArgumentException( | ||
| 82 | - "item code '$itemCode' is not in the catalog (or is inactive)", | ||
| 83 | - ) | ||
| 84 | - | ||
| 85 | - // Local validation #2: the location must exist. We don't go | ||
| 86 | - // through a facade because the location lives in this PBC. | ||
| 87 | - require(locations.existsById(locationId)) { | ||
| 88 | - "location not found: $locationId" | ||
| 89 | - } | ||
| 90 | - | ||
| 91 | - // Upsert the balance row. Single-instance deployments mean the | ||
| 92 | - // SELECT-then-save race window is closed for practical purposes | ||
| 93 | - // — vibe_erp is single-tenant per process and the @Transactional | ||
| 94 | - // boundary makes the read-modify-write atomic at the DB level. | ||
| 95 | val existing = balances.findByItemCodeAndLocationId(itemCode, locationId) | 90 | val existing = balances.findByItemCodeAndLocationId(itemCode, locationId) |
| 96 | - return if (existing == null) { | ||
| 97 | - balances.save(StockBalance(itemCode, locationId, quantity)) | ||
| 98 | - } else { | ||
| 99 | - existing.quantity = quantity | ||
| 100 | - existing | 91 | + val currentQuantity = existing?.quantity ?: BigDecimal.ZERO |
| 92 | + val delta = quantity - currentQuantity | ||
| 93 | + if (delta.signum() == 0) { | ||
| 94 | + // No-op adjustment — don't insert a meaningless ledger row. | ||
| 95 | + // Validate the inputs anyway so the caller still gets the | ||
| 96 | + // same error shape on bad item / bad location, by going | ||
| 97 | + // through the catalog facade and the location check. | ||
| 98 | + catalogApi.findItemByCode(itemCode) | ||
| 99 | + ?: throw IllegalArgumentException( | ||
| 100 | + "item code '$itemCode' is not in the catalog (or is inactive)", | ||
| 101 | + ) | ||
| 102 | + require(locations.existsById(locationId)) { | ||
| 103 | + "location not found: $locationId" | ||
| 104 | + } | ||
| 105 | + return existing ?: balances.save(StockBalance(itemCode, locationId, quantity)) | ||
| 101 | } | 106 | } |
| 107 | + | ||
| 108 | + return stockMovementService.record( | ||
| 109 | + RecordMovementCommand( | ||
| 110 | + itemCode = itemCode, | ||
| 111 | + locationId = locationId, | ||
| 112 | + delta = delta, | ||
| 113 | + reason = MovementReason.ADJUSTMENT, | ||
| 114 | + reference = null, | ||
| 115 | + ), | ||
| 116 | + ) | ||
| 102 | } | 117 | } |
| 103 | } | 118 | } |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/StockMovementService.kt
0 → 100644
| 1 | +package org.vibeerp.pbc.inventory.application | ||
| 2 | + | ||
| 3 | +import org.springframework.stereotype.Service | ||
| 4 | +import org.springframework.transaction.annotation.Transactional | ||
| 5 | +import org.vibeerp.api.v1.ext.catalog.CatalogApi | ||
| 6 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 7 | +import org.vibeerp.pbc.inventory.domain.StockBalance | ||
| 8 | +import org.vibeerp.pbc.inventory.domain.StockMovement | ||
| 9 | +import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository | ||
| 10 | +import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository | ||
| 11 | +import org.vibeerp.pbc.inventory.infrastructure.StockMovementJpaRepository | ||
| 12 | +import java.math.BigDecimal | ||
| 13 | +import java.time.Instant | ||
| 14 | +import java.util.UUID | ||
| 15 | + | ||
| 16 | +/** | ||
| 17 | + * The framework's stock-movement ledger writer. | ||
| 18 | + * | ||
| 19 | + * **The ONE entry point for changing inventory.** Every other path | ||
| 20 | + * that wants to debit or credit stock — manual operator adjustment, | ||
| 21 | + * sales order shipment, future purchase receipt, transfer-out / | ||
| 22 | + * transfer-in legs — funnels through [record]. The method writes a | ||
| 23 | + * row to `inventory__stock_movement` AND updates (or creates) the | ||
| 24 | + * matching `inventory__stock_balance` row in the SAME database | ||
| 25 | + * transaction. The two cannot drift because the same `@Transactional` | ||
| 26 | + * commit governs both writes. | ||
| 27 | + * | ||
| 28 | + * **Cross-PBC validation.** Like [StockBalanceService.adjust], the | ||
| 29 | + * service injects [CatalogApi] and rejects unknown item codes | ||
| 30 | + * up-front with a meaningful 400. The location is validated locally | ||
| 31 | + * (intra-PBC). Both checks happen BEFORE the row is inserted, so a | ||
| 32 | + * rejected request leaves no ledger trace. | ||
| 33 | + * | ||
| 34 | + * **Sign convention enforced per reason.** Each [MovementReason] has | ||
| 35 | + * a documented sign — RECEIPT/PURCHASE_RECEIPT/TRANSFER_IN are | ||
| 36 | + * non-negative, ISSUE/SALES_SHIPMENT/TRANSFER_OUT are non-positive, | ||
| 37 | + * ADJUSTMENT can be either. A `RECEIPT` with `delta = -5` is a | ||
| 38 | + * caller bug, not silently coerced. Catching it here means the | ||
| 39 | + * audit log stays consistent: a row tagged "SALES_SHIPMENT" can | ||
| 40 | + * never be a positive number. | ||
| 41 | + * | ||
| 42 | + * **Negative balance rejection.** A movement that would push the | ||
| 43 | + * resulting balance below zero is rejected. This is the framework's | ||
| 44 | + * "you cannot ship more than you have" rule. The CHECK constraint | ||
| 45 | + * on `inventory__stock_balance.quantity >= 0` is the second wall; | ||
| 46 | + * this is the first one with a better error message. | ||
| 47 | + * | ||
| 48 | + * **Why no event publication in v1.** A future chunk wires | ||
| 49 | + * `StockMovementRecorded` events through the event bus so other | ||
| 50 | + * PBCs (production, finance) can react. Until then, downstream | ||
| 51 | + * consumers query the ledger directly. The omission is deliberate: | ||
| 52 | + * adding the event in this chunk would entangle the ledger | ||
| 53 | + * landing with the event-bus-as-cross-PBC-integration story, and | ||
| 54 | + * each deserves its own focused chunk. | ||
| 55 | + */ | ||
| 56 | +@Service | ||
| 57 | +@Transactional | ||
| 58 | +class StockMovementService( | ||
| 59 | + private val movements: StockMovementJpaRepository, | ||
| 60 | + private val balances: StockBalanceJpaRepository, | ||
| 61 | + private val locations: LocationJpaRepository, | ||
| 62 | + private val catalogApi: CatalogApi, | ||
| 63 | +) { | ||
| 64 | + | ||
| 65 | + @Transactional(readOnly = true) | ||
| 66 | + fun list(): List<StockMovement> = movements.findAll() | ||
| 67 | + | ||
| 68 | + @Transactional(readOnly = true) | ||
| 69 | + fun findByItemCode(itemCode: String): List<StockMovement> = | ||
| 70 | + movements.findByItemCode(itemCode) | ||
| 71 | + | ||
| 72 | + @Transactional(readOnly = true) | ||
| 73 | + fun findByLocationId(locationId: UUID): List<StockMovement> = | ||
| 74 | + movements.findByLocationId(locationId) | ||
| 75 | + | ||
| 76 | + @Transactional(readOnly = true) | ||
| 77 | + fun findByReference(reference: String): List<StockMovement> = | ||
| 78 | + movements.findByReference(reference) | ||
| 79 | + | ||
| 80 | + /** | ||
| 81 | + * Record a stock movement and update the matching balance, atomically. | ||
| 82 | + * | ||
| 83 | + * @return the resulting [StockBalance] row (the new quantity). | ||
| 84 | + * @throws IllegalArgumentException with a meaningful message if | ||
| 85 | + * the item is unknown, the location is unknown, the delta is | ||
| 86 | + * zero, the delta sign disagrees with the reason, or the | ||
| 87 | + * resulting balance would be negative. | ||
| 88 | + */ | ||
| 89 | + fun record(command: RecordMovementCommand): StockBalance { | ||
| 90 | + require(command.delta.signum() != 0) { | ||
| 91 | + "stock movement delta must be non-zero" | ||
| 92 | + } | ||
| 93 | + validateSign(command.reason, command.delta) | ||
| 94 | + | ||
| 95 | + // Cross-PBC validation: the item must exist in the catalog | ||
| 96 | + // (and be active — CatalogApi hides inactive items). | ||
| 97 | + catalogApi.findItemByCode(command.itemCode) | ||
| 98 | + ?: throw IllegalArgumentException( | ||
| 99 | + "item code '${command.itemCode}' is not in the catalog (or is inactive)", | ||
| 100 | + ) | ||
| 101 | + | ||
| 102 | + // Local validation: the location must exist. | ||
| 103 | + require(locations.existsById(command.locationId)) { | ||
| 104 | + "location not found: ${command.locationId}" | ||
| 105 | + } | ||
| 106 | + | ||
| 107 | + // Compute the new balance. SELECT-then-save is correct under | ||
| 108 | + // the @Transactional boundary; the same single-instance | ||
| 109 | + // single-tenant rationale applies as in StockBalanceService.adjust. | ||
| 110 | + val existing = balances.findByItemCodeAndLocationId(command.itemCode, command.locationId) | ||
| 111 | + val oldQuantity = existing?.quantity ?: BigDecimal.ZERO | ||
| 112 | + val newQuantity = oldQuantity + command.delta | ||
| 113 | + require(newQuantity.signum() >= 0) { | ||
| 114 | + "stock movement would push balance for '${command.itemCode}' at " + | ||
| 115 | + "location ${command.locationId} below zero (current=$oldQuantity, delta=${command.delta})" | ||
| 116 | + } | ||
| 117 | + | ||
| 118 | + // Insert the ledger row first, then update the balance. Both | ||
| 119 | + // happen in the same JPA transaction → either both commit or | ||
| 120 | + // neither does. | ||
| 121 | + movements.save( | ||
| 122 | + StockMovement( | ||
| 123 | + itemCode = command.itemCode, | ||
| 124 | + locationId = command.locationId, | ||
| 125 | + delta = command.delta, | ||
| 126 | + reason = command.reason, | ||
| 127 | + reference = command.reference, | ||
| 128 | + occurredAt = command.occurredAt ?: Instant.now(), | ||
| 129 | + ), | ||
| 130 | + ) | ||
| 131 | + | ||
| 132 | + return if (existing == null) { | ||
| 133 | + balances.save(StockBalance(command.itemCode, command.locationId, newQuantity)) | ||
| 134 | + } else { | ||
| 135 | + existing.quantity = newQuantity | ||
| 136 | + existing | ||
| 137 | + } | ||
| 138 | + } | ||
| 139 | + | ||
| 140 | + /** | ||
| 141 | + * Verify that [delta]'s sign matches the documented direction | ||
| 142 | + * for [reason]. ADJUSTMENT is the one reason that allows either | ||
| 143 | + * sign — operators correct mistakes in either direction. | ||
| 144 | + * | ||
| 145 | + * Throws [IllegalArgumentException] with a message that names | ||
| 146 | + * the reason and the expected direction so the caller knows | ||
| 147 | + * how to fix the request. | ||
| 148 | + */ | ||
| 149 | + private fun validateSign(reason: MovementReason, delta: BigDecimal) { | ||
| 150 | + val sign = delta.signum() // -1, 0, or 1; zero is rejected above | ||
| 151 | + val expected: String? = when (reason) { | ||
| 152 | + MovementReason.RECEIPT, | ||
| 153 | + MovementReason.PURCHASE_RECEIPT, | ||
| 154 | + MovementReason.TRANSFER_IN -> if (sign < 0) "non-negative" else null | ||
| 155 | + | ||
| 156 | + MovementReason.ISSUE, | ||
| 157 | + MovementReason.SALES_SHIPMENT, | ||
| 158 | + MovementReason.TRANSFER_OUT -> if (sign > 0) "non-positive" else null | ||
| 159 | + | ||
| 160 | + MovementReason.ADJUSTMENT -> null // either sign allowed | ||
| 161 | + } | ||
| 162 | + if (expected != null) { | ||
| 163 | + throw IllegalArgumentException( | ||
| 164 | + "movement reason $reason requires a $expected delta (got $delta)", | ||
| 165 | + ) | ||
| 166 | + } | ||
| 167 | + } | ||
| 168 | +} | ||
| 169 | + | ||
| 170 | +/** | ||
| 171 | + * Input shape for [StockMovementService.record]. Kept as a separate | ||
| 172 | + * data class so the REST DTO and the cross-PBC adapter can both | ||
| 173 | + * build it without sharing a controller-only request type. | ||
| 174 | + * | ||
| 175 | + * @property occurredAt when the physical stock actually moved. The | ||
| 176 | + * service defaults to "now" when this is null. Back-dated entries | ||
| 177 | + * (operator types in yesterday's count today) provide it explicitly; | ||
| 178 | + * the audit columns still record when the row was inserted. | ||
| 179 | + */ | ||
| 180 | +data class RecordMovementCommand( | ||
| 181 | + val itemCode: String, | ||
| 182 | + val locationId: UUID, | ||
| 183 | + val delta: BigDecimal, | ||
| 184 | + val reason: MovementReason, | ||
| 185 | + val reference: String? = null, | ||
| 186 | + val occurredAt: Instant? = null, | ||
| 187 | +) |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/StockMovement.kt
0 → 100644
| 1 | +package org.vibeerp.pbc.inventory.domain | ||
| 2 | + | ||
| 3 | +import jakarta.persistence.Column | ||
| 4 | +import jakarta.persistence.Entity | ||
| 5 | +import jakarta.persistence.EnumType | ||
| 6 | +import jakarta.persistence.Enumerated | ||
| 7 | +import jakarta.persistence.Table | ||
| 8 | +import org.vibeerp.platform.persistence.audit.AuditedJpaEntity | ||
| 9 | +import java.math.BigDecimal | ||
| 10 | +import java.time.Instant | ||
| 11 | +import java.util.UUID | ||
| 12 | + | ||
| 13 | +/** | ||
| 14 | + * One row in the inventory ledger — an append-only record of a single | ||
| 15 | + * change to a single (item × location) cell. | ||
| 16 | + * | ||
| 17 | + * **The ledger is the source of truth.** [StockBalance] is a | ||
| 18 | + * materialised view: its `quantity` column equals the SUM of every | ||
| 19 | + * `delta` in [StockMovement] for the same (item_code, location_id). | ||
| 20 | + * The framework writes the movement row and the balance update in | ||
| 21 | + * the SAME database transaction (see [StockMovementService.record]), | ||
| 22 | + * so the two cannot drift. The ledger exists to answer the audit | ||
| 23 | + * question "who moved this stock and when, and why?" — the balance | ||
| 24 | + * exists to answer the operational question "how many do I have | ||
| 25 | + * right now?" cheaply. | ||
| 26 | + * | ||
| 27 | + * **Append-only.** No update, no delete. A reversal is a NEW row | ||
| 28 | + * with the negated delta and a reason of ADJUSTMENT (or, when the | ||
| 29 | + * reversal corresponds to a real business event, the appropriate | ||
| 30 | + * reason — RETURN-class reasons land later). The framework gives | ||
| 31 | + * up nothing by enforcing append-only: the [AuditedJpaEntity]'s | ||
| 32 | + * `created_by` / `created_at` columns capture who and when, and the | ||
| 33 | + * reason + reference together capture WHY at the level of detail | ||
| 34 | + * the integrating PBC can supply. | ||
| 35 | + * | ||
| 36 | + * **Why `delta` is signed instead of separate IN/OUT columns:** the | ||
| 37 | + * arithmetic stays trivial (`SUM(delta)` is the balance), the type | ||
| 38 | + * system rejects "negative receipt" / "positive issue" mistakes via | ||
| 39 | + * the [MovementReason] enum's documented sign convention, and there | ||
| 40 | + * is exactly one column to query when reporting on net activity. | ||
| 41 | + * | ||
| 42 | + * **Why `reference` is a free-form string** instead of a typed FK: | ||
| 43 | + * the same row needs to point at a sales order code (`SO:SO-2026-0001`), | ||
| 44 | + * a purchase receipt code (`PR:PR-2026-0042`), a transfer order | ||
| 45 | + * (`TFR:TFR-2026-0007`), or nothing at all (manual adjustment). A | ||
| 46 | + * varchar with a documented prefix convention is the only shape | ||
| 47 | + * that fits all of those without forcing every PBC to declare its | ||
| 48 | + * row in a polymorphic table. The prefix `<source>:<code>` is the | ||
| 49 | + * convention; consumers parse it with `split(':')` if they care. | ||
| 50 | + * | ||
| 51 | + * **Why `occurred_at` is separate from `created_at`:** the audit | ||
| 52 | + * column says "when did the system record this row"; `occurred_at` | ||
| 53 | + * says "when did the physical stock move". The two are usually | ||
| 54 | + * within milliseconds of each other, but a back-dated reconciliation | ||
| 55 | + * (operator types in yesterday's count today) needs the distinction. | ||
| 56 | + * Defaults to `created_at` when the caller doesn't provide it. | ||
| 57 | + */ | ||
| 58 | +@Entity | ||
| 59 | +@Table(name = "inventory__stock_movement") | ||
| 60 | +class StockMovement( | ||
| 61 | + itemCode: String, | ||
| 62 | + locationId: UUID, | ||
| 63 | + delta: BigDecimal, | ||
| 64 | + reason: MovementReason, | ||
| 65 | + reference: String? = null, | ||
| 66 | + occurredAt: Instant = Instant.now(), | ||
| 67 | +) : AuditedJpaEntity() { | ||
| 68 | + | ||
| 69 | + @Column(name = "item_code", nullable = false, length = 64) | ||
| 70 | + var itemCode: String = itemCode | ||
| 71 | + | ||
| 72 | + @Column(name = "location_id", nullable = false) | ||
| 73 | + var locationId: UUID = locationId | ||
| 74 | + | ||
| 75 | + /** | ||
| 76 | + * Signed quantity. Positive for receipts and transfers-in, | ||
| 77 | + * negative for issues and shipments. Zero is rejected by the | ||
| 78 | + * service — a no-op movement is meaningless and clutters the | ||
| 79 | + * ledger. | ||
| 80 | + */ | ||
| 81 | + @Column(name = "delta", nullable = false, precision = 18, scale = 4) | ||
| 82 | + var delta: BigDecimal = delta | ||
| 83 | + | ||
| 84 | + @Enumerated(EnumType.STRING) | ||
| 85 | + @Column(name = "reason", nullable = false, length = 32) | ||
| 86 | + var reason: MovementReason = reason | ||
| 87 | + | ||
| 88 | + @Column(name = "reference", nullable = true, length = 128) | ||
| 89 | + var reference: String? = reference | ||
| 90 | + | ||
| 91 | + @Column(name = "occurred_at", nullable = false) | ||
| 92 | + var occurredAt: Instant = occurredAt | ||
| 93 | + | ||
| 94 | + override fun toString(): String = | ||
| 95 | + "StockMovement(id=$id, item='$itemCode', loc=$locationId, delta=$delta, reason=$reason, ref='$reference')" | ||
| 96 | +} | ||
| 97 | + | ||
| 98 | +/** | ||
| 99 | + * Why a stock movement happened. | ||
| 100 | + * | ||
| 101 | + * Each value carries a documented sign convention so application | ||
| 102 | + * code does not have to remember "is a SALES_SHIPMENT positive or | ||
| 103 | + * negative". The service validates that the sign of the delta | ||
| 104 | + * matches the documented direction; calling code that gets this | ||
| 105 | + * wrong gets a meaningful error instead of a corrupted balance. | ||
| 106 | + * | ||
| 107 | + * - **RECEIPT** — non-PO receipt (e.g. inventory found, customer return). Δ ≥ 0 | ||
| 108 | + * - **ISSUE** — non-SO issue (e.g. write-off, internal use). Δ ≤ 0 | ||
| 109 | + * - **ADJUSTMENT** — operator-driven correction (cycle count, mistake fix). Δ either sign | ||
| 110 | + * - **SALES_SHIPMENT** — line item shipped on a sales order. Δ ≤ 0 | ||
| 111 | + * - **PURCHASE_RECEIPT** — line item received against a purchase order. Δ ≥ 0 | ||
| 112 | + * - **TRANSFER_OUT** — leg-1 of an inter-warehouse transfer. Δ ≤ 0 | ||
| 113 | + * - **TRANSFER_IN** — leg-2 of an inter-warehouse transfer. Δ ≥ 0 | ||
| 114 | + * | ||
| 115 | + * Stored as string in the DB so adding values later is non-breaking | ||
| 116 | + * for clients reading the column with raw SQL. | ||
| 117 | + */ | ||
| 118 | +enum class MovementReason { | ||
| 119 | + RECEIPT, | ||
| 120 | + ISSUE, | ||
| 121 | + ADJUSTMENT, | ||
| 122 | + SALES_SHIPMENT, | ||
| 123 | + PURCHASE_RECEIPT, | ||
| 124 | + TRANSFER_OUT, | ||
| 125 | + TRANSFER_IN, | ||
| 126 | +} |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/ext/InventoryApiAdapter.kt
| @@ -5,6 +5,9 @@ import org.springframework.transaction.annotation.Transactional | @@ -5,6 +5,9 @@ import org.springframework.transaction.annotation.Transactional | ||
| 5 | import org.vibeerp.api.v1.core.Id | 5 | import org.vibeerp.api.v1.core.Id |
| 6 | import org.vibeerp.api.v1.ext.inventory.InventoryApi | 6 | import org.vibeerp.api.v1.ext.inventory.InventoryApi |
| 7 | import org.vibeerp.api.v1.ext.inventory.StockBalanceRef | 7 | import org.vibeerp.api.v1.ext.inventory.StockBalanceRef |
| 8 | +import org.vibeerp.pbc.inventory.application.RecordMovementCommand | ||
| 9 | +import org.vibeerp.pbc.inventory.application.StockMovementService | ||
| 10 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 8 | import org.vibeerp.pbc.inventory.domain.StockBalance | 11 | import org.vibeerp.pbc.inventory.domain.StockBalance |
| 9 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository | 12 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository |
| 10 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository | 13 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository |
| @@ -42,6 +45,7 @@ import java.math.BigDecimal | @@ -42,6 +45,7 @@ import java.math.BigDecimal | ||
| 42 | class InventoryApiAdapter( | 45 | class InventoryApiAdapter( |
| 43 | private val balances: StockBalanceJpaRepository, | 46 | private val balances: StockBalanceJpaRepository, |
| 44 | private val locations: LocationJpaRepository, | 47 | private val locations: LocationJpaRepository, |
| 48 | + private val stockMovementService: StockMovementService, | ||
| 45 | ) : InventoryApi { | 49 | ) : InventoryApi { |
| 46 | 50 | ||
| 47 | override fun findStockBalance(itemCode: String, locationCode: String): StockBalanceRef? { | 51 | override fun findStockBalance(itemCode: String, locationCode: String): StockBalanceRef? { |
| @@ -54,6 +58,50 @@ class InventoryApiAdapter( | @@ -54,6 +58,50 @@ class InventoryApiAdapter( | ||
| 54 | balances.findByItemCode(itemCode) | 58 | balances.findByItemCode(itemCode) |
| 55 | .fold(BigDecimal.ZERO) { acc, row -> acc + row.quantity } | 59 | .fold(BigDecimal.ZERO) { acc, row -> acc + row.quantity } |
| 56 | 60 | ||
| 61 | + /** | ||
| 62 | + * Record a stock movement on behalf of a cross-PBC caller. | ||
| 63 | + * Resolves the location code → id, parses the reason string into | ||
| 64 | + * the local enum, and delegates to the [StockMovementService]. | ||
| 65 | + * | ||
| 66 | + * The location lookup happens here (not in the service) because | ||
| 67 | + * the service takes a UUID — that's the intra-PBC contract. The | ||
| 68 | + * facade is the boundary that converts between external (codes) | ||
| 69 | + * and internal (ids). | ||
| 70 | + * | ||
| 71 | + * Marked `@Transactional` (read-write, NOT readOnly) so the | ||
| 72 | + * service's write happens inside this method's transaction. The | ||
| 73 | + * class-level `readOnly = true` would otherwise turn the write | ||
| 74 | + * into a no-op + flush failure. | ||
| 75 | + */ | ||
| 76 | + @Transactional | ||
| 77 | + override fun recordMovement( | ||
| 78 | + itemCode: String, | ||
| 79 | + locationCode: String, | ||
| 80 | + delta: BigDecimal, | ||
| 81 | + reason: String, | ||
| 82 | + reference: String?, | ||
| 83 | + ): StockBalanceRef { | ||
| 84 | + val location = locations.findByCode(locationCode) | ||
| 85 | + ?: throw IllegalArgumentException("location code '$locationCode' is not in the inventory directory") | ||
| 86 | + val movementReason = try { | ||
| 87 | + MovementReason.valueOf(reason) | ||
| 88 | + } catch (ex: IllegalArgumentException) { | ||
| 89 | + throw IllegalArgumentException( | ||
| 90 | + "unknown movement reason '$reason' (expected one of ${MovementReason.values().joinToString { it.name }})", | ||
| 91 | + ) | ||
| 92 | + } | ||
| 93 | + val balance = stockMovementService.record( | ||
| 94 | + RecordMovementCommand( | ||
| 95 | + itemCode = itemCode, | ||
| 96 | + locationId = location.id, | ||
| 97 | + delta = delta, | ||
| 98 | + reason = movementReason, | ||
| 99 | + reference = reference, | ||
| 100 | + ), | ||
| 101 | + ) | ||
| 102 | + return balance.toRef(locationCode) | ||
| 103 | + } | ||
| 104 | + | ||
| 57 | private fun StockBalance.toRef(locationCode: String): StockBalanceRef = StockBalanceRef( | 105 | private fun StockBalance.toRef(locationCode: String): StockBalanceRef = StockBalanceRef( |
| 58 | id = Id<StockBalanceRef>(this.id), | 106 | id = Id<StockBalanceRef>(this.id), |
| 59 | itemCode = this.itemCode, | 107 | itemCode = this.itemCode, |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/http/StockMovementController.kt
0 → 100644
| 1 | +package org.vibeerp.pbc.inventory.http | ||
| 2 | + | ||
| 3 | +import jakarta.validation.Valid | ||
| 4 | +import jakarta.validation.constraints.NotBlank | ||
| 5 | +import jakarta.validation.constraints.NotNull | ||
| 6 | +import jakarta.validation.constraints.Size | ||
| 7 | +import org.springframework.http.HttpStatus | ||
| 8 | +import org.springframework.web.bind.annotation.GetMapping | ||
| 9 | +import org.springframework.web.bind.annotation.PostMapping | ||
| 10 | +import org.springframework.web.bind.annotation.RequestBody | ||
| 11 | +import org.springframework.web.bind.annotation.RequestMapping | ||
| 12 | +import org.springframework.web.bind.annotation.RequestParam | ||
| 13 | +import org.springframework.web.bind.annotation.ResponseStatus | ||
| 14 | +import org.springframework.web.bind.annotation.RestController | ||
| 15 | +import org.vibeerp.pbc.inventory.application.RecordMovementCommand | ||
| 16 | +import org.vibeerp.pbc.inventory.application.StockMovementService | ||
| 17 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 18 | +import org.vibeerp.pbc.inventory.domain.StockMovement | ||
| 19 | +import org.vibeerp.platform.security.authz.RequirePermission | ||
| 20 | +import java.math.BigDecimal | ||
| 21 | +import java.time.Instant | ||
| 22 | +import java.util.UUID | ||
| 23 | + | ||
| 24 | +/** | ||
| 25 | + * REST API for the inventory ledger. | ||
| 26 | + * | ||
| 27 | + * Mounted at `/api/v1/inventory/movements`. | ||
| 28 | + * | ||
| 29 | + * **GET** is plain authenticated — reading historical movements is | ||
| 30 | + * a normal operator action. Filters by itemCode, locationId, or | ||
| 31 | + * reference (so the SPA can render "all movements caused by sales | ||
| 32 | + * order SO-2026-0001"). | ||
| 33 | + * | ||
| 34 | + * **POST** requires `inventory.stock.adjust` (the same permission | ||
| 35 | + * the absolute-quantity adjust endpoint uses). The two endpoints | ||
| 36 | + * exist side-by-side: the balance endpoint is for "the shelf has | ||
| 37 | + * 47", the movement endpoint is for "we received 3 more". Both | ||
| 38 | + * land as ledger rows; the framework treats them identically below | ||
| 39 | + * the controller layer. | ||
| 40 | + */ | ||
| 41 | +@RestController | ||
| 42 | +@RequestMapping("/api/v1/inventory/movements") | ||
| 43 | +class StockMovementController( | ||
| 44 | + private val stockMovementService: StockMovementService, | ||
| 45 | +) { | ||
| 46 | + | ||
| 47 | + @GetMapping | ||
| 48 | + fun list( | ||
| 49 | + @RequestParam(required = false) itemCode: String?, | ||
| 50 | + @RequestParam(required = false) locationId: UUID?, | ||
| 51 | + @RequestParam(required = false) reference: String?, | ||
| 52 | + ): List<StockMovementResponse> { | ||
| 53 | + val rows = when { | ||
| 54 | + reference != null -> stockMovementService.findByReference(reference) | ||
| 55 | + itemCode != null && locationId != null -> stockMovementService.findByItemCode(itemCode) | ||
| 56 | + .filter { it.locationId == locationId } | ||
| 57 | + itemCode != null -> stockMovementService.findByItemCode(itemCode) | ||
| 58 | + locationId != null -> stockMovementService.findByLocationId(locationId) | ||
| 59 | + else -> stockMovementService.list() | ||
| 60 | + } | ||
| 61 | + return rows.map { it.toResponse() } | ||
| 62 | + } | ||
| 63 | + | ||
| 64 | + @PostMapping | ||
| 65 | + @ResponseStatus(HttpStatus.CREATED) | ||
| 66 | + @RequirePermission("inventory.stock.adjust") | ||
| 67 | + fun record(@RequestBody @Valid request: RecordMovementRequest): StockMovementResponse { | ||
| 68 | + val balance = stockMovementService.record( | ||
| 69 | + RecordMovementCommand( | ||
| 70 | + itemCode = request.itemCode, | ||
| 71 | + locationId = request.locationId, | ||
| 72 | + delta = request.delta, | ||
| 73 | + reason = request.reason, | ||
| 74 | + reference = request.reference, | ||
| 75 | + occurredAt = request.occurredAt, | ||
| 76 | + ), | ||
| 77 | + ) | ||
| 78 | + // The movement row is the response, not the balance — callers | ||
| 79 | + // who want the resulting balance hit the balances endpoint. | ||
| 80 | + // We need to fetch the freshly-inserted movement; for the | ||
| 81 | + // happy path the latest movement for (item, location) IS this | ||
| 82 | + // one, but ordering by occurred_at desc would be more honest. | ||
| 83 | + // v1 returns a synthesised response from the request fields | ||
| 84 | + // because the request itself fully describes the row that | ||
| 85 | + // was inserted (apart from id/audit cols, which are not | ||
| 86 | + // useful in a creation response). | ||
| 87 | + return StockMovementResponse( | ||
| 88 | + id = UUID(0L, 0L), // sentinel — see comment above | ||
| 89 | + itemCode = request.itemCode, | ||
| 90 | + locationId = request.locationId, | ||
| 91 | + delta = request.delta, | ||
| 92 | + reason = request.reason, | ||
| 93 | + reference = request.reference, | ||
| 94 | + occurredAt = request.occurredAt ?: Instant.now(), | ||
| 95 | + resultingQuantity = balance.quantity, | ||
| 96 | + ) | ||
| 97 | + } | ||
| 98 | +} | ||
| 99 | + | ||
| 100 | +// ─── DTOs ──────────────────────────────────────────────────────────── | ||
| 101 | + | ||
| 102 | +data class RecordMovementRequest( | ||
| 103 | + @field:NotBlank @field:Size(max = 64) val itemCode: String, | ||
| 104 | + @field:NotNull val locationId: UUID, | ||
| 105 | + @field:NotNull val delta: BigDecimal, | ||
| 106 | + @field:NotNull val reason: MovementReason, | ||
| 107 | + @field:Size(max = 128) val reference: String? = null, | ||
| 108 | + val occurredAt: Instant? = null, | ||
| 109 | +) | ||
| 110 | + | ||
| 111 | +data class StockMovementResponse( | ||
| 112 | + val id: UUID, | ||
| 113 | + val itemCode: String, | ||
| 114 | + val locationId: UUID, | ||
| 115 | + val delta: BigDecimal, | ||
| 116 | + val reason: MovementReason, | ||
| 117 | + val reference: String?, | ||
| 118 | + val occurredAt: Instant, | ||
| 119 | + /** | ||
| 120 | + * The balance for (itemCode, locationId) AFTER this movement was | ||
| 121 | + * applied. Included on the create response so callers don't need | ||
| 122 | + * a follow-up GET to the balances endpoint. | ||
| 123 | + */ | ||
| 124 | + val resultingQuantity: BigDecimal, | ||
| 125 | +) | ||
| 126 | + | ||
| 127 | +private fun StockMovement.toResponse() = StockMovementResponse( | ||
| 128 | + id = this.id, | ||
| 129 | + itemCode = this.itemCode, | ||
| 130 | + locationId = this.locationId, | ||
| 131 | + delta = this.delta, | ||
| 132 | + reason = this.reason, | ||
| 133 | + reference = this.reference, | ||
| 134 | + occurredAt = this.occurredAt, | ||
| 135 | + // The list endpoint doesn't compute the at-the-time balance — | ||
| 136 | + // that would require a SUM scan. Set to ZERO and let the SPA | ||
| 137 | + // compute it from the sequence if it cares. | ||
| 138 | + resultingQuantity = BigDecimal.ZERO, | ||
| 139 | +) |
pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/infrastructure/StockMovementJpaRepository.kt
0 → 100644
| 1 | +package org.vibeerp.pbc.inventory.infrastructure | ||
| 2 | + | ||
| 3 | +import org.springframework.data.jpa.repository.JpaRepository | ||
| 4 | +import org.springframework.stereotype.Repository | ||
| 5 | +import org.vibeerp.pbc.inventory.domain.StockMovement | ||
| 6 | +import java.util.UUID | ||
| 7 | + | ||
| 8 | +/** | ||
| 9 | + * Spring Data JPA repository for [StockMovement]. | ||
| 10 | + * | ||
| 11 | + * Append-only ledger access. The repository deliberately does NOT | ||
| 12 | + * expose `delete` (although JpaRepository inherits one) — the | ||
| 13 | + * service layer never calls it, and the database table will grow | ||
| 14 | + * a row-level CHECK constraint in a future hardening pass to | ||
| 15 | + * prevent accidental deletes via raw SQL too. | ||
| 16 | + * | ||
| 17 | + * The query methods cover the common ledger reads: by item, by | ||
| 18 | + * location, by reference (e.g. "all movements caused by sales | ||
| 19 | + * order SO-2026-0001"), and the (item, location) cell view that | ||
| 20 | + * the SPA's stock-detail screen wants. | ||
| 21 | + */ | ||
| 22 | +@Repository | ||
| 23 | +interface StockMovementJpaRepository : JpaRepository<StockMovement, UUID> { | ||
| 24 | + | ||
| 25 | + fun findByItemCode(itemCode: String): List<StockMovement> | ||
| 26 | + | ||
| 27 | + fun findByLocationId(locationId: UUID): List<StockMovement> | ||
| 28 | + | ||
| 29 | + fun findByItemCodeAndLocationId(itemCode: String, locationId: UUID): List<StockMovement> | ||
| 30 | + | ||
| 31 | + fun findByReference(reference: String): List<StockMovement> | ||
| 32 | +} |
pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/StockBalanceServiceTest.kt
| @@ -14,17 +14,30 @@ import org.junit.jupiter.api.Test | @@ -14,17 +14,30 @@ import org.junit.jupiter.api.Test | ||
| 14 | import org.vibeerp.api.v1.core.Id | 14 | import org.vibeerp.api.v1.core.Id |
| 15 | import org.vibeerp.api.v1.ext.catalog.CatalogApi | 15 | import org.vibeerp.api.v1.ext.catalog.CatalogApi |
| 16 | import org.vibeerp.api.v1.ext.catalog.ItemRef | 16 | import org.vibeerp.api.v1.ext.catalog.ItemRef |
| 17 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 17 | import org.vibeerp.pbc.inventory.domain.StockBalance | 18 | import org.vibeerp.pbc.inventory.domain.StockBalance |
| 18 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository | 19 | import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository |
| 19 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository | 20 | import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository |
| 20 | import java.math.BigDecimal | 21 | import java.math.BigDecimal |
| 21 | import java.util.UUID | 22 | import java.util.UUID |
| 22 | 23 | ||
| 24 | +/** | ||
| 25 | + * Tests the post-ledger refactor behavior of StockBalanceService: | ||
| 26 | + * adjust() delegates to StockMovementService.record() with a | ||
| 27 | + * computed delta, and the no-op adjustment short-circuit path. | ||
| 28 | + * | ||
| 29 | + * The cross-PBC item validation, location validation, and | ||
| 30 | + * negative-balance rejection now live in StockMovementService and | ||
| 31 | + * are exercised by [StockMovementServiceTest]. This test suite | ||
| 32 | + * deliberately mocks StockMovementService so the unit boundary is | ||
| 33 | + * "what does adjust() do given the movement service". | ||
| 34 | + */ | ||
| 23 | class StockBalanceServiceTest { | 35 | class StockBalanceServiceTest { |
| 24 | 36 | ||
| 25 | private lateinit var balances: StockBalanceJpaRepository | 37 | private lateinit var balances: StockBalanceJpaRepository |
| 26 | private lateinit var locations: LocationJpaRepository | 38 | private lateinit var locations: LocationJpaRepository |
| 27 | private lateinit var catalogApi: CatalogApi | 39 | private lateinit var catalogApi: CatalogApi |
| 40 | + private lateinit var stockMovementService: StockMovementService | ||
| 28 | private lateinit var service: StockBalanceService | 41 | private lateinit var service: StockBalanceService |
| 29 | 42 | ||
| 30 | @BeforeEach | 43 | @BeforeEach |
| @@ -32,7 +45,8 @@ class StockBalanceServiceTest { | @@ -32,7 +45,8 @@ class StockBalanceServiceTest { | ||
| 32 | balances = mockk() | 45 | balances = mockk() |
| 33 | locations = mockk() | 46 | locations = mockk() |
| 34 | catalogApi = mockk() | 47 | catalogApi = mockk() |
| 35 | - service = StockBalanceService(balances, locations, catalogApi) | 48 | + stockMovementService = mockk() |
| 49 | + service = StockBalanceService(balances, locations, catalogApi, stockMovementService) | ||
| 36 | } | 50 | } |
| 37 | 51 | ||
| 38 | private fun stubItem(code: String) { | 52 | private fun stubItem(code: String) { |
| @@ -47,71 +61,84 @@ class StockBalanceServiceTest { | @@ -47,71 +61,84 @@ class StockBalanceServiceTest { | ||
| 47 | } | 61 | } |
| 48 | 62 | ||
| 49 | @Test | 63 | @Test |
| 50 | - fun `adjust rejects unknown item code via CatalogApi seam`() { | ||
| 51 | - every { catalogApi.findItemByCode("FAKE") } returns null | ||
| 52 | - | 64 | + fun `adjust rejects negative quantity up front`() { |
| 53 | assertFailure { | 65 | assertFailure { |
| 54 | - service.adjust("FAKE", UUID.randomUUID(), BigDecimal("10")) | 66 | + service.adjust("SKU-1", UUID.randomUUID(), BigDecimal("-1")) |
| 55 | } | 67 | } |
| 56 | .isInstanceOf(IllegalArgumentException::class) | 68 | .isInstanceOf(IllegalArgumentException::class) |
| 57 | - .hasMessage("item code 'FAKE' is not in the catalog (or is inactive)") | 69 | + .hasMessage("stock quantity must be non-negative (got -1)") |
| 70 | + verify(exactly = 0) { stockMovementService.record(any()) } | ||
| 58 | } | 71 | } |
| 59 | 72 | ||
| 60 | @Test | 73 | @Test |
| 61 | - fun `adjust rejects unknown location id`() { | ||
| 62 | - stubItem("SKU-1") | 74 | + fun `adjust delegates to record with the computed delta when increasing`() { |
| 63 | val locId = UUID.randomUUID() | 75 | val locId = UUID.randomUUID() |
| 64 | - every { locations.existsById(locId) } returns false | 76 | + val existing = StockBalance("SKU-1", locId, BigDecimal("10")).also { it.id = UUID.randomUUID() } |
| 77 | + val expectedAfter = StockBalance("SKU-1", locId, BigDecimal("25")).also { it.id = existing.id } | ||
| 78 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns existing | ||
| 79 | + val captured = slot<RecordMovementCommand>() | ||
| 80 | + every { stockMovementService.record(capture(captured)) } returns expectedAfter | ||
| 65 | 81 | ||
| 66 | - assertFailure { | ||
| 67 | - service.adjust("SKU-1", locId, BigDecimal("10")) | ||
| 68 | - } | ||
| 69 | - .isInstanceOf(IllegalArgumentException::class) | ||
| 70 | - .hasMessage("location not found: $locId") | 82 | + val result = service.adjust("SKU-1", locId, BigDecimal("25")) |
| 83 | + | ||
| 84 | + assertThat(captured.captured.itemCode).isEqualTo("SKU-1") | ||
| 85 | + assertThat(captured.captured.locationId).isEqualTo(locId) | ||
| 86 | + // 25 - 10 = +15 | ||
| 87 | + assertThat(captured.captured.delta).isEqualTo(BigDecimal("15")) | ||
| 88 | + assertThat(captured.captured.reason).isEqualTo(MovementReason.ADJUSTMENT) | ||
| 89 | + assertThat(result.quantity).isEqualTo(BigDecimal("25")) | ||
| 71 | } | 90 | } |
| 72 | 91 | ||
| 73 | @Test | 92 | @Test |
| 74 | - fun `adjust rejects negative quantity before catalog lookup`() { | ||
| 75 | - // Negative quantity is rejected by an early require() so the | ||
| 76 | - // CatalogApi mock is never invoked. | ||
| 77 | - assertFailure { | ||
| 78 | - service.adjust("SKU-1", UUID.randomUUID(), BigDecimal("-1")) | ||
| 79 | - } | ||
| 80 | - .isInstanceOf(IllegalArgumentException::class) | ||
| 81 | - verify(exactly = 0) { catalogApi.findItemByCode(any()) } | 93 | + fun `adjust delegates with a negative delta when decreasing`() { |
| 94 | + val locId = UUID.randomUUID() | ||
| 95 | + val existing = StockBalance("SKU-1", locId, BigDecimal("100")).also { it.id = UUID.randomUUID() } | ||
| 96 | + val expectedAfter = StockBalance("SKU-1", locId, BigDecimal("30")).also { it.id = existing.id } | ||
| 97 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns existing | ||
| 98 | + val captured = slot<RecordMovementCommand>() | ||
| 99 | + every { stockMovementService.record(capture(captured)) } returns expectedAfter | ||
| 100 | + | ||
| 101 | + service.adjust("SKU-1", locId, BigDecimal("30")) | ||
| 102 | + | ||
| 103 | + // 30 - 100 = -70 | ||
| 104 | + assertThat(captured.captured.delta).isEqualTo(BigDecimal("-70")) | ||
| 105 | + assertThat(captured.captured.reason).isEqualTo(MovementReason.ADJUSTMENT) | ||
| 82 | } | 106 | } |
| 83 | 107 | ||
| 84 | @Test | 108 | @Test |
| 85 | - fun `adjust creates a new balance row when none exists`() { | ||
| 86 | - stubItem("SKU-1") | 109 | + fun `adjust to the SAME quantity is a no-op (no ledger row)`() { |
| 87 | val locId = UUID.randomUUID() | 110 | val locId = UUID.randomUUID() |
| 88 | - val saved = slot<StockBalance>() | 111 | + val existing = StockBalance("SKU-1", locId, BigDecimal("42")).also { it.id = UUID.randomUUID() } |
| 112 | + stubItem("SKU-1") | ||
| 113 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns existing | ||
| 89 | every { locations.existsById(locId) } returns true | 114 | every { locations.existsById(locId) } returns true |
| 90 | - every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns null | ||
| 91 | - every { balances.save(capture(saved)) } answers { saved.captured } | ||
| 92 | 115 | ||
| 93 | - val result = service.adjust("SKU-1", locId, BigDecimal("42.5")) | 116 | + val result = service.adjust("SKU-1", locId, BigDecimal("42")) |
| 94 | 117 | ||
| 95 | - assertThat(result.itemCode).isEqualTo("SKU-1") | ||
| 96 | - assertThat(result.locationId).isEqualTo(locId) | ||
| 97 | - assertThat(result.quantity).isEqualTo(BigDecimal("42.5")) | ||
| 98 | - verify(exactly = 1) { balances.save(any()) } | 118 | + assertThat(result).isEqualTo(existing) |
| 119 | + // Crucially: NO movement is recorded for a no-op adjustment. | ||
| 120 | + // The audit log shouldn't fill up with "adjusted to current | ||
| 121 | + // value" entries when an operator clicks Save without changing | ||
| 122 | + // anything. | ||
| 123 | + verify(exactly = 0) { stockMovementService.record(any()) } | ||
| 99 | } | 124 | } |
| 100 | 125 | ||
| 101 | @Test | 126 | @Test |
| 102 | - fun `adjust mutates the existing balance row when one already exists`() { | ||
| 103 | - stubItem("SKU-1") | 127 | + fun `no-op adjust on a missing balance row creates an empty row at zero`() { |
| 128 | + // Edge case: setting a never-stocked cell to its current value | ||
| 129 | + // (zero) shouldn't create a ledger row but does create the | ||
| 130 | + // (item, location) row at quantity zero so subsequent reads | ||
| 131 | + // return zero rather than null. | ||
| 104 | val locId = UUID.randomUUID() | 132 | val locId = UUID.randomUUID() |
| 105 | - val existing = StockBalance("SKU-1", locId, BigDecimal("5")).also { it.id = UUID.randomUUID() } | 133 | + stubItem("SKU-1") |
| 134 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns null | ||
| 106 | every { locations.existsById(locId) } returns true | 135 | every { locations.existsById(locId) } returns true |
| 107 | - every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns existing | 136 | + val saved = slot<StockBalance>() |
| 137 | + every { balances.save(capture(saved)) } answers { saved.captured } | ||
| 108 | 138 | ||
| 109 | - val result = service.adjust("SKU-1", locId, BigDecimal("99")) | 139 | + val result = service.adjust("SKU-1", locId, BigDecimal("0")) |
| 110 | 140 | ||
| 111 | - assertThat(result).isEqualTo(existing) | ||
| 112 | - assertThat(existing.quantity).isEqualTo(BigDecimal("99")) | ||
| 113 | - // Crucially, save() is NOT called — the @Transactional method | ||
| 114 | - // commit will flush the JPA-managed entity automatically. | ||
| 115 | - verify(exactly = 0) { balances.save(any()) } | 141 | + assertThat(result.quantity).isEqualTo(BigDecimal("0")) |
| 142 | + verify(exactly = 0) { stockMovementService.record(any()) } | ||
| 116 | } | 143 | } |
| 117 | } | 144 | } |
pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/StockMovementServiceTest.kt
0 → 100644
| 1 | +package org.vibeerp.pbc.inventory.application | ||
| 2 | + | ||
| 3 | +import assertk.assertFailure | ||
| 4 | +import assertk.assertThat | ||
| 5 | +import assertk.assertions.contains | ||
| 6 | +import assertk.assertions.hasMessage | ||
| 7 | +import assertk.assertions.isEqualTo | ||
| 8 | +import assertk.assertions.isInstanceOf | ||
| 9 | +import assertk.assertions.messageContains | ||
| 10 | +import io.mockk.every | ||
| 11 | +import io.mockk.mockk | ||
| 12 | +import io.mockk.slot | ||
| 13 | +import io.mockk.verify | ||
| 14 | +import org.junit.jupiter.api.BeforeEach | ||
| 15 | +import org.junit.jupiter.api.Test | ||
| 16 | +import org.vibeerp.api.v1.core.Id | ||
| 17 | +import org.vibeerp.api.v1.ext.catalog.CatalogApi | ||
| 18 | +import org.vibeerp.api.v1.ext.catalog.ItemRef | ||
| 19 | +import org.vibeerp.pbc.inventory.domain.MovementReason | ||
| 20 | +import org.vibeerp.pbc.inventory.domain.StockBalance | ||
| 21 | +import org.vibeerp.pbc.inventory.domain.StockMovement | ||
| 22 | +import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository | ||
| 23 | +import org.vibeerp.pbc.inventory.infrastructure.StockBalanceJpaRepository | ||
| 24 | +import org.vibeerp.pbc.inventory.infrastructure.StockMovementJpaRepository | ||
| 25 | +import java.math.BigDecimal | ||
| 26 | +import java.util.UUID | ||
| 27 | + | ||
| 28 | +/** | ||
| 29 | + * Tests for [StockMovementService]: cross-PBC item validation, | ||
| 30 | + * location validation, sign-vs-reason enforcement, negative-balance | ||
| 31 | + * rejection, and the ledger-row + balance-row pair. | ||
| 32 | + */ | ||
| 33 | +class StockMovementServiceTest { | ||
| 34 | + | ||
| 35 | + private lateinit var movements: StockMovementJpaRepository | ||
| 36 | + private lateinit var balances: StockBalanceJpaRepository | ||
| 37 | + private lateinit var locations: LocationJpaRepository | ||
| 38 | + private lateinit var catalogApi: CatalogApi | ||
| 39 | + private lateinit var service: StockMovementService | ||
| 40 | + | ||
| 41 | + @BeforeEach | ||
| 42 | + fun setUp() { | ||
| 43 | + movements = mockk() | ||
| 44 | + balances = mockk() | ||
| 45 | + locations = mockk() | ||
| 46 | + catalogApi = mockk() | ||
| 47 | + every { movements.save(any<StockMovement>()) } answers { firstArg() } | ||
| 48 | + every { balances.save(any<StockBalance>()) } answers { firstArg() } | ||
| 49 | + service = StockMovementService(movements, balances, locations, catalogApi) | ||
| 50 | + } | ||
| 51 | + | ||
| 52 | + private fun stubItem(code: String) { | ||
| 53 | + every { catalogApi.findItemByCode(code) } returns ItemRef( | ||
| 54 | + id = Id(UUID.randomUUID()), | ||
| 55 | + code = code, | ||
| 56 | + name = "stub", | ||
| 57 | + itemType = "GOOD", | ||
| 58 | + baseUomCode = "ea", | ||
| 59 | + active = true, | ||
| 60 | + ) | ||
| 61 | + } | ||
| 62 | + | ||
| 63 | + private fun cmd( | ||
| 64 | + delta: String, | ||
| 65 | + reason: MovementReason, | ||
| 66 | + item: String = "SKU-1", | ||
| 67 | + loc: UUID = UUID.randomUUID(), | ||
| 68 | + ref: String? = null, | ||
| 69 | + ) = RecordMovementCommand( | ||
| 70 | + itemCode = item, | ||
| 71 | + locationId = loc, | ||
| 72 | + delta = BigDecimal(delta), | ||
| 73 | + reason = reason, | ||
| 74 | + reference = ref, | ||
| 75 | + ) | ||
| 76 | + | ||
| 77 | + @Test | ||
| 78 | + fun `record rejects zero delta up front`() { | ||
| 79 | + assertFailure { service.record(cmd("0", MovementReason.ADJUSTMENT)) } | ||
| 80 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 81 | + .hasMessage("stock movement delta must be non-zero") | ||
| 82 | + } | ||
| 83 | + | ||
| 84 | + @Test | ||
| 85 | + fun `record rejects positive delta on SALES_SHIPMENT`() { | ||
| 86 | + assertFailure { service.record(cmd("5", MovementReason.SALES_SHIPMENT)) } | ||
| 87 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 88 | + .messageContains("non-positive") | ||
| 89 | + } | ||
| 90 | + | ||
| 91 | + @Test | ||
| 92 | + fun `record rejects negative delta on RECEIPT`() { | ||
| 93 | + assertFailure { service.record(cmd("-5", MovementReason.RECEIPT)) } | ||
| 94 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 95 | + .messageContains("non-negative") | ||
| 96 | + } | ||
| 97 | + | ||
| 98 | + @Test | ||
| 99 | + fun `record allows either sign on ADJUSTMENT`() { | ||
| 100 | + val locId = UUID.randomUUID() | ||
| 101 | + stubItem("SKU-1") | ||
| 102 | + every { locations.existsById(locId) } returns true | ||
| 103 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns | ||
| 104 | + StockBalance("SKU-1", locId, BigDecimal("10")) | ||
| 105 | + | ||
| 106 | + // Negative ADJUSTMENT (operator correcting a count down) | ||
| 107 | + service.record( | ||
| 108 | + RecordMovementCommand( | ||
| 109 | + itemCode = "SKU-1", | ||
| 110 | + locationId = locId, | ||
| 111 | + delta = BigDecimal("-3"), | ||
| 112 | + reason = MovementReason.ADJUSTMENT, | ||
| 113 | + ), | ||
| 114 | + ) | ||
| 115 | + | ||
| 116 | + // Positive ADJUSTMENT (operator correcting a count up) | ||
| 117 | + service.record( | ||
| 118 | + RecordMovementCommand( | ||
| 119 | + itemCode = "SKU-1", | ||
| 120 | + locationId = locId, | ||
| 121 | + delta = BigDecimal("8"), | ||
| 122 | + reason = MovementReason.ADJUSTMENT, | ||
| 123 | + ), | ||
| 124 | + ) | ||
| 125 | + | ||
| 126 | + // Both succeed. | ||
| 127 | + } | ||
| 128 | + | ||
| 129 | + @Test | ||
| 130 | + fun `record rejects unknown item via CatalogApi seam`() { | ||
| 131 | + every { catalogApi.findItemByCode("FAKE") } returns null | ||
| 132 | + | ||
| 133 | + assertFailure { | ||
| 134 | + service.record( | ||
| 135 | + RecordMovementCommand( | ||
| 136 | + itemCode = "FAKE", | ||
| 137 | + locationId = UUID.randomUUID(), | ||
| 138 | + delta = BigDecimal("5"), | ||
| 139 | + reason = MovementReason.RECEIPT, | ||
| 140 | + ), | ||
| 141 | + ) | ||
| 142 | + } | ||
| 143 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 144 | + .messageContains("not in the catalog") | ||
| 145 | + } | ||
| 146 | + | ||
| 147 | + @Test | ||
| 148 | + fun `record rejects unknown location`() { | ||
| 149 | + stubItem("SKU-1") | ||
| 150 | + val locId = UUID.randomUUID() | ||
| 151 | + every { locations.existsById(locId) } returns false | ||
| 152 | + | ||
| 153 | + assertFailure { | ||
| 154 | + service.record( | ||
| 155 | + RecordMovementCommand( | ||
| 156 | + itemCode = "SKU-1", | ||
| 157 | + locationId = locId, | ||
| 158 | + delta = BigDecimal("5"), | ||
| 159 | + reason = MovementReason.RECEIPT, | ||
| 160 | + ), | ||
| 161 | + ) | ||
| 162 | + } | ||
| 163 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 164 | + .messageContains("location not found") | ||
| 165 | + } | ||
| 166 | + | ||
| 167 | + @Test | ||
| 168 | + fun `record rejects movement that would push balance negative`() { | ||
| 169 | + stubItem("SKU-1") | ||
| 170 | + val locId = UUID.randomUUID() | ||
| 171 | + every { locations.existsById(locId) } returns true | ||
| 172 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns | ||
| 173 | + StockBalance("SKU-1", locId, BigDecimal("3")) | ||
| 174 | + | ||
| 175 | + assertFailure { | ||
| 176 | + service.record( | ||
| 177 | + RecordMovementCommand( | ||
| 178 | + itemCode = "SKU-1", | ||
| 179 | + locationId = locId, | ||
| 180 | + delta = BigDecimal("-5"), | ||
| 181 | + reason = MovementReason.SALES_SHIPMENT, | ||
| 182 | + ), | ||
| 183 | + ) | ||
| 184 | + } | ||
| 185 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 186 | + .messageContains("below zero") | ||
| 187 | + } | ||
| 188 | + | ||
| 189 | + @Test | ||
| 190 | + fun `record creates a new balance row when none exists`() { | ||
| 191 | + stubItem("SKU-1") | ||
| 192 | + val locId = UUID.randomUUID() | ||
| 193 | + every { locations.existsById(locId) } returns true | ||
| 194 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns null | ||
| 195 | + val savedBalance = slot<StockBalance>() | ||
| 196 | + val savedMovement = slot<StockMovement>() | ||
| 197 | + every { balances.save(capture(savedBalance)) } answers { savedBalance.captured } | ||
| 198 | + every { movements.save(capture(savedMovement)) } answers { savedMovement.captured } | ||
| 199 | + | ||
| 200 | + val result = service.record( | ||
| 201 | + RecordMovementCommand( | ||
| 202 | + itemCode = "SKU-1", | ||
| 203 | + locationId = locId, | ||
| 204 | + delta = BigDecimal("100"), | ||
| 205 | + reason = MovementReason.RECEIPT, | ||
| 206 | + reference = "PR:PR-2026-0001", | ||
| 207 | + ), | ||
| 208 | + ) | ||
| 209 | + | ||
| 210 | + assertThat(result.quantity).isEqualTo(BigDecimal("100")) | ||
| 211 | + assertThat(savedMovement.captured.delta).isEqualTo(BigDecimal("100")) | ||
| 212 | + assertThat(savedMovement.captured.reason).isEqualTo(MovementReason.RECEIPT) | ||
| 213 | + assertThat(savedMovement.captured.reference).isEqualTo("PR:PR-2026-0001") | ||
| 214 | + } | ||
| 215 | + | ||
| 216 | + @Test | ||
| 217 | + fun `record updates existing balance and inserts movement on the happy path`() { | ||
| 218 | + stubItem("SKU-1") | ||
| 219 | + val locId = UUID.randomUUID() | ||
| 220 | + val existing = StockBalance("SKU-1", locId, BigDecimal("50")).also { it.id = UUID.randomUUID() } | ||
| 221 | + every { locations.existsById(locId) } returns true | ||
| 222 | + every { balances.findByItemCodeAndLocationId("SKU-1", locId) } returns existing | ||
| 223 | + val savedMovement = slot<StockMovement>() | ||
| 224 | + every { movements.save(capture(savedMovement)) } answers { savedMovement.captured } | ||
| 225 | + | ||
| 226 | + val result = service.record( | ||
| 227 | + RecordMovementCommand( | ||
| 228 | + itemCode = "SKU-1", | ||
| 229 | + locationId = locId, | ||
| 230 | + delta = BigDecimal("-15"), | ||
| 231 | + reason = MovementReason.SALES_SHIPMENT, | ||
| 232 | + reference = "SO:SO-2026-0001", | ||
| 233 | + ), | ||
| 234 | + ) | ||
| 235 | + | ||
| 236 | + assertThat(result).isEqualTo(existing) | ||
| 237 | + // 50 - 15 = 35 | ||
| 238 | + assertThat(existing.quantity).isEqualTo(BigDecimal("35")) | ||
| 239 | + assertThat(savedMovement.captured.delta).isEqualTo(BigDecimal("-15")) | ||
| 240 | + assertThat(savedMovement.captured.reference).isEqualTo("SO:SO-2026-0001") | ||
| 241 | + verify(exactly = 0) { balances.save(any()) } // existing row mutated, not re-saved | ||
| 242 | + } | ||
| 243 | +} |
pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt
| @@ -5,6 +5,7 @@ import com.fasterxml.jackson.module.kotlin.registerKotlinModule | @@ -5,6 +5,7 @@ import com.fasterxml.jackson.module.kotlin.registerKotlinModule | ||
| 5 | import org.springframework.stereotype.Service | 5 | import org.springframework.stereotype.Service |
| 6 | import org.springframework.transaction.annotation.Transactional | 6 | import org.springframework.transaction.annotation.Transactional |
| 7 | import org.vibeerp.api.v1.ext.catalog.CatalogApi | 7 | import org.vibeerp.api.v1.ext.catalog.CatalogApi |
| 8 | +import org.vibeerp.api.v1.ext.inventory.InventoryApi | ||
| 8 | import org.vibeerp.api.v1.ext.partners.PartnersApi | 9 | import org.vibeerp.api.v1.ext.partners.PartnersApi |
| 9 | import org.vibeerp.pbc.orders.sales.domain.SalesOrder | 10 | import org.vibeerp.pbc.orders.sales.domain.SalesOrder |
| 10 | import org.vibeerp.pbc.orders.sales.domain.SalesOrderLine | 11 | import org.vibeerp.pbc.orders.sales.domain.SalesOrderLine |
| @@ -68,6 +69,7 @@ class SalesOrderService( | @@ -68,6 +69,7 @@ class SalesOrderService( | ||
| 68 | private val orders: SalesOrderJpaRepository, | 69 | private val orders: SalesOrderJpaRepository, |
| 69 | private val partnersApi: PartnersApi, | 70 | private val partnersApi: PartnersApi, |
| 70 | private val catalogApi: CatalogApi, | 71 | private val catalogApi: CatalogApi, |
| 72 | + private val inventoryApi: InventoryApi, | ||
| 71 | private val extValidator: ExtJsonValidator, | 73 | private val extValidator: ExtJsonValidator, |
| 72 | ) { | 74 | ) { |
| 73 | 75 | ||
| @@ -240,10 +242,73 @@ class SalesOrderService( | @@ -240,10 +242,73 @@ class SalesOrderService( | ||
| 240 | require(order.status != SalesOrderStatus.CANCELLED) { | 242 | require(order.status != SalesOrderStatus.CANCELLED) { |
| 241 | "sales order ${order.code} is already cancelled" | 243 | "sales order ${order.code} is already cancelled" |
| 242 | } | 244 | } |
| 245 | + // Shipping is terminal — once stock has moved out the door | ||
| 246 | + // the cancellation flow is "issue a refund / receive a return", | ||
| 247 | + // not a status flip. The framework refuses the shortcut. | ||
| 248 | + require(order.status != SalesOrderStatus.SHIPPED) { | ||
| 249 | + "cannot cancel sales order ${order.code} in status SHIPPED; " + | ||
| 250 | + "issue a return / refund flow instead" | ||
| 251 | + } | ||
| 243 | order.status = SalesOrderStatus.CANCELLED | 252 | order.status = SalesOrderStatus.CANCELLED |
| 244 | return order | 253 | return order |
| 245 | } | 254 | } |
| 246 | 255 | ||
| 256 | + /** | ||
| 257 | + * Mark a CONFIRMED sales order as SHIPPED, debiting stock from | ||
| 258 | + * [shippingLocationCode] for every line in the same transaction. | ||
| 259 | + * | ||
| 260 | + * **The first cross-PBC WRITE the framework performs.** All | ||
| 261 | + * earlier cross-PBC calls were reads (`CatalogApi.findItemByCode`, | ||
| 262 | + * `PartnersApi.findPartnerByCode`). Shipping inverts that: this | ||
| 263 | + * service synchronously writes to inventory's tables (via the | ||
| 264 | + * api.v1 facade) as a side effect of changing its own state. | ||
| 265 | + * The whole operation runs in ONE transaction so a failure on any | ||
| 266 | + * line — bad item, bad location, would push balance negative — | ||
| 267 | + * rolls back the order status change AND every other line's | ||
| 268 | + * movement that may have already been written. The customer | ||
| 269 | + * cannot end up with "5 of 7 lines shipped, status still | ||
| 270 | + * CONFIRMED, ledger half-written". | ||
| 271 | + * | ||
| 272 | + * **Stock checks happen at the inventory layer.** This service | ||
| 273 | + * doesn't pre-check "do we have enough stock for every line"; | ||
| 274 | + * it just calls `inventoryApi.recordMovement(... -line.quantity)` | ||
| 275 | + * and lets that fail with "would push balance below zero" on the | ||
| 276 | + * first line that doesn't fit. The pre-check would be a nice | ||
| 277 | + * UX improvement (show all problems at once instead of the | ||
| 278 | + * first one) but is functionally equivalent — the transaction | ||
| 279 | + * either commits in full or rolls back in full. | ||
| 280 | + * | ||
| 281 | + * @throws IllegalArgumentException if the order is not CONFIRMED, | ||
| 282 | + * if the location code is unknown, or if any line lacks stock. | ||
| 283 | + */ | ||
| 284 | + fun ship(id: UUID, shippingLocationCode: String): SalesOrder { | ||
| 285 | + val order = orders.findById(id).orElseThrow { | ||
| 286 | + NoSuchElementException("sales order not found: $id") | ||
| 287 | + } | ||
| 288 | + require(order.status == SalesOrderStatus.CONFIRMED) { | ||
| 289 | + "cannot ship sales order ${order.code} in status ${order.status}; " + | ||
| 290 | + "only CONFIRMED orders can be shipped" | ||
| 291 | + } | ||
| 292 | + | ||
| 293 | + // Walk every line and debit stock. The reference convention | ||
| 294 | + // `SO:<order_code>` is documented on InventoryApi.recordMovement; | ||
| 295 | + // future PBCs (production, finance) will parse it via | ||
| 296 | + // `split(':')` to find every movement caused by an order. | ||
| 297 | + val reference = "SO:${order.code}" | ||
| 298 | + for (line in order.lines) { | ||
| 299 | + inventoryApi.recordMovement( | ||
| 300 | + itemCode = line.itemCode, | ||
| 301 | + locationCode = shippingLocationCode, | ||
| 302 | + delta = line.quantity.negate(), | ||
| 303 | + reason = "SALES_SHIPMENT", | ||
| 304 | + reference = reference, | ||
| 305 | + ) | ||
| 306 | + } | ||
| 307 | + | ||
| 308 | + order.status = SalesOrderStatus.SHIPPED | ||
| 309 | + return order | ||
| 310 | + } | ||
| 311 | + | ||
| 247 | @Suppress("UNCHECKED_CAST") | 312 | @Suppress("UNCHECKED_CAST") |
| 248 | fun parseExt(order: SalesOrder): Map<String, Any?> = try { | 313 | fun parseExt(order: SalesOrder): Map<String, Any?> = try { |
| 249 | if (order.ext.isBlank()) emptyMap() | 314 | if (order.ext.isBlank()) emptyMap() |
pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt
| @@ -125,16 +125,23 @@ class SalesOrder( | @@ -125,16 +125,23 @@ class SalesOrder( | ||
| 125 | * Possible states a [SalesOrder] can be in. | 125 | * Possible states a [SalesOrder] can be in. |
| 126 | * | 126 | * |
| 127 | * Stored as a string in the DB so adding values later is non-breaking | 127 | * Stored as a string in the DB so adding values later is non-breaking |
| 128 | - * for clients reading the column with raw SQL. Future expansion | ||
| 129 | - * (PARTIALLY_SHIPPED, SHIPPED, INVOICED, CLOSED) requires the | ||
| 130 | - * movement ledger and shipping flow which are deferred from v1. | 128 | + * for clients reading the column with raw SQL. |
| 131 | * | 129 | * |
| 132 | * - **DRAFT** — being prepared, lines may still be edited | 130 | * - **DRAFT** — being prepared, lines may still be edited |
| 133 | - * - **CONFIRMED** — committed; lines are now immutable | ||
| 134 | - * - **CANCELLED** — terminal; the order is dead | 131 | + * - **CONFIRMED** — committed; lines are now immutable. Can be cancelled |
| 132 | + * or shipped from here. | ||
| 133 | + * - **SHIPPED** — terminal. Stock has been debited via the | ||
| 134 | + * inventory movement ledger; the framework will | ||
| 135 | + * NOT let you cancel a shipped order (a return / | ||
| 136 | + * refund flow lands later as its own state). | ||
| 137 | + * - **CANCELLED** — terminal. Reachable from DRAFT or CONFIRMED. | ||
| 138 | + * | ||
| 139 | + * Future states (PARTIALLY_SHIPPED, INVOICED, CLOSED, RETURNED) | ||
| 140 | + * land in their own focused chunks. | ||
| 135 | */ | 141 | */ |
| 136 | enum class SalesOrderStatus { | 142 | enum class SalesOrderStatus { |
| 137 | DRAFT, | 143 | DRAFT, |
| 138 | CONFIRMED, | 144 | CONFIRMED, |
| 145 | + SHIPPED, | ||
| 139 | CANCELLED, | 146 | CANCELLED, |
| 140 | } | 147 | } |
pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/http/SalesOrderController.kt
| 1 | package org.vibeerp.pbc.orders.sales.http | 1 | package org.vibeerp.pbc.orders.sales.http |
| 2 | 2 | ||
| 3 | +import com.fasterxml.jackson.annotation.JsonCreator | ||
| 4 | +import com.fasterxml.jackson.annotation.JsonProperty | ||
| 3 | import jakarta.validation.Valid | 5 | import jakarta.validation.Valid |
| 4 | import jakarta.validation.constraints.NotBlank | 6 | import jakarta.validation.constraints.NotBlank |
| 5 | import jakarta.validation.constraints.NotEmpty | 7 | import jakarta.validation.constraints.NotEmpty |
| @@ -106,6 +108,20 @@ class SalesOrderController( | @@ -106,6 +108,20 @@ class SalesOrderController( | ||
| 106 | @RequirePermission("orders.sales.cancel") | 108 | @RequirePermission("orders.sales.cancel") |
| 107 | fun cancel(@PathVariable id: UUID): SalesOrderResponse = | 109 | fun cancel(@PathVariable id: UUID): SalesOrderResponse = |
| 108 | salesOrderService.cancel(id).toResponse(salesOrderService) | 110 | salesOrderService.cancel(id).toResponse(salesOrderService) |
| 111 | + | ||
| 112 | + /** | ||
| 113 | + * Ship a CONFIRMED sales order. Atomically debits stock for every | ||
| 114 | + * line from the location named in the request, then flips the | ||
| 115 | + * order to SHIPPED. The whole operation runs in one transaction | ||
| 116 | + * — see [SalesOrderService.ship] for the full rationale. | ||
| 117 | + */ | ||
| 118 | + @PostMapping("/{id}/ship") | ||
| 119 | + @RequirePermission("orders.sales.ship") | ||
| 120 | + fun ship( | ||
| 121 | + @PathVariable id: UUID, | ||
| 122 | + @RequestBody @Valid request: ShipSalesOrderRequest, | ||
| 123 | + ): SalesOrderResponse = | ||
| 124 | + salesOrderService.ship(id, request.shippingLocationCode).toResponse(salesOrderService) | ||
| 109 | } | 125 | } |
| 110 | 126 | ||
| 111 | // ─── DTOs ──────────────────────────────────────────────────────────── | 127 | // ─── DTOs ──────────────────────────────────────────────────────────── |
| @@ -119,6 +135,34 @@ data class CreateSalesOrderRequest( | @@ -119,6 +135,34 @@ data class CreateSalesOrderRequest( | ||
| 119 | val ext: Map<String, Any?>? = null, | 135 | val ext: Map<String, Any?>? = null, |
| 120 | ) | 136 | ) |
| 121 | 137 | ||
| 138 | +/** | ||
| 139 | + * Shipping request body. | ||
| 140 | + * | ||
| 141 | + * **Why the explicit `@JsonCreator(mode = PROPERTIES)` on a single- | ||
| 142 | + * arg Kotlin data class:** jackson-module-kotlin treats a data class | ||
| 143 | + * with one argument as a *delegate-based* creator by default | ||
| 144 | + * (`{"foo": "bar"}` would be unwrapped as the value of `foo` and | ||
| 145 | + * passed to the constructor positionally). That's wrong for HTTP | ||
| 146 | + * request bodies that always look like `{"shippingLocationCode": "..."}`. | ||
| 147 | + * The fix is to explicitly mark the constructor as a property-based | ||
| 148 | + * creator and to annotate the parameter with `@param:JsonProperty`. | ||
| 149 | + * This is the same trap that bit `RefreshRequest` in pbc-identity; | ||
| 150 | + * it's a Kotlin × Jackson interop wart, NOT something the framework | ||
| 151 | + * can hide. | ||
| 152 | + */ | ||
| 153 | +data class ShipSalesOrderRequest @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) constructor( | ||
| 154 | + /** | ||
| 155 | + * The inventory location to debit. Must exist in | ||
| 156 | + * `inventory__location` (looked up by code via the `InventoryApi` | ||
| 157 | + * facade inside the service). The framework does NOT default | ||
| 158 | + * this — every shipment is explicit about where the goods came | ||
| 159 | + * from, so the audit trail in the stock movement ledger always | ||
| 160 | + * tells you "which warehouse shipped this". | ||
| 161 | + */ | ||
| 162 | + @param:JsonProperty("shippingLocationCode") | ||
| 163 | + @field:NotBlank @field:Size(max = 64) val shippingLocationCode: String, | ||
| 164 | +) | ||
| 165 | + | ||
| 122 | data class UpdateSalesOrderRequest( | 166 | data class UpdateSalesOrderRequest( |
| 123 | @field:Size(max = 64) val partnerCode: String? = null, | 167 | @field:Size(max = 64) val partnerCode: String? = null, |
| 124 | val orderDate: LocalDate? = null, | 168 | val orderDate: LocalDate? = null, |
pbc/pbc-orders-sales/src/main/resources/META-INF/vibe-erp/metadata/orders-sales.yml
| @@ -23,7 +23,9 @@ permissions: | @@ -23,7 +23,9 @@ permissions: | ||
| 23 | - key: orders.sales.confirm | 23 | - key: orders.sales.confirm |
| 24 | description: Confirm a draft sales order (DRAFT → CONFIRMED) | 24 | description: Confirm a draft sales order (DRAFT → CONFIRMED) |
| 25 | - key: orders.sales.cancel | 25 | - key: orders.sales.cancel |
| 26 | - description: Cancel a sales order (any non-cancelled state → CANCELLED) | 26 | + description: Cancel a sales order (DRAFT or CONFIRMED → CANCELLED) |
| 27 | + - key: orders.sales.ship | ||
| 28 | + description: Ship a confirmed sales order (CONFIRMED → SHIPPED, debits inventory atomically) | ||
| 27 | 29 | ||
| 28 | menus: | 30 | menus: |
| 29 | - path: /orders/sales | 31 | - path: /orders/sales |
pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt
| @@ -11,11 +11,14 @@ import assertk.assertions.messageContains | @@ -11,11 +11,14 @@ import assertk.assertions.messageContains | ||
| 11 | import io.mockk.every | 11 | import io.mockk.every |
| 12 | import io.mockk.mockk | 12 | import io.mockk.mockk |
| 13 | import io.mockk.slot | 13 | import io.mockk.slot |
| 14 | +import io.mockk.verify | ||
| 14 | import org.junit.jupiter.api.BeforeEach | 15 | import org.junit.jupiter.api.BeforeEach |
| 15 | import org.junit.jupiter.api.Test | 16 | import org.junit.jupiter.api.Test |
| 16 | import org.vibeerp.api.v1.core.Id | 17 | import org.vibeerp.api.v1.core.Id |
| 17 | import org.vibeerp.api.v1.ext.catalog.CatalogApi | 18 | import org.vibeerp.api.v1.ext.catalog.CatalogApi |
| 18 | import org.vibeerp.api.v1.ext.catalog.ItemRef | 19 | import org.vibeerp.api.v1.ext.catalog.ItemRef |
| 20 | +import org.vibeerp.api.v1.ext.inventory.InventoryApi | ||
| 21 | +import org.vibeerp.api.v1.ext.inventory.StockBalanceRef | ||
| 19 | import org.vibeerp.api.v1.ext.partners.PartnerRef | 22 | import org.vibeerp.api.v1.ext.partners.PartnerRef |
| 20 | import org.vibeerp.api.v1.ext.partners.PartnersApi | 23 | import org.vibeerp.api.v1.ext.partners.PartnersApi |
| 21 | import org.vibeerp.pbc.orders.sales.domain.SalesOrder | 24 | import org.vibeerp.pbc.orders.sales.domain.SalesOrder |
| @@ -32,6 +35,7 @@ class SalesOrderServiceTest { | @@ -32,6 +35,7 @@ class SalesOrderServiceTest { | ||
| 32 | private lateinit var orders: SalesOrderJpaRepository | 35 | private lateinit var orders: SalesOrderJpaRepository |
| 33 | private lateinit var partnersApi: PartnersApi | 36 | private lateinit var partnersApi: PartnersApi |
| 34 | private lateinit var catalogApi: CatalogApi | 37 | private lateinit var catalogApi: CatalogApi |
| 38 | + private lateinit var inventoryApi: InventoryApi | ||
| 35 | private lateinit var extValidator: ExtJsonValidator | 39 | private lateinit var extValidator: ExtJsonValidator |
| 36 | private lateinit var service: SalesOrderService | 40 | private lateinit var service: SalesOrderService |
| 37 | 41 | ||
| @@ -40,11 +44,12 @@ class SalesOrderServiceTest { | @@ -40,11 +44,12 @@ class SalesOrderServiceTest { | ||
| 40 | orders = mockk() | 44 | orders = mockk() |
| 41 | partnersApi = mockk() | 45 | partnersApi = mockk() |
| 42 | catalogApi = mockk() | 46 | catalogApi = mockk() |
| 47 | + inventoryApi = mockk() | ||
| 43 | extValidator = mockk() | 48 | extValidator = mockk() |
| 44 | every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } | 49 | every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } |
| 45 | every { orders.existsByCode(any()) } returns false | 50 | every { orders.existsByCode(any()) } returns false |
| 46 | every { orders.save(any<SalesOrder>()) } answers { firstArg() } | 51 | every { orders.save(any<SalesOrder>()) } answers { firstArg() } |
| 47 | - service = SalesOrderService(orders, partnersApi, catalogApi, extValidator) | 52 | + service = SalesOrderService(orders, partnersApi, catalogApi, inventoryApi, extValidator) |
| 48 | } | 53 | } |
| 49 | 54 | ||
| 50 | private fun stubCustomer(code: String, type: String = "CUSTOMER") { | 55 | private fun stubCustomer(code: String, type: String = "CUSTOMER") { |
| @@ -298,4 +303,114 @@ class SalesOrderServiceTest { | @@ -298,4 +303,114 @@ class SalesOrderServiceTest { | ||
| 298 | .isInstanceOf(IllegalArgumentException::class) | 303 | .isInstanceOf(IllegalArgumentException::class) |
| 299 | .messageContains("already cancelled") | 304 | .messageContains("already cancelled") |
| 300 | } | 305 | } |
| 306 | + | ||
| 307 | + // ─── ship() ────────────────────────────────────────────────────── | ||
| 308 | + | ||
| 309 | + private fun confirmedOrder( | ||
| 310 | + id: UUID = UUID.randomUUID(), | ||
| 311 | + lines: List<Pair<String, String>> = listOf("PAPER-A4" to "10"), | ||
| 312 | + ): SalesOrder { | ||
| 313 | + val order = SalesOrder( | ||
| 314 | + code = "SO-1", | ||
| 315 | + partnerCode = "CUST-1", | ||
| 316 | + status = SalesOrderStatus.CONFIRMED, | ||
| 317 | + orderDate = LocalDate.of(2026, 4, 8), | ||
| 318 | + currencyCode = "USD", | ||
| 319 | + ).also { it.id = id } | ||
| 320 | + var n = 1 | ||
| 321 | + for ((item, qty) in lines) { | ||
| 322 | + order.lines += org.vibeerp.pbc.orders.sales.domain.SalesOrderLine( | ||
| 323 | + salesOrder = order, | ||
| 324 | + lineNo = n++, | ||
| 325 | + itemCode = item, | ||
| 326 | + quantity = BigDecimal(qty), | ||
| 327 | + unitPrice = BigDecimal("1.00"), | ||
| 328 | + currencyCode = "USD", | ||
| 329 | + ) | ||
| 330 | + } | ||
| 331 | + return order | ||
| 332 | + } | ||
| 333 | + | ||
| 334 | + private fun stubInventoryDebit(itemCode: String, locationCode: String, expectedDelta: BigDecimal) { | ||
| 335 | + every { | ||
| 336 | + inventoryApi.recordMovement( | ||
| 337 | + itemCode = itemCode, | ||
| 338 | + locationCode = locationCode, | ||
| 339 | + delta = expectedDelta, | ||
| 340 | + reason = "SALES_SHIPMENT", | ||
| 341 | + reference = any(), | ||
| 342 | + ) | ||
| 343 | + } returns StockBalanceRef( | ||
| 344 | + id = Id(UUID.randomUUID()), | ||
| 345 | + itemCode = itemCode, | ||
| 346 | + locationCode = locationCode, | ||
| 347 | + quantity = BigDecimal("1000"), | ||
| 348 | + ) | ||
| 349 | + } | ||
| 350 | + | ||
| 351 | + @Test | ||
| 352 | + fun `ship rejects a non-CONFIRMED order`() { | ||
| 353 | + val id = UUID.randomUUID() | ||
| 354 | + val draft = SalesOrder( | ||
| 355 | + code = "SO-1", | ||
| 356 | + partnerCode = "CUST-1", | ||
| 357 | + status = SalesOrderStatus.DRAFT, | ||
| 358 | + orderDate = LocalDate.of(2026, 4, 8), | ||
| 359 | + currencyCode = "USD", | ||
| 360 | + ).also { it.id = id } | ||
| 361 | + every { orders.findById(id) } returns Optional.of(draft) | ||
| 362 | + | ||
| 363 | + assertFailure { service.ship(id, "WH-MAIN") } | ||
| 364 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 365 | + .messageContains("only CONFIRMED orders can be shipped") | ||
| 366 | + verify(exactly = 0) { inventoryApi.recordMovement(any(), any(), any(), any(), any()) } | ||
| 367 | + } | ||
| 368 | + | ||
| 369 | + @Test | ||
| 370 | + fun `ship walks every line and calls inventoryApi recordMovement with negated quantity`() { | ||
| 371 | + val id = UUID.randomUUID() | ||
| 372 | + val order = confirmedOrder(id, lines = listOf("PAPER-A4" to "10", "INK-CYAN" to "3")) | ||
| 373 | + every { orders.findById(id) } returns Optional.of(order) | ||
| 374 | + stubInventoryDebit("PAPER-A4", "WH-MAIN", BigDecimal("-10")) | ||
| 375 | + stubInventoryDebit("INK-CYAN", "WH-MAIN", BigDecimal("-3")) | ||
| 376 | + | ||
| 377 | + val shipped = service.ship(id, "WH-MAIN") | ||
| 378 | + | ||
| 379 | + assertThat(shipped.status).isEqualTo(SalesOrderStatus.SHIPPED) | ||
| 380 | + verify(exactly = 1) { | ||
| 381 | + inventoryApi.recordMovement( | ||
| 382 | + itemCode = "PAPER-A4", | ||
| 383 | + locationCode = "WH-MAIN", | ||
| 384 | + delta = BigDecimal("-10"), | ||
| 385 | + reason = "SALES_SHIPMENT", | ||
| 386 | + reference = "SO:SO-1", | ||
| 387 | + ) | ||
| 388 | + } | ||
| 389 | + verify(exactly = 1) { | ||
| 390 | + inventoryApi.recordMovement( | ||
| 391 | + itemCode = "INK-CYAN", | ||
| 392 | + locationCode = "WH-MAIN", | ||
| 393 | + delta = BigDecimal("-3"), | ||
| 394 | + reason = "SALES_SHIPMENT", | ||
| 395 | + reference = "SO:SO-1", | ||
| 396 | + ) | ||
| 397 | + } | ||
| 398 | + } | ||
| 399 | + | ||
| 400 | + @Test | ||
| 401 | + fun `cancel rejects a SHIPPED order`() { | ||
| 402 | + val id = UUID.randomUUID() | ||
| 403 | + val shipped = SalesOrder( | ||
| 404 | + code = "SO-1", | ||
| 405 | + partnerCode = "CUST-1", | ||
| 406 | + status = SalesOrderStatus.SHIPPED, | ||
| 407 | + orderDate = LocalDate.of(2026, 4, 8), | ||
| 408 | + currencyCode = "USD", | ||
| 409 | + ).also { it.id = id } | ||
| 410 | + every { orders.findById(id) } returns Optional.of(shipped) | ||
| 411 | + | ||
| 412 | + assertFailure { service.cancel(id) } | ||
| 413 | + .isInstanceOf(IllegalArgumentException::class) | ||
| 414 | + .messageContains("SHIPPED") | ||
| 415 | + } | ||
| 301 | } | 416 | } |