Conversation
WalkthroughAdds Stripe transactions and coupons: DB migration and Drizzle snapshot, new Drizzle model schemas and repositories, StripeWebhookService extended to record payment-intent transactions and customer discounts, comprehensive unit tests, and test seeders for Stripe webhook events. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Stripe as Stripe
participant Webhook as StripeWebhookService
participant Users as UserRepository
participant TxRepo as StripeTransactionRepository
participant CpRepo as StripeCouponRepository
participant PMRepo as PaymentMethodRepository
participant Checkout as CheckoutSessionRepository
participant Refill as RefillService
rect rgb(245,248,255)
note over Stripe,Webhook: Event routing
Stripe->>Webhook: webhook(event)
alt checkout.session.completed / async_payment_succeeded
Webhook->>Checkout: findBySessionId(...)
Webhook->>Refill: topUpWallet(...)
Webhook->>Checkout: deleteById(...)
else payment_intent.succeeded
Webhook->>Users: findByStripeCustomerId(...)
Webhook->>TxRepo: findByStripeTransactionId(...)
Webhook->>TxRepo: insert (if not exists)
Webhook->>Refill: topUpWallet(...)
else payment_method.attached/detached
Webhook->>Users: findByStripeCustomerId(...)
Webhook->>PMRepo: create/delete
else customer.discount.created
Webhook->>Users: findByStripeCustomerId(...)
Webhook->>CpRepo: findByStripeCouponId(...)
Webhook->>CpRepo: insert (if not exists)
else unknown
Webhook-->>Stripe: ignore
end
end
sequenceDiagram
autonumber
participant Webhook as StripeWebhookService
participant TxRepo as StripeTransactionRepository
rect rgb(245,255,245)
note over Webhook,TxRepo: Record PaymentIntent Transaction
Webhook->>TxRepo: findByStripeTransactionId(pi.id)
alt exists
Webhook-->>Webhook: log PAYMENT_INTENT_TRANSACTION_ALREADY_EXISTS
else not found
Webhook->>TxRepo: insert {stripeTransactionId, userId, amount, currency, status, description, metadata, stripeCreatedAt}
Webhook-->>Webhook: log PAYMENT_INTENT_TRANSACTION_RECORDED
end
end
sequenceDiagram
autonumber
participant Webhook as StripeWebhookService
participant Users as UserRepository
participant CpRepo as StripeCouponRepository
rect rgb(255,248,240)
note over Webhook,CpRepo: Handle customer.discount.created
Webhook->>Users: findByStripeCustomerId(discount.customer)
alt user found
Webhook->>CpRepo: findByStripeCouponId(discount.id)
alt not exists
Webhook->>CpRepo: insert {stripeCouponId, userId, couponCode, discountAmount, stripeCreatedAt}
Webhook-->>Webhook: log DISCOUNT_CLAIMED_RECORDED
else exists
Webhook-->>Webhook: log DISCOUNT_ALREADY_EXISTS
end
else user missing
Webhook-->>Webhook: log USER_NOT_FOUND_FOR_DISCOUNT
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
8cbff4d to
489f6ef
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (5)
550-578: Removeas anyand keep Stripe types in tests.These casts violate our TS guideline. Replace with typed objects and use the existing
stub()helper to satisfy return types.- amount_off: 1000, - percent_off: null as any, + amount_off: 1000, + percent_off: null, ... -jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as any); +jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(stub(mockPromotionCode));
580-599: Avoidas anyon promotion-code path (percent-based).Use
nullandstub()to keep strong types.- percent_off: 20, - amount_off: null as any, + percent_off: 20, + amount_off: null, ... -jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(mockPromotionCode as any); +jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(stub(mockPromotionCode));
636-650: Avoidas anyon coupon path (no amount_off).- percent_off: null as any, - amount_off: null as any, + percent_off: null, + amount_off: null, ... -jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null as any); +jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null); ... -jest.spyOn(service, "listCoupons").mockResolvedValue({ coupons: [mockCoupon] } as any); +jest.spyOn(service, "listCoupons").mockResolvedValue(stub({ coupons: [mockCoupon] }));
618-635: Avoidas anyon percent-based coupon path.- percent_off: 20, - amount_off: null as any, + percent_off: 20, + amount_off: null, ... -jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null as any); +jest.spyOn(service, "findPromotionCodeByCode").mockResolvedValue(null); -jest.spyOn(service, "listCoupons").mockResolvedValue({ coupons: [mockCoupon] } as any); +jest.spyOn(service, "listCoupons").mockResolvedValue(stub({ coupons: [mockCoupon] }));
977-985: Movesetup()inside the rootdescribe(StripeService.name, …)block
Place the function declaration at the bottom of that block instead of outside it.apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (1)
80-85: Guard against undefined amounts; prefer amount_total when available.amount_subtotal can be undefined; using a non-null assertion risks runtime errors.
- if (checkoutSession.payment_status !== "unpaid") { - await this.refillService.topUpWallet(checkoutSession.amount_subtotal!, checkoutSessionCache.userId); + if (checkoutSession.payment_status !== "unpaid") { + const amount = checkoutSession.amount_total ?? checkoutSession.amount_subtotal; + if (typeof amount !== "number") { + this.logger.error({ event: "CHECKOUT_AMOUNT_MISSING", sessionId }); + return; + } + await this.refillService.topUpWallet(amount, checkoutSessionCache.userId); await this.checkoutSessionRepository.deleteBy({ sessionId });
🧹 Nitpick comments (5)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (5)
36-54: Log unhandled events to aid observability.Add a default branch so new/unsupported event types are visible.
case "customer.discount.created": await this.handleCustomerDiscountCreated(event); break; + default: + this.logger.info({ + event: "STRIPE_EVENT_UNHANDLED", + type: event.type, + id: event.id + }); + break;
27-34: Correct “objectId” logging; it currently logs the object type.Log both objectType and actual object id (when present) to avoid confusion.
- this.logger.info({ - event: "STRIPE_EVENT_RECEIVED", - type: event.type, - id: event.id, - objectId: event.data.object.object - }); + const objectInfo = event.data.object as unknown as { id?: string; object: string }; + this.logger.info({ + event: "STRIPE_EVENT_RECEIVED", + type: event.type, + id: event.id, + objectType: objectInfo.object, + objectId: objectInfo.id + });
310-316: Use Stripe event timestamp for coupon record.new Date() stores “now”, not when Stripe created the event/object. Prefer event.created.
- stripeCreatedAt: new Date() + stripeCreatedAt: new Date(event.created * 1000)
203-209: Avoid duplicate “PAYMENT_METHOD_DETACHED” log events.Rename the initial info log to indicate receipt, keeping the later one for post-DB state.
- this.logger.info({ - event: "PAYMENT_METHOD_DETACHED", + this.logger.info({ + event: "PAYMENT_METHOD_DETACHED_RECEIVED", paymentMethodId: paymentMethod.id, customerId, fingerprint });
111-143: Minimize external calls within DB transaction.consumeActiveDiscount hits Stripe inside a @WithTransaction method, extending transaction time and risk of lock contention. Consider performing the Stripe call before starting the transaction or after persisting an idempotency gate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
apps/api/drizzle/0019_robust_lady_ursula.sql(1 hunks)apps/api/drizzle/meta/0019_snapshot.json(1 hunks)apps/api/drizzle/meta/_journal.json(1 hunks)apps/api/src/billing/model-schemas/index.ts(1 hunks)apps/api/src/billing/model-schemas/stripe-coupon/stripe-coupon.schema.ts(1 hunks)apps/api/src/billing/model-schemas/stripe-transaction/stripe-transaction.schema.ts(1 hunks)apps/api/src/billing/repositories/index.ts(1 hunks)apps/api/src/billing/repositories/stripe-coupon/stripe-coupon.repository.ts(1 hunks)apps/api/src/billing/repositories/stripe-transaction/stripe-transaction.repository.ts(1 hunks)apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts(1 hunks)apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts(5 hunks)apps/api/src/billing/services/stripe/stripe.service.spec.ts(15 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/seeders/stripe-webhook-events.seeder.ts(1 hunks)apps/api/test/seeders/stripe.seeder.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/api/drizzle/meta/0019_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/api/src/billing/repositories/index.ts
- apps/api/drizzle/0019_robust_lady_ursula.sql
- apps/api/drizzle/meta/_journal.json
- apps/api/src/billing/model-schemas/index.ts
- apps/api/src/billing/repositories/stripe-transaction/stripe-transaction.repository.ts
- apps/api/test/seeders/index.ts
- apps/api/src/billing/repositories/stripe-coupon/stripe-coupon.repository.ts
- apps/api/src/billing/model-schemas/stripe-coupon/stripe-coupon.schema.ts
- apps/api/src/billing/model-schemas/stripe-transaction/stripe-transaction.schema.ts
- apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/services/stripe-webhook/stripe-webhook.service.tsapps/api/test/seeders/stripe-webhook-events.seeder.tsapps/api/test/seeders/stripe.seeder.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/services/stripe/stripe.service.spec.tsapps/api/src/billing/services/stripe-webhook/stripe-webhook.service.tsapps/api/test/seeders/stripe-webhook-events.seeder.tsapps/api/test/seeders/stripe.seeder.ts
🧬 Code graph analysis (2)
apps/api/src/billing/services/stripe/stripe.service.spec.ts (1)
apps/api/test/seeders/stripe.seeder.ts (1)
StripeSeeder(11-212)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (1)
apps/api/src/core/services/tx/tx.service.ts (1)
WithTransaction(33-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (5)
apps/api/test/seeders/stripe.seeder.ts (1)
11-211: Seeder API looks good; types are precise and data covers common paths.Class-based API with typed return is clear. No blocking issues.
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (4)
312-315: Coupon fields likely misnamed; store identifiers correctly and include currency.couponCode is set to coupon.id (an internal id, not a human-facing code). If you want the public code, use promotion_code (and retrieve for .code), else rename to couponId/promotionCodeId. Also consider persisting currency for discountAmount.
Would you like me to adjust repository/schema and handler to:
- store couponId: discount.coupon.id
- store promotionCodeId: discount.promotion_code ?? null
- store discountCurrency: discount.coupon.currency (when amount_off is set)
and backfill tests accordingly?
51-53: Event routing addition looks good.customer.discount.created is correctly routed to the new handler.
5-5: Wiring of repositories is consistent.Constructor injection and imports for StripeTransactionRepository and StripeCouponRepository look correct.
Also applies to: 22-24
112-116: All amount units align. RefillService.topUpWallet expects cents (per its JSDoc), and paymentIntent.amount, metadata-derived originalAmount (fallback to amount), and checkoutSession.amount_subtotal are all provided in the smallest currency unit (cents) (docs.stripe.com)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (75.28%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1864 +/- ##
==========================================
+ Coverage 43.65% 43.96% +0.31%
==========================================
Files 962 966 +4
Lines 27104 27190 +86
Branches 7029 7038 +9
==========================================
+ Hits 11832 11954 +122
- Misses 14887 14936 +49
+ Partials 385 300 -85
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (2)
51-53: Add handlers for related discount lifecycle events.Consider also handling customer.discount.deleted and customer.discount.updated to keep stored discounts in sync.
219-263: Duplicate log event names for detach flow.You log PAYMENT_METHOD_DETACHED at Line 225 and again at Line 259 for different stages. Rename the first to PAYMENT_METHOD_DETACH_RECEIVED (or similar) for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts(1 hunks)apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts(5 hunks)apps/api/test/seeders/stripe-webhook-events.seeder.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.spec.ts
- apps/api/test/seeders/stripe-webhook-events.seeder.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/api/src/billing/services/stripe-webhook/stripe-webhook.service.ts (1)
5-5: Repository imports wiring — LGTM.Imports for the new repositories look correct and consistent with the folder structure.
| const existingTransaction = await this.stripeTransactionRepository.findByStripeTransactionId(paymentIntent.id); | ||
| if (existingTransaction) { | ||
| this.logger.info({ | ||
| event: "PAYMENT_INTENT_TRANSACTION_ALREADY_EXISTS", | ||
| paymentIntentId: paymentIntent.id, | ||
| userId | ||
| }); | ||
| return false; | ||
| } | ||
|
|
||
| await this.stripeTransactionRepository.create({ | ||
| stripeTransactionId: paymentIntent.id, |
There was a problem hiding this comment.
thought(non-blocking): this doesn't save from race condition, so better to rely on insert on conflict ignore or insert on conflict update
| const existingCoupon = await this.stripeCouponRepository.findByStripeCouponId(discount.id); | ||
| if (existingCoupon) { | ||
| this.logger.info({ | ||
| event: "DISCOUNT_ALREADY_EXISTS", | ||
| discountId: discount.id, | ||
| userId: user.id | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| await this.stripeCouponRepository.create({ | ||
| stripeCouponId: discount.id, | ||
| userId: user.id, | ||
| couponCode: discount.coupon?.id, | ||
| discountAmount: discount.coupon?.amount_off ? discount.coupon.amount_off.toString() : null, | ||
| stripeCreatedAt: new Date() | ||
| }); |
There was a problem hiding this comment.
suggestion(non-blocking): I can't predict consequences but this also can be affected by race condition. So, better to rely on unique key constraint in db
Summary by CodeRabbit
New Features
Chores
Tests