Add POST /api/commissions for manual commission creation - #3968
Conversation
…ommission after approval
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates manual commission creation from a next-safe-action server action to a type-safe REST API. It adds POST /api/commissions with discriminated Zod schemas, implements backend flows for custom/lead/sale (including Stripe invoice import and deduplication), updates the frontend form to call the API, adds OpenAPI wiring, tests, and small helpers. ChangesManual Commission Creation API Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 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, bugs? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== invoice-paid.ts resolvePromotionCodeError usage ==="
rg -n "resolvePromotionCodeError\|resolvePromotionCode" --type ts -A2 -B2
echo ""
echo "=== create-manual-commissions.ts full ==="
cat apps/web/lib/api/commissions/create-manual-commissions.ts 2>/dev/null
echo ""
echo "=== commissions zod schema full ==="
cat apps/web/lib/zod/schemas/commissions.ts 2>/dev/null
echo ""
echo "=== commissions route.ts ==="
cat apps/web/app/\(ee\)/api/commissions/route.ts 2>/dev/null
echo ""
echo "=== create-commission.test.ts ==="
cat apps/web/tests/commissions/create-commission.test.ts 2>/dev/null
echo ""
echo "=== resolveLinkAndCustomer implementation ==="
fd "resolve-link-and-customer" --type f
cat apps/web/lib/api/commissions/resolve-link-and-customer.ts 2>/dev/null
echo ""
echo "=== triggerAggregateDueCommissionsCronJob usage context ==="
rg -n "triggerAggregateDueCommissions" --type ts -A3 -B3
echo ""
echo "=== waitUntil usage in create-manual-commissions ==="
rg -n "waitUntil" apps/web/lib/api/commissions/create-manual-commissions.ts -A2 -B2
echo ""
echo "=== isFirstConversion import/usage ==="
rg -n "isFirstConversion" --type ts -A2 -B2
echo ""
echo "=== Check for any remaining old server action imports ==="
rg -rn "create-manual-commission" -g "*.ts" -g "*.tsx"
echo ""
echo "=== createCommissionBodySchema usage in UI ==="
rg -n "createCommissionBodySchema\|createManualCommissionBodySchema" --type ts -A2 -B2
echo ""
echo "=== Check programId derivation in POST route ==="
rg -n "programId" apps/web/app/\(ee\)/api/commissions/route.ts -A2 -B2
echo ""
echo "=== Check for null/undefined safety in resolveLinkAndCustomer ==="
cat apps/web/lib/api/commissions/resolve-link-and-customer.ts 2>/dev/null | head -150Length of output: 72939
Breaking Changes
No public breaking changes. Issues Found🔴 Debug timing calls left in production route
// Remove these before merging
console.time("createManualCommissions");
await createManualCommissions({ ... });
console.timeEnd("createManualCommissions");🟡
|
| Issue | Status |
|---|---|
superRefine to enforce saleAmount when importStripeInvoices: false |
✅ Already implemented in the schema |
Test case for sale without saleAmount expecting 422 |
✅ Already added to validationCases |
Positive Highlights
- Clean discriminated union with
superRefinecross-field validation — well done. - Conflict detection (409 on duplicate
invoiceId) and link ownership (404) are covered in tests. triggerAggregateDueCommissionsgated on the last commission in the batch — correct, fires once per batch.waitUntilused correctly for side-effects (Prisma stat updates, workflow execution) so the 202 response isn't blocked.isFirstConversionis correctly evaluated synchronously against the just-upserted customer, which will havesales: 0on creation.
🧠 Learnings used
Learnt from: steven-tey
Repo: dubinc/dub PR: 0
File: :0-0
Timestamp: 2026-06-03T20:04:17.265Z
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: pepeladeira
Repo: dubinc/dub PR: 3803
File: apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/analytics/use-commissions-analytics-query.ts:13-18
Timestamp: 2026-04-24T22:23:11.319Z
Learning: In `apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/analytics/use-commissions-analytics-query.ts`, the `status` useMemo intentionally defaults to `"paid"` when `searchParamsObj.commissionStatus` is absent or unrecognized. The Commissions analytics page is designed to show paid commissions by default on first load, not "All". Do not flag this fallback as a bug in future reviews.
Learnt from: devkiran
Repo: dubinc/dub PR: 3919
File: apps/web/lib/sandbox/mock-payout-completion.ts:139-165
Timestamp: 2026-05-22T06:06:02.038Z
Learning: In `apps/web/lib/sandbox/mock-payout-completion.ts`, the grouping logic that reduces multiple payouts per partner to one entry (`partnerPayouts`) and then uses only `partnerPayoutList[0]` when constructing the `sendBatchEmail` payload is intentional. The design deliberately sends at most one email per partner, even when that partner has multiple payouts in the same invoice. Do not flag this as a data-loss issue in future reviews.
Learnt from: steven-tey
Repo: dubinc/dub PR: 3961
File: apps/web/app/(ee)/api/workflows/create-partner-commission/route.ts:388-407
Timestamp: 2026-05-31T20:06:32.526Z
Learning: In dubinc/dub, click commissions are NOT created through the `create-partner-commission` QStash workflow (`POST /api/workflows/create-partner-commission`). Click commissions are handled exclusively by the `/api/cron/aggregate-clicks` cron job. The `create-partner-commission` workflow is only invoked for `lead`, `sale`, and `custom` events.
Learnt from: devkiran
Repo: dubinc/dub PR: 3674
File: apps/web/ui/partners/fraud-risks/commissions-on-hold-table.tsx:44-44
Timestamp: 2026-04-01T10:28:28.392Z
Learning: In the `dubinc/dub` codebase, `fraudEventGroupId` is intentionally excluded from `getCommissionsQuerySchema` (in `apps/web/lib/zod/schemas/commissions.ts`) to avoid exposing it in public OpenAPI specs and SDKs. It is instead extracted directly from `searchParams` in the EE-only API routes (`apps/web/app/(ee)/api/commissions/route.ts` and `apps/web/app/(ee)/api/commissions/count/route.ts`) and forwarded to the underlying query functions (`getCommissions`, `getCommissionsCount`). The filter works correctly at runtime despite being absent from the public schema.
|
@coderabbitai final review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== 1. Check for console.time/timeEnd in route.ts ==="
rg -n "console\.time" apps/web/app/\(ee\)/api/commissions/route.ts
echo ""
echo "=== 2. Full commissions route.ts ==="
cat apps/web/app/\(ee\)/api/commissions/route.ts
echo ""
echo "=== 3. saleAmount schema ==="
rg -n "saleAmount" apps/web/lib/zod/schemas/commissions.ts -A3 -B1
echo ""
echo "=== 4. invoice_id empty string in create-manual-commissions ==="
rg -n "invoice_id" apps/web/lib/api/commissions/create-manual-commissions.ts -A2 -B2
echo ""
echo "=== 5. limit: 60 in create-manual-commissions ==="
rg -n "limit" apps/web/lib/api/commissions/create-manual-commissions.ts -A2 -B2
echo ""
echo "=== 6. setTimeout in test ==="
rg -n "setTimeout" apps/web/tests/commissions/create-commission.test.ts -A2 -B2
echo ""
echo "=== 7. Full test file ==="
cat apps/web/tests/commissions/create-commission.test.ts
echo ""
echo "=== 8. Full zod schemas commissions.ts ==="
cat apps/web/lib/zod/schemas/commissions.ts
echo ""
echo "=== 9. Full create-manual-commissions.ts ==="
cat apps/web/lib/api/commissions/create-manual-commissions.ts
echo ""
echo "=== 10. resolve-link-and-customer full ==="
cat apps/web/lib/api/commissions/resolve-link-and-customer.ts
echo ""
echo "=== 11. Check create-commission openapi ==="
cat apps/web/lib/openapi/commissions/create-commission.ts
echo ""
echo "=== 12. Check for any remaining server action usages ==="
rg -rn "createManualCommissionAction\|createCommissionSchema\|useExistingEvents" --type ts -g "*.ts" -g "*.tsx"
echo ""
echo "=== 13. Check triggerAggregateDueCommissions in workflow route ==="
rg -n "triggerAggregateDueCommissions" apps/web/app/\(ee\)/api/workflows/create-partner-commission/route.ts -A5 -B5
echo ""
echo "=== 14. Check for any null-safety issues in resolveLinkAndCustomer ==="
cat apps/web/lib/api/commissions/resolve-link-and-customer.ts 2>/dev/null
echo ""
echo "=== 15. Check invoice-paid.ts resolvePromotionCodeError fix ==="
rg -n "resolvePromotionCodeError\|resolvePromotionCode" apps/web/app/\(ee\)/api/stripe/integration/webhook/invoice-paid.ts -A3 -B3
echo ""
echo "=== 16. Check createCommissionSheet for any issues ==="
cat "apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx" 2>/dev/null | head -200
echo ""
echo "=== 17. Check for any error handling gaps in POST handler ==="
rg -n "programId\|program" apps/web/app/\(ee\)/api/commissions/route.ts -A3 -B1
echo ""
echo "=== 18. Check requiredPlan in route ==="
rg -n "requiredPlan\|requiredRole" apps/web/app/\(ee\)/api/commissions/route.ts -A2 -B1Length of output: 60843
Breaking ChangesNone. All previously confirmed: Open Issues (from previous reviews + one new critical finding)🔴
|
| Issue | Status |
|---|---|
Hardcoded limit: 60 in getCustomerStripeInvoices call |
✅ Fixed — now uses the default 100 |
superRefine to enforce saleAmount when importStripeInvoices: false |
✅ Present (Line 549) |
Test case for sale without saleAmount expecting 422 |
✅ Present in validationCases |
The empty-string invoiceId propagation is the blocker worth fixing before this ships. Everything else is cleanup.
🧠 Learnings used
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: pepeladeira
Repo: dubinc/dub PR: 3803
File: apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/analytics/use-commissions-analytics-query.ts:13-18
Timestamp: 2026-04-24T22:23:11.319Z
Learning: In `apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/analytics/use-commissions-analytics-query.ts`, the `status` useMemo intentionally defaults to `"paid"` when `searchParamsObj.commissionStatus` is absent or unrecognized. The Commissions analytics page is designed to show paid commissions by default on first load, not "All". Do not flag this fallback as a bug in future reviews.
Learnt from: devkiran
Repo: dubinc/dub PR: 3674
File: apps/web/ui/partners/fraud-risks/commissions-on-hold-table.tsx:44-44
Timestamp: 2026-04-01T10:28:28.392Z
Learning: In the `dubinc/dub` codebase, `fraudEventGroupId` is intentionally excluded from `getCommissionsQuerySchema` (in `apps/web/lib/zod/schemas/commissions.ts`) to avoid exposing it in public OpenAPI specs and SDKs. It is instead extracted directly from `searchParams` in the EE-only API routes (`apps/web/app/(ee)/api/commissions/route.ts` and `apps/web/app/(ee)/api/commissions/count/route.ts`) and forwarded to the underlying query functions (`getCommissions`, `getCommissionsCount`). The filter works correctly at runtime despite being absent from the public schema.
Learnt from: devkiran
Repo: dubinc/dub PR: 3919
File: apps/web/lib/sandbox/mock-payout-completion.ts:139-165
Timestamp: 2026-05-22T06:06:02.038Z
Learning: In `apps/web/lib/sandbox/mock-payout-completion.ts`, the grouping logic that reduces multiple payouts per partner to one entry (`partnerPayouts`) and then uses only `partnerPayoutList[0]` when constructing the `sendBatchEmail` payload is intentional. The design deliberately sends at most one email per partner, even when that partner has multiple payouts in the same invoice. Do not flag this as a data-loss issue in future reviews.
Learnt from: steven-tey
Repo: dubinc/dub PR: 3732
File: apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/payouts/payout-table.tsx:262-305
Timestamp: 2026-04-10T03:43:25.907Z
Learning: In `apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/payouts/payout-table.tsx`, the `disabledTooltip` for the "Confirm selected" button intentionally omits edge-case ineligibility reasons (already-paid/failed status, `mode == null`) for brevity. The tooltip only surfaces the three most common/actionable reasons: below minimum payout amount, partner has not connected payouts, and fraud hold. Do not flag this omission in future reviews.
Learnt from: devkiran
Repo: dubinc/dub PR: 3824
File: apps/web/lib/partners/create-partner-commission.ts:193-195
Timestamp: 2026-04-30T05:05:45.091Z
Learning: In `apps/web/lib/partners/create-partner-commission.ts`, there can only be one reward per partner + program pair. When iterating over multiple products (Stripe line items) to build the `rewards` array, `determinePartnerReward` will always resolve to the same reward object for all products. Therefore, using `rewards[0].reward` as the canonical reward is safe and not order-dependent. Eligibility checks such as `firstCommission.rewardId` and `maxDuration` are safe to run against `rewards[0].reward`.
Learnt from: steven-tey
Repo: dubinc/dub PR: 3961
File: apps/web/app/(ee)/api/workflows/create-partner-commission/route.ts:388-407
Timestamp: 2026-05-31T20:06:35.505Z
Learning: In dubinc/dub, click commissions are NOT created through the `create-partner-commission` QStash workflow (`POST /api/workflows/create-partner-commission`). Click commissions are handled exclusively by the `/api/cron/aggregate-clicks` cron job. The `create-partner-commission` workflow is only invoked for `lead`, `sale`, and `custom` events.
Learnt from: pepeladeira
Repo: dubinc/dub PR: 3923
File: apps/web/lib/actions/partners/update-reward.ts:160-168
Timestamp: 2026-05-18T22:18:20.575Z
Learning: In dubinc/dub, `updateRewardAction` (apps/web/lib/actions/partners/update-reward.ts) only updates the `Reward` row and does NOT touch `programEnrollment` rows or queue any enrollment sync cron job. The enrollment sync cron at `/api/cron/rewards/sync-enrollments` only handles `create` and `delete` actions — there is no update-action cron path. Therefore, calling `notifyPartnersRewardChanged` directly inside `waitUntil(Promise.allSettled([...]))` within `updateRewardAction` is the correct pattern. Do NOT flag this as "premature notification before enrollment sync finalization" or as a duplication risk.
Learnt from: pepeladeira
Repo: dubinc/dub PR: 3923
File: apps/web/lib/actions/partners/update-reward.ts:160-168
Timestamp: 2026-05-18T22:18:20.575Z
Learning: In dubinc/dub, `updateRewardAction` (apps/web/lib/actions/partners/update-reward.ts) only updates the `Reward` row and does NOT write to `programEnrollment` rows. The enrollment sync cron (`/api/cron/rewards/sync-enrollments`) only handles `create` and `delete` actions — there is no "update" enrollment sync. Therefore, calling `notifyPartnersRewardChanged` directly inside `waitUntil(Promise.allSettled([...]))` within `updateRewardAction` is the correct and complete pattern for reward-update partner notifications. Do NOT flag this as premature notification or potential duplication.
Learnt from: devkiran
Repo: dubinc/dub PR: 3676
File: apps/web/lib/api/partner-profile/get-earnings-for-partner.ts:58-59
Timestamp: 2026-04-07T12:03:57.245Z
Learning: In `apps/web/lib/api/partner-profile/get-earnings-for-partner.ts`, the potential conflict between the `linkId` (user-provided filter) and `linkIds` (scope constraint) in the Prisma `where` clause is intentionally handled at the route level (`apps/web/app/(ee)/api/partner-profile/programs/[programId]/earnings/route.ts`): before calling `getEarningsForPartner`, the route explicitly validates that if `parsedQuery.linkId` is present and `assignedLinkIds` is defined, the `linkId` must be in `assignedLinkIds`, throwing a `forbidden` DubApiError otherwise. Therefore, inside `getEarningsForPartner`, when both `linkId` and `linkIds` are present, `linkId` is already guaranteed to be within `linkIds`, making the spread `...(linkIds ? { linkId: { in: linkIds } } : {})` safe and not requiring intersection logic inside the function itself.
Summary by CodeRabbit
New Features
Enhancements
Refactor
Tests