feat(dashboard): implement server-side pagination for my projects #131
Conversation
- Add pagination query parameters (page, limit) to analytics controller - Implement efficient two-query approach with CTE for Snowflake - Refactor component to use BehaviorSubject + toSignal pattern - Remove manual subscriptions for better memory management - Add loading indicator with signal-based state management - Enable PrimeNG lazy loading mode with pagination controls - Remove verbose comments for cleaner code LFXV2-682 Signed-off-by: Asitha de Silva <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughImplements API-driven, paginated my-projects UI with loading overlay; adds AnalyticsService.getMyProjects, server GET /api/analytics/my-projects endpoint and route, and new shared interfaces for project activity and paginated responses. Duplicate controller implementation observed in diff. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component
participant Service
participant Controller
participant DB
Note over Component,Service: Initial load / pagination flow
User->>Component: Open My Projects
Component->>Component: set loading = true
Component->>Service: getMyProjects(page=1, limit=10)
Service->>Controller: GET /api/analytics/my-projects?page=1&limit=10
Controller->>DB: Query totalProjects and paginated project activity (CTE + aggregates)
DB-->>Controller: ProjectItem[] + totalProjects
Controller-->>Service: UserProjectsResponse
Service-->>Component: Observable<UserProjectsResponse]
Component->>Component: update signals (projectsResponse → projects, totalRecords)
Component->>Component: set loading = false
Component-->>User: Render table page
alt User changes page or rows
User->>Component: onPageChange({first, rows})
Component->>Component: update pagination state
Component->>Service: getMyProjects(newPage, newLimit)
Service->>Controller: GET /api/analytics/my-projects?page=X&limit=Y
Controller->>DB: Query with new pagination
DB-->>Controller: ProjectItem[] + totalProjects
Controller-->>Service: UserProjectsResponse
Service-->>Component: Observable<UserProjectsResponse]
Component->>Component: update display
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (5)packages/shared/src/interfaces/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/lfx-one/src/**/*.html📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (2)apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts (1)
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
🔇 Additional comments (11)
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements server-side pagination for the "My Projects" dashboard to improve performance when handling large datasets. The implementation uses Angular 19 reactive patterns with signals and observables for efficient state management and automatic request cancellation.
Key Changes:
- Backend implements two-query pagination approach with count and paginated data fetch using Snowflake CTEs
- Frontend refactored to use BehaviorSubject + toSignal pattern with automatic request cancellation via switchMap
- Added loading indicators and PrimeNG pagination controls with lazy loading mode
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/analytics-data.interface.ts | Added UserProjectActivityRow and UserProjectsResponse interfaces for paginated project data |
| apps/lfx-one/src/server/routes/analytics.route.ts | Added /my-projects route endpoint |
| apps/lfx-one/src/server/controllers/analytics.controller.ts | Implemented getMyProjects controller with pagination logic and CTE-based queries |
| apps/lfx-one/src/app/shared/services/analytics.service.ts | Added getMyProjects service method with pagination parameters |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts | Refactored to use reactive patterns with BehaviorSubject, toSignal, and computed signals |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html | Added loading overlay and PrimeNG pagination controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts
Outdated
Show resolved
Hide resolved
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
120-124: Replace DEV schema references with production ANALYTICS schemaDEV schema references remain in the codebase and must be replaced before merging:
- Line 48:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.ACTIVE_WEEKS_STREAK- Line 246:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM.PROJECT_CODE_ACTIVITY- Line 274:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM.PROJECT_CODE_ACTIVITY- Line 288:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM.PROJECT_CODE_ACTIVITYUpdate all occurrences to use
ANALYTICS.PLATINUM_LFX_ONE(matching the pattern shown at lines 120-124).
🧹 Nitpick comments (7)
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
299-317: UI-coupled DTO and placeholder fieldsServer returns UI-oriented ProjectItem with placeholder logo/role/affiliations/status. Prefer a backend DTO (e.g., UserProjectItem) and let the client adapt to view needs, or populate real values if available.
329-333: Log hygiene: avoid PII; include request idGood metrics. Ensure no PII beyond counts is logged; include a request correlation id if available.
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
70-87: Keep type annotations if using HttpParams refactorThe component pagination correctly converts to 1-based page/limit. The service implementation works, and HttpParams usage would be a valid optional improvement for consistent parameter encoding—but the suggested diff removes type annotations, which should be preserved.
public getMyProjects(page: number = 1, limit: number = 10): Observable<UserProjectsResponse> { - const params = { page: page.toString(), limit: limit.toString() }; + const params = new HttpParams().set('page', String(page)).set('limit', String(limit)); return this.http.get<UserProjectsResponse>('/api/analytics/my-projects', { params }).pipe(Add
HttpParamsto the imports from@angular/common/httpif adopting this change.apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts (4)
9-11: Prep imports for reliability fix.If you adopt the error-handling/loader refactor below, add catchError and of to these imports (RxJS 7+ re-exports operators from 'rxjs'). Otherwise, current imports are fine.
42-42: Single source of truth for page size.rows duplicates paginationState.limit. Either derive rows from the subject/signal or keep only one state to avoid drift.
- public readonly rows = signal(10); + public readonly rows = signal(10); // If kept, always update together with paginationState
54-61: Defensive fallback for activity arrays (optional).If ProjectItem.codeActivities/nonCodeActivities can be null/undefined, guard to avoid Chart.js errors.
- return response.data.map((project) => ({ + return response.data.map((project) => ({ ...project, - codeActivitiesChartData: this.createChartData(project.codeActivities, '#009AFF', 'rgba(0, 154, 255, 0.1)'), - nonCodeActivitiesChartData: this.createChartData(project.nonCodeActivities, '#10b981', 'rgba(16, 185, 129, 0.1)'), + codeActivitiesChartData: this.createChartData(project.codeActivities ?? [], '#009AFF', 'rgba(0, 154, 255, 0.1)'), + nonCodeActivitiesChartData: this.createChartData(project.nonCodeActivities ?? [], '#10b981', 'rgba(16, 185, 129, 0.1)'), }));Confirm these fields are always arrays in the API; if so, ignore.
63-65: Remove redundant alias or document intent.paginatedProjects just proxies projects. Drop it or leave a comment if the template relies on it for future slicing.
- public readonly paginatedProjects = computed<ProjectItemWithCharts[]>(() => this.projects()); + // Alias retained for template compatibility; remove when not needed. + public readonly paginatedProjects = computed<ProjectItemWithCharts[]>(() => this.projects());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts(3 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(4 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(1 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/analytics-data.interface.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
packages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
packages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
🧬 Code graph analysis (3)
packages/shared/src/interfaces/analytics-data.interface.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
ProjectItem(368-383)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (1)
UserProjectsResponse(168-178)
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
UserProjectActivityRow(128-163)UserProjectsResponse(168-178)packages/shared/src/interfaces/components.interface.ts (1)
ProjectItem(368-383)
🔇 Additional comments (8)
apps/lfx-one/src/server/routes/analytics.route.ts (1)
16-16: Route addition looks goodEndpoint name aligns with client service and router conventions.
packages/shared/src/interfaces/analytics-data.interface.ts (1)
4-5: Interfaces added correctly and placed in sharedShapes are clear and documented; import is type-only.
Also applies to: 124-178
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
277-278: Confirm Snowflake bind support for LIMIT/OFFSETSome drivers require numeric literals or CAST for LIMIT/OFFSET. If binds fail, inline sanitized integers or use CAST.
Try:
LIMIT CAST(? AS INTEGER) OFFSET CAST(? AS INTEGER)If not supported, build literals from validated numbers (page>=1, 1<=limit<=100).
Also applies to: 294-295
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html (1)
76-86: No issues found—chart bindings properly handledThe component correctly extends
ProjectItemwith a localProjectItemWithChartsinterface that explicitly typescodeActivitiesChartDataandnonCodeActivitiesChartDataasChartData<'line'>. The mapping transforms the basecodeActivitiesandnonCodeActivitiesarrays into chart datasets viathis.createChartData(). This approach appropriately separates the shared data model from the component's view model.apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts (4)
5-7: Solid move to Angular 19 signals.Adoption of inject/signal/toSignal looks good and aligns with reactive best practices.
32-40: Chart options look fine.No issues. Consider keeping options small to minimize re-renders; current config is lean.
28-31: Loader flicker issue is valid; catchError claim needs clarification.The core concern about
finalize()causing a race condition is accurate—whenswitchMapcancels the previous request,finalize()setsloading=falsebefore the new request starts, creating a flicker.However, the review comment's claim about missing error handling is misleading.
AnalyticsService.getMyProjects()already implementscatchErrorat lines 79-84, returning the exact fallback response{ data: [], totalProjects: 0 }. Errors won't causetoSignalto throw.The refactoring suggestion (using a
pendingRequestscounter +computedloading signal) is still sound for eliminating the flicker. The suggested catchError in the component'sswitchMapwould be defensive but isn't strictly required given the service-level error handling.-import { BehaviorSubject, finalize, switchMap, tap } from 'rxjs'; +import { BehaviorSubject, finalize, of, switchMap, tap } from 'rxjs';- protected readonly loading = signal(true); + private readonly pendingRequests = signal(0); + public readonly loading = computed(() => this.pendingRequests() > 0);- private readonly projectsResponse = toSignal( + private readonly projectsResponse = toSignal<UserProjectsResponse>( this.paginationState$.pipe( - tap(() => this.loading.set(true)), - switchMap(({ page, limit }) => this.analyticsService.getMyProjects(page, limit).pipe(finalize(() => this.loading.set(false)))) + tap(() => this.pendingRequests.update(n => n + 1)), + switchMap(({ page, limit }) => + this.analyticsService.getMyProjects(page, limit).pipe( + finalize(() => this.pendingRequests.update(n => Math.max(0, n - 1))) + ) + ) ), { initialValue: { data: [], totalProjects: 0 }, } );-import type { ProjectItem } from '@lfx-one/shared/interfaces'; +import type { ProjectItem, UserProjectsResponse } from '@lfx-one/shared/interfaces';Apply the pending-requests counter pattern to fix the flicker at lines 28–31 and 44–52.
66-70: I'll verify the correct PrimeNG type and backend pagination expectations.Now let me check the component file to see how the pagination data is sent to the backend:
Let me check the component file to see the backend API calls and verify the page number expectations:
Use
TableLazyLoadEventfrom 'primeng/table' for the pagination event type.The review comment suggests two options (PaginatorState or TableLazyLoadEvent), but only
TableLazyLoadEventshould be imported from 'primeng/table'. Since the template uses(onLazyLoad)="onPageChange($event)"on the p-table component, the event isTableLazyLoadEvent, not a paginator event.The page calculation (
Math.floor(event.first / event.rows) + 1) follows the common formula offset = (page - 1) * pageSize, which is correct for 1-based pagination. However, verify that your backend API endpoint expects 1-based page numbers (not 0-based) to ensure the calculation aligns with backend expectations.Replace the inline type with:
import type { TableLazyLoadEvent } from 'primeng/table'; public onPageChange(event: TableLazyLoadEvent): void { const page = Math.floor(event.first / event.rows) + 1; this.rows.set(event.rows); this.paginationState$.next({ page, limit: event.rows }); }
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
Show resolved
Hide resolved
- Add PROJECT_ID as tie-breaker in ORDER BY for deterministic pagination - Import and use PrimeNG LazyLoadEvent type instead of inline object - Add null coalescing operators for safer LazyLoadEvent property access - Move ProjectItemWithCharts interface to shared package - Remove redundant paginatedProjects computed signal - Use projects() signal directly in template LFXV2-682 Signed-off-by: Asitha de Silva <[email protected]>
Summary
Implements server-side pagination for the My Projects dashboard to improve performance and loading times when dealing with large datasets.
JIRA
LFXV2-682
Changes Made
Backend
Frontend Service
getMyProjects()to accept page and limit parametersComponent
Template
Technical Details
Performance Benefits
Test Plan