(SP:1) [SHOP] add DB guardrails for shipping/payment invariants#400
Merged
ViktorSvertoka merged 4 commits intodevelopfrom Mar 13, 2026
Merged
(SP:1) [SHOP] add DB guardrails for shipping/payment invariants#400ViktorSvertoka merged 4 commits intodevelopfrom
ViktorSvertoka merged 4 commits intodevelopfrom
Conversation
liudmylasovetovs
requested review from
AM1007 and
ViktorSvertoka
as code owners
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
|
Warning Rate limit exceeded
⌛ 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 (1)
📝 WalkthroughWalkthroughAdds DB-level guardrails and a terminal shipping CHECK constraint coordinating orders and shipping_shipments. Introduces two trigger functions and triggers to auto-cancel/mark shipments when orders become non-shippable, plus multiple test updates to reflect the new behaviors. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Orders as Orders Table
participant OrdersTrig as Trigger:\nshop_orders_close_shipping_pipeline_guardrail
participant Shipments as Shipping_Shipments Table
participant ShipTrig as Trigger:\nshop_shipping_shipments_require_shippable_order_guardrail
App->>Orders: UPDATE payment_status/status/inventory_status/shipping_required
Orders->>OrdersTrig: BEFORE UPDATE fires
OrdersTrig->>OrdersTrig: evaluate is_shippable / is_terminal
alt transitioned to non-shippable or terminal
OrdersTrig->>Orders: SET shipping_status = 'cancelled' (when applicable)
OrdersTrig->>Shipments: UPDATE related shipments → status='needs_attention', clear leases, set error fields
end
App->>Shipments: INSERT/UPDATE shipment (status='queued'/'processing')
Shipments->>ShipTrig: BEFORE INSERT/UPDATE fires
ShipTrig->>Orders: SELECT ... FOR UPDATE to verify shippable (shipping_required, paid, PAID, inventory reserved)
alt order is shippable
ShipTrig-->>Shipments: allow operation
else
ShipTrig-->>Shipments: RAISE EXCEPTION (constraint violation)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d90c3fe88b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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: 2
🧹 Nitpick comments (1)
frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts (1)
70-83: Mirror the full terminal predicate in the seeding helper.
requiresPostInsertBlockedTransitiononly special-casesrefundedandCANCELED, butorders_terminal_shipping_status_chkalso treatsfailedandINVENTORY_FAILEDas terminal. The next test that seeds either path will fail during setup instead of at the assertion site.♻️ Minimal alignment
- const requiresPostInsertBlockedTransition = - args.paymentStatus === 'refunded' || args.orderStatus === 'CANCELED'; + const requiresPostInsertBlockedTransition = + args.paymentStatus === 'failed' || + args.paymentStatus === 'refunded' || + args.orderStatus === 'CANCELED' || + args.orderStatus === 'INVENTORY_FAILED';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts` around lines 70 - 83, The seeding helper's terminal predicate requiresPostInsertBlockedTransition only checks for 'refunded' and 'CANCELED' but must mirror the DB constraint (orders_terminal_shipping_status_chk) which also treats payment statuses 'failed' and order/inventory statuses 'INVENTORY_FAILED' as terminal; update the logic that computes requiresPostInsertBlockedTransition to include args.paymentStatus === 'failed' and args.orderStatus === 'INVENTORY_FAILED' (or the appropriate casing used elsewhere) so seedPaymentStatus, seedOrderStatus, and seedInventoryStatus are set to the non-terminal seeded values only when truly non-terminal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/drizzle/0031_lean_nebula.sql`:
- Around line 1-20: The migration backfill and the guardrail function
shop_orders_close_shipping_pipeline_guardrail() only touch statuses 'queued' and
'processing', but claimQueuedShipmentsForProcessing treats 'failed' as runnable
— update both the SQL backfill (update shipping_shipments ...) and the guardrail
to also include rows with status = 'failed' (or at minimum clear next_attempt_at
and lease fields for failed rows) so failed shipments are not re-claimed; locate
the SQL UPDATE statement and the shop_orders_close_shipping_pipeline_guardrail()
implementation and apply the same status changes (status -> 'needs_attention',
lease_owner/lease_expires_at/next_attempt_at -> null, last_error_code/message
and updated_at) or at least ensure next_attempt_at is null for status =
'failed'.
- Around line 125-139: The shippability check uses an unlocked EXISTS on orders
which can race with concurrent updates; change the validation to select and lock
the parent order row by querying orders with WHERE id = new.order_id AND ... FOR
UPDATE (or use SELECT 1 FOR UPDATE / PERFORM with the same predicate) so the row
is locked during the trigger's check; preserve the same error raise (constraint
shipping_shipments_shippable_order_chk and message) when the locked-select
returns no row, ensuring the check references new.order_id and the same columns
(shipping_required, payment_status, status, inventory_status).
---
Nitpick comments:
In `@frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts`:
- Around line 70-83: The seeding helper's terminal predicate
requiresPostInsertBlockedTransition only checks for 'refunded' and 'CANCELED'
but must mirror the DB constraint (orders_terminal_shipping_status_chk) which
also treats payment statuses 'failed' and order/inventory statuses
'INVENTORY_FAILED' as terminal; update the logic that computes
requiresPostInsertBlockedTransition to include args.paymentStatus === 'failed'
and args.orderStatus === 'INVENTORY_FAILED' (or the appropriate casing used
elsewhere) so seedPaymentStatus, seedOrderStatus, and seedInventoryStatus are
set to the non-terminal seeded values only when truly non-terminal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b67276c-91b7-4dea-8cfe-83a5b090a482
📒 Files selected for processing (10)
frontend/db/schema/shop.tsfrontend/drizzle/0031_lean_nebula.sqlfrontend/drizzle/meta/0031_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/lib/tests/shop/admin-shipping-payment-gate.test.tsfrontend/lib/tests/shop/monobank-google-pay-submit-route.test.tsfrontend/lib/tests/shop/monobank-webhook-apply.test.tsfrontend/lib/tests/shop/shipping-db-guardrails-batch6.test.tsfrontend/lib/tests/shop/shipping-shipments-worker-phase5.test.tsfrontend/lib/tests/shop/stripe-webhook-refund-full.test.ts
…nd locked shippability checks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts`:
- Around line 130-137: The returned seed currently doesn't reflect the DB
guardrail change: when requiresPostInsertBlockedTransition is true and
state.shipmentStatus exists you must return 'needs_attention' instead of the
original status; update the return block around shipmentId/shippingStatus to set
shipmentStatus to requiresPostInsertBlockedTransition && state.shipmentStatus ?
'needs_attention' : state.shipmentStatus. Also extend the Seeded type
(referenced near line 41) to include 'needs_attention' as an allowed
shipmentStatus value so the type system accepts this new status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fe205aa1-f8ea-4507-8195-6177abf3060d
📒 Files selected for processing (4)
frontend/drizzle/0032_shipping_guardrails_followup.sqlfrontend/drizzle/meta/0032_snapshot.jsonfrontend/drizzle/meta/_journal.jsonfrontend/lib/tests/shop/admin-shipping-payment-gate.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/drizzle/meta/_journal.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts`:
- Around line 70-86: The current requiresPostInsertBlockedTransition only marks
terminal targets (failed/refunded/CANCELED/INVENTORY_FAILED); update its logic
so it treats any non-shippable target as requiring the two-step (guardrail) path
— e.g. invert the shippable condition instead of listing only terminal states so
cases like pending are included; adjust the same predicate used to derive
seedPaymentStatus, seedOrderStatus and seedInventoryStatus (the variables
requiresPostInsertBlockedTransition, seedPaymentStatus, seedOrderStatus,
seedInventoryStatus) so non-shippable targets always route through the
post-insert blocked transition and mirror the persisted DB state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5789e8e3-6b30-4e36-bcc0-fd65521742cb
📒 Files selected for processing (1)
frontend/lib/tests/shop/admin-shipping-payment-gate.test.ts
ViktorSvertoka
approved these changes
Mar 13, 2026
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
2 participants
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.
Description
Implements Batch 6 shipping DB guardrails to enforce order/shipment invariants at the database level.
This PR adds narrow database protections for terminal and non-shippable orders so invalid shipping pipeline states cannot be persisted. It also updates the Shop test fixtures/specs to reflect the new DB-enforced behavior and verifies that checkout, webhook, admin-shipping, and worker flows still behave correctly after the migration.
Related Issue
Issue: #<issue_number>
Changes
ordersandshipping_shipmentsDatabase Changes (if applicable)
How Has This Been Tested?
Additional verification performed:
admin-shipping-payment-gate.test.tsshipping-shipments-worker-phase5.test.tsmonobank-webhook-apply.test.tsstripe-webhook-refund-full.test.ts126 passed / 461 passedorders_terminal_shipping_status_chktrg_orders_close_shipping_pipeline_guardrailtrg_shipping_shipments_require_shippable_order_guardrailScreenshots (if applicable)
N/A
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
Bug Fixes
Reliability Improvements
Tests