Improve aggregate clicks cron #2725
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce batch processing for click commission aggregation, enhance analytics and events endpoints to support multiple link IDs with folder-based permission checks, and update the analytics query schema to accept a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (Analytics/Events)
participant DB
participant Permissions
Client->>API (Analytics/Events): GET request with linkIds
API (Analytics/Events)->>DB: Fetch links by linkIds
API (Analytics/Events)->>Permissions: Check folder access for each link
Permissions-->>API (Analytics/Events): Allowed links/folders
API (Analytics/Events)->>DB: Query events/analytics for allowed links/folders
DB-->>API (Analytics/Events): Return results
API (Analytics/Events)-->>Client: Respond with data
sequenceDiagram
participant CronJob
participant AggregateRoute
participant DB
participant SyncService
CronJob->>AggregateRoute: Trigger aggregation
AggregateRoute->>DB: Fetch all links for program
AggregateRoute->>DB: Fetch click analytics in bulk
AggregateRoute->>DB: Bulk insert commissions
AggregateRoute->>SyncService: Sync total commissions per partner (async)
SyncService-->>AggregateRoute: (async) Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/web/app/(ee)/api/cron/aggregate-clicks/route.ts (1)
102-113: Add safety check for link ID mapping.While the non-null assertion should be safe given the data flow, consider adding a safety check to handle unexpected analytics results gracefully.
Apply this diff for safer mapping:
const commissions = clickStats.map( ({ id: linkId, clicks: quantity }) => { + const partnerId = linkToPartnerMap.get(linkId); + if (!partnerId) { + console.warn(`Link ${linkId} not found in linkToPartnerMap`); + return null; + } + return { id: createId({ prefix: "cm_" }), programId, - partnerId: linkToPartnerMap.get(linkId)!, + partnerId, linkId, quantity, type: CommissionType.click, amount: 0, earnings: reward.amount * quantity, - }), - ); + }; + }).filter(Boolean) as NonNullable<typeof commissions[0]>[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(ee)/api/cron/aggregate-clicks/route.ts(3 hunks)apps/web/app/(ee)/api/events/route.ts(4 hunks)apps/web/app/api/analytics/route.ts(5 hunks)apps/web/lib/zod/schemas/analytics.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-16T19:21:23.506Z
Learnt from: TWilson023
PR: dubinc/dub#2519
File: apps/web/ui/analytics/utils.ts:35-37
Timestamp: 2025-06-16T19:21:23.506Z
Learning: In the `useAnalyticsFilterOption` function in `apps/web/ui/analytics/utils.ts`, the pattern `options?.context ?? useContext(AnalyticsContext)` is intentionally designed as a complete replacement strategy, not a merge. When `options.context` is provided, it should contain all required fields (`baseApiPath`, `queryString`, `selectedTab`, `requiresUpgrade`) and completely replace the React context, not be merged with it. This is used for dependency injection or testing scenarios.
Applied to files:
apps/web/lib/zod/schemas/analytics.tsapps/web/app/api/analytics/route.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/lib/zod/schemas/analytics.tsapps/web/app/(ee)/api/events/route.tsapps/web/app/api/analytics/route.tsapps/web/app/(ee)/api/cron/aggregate-clicks/route.ts
📚 Learning: 2025-07-17T06:41:45.620Z
Learnt from: devkiran
PR: dubinc/dub#2637
File: apps/web/app/(ee)/api/singular/webhook/route.ts:0-0
Timestamp: 2025-07-17T06:41:45.620Z
Learning: In the Singular integration (apps/web/app/(ee)/api/singular/webhook/route.ts), the event names in the singularToDubEvent object have intentionally different casing: "Copy GAID" and "copy IDFA". This casing difference is valid and should not be changed, as these are the correct event names expected from Singular.
Applied to files:
apps/web/app/(ee)/api/events/route.ts
📚 Learning: 2025-05-29T04:45:18.504Z
Learnt from: devkiran
PR: dubinc/dub#2448
File: packages/email/src/templates/partner-program-summary.tsx:0-0
Timestamp: 2025-05-29T04:45:18.504Z
Learning: In the PartnerProgramSummary email template (packages/email/src/templates/partner-program-summary.tsx), the stat titles are hardcoded constants ("Clicks", "Leads", "Sales", "Earnings") that will always match the ICONS object keys after toLowerCase() conversion, so icon lookup failures are not possible.
Applied to files:
apps/web/app/(ee)/api/cron/aggregate-clicks/route.ts
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
apps/web/lib/zod/schemas/analytics.ts (1)
90-94: LGTM! Well-structured schema addition.The
linkIdsfield follows the established pattern used fortagIdsand correctly handles both single string and array inputs with appropriate transformation.
| }), | ||
| ); | ||
|
|
||
| console.log(commissions); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or sanitize commission data logging.
Logging full commission objects may expose sensitive financial data. Consider removing this log or logging only non-sensitive summary information.
Apply this diff to log summary information instead:
- console.log(commissions);
+ console.log(`Creating ${commissions.length} commissions for program ${programId}`);📝 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.
Suggested change
| console.log(commissions); | |
| console.log(`Creating ${commissions.length} commissions for program ${programId}`); |
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/cron/aggregate-clicks/route.ts at line 115, the current
console.log outputs full commission objects, which may expose sensitive
financial data. Replace this log with a summary that excludes sensitive details,
such as logging only the count of commissions or aggregated totals, or remove
the log entirely to prevent sensitive data exposure.
Comment on lines
+59
to
+68
| if (linkIds && linkIds.length) { | ||
| links = await prisma.link.findMany({ | ||
| where: { | ||
| id: { | ||
| in: linkIds, | ||
| }, | ||
| programId: workspace.id, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Fix incorrect field name in database query.
The query uses programId but should use workspaceId to match the workspace's links.
Apply this diff to fix the field name:
links = await prisma.link.findMany({
where: {
id: {
in: linkIds,
},
- programId: workspace.id,
+ workspaceId: workspace.id,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (linkIds && linkIds.length) { | |
| links = await prisma.link.findMany({ | |
| where: { | |
| id: { | |
| in: linkIds, | |
| }, | |
| programId: workspace.id, | |
| }, | |
| }); | |
| if (linkIds && linkIds.length) { | |
| links = await prisma.link.findMany({ | |
| where: { | |
| id: { | |
| in: linkIds, | |
| }, | |
| workspaceId: workspace.id, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/events/route.ts between lines 59 and 68, the Prisma
query incorrectly uses the field name 'programId' when filtering links by
workspace. Change the field name from 'programId' to 'workspaceId' in the where
clause to correctly filter links associated with the workspace.
Comment on lines
+69
to
+100
| if (checkIfLinksHaveFolders(links)) { | ||
| const linkFolderIds = Array.from( | ||
| new Set( | ||
| links.map((link) => link.folderId).filter(Boolean) as string[], | ||
| ), | ||
| ); | ||
|
|
||
| const folderPermissions = await checkFolderPermissions({ | ||
| workspaceId: workspace.id, | ||
| userId: session.user.id, | ||
| folderIds: linkFolderIds, | ||
| requiredPermission: "folders.read", | ||
| }); | ||
|
|
||
| links = links.filter((link) => { | ||
| if (!link.folderId) { | ||
| return true; | ||
| } | ||
|
|
||
| const validFolder = folderPermissions.find( | ||
| (folder) => folder.folderId === link.folderId, | ||
| ); | ||
|
|
||
| if (!validFolder?.hasPermission) { | ||
| return false; | ||
| } | ||
|
|
||
| folderIds?.push(link.folderId); | ||
|
|
||
| return true; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Initialize folderIds array before usage.
The folderIds array is undefined when trying to push folder IDs, causing the collection to fail silently.
Apply this diff to initialize the array:
if (checkIfLinksHaveFolders(links)) {
+ folderIds = [];
const linkFolderIds = Array.from(
new Set(
links.map((link) => link.folderId).filter(Boolean) as string[],
),
);📝 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.
Suggested change
| if (checkIfLinksHaveFolders(links)) { | |
| const linkFolderIds = Array.from( | |
| new Set( | |
| links.map((link) => link.folderId).filter(Boolean) as string[], | |
| ), | |
| ); | |
| const folderPermissions = await checkFolderPermissions({ | |
| workspaceId: workspace.id, | |
| userId: session.user.id, | |
| folderIds: linkFolderIds, | |
| requiredPermission: "folders.read", | |
| }); | |
| links = links.filter((link) => { | |
| if (!link.folderId) { | |
| return true; | |
| } | |
| const validFolder = folderPermissions.find( | |
| (folder) => folder.folderId === link.folderId, | |
| ); | |
| if (!validFolder?.hasPermission) { | |
| return false; | |
| } | |
| folderIds?.push(link.folderId); | |
| return true; | |
| }); | |
| } | |
| if (checkIfLinksHaveFolders(links)) { | |
| // Initialize the folderIds array before collecting folder IDs | |
| folderIds = []; | |
| const linkFolderIds = Array.from( | |
| new Set( | |
| links.map((link) => link.folderId).filter(Boolean) as string[], | |
| ), | |
| ); | |
| const folderPermissions = await checkFolderPermissions({ | |
| workspaceId: workspace.id, | |
| userId: session.user.id, | |
| folderIds: linkFolderIds, | |
| requiredPermission: "folders.read", | |
| }); | |
| links = links.filter((link) => { | |
| if (!link.folderId) { | |
| return true; | |
| } | |
| const validFolder = folderPermissions.find( | |
| (folder) => folder.folderId === link.folderId, | |
| ); | |
| if (!validFolder?.hasPermission) { | |
| return false; | |
| } | |
| folderIds?.push(link.folderId); | |
| return true; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/events/route.ts around lines 69 to 100, the folderIds
array is used without initialization, causing errors when pushing folder IDs.
Initialize folderIds as an empty array before the if block that checks for links
with folders to ensure it is defined before pushing values into it.
Comment on lines
+90
to
+98
| if (linkIds && linkIds.length) { | ||
| links = await prisma.link.findMany({ | ||
| where: { | ||
| id: { | ||
| in: linkIds, | ||
| }, | ||
| programId: workspace.id, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix incorrect field name in database query.
The query uses programId but should use workspaceId to match the workspace's links.
Apply this diff to fix the field name:
links = await prisma.link.findMany({
where: {
id: {
in: linkIds,
},
- programId: workspace.id,
+ workspaceId: workspace.id,
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (linkIds && linkIds.length) { | |
| links = await prisma.link.findMany({ | |
| where: { | |
| id: { | |
| in: linkIds, | |
| }, | |
| programId: workspace.id, | |
| }, | |
| }); | |
| if (linkIds && linkIds.length) { | |
| links = await prisma.link.findMany({ | |
| where: { | |
| id: { | |
| in: linkIds, | |
| }, | |
| workspaceId: workspace.id, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/app/api/analytics/route.ts between lines 90 and 98, the database
query incorrectly uses the field name programId when filtering links. Change the
field name from programId to workspaceId in the where clause to correctly match
links associated with the workspace.
Comment on lines
+100
to
+131
| if (checkIfLinksHaveFolders(links)) { | ||
| const linkFolderIds = Array.from( | ||
| new Set( | ||
| links.map((link) => link.folderId).filter(Boolean) as string[], | ||
| ), | ||
| ); | ||
|
|
||
| const folderPermissions = await checkFolderPermissions({ | ||
| workspaceId: workspace.id, | ||
| userId: session.user.id, | ||
| folderIds: linkFolderIds, | ||
| requiredPermission: "folders.read", | ||
| }); | ||
|
|
||
| links = links.filter((link) => { | ||
| if (!link.folderId) { | ||
| return true; | ||
| } | ||
|
|
||
| const validFolder = folderPermissions.find( | ||
| (folder) => folder.folderId === link.folderId, | ||
| ); | ||
|
|
||
| if (!validFolder?.hasPermission) { | ||
| return false; | ||
| } | ||
|
|
||
| folderIds?.push(link.folderId); | ||
|
|
||
| return true; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Initialize folderIds array before usage.
The folderIds array is undefined when trying to push folder IDs, causing the collection to fail silently.
Apply this diff to initialize the array:
if (checkIfLinksHaveFolders(links)) {
+ folderIds = [];
const linkFolderIds = Array.from(
new Set(
links.map((link) => link.folderId).filter(Boolean) as string[],
),
);📝 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.
Suggested change
| if (checkIfLinksHaveFolders(links)) { | |
| const linkFolderIds = Array.from( | |
| new Set( | |
| links.map((link) => link.folderId).filter(Boolean) as string[], | |
| ), | |
| ); | |
| const folderPermissions = await checkFolderPermissions({ | |
| workspaceId: workspace.id, | |
| userId: session.user.id, | |
| folderIds: linkFolderIds, | |
| requiredPermission: "folders.read", | |
| }); | |
| links = links.filter((link) => { | |
| if (!link.folderId) { | |
| return true; | |
| } | |
| const validFolder = folderPermissions.find( | |
| (folder) => folder.folderId === link.folderId, | |
| ); | |
| if (!validFolder?.hasPermission) { | |
| return false; | |
| } | |
| folderIds?.push(link.folderId); | |
| return true; | |
| }); | |
| } | |
| if (checkIfLinksHaveFolders(links)) { | |
| folderIds = []; | |
| const linkFolderIds = Array.from( | |
| new Set( | |
| links.map((link) => link.folderId).filter(Boolean) as string[], | |
| ), | |
| ); | |
| const folderPermissions = await checkFolderPermissions({ | |
| workspaceId: workspace.id, | |
| userId: session.user.id, | |
| folderIds: linkFolderIds, | |
| requiredPermission: "folders.read", | |
| }); | |
| links = links.filter((link) => { | |
| if (!link.folderId) { | |
| return true; | |
| } | |
| const validFolder = folderPermissions.find( | |
| (folder) => folder.folderId === link.folderId, | |
| ); | |
| if (!validFolder?.hasPermission) { | |
| return false; | |
| } | |
| folderIds?.push(link.folderId); | |
| return true; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/analytics/route.ts around lines 100 to 131, the folderIds
array is used without initialization, causing errors when pushing folder IDs.
Initialize folderIds as an empty array before the if block or before it is first
used to ensure it is defined and can collect folder IDs properly.
This was referenced Aug 9, 2025
This was referenced Sep 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor