improvement: Improve loading speed of visitors tab #1716
Conversation
|
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis set of changes introduces comprehensive support for server-side pagination and sorting to the viewers and document views features. The React components for listing viewers and their document views are refactored to be fully controlled via external props and callbacks, delegating pagination and sorting to parent components. The associated SWR hooks and API endpoints are updated to accept and process pagination and sorting parameters, returning structured metadata alongside data. Database performance is improved through new indexes and optimized SQL queries, and caching is enhanced at both the API and Redis levels. Changes
Sequence Diagram(s)Viewer List Pagination & Sorting (Visitors Page)sequenceDiagram
participant User
participant VisitorsPage
participant ContactsTable
participant useViewers
participant API
participant DB
User->>ContactsTable: Interacts (sorts, paginates)
ContactsTable->>VisitorsPage: Calls onSortChange/onPageChange
VisitorsPage->>useViewers: Updates state, calls hook with new params
useViewers->>API: Fetch /api/teams/{teamId}/viewers with pagination/sorting
API->>DB: Query viewers with pagination/sorting
DB-->>API: Return viewers, stats, counts
API-->>useViewers: Return viewers, pagination, sorting
useViewers-->>VisitorsPage: Return data
VisitorsPage-->>ContactsTable: Pass new data/metadata
Viewer Detail Document Views Pagination & SortingsequenceDiagram
participant User
participant VisitorDetailPage
participant ContactsDocumentsTable
participant useViewer
participant API
participant DB
participant Redis
User->>ContactsDocumentsTable: Interacts (sorts, paginates)
ContactsDocumentsTable->>VisitorDetailPage: Calls onSortChange/onPageChange
VisitorDetailPage->>useViewer: Updates state, calls hook with new params
useViewer->>API: Fetch /api/teams/{teamId}/viewers/{id} with pagination/sorting
API->>DB: Query views grouped by document, aggregate stats
API->>Redis: Check/set cache for durations
Redis-->>API: Return cached durations (if any)
API-->>useViewer: Return viewer, views, durations, pagination, sorting
useViewer-->>VisitorDetailPage: Return data
VisitorDetailPage-->>ContactsDocumentsTable: Pass new data/metadata
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
npm error Exit handler never called! ✨ 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: 5
🧹 Nitpick comments (4)
prisma/migrations/20250624020507_indexed_view_modal/migration.sql (1)
1-2: Typo in migration filename: "modal" should be "model".The migration filename contains "indexed_view_modal" but should be "indexed_view_model". While this doesn't affect functionality, it's important to maintain consistent naming conventions for clarity.
pages/api/teams/[teamId]/viewers/[id]/index.ts (1)
78-79: Consider extracting cache key generation into a helper function.Cache key construction is repeated and could be centralized for consistency.
function generateCacheKey(prefix: string, params: { teamId: string; viewerId: string; page?: number; pageSize?: number; sortBy?: string; sortOrder?: string; }): string { const parts = [prefix, params.teamId, params.viewerId]; if (params.page !== undefined) parts.push(params.page.toString()); if (params.pageSize !== undefined) parts.push(params.pageSize.toString()); if (params.sortBy) parts.push(params.sortBy); if (params.sortOrder) parts.push(params.sortOrder); return parts.join(':'); }Also applies to: 93-93, 183-183, 287-287
components/visitors/contacts-document-table.tsx (1)
212-213: Consider using optional chaining for consistency.For consistency with other parts of the codebase, consider using optional chaining.
- const duration = durations[view.documentId] ?? view.totalDuration; + const duration = durations?.[view.documentId] ?? view.totalDuration;components/visitors/contacts-table.tsx (1)
84-110: Simplify the sorting logic for better readability.The current sorting logic is complex and could be simplified to match the pattern in ContactsDocumentsTable.
const handleSort = useCallback( (columnId: string) => { if (!onSortChange) return; const currentSortBy = sorting?.sortBy; const currentSortOrder = sorting?.sortOrder; - if (currentSortBy === columnId) { - if (columnId === "lastViewed") { - if (currentSortOrder === "asc") { - onSortChange("lastViewed", "desc"); - } else { - onSortChange("lastViewed", "asc"); - } - } else { - if (currentSortOrder === "asc") { - onSortChange(columnId, "desc"); - } else if (currentSortOrder === "desc") { - onSortChange("lastViewed", "desc"); - } - } - } else { - onSortChange(columnId, "asc"); - } + let newSortOrder = "desc"; + if (currentSortBy === columnId) { + if (currentSortOrder === "desc") { + newSortOrder = "asc"; + } else if (currentSortOrder === "asc") { + // Reset to default sort + onSortChange("lastViewed", "desc"); + return; + } + } + + onSortChange(columnId, newSortOrder); }, [onSortChange, sorting?.sortBy, sorting?.sortOrder], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
components/visitors/contacts-document-table.tsx(4 hunks)components/visitors/contacts-table.tsx(3 hunks)lib/swr/use-viewer.ts(1 hunks)lib/swr/use-viewers.ts(2 hunks)pages/api/teams/[teamId]/viewers/[id]/index.ts(3 hunks)pages/api/teams/[teamId]/viewers/index.ts(3 hunks)pages/visitors/[id]/index.tsx(3 hunks)pages/visitors/index.tsx(2 hunks)prisma/migrations/20250624020507_indexed_view_modal/migration.sql(1 hunks)prisma/schema/schema.prisma(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
lib/swr/use-viewer.ts
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
prisma/migrations/20250624020507_indexed_view_modal/migration.sql (1)
1-2: Good addition of viewerId index for performance optimization.This index will significantly improve query performance when filtering or joining on
viewerId, which aligns with the refactored API queries that aggregate view statistics by viewer.prisma/schema/schema.prisma (1)
299-299: Schema index addition is correct and consistent with migration.The index on
viewerIdis properly defined and will optimize queries that filter or aggregate views by viewer.pages/visitors/index.tsx (2)
18-22: Well-structured state management for pagination and sorting.Good default values chosen - starting with page 1, 10 items per page, and sorting by lastViewed in descending order provides a sensible initial view.
34-43: Correct implementation of state change handlers.Excellent practice to reset to page 1 when changing page size or sorting criteria. This prevents users from landing on non-existent pages.
pages/api/teams/[teamId]/viewers/index.ts (2)
36-40: Good input validation for sort parameters.The validation against whitelisted values prevents invalid sort fields and orders from being used.
79-102: Excellent performance optimization with raw SQL query.The refactored query efficiently aggregates view statistics in a single query with proper LEFT JOIN, avoiding N+1 query problems. The use of parameterized queries for teamId and search conditions is secure.
pages/visitors/[id]/index.tsx (1)
27-55: Consistent implementation of pagination and sorting.The implementation mirrors the pattern used in the visitors list page, maintaining consistency across the application. The state management and handler logic are correctly implemented.
pages/api/teams/[teamId]/viewers/[id]/index.ts (1)
158-180: Verify SQL injection safety with Prisma parameters.The raw SQL queries appear to be using parameterized queries correctly, which is good for SQL injection prevention. The use of
${id}and other parameters should be safe as Prisma handles the escaping.Also applies to: 240-265
components/visitors/contacts-document-table.tsx (1)
92-112: Well-implemented sorting logic with proper state cycling.The handleSort implementation correctly cycles through desc → asc → reset states, providing good UX.
components/visitors/contacts-table.tsx (1)
1-324: Overall implementation looks good!The refactoring to controlled components with external pagination and sorting is well-executed. This pattern will improve performance by allowing parent components to manage state and reduce unnecessary re-renders.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/swr/use-viewer.ts (2)
52-57: URLSearchParams implementation addresses previous feedback.This properly addresses the past review comment about using URLSearchParams for safer query string construction.
69-69: Optional chaining implementation needs correction.The current implementation
(viewer?.views?.length ?? 0) > 0is unnecessarily complex. The past review comment suggested a simpler approach.Apply this diff for cleaner optional chaining:
- const shouldFetchDurations = (viewer?.views?.length ?? 0) > 0; + const shouldFetchDurations = viewer?.views?.length > 0;pages/api/teams/[teamId]/viewers/[id]/index.ts (1)
13-60: Excellent helper function extraction that addresses previous feedback.The
fetchAndCacheDurationshelper successfully eliminates code duplication as suggested in the past review. The implementation includes proper error handling, batch processing, and caching strategy.
🧹 Nitpick comments (1)
pages/api/teams/[teamId]/viewers/[id]/index.ts (1)
168-250: Optimize the totalDuration sorting approach.The current implementation fetches all documents first, then sorts in memory. For large datasets, this could be inefficient.
Consider implementing database-level sorting for totalDuration when the dataset grows large:
// Alternative approach: Use a CTE or subquery to sort at database level const documentsWithDurations = await prisma.$queryRaw` WITH document_durations AS ( SELECT v."documentId", -- Fetch durations via database function if possible get_document_duration(v."documentId", v."viewerId") as "totalDuration" FROM "View" v WHERE v."viewerId" = ${id} GROUP BY v."documentId" ) SELECT * FROM document_durations ORDER BY "totalDuration" ${Prisma.raw(orderDirection)} LIMIT ${limit} OFFSET ${offset} `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/swr/use-viewer.ts(1 hunks)lib/swr/use-viewers.ts(2 hunks)pages/api/teams/[teamId]/viewers/[id]/index.ts(3 hunks)pages/api/teams/[teamId]/viewers/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pages/api/teams/[teamId]/viewers/index.ts
- lib/swr/use-viewers.ts
🔇 Additional comments (8)
lib/swr/use-viewer.ts (4)
9-38: Well-structured type definition with comprehensive metadata.The expanded
ViewerWithViewstype provides detailed document information and proper pagination/sorting metadata structure. This will improve type safety and developer experience.
40-45: Good default parameter choices for pagination and sorting.The default values (page=1, pageSize=10, sortBy="lastViewed", sortOrder="desc") are sensible for a visitors table use case.
71-83: Effective separation of concerns with dedicated duration fetching.The conditional second SWR hook for durations is well-implemented with appropriate caching settings and error handling. The
withDuration=trueparameter clearly separates the two concerns.
89-91: Robust loading state management.The distinction between main loading and durations loading provides clear feedback to components about what data is being fetched.
pages/api/teams/[teamId]/viewers/[id]/index.ts (4)
73-92: Robust parameter validation and parsing.The query parameter parsing includes proper validation with whitelisted sort fields and orders, preventing potential security issues and ensuring data integrity.
264-266: Smart caching strategy prevents cache conflicts.The conditional caching logic (
if (withDuration !== "true")) prevents caching conflicts between different request types, which is crucial for data consistency.
277-283: Graceful error handling with fallback values.The error handling for duration processing provides meaningful fallbacks, ensuring the API remains functional even when duration data is unavailable.
224-249: ```shell
#!/bin/bashLocate the declaration and assignments of
orderClauseto assess input validationrg -n "orderClause" pages/api/teams/[teamId]/viewers/[id]/index.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/swr/use-viewers.ts (1)
47-57: URLSearchParams implementation addresses previous review feedback effectively.The implementation resolves the past review concern about manual string concatenation. The approach using
append()method is consistent and includes proper type checking for the search query withtypeof searchQuery === 'string'.
🧹 Nitpick comments (2)
pages/api/teams/[teamId]/viewers/[id]/index.ts (2)
153-231: Consider performance implications of in-memory sorting for totalDuration.The current approach fetches all documents when sorting by
totalDurationand then applies pagination in-memory. This could become a performance bottleneck for viewers with many documents.Consider implementing database-level duration aggregation or caching sorted results to avoid loading all records into memory.
The conditional logic for different sorting scenarios is also quite complex. Consider extracting query-building logic into separate functions:
+async function buildViewsQuery( + viewerId: string, + sort: string, + order: string, + limit: number, + offset: number +) { + // Query building logic here +} +async function getViewsWithDurationSort(viewerId: string, order: string) { + // Duration sorting logic here +}
61-309: Consider breaking down the main handler for better maintainability.The main handler function has grown quite complex with multiple responsibilities. Consider extracting logic into focused helper functions:
async function validateTeamAndViewer(teamId: string, viewerId: string, userId: string) { // Team and viewer validation logic } async function buildPaginatedViews( viewerId: string, pagination: PaginationParams, sorting: SortingParams ) { // Query building and execution logic } async function formatResponse( viewer: any, views: any[], pagination: any, sorting: any ) { // Response formatting logic }This would improve code organization and make the main handler more readable and testable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/swr/use-viewers.ts(1 hunks)pages/api/teams/[teamId]/viewers/[id]/index.ts(4 hunks)pages/api/teams/[teamId]/viewers/index.ts(3 hunks)prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql(1 hunks)prisma/schema/schema.prisma(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- prisma/schema/schema.prisma
- pages/api/teams/[teamId]/viewers/index.ts
🔇 Additional comments (9)
lib/swr/use-viewers.ts (5)
9-16: Well-defined type structure for viewer statistics.The
ViewerWithStatstype provides a clean interface for viewer data with aggregated statistics. The field selection is appropriate for the use case.
18-32: Comprehensive response structure supports full pagination and sorting workflow.The
ViewersResponsetype is well-designed with:
- Clear separation of data, pagination metadata, and sorting state
- Complete pagination fields for UI implementation
- Consistent naming conventions
34-39: Function signature provides sensible defaults for pagination and sorting.The parameter defaults are well-chosen:
page: 1andpageSize: 10are standard pagination defaultssortBy: "lastViewed"andsortOrder: "desc"provide logical initial sorting
64-79: SWR configuration optimized for pagination use case.The enhanced configuration includes appropriate settings:
- Disabled automatic revalidation to prevent unwanted data refreshes during pagination
- Reasonable error retry settings (2 attempts, 5s intervals)
keepPreviousData: truemaintains smooth UX during page transitions
81-90: Return object structure aligns with new API response format.The destructured return provides direct access to:
viewers,pagination,sortingfrom the responsemutatefunction for manual cache updates- Consistent loading state logic
pages/api/teams/[teamId]/viewers/[id]/index.ts (4)
13-59: Good implementation of the duration fetching helper function.This addresses the duplicate code concern from previous reviews. The batch processing and error handling are well-implemented.
Consider documenting why the batch size was increased to 10 instead of the previously suggested 5, as this could impact API rate limits or memory usage.
72-91: Well-structured parameter parsing with good validation.The pagination and sorting parameter validation is comprehensive, with appropriate defaults and performance safeguards (100 item limit).
243-261: Excellent optimization for duration-only requests.The
withDurationparameter provides a clean way to fetch only duration data when needed, reducing response payload and processing time. The error handling gracefully falls back to empty durations.
126-127: Well-designed caching strategy with comprehensive cache keys.The cache key construction includes all parameters that affect the response, and the 10-minute expiry provides a good balance between performance and data freshness. The conditional caching based on the
withDurationparameter is a smart optimization.Also applies to: 294-298
prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql
Outdated
Show resolved
Hide resolved
|
Deployment failed with the following error: |
prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pages/api/teams/[teamId]/viewers/index.ts (1)
32-34: Consider adding validation for edge cases in pagination parameters.While the current validation is good, consider adding checks for negative values and zero to prevent potential issues:
-const currentPage = parseInt(page || "1", 10); -const limit = Math.min(parseInt(pageSize || "10", 10), 100); +const currentPage = Math.max(1, parseInt(page || "1", 10)); +const limit = Math.min(Math.max(1, parseInt(pageSize || "10", 10)), 100);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pages/api/teams/[teamId]/viewers/index.ts(3 hunks)prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql(1 hunks)
🔇 Additional comments (4)
prisma/migrations/20250624042156_viewers_performance_optimization/migration.sql (1)
1-16: LGTM! Well-designed indexes for query optimization.The indexes are strategically placed to support the common query patterns in the viewers API:
- Email prefix search with
text_pattern_ops(addressing previous feedback about using standard b-tree indexes)- Team-scoped filtering and sorting
- View aggregation queries
The partial index on line 15 is particularly well-optimized for filtering views with documents. All indexes properly use
IF NOT EXISTSfor safe reapplication.pages/api/teams/[teamId]/viewers/index.ts (3)
64-77: Excellent security improvement! SQL injection vulnerability resolved.The use of
Prisma.sqltemplate literals with validated inputs effectively addresses the previous SQL injection concerns. The whitelist approach for sort fields and orders provides robust protection against malicious input.
79-104: Well-optimized CTE query design aligned with database indexes.The CTE structure efficiently aggregates view statistics and should perform well with the new indexes from the migration:
teamIdfiltering usesViewer_teamId_*indexes- Email search leverages
Viewer_email_prefix_idx- View aggregation benefits from
View_viewerId_*indexes- Sorting utilizes the appropriate date and composite indexes
The query design demonstrates good understanding of database performance optimization.
150-150: Appropriate caching strategy for viewer data.The 30-second cache with 5-minute stale-while-revalidate is well-balanced for viewer data that changes moderately frequently while maintaining good performance.
mfts
left a comment
mfts
left a comment
There was a problem hiding this comment.
well done! I really like the use of the cache here.
I think there's lots of potential for more like it
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Performance