Improve merge-partners flow#3584
Merged
steven-tey merged 5 commits intomainfrom Mar 14, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughSwitch partner deletions to PlanetScale raw SQL and delete related programEnrollment rows first. In partner-merge cron, reconcile overlapping enrollments by deleting source enrollments and propagating tenantId to target enrollments when present; expand selected partner fields (image, tenantId); remove rewind/upsert logic; update logging, cache clearing, storage cleanup, and notifications. Changes
Sequence DiagramsequenceDiagram
participant Cron as Cron Job
participant DB as Database (PlanetScale)
participant Reconcile as Reconciliation Logic
participant Storage as Storage Service
participant Mailer as Batch Email Service
Cron->>DB: Fetch source & target partners + enrollments (include program.tenantId, image)
DB-->>Cron: Return partners and enrollments
Cron->>Reconcile: Identify overlapping program enrollments
Reconcile-->>Cron: Return duplicate enrollment pairs
loop for each duplicate
Cron->>DB: DELETE source programEnrollment (raw SQL)
DB-->>Cron: Deletion confirmed
alt target missing tenantId && source has tenantId
Cron->>DB: UPDATE target enrollment set tenantId = source.tenantId
DB-->>Cron: Update confirmed
end
end
Cron->>DB: DELETE source partner (raw SQL)
DB-->>Cron: Partner deleted
Cron->>Storage: Remove source partner image/files
Storage-->>Cron: Cleanup done
Cron->>Mailer: Send merge notifications
Mailer-->>Cron: Emails dispatched
Cron->>Cron: Log completion
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Collaborator
Author
|
@coderabbitai full review, any breaking changes? |
Contributor
|
✅ Actions performedFull review triggered. |
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/admin/delete-partner-account/route.ts:
- Around line 120-134: The raw SQL call conn.execute(`DELETE FROM Partner WHERE
id = ?`, [partner.id]) bypasses Prisma's relationMode="prisma" cascade rules and
will orphan many records; replace that direct SQL delete with a Prisma delete
call such as prisma.partner.delete({ where: { id: partner.id } }) so Prisma
enforces cascade behavior for models like Commission, PartnerRewind, Payout,
PartnerReferral and cleans up the cascade=true models (PartnerInvite,
PartnerUser, Message, Postback, etc.); locate the deletion logic that currently
references conn.execute and swap it to use prisma.partner.delete with the same
partner.id (and keep the existing programEnrollment deletes if still required).
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 266-279: The current update unconditionally overwrites tenantId on
the target enrollment using tx.programEnrollment.update when enrollment.tenantId
is present; change this to first fetch the target enrollment (e.g., via
tx.programEnrollment.findUnique with partnerId_programId for targetPartnerId and
enrollment.programId), check if a tenantId already exists, and only update if it
is empty or matches the source; if it differs, either log the conflict (using
the same logger used in this route) and skip the update or surface an error to
abort the merge per team policy—ensure the code references enrollment.tenantId,
targetPartnerId, and tx.programEnrollment.update/findUnique so the conflict
handling is explicit and logged.
- Around line 335-346: The raw SQL removal of the Partner (the DELETE executed
near the block using conn.execute and sourcePartnerId) will orphan several
related records; before calling that delete, remove or cascade the following
related records for the partner: PartnerReferral, Payout, Link (rows where
partnerId = sourcePartnerId), DiscountCode, Commission, and NotificationEmail
(rows where partnerId = sourcePartnerId). Update either the Prisma schema to add
onDelete: Cascade for these relations or add explicit cleanup queries (using the
same DB connection/transaction as the existing ProgramEnrollment and
PartnerRewind cleanup) to delete from PartnerReferral, Payout, Link,
DiscountCode, Commission, and NotificationEmail keyed by sourcePartnerId prior
to executing the DELETE FROM Partner; keep the image cleanup (storage.delete
using sourceAccount.image and R2_URL) as-is after these deletions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb8237b4-e013-4ca0-913b-2856180909ac
📒 Files selected for processing (2)
apps/web/app/(ee)/api/admin/delete-partner-account/route.tsapps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts
Contributor
There was a problem hiding this comment.
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/cron/partners/merge-accounts/route.ts (1)
347-350:⚠️ Potential issue | 🟠 MajorDo not continue success flow when source partner deletion fails.
This
catchonly logs. The route can still clear cache, send “merged” emails, and return success while the source partner remains undeleted.Suggested fail-fast change
} catch (error) { console.error( `Error deleting partner ${sourcePartnerId}: ${error.message}`, ); + throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 347 - 350, The catch block that currently only logs deletion failures for sourcePartnerId must fail-fast: when deleting the source partner (the try/catch around the deletion of sourcePartnerId) throws, stop the merge flow instead of proceeding to clear caches, send merged emails, or return success. Modify the catch to either rethrow the error or return an error HTTP response (using the same route handler response flow) so downstream calls like clearPartnerCache (or whatever cache-clearing function is used), sendMergedEmails, and the final success response are not executed when deletion fails.
♻️ Duplicate comments (3)
apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts (2)
337-340:⚠️ Potential issue | 🔴 CriticalRaw SQL delete keeps the same partner-orphan risk in merge flow.
Using
conn.execute("DELETE FROM Partner ...")bypasses Prisma relation cleanup guarantees; this merge flow updates many relations, but not all partner-linked records are explicitly transferred/removed here.#!/bin/bash set -euo pipefail echo "== Prisma relation mode ==" rg -n 'relationMode\s*=' packages/prisma/schema || true echo echo "== Partner-related relation fields ==" rg -n '@relation\([^)]*Partner|fields:\s*\[partnerId\]' packages/prisma/schema || true echo echo "== Transfer/cleanup operations in merge route ==" rg -n 'updateMany|deleteMany|DELETE FROM Partner|partner\.delete' "apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts" || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 337 - 340, The raw SQL call conn.execute(`DELETE FROM Partner ...`) risks leaving orphaned relations because it bypasses Prisma's relation handling; replace this with Prisma-level cleanup: transfer or delete all partner-linked records using the existing updateMany/deleteMany operations (run inside the same transaction) for models referencing partnerId, then call prisma.partner.delete({ where: { id: sourcePartnerId } }) (or include it in prisma.$transaction) instead of conn.execute; ensure you reference sourcePartnerId and sourceAccount when performing the final delete so logs and transactional context remain correct.
266-279:⚠️ Potential issue | 🟠 MajorGuard against silent tenantId overwrite on existing target enrollments.
This unconditionally sets target
tenantIdwhen source has one. If target already has a different tenant mapping, this can silently break tenant-based lookups.Suggested conflict-safe update
// update target enrollment if (enrollment.tenantId) { + const targetEnrollment = targetPartnerEnrollments.find( + ({ programId }) => programId === enrollment.programId, + ); + + if ( + targetEnrollment?.tenantId && + targetEnrollment.tenantId !== enrollment.tenantId + ) { + console.warn( + `TenantId conflict for program ${enrollment.programId}: target=${targetEnrollment.tenantId}, source=${enrollment.tenantId}. Skipping overwrite.`, + ); + return; + } + await tx.programEnrollment.update({ where: { partnerId_programId: { partnerId: targetPartnerId, programId: enrollment.programId, }, }, data: { tenantId: enrollment.tenantId, }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 266 - 279, The current block unconditionally writes enrollment.tenantId into the target via tx.programEnrollment.update which can overwrite a different existing tenant mapping; change it to first fetch the target enrollment (e.g., via tx.programEnrollment.findUnique using partnerId_programId with partnerId: targetPartnerId and programId: enrollment.programId) and then: if no target exists, create it; if target.tenantId is null, perform the update; if target.tenantId equals enrollment.tenantId do nothing; otherwise handle the conflict (throw an error, log and skip, or surface it to the caller) instead of blindly updating. Use the fetched record to decide whether to call tx.programEnrollment.update for the tenantId write to avoid silent overwrites.apps/web/app/(ee)/api/admin/delete-partner-account/route.ts (1)
133-134:⚠️ Potential issue | 🔴 CriticalRaw SQL partner deletion can bypass Prisma-managed relational cleanup.
conn.execute("DELETE FROM Partner ...")skips Prisma-level relation handling, which can leave partner-linked rows orphaned unless every relation is manually handled first.Suggested change
- await conn.execute(`DELETE FROM Partner WHERE id = ?`, [partner.id]); + await prisma.partner.delete({ + where: { + id: partner.id, + }, + });#!/bin/bash set -euo pipefail echo "== Prisma relation mode ==" rg -n 'relationMode\s*=' packages/prisma/schema || true echo echo "== Models with relation fields using partnerId ==" rg -n '@relation\([^)]*fields:\s*\[partnerId\]' packages/prisma/schema || true echo echo "== Current cleanup in this route ==" rg -n 'deleteMany|DELETE FROM Partner|partner\.delete' "apps/web/app/(ee)/api/admin/delete-partner-account/route.ts" || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/admin/delete-partner-account/route.ts around lines 133 - 134, The raw SQL call conn.execute("DELETE FROM Partner ...") bypasses Prisma's relation handling and can orphan related rows; replace that raw DELETE with the Prisma client so relation cleanup runs (e.g., use prisma.partner.delete({ where: { id: partner.id } }) or wrap related deletes and the partner deletion in a prisma.$transaction and/or use deleteMany for dependent models before calling prisma.partner.delete), and remove the conn.execute(...) line and its console.log; target the DELETE FROM Partner usage and references to conn.execute and partner.id in this route handler to implement the Prisma-based deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 347-350: The catch block that currently only logs deletion
failures for sourcePartnerId must fail-fast: when deleting the source partner
(the try/catch around the deletion of sourcePartnerId) throws, stop the merge
flow instead of proceeding to clear caches, send merged emails, or return
success. Modify the catch to either rethrow the error or return an error HTTP
response (using the same route handler response flow) so downstream calls like
clearPartnerCache (or whatever cache-clearing function is used),
sendMergedEmails, and the final success response are not executed when deletion
fails.
---
Duplicate comments:
In `@apps/web/app/`(ee)/api/admin/delete-partner-account/route.ts:
- Around line 133-134: The raw SQL call conn.execute("DELETE FROM Partner ...")
bypasses Prisma's relation handling and can orphan related rows; replace that
raw DELETE with the Prisma client so relation cleanup runs (e.g., use
prisma.partner.delete({ where: { id: partner.id } }) or wrap related deletes and
the partner deletion in a prisma.$transaction and/or use deleteMany for
dependent models before calling prisma.partner.delete), and remove the
conn.execute(...) line and its console.log; target the DELETE FROM Partner usage
and references to conn.execute and partner.id in this route handler to implement
the Prisma-based deletion.
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 337-340: The raw SQL call conn.execute(`DELETE FROM Partner ...`)
risks leaving orphaned relations because it bypasses Prisma's relation handling;
replace this with Prisma-level cleanup: transfer or delete all partner-linked
records using the existing updateMany/deleteMany operations (run inside the same
transaction) for models referencing partnerId, then call prisma.partner.delete({
where: { id: sourcePartnerId } }) (or include it in prisma.$transaction) instead
of conn.execute; ensure you reference sourcePartnerId and sourceAccount when
performing the final delete so logs and transactional context remain correct.
- Around line 266-279: The current block unconditionally writes
enrollment.tenantId into the target via tx.programEnrollment.update which can
overwrite a different existing tenant mapping; change it to first fetch the
target enrollment (e.g., via tx.programEnrollment.findUnique using
partnerId_programId with partnerId: targetPartnerId and programId:
enrollment.programId) and then: if no target exists, create it; if
target.tenantId is null, perform the update; if target.tenantId equals
enrollment.tenantId do nothing; otherwise handle the conflict (throw an error,
log and skip, or surface it to the caller) instead of blindly updating. Use the
fetched record to decide whether to call tx.programEnrollment.update for the
tenantId write to avoid silent overwrites.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b893a2f0-de6a-4b15-b1c7-391cf787ad34
📒 Files selected for processing (2)
apps/web/app/(ee)/api/admin/delete-partner-account/route.tsapps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts
Contributor
There was a problem hiding this comment.
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/cron/partners/merge-accounts/route.ts (1)
338-354:⚠️ Potential issue | 🔴 CriticalRaw SQL deletion will fail due to missing cascade rules on related records.
The raw SQL
DELETEat line 340 will fail if there are anyDiscountCodeorPartnerReferralrecords referencing the source partner, as these relations have noonDelete: Cascadedefined in the Prisma schema. The database will enforce a foreign key constraint violation since they default to RESTRICT.While other Partner-related models like
PartnerUser,FraudEvent,FraudEventGroup, andMessagehave explicitonDelete: Cascaderules and will be properly deleted, the code currently does not handle:
DiscountCode(no onDelete clause)PartnerReferral(no onDelete clause)Before the raw SQL
DELETE, explicitly clean up these records via Prisma or add bulk delete operations to handle them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 338 - 354, The raw SQL DELETE at conn.execute(`DELETE FROM Partner WHERE id = ?`, [sourcePartnerId]) will fail due to FK constraints from DiscountCode and PartnerReferral; before that delete (and before deleting sourceAccount.image), add explicit cleanup using Prisma's client (e.g., prisma.discountCode.deleteMany({ where: { partnerId: sourcePartnerId } }) and prisma.partnerReferral.deleteMany({ where: { partnerId: sourcePartnerId } })) or equivalent bulk deletes to remove those dependent rows, then proceed with the existing conn.execute deletion and storage.delete; keep the same error handling around the deletion block and use sourcePartnerId and sourceAccount.id/email to locate/log the operations.
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts (1)
258-283: Non-atomic operations between Prisma and raw SQL create crash-consistency window.The
connobject is an HTTP-based client that does NOT participate in any shared transaction with the preceding Prisma operations. If a failure occurs after the Prisma transaction (lines 258-283) but before the raw SQL delete (line 340), the source partner would remain with some data already transferred to the target.This follows the same pattern as the admin route which performs
programEnrollment.deleteMany()before the raw SQL partner delete, so it's consistent with existing codebase patterns. However, consider adding a comment documenting this intentional non-atomicity for future maintainers.Also applies to: 340-340
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 258 - 283, The Prisma transaction block (prisma.$transaction) performing tx.programEnrollment.delete and tx.programEnrollment.update is not part of the external HTTP-based conn operation that later issues the raw SQL partner delete, creating a known crash-consistency window; add a concise inline comment above the prisma.$transaction (or directly before the raw SQL partner delete) explaining that conn is an HTTP client that does not participate in the DB transaction, that the delete/update are intentionally non-atomic with the downstream raw SQL partner deletion, and note the rationale (or refer to existing admin route behavior) and potential alternatives (e.g., performing deletes inside a single DB transaction or using DB-side cascading) so future maintainers understand the tradeoff.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 338-354: The raw SQL DELETE at conn.execute(`DELETE FROM Partner
WHERE id = ?`, [sourcePartnerId]) will fail due to FK constraints from
DiscountCode and PartnerReferral; before that delete (and before deleting
sourceAccount.image), add explicit cleanup using Prisma's client (e.g.,
prisma.discountCode.deleteMany({ where: { partnerId: sourcePartnerId } }) and
prisma.partnerReferral.deleteMany({ where: { partnerId: sourcePartnerId } })) or
equivalent bulk deletes to remove those dependent rows, then proceed with the
existing conn.execute deletion and storage.delete; keep the same error handling
around the deletion block and use sourcePartnerId and sourceAccount.id/email to
locate/log the operations.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 258-283: The Prisma transaction block (prisma.$transaction)
performing tx.programEnrollment.delete and tx.programEnrollment.update is not
part of the external HTTP-based conn operation that later issues the raw SQL
partner delete, creating a known crash-consistency window; add a concise inline
comment above the prisma.$transaction (or directly before the raw SQL partner
delete) explaining that conn is an HTTP client that does not participate in the
DB transaction, that the delete/update are intentionally non-atomic with the
downstream raw SQL partner deletion, and note the rationale (or refer to
existing admin route behavior) and potential alternatives (e.g., performing
deletes inside a single DB transaction or using DB-side cascading) so future
maintainers understand the tradeoff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae347779-5964-4218-b5e3-36a38c571e10
📒 Files selected for processing (1)
apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts
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/cron/partners/merge-accounts/route.ts (1)
340-356:⚠️ Potential issue | 🔴 CriticalDo not report success when the raw partner delete fails.
This
catchonly logs, so the job can still resolve fraud, send merge emails, and return 200 with the source partner still present. Let the SQL delete failure escape to the outercatch; keep image cleanup best-effort in its owntry/catch.🧯 Proposed split between required delete and best-effort image cleanup
- try { - // Finally, delete the partner account - await conn.execute(`DELETE FROM Partner WHERE id = ?`, [sourcePartnerId]); - console.log( - `Deleted partner ${sourceAccount.email} (${sourceAccount.id})`, - ); - - if (sourceAccount.image) { - await storage.delete({ - key: sourceAccount.image.replace(`${R2_URL}/`, ""), - }); - } - } catch (error) { - console.error( - `Error deleting partner ${sourcePartnerId}: ${error.message}`, - ); - } + // Finally, delete the partner account + await conn.execute(`DELETE FROM Partner WHERE id = ?`, [sourcePartnerId]); + console.log( + `Deleted partner ${sourceAccount.email} (${sourceAccount.id})`, + ); + + if (sourceAccount.image) { + try { + await storage.delete({ + key: sourceAccount.image.replace(`${R2_URL}/`, ""), + }); + } catch (error) { + console.error( + `Error deleting partner image ${sourcePartnerId}: ${error.message}`, + ); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 340 - 356, The delete block currently swallows SQL errors and still logs success; change it so the required DB delete (await conn.execute(`DELETE FROM Partner WHERE id = ?`, [sourcePartnerId])) is not wrapped in a catch that suppresses errors—allow that call to throw so the outer catch can handle failure (and remove the console.log success message from that failing path). Move the image cleanup (storage.delete with key built from sourceAccount.image.replace(`${R2_URL}/`, "")) into its own try/catch as a best-effort operation so R2 failures are logged but do not mask SQL errors; keep references to conn.execute, sourcePartnerId, sourceAccount, storage.delete, and R2_URL when applying the change.
♻️ Duplicate comments (1)
apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts (1)
170-180:⚠️ Potential issue | 🔴 CriticalHandle
PartnerReferralbefore the raw partner delete.This block now transfers most Partner-owned rows, but I still don't see any
partnerReferralcleanup anywhere in the route. When Line 342 runsDELETE FROM Partnervia raw SQL, those rows will keepsourcePartnerIdand become orphaned unless you explicitly move or delete them here.#!/bin/bash set -euo pipefail # Expectation: # 1) Prisma schema shows PartnerReferral -> Partner. # 2) This merge route has no explicit partnerReferral update/delete before the raw SQL delete. echo "=== PartnerReferral relation definition ===" rg -n -C3 'model PartnerReferral|@relation\(.*Partner|partnerId' packages/prisma/schema/ echo "=== Merge route handling ===" rg -n -C2 'partnerReferral' apps/web/app/\(ee\)/api/cron/partners/merge-accounts/route.tsBased on learnings,
PartnerReferralis one of the non-cascadePartnerrelations that must be handled explicitly before deleting the partner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts around lines 170 - 180, The merge logic updates many related tables but omits handling PartnerReferral, so before the raw partner deletion (the prisma.$executeRaw DELETE FROM Partner call) you must explicitly handle PartnerReferral rows: either transfer their sourcePartnerId to the surviving partner or remove them. Add a step using prisma.partnerReferral.updateMany (to set sourcePartnerId to the new partner id) or prisma.partnerReferral.deleteMany (if you intend to drop them) as part of the Promise.all block that updates related records, ensuring this runs prior to the raw partner deletion to avoid orphaned PartnerReferral rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 255-269: After resolving sourceAccount and targetAccount, add a
guard that rejects/throws when their partner IDs are identical to prevent a
self-merge; specifically check that sourcePartnerId !== targetPartnerId (the
variables used around existingEnrollments, sourcePartnerId, targetPartnerId) and
return an error or abort the operation before the overlap cleanup loop that
deletes programEnrollment records runs. Ensure this check runs immediately after
the account lookup and before any mutation (including the loop over
existingEnrollments and prisma.$transaction) so no deletions occur for the same
partner.
---
Outside diff comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 340-356: The delete block currently swallows SQL errors and still
logs success; change it so the required DB delete (await conn.execute(`DELETE
FROM Partner WHERE id = ?`, [sourcePartnerId])) is not wrapped in a catch that
suppresses errors—allow that call to throw so the outer catch can handle failure
(and remove the console.log success message from that failing path). Move the
image cleanup (storage.delete with key built from
sourceAccount.image.replace(`${R2_URL}/`, "")) into its own try/catch as a
best-effort operation so R2 failures are logged but do not mask SQL errors; keep
references to conn.execute, sourcePartnerId, sourceAccount, storage.delete, and
R2_URL when applying the change.
---
Duplicate comments:
In `@apps/web/app/`(ee)/api/cron/partners/merge-accounts/route.ts:
- Around line 170-180: The merge logic updates many related tables but omits
handling PartnerReferral, so before the raw partner deletion (the
prisma.$executeRaw DELETE FROM Partner call) you must explicitly handle
PartnerReferral rows: either transfer their sourcePartnerId to the surviving
partner or remove them. Add a step using prisma.partnerReferral.updateMany (to
set sourcePartnerId to the new partner id) or prisma.partnerReferral.deleteMany
(if you intend to drop them) as part of the Promise.all block that updates
related records, ensuring this runs prior to the raw partner deletion to avoid
orphaned PartnerReferral rows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 688a7679-4e26-4939-bcd5-42d40d61e7bb
📒 Files selected for processing (1)
apps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/`(ee)/api/admin/delete-partner-account/route.ts:
- Around line 120-134: The cleanup is non-atomic:
prisma.programEnrollment.deleteMany and conn.execute(`DELETE FROM Partner WHERE
id = ?`) run separately so a failure can leave inconsistent state; fix by
performing both deletes inside a single transactional boundary—use Prisma
transactions rather than mixing raw conn calls. Replace the raw conn.execute
partner deletion with a Prisma partner delete and run both operations under
prisma.$transaction (e.g. use prisma.$transaction or tx in a transaction
callback to call tx.programEnrollment.deleteMany(...) and tx.partner.delete({
where: { id: partner.id } }) so both succeed or roll back together).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c84ce65b-addd-46e3-992c-9754baf6631e
📒 Files selected for processing (2)
apps/web/app/(ee)/api/admin/delete-partner-account/route.tsapps/web/app/(ee)/api/cron/partners/merge-accounts/route.ts
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
1 participant
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