Fix partners with payoutsEnabledAt set but defaultPayoutMethod null#3973
Merged
Conversation
…faultPayoutMethod
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughCentralizes payout-state change detection (new hasPayoutStateChanged), updates the Stripe webhook to use it, clears defaultPayoutMethod consistently when disabling payouts across codepaths, and adds a migration to reconcile existing partner records. ChangesPayout State Management and Partner Account Sync
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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[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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts`:
- Around line 50-72: The migration runs Promise.allSettled over a batch which
may trigger Stripe rate limits because recomputePartnerPayoutState performs
multiple Stripe calls per partner; modify the batching so you process chunks of
partners sequentially (use BATCH_SIZE) instead of firing all batches
concurrently: iterate over chunks, await
Promise.allSettled(partnersChunk.map(...recomputePartnerPayoutState...)) to
finish each chunk, then await a small sleep/delay (e.g., 200–1000ms) before
continuing to the next chunk; also consider reducing BATCH_SIZE to a safer value
and return/collect results as before.
- Around line 52-63: The migration updates payoutsEnabledAt and
defaultPayoutMethod from recomputePartnerPayoutState but omits the returned
cryptoWalletAddress; modify the prisma.partner.update call in this migration to
also set cryptoWalletAddress to the value returned by
recomputePartnerPayoutState (use the cryptoWalletAddress variable alongside
payoutsEnabledAt and defaultPayoutMethod) so the DB state matches the
recipient-configuration-updated webhook behavior and prevents stale wallet
addresses.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 628e4bb5-51cc-407d-9324-b087059fd3c8
📒 Files selected for processing (2)
apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.tsapps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts
💤 Files with no reviewable changes (1)
- apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts (1)
107-107: ⚡ Quick winAdd top-level error handling and teardown for the script entrypoint.
main()is invoked without awaiting completion, catching errors, or disconnecting Prisma. On rejection this surfaces as an unhandled promise rejection, and on success the open Prisma connection can keep the process from exiting cleanly.♻️ Proposed entrypoint handling
-main(); +main() + .catch((error) => { + console.error(error); + process.exitCode = 1; + }) + .finally(async () => { + await prisma.$disconnect(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts` at line 107, The script currently calls main() with no awaiting, error handling, or teardown; update the entrypoint to await main(), catch and log errors, ensure Prisma is disconnected (call prisma.$disconnect() or equivalent) in a finally block, and exit with non-zero code on error; specifically modify the top-level invocation of main() to an async wrapper or Promise chain that uses try/catch/finally, references main() and the Prisma client (e.g., prisma), logs errors via the existing logger or console, calls prisma.$disconnect(), and calls process.exit(1) on failure and process.exit(0) on success to ensure clean shutdown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts`:
- Line 107: The script currently calls main() with no awaiting, error handling,
or teardown; update the entrypoint to await main(), catch and log errors, ensure
Prisma is disconnected (call prisma.$disconnect() or equivalent) in a finally
block, and exit with non-zero code on error; specifically modify the top-level
invocation of main() to an async wrapper or Promise chain that uses
try/catch/finally, references main() and the Prisma client (e.g., prisma), logs
errors via the existing logger or console, calls prisma.$disconnect(), and calls
process.exit(1) on failure and process.exit(0) on success to ensure clean
shutdown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a9a4603-ab7b-4263-a5a3-b59e732cb3af
📒 Files selected for processing (1)
apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts
Return hasPayoutStateChanged from recomputePartnerPayoutState and use it in account-updated webhook.
Collaborator
|
@coderabbitai full review please |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/lib/payouts/recompute-partner-payout-state.ts (1)
116-118: ⚡ Quick win
hasPayoutStateChangedcomparesDates by reference, not value.
partner.payoutsEnabledAt !== payoutsEnabledAtis a reference comparison. It happens to be correct today only because Line 93 reuses the exact sameDateinstance (payoutsEnabledAt = partner.payoutsEnabledAt) in the no-change path. If that branch is ever refactored to clone/normalize the date (e.g.new Date(partner.payoutsEnabledAt)), this flag would always report a change and defeat its purpose of suppressing unnecessary writes.Suggest comparing by time value to make the contract robust:
♻️ Compare timestamps instead of references
- const hasPayoutStateChanged = - partner.payoutsEnabledAt !== payoutsEnabledAt || - partner.defaultPayoutMethod !== defaultPayoutMethod; + const hasPayoutStateChanged = + (partner.payoutsEnabledAt?.getTime() ?? null) !== + (payoutsEnabledAt?.getTime() ?? null) || + partner.defaultPayoutMethod !== defaultPayoutMethod;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/payouts/recompute-partner-payout-state.ts` around lines 116 - 118, The equality check for hasPayoutStateChanged currently compares Date objects by reference (partner.payoutsEnabledAt !== payoutsEnabledAt), which is fragile; change the comparison to compare their time values instead (e.g., compare partner.payoutsEnabledAt?.getTime() to payoutsEnabledAt?.getTime() or use numeric coercion) while keeping the existing defaultPayoutMethod inequality check, so hasPayoutStateChanged becomes true only when the actual timestamp value or defaultPayoutMethod differ.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/lib/payouts/recompute-partner-payout-state.ts`:
- Around line 116-118: The equality check for hasPayoutStateChanged currently
compares Date objects by reference (partner.payoutsEnabledAt !==
payoutsEnabledAt), which is fragile; change the comparison to compare their time
values instead (e.g., compare partner.payoutsEnabledAt?.getTime() to
payoutsEnabledAt?.getTime() or use numeric coercion) while keeping the existing
defaultPayoutMethod inequality check, so hasPayoutStateChanged becomes true only
when the actual timestamp value or defaultPayoutMethod differ.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dbd6d697-17fd-48a9-a334-46211b18f5b0
📒 Files selected for processing (2)
apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.tsapps/web/lib/payouts/recompute-partner-payout-state.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.ts
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== recompute-partner-payout-state.ts ==="
cat apps/web/lib/payouts/recompute-partner-payout-state.ts
echo ""
echo "=== account-updated.ts ==="
cat "apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.ts"
echo ""
echo "=== fix-payouts-enabled-without-default-method.ts ==="
cat apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.tsLength of output: 14733
Review: Fix partners with
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.ts (1)
109-131:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical: early return on
!hasPayoutStateChangedskipspayoutMethodHashrefresh and fraud detection for bank-account-only updates.When a partner updates their bank account but
defaultPayoutMethod/payoutsEnabledAtare unchanged (e.g., a new bank fingerprint for the sameconnectmethod),hasPayoutStateChangedisfalse, so we return at Line 109-111 before reaching thegetPartnerBankAccountfetch (Line 117), thepayoutMethodHashupdate (Line 124), anddetectDuplicatePayoutMethodFraud(Line 144). This is a regression:payoutMethodHashgoes stale and duplicate-payout-method fraud detection no longer fires on bank changes.The bank fingerprint refresh and fraud detection should run whenever a usable bank account is present, independent of
hasPayoutStateChanged. Consider running the bank-account hash refresh + fraud detection branch first (gating only the downstream payout-retry/queue logic onhasPayoutStateChanged), rather than returning early before it.Want me to draft a restructured version that always refreshes
payoutMethodHashand runs fraud detection, while keeping the payout-retry queueing gated onhasPayoutStateChanged?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(ee)/api/stripe/connect/webhook/account-updated.ts around lines 109 - 131, The early return when !hasPayoutStateChanged prevents bank fingerprint refresh and fraud checks; refactor so getPartnerBankAccount(partner.stripeConnectId!), updating payoutMethodHash via prisma.partner.update(... payoutMethodHash: bankAccount.fingerprint ...), and calling detectDuplicatePayoutMethodFraud(...) always run when a usable bank account exists, but keep the existing payout-retry/queue logic gated behind hasPayoutStateChanged (i.e., move the bank-account fetch/update and detectDuplicatePayoutMethodFraud above the hasPayoutStateChanged return and only bail out or run downstream retry/queue code based on hasPayoutStateChanged). Ensure you still handle the no-bank-account case (return/skip) and preserve the payoutsEnabledAt null branch behavior for disabling payouts.
🧹 Nitpick comments (1)
apps/web/lib/payouts/recompute-partner-payout-state.ts (1)
116-118: 💤 Low value
payoutsEnabledAtequality depends on intentional Date reference reuse — add a clarifying comment.
hasPayoutStateChangedcomparesDateobjects with!==, which is reference equality. This is currently correct only because the unchanged branch (Line 92-93) reusespartner.payoutsEnabledAtby reference rather than constructing a newDate. A future refactor that clones the date (e.g.,new Date(partner.payoutsEnabledAt)) would silently make this always report a change. A short comment documenting the intentional reference comparison would protect against that.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/payouts/recompute-partner-payout-state.ts` around lines 116 - 118, The reference-equality comparison in hasPayoutStateChanged (partner.payoutsEnabledAt !== payoutsEnabledAt || partner.defaultPayoutMethod !== defaultPayoutMethod) relies on intentionally reusing the original Date object rather than cloning it; add a short clarifying comment directly above this expression (or on the payoutsEnabledAt assignment) stating that partner.payoutsEnabledAt is intentionally reused for reference equality and warning that creating a new Date(partner.payoutsEnabledAt) would break this check — alternatively note that switching to value equality (e.g., comparing getTime()) is the safe refactor if cloning is ever required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts`:
- Around line 53-65: The migration currently always writes recomputed payouts
(payoutsEnabledAt, defaultPayoutMethod, cryptoWalletAddress) which can clobber
live state on transient Stripe failures; change the logic around
recomputePartnerPayoutState to read and use the returned hasPayoutStateChanged
flag and only call prisma.partner.update when hasPayoutStateChanged is true,
skip the update when false, and emit a warning log if the recomputed values are
null (or indicate missing Stripe data) so transient failures are visible rather
than persisted; reference recomputePartnerPayoutState, hasPayoutStateChanged,
and prisma.partner.update to implement this guard and conditional logging.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/stripe/connect/webhook/account-updated.ts:
- Around line 109-131: The early return when !hasPayoutStateChanged prevents
bank fingerprint refresh and fraud checks; refactor so
getPartnerBankAccount(partner.stripeConnectId!), updating payoutMethodHash via
prisma.partner.update(... payoutMethodHash: bankAccount.fingerprint ...), and
calling detectDuplicatePayoutMethodFraud(...) always run when a usable bank
account exists, but keep the existing payout-retry/queue logic gated behind
hasPayoutStateChanged (i.e., move the bank-account fetch/update and
detectDuplicatePayoutMethodFraud above the hasPayoutStateChanged return and only
bail out or run downstream retry/queue code based on hasPayoutStateChanged).
Ensure you still handle the no-bank-account case (return/skip) and preserve the
payoutsEnabledAt null branch behavior for disabling payouts.
---
Nitpick comments:
In `@apps/web/lib/payouts/recompute-partner-payout-state.ts`:
- Around line 116-118: The reference-equality comparison in
hasPayoutStateChanged (partner.payoutsEnabledAt !== payoutsEnabledAt ||
partner.defaultPayoutMethod !== defaultPayoutMethod) relies on intentionally
reusing the original Date object rather than cloning it; add a short clarifying
comment directly above this expression (or on the payoutsEnabledAt assignment)
stating that partner.payoutsEnabledAt is intentionally reused for reference
equality and warning that creating a new Date(partner.payoutsEnabledAt) would
break this check — alternatively note that switching to value equality (e.g.,
comparing getTime()) is the safe refactor if cloning is ever required.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4c47d16-88b0-47ac-84ef-4af3508c586c
📒 Files selected for processing (3)
apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.tsapps/web/lib/payouts/recompute-partner-payout-state.tsapps/web/scripts/migrations/fix-payouts-enabled-without-default-method.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/scripts/migrations/backfill-default-payout-method.ts (1)
12-57: 💤 Low valueConsider batching for consistency and safety.
Unlike
scriptTwo, this function loads all matching partners into memory at once. While the specific constraints likely yield a small dataset, adding batch processing (similar toscriptTwo) would make the script more resilient to unexpected data volumes and provide consistent progress logging.🔧 Suggested batch processing pattern
async function scriptOne() { - const partners = await prisma.partner.findMany({ - where: { - payoutsEnabledAt: { - not: null, + let totalUpdated = 0; + + while (true) { + const partners = await prisma.partner.findMany({ + where: { + payoutsEnabledAt: { + not: null, + }, + stripeConnectId: { + not: null, + }, + defaultPayoutMethod: null, + paypalEmail: null, + stripeRecipientId: null, }, - stripeConnectId: { - not: null, - }, - defaultPayoutMethod: null, - paypalEmail: null, - stripeRecipientId: null, - }, - select: { - id: true, - stripeConnectId: true, - stripeRecipientId: true, - paypalEmail: true, - payoutsEnabledAt: true, - defaultPayoutMethod: true, - }, - orderBy: { - id: "asc", - }, - }); + select: { + id: true, + }, + take: 500, + }); - console.table(partners); + if (partners.length === 0) { + break; + } - if (partners.length === 0) { - console.log("No partners found"); - return; - } + const { count } = await prisma.partner.updateMany({ + where: { + id: { + in: partners.map((partner) => partner.id), + }, + }, + data: { + defaultPayoutMethod: "connect", + }, + }); - const { count } = await prisma.partner.updateMany({ - where: { - id: { - in: partners.map((partner) => partner.id), - }, - }, - data: { - defaultPayoutMethod: "connect", - }, - }); + totalUpdated += count; + console.log(`Updated ${count} partners (total: ${totalUpdated})`); + } - console.log(`Updated ${count} partners`); + console.log(`Completed scriptOne: ${totalUpdated} partners updated`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/scripts/migrations/backfill-default-payout-method.ts` around lines 12 - 57, scriptOne currently loads all matching partners into memory; change it to batch-process to avoid large-memory loads and to log progress: iterate using a paginated loop (use prisma.partner.findMany with take and cursor (or skip/take) to fetch e.g., 100–1000 partners per batch, selecting id and the same fields), for each batch call prisma.partner.updateMany with ids from that batch to set defaultPayoutMethod to "connect", and accumulate/log the count after each batch until no more results. Ensure you keep the same selection/filter logic and preserve ordering (orderBy id asc) so cursor pagination with id works correctly and stop when a batch returns an empty array.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/scripts/migrations/backfill-default-payout-method.ts`:
- Around line 12-57: scriptOne currently loads all matching partners into
memory; change it to batch-process to avoid large-memory loads and to log
progress: iterate using a paginated loop (use prisma.partner.findMany with take
and cursor (or skip/take) to fetch e.g., 100–1000 partners per batch, selecting
id and the same fields), for each batch call prisma.partner.updateMany with ids
from that batch to set defaultPayoutMethod to "connect", and accumulate/log the
count after each batch until no more results. Ensure you keep the same
selection/filter logic and preserve ordering (orderBy id asc) so cursor
pagination with id works correctly and stop when a batch returns an empty array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54a22aa7-7bf5-4aae-a87b-acb02cbf2a39
📒 Files selected for processing (4)
apps/web/app/(ee)/api/admin/partners/delete-account/route.tsapps/web/lib/partners/create-stablecoin-payout.tsapps/web/lib/partners/create-stripe-transfer.tsapps/web/scripts/migrations/backfill-default-payout-method.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/lib/partners/create-stablecoin-payout.ts
Collaborator
|
@coderabbitai full review please |
Contributor
|
🧠 Learnings used✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/scripts/migrations/backfill-default-payout-method.ts (2)
63-71: ⚡ Quick winConsider adding
select: { id: true }to reduce query payload.The query fetches full partner records but only uses
partner.id. Adding aselectclause would reduce database and network overhead for potentially large batches.const partners = await prisma.partner.findMany({ where: { payoutsEnabledAt: null, defaultPayoutMethod: { not: null, }, }, + select: { + id: true, + }, take: 500, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/scripts/migrations/backfill-default-payout-method.ts` around lines 63 - 71, The query using prisma.partner.findMany currently loads full partner records into the partners array but only uses partner.id; update the call to include a select clause (select: { id: true }) so only the id is returned—keep the existing where filter and take: 500 and adjust any downstream usage that expects full partner objects to use the id property from the now-selected shape.
5-8: ⚡ Quick winConsider adding error handling and a summary.
The script modifies production data but lacks error handling. If a batch fails mid-execution, there's no indication of progress or which partners were affected.
Suggested improvement
async function main() { - await scriptOne(); - await scriptTwo(); + try { + await scriptOne(); + await scriptTwo(); + console.log("Migration completed successfully"); + } catch (error) { + console.error("Migration failed:", error); + process.exit(1); + } } -main(); +main().finally(() => prisma.$disconnect());Also applies to: 92-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/scripts/migrations/backfill-default-payout-method.ts` around lines 5 - 8, The main function lacks error handling and progress reporting; wrap the calls to scriptOne and scriptTwo in a try/catch and ensure each script call is awaited with per-batch logging and error capture (reference functions scriptOne, scriptTwo, and main). Inside each script implement progress checkpoints (e.g., log partner IDs or batch ranges before/after processing) and throw or return structured errors so main can log full error details and which batch/partner failed. In main, on success log a concise summary of processed counts and on failure log the error, the last successful checkpoint, and exit non-zero so CI/ops know it failed. Ensure any resources are cleaned up in a finally block if needed.apps/web/lib/payouts/recompute-partner-payout-state.ts (1)
116-118: 💤 Low valueDocument the Date reference equality invariant.
hasPayoutStateChangedrelies on reference equality (!==) forpayoutsEnabledAt, which only works because unchanged paths reusepartner.payoutsEnabledAtdirectly (line 93) while changed paths createnew Date(). This is fragile—if someone refactors to clone the date, the comparison breaks silently.Consider adding a brief comment above this block to preserve the invariant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/payouts/recompute-partner-payout-state.ts` around lines 116 - 118, The comparison that computes hasPayoutStateChanged uses reference equality for dates and depends on unchanged control flow reusing partner.payoutsEnabledAt rather than cloning it; add a short comment immediately above the hasPayoutStateChanged declaration explaining the Date reference-equality invariant (i.e., that unchanged code paths reuse partner.payoutsEnabledAt while changed paths create a new Date()), referencing partner.payoutsEnabledAt, payoutsEnabledAt and defaultPayoutMethod so future readers/maintainers know not to clone the Date or the check will break.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/stripe/connect/webhook/account-updated.ts:
- Around line 109-111: The early return when hasPayoutStateChanged is false
skips fetching the bank account and running detectDuplicatePayoutMethodFraud,
leaving payoutMethodHash stale and bypassing fraud/email logic; refactor the
handler so that when partner.stripeConnectId is present you always fetch the
bank account and run detectDuplicatePayoutMethodFraud and update
payoutMethodHash (and send the connected payout method email) first, then only
after that block check hasPayoutStateChanged and return early if still false
(i.e., postpone the early return until after payoutMethodHash/fraud/email
processing and only prevent queuing payout retries when appropriate).
---
Nitpick comments:
In `@apps/web/lib/payouts/recompute-partner-payout-state.ts`:
- Around line 116-118: The comparison that computes hasPayoutStateChanged uses
reference equality for dates and depends on unchanged control flow reusing
partner.payoutsEnabledAt rather than cloning it; add a short comment immediately
above the hasPayoutStateChanged declaration explaining the Date
reference-equality invariant (i.e., that unchanged code paths reuse
partner.payoutsEnabledAt while changed paths create a new Date()), referencing
partner.payoutsEnabledAt, payoutsEnabledAt and defaultPayoutMethod so future
readers/maintainers know not to clone the Date or the check will break.
In `@apps/web/scripts/migrations/backfill-default-payout-method.ts`:
- Around line 63-71: The query using prisma.partner.findMany currently loads
full partner records into the partners array but only uses partner.id; update
the call to include a select clause (select: { id: true }) so only the id is
returned—keep the existing where filter and take: 500 and adjust any downstream
usage that expects full partner objects to use the id property from the
now-selected shape.
- Around line 5-8: The main function lacks error handling and progress
reporting; wrap the calls to scriptOne and scriptTwo in a try/catch and ensure
each script call is awaited with per-batch logging and error capture (reference
functions scriptOne, scriptTwo, and main). Inside each script implement progress
checkpoints (e.g., log partner IDs or batch ranges before/after processing) and
throw or return structured errors so main can log full error details and which
batch/partner failed. In main, on success log a concise summary of processed
counts and on failure log the error, the last successful checkpoint, and exit
non-zero so CI/ops know it failed. Ensure any resources are cleaned up in a
finally block if needed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: db6c0210-c6ad-4104-954d-46f7292f0663
📒 Files selected for processing (6)
apps/web/app/(ee)/api/admin/partners/delete-account/route.tsapps/web/app/(ee)/api/stripe/connect/webhook/account-updated.tsapps/web/lib/partners/create-stablecoin-payout.tsapps/web/lib/partners/create-stripe-transfer.tsapps/web/lib/payouts/recompute-partner-payout-state.tsapps/web/scripts/migrations/backfill-default-payout-method.ts
steven-tey
approved these changes
Jun 6, 2026
Collaborator
|
@coderabbitai full review, any breaking changes? incorrect logic? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== recompute-partner-payout-state.ts ==="
cat apps/web/lib/payouts/recompute-partner-payout-state.ts
echo ""
echo "=== account-updated.ts ==="
cat "apps/web/app/(ee)/api/stripe/connect/webhook/account-updated.ts"
echo ""
echo "=== fix-payouts-enabled-without-default-method.ts (migration) ==="
fd "fix-payouts-enabled-without-default-method" --type f | xargs cat 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== backfill-default-payout-method.ts ==="
fd "backfill-default-payout-method" --type f | xargs cat 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== create-stablecoin-payout.ts (relevant section) ==="
grep -n "defaultPayoutMethod\|payoutsEnabledAt\|stripeRecipientId" apps/web/lib/partners/create-stablecoin-payout.ts | head -40
echo ""
echo "=== create-stripe-transfer.ts (relevant section) ==="
grep -n "defaultPayoutMethod\|payoutsEnabledAt" apps/web/lib/partners/create-stripe-transfer.ts | head -20
echo ""
echo "=== delete-account route ==="
fd "delete-account" --type f -g "*.ts" | xargs grep -l "defaultPayoutMethod" 2>/dev/null | xargs cat 2>/dev/null | head -80
echo ""
echo "=== getPartnerBankAccount usages ==="
rg -n "getPartnerBankAccount" --type ts -C 3Length of output: 21940
🔴 Breaking Change —
|
| Area | Status |
|---|---|
create-stablecoin-payout.ts — now clears defaultPayoutMethod on closed account |
✅ |
create-stripe-transfer.ts — now clears defaultPayoutMethod when payout disabled |
✅ |
recomputePartnerPayoutState — preserves existing default when still active |
✅ |
account-updated.ts — always syncs country/profileType unconditionally |
✅ |
backfill-default-payout-method.ts scriptTwo() — clears orphan defaultPayoutMethod values |
✅ |
First-time "connected payout method" email — correctly reads old partner.payoutsEnabledAt |
✅ |
TL;DR: The critical regression is the fraud detection bypass in account-updated.ts. The scriptOne Stripe-verification gap is a data-integrity risk for the one-time migration. Both should be addressed before merging.
🧠 Learnings used
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: 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: 3952
File: apps/web/lib/tremendous/send-tremendous-payouts.ts:201-221
Timestamp: 2026-05-29T08:18:35.355Z
Learning: In dubinc/dub, the partner payout notification email subject intentionally uses `payouts[0].program.name` (the first payout's program) rather than a deduped list of all program names. This mirrors the pattern in `apps/web/lib/partners/create-stripe-transfer.ts`, which also uses `currentInvoicePayouts[0].program.name` in the email subject while separately deduping program names only for the Stripe transfer description. Do not flag this as an inconsistency in future reviews.
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: 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: devkiran
Repo: dubinc/dub PR: 3449
File: apps/web/lib/partners/create-stablecoin-payout.ts:99-137
Timestamp: 2026-02-23T11:40:48.138Z
Learning: In apps/web/lib/partners/create-stablecoin-payout.ts, when handling Stripe recipient account issues: (1) closed account is treated as a permanent failure—clears stripeRecipientId, marks payouts as processed, and returns silently; (2) missing crypto wallet capability is treated as a temporary/retriable failure—clears only payoutsEnabledAt, keeps stripeRecipientId for retry, throws an error to signal the issue. The throw vs. return difference is intentional to enable retry after the partner fixes their payout method in case 2.
<!-- [/add_learning]
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: steven-tey
Repo: dubinc/dub PR: 3584
File: apps/web/app/(ee)/api/admin/delete-partner-account/route.ts:120-134
Timestamp: 2026-03-14T20:01:18.394Z
Learning: In the dubinc/dub codebase, `prisma.partner.delete()` throws a Prisma error because `relationMode = "prisma"` enforces Restrict (default) on models without explicit `onDelete`: Payout, PartnerRewind, PartnerReferral, DiscountCode, and NotificationEmail all reference Partner without onDelete Cascade. The workaround is to explicitly `deleteMany` those non-cascade relations before calling `prisma.partner.delete()`.
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: 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.
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
3 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.
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores