Flag partner through the rejected application modal#3866
Conversation
|
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:
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends partner rejection with fraud-flagging capability. It adds ChangesFraud Flagging on Partner Rejection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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: 3
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/admin/fraud-alerts/[id]/route.ts (1)
109-217:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftClaim the pending alerts before applying ban side effects.
This branch bans enrollments, resolves fraud groups, and queues downstream work before
updateManyproves the alerts were still pending. If another reviewer confirms or dismisses the same partner's alerts in between, this path can still mutate enrollment state and then return409, which is a bad TOCTOU bug. The alert-state transition and the enrollment updates need to be claimed atomically first, ideally in a transaction, and only then should the side effects run.🤖 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/admin/fraud-alerts/[id]/route.ts around lines 109 - 217, The current loop applies enrollment bans, calls resolveFraudGroups, and enqueues banPartnerQueue before the call to prisma.fraudAlert.updateMany, creating a TOCTOU where another reviewer could claim the same alerts and cause conflicting state; fix by atomically claiming the alerts first and only running side-effects after the claim succeeds: use prisma.$transaction (or an atomic updateMany) to mark the targeted fraud alerts as confirmed (the same filter used in the existing prisma.fraudAlert.updateMany) and, if desired, update corresponding programEnrollment rows inside that same transaction; check the returned count from the transaction and abort (return 409) if zero, then after the transaction completes run resolveFraudGroups, prisma.programEnrollment.update (or enqueue work) and push to postConfirmTasks/banPartnerQueue as side-effects only when the claim succeeded.
🤖 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/admin/fraud-alerts/[id]/route.ts:
- Around line 219-243: The loop currently pushes reportCrossProgramBanToNetwork
for every item in pendingFraudAlerts even if the enrollment wasn't transitioned
to banned; change it to only fan out network reports for enrollments that
actually became banned in this request by building the report list from the set
of enrollments you transitioned to banned (track the programEnrollment objects
you updated to banned or collect from the code path that sets
bannedReason/bannedAt), and only call reportCrossProgramBanToNetwork for those
entries (dedupe by partnerId+programId to avoid duplicate reports) so
bannedReason/bannedAt are never null when creating partnerCrossProgramBan
events.
In `@apps/web/lib/api/partners/applications/reject-partner.ts`:
- Around line 191-199: The fraudAlert.create call is being run outside the main
transaction (via waitUntil), so if it fails the partner rejection may commit
without persisting the fraud flag; move the prisma.fraudAlert.create into the
same Prisma transaction that updates the enrollment/reviews (the code path
handling the enrollment update / application review), so the fraudAlert insert
is part of that transactional call and will rollback together on failure; keep
waitUntil only for non-critical side effects and remove or replace the existing
conditional that calls prisma.fraudAlert.create from the waitUntil block,
referencing the existing prisma.fraudAlert.create invocation, the waitUntil
usage, and the enrollment/application review transactional update code to
colocate the insert.
In `@apps/web/ui/modals/reject-partner-application-modal.tsx`:
- Around line 289-303: The textarea for flagForFraudReason lacks an accessible
label and descriptive relationship; add a visible <label htmlFor="..."> or an
aria-label on the textarea and connect the counter/help text via
aria-describedby. Ensure the textarea's id matches the label's htmlFor
(referencing the textarea controlled by flagForFraudReason, rows/maxLength using
MAX_FRAUD_REASON_LENGTH and disabled via isPending) and give the counter
paragraph an id that you reference in aria-describedby so screen readers
announce both the label and the remaining character count.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/admin/fraud-alerts/[id]/route.ts:
- Around line 109-217: The current loop applies enrollment bans, calls
resolveFraudGroups, and enqueues banPartnerQueue before the call to
prisma.fraudAlert.updateMany, creating a TOCTOU where another reviewer could
claim the same alerts and cause conflicting state; fix by atomically claiming
the alerts first and only running side-effects after the claim succeeds: use
prisma.$transaction (or an atomic updateMany) to mark the targeted fraud alerts
as confirmed (the same filter used in the existing prisma.fraudAlert.updateMany)
and, if desired, update corresponding programEnrollment rows inside that same
transaction; check the returned count from the transaction and abort (return
409) if zero, then after the transaction completes run resolveFraudGroups,
prisma.programEnrollment.update (or enqueue work) and push to
postConfirmTasks/banPartnerQueue as side-effects only when the claim succeeded.
🪄 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: 849f1398-9a75-4c34-8ee2-6ae2e5f5cbaf
📒 Files selected for processing (8)
apps/web/app/(ee)/admin.dub.co/(dashboard)/fraud-alerts/review-fraud-alert-sheet.tsxapps/web/app/(ee)/api/admin/fraud-alerts/[id]/route.tsapps/web/app/(ee)/api/partners/applications/reject/route.tsapps/web/lib/actions/partners/reject-partner-application.tsapps/web/lib/api/fraud/report-cross-program-ban-to-network.tsapps/web/lib/api/partners/applications/reject-partner.tsapps/web/lib/zod/schemas/partners.tsapps/web/ui/modals/reject-partner-application-modal.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/admin/fraud-alerts/[id]/route.ts (1)
256-291: ⚡ Quick winDrop the redundant per-key
programEnrollment.findUnique.The transaction at lines 168-184 just set
bannedReason: PartnerBannedReason.fraudandbannedAt: <now>for every entry inbansForSideEffects, so the second loop'sfindUnique(and thebannedReason == null || bannedAt == nullcontinue) is always satisfied and adds N sequential round trips per request. You can carrybannedAtonbansForSideEffectsand iterate that directly, dropping the key set + string-split + lookup entirely. This also lets thesereportCrossProgramBanToNetworkcalls run in parallel viaPromise.allSettled(postConfirmTasks).♻️ Proposed refactor
const bansForSideEffects: Array<{ programId: string; partnerId: string; workspaceId: string; previousStatus: ProgramEnrollmentStatus; + bannedAt: Date; }> = [];+ const bannedAt = new Date(); await tx.programEnrollment.update({ where: { partnerId_programId: { partnerId, programId }, }, data: { status: ProgramEnrollmentStatus.banned, - bannedAt: new Date(), + bannedAt, bannedReason: PartnerBannedReason.fraud, clickRewardId: null, leadRewardId: null, saleRewardId: null, discountId: null, }, }); bansForSideEffects.push({ programId, partnerId, workspaceId: enrollment.program.workspaceId, previousStatus: enrollment.status, + bannedAt, });- const crossProgramSourceKeys = new Set( - bansForSideEffects.map((b) => `${b.programId}:${b.partnerId}`), - ); - - for (const key of crossProgramSourceKeys) { - const delimiter = key.indexOf(":"); - const programId = key.slice(0, delimiter); - const partnerId = key.slice(delimiter + 1); - - const enrollment = await prisma.programEnrollment.findUnique({ - where: { - partnerId_programId: { partnerId, programId }, - }, - select: { - bannedReason: true, - bannedAt: true, - }, - }); - - if (enrollment?.bannedReason == null || enrollment.bannedAt == null) { - continue; - } - + for (const { programId, partnerId, bannedAt } of bansForSideEffects) { + const key = `${programId}:${partnerId}`; postConfirmTasks.push( reportCrossProgramBanToNetwork({ partnerId, programId, - bannedReason: enrollment.bannedReason, - bannedAt: enrollment.bannedAt, + bannedReason: PartnerBannedReason.fraud, + bannedAt, fraudAlertReason: fraudReasonByEnrollmentKey.get(key) ?? "", }), ); }🤖 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/admin/fraud-alerts/[id]/route.ts around lines 256 - 291, Remove the redundant per-key prisma.programEnrollment.findUnique and string-splitting loop: iterate over bansForSideEffects directly (each item already had bannedReason set to PartnerBannedReason.fraud and bannedAt set in the transaction), push reportCrossProgramBanToNetwork calls built from each ban's programId, partnerId, bannedReason and bannedAt, and lookup the fraud reason using fraudReasonByEnrollmentKey.get(`${programId}:${partnerId}`) if needed; drop crossProgramSourceKeys, the key.indexOf/key.slice logic, and the null-check continue, and run the resulting postConfirmTasks in parallel (e.g. Promise.allSettled(postConfirmTasks)) to avoid N extra DB round-trips and serialize.
🤖 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/admin/fraud-alerts/[id]/route.ts:
- Around line 205-246: The loop currently awaits resolveFraudGroups per
iteration which can abort the loop on error and leave later bans without
enqueued side-effects; change behavior so resolveFraudGroups is not awaited
inline but added to postConfirmTasks alongside trackActivityLog and
banPartnerQueue.enqueueJSON (i.e., push a promise/task for resolveFraudGroups
into postConfirmTasks so all entries run in parallel/fire-and-forget), following
the same pattern used in reject-partner.ts (use waitUntil/Promise wrapper as
appropriate) and keep function identifiers resolveFraudGroups, postConfirmTasks,
trackActivityLog, and banPartnerQueue.enqueueJSON to locate and update the code.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/admin/fraud-alerts/[id]/route.ts:
- Around line 256-291: Remove the redundant per-key
prisma.programEnrollment.findUnique and string-splitting loop: iterate over
bansForSideEffects directly (each item already had bannedReason set to
PartnerBannedReason.fraud and bannedAt set in the transaction), push
reportCrossProgramBanToNetwork calls built from each ban's programId, partnerId,
bannedReason and bannedAt, and lookup the fraud reason using
fraudReasonByEnrollmentKey.get(`${programId}:${partnerId}`) if needed; drop
crossProgramSourceKeys, the key.indexOf/key.slice logic, and the null-check
continue, and run the resulting postConfirmTasks in parallel (e.g.
Promise.allSettled(postConfirmTasks)) to avoid N extra DB round-trips and
serialize.
🪄 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: 6c0fed99-d905-4e9d-95af-173db0572d99
📒 Files selected for processing (3)
apps/web/app/(ee)/api/admin/fraud-alerts/[id]/route.tsapps/web/lib/api/partners/applications/reject-partner.tsapps/web/ui/modals/reject-partner-application-modal.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/admin/fraud-alerts/[id]/route.ts (1)
247-290: ⚡ Quick winAvoid re-querying enrollments you just wrote in the transaction.
The cross-program reporting block re-fetches each enrollment via
prisma.programEnrollment.findUniquesolely to readbannedReason/bannedAt, but those values are deterministic — you just set them toPartnerBannedReason.fraudand anew Date()inside the transaction. This adds N sequential DB round-trips on the request thread beforewaitUntil, and the defensiveenrollment?.bannedReason == nullguard at line 277 only exists because of the re-read. Additionally,crossProgramSourceKeysis rebuilt frombansForSideEffects, which is already deduped viabannedRejectedKey, so theSet+key.indexOf(":")/sliceround-trip is redundant — you can iteratebansForSideEffectsdirectly.Capture
bannedAt(and the constantbannedReason) when you push tobansForSideEffects, then drive the report loop straight off it.♻️ Proposed simplification
bannedRejectedKey.add(key); + const bannedAt = new Date(); + await tx.programEnrollment.update({ where: { partnerId_programId: { partnerId, programId, }, }, data: { status: ProgramEnrollmentStatus.banned, - bannedAt: new Date(), + bannedAt, bannedReason: PartnerBannedReason.fraud, clickRewardId: null, leadRewardId: null, saleRewardId: null, discountId: null, }, }); bansForSideEffects.push({ programId, partnerId, workspaceId: enrollment.program.workspaceId, previousStatus: enrollment.status, + bannedAt, + bannedReason: PartnerBannedReason.fraud, });- const crossProgramSourceKeys = new Set( - bansForSideEffects.map((b) => `${b.programId}:${b.partnerId}`), - ); - - for (const key of crossProgramSourceKeys) { - const delimiter = key.indexOf(":"); - const programId = key.slice(0, delimiter); - const partnerId = key.slice(delimiter + 1); - - const enrollment = await prisma.programEnrollment.findUnique({ - where: { - partnerId_programId: { - partnerId, - programId, - }, - }, - select: { - bannedReason: true, - bannedAt: true, - }, - }); - - if (enrollment?.bannedReason == null || enrollment.bannedAt == null) { - continue; - } - - postConfirmTasks.push( - reportCrossProgramBanToNetwork({ - partnerId, - programId, - bannedReason: enrollment.bannedReason, - bannedAt: enrollment.bannedAt, - fraudAlertReason: fraudReasonByEnrollmentKey.get(key) ?? "", - }), - ); - } + for (const { + programId, + partnerId, + bannedReason, + bannedAt, + } of bansForSideEffects) { + postConfirmTasks.push( + reportCrossProgramBanToNetwork({ + partnerId, + programId, + bannedReason, + bannedAt, + fraudAlertReason: + fraudReasonByEnrollmentKey.get(`${programId}:${partnerId}`) ?? "", + }), + ); + }You'll also need to widen the
bansForSideEffectselement type withbannedAt: DateandbannedReason: PartnerBannedReason.🤖 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/admin/fraud-alerts/[id]/route.ts around lines 247 - 290, The loop re-queries enrollments; instead, when you push into bansForSideEffects capture and store bannedAt: Date and bannedReason: PartnerBannedReason (the constant you set earlier) and widen the bansForSideEffects element type accordingly, then replace the cross-program reporting block that builds crossProgramSourceKeys and calls prisma.programEnrollment.findUnique with a direct iteration over bansForSideEffects: extract programId/partnerId/bannedAt/bannedReason from each element and call reportCrossProgramBanToNetwork with those values and fraudAlertReason from fraudReasonByEnrollmentKey; remove the redundant Set/slicing and the enrollment?.bannedReason null-guard since bannedAt/bannedReason will now be present.
🤖 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/app/`(ee)/api/admin/fraud-alerts/[id]/route.ts:
- Around line 247-290: The loop re-queries enrollments; instead, when you push
into bansForSideEffects capture and store bannedAt: Date and bannedReason:
PartnerBannedReason (the constant you set earlier) and widen the
bansForSideEffects element type accordingly, then replace the cross-program
reporting block that builds crossProgramSourceKeys and calls
prisma.programEnrollment.findUnique with a direct iteration over
bansForSideEffects: extract programId/partnerId/bannedAt/bannedReason from each
element and call reportCrossProgramBanToNetwork with those values and
fraudAlertReason from fraudReasonByEnrollmentKey; remove the redundant
Set/slicing and the enrollment?.bannedReason null-guard since
bannedAt/bannedReason will now be present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aea1558-6a9e-42a7-97c7-18e481c5b0b3
📒 Files selected for processing (1)
apps/web/app/(ee)/api/admin/fraud-alerts/[id]/route.ts
Removed label for the fraud reason textarea.
Summary by CodeRabbit