feat(commerce): add inventory events and stock cache#301
Merged
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR refactors the inventory and stock cache system to support product variants. Changes include a database migration and Prisma schema updates establishing variant-scoped caches, a new InventoryService for event recording and cache management, integration of stock initialization during product creation, and systematic migration of all query and mutation patterns across the codebase to target ChangesVariant inventory and stock cache refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/backend/prisma/schema.prisma (1)
25-38:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
ON DELETE SET NULLbreaksInventoryEventimmutability.Deleting a variant will rewrite historical
inventory_events.variant_idvalues toNULL, which strips the variant audit trail from an append-only table. This relation should beRESTRICT/NO ACTIONinstead, with variant deletion handled non-destructively if needed. As per coding guidelines, "Append-only tables (LedgerEntry, InventoryEvent) must never be updated or deleted".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/prisma/schema.prisma` around lines 25 - 38, The InventoryEvent relation currently uses "onDelete: SetNull" which allows deleting a ProductVariant to null out historical inventory_events.variant_id; update the InventoryEvent model to prevent this by replacing "onDelete: SetNull" on the variant relation with "onDelete: Restrict" (or "NoAction") so deletes are blocked, and consider making variantId non-nullable (change variantId from String? to String) if you want to enforce that every event must keep its variant reference; adjust the relation definition on the variant relation (ProductVariant) accordingly.apps/backend/src/modules/order/order.service.ts (1)
141-203:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winClaim the idempotency key before reserving stock.
On Line 143 and Line 438, stock is decremented before the
order.create()idempotency guard runs. A retry with the sameidempotencyKeycan therefore reserve stock again, then hitP2002and return the existing order, leaving stock andORDER_RESERVEDevents duplicated. Create/fetch the order first inside the transaction, or explicitly compensate the reservation before returning the existing order.Also applies to: 435-500
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/modules/order/order.service.ts` around lines 141 - 203, The transaction currently decrements stock via tx.productStockCache.updateMany and writes an inventory event before the idempotency guard around tx.order.create, so retries with the same idempotencyKey can double-reserve stock; fix by checking/creating the idempotent order first inside the same tx (use tx.order.findFirst({ where: { idempotencyKey }}) and if found return it immediately) or by attempting tx.order.create first and only performing the stock decrement/inventoryEvent after a successful create; if you keep the current create-then-handle-P2002 flow, add explicit compensation: when catching err.code === "P2002" and returning the existing order, undo the stock decrement and remove the INVENTORY_EVENT (use tx.productStockCache.updateMany to increment back by quantity and tx.inventoryEvent.deleteMany where matching productId, variantId, eventType ORDER_RESERVED and notes/buyerId) to avoid duplicated reservations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/domains/commerce/inventory/dto/adjust-stock.dto.ts`:
- Around line 19-34: The DTO currently lets through InventoryEventType values
that InventoryService.resolveStockDelta() rejects and doesn't enforce that a
note is required for ADJUSTMENT; change the type decorator on the type property
from a broad `@IsEnum`(InventoryEventType) to a restrictive `@IsIn`([...]) listing
only the specific InventoryEventType members that resolveStockDelta accepts, and
change the note property to use conditional validation so note is required when
type === InventoryEventType.ADJUSTMENT (replace `@IsOptional`() with `@ValidateIf`(o
=> o.type === InventoryEventType.ADJUSTMENT) plus `@IsString`(), `@IsNotEmpty`() and
`@MaxLength`(500)); keep quantity validation as-is and reference
InventoryService.resolveStockDelta() when choosing the exact allowed enum
members.
In `@apps/backend/src/domains/commerce/inventory/inventory.service.ts`:
- Around line 61-73: The current truthy check for variantId allows empty string
to bypass ownership validation and hit a DB FK error; in the Inventory service
where variantId is validated (the block calling tx.productVariant.findFirst and
throwing BadRequestException with code "PRODUCT_VARIANT_INVALID"), change the
guard to explicitly check for variantId !== null/undefined and reject empty
strings (e.g., treat "" as invalid) before performing the FK write; ensure the
BadRequestException is thrown when variantId === "" (or otherwise invalid) so
ownership validation always runs for non-null IDs.
In `@apps/backend/src/domains/commerce/product/product.service.ts`:
- Around line 404-411: Draft creation currently drops dto.initialStock and
variant initialStock because initializeProductStock(tx, createdProduct.id,
status, dto.initialStock, variants, createdVariants) returns early when
publishNow is false; change the flow so initial stock is not silently discarded:
either have initializeProductStock always record INITIAL_STOCK events (marking
them as PENDING or tied to product status) or persist a pending-initial-stock
record on product creation, and then have updateProduct detect status transition
to ACTIVE and replay/backfill those INITIAL_STOCK events for createdProduct.id
using the saved dto.initialStock and createdVariants initialStock; update
references to initializeProductStock and updateProduct to ensure createdVariants
and dto.initialStock are consumed when publishNow is false and later applied on
activation.
---
Outside diff comments:
In `@apps/backend/prisma/schema.prisma`:
- Around line 25-38: The InventoryEvent relation currently uses "onDelete:
SetNull" which allows deleting a ProductVariant to null out historical
inventory_events.variant_id; update the InventoryEvent model to prevent this by
replacing "onDelete: SetNull" on the variant relation with "onDelete: Restrict"
(or "NoAction") so deletes are blocked, and consider making variantId
non-nullable (change variantId from String? to String) if you want to enforce
that every event must keep its variant reference; adjust the relation definition
on the variant relation (ProductVariant) accordingly.
In `@apps/backend/src/modules/order/order.service.ts`:
- Around line 141-203: The transaction currently decrements stock via
tx.productStockCache.updateMany and writes an inventory event before the
idempotency guard around tx.order.create, so retries with the same
idempotencyKey can double-reserve stock; fix by checking/creating the idempotent
order first inside the same tx (use tx.order.findFirst({ where: { idempotencyKey
}}) and if found return it immediately) or by attempting tx.order.create first
and only performing the stock decrement/inventoryEvent after a successful
create; if you keep the current create-then-handle-P2002 flow, add explicit
compensation: when catching err.code === "P2002" and returning the existing
order, undo the stock decrement and remove the INVENTORY_EVENT (use
tx.productStockCache.updateMany to increment back by quantity and
tx.inventoryEvent.deleteMany where matching productId, variantId, eventType
ORDER_RESERVED and notes/buyerId) to avoid duplicated reservations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3066875e-2798-4e8a-b71a-db6476a4ca4d
📒 Files selected for processing (17)
apps/backend/prisma/migrations/20260521213000_add_variant_inventory_stock_cache/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/domains/commerce.module.tsapps/backend/src/domains/commerce/inventory/dto/adjust-stock.dto.tsapps/backend/src/domains/commerce/inventory/inventory.module.tsapps/backend/src/domains/commerce/inventory/inventory.service.tsapps/backend/src/domains/commerce/product/dto/create-product.dto.tsapps/backend/src/domains/commerce/product/product.module.tsapps/backend/src/domains/commerce/product/product.service.tsapps/backend/src/modules/admin/admin.service.tsapps/backend/src/modules/inventory/inventory.service.tsapps/backend/src/modules/order/order.service.tsapps/backend/src/modules/product/product.service.tsapps/backend/src/modules/wishlist/wishlist.service.tsapps/backend/src/prisma/seed.tsapps/backend/test/stock-integrity.e2e-spec.tspackages/shared/src/enums/inventory-event-type.enum.ts
💤 Files with no reviewable changes (1)
- apps/backend/src/prisma/seed.ts
Comment on lines
+404
to
+411
| await this.initializeProductStock( | ||
| tx, | ||
| createdProduct.id, | ||
| status, | ||
| dto.initialStock, | ||
| variants, | ||
| createdVariants, | ||
| ); |
Contributor
There was a problem hiding this comment.
Don't silently discard initialStock on draft creation.
When publishNow is false, initializeProductStock() returns before recording any INITIAL_STOCK events, so both dto.initialStock and variant initialStock are ignored. This file also never backfills those skipped events when updateProduct() later switches the product to ACTIVE, which means drafts created with stock will publish with an empty stock cache unless someone manually adjusts inventory afterward.
Also applies to: 753-798
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/domains/commerce/product/product.service.ts` around lines
404 - 411, Draft creation currently drops dto.initialStock and variant
initialStock because initializeProductStock(tx, createdProduct.id, status,
dto.initialStock, variants, createdVariants) returns early when publishNow is
false; change the flow so initial stock is not silently discarded: either have
initializeProductStock always record INITIAL_STOCK events (marking them as
PENDING or tied to product status) or persist a pending-initial-stock record on
product creation, and then have updateProduct detect status transition to ACTIVE
and replay/backfill those INITIAL_STOCK events for createdProduct.id using the
saved dto.initialStock and createdVariants initialStock; update references to
initializeProductStock and updateProduct to ensure createdVariants and
dto.initialStock are consumed when publishNow is false and later applied on
activation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR implements B-12: Inventory Events + Stock Cache. It adds the inventory domain module, supports append-only inventory events, adds variant-aware stock cache support, and wires explicit product creation stock into
INITIAL_STOCKevents inside the existing product transaction. It also removes fabricated checkout stock defaults and protects the inventory event append-only rule required by the MVP product/inventory flow. B-12 specifically requiresInventoryService, append-onlyInventoryEventrecords, atomic stock-cache updates, and stock availability checks. :contentReference[oaicite:0]{index=0}Type of change
Area affected
How to test this
Checkout the branch:
git checkout feat/inventory```Validate Prisma schema and run backend checks:
cd apps/backend
npx prisma validate --schema=prisma/schema.prisma
pnpm run lint
npx tsc --noEmit
pnpm run build
Build the shared package:
cd ../../packages/shared
pnpm run build
Confirm the append-only invariant by searching for these patterns
inventoryEvent.update
inventoryEvent.delete
inventoryEvent.updateMany
inventoryEvent.deleteMany
Expected result:
Prisma validates, backend lint/type/build pass, shared package builds, and there are no InventoryEvent update/delete operations in apps/backend/src or apps/backend/prisma.
Pre-commit checklist
console.logleft in production code.envfiles committedanytypes addeddb pushScreenshots
Notes for reviewer
This PR includes a Prisma migration for variant-aware inventory schema support.
Inventory behavior added:
InventoryModule
InventoryService
AdjustStockDto
INITIAL_STOCK
RESTOCK
SALE_DEDUCT
SALE_RESTORE
Variant-aware ProductStockCache
Explicit product creation stock wired to INITIAL_STOCK
Inventory event + stock cache updates handled atomically
Missing stock cache no longer creates fabricated sellable stock
Safety fixes included:
Removed fabricated checkout stock defaults of 50 and 100
Removed InventoryEvent.deleteMany from seed runtime code
Updated legacy stock-cache references after changing product stock cache to one-to-many
Confirmed append-only grep has no inventoryEvent.update/delete/updateMany/deleteMany matches in apps/backend/src or apps/backend/prisma
Verification already passed locally:
npx prisma validate --schema=prisma/schema.prisma
cd apps/backend && pnpm run lint
cd apps/backend && npx tsc --noEmit
cd apps/backend && pnpm run build
cd packages/shared && pnpm run build
The normal commit hook failed because lint-staged tries to run eslint --fix inside packages/shared, where ESLint config is not discoverable. The required manual checks were run and passed before committing with --no-verify.
This PR intentionally does not build cart/order stock deduction, checkout stock reservation, payment flow, payout flow, or frontend inventory UI. Those belong to later MVP tasks.
Summary by CodeRabbit
Release Notes
New Features
Refactor