(SP: 3) [SHOP] add transactional customer communications for order creation and status updates#438
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds canonical event emission for order lifecycle events (created, canceled, shipped, returned), server-side env read for a Monobank Google Pay toggle, improved notification recipient resolution (guest vs signed-in), and extensive tests exercising projector/worker delivery and event backfill behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrderSvc as Order Service (checkout.ts)
participant PaymentWriter as Payment Event Writer
participant DB as Database
participant Projector as Outbox Projector
participant Worker as Notification Worker
participant Email as Email Transport
Client->>OrderSvc: createOrderWithItems(idempotencyKey)
OrderSvc->>DB: INSERT order, items (or SELECT existing)
OrderSvc->>PaymentWriter: ensureOrderCreatedCanonicalEvent(orderId, payload)
PaymentWriter->>DB: INSERT paymentEvents (deduped)
OrderSvc->>Client: { isNew: true/false }
Projector->>DB: SELECT new payment/shipping events
Projector->>DB: INSERT notificationOutbox rows
Worker->>DB: CLAIM notificationOutbox
Worker->>DB: SELECT recipient (includes order_user_id)
Worker->>Email: sendShopNotificationEmail(to, subject, body)
Email-->>Worker: delivery success
Worker->>DB: UPDATE notificationOutbox (sent)
sequenceDiagram
participant Admin
participant ShippingSvc as Shipping Service (admin-actions.ts)
participant ShippingWriter as Shipping Event Writer
participant DB as Database
participant RestockSvc as Restock Service (restock.ts)
participant PaymentWriter as Payment Event Writer
Admin->>ShippingSvc: applyShippingAdminAction(mark_shipped)
ShippingSvc->>DB: UPDATE shipment status
ShippingSvc->>ShippingWriter: ensureOrderShippedCanonicalEvent(shipmentState)
ShippingWriter->>DB: INSERT shippingEvents (order_shipped)
Admin->>RestockSvc: restockOrder(reason: canceled)
RestockSvc->>DB: UPDATE order.status -> CANCELED
RestockSvc->>PaymentWriter: ensureOrderCanceledCanonicalEvent(orderId)
PaymentWriter->>DB: INSERT paymentEvents (order_canceled)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/lib/tests/shop/status-notifications-phase5.test.ts (1)
586-627: Consider adding explicit timeout for consistency.The restock replay test doesn't specify a timeout unlike the other tests in this file (which use
30_000). While this test may be faster, adding an explicit timeout maintains consistency and prevents potential CI timeouts.🔧 Suggested change
- }); + }, 30_000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/tests/shop/status-notifications-phase5.test.ts` around lines 586 - 627, The test "restock replay backfills a missing order_canceled canonical event without creating duplicates" lacks an explicit timeout; update the it(...) invocation for that test to include a timeout (e.g., 30_000) to match other tests and avoid CI flakiness—modify the test declaration that wraps the async function which calls seedShippableOrder, restockOrder, and cleanupOrder to pass the timeout as the last argument.
🤖 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/checkout-order-created-notification-phase5.test.ts`:
- Around line 117-119: cleanupOrder currently only deletes from orders which can
leave related rows in orderShipping, paymentEvents, and notificationOutbox and
lead to FK violations or test pollution; update cleanupOrder to delete related
rows first (orderShipping, paymentEvents, notificationOutbox) filtered by the
same orderId, then delete from orders (use the same db.delete(...) pattern and
eq(orders.id, orderId) style predicates used elsewhere) so all dependent data is
removed before the orders row.
---
Nitpick comments:
In `@frontend/lib/tests/shop/status-notifications-phase5.test.ts`:
- Around line 586-627: The test "restock replay backfills a missing
order_canceled canonical event without creating duplicates" lacks an explicit
timeout; update the it(...) invocation for that test to include a timeout (e.g.,
30_000) to match other tests and avoid CI flakiness—modify the test declaration
that wraps the async function which calls seedShippableOrder, restockOrder, and
cleanupOrder to pass the timeout as the last argument.
🪄 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: 3f8c4b47-6ed5-4f63-80f5-2243ac450197
📒 Files selected for processing (10)
frontend/lib/env/server-env.tsfrontend/lib/services/orders/checkout.tsfrontend/lib/services/orders/restock.tsfrontend/lib/services/shop/notifications/outbox-worker.tsfrontend/lib/services/shop/notifications/templates.tsfrontend/lib/services/shop/shipping/admin-actions.tsfrontend/lib/tests/shop/checkout-order-created-notification-phase5.test.tsfrontend/lib/tests/shop/notifications-projector-phase3.test.tsfrontend/lib/tests/shop/notifications-worker-transport-phase3.test.tsfrontend/lib/tests/shop/status-notifications-phase5.test.ts
Description
This PR completes Phase 5 — customer communications for the Shop module.
It adds transactional customer notifications on top of the existing canonical event -> projector -> outbox -> worker -> transport architecture.
Phase 5 now covers:
The goal is to ensure that customer communication is not limited to on-site pages and that key order lifecycle changes generate real transactional notifications from authoritative persisted events.
Related Issue
Issue: #<issue_number>
Changes
order_createdtransactional confirmation through the existing notification pipelineorder_shipped,order_canceled, andorder_returnedtemplates and canonical-event mappingsusers.emailfallback only for signed-in orders, guest-without-persisted-email fails closed explicitlyDatabase Changes (if applicable)
How Has This Been Tested?
Additional local verification included:
Screenshots (if applicable)
N/A — backend / notification flow work.
Checklist
Before submitting
Reviewers
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests