Fix race conditions in partner create-and-enroll upsert - #4237
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe partner enrollment flow now uses dedicated helpers to resolve partners and create or retrieve program enrollments, with existing-enrollment handling and downstream operations based on the returned enrollment data. ChangesPartner enrollment flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant createAndEnrollPartner
participant getOrCreatePartner
participant createOrGetProgramEnrollment
participant Prisma
createAndEnrollPartner->>getOrCreatePartner: resolve partner by email or create partner
getOrCreatePartner->>Prisma: find or create Partner
createAndEnrollPartner->>createOrGetProgramEnrollment: create or retrieve program enrollment
createOrGetProgramEnrollment->>Prisma: create or find ProgramEnrollment
createOrGetProgramEnrollment-->>createAndEnrollPartner: return enrollment and created flag
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 5
🤖 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/lib/api/partners/create-and-enroll-partner.ts`:
- Around line 140-158: In the partner enrollment flow, change the concurrent
Promise.all around getGroupOrThrow and getOrCreatePartner so getGroupOrThrow
completes first and validates finalGroupId before any partner mutation begins.
Only invoke getOrCreatePartner after the group has been successfully retrieved,
while preserving the existing partner creation data and subsequent group usage.
- Around line 147-157: Extend getOrCreatePartner’s existing-partner conflict
path to update the submitted profile fields instead of only returning
findUniqueOrThrow’s stale record. Reuse the normalized values from the create
payload, including name, country, description, and image, so existing partners
are refreshed consistently with newly created ones.
In `@apps/web/lib/api/partners/create-or-get-program-enrollment.ts`:
- Around line 94-99: Update the existing-enrollment return branch in
createOrGetProgramEnrollment to compare programEnrollment.tenantId with the
requested tenantId before returning. Preserve the successful return for matching
tenants, but raise the same conflict or reconciliation error used by the
non-race path when they differ.
In `@apps/web/lib/api/partners/get-or-create-partner.ts`:
- Around line 27-38: Validate the result of generatePartnerUsername in the
get-or-create flow before calling prisma.partner.create. When it returns null
after exhausting retries, throw a clear, descriptive error and do not invoke
create; preserve the existing creation path for valid usernames.
- Around line 44-67: Update the P2002 handling in getOrCreatePartner to inspect
error.meta?.target and distinguish username conflicts from email conflicts. For
username collisions, retry the create flow with a newly generated username; only
use findUniqueOrThrow({ where: { email } }) when the unique constraint targets
email, preserving the existing return shape and error propagation for other
conflicts.
🪄 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 Plus
Run ID: 117da1db-96c2-470c-b2cd-acdcbeea4c19
📒 Files selected for processing (3)
apps/web/lib/api/partners/create-and-enroll-partner.tsapps/web/lib/api/partners/create-or-get-program-enrollment.tsapps/web/lib/api/partners/get-or-create-partner.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/api/partners/create-or-get-program-enrollment.ts`:
- Around line 102-104: The conflict message in the enrollment error thrown by
create-or-get program enrollment incorrectly attributes the conflict to the
requested tenant. Update it to state that the same partner is already associated
with a different tenant in the program, using grammatically correct wording;
leave the conflict code and surrounding logic unchanged.
🪄 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 Plus
Run ID: 1dd3b848-fcd9-4653-95a4-8b8e43f5cf16
📒 Files selected for processing (3)
apps/web/lib/api/partners/create-and-enroll-partner.tsapps/web/lib/api/partners/create-or-get-program-enrollment.tsapps/web/lib/api/partners/get-or-create-partner.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/lib/api/partners/get-or-create-partner.ts
- apps/web/lib/api/partners/create-and-enroll-partner.ts
| throw new DubApiError({ | ||
| message: `The tenantId '${tenantId}' is already in associated with another partner in this program.`, | ||
| code: "conflict", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the conflict message.
This lookup proves the same partner is already enrolled with a different tenant; it does not prove the requested tenant belongs to another partner. The current text is also grammatically malformed.
Proposed fix
- message: `The tenantId '${tenantId}' is already in associated with another partner in this program.`,
+ message: "This partner is already enrolled in this program with a different tenant ID.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new DubApiError({ | |
| message: `The tenantId '${tenantId}' is already in associated with another partner in this program.`, | |
| code: "conflict", | |
| throw new DubApiError({ | |
| message: "This partner is already enrolled in this program with a different tenant ID.", | |
| code: "conflict", |
🤖 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/api/partners/create-or-get-program-enrollment.ts` around lines
102 - 104, The conflict message in the enrollment error thrown by create-or-get
program enrollment incorrectly attributes the conflict to the requested tenant.
Update it to state that the same partner is already associated with a different
tenant in the program, using grammatically correct wording; leave the conflict
code and surrounding logic unchanged.
Summary by CodeRabbit