-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Optimize partner analytics #2806
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.
|
WalkthroughShifts analytics fetching to link ID-based queries. Updates two API routes to derive linkIds from program enrollment links, validate optional link parameters, and call getAnalytics with event "composite". Adjusts getProgramEnrollmentOrThrow to return links alongside program. Removes an unused next-auth session import in a SWR hook. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as route.ts (partner-profile/.../[programId]/analytics)
participant Svc as getProgramEnrollmentOrThrow
participant ANA as getAnalytics
Client->>API: GET /partner-profile/programs/:programId/analytics?linkId|domain&key
API->>Svc: fetch enrollment for programId
Svc-->>API: { program, links }
alt linkId provided
API->>API: validate linkId exists in links
API->>ANA: getAnalytics({ event: "composite", linkId })
else domain+key provided
API->>API: find link by domain+key in links
API->>ANA: getAnalytics({ event: "composite", linkId })
else no specific link
API->>ANA: getAnalytics({ event: "composite", linkIds: links.map(id) })
end
ANA-->>API: analytics result
API-->>Client: JSON response
sequenceDiagram
autonumber
actor Client
participant API as route.ts (partners/analytics)
participant DB as load programEnrollment (include links)
participant ANA as getAnalytics
Client->>API: GET /partners/analytics?...groupBy=...
API->>DB: fetch programEnrollment { include: { links (orderBy clicks desc) } }
DB-->>API: { programEnrollment, links }
API->>ANA: getAnalytics({ event: "composite", linkIds: links.map(id), ...filters })
ANA-->>API: analytics data
API-->>Client: JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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/app/(ee)/api/partners/analytics/route.ts (1)
189-200
: Fix potential runtime crash and limit to actual “top” linksWhen
groupBy === "top_links"
,analytics.find(...)
can returnundefined
(e.g., if TB returns only top N butprogramEnrollment.links
includes more). Spreading...analyticsData
then throws. Also, this returns all enrollment links, not just the top subset.Use analytics as the driving set, then join to the enrollment link for metadata.
Apply this diff:
- const topLinksWithEarnings = programEnrollment.links.map((link) => { - const analyticsData = analytics.find((a) => a.id === link.id); - const earnings = topLinkEarnings.find((t) => t.linkId === link.id); - - return partnersTopLinksSchema.parse({ - ...link, - ...analyticsData, - link: link.id, - createdAt: link.createdAt.toISOString(), - earnings: Number(earnings?._sum.earnings ?? 0), - }); - }); + const linkById = new Map(programEnrollment.links.map((l) => [l.id, l])); + const topLinksWithEarnings = (analytics as Array<{ id: string }>).map((a) => { + const link = linkById.get(a.id); + if (!link) return null; + const earnings = topLinkEarnings.find((t) => t.linkId === a.id); + return partnersTopLinksSchema.parse({ + ...link, + ...a, + link: a.id, + createdAt: link.createdAt.toISOString(), + earnings: Number(earnings?._sum.earnings ?? 0), + }); + }).filter(Boolean);
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.ts (2)
26-38
: Good fallback to (domain, key); consider normalizing for case/punycodeIf keys can be case-sensitive or punycoded in storage, a straight
===
may miss matches. Consider normalizing both sides (e.g., decode-case/IDNA on stored, and normalize incoming) to avoid false negatives.
40-44
: Consider passing workspaceId for parity and future-proofingWhile this endpoint typically targets a single link/timeseries,
getAnalytics
usesworkspaceId
for certain groupBys (e.g.,top_links
) and joins. Passing it costs little and keeps behavior consistent.Apply this diff:
- const response = await getAnalytics({ - ...rest, - ...(linkId ? { linkId } : { linkIds: links.map((link) => link.id) }), - dataAvailableFrom: program.createdAt, - }); + const response = await getAnalytics({ + ...rest, + ...(linkId ? { linkId } : { linkIds: links.map((link) => link.id) }), + workspaceId: program.workspaceId, + dataAvailableFrom: program.createdAt, + });Also, ensure callers explicitly set
event
as intended (e.g.,"composite"
) or that your schema defaults it correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.ts
(1 hunks)apps/web/app/(ee)/api/partners/analytics/route.ts
(2 hunks)apps/web/lib/swr/use-partner-analytics.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/lib/swr/use-partner-analytics.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.ts (3)
apps/web/lib/api/programs/get-program-enrollment-or-throw.ts (1)
getProgramEnrollmentOrThrow
(5-75)apps/web/lib/api/errors.ts (1)
DubApiError
(75-92)apps/web/lib/analytics/get-analytics.ts (1)
getAnalytics
(20-229)
⏰ 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/partners/analytics/route.ts (1)
54-58
: LGTM: including links for linkIds is the right directionAlways including
links
to drivelinkIds
keeps the analytics call consistent across groupBys.apps/web/app/(ee)/api/partner-profile/programs/[programId]/analytics/route.ts (3)
2-2
: LGTM: consistent error handling via DubApiErrorImporting and using the shared error type keeps responses uniform.
11-14
: LGTM: fetching enrollment with links up frontDestructuring
{ program, links }
fromgetProgramEnrollmentOrThrow
simplifies downstream logic.
19-26
: LGTM: validate linkId belongs to this enrollmentEnsures partners can’t query analytics for links outside their enrollment.
const analytics = await getAnalytics({ | ||
programId, | ||
partnerId, | ||
tenantId, | ||
event: "composite", | ||
groupBy, | ||
linkIds: programEnrollment.links.map((link) => link.id), | ||
interval, | ||
start, | ||
end, | ||
timezone, | ||
query, | ||
event: "composite", | ||
}); |
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.
Pass workspaceId to getAnalytics (prevents empty top_links and join miss)
getAnalytics
’s top_links
path joins on workspaceId
to enrich link metadata. Without it, the Tinybird result gets filtered to nothing during the Prisma join, and later code may spread undefined
. Add workspaceId: workspace.id
.
Apply this diff:
- const analytics = await getAnalytics({
- event: "composite",
- groupBy,
- linkIds: programEnrollment.links.map((link) => link.id),
- interval,
- start,
- end,
- timezone,
- query,
- });
+ const analytics = await getAnalytics({
+ event: "composite",
+ groupBy,
+ linkIds: programEnrollment.links.map((link) => link.id),
+ workspaceId: workspace.id,
+ interval,
+ start,
+ end,
+ timezone,
+ query,
+ });
📝 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 analytics = await getAnalytics({ | |
programId, | |
partnerId, | |
tenantId, | |
event: "composite", | |
groupBy, | |
linkIds: programEnrollment.links.map((link) => link.id), | |
interval, | |
start, | |
end, | |
timezone, | |
query, | |
event: "composite", | |
}); | |
const analytics = await getAnalytics({ | |
event: "composite", | |
groupBy, | |
linkIds: programEnrollment.links.map((link) => link.id), | |
workspaceId: workspace.id, | |
interval, | |
start, | |
end, | |
timezone, | |
query, | |
}); |
🤖 Prompt for AI Agents
In apps/web/app/(ee)/api/partners/analytics/route.ts around lines 69 to 78, the
getAnalytics call omits workspaceId so the top_links path's Prisma join filters
out results and later code can spread undefined; pass workspaceId: workspace.id
in the getAnalytics argument object (add workspaceId: workspace.id alongside
event, groupBy, linkIds, interval, start, end, timezone, query).
Summary by CodeRabbit