Fix discount code Stripe/DB drift on creation failure#3985
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds link pre-checks, converts duplicate/taken external provider errors to API errors, wraps DB create with rollback of the external code on failure, and corrects the API duplicate-check to use ChangesDiscount Code Validation and Error Handling
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIHandler
participant createDiscountCode
participant prisma_link as prisma.link
participant discountProvider
participant prisma_discountCode as prisma.discountCode
Client->>APIHandler: POST /api/discount-codes (payload)
APIHandler->>createDiscountCode: call createDiscountCode(...)
createDiscountCode->>prisma_link: findUnique(link by id)
alt link has discountCode
createDiscountCode-->>APIHandler: throw DubApiError(bad_request)
APIHandler-->>Client: 400 response
else proceed
createDiscountCode->>discountProvider: createDiscountCode(finalCode,...)
discountProvider-->>createDiscountCode: externalCode
createDiscountCode->>prisma_discountCode: create(mapping)
alt success
prisma_discountCode-->>createDiscountCode: saved record
createDiscountCode-->>APIHandler: return record
APIHandler-->>Client: 200 response
else Prisma P2002 uniqueness error
createDiscountCode->>discountProvider: disableDiscountCode(rollback)
createDiscountCode-->>APIHandler: throw DubApiError(conflict)
APIHandler-->>Client: 409 response
else other error
createDiscountCode->>discountProvider: disableDiscountCode(rollback)
createDiscountCode-->>APIHandler: rethrow error
APIHandler-->>Client: 500 response
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@coderabbitai full review, any breaking changes? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the changed files
cat apps/web/app/\(ee\)/api/discount-codes/route.ts
echo "=== SEPARATOR ==="
cat apps/web/lib/discounts/create-discount-code.ts
echo "=== SEPARATOR ==="
# Check the discount provider interface/types
fd -e ts "discount-provider" --exec cat {}
echo "=== SEPARATOR ==="
# Check disable-discount-code utility
fd -e ts "disable" --exec grep -l "disableDiscountCode" {} 2>/dev/null
rg -n "disableDiscountCode" --type ts -A 5
echo "=== SEPARATOR ==="
# Check how discountCode relation is modeled on Link
rg -n "discountCode" --type prisma -A 2 -B 2
fd -e prisma --exec grep -n "discountCode" {}
echo "=== SEPARATOR ==="
# Check DubApiError usage
rg -n "conflict\|bad_request" --type ts apps/web/lib/discounts/ -A 2Length of output: 28563
SummaryNo breaking changes. Both files only affect internal behavior and error handling. API response shapes and endpoint signatures are unchanged.
|
| Area | Status |
|---|---|
| API contract | ✅ No breaking changes |
| Core drift fix (rollback) | ✅ Correct |
programId fix in route |
✅ Correct |
| Pre-check defense-in-depth | ✅ Good |
| TOCTOU race condition | ✅ Mitigated by P2002 catch |
| Rollback failure handling | ✅ Best-effort + logging |
| P2002 error message clarity |
The fix is solid. The only actionable suggestion is refining the P2002 error message for better UX. Everything else looks good to ship.
🧠 Learnings used
Learnt from: devkiran
Repo: dubinc/dub PR: 3804
File: apps/web/lib/discounts/discount-provider-shopify.ts:144-152
Timestamp: 2026-04-27T09:44:44.629Z
Learning: In the dubinc/dub codebase, `discount.amount` is stored as an integer in the smallest currency unit (e.g., cents for flat discounts, whole percentage points for percentage discounts). When creating Shopify discount codes, dividing by 100 (e.g., `discount.amount / 100`) correctly converts to the decimal amount in the store's own currency — there is no USD-specific currency mismatch because Shopify applies the discount in the store's native currency. Also, Shopify's GraphQL Decimal scalar accepts string representations (e.g., from `toFixed(2)`), so this is valid. Do not flag these patterns as bugs in future reviews.
Learnt from: steven-tey
Repo: dubinc/dub PR: 3679
File: apps/web/lib/stripe/disable-stripe-discount-code.ts:2-2
Timestamp: 2026-04-01T17:57:43.749Z
Learning: In dubinc/dub, there is a known StripeMode type divergence between packages: `apps/web/lib/types.ts` defines `StripeMode = "test" | "sandbox" | "live"`, while `packages/stripe-app/src/utils/types.ts` defines `StripeMode = "test" | "live"` (missing "sandbox"). This is intentional for now; the Stripe app package needs to be updated separately to support "sandbox" mode. Do not flag this divergence as a blocking issue in future reviews.
Learnt from: steven-tey
Repo: dubinc/dub PR: 0
File: :0-0
Timestamp: 2026-06-01T20:11:39.391Z
Learning: In the Dub codebase (`apps/web/app/(ee)/api/stripe/integration/webhook/`), both `invoice.paid` and `checkout.session.completed` handlers deduplicate sale recording using a shared Redis key `trackSale:stripe:invoiceId:${invoiceId}` with `nx: true`. This means even if both handlers attempt to process the same invoice, only the first one to acquire the key will record the sale. `attributeViaPromoCode` also independently deduplicates customer creation via an `existingCustomer` re-check and a `P2002` (Prisma unique constraint) catch.
Learnt from: steven-tey
Repo: dubinc/dub PR: 0
File: :0-0
Timestamp: 2026-06-03T20:04:17.294Z
Learning: In dubinc/dub PR `#3824`, replacing `context.sale.productId` with `context.sale.products` (array) in `checkout-session-completed.ts` and `invoice-paid.ts` is NOT a breaking change for existing reward conditions. The updated `create-partner-commission/route.ts` workflow mediates this: when `context.sale.products` is present and a `sale.productId` modifier is detected, the workflow iterates through each product, reconstructs `productId` from each product entry, and calls `determinePartnerReward` with a per-product context. So `evaluate-reward-conditions.ts` still receives `productId` correctly — it just comes from the `products` array iteration rather than a top-level `context.sale.productId`. Do not flag this as a breaking change in future reviews.
Learnt from: devkiran
Repo: dubinc/dub PR: 2635
File: packages/prisma/schema/payout.prisma:24-25
Timestamp: 2025-07-11T16:28:55.693Z
Learning: In the Dub codebase, multiple payout records can now share the same stripeTransferId because payouts are grouped by partner and processed as single Stripe transfers. This is why the unique constraint was removed from the stripeTransferId field in the Payout model - a single transfer can include multiple payouts for the same partner.
Summary by CodeRabbit