-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Move link settings under partner groups #2774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduce PartnerGroup and PartnerGroupDefaultLink models, move link/UTM config from program to group scope, add group-aware link generation and defaults, refactor partner enroll/approve flows to create default links, add cron jobs for create/update/remap/sync, update Prisma schemas, types, APIs, UI, importers, scripts, and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Q as QStash
participant API as /api/cron/groups/create-default-links
participant DB as Prisma
participant Gen as generatePartnerLink
participant Bulk as bulkCreateLinks
participant Cache as linkCache
Note over API: verify QStash signature, parse payload (defaultLinkId, userId, cursor?)
API->>DB: fetch defaultLink & group (utmTemplate, program, workspace)
API->>DB: fetch approved enrollments (PAGE_SIZE, cursor)
loop per enrollment
API->>Gen: generatePartnerLink(workspace, program, partner, linkTemplate, userId)
Gen-->>API: processedLink
end
API->>Bulk: bulkCreateLinks(processedLinks)
Bulk-->>API: created links
API->>Cache: expireMany(createdLinkIds)
alt more enrollments
API->>Q: publish JSON to requeue with new cursor
end
API-->>Q: logAndRespond finished
sequenceDiagram
autonumber
participant UI as PATCH /api/groups/:group/default-links/:id
participant DB as Prisma
participant Q as QStash
participant Cron as /api/cron/groups/update-default-links
participant Cache as linkCache
UI->>DB: update default link (construct URL with UTMs)
DB-->>UI: updated defaultLink
UI->>Q: publish JSON -> /api/cron/groups/update-default-links { defaultLinkId }
Q->>Cron: POST with defaultLinkId
Cron->>DB: find links referencing defaultLink (cursor, PAGE_SIZE)
loop per batch
Cron->>DB: update link URLs (apply UTMs)
Cron->>Cache: expireMany(updatedLinkIds)
end
alt more links
Cron->>Q: requeue with updated cursor
end
Cron-->>UI: logAndRespond finished
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
…l when group is deleted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (5)
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (2)
124-158
: Guard partner/email; pass group UTM to new links; avoid undefined into generatePartnerLink.Prevents runtime crashes and aligns with create-default-links behavior. Similar to prior bot comments; consolidating here with a single patch.
+ const { utmTemplate } = partnerGroup; @@ - const processedLinks = ( - await Promise.allSettled( - linksToCreate.map((link) => { + const processedLinks = ( + await Promise.allSettled( + linksToCreate + .map((link) => { const programEnrollment = programEnrollments.find( (p) => p.partner.id === link.partnerId, ); const partner = programEnrollment?.partner; - return generatePartnerLink({ + // Skip if partner missing or email absent (derivePartnerLinkKey requires email) + if (!partner?.id || !partner.email) { + console.warn( + `Skipping link creation for partnerId ${link.partnerId}: partner missing or email not set`, + ); + return null; + } + + return generatePartnerLink({ workspace: { id: program.workspace.id, plan: program.workspace.plan as WorkspaceProps["plan"], }, program: { id: program.id, defaultFolderId: program.defaultFolderId, }, partner: { - id: partner?.id, - name: partner?.name, - email: partner?.email!, - tenantId: programEnrollment?.tenantId ?? undefined, + id: partner.id, + name: partner.name, + email: partner.email, + tenantId: programEnrollment?.tenantId ?? undefined, }, link: { domain: link.domain, url: link.url, tenantId: programEnrollment?.tenantId ?? undefined, partnerGroupDefaultLinkId: link.partnerGroupDefaultLinkId, + // inherit UTMs from group template if present + utm_source: utmTemplate?.utm_source, + utm_medium: utmTemplate?.utm_medium, + utm_campaign: utmTemplate?.utm_campaign, + utm_term: utmTemplate?.utm_term, + utm_content: utmTemplate?.utm_content, + ref: utmTemplate?.ref, }, userId, }); - }), + }) + .filter((p) => p !== null) as Promise<any>[], ) ) .filter(isFulfilled) .map(({ value }) => value);
173-202
: Also update destination URL when remapping default-link IDs.Currently only the mapping ID is updated; URLs remain stale. Add a map of defaultLinkId → url and include url in update. Prior bots noted this; patch below is safe and idempotent.
if (linksToUpdate.length > 0) { const groupedLinksToUpdate = linksToUpdate.reduce( (acc, link) => { acc[link.partnerGroupDefaultLinkId] = acc[link.partnerGroupDefaultLinkId] || []; acc[link.partnerGroupDefaultLinkId].push(link.id); return acc; }, {} as Record<string, string[]>, ); + const defaultLinkMap = Object.fromEntries( + partnerGroup.partnerGroupDefaultLinks.map((d) => [d.id, d.url]), + ) as Record<string, string>; + for (const [partnerGroupDefaultLinkId, linkIds] of Object.entries( groupedLinksToUpdate, )) { const updatedLinks = await prisma.link.updateMany({ where: { id: { in: linkIds, }, }, data: { partnerGroupDefaultLinkId: partnerGroupDefaultLinkId, + ...(defaultLinkMap[partnerGroupDefaultLinkId] + ? { url: defaultLinkMap[partnerGroupDefaultLinkId] } + : {}), }, }); console.log( `Updated ${updatedLinks.count} links with partnerGroupDefaultLinkId: ${partnerGroupDefaultLinkId}`, ); } }apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (3)
92-101
: Prisma: invalid findUniqueOrThrow where shape (id + projectId) — use findFirstOrThrowfindUniqueOrThrow only accepts a unique selector. Combining id with projectId will type-check poorly and can fail at runtime. Use findFirstOrThrow with both filters.
Apply this diff:
- // Find the UTM template - const utmTemplate = utmTemplateId - ? await prisma.utmTemplate.findUniqueOrThrow({ + // Find the UTM template + const utmTemplate = utmTemplateId + ? await prisma.utmTemplate.findFirstOrThrow({ where: { id: utmTemplateId, projectId: workspace.id, }, }) : null;
116-120
: Avoid passing undefined fields to Prisma updateUnconditionally passing maxPartnerLinks/utmTemplateId/linkStructure risks unintended updates or noisy diffs. Use conditional spreads like the other inputs.
Apply this diff:
- ...(additionalLinksInput && { additionalLinks: additionalLinksInput }), - maxPartnerLinks, - utmTemplateId, - linkStructure, + ...(additionalLinksInput !== undefined && { + additionalLinks: additionalLinksInput, + }), + ...(maxPartnerLinks !== undefined && { maxPartnerLinks }), + ...(utmTemplateId !== undefined && { utmTemplateId }), + ...(linkStructure !== undefined && { linkStructure }),
206-223
: Authz bypass risk: id-branch not scoped by programIdUsing findUniqueOrThrow({ id }) allows cross-program access if someone knows the id. Scope by programId in the id path and narrow the include to just partnerId.
Apply this diff:
- prisma.partnerGroup.findUniqueOrThrow({ + prisma.partnerGroup.findFirstOrThrow({ where: { ...(groupIdOrSlug.startsWith("grp_") - ? { - id: groupIdOrSlug, - } + ? { id: groupIdOrSlug, programId } : { programId_slug: { programId, slug: groupIdOrSlug, }, }), }, include: { - partners: true, + partners: { select: { partnerId: true } }, }, }),
🧹 Nitpick comments (4)
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (3)
120-123
: Reduce log noise; log counts instead of full arrays.Safer for prod logs and large batches.
- console.log("linksToUpdate", linksToUpdate); - console.log("linksToCreate", linksToCreate); - console.log("linksToRemoveMapping", linksToRemoveMapping); + console.log("Remap plan", { + toUpdate: linksToUpdate.length, + toCreate: linksToCreate.length, + toUnmap: linksToRemoveMapping.length, + });
231-231
: Fix success copy.This route remaps defaults, not “creates” defaults.
- return logAndRespond(`Finished creating default links for the partners.`); + return logAndRespond(`Finished remapping default links for the partners.`);
233-241
: Defensive error logging and accurate message.
error
may not be anError
; accessing.message
can throw. Also align wording with “remapping”.- await log({ - message: `Error creating default links for the partners: ${error.message}.`, - type: "errors", - }); + const errMsg = + error instanceof Error ? error.message : JSON.stringify(error); + await log({ + message: `Error remapping default links for the partners: ${errMsg}.`, + type: "errors", + });apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)
288-298
: Be explicit with QStash method for remap-default-linksMinor, but being explicit avoids surprises if defaults change.
Apply this diff:
qstash.publishJSON({ url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/remap-default-links`, + method: "POST", body: { programId, groupId: defaultGroup.id, partnerIds: group.partners.map(({ partnerId }) => partnerId), userId: session.user.id, isGroupDeleted: true, }, }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
(1 hunks)apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts
(9 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-default-links.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/group-default-links.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-06T07:59:03.120Z
Learnt from: devkiran
PR: dubinc/dub#2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
Applied to files:
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts
📚 Learning: 2025-07-30T15:29:54.131Z
Learnt from: TWilson023
PR: dubinc/dub#2673
File: apps/web/ui/partners/rewards/rewards-logic.tsx:268-275
Timestamp: 2025-07-30T15:29:54.131Z
Learning: In apps/web/ui/partners/rewards/rewards-logic.tsx, when setting the entity field in a reward condition, dependent fields (attribute, operator, value) should be reset rather than preserved because different entities (customer vs sale) have different available attributes. Maintaining existing fields when the entity changes would create invalid state combinations and confusing UX.
Applied to files:
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts
📚 Learning: 2025-08-26T15:38:48.173Z
Learnt from: TWilson023
PR: dubinc/dub#2736
File: apps/web/lib/api/bounties/get-bounty-or-throw.ts:53-63
Timestamp: 2025-08-26T15:38:48.173Z
Learning: In bounty-related code, getBountyOrThrow returns group objects with { id } field (transformed from BountyGroup.groupId), while other routes working directly with BountyGroup Prisma records use the actual groupId field. This is intentional - getBountyOrThrow abstracts the join table details.
Applied to files:
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts
🧬 Code graph analysis (2)
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (7)
apps/web/lib/zod/schemas/groups.ts (1)
MAX_DEFAULT_PARTNER_LINKS
(18-18)apps/web/app/(ee)/api/cron/groups/remap-default-links/utils.ts (1)
remapPartnerGroupDefaultLinks
(14-87)apps/web/lib/api/partners/generate-partner-link.ts (1)
generatePartnerLink
(27-90)apps/web/lib/api/links/bulk-create-links.ts (1)
bulkCreateLinks
(18-236)packages/utils/src/constants/main.ts (1)
APP_DOMAIN_WITH_NGROK
(20-25)apps/web/app/(ee)/api/cron/utils.ts (1)
logAndRespond
(1-13)apps/web/lib/api/errors.ts (1)
handleAndReturnErrorResponse
(175-181)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (4)
apps/web/lib/zod/schemas/groups.ts (2)
updateGroupSchema
(91-99)DEFAULT_PARTNER_GROUP
(12-16)packages/utils/src/functions/urls.ts (1)
constructURLFromUTMParams
(91-109)apps/web/lib/api/utm/extract-utm-params.ts (1)
extractUtmParams
(3-16)packages/utils/src/constants/main.ts (1)
APP_DOMAIN_WITH_NGROK
(20-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vade Review
- GitHub Check: build
🔇 Additional comments (4)
apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (2)
39-45
: Good: QStash signature verification + strict schema parse upfront.This is the right order (read raw body → verify signature → parse/validate).
220-229
: Confirmed — sync-utm expects { groupId, partnerIds } (no utmTemplateId).Schema in apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts parses { groupId, partnerIds?, startAfterProgramEnrollmentId? } and callers enqueue either { groupId } or { groupId, partnerIds } as expected.
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (2)
25-31
: GET: includeExpandedFields usage looks goodSwitch to includeExpandedFields: true aligns with the enriched payload needs for the UI. Response is still type-safe via GroupSchema.
102-107
: Confirm null semantics for additionalLinks (DbNull vs JsonNull)Code uses Prisma.DbNull when an empty array is provided. If additionalLinks is a JSON column and you intend to store JSON null (vs DB NULL), Prisma.JsonNull is the correct sentinel. Please confirm the column type and desired semantics.
Would you confirm the Prisma schema for partnerGroup.additionalLinks and whether you want DB NULL or JSON null stored when clients submit []?
// if this was invoked from the DELETE /groups/[groupId] route, the partnerGroupDefaultLinkId will be null | ||
// due to Prisma cascade SetNull on delete – therefore we should take all links and remap them instead. | ||
...(isGroupDeleted | ||
? {} | ||
: { | ||
where: { | ||
partnerGroupDefaultLinkId: { | ||
not: null, | ||
}, | ||
}, | ||
}), | ||
orderBy: { | ||
createdAt: "asc", | ||
}, | ||
take: MAX_DEFAULT_PARTNER_LINKS, // there can only be up to MAX_DEFAULT_PARTNER_LINKS default links per group | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: take limit drops links when isGroupDeleted=true.
Comment says “take all links,” but take: MAX_DEFAULT_PARTNER_LINKS
still applies and will silently skip links.
Apply:
links: {
// if this was invoked from the DELETE /groups/[groupId] route, the partnerGroupDefaultLinkId will be null
// due to Prisma cascade SetNull on delete – therefore we should take all links and remap them instead.
...(isGroupDeleted
? {}
: {
where: {
partnerGroupDefaultLinkId: {
not: null,
},
},
}),
orderBy: {
createdAt: "asc",
},
- take: MAX_DEFAULT_PARTNER_LINKS, // there can only be up to MAX_DEFAULT_PARTNER_LINKS default links per group
+ ...(isGroupDeleted
+ ? {}
+ : {
+ // cap only when restricting to already default-mapped links
+ take: MAX_DEFAULT_PARTNER_LINKS,
+ }),
},
📝 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.
// if this was invoked from the DELETE /groups/[groupId] route, the partnerGroupDefaultLinkId will be null | |
// due to Prisma cascade SetNull on delete – therefore we should take all links and remap them instead. | |
...(isGroupDeleted | |
? {} | |
: { | |
where: { | |
partnerGroupDefaultLinkId: { | |
not: null, | |
}, | |
}, | |
}), | |
orderBy: { | |
createdAt: "asc", | |
}, | |
take: MAX_DEFAULT_PARTNER_LINKS, // there can only be up to MAX_DEFAULT_PARTNER_LINKS default links per group | |
}, | |
}, | |
// if this was invoked from the DELETE /groups/[groupId] route, the partnerGroupDefaultLinkId will be null | |
// due to Prisma cascade SetNull on delete – therefore we should take all links and remap them instead. | |
...(isGroupDeleted | |
? {} | |
: { | |
where: { | |
partnerGroupDefaultLinkId: { | |
not: null, | |
}, | |
}, | |
}), | |
orderBy: { | |
createdAt: "asc", | |
}, | |
...(isGroupDeleted | |
? {} | |
: { | |
// cap only when restricting to already default-mapped links | |
take: MAX_DEFAULT_PARTNER_LINKS, | |
}), | |
}, | |
}, |
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts around lines
75 to 91, the query always applies take: MAX_DEFAULT_PARTNER_LINKS even when
isGroupDeleted is true (contradicting the comment “take all links”) which causes
links to be skipped; change the query construction so that take is only included
when isGroupDeleted is false (i.e., conditionally spread the take:
MAX_DEFAULT_PARTNER_LINKS entry alongside the where/orderBy block or omit it
entirely when isGroupDeleted is true) so that deletions will fetch all links
instead of being limited.
waitUntil( | ||
recordAuditLog({ | ||
workspaceId: workspace.id, | ||
programId, | ||
action: "group.updated", | ||
description: `Group ${updatedGroup.name} (${group.id}) updated`, | ||
actor: session.user, | ||
targets: [ | ||
{ | ||
type: "group", | ||
id: group.id, | ||
metadata: updatedGroup, | ||
}, | ||
], | ||
}), | ||
(async () => { | ||
const isTemplateAdded = group.utmTemplateId !== utmTemplateId; | ||
|
||
// If the UTM template is added, update the default links with the UTM parameters | ||
if (isTemplateAdded && utmTemplate) { | ||
const defaultLinks = await prisma.partnerGroupDefaultLink.findMany({ | ||
where: { | ||
groupId: group.id, | ||
}, | ||
}); | ||
|
||
if (defaultLinks.length > 0) { | ||
for (const defaultLink of defaultLinks) { | ||
await prisma.partnerGroupDefaultLink.update({ | ||
where: { | ||
id: defaultLink.id, | ||
}, | ||
data: { | ||
url: constructURLFromUTMParams( | ||
defaultLink.url, | ||
extractUtmParams(utmTemplate), | ||
), | ||
}, | ||
}); | ||
} | ||
} | ||
} | ||
|
||
await Promise.allSettled([ | ||
recordAuditLog({ | ||
workspaceId: workspace.id, | ||
programId, | ||
action: "group.updated", | ||
description: `Group ${updatedGroup.name} (${group.id}) updated`, | ||
actor: session.user, | ||
targets: [ | ||
{ | ||
type: "group", | ||
id: group.id, | ||
metadata: updatedGroup, | ||
}, | ||
], | ||
}), | ||
|
||
group.utmTemplateId !== utmTemplateId && | ||
qstash.publishJSON({ | ||
url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/sync-utm`, | ||
body: { | ||
groupId: group.id, | ||
}, | ||
}), | ||
]); | ||
})(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
UTM change detection false positives and sync payload gaps
- group.utmTemplateId !== utmTemplateId will be true when utmTemplateId is omitted (undefined), causing unnecessary syncs.
- qstash sync body lacks utmTemplateId and explicit method; if the cron expects it (or needs to clear), this can mis-sync.
Fix by gating on presence (utmTemplateId !== undefined), using a single utmTemplateDiff, parallelizing default-link updates, and sending method + resolved utmTemplateId.
Apply this diff:
- waitUntil(
- (async () => {
- const isTemplateAdded = group.utmTemplateId !== utmTemplateId;
+ const utmTemplatePresent = utmTemplateId !== undefined;
+ const utmTemplateDiff =
+ utmTemplatePresent && group.utmTemplateId !== utmTemplateId;
+ waitUntil(
+ (async () => {
- // If the UTM template is added, update the default links with the UTM parameters
- if (isTemplateAdded && utmTemplate) {
+ // If the UTM template changed, update the default links with the UTM parameters
+ if (utmTemplateDiff && utmTemplate) {
const defaultLinks = await prisma.partnerGroupDefaultLink.findMany({
where: {
groupId: group.id,
},
});
if (defaultLinks.length > 0) {
- for (const defaultLink of defaultLinks) {
- await prisma.partnerGroupDefaultLink.update({
- where: {
- id: defaultLink.id,
- },
- data: {
- url: constructURLFromUTMParams(
- defaultLink.url,
- extractUtmParams(utmTemplate),
- ),
- },
- });
- }
+ await Promise.all(
+ defaultLinks.map((defaultLink) =>
+ prisma.partnerGroupDefaultLink.update({
+ where: { id: defaultLink.id },
+ data: {
+ url: constructURLFromUTMParams(
+ defaultLink.url,
+ extractUtmParams(utmTemplate),
+ ),
+ },
+ }),
+ ),
+ );
}
}
await Promise.allSettled([
recordAuditLog({
workspaceId: workspace.id,
programId,
action: "group.updated",
description: `Group ${updatedGroup.name} (${group.id}) updated`,
actor: session.user,
targets: [
{
type: "group",
id: group.id,
metadata: updatedGroup,
},
],
}),
- group.utmTemplateId !== utmTemplateId &&
- qstash.publishJSON({
- url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/sync-utm`,
- body: {
- groupId: group.id,
- },
- }),
+ utmTemplateDiff &&
+ qstash.publishJSON({
+ url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/sync-utm`,
+ method: "POST",
+ body: {
+ groupId: updatedGroup.id,
+ utmTemplateId: utmTemplateId ?? updatedGroup.utmTemplateId ?? null,
+ },
+ }),
]);
})(),
);
Please confirm the schema of apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts matches the body above (expects groupId and utmTemplateId nullable). Run:
🏁 Script executed:
#!/bin/bash
rg -nC3 'api/cron/groups/sync-utm/route\.ts|z\.object\(|utmTemplateId|groupId' apps/web
Length of output: 311063
Gate UTM sync, parallelize default-link updates, and keep cron payload minimal
- Only trigger sync when utmTemplateId is provided and changed: compute utmTemplatePresent = utmTemplateId !== undefined and utmTemplateDiff = utmTemplatePresent && group.utmTemplateId !== utmTemplateId.
- Update default links in parallel: await Promise.all(defaultLinks.map(...)) instead of sequential await in the loop.
- Call qstash.publishJSON with method: "POST" and body: { groupId: updatedGroup.id } — do NOT add utmTemplateId to the payload (apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts schema expects groupId and optionally partnerIds / startAfterProgramEnrollmentId).
Files: apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts and apps/web/app/(ee)/api/cron/groups/sync-utm/route.ts.
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts around lines 129-182,
compute utmTemplatePresent = utmTemplateId !== undefined and utmTemplateDiff =
utmTemplatePresent && group.utmTemplateId !== utmTemplateId, only run the
UTM-related logic when utmTemplateDiff is true, update defaultLinks in parallel
using await Promise.all(defaultLinks.map(...)) instead of awaiting inside the
for loop, and when publishing the cron job call qstash.publishJSON include
method: "POST" and body: { groupId: updatedGroup.id } (do not include
utmTemplateId) and only invoke qstash.publishJSON when utmTemplateDiff is true
so the cron payload stays minimal and schema-compatible.
partner: { | ||
id: partner?.id, | ||
name: partner?.name, | ||
email: partner?.email!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code uses a non-null assertion (partner?.email!
) on a potentially undefined value, which will throw a runtime error if the partner is not found.
View Details
📝 Patch Details
diff --git a/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts b/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
index 66d989531..ed73b9b6b 100644
--- a/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
+++ b/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
@@ -125,37 +125,47 @@ export async function POST(req: Request) {
if (linksToCreate.length > 0) {
const processedLinks = (
await Promise.allSettled(
- linksToCreate.map((link) => {
- const programEnrollment = programEnrollments.find(
- (p) => p.partner.id === link.partnerId,
- );
-
- const partner = programEnrollment?.partner;
-
- return generatePartnerLink({
- workspace: {
- id: program.workspace.id,
- plan: program.workspace.plan as WorkspaceProps["plan"],
- },
- program: {
- id: program.id,
- defaultFolderId: program.defaultFolderId,
- },
- partner: {
- id: partner?.id,
- name: partner?.name,
- email: partner?.email!,
- tenantId: programEnrollment?.tenantId ?? undefined,
- },
- link: {
- domain: link.domain,
- url: link.url,
- tenantId: programEnrollment?.tenantId ?? undefined,
- partnerGroupDefaultLinkId: link.partnerGroupDefaultLinkId,
- },
- userId,
- });
- }),
+ linksToCreate
+ .map((link) => {
+ const programEnrollment = programEnrollments.find(
+ (p) => p.partner.id === link.partnerId,
+ );
+
+ const partner = programEnrollment?.partner;
+
+ // Skip processing if partner enrollment or partner is not found
+ if (!programEnrollment || !partner) {
+ console.warn(
+ `Skipping link creation for partnerId ${link.partnerId}: partner enrollment not found`,
+ );
+ return null;
+ }
+
+ return generatePartnerLink({
+ workspace: {
+ id: program.workspace.id,
+ plan: program.workspace.plan as WorkspaceProps["plan"],
+ },
+ program: {
+ id: program.id,
+ defaultFolderId: program.defaultFolderId,
+ },
+ partner: {
+ id: partner.id,
+ name: partner.name,
+ email: partner.email!,
+ tenantId: programEnrollment.tenantId ?? undefined,
+ },
+ link: {
+ domain: link.domain,
+ url: link.url,
+ tenantId: programEnrollment.tenantId ?? undefined,
+ partnerGroupDefaultLinkId: link.partnerGroupDefaultLinkId,
+ },
+ userId,
+ });
+ })
+ .filter((linkPromise) => linkPromise !== null),
)
)
.filter(isFulfilled)
Analysis
Non-null assertion on undefined partner email causes runtime error in remap-default-links cron
What fails: remap-default-links
cron job crashes when programEnrollments.find()
returns undefined, making partner?.email!
resolve to undefined!
which gets passed to generatePartnerLink()
How to reproduce:
- Create a link for a partner that doesn't exist in the
programEnrollments
array - The
find()
operation returns undefined partner?.email!
becomes undefined (TypeScript!
is compile-time only)generatePartnerLink()
receives undefined email and fails
Result: Runtime error in link creation process, causing the entire cron job to fail when processing partners with missing enrollments
Expected: Skip processing missing partners gracefully with warning, allowing cron job to continue
Technical details: TypeScript non-null assertion operator (!
) provides no runtime protection - it's removed in emitted JavaScript and doesn't add runtime checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/api/create-id.ts (1)
55-55
: Bug:crypto.getRandomValues
call will fail in NodeIn Node,
getRandomValues
lives onglobalThis.crypto
(Web Crypto) orcrypto.webcrypto
, not on thecrypto
module namespace. Callingcrypto.getRandomValues
risks a runtime TypeError and non‑random IDs. Use Web Crypto when available, otherwise fall back torandomFillSync
.Apply this minimal fix:
- crypto.getRandomValues(buf.subarray(6)); + if (typeof globalThis.crypto?.getRandomValues === "function") { + globalThis.crypto.getRandomValues(buf.subarray(6)); + } else { + // Node fallback + crypto.randomFillSync(buf, 6, 10); + }
♻️ Duplicate comments (8)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts (8)
73-82
: Compare against the prior URL of the target link and use its id in the job payloadKeeps change detection precise and avoids array indexing.
- if (updatedDefaultLink.url !== group.partnerGroupDefaultLinks[0].url) { + if (updatedDefaultLink.url !== targetDefaultLink.url) { waitUntil( qstash.publishJSON({ url: `${APP_DOMAIN_WITH_NGROK}/api/cron/groups/update-default-links`, body: { - defaultLinkId: group.partnerGroupDefaultLinks[0].id, + defaultLinkId: targetDefaultLink.id, }, }), ); }
1-15
: Import missing helpers and types for safer lookups and error narrowingBring in getGroupOrThrow (consistent with other group routes) and Prisma for typed error checks.
import { DubApiError } from "@/lib/api/errors"; import { getDefaultProgramIdOrThrow } from "@/lib/api/programs/get-default-program-id-or-throw"; import { parseRequestBody } from "@/lib/api/utils"; import { extractUtmParams } from "@/lib/api/utm/extract-utm-params"; import { withWorkspace } from "@/lib/auth"; import { qstash } from "@/lib/cron"; +import { getGroupOrThrow } from "@/lib/api/groups/get-group-or-throw"; import { createOrUpdateDefaultLinkSchema, PartnerGroupDefaultLinkSchema, } from "@/lib/zod/schemas/groups"; import { prisma } from "@dub/prisma"; import { APP_DOMAIN_WITH_NGROK, constructURLFromUTMParams } from "@dub/utils"; import { waitUntil } from "@vercel/functions"; import { NextResponse } from "next/server"; +import { Prisma } from "@prisma/client";
27-50
: Use getGroupOrThrow; current findUniqueOrThrow mixes unique and non-unique keysThe where clause includes programId alongside id/programId_slug, which is not a unique filter. Reuse the helper and locate the target default link explicitly.
- const group = await prisma.partnerGroup.findUniqueOrThrow({ - where: { - ...(groupIdOrSlug.startsWith("grp_") - ? { - id: groupIdOrSlug, - } - : { - programId_slug: { - programId, - slug: groupIdOrSlug, - }, - }), - programId, - }, - include: { - utmTemplate: true, - partnerGroupDefaultLinks: { - where: { - id: params.defaultLinkId, - }, - }, - }, - }); + const group = await getGroupOrThrow({ + programId, + groupId: params.groupIdOrSlug, + includeExpandedFields: true, // ensure utmTemplate and default links are included + }); + const targetDefaultLink = group.partnerGroupDefaultLinks?.find( + (l) => l.id === params.defaultLinkId, + );
51-56
: Check the resolved target default link, not list lengthLength check can be true even if the specific id doesn’t match the request. Validate targetDefaultLink instead.
- if (group.partnerGroupDefaultLinks.length === 0) { + if (!targetDefaultLink) { throw new DubApiError({ code: "bad_request", message: `Default link ${params.defaultLinkId} not found for this group.`, }); }
58-71
: Update using targetDefaultLink.id and compute finalUrl oncePrecompute finalUrl and update the specific record by id.
- const updatedDefaultLink = await prisma.partnerGroupDefaultLink.update({ + const finalUrl = group.utmTemplate + ? constructURLFromUTMParams( + url, + extractUtmParams(group.utmTemplate), + ) + : url; + const updatedDefaultLink = await prisma.partnerGroupDefaultLink.update({ where: { - id: group.partnerGroupDefaultLinks[0].id, + id: targetDefaultLink.id, }, data: { - url: group.utmTemplate - ? constructURLFromUTMParams( - url, - extractUtmParams(group.utmTemplate), - ) - : url, + url: finalUrl, }, });
121-142
: DELETE: Use getGroupOrThrow; current findUniqueOrThrow mixes keys similarlyMirror the PATCH fix and resolve the specific default link explicitly.
- const group = await prisma.partnerGroup.findUniqueOrThrow({ - where: { - ...(groupIdOrSlug.startsWith("grp_") - ? { - id: groupIdOrSlug, - } - : { - programId_slug: { - programId, - slug: groupIdOrSlug, - }, - }), - programId, - }, - include: { - partnerGroupDefaultLinks: { - where: { - id: params.defaultLinkId, - }, - }, - }, - }); + const group = await getGroupOrThrow({ + programId, + groupId: params.groupIdOrSlug, + includeExpandedFields: true, + }); + const targetDefaultLink = group.partnerGroupDefaultLinks?.find( + (l) => l.id === params.defaultLinkId, + );
144-149
: DELETE: Validate targetDefaultLink, not list lengthEnsures we error when the specific id isn’t present.
- if (group.partnerGroupDefaultLinks.length === 0) { + if (!targetDefaultLink) { throw new DubApiError({ code: "bad_request", message: `Default link ${params.defaultLinkId} not found for this group.`, }); }
151-159
: DELETE: Delete and respond using targetDefaultLink.idAvoids reliance on array index.
- await prisma.partnerGroupDefaultLink.delete({ - where: { - id: group.partnerGroupDefaultLinks[0].id, - }, - }); - - return NextResponse.json({ - id: group.partnerGroupDefaultLinks[0].id, - }); + await prisma.partnerGroupDefaultLink.delete({ + where: { id: targetDefaultLink.id }, + }); + return NextResponse.json({ id: targetDefaultLink.id });
🧹 Nitpick comments (11)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts (1)
16-112
: Optional: add audit logs for default link PATCH/DELETEFor parity with group updates/deletes elsewhere, consider recording audit logs for default link changes.
Also applies to: 114-172
apps/web/lib/api/create-id.ts (2)
60-60
: Comment drift: update descriptionComment says “optional prefix” but the function now requires it. Please update to avoid confusion.
60-66
: Optional: add a runtime guard and reusable typeEven with TS, a runtime assert helps JS/migrations and protects against
any
.Apply:
export const createId = ({ prefix }: { prefix: (typeof prefixes)[number] }) => { + if (process.env.NODE_ENV !== "production" && !prefixes.includes(prefix)) { + throw new Error(`Invalid ID prefix: ${prefix}`); + } const buf = createULIDBuffer(); const id = base32.encode(buf); return `${prefix}${id}`; };Optionally export the type for reuse:
-const prefixes = [ ... ] as const; +export const prefixes = [ ... ] as const; +export type IdPrefix = (typeof prefixes)[number];apps/web/tests/misc/create-id.test.ts (2)
5-13
: Optional: remove busy-wait to speed up testsThe tight spin wastes CPU and can flake under load. Use fake timers to advance time deterministically.
Proposed change:
-import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; ... - it("should create ids in lexicographical order", () => { + it("should create ids in lexicographical order", () => { + vi.useFakeTimers(); const ids: string[] = []; for (let i = 0; i < 10; i++) { ids.push(createId({ prefix: "link_" })); - - const now = Date.now(); - while (Date.now() - now < 10) {} // busy wait for 10ms + vi.advanceTimersByTime(10); } + vi.useRealTimers();
20-32
: Nit: assert prefix presence for clarityAdd a quick check to ensure IDs include the expected prefix.
const sortedIds = [...ids].sort(); expect(ids).toEqual(sortedIds); + expect(ids.every((id) => id.startsWith("link_"))).toBe(true);
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/add-edit-group-default-link-sheet.tsx (4)
65-67
: Avoid shadowing thelink
prop in.find
callback.Use a distinct identifier to prevent confusion.
- const existingLink = defaultLinks.find( - (link) => normalizeUrl(link.url) === normalizeUrl(data.url), - ); + const existingLink = defaultLinks.find( + (dl) => normalizeUrl(dl.url) === normalizeUrl(data.url), + );
60-63
: Don’t block submit whendefaultLinks
are still loading.Let the API enforce uniqueness; only run the client-side duplicate check if data is present.
- if (!group || !defaultLinks) return; + if (!group) return; ... - const existingLink = defaultLinks.find( + const existingLink = defaultLinks?.find( (dl) => normalizeUrl(dl.url) === normalizeUrl(data.url), );Also applies to: 65-67
81-82
: Trim before sending to API (defensive sanitation).Prevents accidental leading/trailing spaces.
- url: data.url, + url: data.url.trim(),
165-168
: Guard preview when domain/structure aren’t ready.Avoids rendering with empty domain.
- <PartnerLinkPreview - url={url} - domain={program?.domain || ""} - linkStructure={group?.linkStructure || "query"} - /> + {program?.domain && group?.linkStructure ? ( + <PartnerLinkPreview + url={url} + domain={program.domain} + linkStructure={group.linkStructure} + /> + ) : ( + <div className="h-12 rounded-md bg-neutral-100" /> + )}apps/web/lib/actions/partners/create-program.ts (2)
6-6
: Prefer aligning types to avoid downstream cast.PlanProps import is only used to cast later. Consider unifying Project["plan"] and PlanProps (or updating createAndEnrollPartner to accept Project["plan"]) to remove the "as" cast.
138-145
: Default-link create: drop redundant programId and avoid non-null assertions.Since this is a nested create under partnerGroup, let Prisma bind the relation and use the parsed domain/url to avoid
!
. If schema requires programId, keep it; otherwise prefer single source of truth.- partnerGroupDefaultLinks: { - create: { - id: createId({ prefix: "pgdl_" }), - programId, - domain: programData.domain!, - url: programData.url!, - }, - }, + partnerGroupDefaultLinks: { + create: { + id: createId({ prefix: "pgdl_" }), + domain, + url, + }, + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/app/(ee)/api/cron/groups/remap-default-links/utils.ts
(1 hunks)apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts
(1 hunks)apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/route.ts
(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/add-edit-group-default-link-sheet.tsx
(1 hunks)apps/web/lib/actions/partners/create-program.ts
(3 hunks)apps/web/lib/api/create-id.ts
(2 hunks)apps/web/tests/misc/create-id.test.ts
(1 hunks)packages/utils/src/functions/urls.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/utils/src/functions/urls.ts
- apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/route.ts
- apps/web/app/(ee)/api/cron/groups/remap-default-links/utils.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-06T07:59:03.120Z
Learnt from: devkiran
PR: dubinc/dub#2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
Applied to files:
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts
apps/web/lib/actions/partners/create-program.ts
📚 Learning: 2025-08-26T15:38:48.173Z
Learnt from: TWilson023
PR: dubinc/dub#2736
File: apps/web/lib/api/bounties/get-bounty-or-throw.ts:53-63
Timestamp: 2025-08-26T15:38:48.173Z
Learning: In bounty-related code, getBountyOrThrow returns group objects with { id } field (transformed from BountyGroup.groupId), while other routes working directly with BountyGroup Prisma records use the actual groupId field. This is intentional - getBountyOrThrow abstracts the join table details.
Applied to files:
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts
🧬 Code graph analysis (4)
apps/web/tests/misc/create-id.test.ts (1)
apps/web/lib/api/create-id.ts (1)
createId
(61-66)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/add-edit-group-default-link-sheet.tsx (11)
apps/web/lib/types.ts (1)
PartnerGroupDefaultLink
(539-541)apps/web/lib/zod/schemas/groups.ts (1)
createOrUpdateDefaultLinkSchema
(63-65)apps/web/lib/swr/use-group.ts (1)
useGroup
(7-33)apps/web/lib/swr/use-workspace.ts (1)
useWorkspace
(6-45)apps/web/lib/swr/use-partner-group-default-links.ts (1)
usePartnerGroupDefaultLinks
(7-27)apps/web/lib/swr/use-api-mutation.ts (1)
useApiMutation
(36-123)packages/utils/src/functions/urls.ts (1)
normalizeUrl
(173-180)packages/ui/src/sheet.tsx (1)
Sheet
(74-78)apps/web/ui/partners/rewards/reward-icon-square.tsx (1)
RewardIconSquare
(3-7)packages/ui/src/tooltip.tsx (2)
InfoTooltip
(193-199)SimpleTooltipContent
(130-158)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/partner-link-preview.tsx (1)
PartnerLinkPreview
(8-76)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts (7)
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (2)
PATCH
(47-198)DELETE
(201-330)apps/web/lib/auth/workspace.ts (1)
withWorkspace
(41-435)apps/web/lib/zod/schemas/groups.ts (2)
createOrUpdateDefaultLinkSchema
(63-65)PartnerGroupDefaultLinkSchema
(101-105)apps/web/lib/api/errors.ts (1)
DubApiError
(75-92)packages/utils/src/functions/urls.ts (1)
constructURLFromUTMParams
(91-109)apps/web/lib/api/utm/extract-utm-params.ts (1)
extractUtmParams
(3-16)packages/utils/src/constants/main.ts (1)
APP_DOMAIN_WITH_NGROK
(20-25)
apps/web/lib/actions/partners/create-program.ts (2)
apps/web/lib/api/create-id.ts (1)
createId
(61-66)apps/web/lib/types.ts (1)
PlanProps
(176-176)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vade Review
- GitHub Check: build
🔇 Additional comments (6)
apps/web/lib/api/create-id.ts (2)
24-24
: LGTM: new program-group default link prefix added
"pgdl_"
fits the existing naming convention and unblocks group default link IDs. No issues.
61-66
: Breaking API change:prefix
is now required — verification failedType tightening is fine, but the ripgrep check failed (rg returned "unrecognized file type: tsx" and a PCRE2 error), so I couldn't confirm call sites. Run these locally and fix any invocations that don't pass an object with
prefix
:# list all createId calls rg -n -S 'createId\s*\(' apps -C1 # calls that pass an object with `prefix:` rg -n -S "createId\s*\(\s*{\s*[^)]*prefix\s*:" apps -C1 # fallback if rg patterns fail git grep -n "createId("Update any remaining invocations to use createId({ prefix: '' }) or revert/make the param optional if callers cannot be updated.
apps/web/tests/misc/create-id.test.ts (1)
9-9
: LGTM: test updated to pass required prefixMatches the new API and keeps the order property check intact.
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/add-edit-group-default-link-sheet.tsx (1)
74-83
: Incorrect — backend accepts ID or slug; changing to slug is optional for consistency.Route param is [groupIdOrSlug] (apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/route.ts), so
/api/groups/${group.id}/default-links
should work; prefer${group.slug}
to match other client keys (useSWR/mutate).- link - ? `/api/groups/${group.id}/default-links/${link.id}` - : `/api/groups/${group.id}/default-links`, + link + ? `/api/groups/${group.slug}/default-links/${link.id}` + : `/api/groups/${group.slug}/default-links`,Likely an incorrect or invalid review comment.
apps/web/lib/actions/partners/create-program.ts (2)
253-253
: LGTM on userId propagation.Passing userId through to createAndEnrollPartner is correct.
243-247
: Don't hardcode webhookEnabled=false — preserve workspace webhooks or make suppression explicit.Hardcoding false here suppresses workspace webhooks (sendWorkspaceWebhook checks workspace.webhookEnabled). If suppression is intentional, add a comment/explicit flag; otherwise pass the workspace setting.
Location: apps/web/lib/actions/partners/create-program.ts — invitePartner
- webhookEnabled: false, + webhookEnabled: workspace.webhookEnabled,
} catch (error) { | ||
if (error.code === "P2002") { | ||
throw new DubApiError({ | ||
code: "conflict", | ||
message: "A default link with this URL already exists.", | ||
}); | ||
} | ||
|
||
throw new DubApiError({ | ||
code: "unprocessable_entity", | ||
message: error.message, | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Narrow Prisma errors safely; unknown doesn’t have .code
or .message
Use PrismaClientKnownRequestError for P2002 and fall back to generic error messages.
- } catch (error) {
- if (error.code === "P2002") {
+ } catch (error: unknown) {
+ if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") {
throw new DubApiError({
code: "conflict",
message: "A default link with this URL already exists.",
});
}
- throw new DubApiError({
+ throw new DubApiError({
code: "unprocessable_entity",
- message: error.message,
+ message: error instanceof Error ? error.message : "Unable to process the request.",
});
}
📝 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.
} catch (error) { | |
if (error.code === "P2002") { | |
throw new DubApiError({ | |
code: "conflict", | |
message: "A default link with this URL already exists.", | |
}); | |
} | |
throw new DubApiError({ | |
code: "unprocessable_entity", | |
message: error.message, | |
}); | |
} | |
} catch (error: unknown) { | |
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { | |
throw new DubApiError({ | |
code: "conflict", | |
message: "A default link with this URL already exists.", | |
}); | |
} | |
throw new DubApiError({ | |
code: "unprocessable_entity", | |
message: error instanceof Error ? error.message : "Unable to process the request.", | |
}); | |
} |
🤖 Prompt for AI Agents
In
apps/web/app/(ee)/api/groups/[groupIdOrSlug]/default-links/[defaultLinkId]/route.ts
around lines 87 to 99, the catch block assumes the caught error has a .code and
.message; change it to narrow Prisma errors safely by checking the error is a
PrismaClientKnownRequestError (e.g., import and use
PrismaClientKnownRequestError or check error.name ===
'PrismaClientKnownRequestError') before inspecting .code === 'P2002', and for
all other cases throw a DubApiError with a safe generic message (or use
String(error) / error?.message ?? 'An unexpected error occurred') so you never
access properties on unknown values.
import { useForm } from "react-hook-form"; | ||
import { toast } from "sonner"; | ||
import { mutate } from "swr"; | ||
import { z } from "zod"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add client-side Zod validation + trim input; gate submit on validity.
Prevents bad URLs/whitespace and reduces server round‑trips.
+import { zodResolver } from "@hookform/resolvers/zod";
import { useForm } from "react-hook-form";
...
- const { handleSubmit, watch, setValue } = useForm<FormData>({
+ const {
+ handleSubmit,
+ watch,
+ setValue,
+ formState: { isValid, errors },
+ } = useForm<FormData>({
+ resolver: zodResolver(createOrUpdateDefaultLinkSchema),
+ mode: "onChange",
defaultValues: {
url: link?.url || "",
},
});
...
- <div className="flex items-center gap-x-2">
- <label className="text-content-emphasis block text-sm font-medium">
+ <div className="flex items-center gap-x-2">
+ <label
+ htmlFor="destination-url"
+ className="text-content-emphasis block text-sm font-medium"
+ >
Destination URL
</label>
...
- <Input
- value={url || ""}
- onChange={(e) =>
- setValue("url", e.target.value, { shouldDirty: true })
- }
+ <Input
+ id="destination-url"
+ name="url"
+ value={url || ""}
+ onChange={(e) =>
+ setValue("url", e.target.value.trim(), {
+ shouldDirty: true,
+ shouldValidate: true,
+ })
+ }
type="url"
placeholder="https://acme.dub.sh"
className="max-w-full"
+ aria-invalid={!!errors?.url}
/>
...
- <Button
+ <Button
type="submit"
variant="primary"
text={isEditing ? "Update link" : "Create link"}
className="w-fit"
loading={isSubmitting}
- disabled={!url}
+ disabled={!isValid || isSubmitting}
/>
Also applies to: 52-56, 124-137, 139-146, 184-191
🤖 Prompt for AI Agents
In
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/links/add-edit-group-default-link-sheet.tsx
around lines 28-31 (and similarly at 52-56, 124-137, 139-146, 184-191), the form
currently lacks client-side Zod validation and does not trim inputs before
submit; update the component to define a Zod schema for the link fields (e.g.,
required, trimmed string, valid URL or allowed formats), integrate it with
react-hook-form via zodResolver, ensure inputs are trimmed onChange or before
submit, and prevent submission unless formState.isValid (or use handleSubmit
only when validation passes); update error handling to show validation errors to
the user and only call mutate/toast/server API when the validated/trimmed data
is available.
|
||
const { link, error, code } = await processLink({ | ||
payload: { | ||
key: key || undefined, | ||
url: url || program.url, | ||
url: constructURLFromUTMParams( | ||
url || partnerGroup.partnerGroupDefaultLinks[0].url, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing partnerGroupDefaultLinks[0].url
without checking if the array has elements will cause a runtime error when a group has no default links.
View Details
📝 Patch Details
diff --git a/apps/web/app/(ee)/api/embed/referrals/links/route.ts b/apps/web/app/(ee)/api/embed/referrals/links/route.ts
index 2f035638e..76862adbd 100644
--- a/apps/web/app/(ee)/api/embed/referrals/links/route.ts
+++ b/apps/web/app/(ee)/api/embed/referrals/links/route.ts
@@ -82,7 +82,7 @@ export const POST = withReferralsEmbedToken(
payload: {
key: key || undefined,
url: constructURLFromUTMParams(
- url || partnerGroup.partnerGroupDefaultLinks[0].url,
+ url || partnerGroup.partnerGroupDefaultLinks[0]?.url,
extractUtmParams(partnerGroup.utmTemplate),
),
...extractUtmParams(partnerGroup.utmTemplate, { excludeRef: true }),
diff --git a/apps/web/app/(ee)/api/partners/links/route.ts b/apps/web/app/(ee)/api/partners/links/route.ts
index 94d74eaea..2ca770c29 100644
--- a/apps/web/app/(ee)/api/partners/links/route.ts
+++ b/apps/web/app/(ee)/api/partners/links/route.ts
@@ -129,7 +129,7 @@ export const POST = withWorkspace(
domain: program.domain,
key: key || undefined,
url: constructURLFromUTMParams(
- url || partnerGroup.partnerGroupDefaultLinks[0].url,
+ url || partnerGroup.partnerGroupDefaultLinks[0]?.url,
extractUtmParams(partnerGroup.utmTemplate),
),
...extractUtmParams(partnerGroup.utmTemplate, { excludeRef: true }),
diff --git a/apps/web/app/(ee)/api/partners/links/upsert/route.ts b/apps/web/app/(ee)/api/partners/links/upsert/route.ts
index 2ccac7580..ed9c47c32 100644
--- a/apps/web/app/(ee)/api/partners/links/upsert/route.ts
+++ b/apps/web/app/(ee)/api/partners/links/upsert/route.ts
@@ -205,7 +205,7 @@ export const PUT = withWorkspace(
domain: program.domain,
key: key || undefined,
url: constructURLFromUTMParams(
- url || partnerGroup.partnerGroupDefaultLinks[0].url,
+ url || partnerGroup.partnerGroupDefaultLinks[0]?.url,
extractUtmParams(partnerGroup.utmTemplate),
),
...extractUtmParams(partnerGroup.utmTemplate, { excludeRef: true }),
Analysis
Runtime error when accessing partner group default links without array bounds checking
What fails: API routes POST /api/embed/referrals/links
, POST /api/partners/links
, and PUT /api/partners/links/upsert
crash when accessing partnerGroup.partnerGroupDefaultLinks[0].url
if a partner group has no default links configured
How to reproduce:
- Create partner group with empty
partnerGroupDefaultLinks
array - Make API call to create partner link without providing
url
parameter - Code attempts to access
partnerGroup.partnerGroupDefaultLinks[0].url
as fallback
Result: TypeError: Cannot read properties of undefined (reading 'url')
- API returns 500 error
Expected: Should handle missing default links gracefully, either by using undefined
as fallback or providing descriptive error message
Files affected:
apps/web/app/(ee)/api/embed/referrals/links/route.ts:85
apps/web/app/(ee)/api/partners/links/route.ts:132
apps/web/app/(ee)/api/partners/links/upsert/route.ts:208
partner: { | ||
id: partner?.id, | ||
name: partner?.name, | ||
email: partner?.email!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cron job uses non-null assertion on a potentially undefined value, which will cause a runtime error if no partner is found.
View Details
📝 Patch Details
diff --git a/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts b/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
index 66d989531..5ec017846 100644
--- a/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
+++ b/apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
@@ -125,37 +125,52 @@ export async function POST(req: Request) {
if (linksToCreate.length > 0) {
const processedLinks = (
await Promise.allSettled(
- linksToCreate.map((link) => {
- const programEnrollment = programEnrollments.find(
- (p) => p.partner.id === link.partnerId,
- );
-
- const partner = programEnrollment?.partner;
-
- return generatePartnerLink({
- workspace: {
- id: program.workspace.id,
- plan: program.workspace.plan as WorkspaceProps["plan"],
- },
- program: {
- id: program.id,
- defaultFolderId: program.defaultFolderId,
- },
- partner: {
- id: partner?.id,
- name: partner?.name,
- email: partner?.email!,
- tenantId: programEnrollment?.tenantId ?? undefined,
- },
- link: {
- domain: link.domain,
- url: link.url,
- tenantId: programEnrollment?.tenantId ?? undefined,
- partnerGroupDefaultLinkId: link.partnerGroupDefaultLinkId,
- },
- userId,
- });
- }),
+ linksToCreate
+ .filter((link) => {
+ const programEnrollment = programEnrollments.find(
+ (p) => p.partner.id === link.partnerId,
+ );
+
+ if (!programEnrollment?.partner) {
+ console.warn(
+ `Skipping link creation for missing partner: ${link.partnerId}`,
+ );
+ return false;
+ }
+
+ return true;
+ })
+ .map((link) => {
+ const programEnrollment = programEnrollments.find(
+ (p) => p.partner.id === link.partnerId,
+ );
+
+ const partner = programEnrollment!.partner;
+
+ return generatePartnerLink({
+ workspace: {
+ id: program.workspace.id,
+ plan: program.workspace.plan as WorkspaceProps["plan"],
+ },
+ program: {
+ id: program.id,
+ defaultFolderId: program.defaultFolderId,
+ },
+ partner: {
+ id: partner.id,
+ name: partner.name,
+ email: partner.email,
+ tenantId: programEnrollment!.tenantId ?? undefined,
+ },
+ link: {
+ domain: link.domain,
+ url: link.url,
+ tenantId: programEnrollment!.tenantId ?? undefined,
+ partnerGroupDefaultLinkId: link.partnerGroupDefaultLinkId,
+ },
+ userId,
+ });
+ }),
)
)
.filter(isFulfilled)
Analysis
Cron job crashes when partner not found in remap-default-links
What fails: POST /api/cron/groups/remap-default-links
crashes with "Cannot read properties of undefined (reading 'split')" when linksToCreate
contains a partnerId
that doesn't exist in the programEnrollments
array
How to reproduce:
// In apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts
// When programEnrollments.find() returns undefined for a partnerId:
const programEnrollment = programEnrollments.find(p => p.partner.id === "nonexistent");
const partner = programEnrollment?.partner; // undefined
// Later: partner?.email! becomes undefined, passed to generatePartnerLink()
// Which calls derivePartnerLinkKey() → email.split("@") on undefined
Result: Runtime error crashes the cron job during partner link generation
Expected: Cron jobs should be resilient to data inconsistencies and skip invalid entries with appropriate logging
Fix: Filter out links where no corresponding partner enrollment is found before attempting link generation, with warning logs for visibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web/ui/partners/add-partner-link-modal.tsx (2)
65-69
: LGTM on group fallback; no extra null-check needed.Using
partner.groupId ?? DEFAULT_PARTNER_GROUP.slug
is consistent with the groups work; treatingadditionalLinks
as[]
while loading is fine. No additional null-check required.
70-78
: Auto-select and re-sync destination domain when data loads/changes.Prevents a confusing state where no domain is selected after groups load or when the single option should be auto-picked.
Apply this diff:
const [destinationDomain, setDestinationDomain] = useState( destinationDomains?.[0] ?? null, ); + // Keep destinationDomain in sync with available domains + useEffect(() => { + if (destinationDomains.length === 1) { + // Auto-pick the only option + setDestinationDomain(destinationDomains[0]); + return; + } + // If current selection becomes invalid, fall back to first or null + if ( + destinationDomain && + !destinationDomains.includes(destinationDomain) + ) { + setDestinationDomain(destinationDomains[0] ?? null); + } + }, [destinationDomains, destinationDomain]);
🧹 Nitpick comments (11)
apps/web/lib/swr/use-partner.ts (2)
18-18
: Encode query param to preserve correctnessDirect interpolation skips encoding that
URLSearchParams
previously provided. WrapworkspaceId
withencodeURIComponent
to avoid malformed URLs if the ID ever contains special characters.- ? `/api/partners/${partnerId}?workspaceId=${workspaceId}` + ? `/api/partners/${partnerId}?workspaceId=${encodeURIComponent(workspaceId)}`
17-19
: Gate only on partnerId (workspaceId guaranteed by WorkspaceAuth)Workspace routing / WorkspaceAuth guarantees workspaceId here — drop workspaceId from the SWR key to avoid unnecessary no-key states.
File: apps/web/lib/swr/use-partner.ts — lines 17–19
- partnerId && workspaceId
- partnerId
?/api/partners/${partnerId}?workspaceId=${encodeURIComponent(workspaceId)}
: undefined,apps/web/ui/partners/design/rewards-discounts-preview.tsx (1)
12-17
: Avoid indefinite spinner on 404/error.Currently, any fetch error (e.g., no “default” group for the workspace) leaves the UI in a perpetual spinner. Use the hook’s
loading
anderror
for deterministic states.Apply:
- const { group } = useGroup({ groupIdOrSlug: DEFAULT_PARTNER_GROUP.slug }); + const { group, loading, error } = useGroup({ groupIdOrSlug: DEFAULT_PARTNER_GROUP.slug }); - if (!group) + if (loading) return ( <div className="flex h-[117px] items-center justify-center"> <LoadingSpinner /> </div> ); + if (error || !group) { + return null; // or render a lightweight empty/placeholder state + }apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/discount/group-discount.tsx (2)
97-121
: Avoid initializing the sheet with undefined defaults (mount only when data is ready).When defaultGroup is loading, the hook may initialize without defaults and not rehydrate as expected depending on useDiscountSheet internals. Mount the hook only after discount is available using a tiny wrapper.
-const CopyDefaultDiscountButton = () => { - const { group: defaultGroup } = useGroup({ - groupIdOrSlug: DEFAULT_PARTNER_GROUP.slug, - }); - - const { DiscountSheet, setIsOpen } = useDiscountSheet({ - defaultDiscountValues: defaultGroup?.discount ?? undefined, - }); - - return defaultGroup?.discount ? ( - <> - {DiscountSheet} - <Button - text="Duplicate default group" - variant="secondary" - className="animate-fade-in h-9 w-full rounded-lg md:w-fit" - onClick={(e) => { - e.preventDefault(); - e.stopPropagation(); - setIsOpen(true); - }} - /> - </> - ) : null; -}; +const CopyDefaultDiscountButton = () => { + const { group: defaultGroup } = useGroup({ + groupIdOrSlug: DEFAULT_PARTNER_GROUP.slug, + }); + const discount = defaultGroup?.discount; + return discount ? <CopyDefaultDiscountButtonInner discount={discount} /> : null; +}; + +const CopyDefaultDiscountButtonInner = ({ + discount, +}: { + discount: DiscountProps; +}) => { + const { DiscountSheet, setIsOpen } = useDiscountSheet({ + defaultDiscountValues: discount, + }); + return ( + <> + {DiscountSheet} + <Button + text="Duplicate default group" + variant="secondary" + className="animate-fade-in h-9 w-full rounded-lg md:w-fit" + onClick={(e) => { + e.preventDefault(); + e.stopPropagation(); + setIsOpen(true); + }} + /> + </> + ); +};
185-194
: Add rel="noopener noreferrer" to external link.Security best practice for target="_blank".
- <a - href="https://dub.co/help/article/dual-sided-incentives" - target="_blank" + <a + href="https://dub.co/help/article/dual-sided-incentives" + target="_blank" + rel="noopener noreferrer"apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/rewards/group-rewards.tsx (4)
213-239
: Consider hoisting the default-group fetch to avoid duplicate SWR calls.This button can render up to 3x on the page; SWR will dedupe, but hoisting once (e.g., in GroupRewards and passing defaultReward down) avoids extra hooks and simplifies testing.
If you keep it local, verify SWR key ensures perfect dedupe across identical params in useGroup.
141-147
: Avoid invalid DOM props when As is 'div'.Currently div receives href/scroll. Pass these conditionally only for Link.
- <As - href={ - reward - ? `/${slug}/program/groups/${group.slug}/rewards?rewardId=${reward.id}` - : "" - } - scroll={false} + <As + {...(reward + ? { + href: `/${slug}/program/groups/${group.slug}/rewards?rewardId=${reward.id}`, + scroll: false as const, + } + : {})}
218-223
: Prefer type-safe selection over dynamic key indexing.Avoid string interpolation to access rewards; it weakens type safety.
Example:
const selectRewardByEvent = (g: GroupProps, e: EventType) => e === "sale" ? g.saleReward : e === "lead" ? g.leadReward : g.clickReward; const defaultReward = defaultGroup ? selectRewardByEvent(defaultGroup, event) : undefined;
304-312
: Add rel="noopener noreferrer" to external link.Same rationale as the discounts page.
- <a - href="https://dub.co/help/article/partner-rewards" - target="_blank" + <a + href="https://dub.co/help/article/partner-rewards" + target="_blank" + rel="noopener noreferrer"apps/web/ui/partners/add-partner-link-modal.tsx (2)
97-101
: Avoid silent no-op submits; surface actionable errors and gate submit.Currently returns early with no feedback when
destinationDomain
/context is missing.Apply this diff:
const onSubmit = async (formData: FormData) => { - if (!destinationDomain || !program?.id || !partner.id) { - return; - } + if (!program?.id || !partner.id) { + setErrorMessage("Missing program or partner context. Please refresh."); + return; + } + if (!destinationDomain) { + setErrorMessage("Select a destination domain."); + return; + }<Button type="submit" text={ <span className="flex items-center gap-2"> Create link <div className="rounded border border-white/20 p-1"> <ArrowTurnLeft className="size-3.5" /> </div> </span> } className="h-8 w-fit pl-2.5 pr-1.5" loading={isSubmitting} - disabled={!key} + disabled={!key || (!hideDestinationUrl && !destinationDomain)} />Also applies to: 281-296
163-169
: Add accessible name to the close button.Improves a11y for screen readers.
Apply this diff:
- <button + <button type="button" onClick={() => setShowModal(false)} className="group rounded-full p-2 text-neutral-500 transition-all duration-75 hover:bg-neutral-100 focus:outline-none active:bg-neutral-200" + aria-label="Close" + title="Close" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/discount/group-discount.tsx
(1 hunks)apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/rewards/group-rewards.tsx
(1 hunks)apps/web/lib/api/utm/extract-utm-params.ts
(1 hunks)apps/web/lib/swr/use-group.ts
(1 hunks)apps/web/lib/swr/use-partner.ts
(1 hunks)apps/web/ui/partners/add-partner-link-modal.tsx
(9 hunks)apps/web/ui/partners/design/previews/embed-preview.tsx
(2 hunks)apps/web/ui/partners/design/previews/portal-preview.tsx
(2 hunks)apps/web/ui/partners/design/rewards-discounts-preview.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/ui/partners/design/previews/embed-preview.tsx
- apps/web/ui/partners/design/previews/portal-preview.tsx
- apps/web/lib/api/utm/extract-utm-params.ts
- apps/web/lib/swr/use-group.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-26T15:05:55.081Z
Learnt from: TWilson023
PR: dubinc/dub#2736
File: apps/web/lib/swr/use-bounty.ts:11-16
Timestamp: 2025-08-26T15:05:55.081Z
Learning: In the Dub codebase, workspace authentication and route structures prevent endless loading states when workspaceId or similar route parameters are missing, so gating SWR loading states on parameter availability is often unnecessary.
Applied to files:
apps/web/lib/swr/use-partner.ts
📚 Learning: 2025-06-06T07:59:03.120Z
Learnt from: devkiran
PR: dubinc/dub#2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
Applied to files:
apps/web/ui/partners/add-partner-link-modal.tsx
📚 Learning: 2025-08-14T05:57:35.546Z
Learnt from: devkiran
PR: dubinc/dub#2735
File: apps/web/lib/actions/partners/update-discount.ts:60-66
Timestamp: 2025-08-14T05:57:35.546Z
Learning: In the partner groups system, discounts should always belong to a group. The partnerGroup relation should never be null when updating discounts, so optional chaining on partnerGroup?.id may be unnecessary defensive programming.
Applied to files:
apps/web/ui/partners/add-partner-link-modal.tsx
📚 Learning: 2025-08-16T11:14:00.667Z
Learnt from: devkiran
PR: dubinc/dub#2754
File: apps/web/lib/partnerstack/schemas.ts:47-52
Timestamp: 2025-08-16T11:14:00.667Z
Learning: The PartnerStack API always includes the `group` field in partner responses, so the schema should use `.nullable()` rather than `.nullish()` since the field is never omitted/undefined.
Applied to files:
apps/web/ui/partners/add-partner-link-modal.tsx
🧬 Code graph analysis (4)
apps/web/ui/partners/design/rewards-discounts-preview.tsx (2)
apps/web/lib/swr/use-group.ts (1)
useGroup
(7-35)apps/web/lib/zod/schemas/groups.ts (1)
DEFAULT_PARTNER_GROUP
(12-16)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/discount/group-discount.tsx (1)
apps/web/lib/zod/schemas/groups.ts (1)
DEFAULT_PARTNER_GROUP
(12-16)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/rewards/group-rewards.tsx (1)
apps/web/lib/zod/schemas/groups.ts (1)
DEFAULT_PARTNER_GROUP
(12-16)
apps/web/ui/partners/add-partner-link-modal.tsx (5)
apps/web/lib/swr/use-group.ts (1)
useGroup
(7-35)apps/web/lib/zod/schemas/groups.ts (1)
DEFAULT_PARTNER_GROUP
(12-16)packages/utils/src/functions/urls.ts (2)
getPathnameFromUrl
(156-170)constructURLFromUTMParams
(91-109)apps/web/lib/api/utm/extract-utm-params.ts (1)
extractUtmParams
(3-33)packages/ui/src/combobox/index.tsx (1)
Combobox
(81-351)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vade Review
- GitHub Check: build
🔇 Additional comments (4)
apps/web/ui/partners/design/rewards-discounts-preview.tsx (1)
10-10
: Hook API migration verified — no remaining useGroup({ slug: ... }) usages found.Repository-wide search across .ts/.tsx/.js/.jsx shows only useGroup() or useGroup({ groupIdOrSlug: ... }) call sites; change approved.
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/rewards/group-rewards.tsx (1)
214-216
: Param rename to useGroup({ groupIdOrSlug }) looks correct.Aligned with the updated hook signature and default-group fetch pattern.
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/groups/[groupSlug]/discount/group-discount.tsx (1)
97-101
: Param rename to useGroup({ groupIdOrSlug }) — verified. Hook at apps/web/lib/swr/use-group.ts uses groupIdOrSlug; repo search shows callsites either pass groupIdOrSlug or call useGroup() with no args — no remaining callsites pass { slug: ... }.apps/web/ui/partners/add-partner-link-modal.tsx (1)
380-386
: Breaking prop: ensure all call sites pass partner.groupId. Found a call in apps/web/ui/partners/partner-details-sheet.tsx that passespartner
; automated verification couldn't finish—manually confirm every use ofuseAddPartnerLinkModal
suppliespartner.groupId
(or update call sites to pass{ id, email, groupId }
).
const url = linkConstructor({ | ||
domain: destinationDomain, | ||
key: getPathnameFromUrl(pathname), | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Precompute UTM params once and validate final URL before POST.
Reduces duplication and fails fast if URL construction yields an empty string.
Apply this diff:
const url = linkConstructor({
domain: destinationDomain,
key: getPathnameFromUrl(pathname),
});
+ const utmAll = extractUtmParams(partnerGroup?.utmTemplate as UtmTemplate);
+ const { ref: _ref, ...utmNoRef } = utmAll as Record<string, string | null>;
+ const finalUrl = constructURLFromUTMParams(url, utmAll);
+ if (!finalUrl) {
+ setErrorMessage("Invalid destination URL.");
+ setIsSubmitting(false);
+ return;
+ }
- url: constructURLFromUTMParams(
- url,
- extractUtmParams(partnerGroup?.utmTemplate as UtmTemplate),
- ),
- ...extractUtmParams(partnerGroup?.utmTemplate as UtmTemplate, {
- excludeRef: true,
- }),
+ url: finalUrl,
+ ...utmNoRef,
Also applies to: 121-128
🤖 Prompt for AI Agents
In apps/web/ui/partners/add-partner-link-modal.tsx around lines 105-109 (and
also apply the same change at 121-128), avoid constructing UTM params multiple
times and validate the resulting URL before making the POST: compute the
UTM/query params once into a variable, merge them into the linkConstructor
input, then check the resulting url string is non-empty and a valid URL (or at
least non-empty) and throw/log an error or return early if invalid; update both
locations to reuse the precomputed params and perform the validation prior to
the POST to fail fast if URL construction yields an empty/invalid string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts (1)
95-96
: Ensure Prisma disconnects and unhandled errors set exit code.Avoid hanging connections and surface failures in CI.
-main(); +main() + .catch((err) => { + console.error(err); + process.exitCode = 1; + }) + .finally(async () => { + try { + await prisma.$disconnect(); + } catch {} + });
♻️ Duplicate comments (4)
apps/web/lib/zod/schemas/groups.ts (4)
23-33
: Normalize domain to avoid duplicates and whitespace/case bugs.Trim + lowercase before regex; keep current validation.
Apply:
export const additionalPartnerLinkSchema = z.object({ - domain: z - .string() - .min(1, "domain is required") - .refine((v) => validDomainRegex.test(v), { message: "Invalid domain" }), + domain: z + .string() + .trim() + .toLowerCase() + .min(1, "domain is required") + .refine((v) => validDomainRegex.test(v), { message: "Invalid domain" }), validationMode: z.enum([
44-48
: Constrain maxPartnerLinks to a positive integer; keep enum as-is.Prevents floats/zero/negatives in API responses.
- maxPartnerLinks: z.number(), + maxPartnerLinks: z.number().int().min(1),
90-98
: Harden update schema: positive int for maxPartnerLinks; allow clearing UTM template.Support null to remove template; reject empty-string IDs; enforce int>0.
export const updateGroupSchema = createGroupSchema.partial().extend({ additionalLinks: z .array(additionalPartnerLinkSchema) .max(MAX_ADDITIONAL_PARTNER_LINKS) .optional(), - maxPartnerLinks: z.number().optional(), - utmTemplateId: z.string().optional(), + maxPartnerLinks: z.number().int().min(1).optional(), + utmTemplateId: z.string().min(1).nullish(), linkStructure: z.nativeEnum(PartnerLinkStructure).optional(), });If you prefer “leave unchanged when omitted, clear when null,” ensure the route distinguishes undefined vs null.
100-104
: Normalize + validate domain on PartnerGroupDefaultLinkSchema.Keep consistent with create/update behaviors.
export const PartnerGroupDefaultLinkSchema = z.object({ id: z.string(), - domain: z.string(), + domain: z + .string() + .trim() + .toLowerCase() + .refine((v) => validDomainRegex.test(v), { message: "Invalid domain" }), url: parseUrlSchema, });Optional: extract a shared domain schema to DRY this across files.
🧹 Nitpick comments (3)
apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts (3)
36-45
: Variable name is misleading.
firstPartnerLink
is not “first”; it’s the link whose URL matches the group default. ConsidermatchingPartnerLink
for clarity.
91-92
: Console output may include PII; gate verbosity.Use a verbosity flag or redact partner identifiers if this runs against production data.
36-89
: Optional: bound write concurrency to protect the DB.Large programs may trigger many concurrent updates. Consider chunking or a small concurrency pool (e.g., p-limit 10).
Example without new deps:
- const firstPartnerLinks = await Promise.all( - programEnrollments.map(async (programEnrollment) => { + const concurrency = Number(process.env.CONCURRENCY ?? 10); + const chunks = Array.from({ length: Math.ceil(programEnrollments.length / concurrency) }, (_, i) => + programEnrollments.slice(i * concurrency, (i + 1) * concurrency), + ); + const firstPartnerLinks: Array<unknown> = []; + for (const chunk of chunks) { + const results = await Promise.all( + chunk.map(async (programEnrollment) => { // ... existing per‑enrollment logic ... - }), - ); + }), + ); + firstPartnerLinks.push(...results); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/lib/zod/schemas/groups.ts
(4 hunks)apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts
(1 hunks)apps/web/scripts/migrations/backfill-group-links-pgdl.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/scripts/migrations/backfill-group-links-pgdl.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T11:14:00.667Z
Learnt from: devkiran
PR: dubinc/dub#2754
File: apps/web/lib/partnerstack/schemas.ts:47-52
Timestamp: 2025-08-16T11:14:00.667Z
Learning: The PartnerStack API always includes the `group` field in partner responses, so the schema should use `.nullable()` rather than `.nullish()` since the field is never omitted/undefined.
Applied to files:
apps/web/lib/zod/schemas/groups.ts
🧬 Code graph analysis (2)
apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts (1)
packages/utils/src/functions/urls.ts (1)
normalizeUrl
(173-180)
apps/web/lib/zod/schemas/groups.ts (3)
apps/web/lib/zod/schemas/utm.ts (1)
UTMTemplateSchema
(51-90)packages/prisma/client.ts (1)
PartnerLinkStructure
(14-14)apps/web/lib/zod/schemas/utils.ts (1)
parseUrlSchema
(4-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vade Review
- GitHub Check: build
🔇 Additional comments (5)
apps/web/lib/zod/schemas/groups.ts (4)
2-3
: LGTM on new imports.Enum and validators are correctly sourced.
9-11
: LGTM on URL/UTM schema imports.Consistent with existing utils and utm schemas.
18-22
: Confirm max constants usage and page-size intent (100).Ensure consumers enforce these limits consistently at API/DB/UI layers; 100 aligns with backend pagination defaults.
Would you like a quick grep script to list all usages of these constants across the repo?
62-65
: Confirm domain omission is intentional for default-link create/update.If domain comes from path or group context, this is fine. If not, add domain with normalization/validation for parity.
I can scan the API routes to verify the path params and payload shape if helpful.
apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts (1)
36-45
: Confirm URL comparison semantics (UTMs dropped).
normalizeUrl
strips query strings. If default links differ only by UTMs, they’ll be considered equal. Verify that this is intended for remapping defaults. See packages/utils/src/functions/urls.ts Lines 172–179.
// special script for checking if acme default links are set up properly | ||
async function main() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This “check” script mutates data; add a DRY‑RUN/--write safety switch and precompute normalized URL.
Current code updates records unconditionally. Add an explicit write gate and avoid repeated normalization.
-import "dotenv-flow/config";
+import "dotenv-flow/config";
+const WRITE = process.env.WRITE === "1" || process.argv.includes("--write");
- const firstPartnerLink = links.find(
- (link) => normalizeUrl(link.url) === normalizeUrl(defaultLink.url),
- );
+ const normalizedDefaultUrl = normalizeUrl(defaultLink.url);
+ const firstPartnerLink = links.find(
+ (link) => normalizeUrl(link.url) === normalizedDefaultUrl,
+ );
Also applies to: 36-45
const program = await prisma.program.findUniqueOrThrow({ | ||
where: { | ||
slug: "acme", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Don’t hard‑code the program slug; parameterize via CLI/ENV.
Hard‑coding "acme" risks accidental runs against the wrong tenant and makes reuse harder.
- const program = await prisma.program.findUniqueOrThrow({
- where: {
- slug: "acme",
- },
+ const slug = process.env.PROGRAM_SLUG ?? process.argv[2] ?? "acme";
+ const program = await prisma.program.findUniqueOrThrow({
+ where: {
+ slug,
+ },
📝 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.
const program = await prisma.program.findUniqueOrThrow({ | |
where: { | |
slug: "acme", | |
}, | |
const slug = process.env.PROGRAM_SLUG ?? process.argv[2] ?? "acme"; | |
const program = await prisma.program.findUniqueOrThrow({ | |
where: { | |
slug, | |
}, |
🤖 Prompt for AI Agents
In apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts around lines 7
to 10, the program slug is hard-coded as "acme"; change this to read the slug
from a CLI argument or environment variable (e.g., process.env.PROGRAM_SLUG or
parse process.argv) and validate it is present before running; replace the
literal "acme" in the prisma.findUniqueOrThrow where clause with the variable,
and add a clear error/exit if the variable is missing or empty to prevent
accidental runs against the wrong tenant.
groups: { | ||
include: { | ||
partnerGroupDefaultLinks: true, | ||
}, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard empty default‑links and make selection deterministic.
If a group has zero partnerGroupDefaultLinks, group.partnerGroupDefaultLinks[0]
is undefined → downstream crashes; also ordering is unspecified.
- groups: {
- include: {
- partnerGroupDefaultLinks: true,
- },
- },
+ groups: {
+ include: {
+ partnerGroupDefaultLinks: {
+ orderBy: { createdAt: "asc" }, // or by `id` if preferred
+ },
+ },
+ },
- for (const group of program.groups) {
- const defaultLink = group.partnerGroupDefaultLinks[0];
+ for (const group of program.groups) {
+ if (group.partnerGroupDefaultLinks.length === 0) {
+ console.warn(`Group ${group.id} has no partnerGroupDefaultLinks; skipping.`);
+ continue;
+ }
+ const defaultLink = group.partnerGroupDefaultLinks[0];
Also applies to: 21-22
🤖 Prompt for AI Agents
In apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts around lines
12–17 (and also apply same fix to lines 21–22), the migration assumes
group.partnerGroupDefaultLinks[0] exists and relies on unspecified array order;
update the Prisma query to request partnerGroupDefaultLinks ordered
deterministically (e.g., orderBy a stable column such as id or createdAt) and
add a guard so you check for length > 0 before accessing [0] (skip the group or
handle missing default link appropriately). Ensure both occurrences use the
ordered include and runtime guard to avoid undefined access and
non-deterministic selection.
const foundDefaultLink = links.find( | ||
(link) => link.partnerGroupDefaultLinkId, | ||
); | ||
const firstPartnerLink = links.find( | ||
(link) => normalizeUrl(link.url) === normalizeUrl(defaultLink.url), | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix matching logic; handle the two mismatches independently.
Requiring both ids to equal the group default via .every
misses cases and can’t correct them (e.g., you only update one link). Compute and act on each mismatch separately.
- const {
- groupDefaultLinkId,
- partnerDefaultLinkId,
- firstPartnerLinkDefaultLinkId,
- } = {
- groupDefaultLinkId: defaultLink.id,
- partnerDefaultLinkId: foundDefaultLink?.partnerGroupDefaultLinkId,
- firstPartnerLinkDefaultLinkId:
- firstPartnerLink?.partnerGroupDefaultLinkId,
- };
-
- const matchingDefaultLink = [
- partnerDefaultLinkId,
- firstPartnerLinkDefaultLinkId,
- ].every((id) => id === groupDefaultLinkId);
+ const groupDefaultLinkId = defaultLink.id;
+ const partnerDefaultLinkId = foundDefaultLink?.partnerGroupDefaultLinkId ?? null;
+ const firstPartnerLinkDefaultLinkId = firstPartnerLink?.partnerGroupDefaultLinkId ?? null;
+
+ const partnerDefaultMismatch =
+ partnerDefaultLinkId !== null && partnerDefaultLinkId !== groupDefaultLinkId;
+ const firstLinkMismatch =
+ !!firstPartnerLink && firstPartnerLinkDefaultLinkId !== groupDefaultLinkId;
+ const matchingDefaultLink = !partnerDefaultMismatch && !firstLinkMismatch;
Also applies to: 47-61
if (!matchingDefaultLink) { | ||
await prisma.link.update({ | ||
where: { | ||
id: firstPartnerLink?.id, | ||
}, | ||
data: { | ||
partnerGroupDefaultLinkId: groupDefaultLinkId, | ||
}, | ||
}); | ||
|
||
console.log( | ||
`Updated link ${firstPartnerLink?.id} to have default link ${groupDefaultLinkId}`, | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible runtime error: updating with an undefined ID; also update both offending links.
where: { id: firstPartnerLink?.id }
may pass undefined
. Guard it, and fix both mismatches (the pre‑existing “default” link and the matching URL link). Respect DRY‑RUN.
- if (!matchingDefaultLink) {
- await prisma.link.update({
- where: {
- id: firstPartnerLink?.id,
- },
- data: {
- partnerGroupDefaultLinkId: groupDefaultLinkId,
- },
- });
-
- console.log(
- `Updated link ${firstPartnerLink?.id} to have default link ${groupDefaultLinkId}`,
- );
- }
+ if (!matchingDefaultLink) {
+ if (!WRITE) {
+ console.log(
+ `[DRY-RUN] Would update:`,
+ {
+ updateFoundDefaultLinkId: foundDefaultLink?.id,
+ updateFirstPartnerLinkId: firstPartnerLink?.id,
+ toDefaultLinkId: groupDefaultLinkId,
+ },
+ );
+ } else {
+ const ops: Promise<unknown>[] = [];
+ if (partnerDefaultMismatch && foundDefaultLink) {
+ ops.push(
+ prisma.link.update({
+ where: { id: foundDefaultLink.id },
+ data: { partnerGroupDefaultLinkId: groupDefaultLinkId },
+ }),
+ );
+ }
+ if (firstLinkMismatch && firstPartnerLink) {
+ ops.push(
+ prisma.link.update({
+ where: { id: firstPartnerLink.id },
+ data: { partnerGroupDefaultLinkId: groupDefaultLinkId },
+ }),
+ );
+ }
+ if (ops.length) {
+ await Promise.all(ops);
+ console.log(
+ `Updated partnerEnrollment ${programEnrollment.id}:`,
+ {
+ updatedFoundDefaultLinkId: foundDefaultLink?.id,
+ updatedFirstPartnerLinkId: firstPartnerLink?.id,
+ toDefaultLinkId: groupDefaultLinkId,
+ },
+ );
+ }
+ }
+ }
📝 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.
if (!matchingDefaultLink) { | |
await prisma.link.update({ | |
where: { | |
id: firstPartnerLink?.id, | |
}, | |
data: { | |
partnerGroupDefaultLinkId: groupDefaultLinkId, | |
}, | |
}); | |
console.log( | |
`Updated link ${firstPartnerLink?.id} to have default link ${groupDefaultLinkId}`, | |
); | |
} | |
if (!matchingDefaultLink) { | |
if (!WRITE) { | |
console.log( | |
`[DRY-RUN] Would update:`, | |
{ | |
updateFoundDefaultLinkId: foundDefaultLink?.id, | |
updateFirstPartnerLinkId: firstPartnerLink?.id, | |
toDefaultLinkId: groupDefaultLinkId, | |
}, | |
); | |
} else { | |
const ops: Promise<unknown>[] = []; | |
if (partnerDefaultMismatch && foundDefaultLink) { | |
ops.push( | |
prisma.link.update({ | |
where: { id: foundDefaultLink.id }, | |
data: { partnerGroupDefaultLinkId: groupDefaultLinkId }, | |
}), | |
); | |
} | |
if (firstLinkMismatch && firstPartnerLink) { | |
ops.push( | |
prisma.link.update({ | |
where: { id: firstPartnerLink.id }, | |
data: { partnerGroupDefaultLinkId: groupDefaultLinkId }, | |
}), | |
); | |
} | |
if (ops.length) { | |
await Promise.all(ops); | |
console.log( | |
`Updated partnerEnrollment ${programEnrollment.id}:`, | |
{ | |
updatedFoundDefaultLinkId: foundDefaultLink?.id, | |
updatedFirstPartnerLinkId: firstPartnerLink?.id, | |
toDefaultLinkId: groupDefaultLinkId, | |
}, | |
); | |
} | |
} | |
} |
🤖 Prompt for AI Agents
In apps/web/scripts/migrations/backfill-group-links-pgdl-acme.ts around lines 62
to 75, the migration may call prisma.link.update with an undefined id and only
updates one link; change the logic to first guard that firstPartnerLink?.id is
defined before calling prisma.update (skip or log when undefined), then perform
updates for both offending links — update the pre-existing groupDefaultLink
(clear or correct its partnerGroupDefaultLinkId) and the matching URL link (set
partnerGroupDefaultLinkId to groupDefaultLinkId) so both mismatches are fixed;
also ensure these updates are no-ops when running in DRY_RUN mode (log intended
changes instead of performing DB writes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/scripts/migrations/backfill-group-links-pgdl.ts (1)
94-95
: Add error handling and ensure Prisma disconnects.Prevents hanging connections in CI and surfaces failures cleanly.
-main(); +main() + .catch((err) => { + console.error(err); + process.exitCode = 1; + }) + .finally(async () => { + await prisma.$disconnect(); + });
♻️ Duplicate comments (5)
apps/web/scripts/migrations/backfill-group-links-pgdl.ts (2)
29-47
: Remove the 500-enrollment cap or batch; current cap will leave data partially migrated.This
take: 500
truncates work for large groups, producing inconsistent defaults.Apply this minimal fix (load all at once):
}, - take: 500, });
If memory is a concern, I can provide a cursor-based batching version.
27-27
: Pick the correct default link (match by program URL + domain) instead of taking the first.Using index 0 can assign the wrong default when multiple PGDLs exist per group.
- const defaultLink = group.partnerGroupDefaultLinks[0]; + const normalizedProgramUrl = normalizeUrl(program.url!); + const defaultLink = + group.partnerGroupDefaultLinks.find( + (l) => + normalizeUrl(l.url) === normalizedProgramUrl && + l.domain === program.domain, + ) ?? group.partnerGroupDefaultLinks[0];apps/web/scripts/partners/get-largest-programs.ts (3)
5-16
: Restrict counts to active enrollments (avoid inflated results).Filter out non-active/soft-deleted enrollments at query time. Prisma groupBy supports pre-group filtering via where. (prisma.io)
const programsByEnrollmentCount = await prisma.programEnrollment.groupBy({ - by: ["programId"], + by: ["programId"], + where: { + status: { + in: [ProgramEnrollmentStatus.approved, ProgramEnrollmentStatus.invited], + }, + }, _count: { programId: true, },To confirm enum names, run:
#!/bin/bash # Show ProgramEnrollmentStatus enum values fd -HI -t f 'schema.prisma' | xargs rg -nU "enum\\s+ProgramEnrollmentStatus\\b|\\bProgramEnrollmentStatus\\b" -n -C2
1-2
: Load env before instantiating Prisma (import order bug).dotenv must be loaded before importing the Prisma client to ensure DATABASE_URL, etc., are set.
-import { prisma } from "@dub/prisma"; -import "dotenv-flow/config"; +import "dotenv-flow/config"; +import { prisma } from "@dub/prisma"; +import { ProgramEnrollmentStatus } from "@dub/prisma/client";
4-5
: Handle errors and always disconnect Prisma.Prevents noisy unhandled rejections and dangling connections.
-async function main() { - const programsByEnrollmentCount = await prisma.programEnrollment.groupBy({ +async function main() { + try { + const programsByEnrollmentCount = await prisma.programEnrollment.groupBy({ @@ - console.table(programs); -} - -main(); + console.table(programs); + } catch (err) { + console.error("Failed to fetch largest programs:", err); + process.exitCode = 1; + } finally { + await prisma.$disconnect(); + } +} + +void main();Also applies to: 47-50
🧹 Nitpick comments (7)
apps/web/scripts/migrations/backfill-group-links-pgdl.ts (4)
49-53
: Fix log typo.Minor readability nit.
- console.log( - `No program enrollments needfound for group ${group.id}. Skipping...`, - ); + console.log( + `No program enrollments needed for group ${group.id}. Skipping...`, + );
76-85
: Skip no-op updates when there’s nothing to update.Avoids unnecessary DB calls and cleaner logs.
- const res = await prisma.link.updateMany({ + const idsToUpdate = firstPartnerLinkIds.filter( + (id): id is string => id !== null, + ); + if (idsToUpdate.length === 0) { + console.log(`No links to update for group ${group.id}. Skipping...`); + continue; + } + const res = await prisma.link.updateMany({ where: { id: { - in: firstPartnerLinkIds.filter((id): id is string => id !== null), + in: idsToUpdate, }, }, data: { partnerGroupDefaultLinkId: defaultLink.id, }, });
33-37
: Use a “none” relation filter for clarity and potential planner wins.Semantically “no link has a default set yet”; equivalent to “every is null” but often clearer.
- links: { - every: { - partnerGroupDefaultLinkId: null, - }, - }, + links: { + none: { + partnerGroupDefaultLinkId: { not: null }, + }, + },
74-74
: Guard verbose table output behind DEBUG.Prevents noisy logs on large datasets.
- console.table(firstPartnerLinkIds); + if (process.env.DEBUG) console.table(firstPartnerLinkIds);apps/web/scripts/partners/get-largest-programs.ts (3)
24-33
: Fetch only needed fields and skip the query when empty.Avoid unnecessary DB work and over-fetching.
- const programs = await prisma.program - .findMany({ - where: { - id: { - in: filteredProgramsByEnrollmentCount.map( - (program) => program.programId, - ), - }, - }, - }) + if (filteredProgramsByEnrollmentCount.length === 0) { + console.table([]); + return; + } + + const programs = await prisma.program + .findMany({ + where: { + id: { + in: filteredProgramsByEnrollmentCount.map((p) => p.programId), + }, + }, + select: { id: true, slug: true }, + })
34-45
: Avoid O(n²) lookups when enriching results.Use a Map for O(1) joins.
- .then((programs) => - programs - .map((program) => ({ - id: program.id, - slug: program.slug, - enrollmentCount: - filteredProgramsByEnrollmentCount.find( - (p) => p.programId === program.id, - )?._count.programId ?? 0, - })) - .sort((a, b) => b.enrollmentCount - a.enrollmentCount), - ); + .then((programs) => { + const countMap = new Map( + filteredProgramsByEnrollmentCount.map((p) => [p.programId, p._count.programId]), + ); + return programs + .map((program) => ({ + id: program.id, + slug: program.slug, + enrollmentCount: countMap.get(program.id) ?? 0, + })) + .sort((a, b) => b.enrollmentCount - a.enrollmentCount); + });
15-20
: Make LIMIT and MIN_COUNT configurable — don't silently drop valid programsFile: apps/web/scripts/partners/get-largest-programs.ts (lines 15–20)
Reason: take: 100 can drop matching programs when >100 meet the threshold; expose MIN_COUNT and LIMIT via env vars and apply them to take and the filter.
-import "dotenv-flow/config"; +import "dotenv-flow/config"; +const MIN_COUNT = Number(process.env.MIN_COUNT ?? "1000"); +const LIMIT = Number(process.env.LIMIT ?? "100"); // previous default async function main() { const programsByEnrollmentCount = await prisma.programEnrollment.groupBy({ @@ - take: 100, + take: LIMIT, }); @@ - const filteredProgramsByEnrollmentCount = programsByEnrollmentCount.filter( - (program) => program._count.programId > 1000, + const filteredProgramsByEnrollmentCount = programsByEnrollmentCount.filter( + (program) => program._count.programId > MIN_COUNT, );Optionally bump the default LIMIT after you inspect typical volumes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/scripts/migrations/backfill-group-links-pgdl.ts
(1 hunks)apps/web/scripts/partners/get-largest-programs.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-06T07:59:03.120Z
Learnt from: devkiran
PR: dubinc/dub#2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
Applied to files:
apps/web/scripts/migrations/backfill-group-links-pgdl.ts
🧬 Code graph analysis (1)
apps/web/scripts/migrations/backfill-group-links-pgdl.ts (1)
packages/utils/src/functions/urls.ts (1)
normalizeUrl
(173-180)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Vade Review
- GitHub Check: build
include: { | ||
links: { | ||
orderBy: { | ||
createdAt: "asc", | ||
}, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Reduce payload and repeated work (select only needed fields; precompute normalized default URL).
Speeds up the script and avoids repeated normalization in the loop.
include: {
links: {
+ select: { id: true, url: true, createdAt: true },
orderBy: {
createdAt: "asc",
},
},
},
- const firstPartnerLinkIds = programEnrollments.map(
+ const normalizedDefaultUrl = normalizeUrl(defaultLink.url);
+ const firstPartnerLinkIds = programEnrollments.map(
(programEnrollment) => {
const { links } = programEnrollment;
const firstPartnerLink = links.find(
- (link) => normalizeUrl(link.url) === normalizeUrl(defaultLink.url),
+ (link) => normalizeUrl(link.url) === normalizedDefaultUrl,
);
Also applies to: 56-61
🤖 Prompt for AI Agents
In apps/web/scripts/migrations/backfill-group-links-pgdl.ts around lines 39-45
(and similarly 56-61), the query is pulling full link objects and the loop
repeatedly calls URL normalization; change the includes to select only the
fields needed (e.g., id, url, any id/key required) to reduce payload and
processing, and compute the normalized default URL once before entering the loop
(store it in a variable) and reuse it instead of normalizing inside each
iteration.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores