feat: add dashboard analytics to sdk and public api#353
feat: add dashboard analytics to sdk and public api#353KMKoushik merged 2 commits 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. |
WalkthroughAdds two analytics endpoints: GET /v1/analytics/email-time-series and GET /v1/analytics/reputation-metrics. Changes include OpenAPI spec additions, new MDX docs and docs navigation entries, server public-api route handlers, service-layer implementations (emailTimeSeries and reputationMetricsData), integration into the public-api initializer, and SDK client support (Analytics class and UseSend.analytics). The service functions perform DB aggregation and return typed result shapes; public routes wire requests to those services and expose the new responses. 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. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/server/public-api/index.ts`:
- Around line 31-32: The two new imports emailTimeSeries and
reputationMetricsData in public-api index.ts use relative paths; update their
import specifiers to use the ~/ alias for modules under apps/web/src (i.e.,
replace "./api/analytics/email-time-series" and
"./api/analytics/reputation-metrics-data" with their corresponding "~/..."
paths) so they follow the apps/web src import convention and resolver.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d746c5f9-a80e-48a2-9cd4-278980d8990d
📒 Files selected for processing (5)
apps/docs/api-reference/openapi.jsonapps/docs/docs.jsonapps/web/src/server/public-api/index.tspackages/sdk/src/usesend.tspackages/sdk/types/schema.d.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/docs/docs.json
- apps/docs/api-reference/openapi.json
- packages/sdk/types/schema.d.ts
| import emailTimeSeries from "./api/analytics/email-time-series"; | ||
| import reputationMetricsData from "./api/analytics/reputation-metrics-data"; |
There was a problem hiding this comment.
Use ~/ aliases for the new analytics imports.
These new imports point to modules under apps/web/src, so they should use the ~/ alias instead of relative paths.
Proposed change
-import emailTimeSeries from "./api/analytics/email-time-series";
-import reputationMetricsData from "./api/analytics/reputation-metrics-data";
+import emailTimeSeries from "~/server/public-api/api/analytics/email-time-series";
+import reputationMetricsData from "~/server/public-api/api/analytics/reputation-metrics-data";As per coding guidelines apps/web/src/**/*.{ts,tsx}: Use the ~/ alias for imports from src in apps/web (e.g., import { x } from "~/utils/x").
📝 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.
| import emailTimeSeries from "./api/analytics/email-time-series"; | |
| import reputationMetricsData from "./api/analytics/reputation-metrics-data"; | |
| import emailTimeSeries from "~/server/public-api/api/analytics/email-time-series"; | |
| import reputationMetricsData from "~/server/public-api/api/analytics/reputation-metrics-data"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/server/public-api/index.ts` around lines 31 - 32, The two new
imports emailTimeSeries and reputationMetricsData in public-api index.ts use
relative paths; update their import specifiers to use the ~/ alias for modules
under apps/web/src (i.e., replace "./api/analytics/email-time-series" and
"./api/analytics/reputation-metrics-data" with their corresponding "~/..."
paths) so they follow the apps/web src import convention and resolver.
KMKoushik
left a comment
KMKoushik
left a comment
There was a problem hiding this comment.
Reviewed locally. LGTM with minor non-blocking input validation follow-up.
|
LGTM |
2 participants
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. Centralized analytics logic in a shared service and updated docs.
New Features
Refactors
Written for commit 719fc02. Summary will update on new commits.
Summary by CodeRabbit
New Features
SDK
Documentation