feat(dashboards): integrate snowflake analytics for board member metrics #140
Conversation
- Add 5 new API endpoints for organization-level analytics data - Integrate real Snowflake data into organization involvement metrics - Refactor board member dashboard to use reactive form pattern - Add support for projects participating, total commits, certified employees, board meeting attendance, and event sponsorships metrics - Update organization involvement component to display live analytics data - Add multi-currency formatting for event sponsorships - Create TypeScript interfaces in shared package for type safety LFXV2-701 Signed-off-by: Asitha de Silva <asithade@gmail.com>
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
|
Caution Review failedThe pull request is closed. 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. WalkthroughConsolidates analytics endpoints and interfaces, adds Snowflake-backed organization/user analytics services, removes legacy analytics routes, updates server controllers to delegate to services, introduces ANALYTICS_DEFAULTS, and refactors frontend dashboard components and metrics to use new consolidated signals, CurrencyPipe, and reactive FormGroup subscription. Changes
Sequence Diagram(s)sequenceDiagram
%% Styling: subtle colored notes to highlight new/changed interactions
participant UI as Frontend Component
participant Signals as Component Signals
participant Client as AnalyticsService (HTTP)
participant API as Analytics Controller
participant Service as OrganizationService / UserService
participant DB as Snowflake
UI->>Signals: init & subscribe
Signals->>Client: request consolidated data (e.g., board-member-dashboard)
Client->>API: GET /api/analytics/board-member-dashboard?accountId=...
API->>Service: delegate call (uses ANALYTICS_DEFAULTS if missing)
Service->>DB: execute Snowflake query(ies)
alt success
DB-->>Service: rows
Service-->>API: mapped consolidated response
API-->>Client: JSON response
Client-->>Signals: emit consolidated signals
Signals-->>UI: update metrics (CurrencyPipe transforms applied)
else error/empty
Service-->>API: fallback default response
API-->>Client: fallback JSON
Client-->>Signals: emit fallback
Signals-->>UI: render fallback values
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds five new analytics endpoints to provide organization-level metrics for a board member dashboard, including projects participating, total commits, certified employees, board meeting attendance, and event sponsorships.
- Adds five new Snowflake query interfaces for member dashboard metrics (projects, commits, certifications, meetings, sponsorships)
- Implements five new API endpoints in the analytics controller with hardcoded defaults for accountId, segmentId, and projectId
- Creates corresponding frontend service methods and component integration with real-time data transformations
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/analytics-data.interface.ts | Adds 15 new TypeScript interfaces for Snowflake queries and API responses |
| apps/lfx-one/src/server/routes/analytics.route.ts | Registers five new GET routes for organization analytics endpoints |
| apps/lfx-one/src/server/controllers/analytics.controller.ts | Implements five new controller methods with Snowflake queries and error handling |
| apps/lfx-one/src/app/shared/services/analytics.service.ts | Adds five frontend service methods with error handling fallbacks |
| apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts | Integrates new data streams, adds CurrencyPipe provider, and transforms data for display with formatting |
| apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html | Removes number pipe formatting from metric values to support pre-formatted strings |
| apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts | Refactors form handling to use reactive subscription pattern instead of event handler |
| apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html | Updates template to use refactored form name and removes onChange handler |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 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 (8)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html(2 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/organization-involvement/organization-involvement.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(9 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(2 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(1 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.html
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.htmlapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.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:
apps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.ts
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
🧠 Learnings (1)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
🧬 Code graph analysis (4)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (5)
OrganizationProjectsParticipatingResponse(433-448)OrganizationTotalCommitsResponse(475-490)OrganizationCertifiedEmployeesResponse(522-537)OrganizationBoardMeetingAttendanceResponse(579-604)OrganizationEventSponsorshipsResponse(703-718)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
packages/shared/src/interfaces/account.interface.ts (1)
Account(8-13)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (4)
packages/shared/src/constants/organization-involvement.constants.ts (3)
CONTRIBUTIONS_METRICS(66-87)IMPACT_METRICS(93-114)PRIMARY_INVOLVEMENT_METRICS(20-60)packages/shared/src/interfaces/analytics-data.interface.ts (1)
OrganizationEventSponsorshipsResponse(703-718)packages/shared/src/interfaces/dashboard.interface.ts (2)
PrimaryInvolvementMetric(63-88)OrganizationInvolvementMetricWithChart(124-167)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (11)
MemberDashboardProjectsParticipatingRow(412-427)OrganizationProjectsParticipatingResponse(433-448)MemberDashboardTotalCommitsRow(454-469)OrganizationTotalCommitsResponse(475-490)MemberDashboardCertifiedEmployeesRow(496-516)OrganizationCertifiedEmployeesResponse(522-537)MemberDashboardBoardMeetingAttendanceRow(543-573)OrganizationBoardMeetingAttendanceResponse(579-604)OrganizationEventSponsorshipsAggregateRow(656-671)OrganizationEventSponsorshipsEventCountRow(677-682)OrganizationEventSponsorshipsResponse(703-718)
🔇 Additional comments (1)
packages/shared/src/interfaces/analytics-data.interface.ts (1)
408-718: LGTM! Well-structured interface definitions with comprehensive documentation.The new analytics interfaces follow the established patterns in this file consistently:
- Snowflake row interfaces use ALL_CAPS naming convention
- API response interfaces use camelCase properties
- All fields are properly typed and documented
- The separation between raw Snowflake rows and API responses is clear and maintainable
The omission of
projectIdin the response interfaces (despitePROJECT_IDbeing present in some row interfaces likeMemberDashboardCertifiedEmployeesRow,MemberDashboardBoardMeetingAttendanceRow, andMemberDashboardEventSponsorshipRow) is consistent with other organization-level aggregated responses in this file, indicating the data is aggregated across projects at the backend level.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts
Show resolved
Hide resolved
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Outdated
Show resolved
Hide resolved
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-140.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
- Extract hardcoded accountId, projectId, and segmentId to shared constants - Create ANALYTICS_DEFAULTS constant in shared package for temporary values - Replace 16 inline hardcoded values in analytics controller with constants - Fix InvalidPipeArgument crash when Snowflake returns null/empty currency codes - Add filtering in transformEventSponsorship to handle imperfect data gracefully - Dashboard now resilient to legacy/draft sponsorship records with missing currency LFXV2-701 Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/constants/index.ts (1)
23-23: Verify barrel export compliance with coding guidelines.The coding guideline states "Do not use barrel exports; always use direct imports for standalone components." While this export is for constants rather than components, clarify whether the guideline applies only to component barrels or to all index.ts files.
📜 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 (4)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(9 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(7 hunks)packages/shared/src/constants/analytics.constants.ts(1 hunks)packages/shared/src/constants/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not use barrel exports; always use direct imports for standalone components
Files:
packages/shared/src/constants/index.ts
packages/shared/src/constants/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all reusable constants in the shared package at packages/shared/src/constants
Files:
packages/shared/src/constants/index.tspackages/shared/src/constants/analytics.constants.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/constants/index.tspackages/shared/src/constants/analytics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.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/constants/index.tspackages/shared/src/constants/analytics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
packages/shared/src/constants/index.tspackages/shared/src/constants/analytics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/server/controllers/analytics.controller.ts
🧠 Learnings (2)
📚 Learning: 2025-09-16T03:32:46.518Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T03:32:46.518Z
Learning: Applies to packages/shared/src/constants/**/*.ts : Place all reusable constants in the shared package at packages/shared/src/constants
Applied to files:
packages/shared/src/constants/index.ts
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (4)
packages/shared/src/constants/organization-involvement.constants.ts (3)
CONTRIBUTIONS_METRICS(66-87)IMPACT_METRICS(93-114)PRIMARY_INVOLVEMENT_METRICS(20-60)packages/shared/src/interfaces/analytics-data.interface.ts (1)
OrganizationEventSponsorshipsResponse(703-718)packages/shared/src/interfaces/dashboard.interface.ts (2)
PrimaryInvolvementMetric(63-88)OrganizationInvolvementMetricWithChart(124-167)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)
apps/lfx-one/src/server/controllers/analytics.controller.ts (3)
packages/shared/src/constants/analytics.constants.ts (1)
ANALYTICS_DEFAULTS(8-26)packages/shared/src/interfaces/analytics-data.interface.ts (11)
MemberDashboardProjectsParticipatingRow(412-427)OrganizationProjectsParticipatingResponse(433-448)MemberDashboardTotalCommitsRow(454-469)OrganizationTotalCommitsResponse(475-490)MemberDashboardCertifiedEmployeesRow(496-516)OrganizationCertifiedEmployeesResponse(522-537)MemberDashboardBoardMeetingAttendanceRow(543-573)OrganizationBoardMeetingAttendanceResponse(579-604)OrganizationEventSponsorshipsAggregateRow(656-671)OrganizationEventSponsorshipsEventCountRow(677-682)OrganizationEventSponsorshipsResponse(703-718)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
⏰ 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: build-and-push
🔇 Additional comments (13)
packages/shared/src/constants/analytics.constants.ts (1)
8-26: LGTM! Temporary defaults well-documented.The constant structure is clean with clear documentation indicating these are temporary values. The TODO comment appropriately flags the need to replace with dynamic values from user context.
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (6)
4-35: LGTM! CurrencyPipe integration follows Angular best practices.The pipe is correctly imported, provided, and injected for programmatic currency formatting.
97-152: LGTM! New data signals follow established patterns.All five signals use consistent reactive composition with
toSignalandswitchMap, matching the existing implementation pattern. Initial values align with their respective response interfaces.
189-222: LGTM! Contributions metrics correctly integrate live data.The computed property properly transforms Snowflake data into display values with appropriate formatting (
toLocaleStringfor commits,toFixedfor percentages).
224-263: LGTM! Impact metrics display enriched certification details.The combined display format for certified employees (showing both certifications count and employee count) provides valuable context.
336-350: LGTM! Empty membership state handling is appropriate.Treating "no membership" as a connected state (
isConnected: true) correctly distinguishes between successfully queried data with no membership versus a data connection failure.
372-402: LGTM! Currency code filtering prevents pipe errors.The implementation correctly addresses the previous review concern by filtering out null/empty currency codes (line 375) and null pipe results (line 377) before building the display string. This ensures resilience against imperfect Snowflake data.
apps/lfx-one/src/server/controllers/analytics.controller.ts (6)
4-37: LGTM! Centralized defaults improve maintainability.The import of
ANALYTICS_DEFAULTSreplaces scattered hardcoded fallback values, addressing previous maintainability concerns.
633-681: LGTM! Projects participating endpoint follows established patterns.The implementation correctly queries Snowflake with both
accountIdandsegmentIdparameters, handles empty results withResourceNotFoundError, and provides comprehensive logging.
683-731: LGTM! Total commits endpoint implementation is solid.Consistent error handling, parameterized queries, and logging align with the existing codebase standards.
733-782: LGTM! Certified employees endpoint handles dual metrics correctly.The endpoint properly returns both
certificationscount andcertifiedEmployeescount, providing the granular data needed for the frontend display.
784-845: LGTM! Attendance percentage calculation includes divide-by-zero protection.The SQL
CASEstatement (lines 803-806) correctly guards against division by zero whenTOTAL_MEETINGSis zero, returning 0 instead.
847-917: LGTM! Dual-query approach correctly handles multi-currency sponsorships.The two-query strategy (lines 862-872 for per-currency sums, lines 883-888 for total events) properly addresses the distinct aggregation requirements. The frontend currency filtering complements the backend implementation.
Refactored analytics.controller.ts to follow the established Controller → Service → Data Source pattern used by other controllers in the codebase. Changes: - Moved 4 user analytics methods to UserService (active weeks streak, pull requests, code commits, projects) - Moved 10 organization analytics methods to OrganizationService (maintainers, membership tier, contributors, events, technical committee, certifications, board meetings, sponsorships) - Removed inline SQL queries and business logic from analytics.controller.ts - Controller now only handles HTTP concerns (validation, logging, response) - Reduced controller complexity from 930 lines to ~400 lines - Removed all unused interface imports from controller LFXV2-701 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/lfx-one/src/server/utils/lock-manager.ts (2)
11-11: Good refactor to type-only import.Changing to
import typeis correct sinceBindis only used in type annotations (lines 74, 208) and not at runtime. This reduces the bundle size and follows TypeScript best practices.
31-43: Consider retaining initialization logging for observability.The removal of the initialization log creates an asymmetry with the shutdown log (line 113) and may impact debugging during service startup. While the change aligns with the PR's goal of reducing runtime logging, a simple initialization log (perhaps at debug level) could aid troubleshooting without significant overhead.
apps/lfx-one/src/server/services/user.service.ts (1)
320-355: Consider extracting shared logicThe
getCodeCommitsandgetPullRequestsMergedmethods share nearly identical structure. Consider extracting a generic helper method to reduce duplication:private async getUserActivityData<T extends { ACTIVITY_DATE: string; DAILY_COUNT: number; TOTAL_COUNT: number }>( userEmail: string, tableName: string, errorMessage: string, operation: string ): Promise<{ data: T[]; total: number; totalDays: number }> { const query = ` SELECT ACTIVITY_DATE, DAILY_COUNT, SUM(DAILY_COUNT) OVER () as TOTAL_COUNT FROM ${tableName} WHERE EMAIL = ? AND ACTIVITY_DATE >= DATEADD(DAY, -30, CURRENT_DATE()) ORDER BY ACTIVITY_DATE ASC `; const result = await this.snowflakeService.execute<T>(query, [userEmail]); if (result.rows.length === 0) { throw new ResourceNotFoundError(errorMessage, userEmail, { operation }); } return { data: result.rows, total: result.rows[0].TOTAL_COUNT, totalDays: result.rows.length, }; }apps/lfx-one/src/server/services/organization.service.ts (2)
103-109: Consider explicit column selectionUsing
SELECT *can lead to performance issues and breaks if the table schema changes. Consider explicitly listing required columns.- SELECT * + SELECT + MEMBERSHIP_TIER, + CURRENT_MEMBERSHIP_START_DATE, + CURRENT_MEMBERSHIP_END_DATE, + MEMBERSHIP_PRICE, + MEMBERSHIP_STATUS, + ACCOUNT_ID, + PROJECT_ID FROM ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.MEMBER_DASHBOARD_MEMBERSHIP_TIER
235-297: Consider extracting shared pattern
getProjectsParticipatingandgetTotalCommitsshare nearly identical structure. Consider a generic helper to reduce duplication:private async getOrganizationMetricBySegment<TRow, TResponse>( accountId: string, segmentId: string, tableName: string, columnName: string, operation: string, errorMessage: string, mapFn: (row: TRow, accountId: string, segmentId: string) => TResponse ): Promise<TResponse> { const query = ` SELECT ACCOUNT_ID, SEGMENT_ID, ${columnName} FROM ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE.${tableName} WHERE ACCOUNT_ID = ? AND SEGMENT_ID = ? LIMIT 1 `; const result = await this.snowflakeService.execute<TRow>(query, [accountId, segmentId]); if (result.rows.length === 0) { throw new ResourceNotFoundError(errorMessage, accountId, { operation }); } return mapFn(result.rows[0], accountId, segmentId); }
📜 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 (5)
apps/lfx-one/src/server/controllers/analytics.controller.ts(11 hunks)apps/lfx-one/src/server/services/organization.service.ts(2 hunks)apps/lfx-one/src/server/services/snowflake.service.ts(0 hunks)apps/lfx-one/src/server/services/user.service.ts(2 hunks)apps/lfx-one/src/server/utils/lock-manager.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/lfx-one/src/server/services/snowflake.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:
apps/lfx-one/src/server/utils/lock-manager.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/server/utils/lock-manager.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/utils/lock-manager.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.ts
🧬 Code graph analysis (3)
apps/lfx-one/src/server/services/user.service.ts (5)
apps/lfx-one/src/server/services/nats.service.ts (1)
NatsService(13-133)apps/lfx-one/src/server/services/snowflake.service.ts (1)
SnowflakeService(26-400)packages/shared/src/interfaces/analytics-data.interface.ts (9)
ActiveWeeksStreakResponse(25-40)ActiveWeeksStreakRow(10-20)UserPullRequestsResponse(66-81)UserPullRequestsRow(46-61)UserCodeCommitsResponse(107-122)UserCodeCommitsRow(87-102)UserProjectsResponse(168-178)ProjectCountRow(307-309)UserProjectActivityRow(128-163)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)packages/shared/src/interfaces/components.interface.ts (1)
ProjectItem(368-383)
apps/lfx-one/src/server/controllers/analytics.controller.ts (3)
apps/lfx-one/src/server/services/user.service.ts (1)
UserService(33-511)apps/lfx-one/src/server/services/organization.service.ts (1)
OrganizationService(40-425)packages/shared/src/constants/analytics.constants.ts (1)
ANALYTICS_DEFAULTS(8-26)
apps/lfx-one/src/server/services/organization.service.ts (3)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
SnowflakeService(26-400)packages/shared/src/interfaces/analytics-data.interface.ts (21)
OrganizationMaintainersResponse(184-199)MemberDashboardMaintainersRow(267-272)MembershipTierResponse(205-235)MembershipTierRow(278-290)OrganizationContributorsResponse(241-261)OrganizationContributorsRow(296-301)OrganizationEventAttendanceResponse(340-365)OrganizationEventAttendanceRow(328-334)OrganizationTechnicalCommitteeResponse(391-406)OrganizationTechnicalCommitteeRow(381-385)OrganizationProjectsParticipatingResponse(433-448)MemberDashboardProjectsParticipatingRow(412-427)OrganizationTotalCommitsResponse(475-490)MemberDashboardTotalCommitsRow(454-469)OrganizationCertifiedEmployeesResponse(522-537)MemberDashboardCertifiedEmployeesRow(496-516)OrganizationBoardMeetingAttendanceResponse(579-604)MemberDashboardBoardMeetingAttendanceRow(543-573)OrganizationEventSponsorshipsResponse(703-718)OrganizationEventSponsorshipsAggregateRow(656-671)OrganizationEventSponsorshipsEventCountRow(677-682)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
⏰ 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: build-and-push
🔇 Additional comments (10)
apps/lfx-one/src/server/services/user.service.ts (3)
6-40: LGTM: Service initialization with SnowflakeServiceThe analytics imports and SnowflakeService initialization are properly structured, following the same pattern as the NatsService integration.
264-274: Verify streak calculation starts at week 0The current streak calculation assumes
result.rows[0]corresponds toWEEKS_AGO = 0. If the query returns data starting at a different week (e.g., no data for week 0), the streak would be incorrect.Consider adding a verification step:
// Calculate current streak (consecutive weeks of activity from week 0) // Data is ordered by WEEKS_AGO ASC, so index 0 is week 0 (current week) let currentStreak = 0; + +// Verify data starts at week 0 +if (result.rows[0].WEEKS_AGO !== 0) { + // No data for current week, streak is 0 + return { + data: result.rows, + currentStreak: 0, + totalWeeks: result.rows.length, + }; +} for (const row of result.rows) {
296-296: Schema inconsistency across analytics methods requires clarificationLines 296 and 333 query
ANALYTICS.PLATINUM_LFX_ONE(production schema), while other methods in the same file useANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONE(dev schema). All queries inorganization.service.tsconsistently use the dev schema. Confirm whether this intentional mixed-schema approach is correct or if standardization is needed.apps/lfx-one/src/server/controllers/analytics.controller.ts (4)
4-25: LGTM: Clean service layer delegationThe controller properly delegates to domain services (UserService and OrganizationService) following dependency injection principles.
125-147: Missing userEmail parameter in getMyProjects callThe call to
this.userService.getMyProjects(page, limit)at line 133 doesn't pass the user's email. This aligns with the missing filter issue identified inuser.service.ts(lines 357-441). Once that service method is updated to requireuserEmail, this controller method must pass it.Update the service call:
+ const userEmail = req.oidc?.user?.['email']; + + if (!userEmail) { + throw new AuthenticationError('User email not found in authentication context', { + operation: 'get_my_projects', + }); + } + // Parse pagination parameters const page = Math.max(1, parseInt(req.query['page'] as string, 10) || 1); const limit = Math.max(1, Math.min(100, parseInt(req.query['limit'] as string, 10) || 10)); - const response = await this.userService.getMyProjects(page, limit); + const response = await this.userService.getMyProjects(userEmail, page, limit);
154-280: Consistent organization analytics patternThe organization analytics endpoints follow a consistent pattern with proper error handling and logging. Using
ANALYTICS_DEFAULTSaddresses previous concerns about hardcoded values.
282-425: New organization endpoints implemented consistentlyThe five new organization analytics endpoints follow the established pattern. The TODO comments (lines 349, 379) about retrieving PROJECT_ID from user profile/session are appropriate reminders for future enhancement.
apps/lfx-one/src/server/services/organization.service.ts (3)
4-47: LGTM: Proper Snowflake integrationThe service correctly initializes SnowflakeService and imports all necessary types for organization analytics.
337-372: Correct percentage calculation with zero-division handlingThe board meeting attendance query properly handles the edge case where
TOTAL_MEETINGS = 0by returning 0% instead of causing a division error.
374-424: Well-structured event sponsorships aggregationThe two-query approach (currency aggregation + event count) is clear and maintainable. The currency grouping properly handles multi-currency sponsorships.
Consolidate 15 individual analytics endpoints into 4 optimized endpoints: - getOrganizationContributionsOverview (maintainers + contributors + technical committee) - getOrganizationSegmentOverview (projects participating + total commits) - getBoardMemberDashboard (membership tier + certified employees + board attendance) - getOrganizationEventsOverview (event attendance + event sponsorships) Backend changes: - Inline getEventAttendance and getEventSponsorships into getEventsOverview - Remove 10 legacy endpoint handlers from analytics controller - Rename private service methods for consistency (remove "Consolidated" suffix) - Clean up JSDoc comments and simplify method documentation Frontend changes: - Remove 10 unused HTTP methods from analytics service - Update organization-involvement component to use consolidated endpoints Shared package changes: - Remove 21 unused interfaces from analytics-data.interface.ts (~450 lines) - Clean up legacy interfaces no longer used after consolidation This refactor reduces codebase by ~602 lines while improving API efficiency through consolidated database queries and parallel execution patterns. Generated with [Claude Code](https://claude.ai/code) Refs: LFXV2-701 Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
104-104: Consider removing the attribution comment.The "Generated with [Claude Code]" comment is non-standard for production code and doesn't provide maintenance value. This comment also appears on lines 137, 160, and 198.
- * - * Generated with [Claude Code](https://claude.ai/code) */
📜 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/organization-involvement/organization-involvement.component.ts(7 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(6 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(1 hunks)apps/lfx-one/src/server/services/organization.service.ts(2 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
apps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.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:
apps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/routes/analytics.route.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
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
🧠 Learnings (1)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts
🧬 Code graph analysis (4)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (4)
OrganizationContributionsOverviewResponse(272-306)OrganizationSegmentOverviewResponse(312-332)BoardMemberDashboardResponse(338-377)OrganizationEventsOverviewResponse(385-416)
apps/lfx-one/src/server/services/organization.service.ts (3)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
SnowflakeService(26-400)packages/shared/src/interfaces/analytics-data.interface.ts (9)
OrganizationContributionsOverviewResponse(272-306)BoardMemberDashboardResponse(338-377)OrganizationSegmentOverviewResponse(312-332)OrganizationEventsOverviewResponse(385-416)OrganizationEventAttendanceRow(192-198)OrganizationEventSponsorshipsAggregateRow(423-444)OrganizationContributionsConsolidatedRow(233-249)SegmentContributionsConsolidatedRow(256-266)BoardMemberDashboardConsolidatedRow(205-226)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
apps/lfx-one/src/server/controllers/analytics.controller.ts (3)
apps/lfx-one/src/server/services/user.service.ts (1)
UserService(33-511)apps/lfx-one/src/server/services/organization.service.ts (1)
OrganizationService(28-326)packages/shared/src/constants/analytics.constants.ts (1)
ANALYTICS_DEFAULTS(8-26)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (3)
packages/shared/src/constants/organization-involvement.constants.ts (3)
CONTRIBUTIONS_METRICS(66-87)IMPACT_METRICS(93-114)PRIMARY_INVOLVEMENT_METRICS(20-60)packages/shared/src/interfaces/dashboard.interface.ts (2)
PrimaryInvolvementMetric(63-88)OrganizationInvolvementMetricWithChart(124-167)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)
⏰ 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: build-and-push
🔇 Additional comments (3)
apps/lfx-one/src/app/shared/services/analytics.service.ts (3)
1-16: LGTM!The license header and imports are properly structured. The new consolidated response interfaces are correctly imported from the shared package.
109-127: Verify that silent error handling is intentional.All four new methods silently catch errors and return fallback defaults (zeros/empty strings). While this provides UI resilience, callers cannot distinguish between "no data available" and "request failed", potentially causing misleading UX where errors appear as empty dashboards.
This pattern is consistent with existing methods in the service (e.g.,
getActiveWeeksStreak,getPullRequestsMerged), suggesting it may be an intentional design choice. Please confirm:
- Is this the desired error handling strategy for analytics data?
- Should the UI layer handle these scenarios differently (e.g., show a loading skeleton vs. "no data" state)?
Also applies to: 142-150, 165-188, 203-219
139-152: LGTM! Consolidated endpoints follow consistent patterns.The three new methods (
getOrganizationSegmentOverview,getBoardMemberDashboard,getOrganizationEventsOverview) are well-implemented:
- Consistent method signatures with optional
accountIdparameters- Proper TypeScript interfaces for type safety
- Fallback defaults correctly match interface structures
- Clear documentation describing the consolidated nature of these endpoints
- Follows the established pattern from existing analytics methods
The consolidation approach should reduce API roundtrips and improve performance as documented.
Also applies to: 162-190, 200-221
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Show resolved
Hide resolved
LFXV2-701 - Refactor SQL queries to use CTE base with LEFT JOINs instead of anchoring on specific tables - This fixes 404 errors when organizations have partial data (e.g., contributors but no maintainers) - Remove MEMBERSHIP_PRICE/annual fee field from backend, shared interfaces, and frontend - Update membership tier card layout to show three clear rows: Tier, Member Since, and Renewal Date Signed-off-by: Asitha de Silva <asithade@gmail.com>
🧹 Deployment RemovedThe deployment for PR #140 has been removed. |
Summary
Integrates real Snowflake analytics data into the Board Member Dashboard's organization involvement metrics, with optimized API architecture through endpoint consolidation. This PR replaces hardcoded placeholder data with live organization-level analytics while reducing API complexity from 15 endpoints to 4 consolidated endpoints.
JIRA Ticket
LFXV2-701
Changes
Phase 1: Initial Snowflake Integration
Frontend Updates
valueChangessubscription andtakeUntilDestroyed()for automatic cleanupCurrencyPipefor multi-currency event sponsorship formattingBackend Updates
Created 15 new API endpoints for organization-level analytics across multiple data domains:
Each endpoint:
ANALYTICS_DEV.DEV_JEVANS_PLATINUM_LFX_ONEschemaResourceNotFoundErrorPhase 2: API Consolidation & Optimization
Refactored 15 individual analytics endpoints into 4 optimized consolidated endpoints:
Consolidated Endpoints
/api/analytics/organization-contributions-overview/api/analytics/organization-segment-overview/api/analytics/board-member-dashboard/api/analytics/organization-events-overviewBackend Refactoring
getEventAttendanceandgetEventSponsorshipswrapper methodsgetOrganizationContributionsConsolidated→getContributionsDatagetSegmentContributionsConsolidated→getSegmentDatagetBoardMemberDashboardConsolidated→getDashboardDataFrontend Optimization
Shared Package Cleanup
analytics-data.interface.ts(~450 lines removed, 36% file size reduction)Type Safety
@lfx-one/sharedpackage for all analytics responsesPerformance Impact
API Efficiency
Query Optimization
Promise.all()) for independent queries on different tablesFiles Modified
Phase 1 (8 files)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.htmlapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.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.tspackages/shared/src/interfaces/analytics-data.interface.tsPhase 2 - Additional Changes (6 files)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts- Updated to use consolidated endpointsapps/lfx-one/src/app/shared/services/analytics.service.ts- Removed 10 unused methodsapps/lfx-one/src/server/controllers/analytics.controller.ts- Removed 10 legacy handlersapps/lfx-one/src/server/routes/analytics.route.ts- Removed 10 legacy routesapps/lfx-one/src/server/services/organization.service.ts- Inlined methods, renamed helperspackages/shared/src/interfaces/analytics-data.interface.ts- Removed 21 unused interfacesTechnical Details
takeUntilDestroyed()for memory leak preventionPromise.all()for independent queries on different Snowflake tablesResourceNotFoundErrorfor missing data scenarios with proper HTTP status codesTesting
Migration Notes
Generated with Claude Code