feat(dashboards): refactor my-projects to use user contribution data#196
feat(dashboards): refactor my-projects to use user contribution data#196
Conversation
- Remove pagination from my-projects component, add empty state - Update analytics service and controller to use LF username - Refactor user service to query USER_PROJECT_CONTRIBUTIONS_DAILY table - Update interfaces with new fields (PROJECT_LOGO, IS_MAINTAINER, AFFILIATION) - Fix board member dashboard available accounts binding (LFXV2-874) - Update RSVP disabled message for legacy meetings (LFXV2-875) - Hide data copilot sparkle icon temporarily (LFXV2-876) LFXV2-873 LFXV2-874 LFXV2-875 LFXV2-876 Signed-off-by: Asitha de Silva <asithade@gmail.com>
WalkthroughThis pull request refactors the projects data retrieval from a pagination-based approach to an authentication-based username lookup. It removes lazy-loading pagination from the projects table UI, simplifies the analytics service API, updates backend controllers and services to use authenticated username parameters, and modifies related interface definitions and component properties. Changes
Sequence DiagramsequenceDiagram
participant Client as Client / Component
participant Service as Analytics Service
participant Controller as Analytics Controller
participant UserSvc as User Service
participant DB as Snowflake DB
Client->>Service: getMyProjects()
Service->>Controller: GET /api/analytics/my-projects
Controller->>Controller: Extract lfUsername from auth
alt Username present
Controller->>UserSvc: getMyProjects(lfUsername)
UserSvc->>DB: Query projects + affiliations + activities<br/>filtered by LF_USERNAME
DB-->>UserSvc: Result rows with contribution data
UserSvc->>UserSvc: Group by PROJECT_ID<br/>Parse affiliations & role
UserSvc-->>Controller: UserProjectsResponse<br/>(projects[], totalProjects)
Controller-->>Service: 200 OK + response
else Username missing
Controller-->>Service: 401 AuthenticationError
end
Service-->>Client: Observable<UserProjectsResponse>
Client->>Client: Update projects signal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
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/app/modules/dashboards/components/my-projects/my-projects.component.ts (1)
37-45: Incorrect loading state logic intapoperator.The
tap(() => this.loading.set(true))executes when the observable emits a value, meaning it sets loading to true after data has arrived. Sinceloadingis already initialized totrueon line 25, thistapis both redundant and semantically incorrect.Apply this diff to fix the loading state flow:
private readonly projectsResponse = toSignal( this.analyticsService.getMyProjects().pipe( - tap(() => this.loading.set(true)), finalize(() => this.loading.set(false)) ), { initialValue: { data: [], totalProjects: 0 }, } );
🧹 Nitpick comments (3)
apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html (1)
264-272: Consider adding data-testid attributes to RSVP components for better test coverage.The
lfx-meeting-rsvp-details(line 264) andlfx-rsvp-button-group(line 276) components lack data-testid attributes. Given the file's existing convention and the fact that these components now have disabled states and custom messages worth testing, adding identifiers likedata-testid="rsvp-details-card"anddata-testid="rsvp-button-group"would improve test reliability and consistency with the rest of the component.Also applies to: 276-281
apps/lfx-one/src/server/services/user.service.ts (2)
382-421: Consider adding a limit for very active users.The query returns all projects and all daily activity records for a user without any limit. For users with many project contributions over extended periods, this could result in large result sets impacting performance and memory.
Consider adding a date range filter (e.g., last 90 days) or limiting the number of projects returned:
-- In UserProjects CTE, add ORDER BY and LIMIT ORDER BY PROJECT_NAME, PROJECT_ID LIMIT 50 -- Or in DailyActivities CTE, add date filter WHERE LF_USERNAME = ? AND ACTIVITY_DATE >= DATEADD(DAY, -90, CURRENT_DATE())
379-379: Missing structured logging per backend service guidelines.The method lacks logging for operation tracking. Per coding guidelines, backend services should log INFO level for successful data retrieval and DEBUG level for internal operations.
The method needs a
req: Requestparameter for logging, or useserverLoggerdirectly. Example additions:public async getMyProjects(req: Request, lfUsername: string): Promise<UserProjectsResponse> { req.log.debug({ lfUsername }, 'Fetching user project contributions'); // ... query execution ... req.log.info( { projectCount: projects.length, lfUsername }, 'User project contributions retrieved successfully' ); return { data: projects, totalProjects: projects.length }; }Based on coding guidelines for
apps/**/**/server/**/*.{ts,js}.
📜 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 (11)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts(2 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html(2 hunks)apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html(1 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(1 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(2 hunks)apps/lfx-one/src/server/services/user.service.ts(2 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(2 hunks)packages/shared/src/interfaces/components.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.htmlapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.htmlapps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.tspackages/shared/src/interfaces/components.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.ts
apps/**/**/server/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/**/server/**/*.{ts,js}: Use Pino for structured JSON logs with sensitive data redaction in all backend services
Always use err field for errors to leverage Pino's error serializer - correct: req.log.error({ err: error, ...metadata }, 'message')
Log INFO level for business operation completions (created, updated, deleted) and successful data retrieval
Log WARN level for error conditions leading to exceptions and data quality issues
Log DEBUG level for internal operations, preparation steps, and intent statements
Log ERROR level for system failures, unhandled exceptions, and critical errors
Files:
apps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/user.service.ts
apps/**/**/server/**/*controller*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
All controller functions must use Logger helper methods: Logger.start(), Logger.success(), Logger.error(), Logger.warning(), Logger.validation()
Files:
apps/lfx-one/src/server/controllers/analytics.controller.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-one/shared package
Files:
packages/shared/src/interfaces/analytics-data.interface.tspackages/shared/src/interfaces/components.interface.ts
🧬 Code graph analysis (4)
apps/lfx-one/src/server/controllers/analytics.controller.ts (3)
apps/lfx-one/src/server/helpers/logger.ts (1)
Logger(10-129)apps/lfx-one/src/server/utils/auth-helper.ts (1)
getUsernameFromAuth(10-22)apps/lfx-one/src/server/errors/index.ts (1)
AuthenticationError(5-5)
apps/lfx-one/src/server/services/user.service.ts (2)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
UserProjectsResponse(178-188)UserProjectContributionRow(128-173)packages/shared/src/interfaces/components.interface.ts (1)
ProjectItem(420-435)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/constants/accounts.constants.ts (1)
ACCOUNTS(10-31)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (1)
UserProjectsResponse(178-188)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (13)
apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html (1)
271-271: Message text updated correctly per LFXV2-875 requirements.Both RSVP component
disabledMessageattributes have been updated with the correct text as specified in the PR objective. The message change—from "Meetings created outside of LFX One do not have RSVP functionality" to "RSVP functionality will soon be available for all upcoming LFX meetings visible to you"—improves messaging for legacy meetings.Also applies to: 280-280
apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html (1)
14-14: LGTM! Temporary icon removal as per requirements.The sparkle icon is appropriately commented out rather than deleted, making it easy to re-enable in the future per LFXV2-876 requirements.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html (1)
19-19: LGTM! Template binding correctly updated to match TypeScript changes.The binding now uses
availableAccountsas a property instead ofavailableAccounts()as a Signal method call, which correctly aligns with the TypeScript component change whereavailableAccountsis now the staticACCOUNTSconstant.apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html (2)
19-19: Good simplification! Consider monitoring performance with large project lists.The removal of pagination simplifies the implementation and aligns with the backend changes. However, if users have a large number of projects, loading all at once could impact performance.
Consider monitoring the performance impact for users with many projects. If needed, you could implement client-side virtual scrolling or re-introduce server-side pagination in the future.
81-90: LGTM! Empty state properly implemented with good UX.The empty state template is well-structured with:
- Proper colspan for full-width display
- Icon and message for clear communication
- Correct
data-testidattribute per coding guidelinesapps/lfx-one/src/server/controllers/analytics.controller.ts (1)
126-133: LGTM! Authentication-based filtering properly implemented.The change from pagination parameters to username-based filtering is well-implemented:
- Uses
getUsernameFromAuthhelper for consistent auth handling- Properly throws
AuthenticationErrorwhen username is missing- Clear error message for debugging
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
96-109: LGTM! Pagination removed cleanly with proper error handling.The method signature and HTTP call have been correctly updated to remove pagination support, aligning with the backend changes. Error handling remains appropriate with a proper fallback response structure.
packages/shared/src/interfaces/analytics-data.interface.ts (1)
125-173: All references to the old interface name have been successfully updated.The interface has been renamed from
UserProjectActivityRowtoUserProjectContributionRowwith the following changes:
- Added:
PROJECT_LOGO,IS_MAINTAINER,AFFILIATION- Removed:
DAILY_TOTAL_ACTIVITIESVerification confirms no references to the old interface name remain in the codebase. The new interface is properly exported through the barrel export in
packages/shared/src/interfaces/index.ts, and the migration appears complete with no breaking references outstanding.apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
8-8: No actionable changes needed. The import and usage are correct.The component intentionally uses the static
ACCOUNTSconstant for the display list (line 39), while line 55 correctly usesthis.accountContextService.availableAccountsto retrieve the account for selection. The service'savailableAccountsgetter is a wrapper that returns either user-specific organizations from committee memberships or defaults to the staticACCOUNTSconstant, ensuring the lookup respects the user's access level. This is a sound architectural pattern.packages/shared/src/interfaces/components.interface.ts (1)
420-435: All ProjectItem consumers have been updated to handle the interface changes.The breaking changes have been properly addressed:
slugfield added: user.service.ts correctly populatesslug: row.PROJECT_SLUGroletype narrowed to'Maintainer' | 'Contributor': user.service.ts correctly assignsrole: row.IS_MAINTAINER ? 'Maintainer' : 'Contributor'- All other required fields: affiliations, codeActivities, and nonCodeActivities are properly initialized
Verified consumers:
- user.service.ts: Creates ProjectItem objects with complete and correct field mapping
- my-projects.component.ts: Successfully spreads project data and extends with chart configurations
- analytics-data.interface.ts: Properly uses ProjectItem[] in response type
Note: The claim about a removed
statusfield could not be verified in the current interface or any consumer code.apps/lfx-one/src/server/services/user.service.ts (3)
21-21: LGTM!Import updated to reflect the new
UserProjectContributionRowinterface which includes the additional fields (PROJECT_LOGO,IS_MAINTAINER,AFFILIATION) required by the refactored query.
373-379: LGTM on the method signature change.The transition from pagination-based
(page, limit)to username-based(lfUsername)aligns with the PR objective to fetch user-specific contributions directly from OIDC context.
428-451: LGTM on result processing.The processing logic correctly handles:
- Parsing comma-separated affiliations with empty string filtering
- Converting null
PROJECT_LOGOto undefined for the optional interface property- Using truthy evaluation for
IS_MAINTAINERto handle Snowflake's boolean representation- Safely adding activity data only when
ACTIVITY_DATEexists
There was a problem hiding this comment.
Pull request overview
This PR refactors the my-projects dashboard to fetch user-specific project contributions from the Snowflake USER_PROJECT_CONTRIBUTIONS_DAILY table instead of the previous PROJECT_CODE_ACTIVITY table. The refactoring removes client-side pagination in favor of loading all projects at once, adds an empty state when no projects are found, and updates the data model to include new fields like project slug, logo, and user role (Maintainer/Contributor). Additionally, the PR includes several minor fixes: correcting the board member dashboard's available accounts binding to use a constant array, updating RSVP disabled messages for legacy meetings to be more user-friendly, and temporarily hiding the data copilot sparkle icon.
- Refactored my-projects to query USER_PROJECT_CONTRIBUTIONS_DAILY table filtered by LF username from OIDC context
- Removed client-side pagination and added empty state template for better UX
- Updated interfaces to reflect new data structure with slug, logo, role, and improved documentation
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/components.interface.ts | Updated ProjectItem interface to include slug field and changed role to union type ('Maintainer' | 'Contributor'), improved documentation |
| packages/shared/src/interfaces/analytics-data.interface.ts | Renamed interface from UserProjectActivityRow to UserProjectContributionRow, added PROJECT_LOGO, IS_MAINTAINER, and AFFILIATION fields |
| apps/lfx-one/src/server/services/user.service.ts | Refactored getMyProjects to query USER_PROJECT_CONTRIBUTIONS_DAILY with LF username filter, removed pagination, added affiliation aggregation and maintainer role logic |
| apps/lfx-one/src/server/controllers/analytics.controller.ts | Removed pagination parameters, added authentication check to get LF username from OIDC context |
| apps/lfx-one/src/app/shared/services/analytics.service.ts | Removed pagination parameters from getMyProjects method |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.ts | Removed pagination state management and related imports, simplified data fetching |
| apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html | Removed pagination attributes from table, added empty state template |
| apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts | Changed availableAccounts from Signal to constant ACCOUNTS array |
| apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html | Updated template binding to use non-callable ACCOUNTS constant |
| apps/lfx-one/src/app/shared/components/data-copilot/data-copilot.component.html | Commented out sparkle icon |
| apps/lfx-one/src/app/modules/meetings/components/meeting-card/meeting-card.component.html | Updated RSVP disabled message to be more user-friendly |
Comments suppressed due to low confidence (1)
packages/shared/src/interfaces/analytics-data.interface.ts:172
- The
DAILY_CODE_ACTIVITIESandDAILY_NON_CODE_ACTIVITIESfields should be nullable (number | null) since the SQL query usesLEFT JOIN DailyActivities aat line 419 of user.service.ts. When projects have no activity data, these fields will be null. The code at line 448 of user.service.ts already handles this with|| 0, but the interface type doesn't reflect the nullable nature.
/**
* Code-related activities for this date
*/
DAILY_CODE_ACTIVITIES: number;
/**
* Non-code-related activities for this date
*/
DAILY_NON_CODE_ACTIVITIES: number;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Changes
JIRA Tickets