feat: add dashboard analytics to sdk and public api#353
feat: add dashboard analytics to sdk and public api#353magicspon wants to merge 1 commit intousesend:mainfrom
Conversation
|
@magicspon is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR introduces two new analytics API endpoints (GET /v1/analytics/email-time-series and GET /v1/analytics/reputation-metrics) with complete supporting infrastructure. The changes include public API route handlers, service layer functions for data aggregation, documentation pages, OpenAPI schema definitions, TypeScript type definitions, and an SDK wrapper class. The endpoints enable retrieval of email metrics over configurable time periods and reputation metrics, with optional domain filtering via domainId query parameter or team apiKey domain association. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/server/public-api/api/analytics/reputation-metrics-data.ts`:
- Around line 38-40: The code assigns domain using Number(domainIdParam) which
can produce NaN and then be passed into the DB query; update the logic around
the domain variable (referencing domain, team.apiKey.domainId, and domainIdParam
in reputation-metrics-data.ts) to validate/coerce the incoming domainIdParam
before using it—either use a Zod coercion schema (e.g.,
z.coerce.number().int().optional()) to parse and validate domainIdParam or add
an explicit guard that calls Number(domainIdParam), checks Number.isFinite(...)
and Number.isInteger(...), and only uses the numeric value when valid; if
invalid, fall back to team.apiKey.domainId or undefined so Prisma never receives
NaN.
🧹 Nitpick comments (4)
apps/web/src/server/public-api/api/analytics/email-time-series.ts (2)
5-49: Consider adding error response schemas (e.g., 401, 500) for completeness.The route definition only specifies a 200 response. Other endpoints in this codebase (e.g., domain and contact book endpoints in
schema.d.ts) define 403 and 404 error responses. While unauthenticated requests may be handled by middleware, documenting error responses in the OpenAPI schema improves the developer experience for API consumers.
52-65: Usec.req.valid("query")instead ofc.req.query()to leverage validated and typed query params.Lines 54–55 bypass the Zod-validated query by using
c.req.query("days")andc.req.query("domainId")directly. With@hono/zod-openapi, the validated query object is available viac.req.valid("query"), which provides proper typing and ensures you're working with the already-validated values. This pattern is demonstrated inapps/web/src/server/public-api/api/emails/list-emails.ts:109.♻️ Proposed refactor
function emailTimeSeries(app: PublicAPIApp) { app.openapi(route, async (c) => { const team = c.var.team; - const daysParam = c.req.query("days"); - const domainIdParam = c.req.query("domainId"); + const { days: daysParam, domainId: domainIdParam } = c.req.valid("query"); const days = daysParam ? Number(daysParam) : undefined; const domain = team.apiKey.domainId ?? (domainIdParam ? Number(domainIdParam) : undefined); const data = await emailTimeSeriesService({ days, domain, team }); return c.json(data); }); }packages/sdk/src/analytics.ts (1)
27-30: Redundant assignment in constructor.The
private readonly usesendparameter property already assignsthis.usesend. The explicitthis.usesend = usesendon Line 29 is unnecessary.Proposed fix
export class Analytics { - constructor(private readonly usesend: UseSend) { - this.usesend = usesend; - } + constructor(private readonly usesend: UseSend) {}apps/web/src/server/service/dashboard-service.ts (1)
11-12: Thedayscoercion logic silently maps all non-7 values to 30.
input.days !== 7 ? 30 : 7means values like14or1are silently treated as 30. The TRPC input schema (z.number().optional()) allows any number, while the public API restricts to"7" | "30". Consider constraining the TRPC schema to match, or at least accepting30explicitly:-const days = input.days !== 7 ? 30 : 7; +const days = input.days === 7 ? 7 : 30;The behavior is identical, but the current expression reads as if 7 is the special case being checked. More importantly, consider adding validation in the TRPC input schema:
days: z.union([z.literal(7), z.literal(30)]).optional(),This would align both API surfaces and prevent silent coercion of unexpected values.
| const domain = | ||
| team.apiKey.domainId ?? | ||
| (domainIdParam ? Number(domainIdParam) : undefined); |
There was a problem hiding this comment.
Number(domainIdParam) can produce NaN, which will be passed to the database query.
If a caller passes a non-numeric domainId (e.g., ?domainId=abc), Number("abc") yields NaN, which will be forwarded to Prisma's findMany as domainId: NaN. This will likely cause an unexpected query error or silently return no results.
Consider validating/coercing with the Zod schema (e.g., z.coerce.number().int().optional()) or adding a guard:
Proposed fix
const domain =
team.apiKey.domainId ??
- (domainIdParam ? Number(domainIdParam) : undefined);
+ (domainIdParam ? Number(domainIdParam) : undefined);
+
+ if (domain !== undefined && Number.isNaN(domain)) {
+ return c.json({ error: "Invalid domainId" }, 400);
+ }📝 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 domain = | |
| team.apiKey.domainId ?? | |
| (domainIdParam ? Number(domainIdParam) : undefined); | |
| const domain = | |
| team.apiKey.domainId ?? | |
| (domainIdParam ? Number(domainIdParam) : undefined); | |
| if (domain !== undefined && Number.isNaN(domain)) { | |
| return c.json({ error: "Invalid domainId" }, 400); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/server/public-api/api/analytics/reputation-metrics-data.ts`
around lines 38 - 40, The code assigns domain using Number(domainIdParam) which
can produce NaN and then be passed into the DB query; update the logic around
the domain variable (referencing domain, team.apiKey.domainId, and domainIdParam
in reputation-metrics-data.ts) to validate/coerce the incoming domainIdParam
before using it—either use a Zod coercion schema (e.g.,
z.coerce.number().int().optional()) to parse and validate domainIdParam or add
an explicit guard that calls Number(domainIdParam), checks Number.isFinite(...)
and Number.isInteger(...), and only uses the numeric value when valid; if
invalid, fall back to team.apiKey.domainId or undefined so Prisma never receives
NaN.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 12 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/server/public-api/api/analytics/email-time-series.ts">
<violation number="1" location="apps/web/src/server/public-api/api/analytics/email-time-series.ts:57">
P2: domainId is validated only as a string, but the handler converts raw query params with Number(), which can yield NaN and pass it to the service. Use validated query data and/or coerce domainId to a number in the schema to prevent NaN inputs.</violation>
</file>
<file name="apps/web/src/server/service/dashboard-service.ts">
<violation number="1" location="apps/web/src/server/service/dashboard-service.ts:49">
P2: The time series returns `days + 1` entries (loop runs from days to 0 inclusive), so a 7‑day request yields 8 days of data. This conflicts with the API’s “number of days” contract and likely produces an extra day.</violation>
<violation number="2" location="apps/web/src/server/service/dashboard-service.ts:58">
P2: `format` is given a YYYY-MM-DD string, which date-fns parses via `new Date` (UTC) and can shift the day in non-UTC timezones, producing mislabeled chart dates. Use a Date object (e.g., parseISO or the existing Date) instead of the string.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const daysParam = c.req.query("days"); | ||
| const domainIdParam = c.req.query("domainId"); | ||
|
|
||
| const days = daysParam ? Number(daysParam) : undefined; |
There was a problem hiding this comment.
P2: domainId is validated only as a string, but the handler converts raw query params with Number(), which can yield NaN and pass it to the service. Use validated query data and/or coerce domainId to a number in the schema to prevent NaN inputs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/public-api/api/analytics/email-time-series.ts, line 57:
<comment>domainId is validated only as a string, but the handler converts raw query params with Number(), which can yield NaN and pass it to the service. Use validated query data and/or coerce domainId to a number in the schema to prevent NaN inputs.</comment>
<file context>
@@ -0,0 +1,68 @@
+ const daysParam = c.req.query("days");
+ const domainIdParam = c.req.query("domainId");
+
+ const days = daysParam ? Number(daysParam) : undefined;
+ const domain =
+ team.apiKey.domainId ??
</file context>
| const filledResult: DailyEmailUsage[] = []; | ||
| const endDateObj = new Date(); | ||
|
|
||
| for (let i = days; i > -1; i--) { |
There was a problem hiding this comment.
P2: The time series returns days + 1 entries (loop runs from days to 0 inclusive), so a 7‑day request yields 8 days of data. This conflicts with the API’s “number of days” contract and likely produces an extra day.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/service/dashboard-service.ts, line 49:
<comment>The time series returns `days + 1` entries (loop runs from days to 0 inclusive), so a 7‑day request yields 8 days of data. This conflicts with the API’s “number of days” contract and likely produces an extra day.</comment>
<file context>
@@ -0,0 +1,133 @@
+ const filledResult: DailyEmailUsage[] = [];
+ const endDateObj = new Date();
+
+ for (let i = days; i > -1; i--) {
+ const dateStr = subDays(endDateObj, i)
+ .toISOString()
</file context>
| if (existingData) { | ||
| filledResult.push({ | ||
| ...existingData, | ||
| date: format(dateStr, "MMM dd"), |
There was a problem hiding this comment.
P2: format is given a YYYY-MM-DD string, which date-fns parses via new Date (UTC) and can shift the day in non-UTC timezones, producing mislabeled chart dates. Use a Date object (e.g., parseISO or the existing Date) instead of the string.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/server/service/dashboard-service.ts, line 58:
<comment>`format` is given a YYYY-MM-DD string, which date-fns parses via `new Date` (UTC) and can shift the day in non-UTC timezones, producing mislabeled chart dates. Use a Date object (e.g., parseISO or the existing Date) instead of the string.</comment>
<file context>
@@ -0,0 +1,133 @@
+ if (existingData) {
+ filledResult.push({
+ ...existingData,
+ date: format(dateStr, "MMM dd"),
+ });
+ } else {
</file context>
Greptile OverviewGreptile SummaryThis PR adds analytics endpoints for dashboard-style metrics (email time series and reputation metrics) to both the internal dashboard tRPC router and the public API, and exposes matching client methods via the SDK ( Confidence Score: 5/5
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type EmailTimeSeries = { | ||
| days?: number; | ||
| domain?: number | ||
| team: Team |
There was a problem hiding this comment.
Inconsistent formatting likely breaks lint
This new service file mixes tabs/spaces and omits semicolons (e.g., domain?: number / team: Team in EmailTimeSeries and ReputationMetricsData, and const { domain, team } = input at line 13/103). If this repo enforces @typescript-eslint/semi / no-tabs / prettier defaults, CI will fail. Please run formatting (prettier) and make indentation consistent (2 spaces) + add missing semicolons.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary by cubic
Expose dashboard analytics via two new public API endpoints and a typed SDK to let developers fetch email time series and reputation metrics programmatically. Also moved analytics logic into a shared service and updated docs.
New Features
Refactors
Written for commit afcdd46. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation