Stats api #461
Stats api #461
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce new API controllers and validation modules for retrieving and aggregating email statistics by day and by period. Route handlers are refactored to delegate logic to these controllers, and OpenAPI documentation is updated to include the new endpoints. Type-safe schemas for queries and responses are added using Zod, and import paths in frontend components are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant Controller
participant DB/Gmail
Client->>API_Route: HTTP GET /api/v1/stats/summary or /stats/day
API_Route->>API_Route: Validate API key and parse query
API_Route->>Controller: Call getStatsByPeriod or getPastSevenDayStats
Controller->>DB/Gmail: Query statistics (by period or by day)
DB/Gmail-->>Controller: Return aggregated stats
Controller-->>API_Route: Return formatted stats
API_Route-->>Client: JSON response with stats
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/api/v1/openapi/route.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/app/api/v1/stats/day/route.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by apps/web/app/api/v1/stats/summary/route.tsOops! Something went wrong! :( ESLint: 9.28.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/web/app/api/user/stats/by-period/controller.ts (1)
97-123: Efficient SQL query with proper aggregations!The query is well-structured with:
- CTE for better organization
- Efficient conditional aggregations
- Proper aliasing for TypeScript compatibility
Consider adding an index on
(emailAccountId, date)if not already present for optimal query performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/app/(app)/[emailAccountId]/stats/CombinedStatsChart.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/DetailedStats.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/StatsChart.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx(1 hunks)apps/web/app/api/user/stats/by-period/controller.ts(1 hunks)apps/web/app/api/user/stats/by-period/route.ts(1 hunks)apps/web/app/api/user/stats/day/controller.ts(1 hunks)apps/web/app/api/user/stats/day/route.ts(1 hunks)apps/web/app/api/v1/openapi/route.ts(2 hunks)apps/web/app/api/v1/stats/day/route.ts(1 hunks)apps/web/app/api/v1/stats/summary/route.ts(1 hunks)apps/web/app/api/v1/stats/validation.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
apps/web/app/api/v1/stats/summary/route.ts (6)
apps/web/utils/logger.ts (1)
createScopedLogger(17-65)apps/web/utils/middleware.ts (1)
withError(182-184)apps/web/utils/api-auth.ts (1)
validateApiKeyAndGetGmailClient(66-90)apps/web/app/api/v1/stats/validation.ts (1)
summaryStatsQuerySchema(19-35)apps/web/app/api/v1/helpers.ts (1)
getEmailAccountId(9-36)apps/web/app/api/user/stats/by-period/controller.ts (1)
getStatsByPeriod(16-52)
apps/web/app/api/v1/openapi/route.ts (1)
apps/web/app/api/v1/stats/validation.ts (4)
summaryStatsQuerySchema(19-35)summaryStatsResponseSchema(37-55)dayStatsQuerySchema(58-68)dayStatsResponseSchema(70-77)
apps/web/app/api/user/stats/day/controller.ts (2)
apps/web/utils/gmail/message.ts (1)
getMessages(207-225)apps/web/utils/date.ts (1)
dateToSeconds(50-52)
apps/web/app/api/v1/stats/validation.ts (1)
packages/tinybird/src/query.ts (1)
zodPeriod(4-4)
apps/web/app/api/user/stats/by-period/controller.ts (1)
packages/tinybird/src/query.ts (1)
zodPeriod(4-4)
🔇 Additional comments (22)
apps/web/app/api/user/stats/day/controller.ts (2)
6-12: LGTM: Well-structured schema and type definitionsThe Zod schema for query validation and type inference is properly implemented. The type exports provide good type safety for consumers.
62-76: LGTM: Gmail query generation logic is correctThe
getQueryfunction properly constructs Gmail search queries for different email types using Unix timestamps. The logic for inbox, sent, and archived emails is accurate.apps/web/app/api/v1/stats/validation.ts (4)
1-3: LGTM: Clean imports and dependenciesThe imports are properly organized and the use of
zodPeriodfrom the tinybird package ensures consistency across the codebase.
4-16: LGTM: Well-defined base stats query schemaThe
statsQuerySchemaprovides good validation for optional timestamp filters and email parameters with helpful descriptions.
79-83: LGTM: Comprehensive type exportsAll schemas have corresponding TypeScript type exports, enabling type-safe usage throughout the codebase.
70-77:⚠️ Potential issueFix date format description inconsistency
The description states "Date in YYYY-MM-DD format" but the controller implementation in
apps/web/app/api/user/stats/day/controller.ts(line 36) usesDD/MMformat. This inconsistency needs to be resolved.Either update the description to match the actual format or fix the controller implementation:
- date: z.string().describe("Date in YYYY-MM-DD format"), + date: z.string().describe("Date in DD/MM format"),Or preferably, update the controller to use the standard ISO date format as suggested in the controller review.
Likely an incorrect or invalid review comment.
apps/web/app/(app)/[emailAccountId]/stats/CombinedStatsChart.tsx (1)
7-7: LGTM: Import path update aligns with controller refactoringThe import path change from
/routeto/controllercorrectly reflects the architectural refactoring where business logic was moved to dedicated controller modules.apps/web/app/(app)/[emailAccountId]/stats/StatsChart.tsx (1)
6-6: LGTM: Consistent import path updateThe import path change from
/routeto/controlleris consistent with the refactoring pattern and maintains the same functionality while using the new controller module structure.apps/web/app/(app)/[emailAccountId]/stats/DetailedStats.tsx (1)
14-14: LGTM! Clean import path update.The import path change aligns perfectly with the architectural refactoring to move business logic from route handlers to dedicated controller modules. This maintains type safety while supporting better separation of concerns.
apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx (1)
16-16: LGTM! Consistent import path update.This change maintains consistency with the refactoring pattern seen across other stats components. The import path correctly points to the new controller module.
apps/web/app/api/user/stats/day/route.ts (2)
4-7: Excellent architectural refactoring!The delegation of business logic to a dedicated controller module is a great improvement. This separation of concerns makes the code more maintainable and testable while keeping the route handler focused on HTTP request/response handling.
14-22: Clean implementation of the delegation pattern.The route handler correctly parses the query parameters using the controller's schema and delegates the core logic while maintaining the same API interface. This is exactly how route handlers should be structured.
apps/web/app/api/user/stats/by-period/route.ts (2)
3-6: Consistent architectural improvement.This refactoring follows the same excellent pattern as the day stats route, showing a well-planned and systematic approach to improving the codebase architecture.
12-21: Well-implemented delegation pattern.The parameter parsing and delegation to the controller is clean and maintains the existing API contract. The route handler is now properly focused on HTTP concerns while business logic is handled by the controller.
apps/web/app/api/v1/stats/summary/route.ts (2)
1-10: LGTM!The imports are well-organized and the scoped logger setup follows best practices for debugging and monitoring.
11-60: Well-structured API handler implementation!The handler follows best practices:
- Proper authentication and authorization flow
- Clear separation of concerns by delegating to the controller
- Comprehensive error handling with appropriate status codes
- Structured logging with relevant context for debugging
apps/web/app/api/v1/openapi/route.ts (2)
16-21: LGTM!The imports for stats validation schemas follow the existing pattern and are properly organized.
81-121: Well-documented API endpoint registrations!The new endpoints are properly configured with:
- Clear, descriptive documentation
- Consistent security configuration
- Proper request/response schema validation
apps/web/app/api/v1/stats/day/route.ts (1)
1-60: Consistent implementation with proper Gmail client handling!The handler correctly:
- Extracts the Gmail client for API calls
- Passes all required parameters to the controller
- Includes relevant context (type) in error logging
apps/web/app/api/user/stats/by-period/controller.ts (3)
1-14: LGTM!Good use of Zod schemas for validation and proper type exports for TypeScript support across modules.
16-52: Well-structured data transformation and aggregation!The implementation properly:
- Formats dates for user-friendly display
- Handles bigint to number conversion for JSON compatibility
- Efficiently calculates totals using lodash
68-95:✅ Verification successful
Verify the safety of Prisma.raw usage for SQL injection prevention.
While the
periodvalue is validated by the zodPeriod enum, usingPrisma.rawwith string interpolation (lines 100-101) could potentially be unsafe if the validation is bypassed.
🏁 Script executed:
#!/bin/bash # Description: Verify that zodPeriod properly restricts values to prevent SQL injection # Check the zodPeriod definition to ensure it's an enum with fixed values ast-grep --pattern 'const zodPeriod = $_' # Also check for any other usages of this pattern in the codebase rg -A 5 'Prisma\.raw.*\$\{.*\}' --type tsLength of output: 1004
No SQL injection risk in Prisma.raw usage
Verified that
zodPeriodis defined as a strict enum ("day" | "week" | "month" | "year") and thatdateFormatis deterministically derived from those same values. Since bothperiodanddateFormatcan only ever be one of these fixed strings, the interpolations in:
TO_CHAR(date, ${Prisma.raw(\'${dateFormat}'`)})`DATE_TRUNC(${Prisma.raw(\'${period}'`)}, date)`pose no SQL injection risk. All user input is fully constrained by the Zod schema before reaching
Prisma.raw.
| date.setDate(date.getDate() + i); | ||
| return date; | ||
| }).map(async (date) => { | ||
| const dateString = `${date.getDate()}/${date.getMonth() + 1}`; |
There was a problem hiding this comment.
Fix date format inconsistency
The date formatting here uses DD/MM format, but the validation schema in apps/web/app/api/v1/stats/validation.ts (line 73) expects "Date in YYYY-MM-DD format". This inconsistency could cause issues for API consumers.
Apply this fix to use consistent date formatting:
- const dateString = `${date.getDate()}/${date.getMonth() + 1}`;
+ const dateString = date.toISOString().split('T')[0]; // YYYY-MM-DD format📝 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 dateString = `${date.getDate()}/${date.getMonth() + 1}`; | |
| const dateString = date.toISOString().split('T')[0]; // YYYY-MM-DD format |
🤖 Prompt for AI Agents
In apps/web/app/api/user/stats/day/controller.ts at line 36, the date string is
formatted as DD/MM, which conflicts with the expected YYYY-MM-DD format defined
in the validation schema at apps/web/app/api/v1/stats/validation.ts line 73.
Update the dateString assignment to format the date as YYYY-MM-DD by extracting
the full year, month (adding 1), and day, and concatenating them with hyphens to
ensure consistency with the validation schema.
| const today = new Date(); | ||
| const sevenDaysAgo = new Date( | ||
| today.getFullYear(), | ||
| today.getMonth(), | ||
| today.getDate() - (DAYS - 1), // include today in stats | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider timezone handling for date calculations
The date calculation logic doesn't account for timezone differences, which could lead to incorrect day boundaries for users in different timezones.
Consider using a timezone-aware date library or explicitly handling the user's timezone:
- const today = new Date();
- const sevenDaysAgo = new Date(
- today.getFullYear(),
- today.getMonth(),
- today.getDate() - (DAYS - 1), // include today in stats
- );
+ // Consider accepting timezone as parameter or using user's timezone
+ const today = new Date();
+ const sevenDaysAgo = new Date(
+ today.getFullYear(),
+ today.getMonth(),
+ today.getDate() - (DAYS - 1), // include today in stats
+ );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/api/user/stats/day/controller.ts around lines 22 to 27, the
current date calculation does not handle timezones, which can cause incorrect
day boundaries for users in different timezones. To fix this, use a
timezone-aware date library like date-fns-tz or moment-timezone to calculate
today and sevenDaysAgo based on the user's timezone, or explicitly adjust the
date objects to the correct timezone before performing date arithmetic.
| const messages = await getMessages(options.gmail, { | ||
| query, | ||
| maxResults: 500, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make maxResults configurable and add error handling
The hardcoded maxResults: 500 should be configurable, and Gmail API calls lack error handling which could cause unhandled promise rejections.
+const DEFAULT_MAX_RESULTS = 500;
+
export async function getPastSevenDayStats(
options: {
emailAccountId: string;
gmail: gmail_v1.Gmail;
+ maxResults?: number;
} & StatsByDayQuery,
) {
// ... existing code ...
- const messages = await getMessages(options.gmail, {
- query,
- maxResults: 500,
- });
+ try {
+ const messages = await getMessages(options.gmail, {
+ query,
+ maxResults: options.maxResults ?? DEFAULT_MAX_RESULTS,
+ });
+ count = messages.messages?.length || 0;
+ } catch (error) {
+ console.error(`Failed to fetch messages for ${dateString}:`, error);
+ count = 0; // or throw error based on requirements
+ }📝 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 messages = await getMessages(options.gmail, { | |
| query, | |
| maxResults: 500, | |
| }); | |
| // Add at top of file | |
| const DEFAULT_MAX_RESULTS = 500; | |
| export async function getPastSevenDayStats( | |
| options: { | |
| emailAccountId: string; | |
| gmail: gmail_v1.Gmail; | |
| maxResults?: number; | |
| } & StatsByDayQuery, | |
| ) { | |
| // ... existing code ... | |
| try { | |
| const messages = await getMessages(options.gmail, { | |
| query, | |
| maxResults: options.maxResults ?? DEFAULT_MAX_RESULTS, | |
| }); | |
| count = messages.messages?.length || 0; | |
| } catch (error) { | |
| console.error(`Failed to fetch messages for ${dateString}:`, error); | |
| count = 0; // or rethrow based on requirements | |
| } | |
| // ... rest of function ... | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/user/stats/day/controller.ts around lines 44 to 47, the
maxResults parameter for getMessages is hardcoded to 500 and there is no error
handling for the Gmail API call. Refactor the code to accept maxResults as a
configurable parameter, possibly passed via options or environment variables.
Wrap the getMessages call in a try-catch block to handle any errors gracefully
and prevent unhandled promise rejections, logging or returning an appropriate
error response.
|
Bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Ping me, if I can contribute here. |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation