feat(dashboards): add metrics api integration and metric card component #186
Conversation
LFXV2-834 - Add health metrics daily, unique contributors daily, and events monthly API endpoints - Create reusable MetricCardComponent for unified dashboard metric display - Refactor foundation-health, recent-progress, and organization-involvement to use MetricCard - Add DashboardMetricCard interface for type-safe metric definitions - Implement chart tooltip callbacks for enhanced data visualization - Consolidate progress metrics constants and analytics data interfaces 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. WalkthroughConsolidates dashboard rendering around a new MetricCardComponent and a unified DashboardMetricCard type, migrates multiple dashboard components to use chartData/chartOptions and customContent templates, adds frontend/backend analytics endpoints and Snowflake-backed services for daily/monthly metrics, and updates shared interfaces/constants. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Dashboard<br/>Component(s)
participant MCC as MetricCardComponent
participant FS as Frontend<br/>AnalyticsService
participant API as AnalyticsController
participant PS as ProjectService
participant DB as Snowflake
Note over UI,FS: Frontend requests daily/monthly analytics
UI->>FS: getUniqueContributorsDaily(slug, type)
FS->>API: GET /api/analytics/unique-contributors-daily
API->>PS: getUniqueContributorsDaily(slug, type)
PS->>DB: Run query
DB-->>PS: Rows
PS-->>API: UniqueContributorsDailyResponse
API-->>FS: JSON response
FS-->>UI: Observable<UniqueContributorsDailyResponse>
Note over UI: UI transforms response into DashboardMetricCard
UI->>UI: transformActiveContributors(data, metric)
UI-->>MCC: Render with (title, icon, chartType, chartData, chartOptions, value)
MCC->>MCC: Render header → chart/customContent → footer
MCC-->>UI: Rendered metric card
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
requested review from
andrest50,
dealako and
mauriciozanettisalomao
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/shared/src/constants/progress-metrics.constants.ts (2)
63-76: Non-deterministic data in constants causes inconsistent renders.Using
Math.random()in constant definitions generates new values on every module import/evaluation. This can cause:
- Inconsistent snapshots in tests
- Hydration mismatches in SSR
- Unexpected re-renders
Since this is meant to be sample/mock data, consider using fixed values or moving this to a factory function if randomness is intentional.
chartData: { labels: Array.from({ length: 30 }, (_, i) => `Day ${i + 1}`), datasets: [ { - data: Array.from({ length: 30 }, () => Math.floor(Math.random() * 5)), + data: [2, 1, 3, 0, 2, 4, 1, 3, 2, 0, 1, 4, 2, 3, 1, 0, 2, 3, 4, 1, 2, 0, 3, 1, 2, 4, 1, 3, 2, 1], borderColor: lfxColors.blue[500], backgroundColor: hexToRgba(lfxColors.blue[500], 0.1),
125-141: Same random data issue in maintainer metrics.The
Critical Security Issuesmetric also usesMath.random()which will produce different values on each import. Apply the same fix as suggested above.
🧹 Nitpick comments (10)
packages/shared/src/interfaces/dashboard.interface.ts (1)
124-126: Consider usingunknowninstead ofanyfor better type safety.The
Record<string, any>type is overly permissive and requires an ESLint disable comment. Consider usingRecord<string, unknown>or defining a more specific Chart.js options interface to maintain type safety.Apply this diff:
- /** Chart.js options configuration */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - chartOptions?: Record<string, any>; + /** Chart.js options configuration */ + chartOptions?: Record<string, unknown>;apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts (1)
60-60: Move spacing to template for better separation of concerns.Concatenating the
mr-2margin class directly to the icon identifier mixes presentation logic with data. Consider handling spacing in the template where the icon is rendered, keeping the icon value as a pure identifier.For example, in the template:
<i [class]="meetingTypeInfo().icon + ' mr-2'"></i>Or define the spacing as a separate property in the computed signal.
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (1)
49-62: Consider sanitizing thetestIdto avoid spaces in data-testid attributes.The
testIdis constructed usingitem.titledirectly, which may contain spaces (e.g., "Active Weeks Streak"). Spaces indata-testidattributes can cause issues with CSS selectors and test frameworks.- [testId]="'dashboard-recent-progress-item-' + item.title" + [testId]="'dashboard-recent-progress-item-' + item.title.toLowerCase().replace(/\s+/g, '-')"Alternatively, add a dedicated
testIdfield toDashboardMetricCardand set it in the data source.apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts (1)
11-16: Consider addingChangeDetectionStrategy.OnPushfor zoneless change detection compatibility.Per coding guidelines, Angular 19 components should use zoneless change detection with signals. While the component correctly uses
input()signals, addingOnPushchange detection would align with zoneless best practices.-import { CommonModule } from '@angular/common'; -import { Component, ContentChild, input, TemplateRef } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { ChangeDetectionStrategy, Component, ContentChild, input, TemplateRef } from '@angular/core'; import { ChartComponent } from '@components/chart/chart.component'; import { TooltipModule } from 'primeng/tooltip'; import type { ChartData, ChartOptions, ChartType } from 'chart.js'; @Component({ selector: 'lfx-metric-card', standalone: true, imports: [CommonModule, ChartComponent, TooltipModule], templateUrl: './metric-card.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, })packages/shared/src/constants/progress-metrics.constants.ts (1)
38-110: Missing testId attributes for E2E testing.As per coding guidelines, new components should have
data-testidattributes. Consider addingtestIdto each metric for reliable test targeting:{ title: 'Code Commits', value: '0', trend: 'up', subtitle: 'Last 30 days', chartType: 'line', chartData: EMPTY_CHART_DATA, chartOptions: PROGRESS_BASE_CHART_OPTIONS, + testId: 'metric-code-commits', },apps/lfx-one/src/server/services/project.service.ts (1)
1163-1214: Consider adding DEBUG logging for observability.The other methods in this service use
req.log.debug()for tracing, but these new analytics methods lack logging. While the methods don't receive aRequestobject, consider usingserverLogger(already imported) for consistency with operational patterns. As per coding guidelines, DEBUG level is appropriate for internal operations.public async getUniqueContributorsDaily(slug: string, entityType: 'foundation' | 'project'): Promise<UniqueContributorsDailyResponse> { + serverLogger.debug({ slug, entityType }, 'Fetching unique contributors daily data'); // Query switching based on entity type const query = entityType === 'foundation'apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
49-149: Consider extracting repetitive tooltip callback patterns.The chart option configurations follow a consistent pattern but are verbose. Consider creating a helper function to reduce duplication:
private createChartOptionsWithTooltip( baseOptions: ChartOptions<'bar' | 'line'>, labelFormatter: (count: number) => string ): ChartOptions<'bar' | 'line'> { return { ...baseOptions, plugins: { ...baseOptions.plugins, tooltip: { ...(baseOptions.plugins?.tooltip ?? {}), callbacks: { title: (context) => context[0].label, label: (context) => labelFormatter(context.parsed.y), }, }, }, }; } // Usage: private readonly activeContributorsChartOptions = this.createChartOptionsWithTooltip( BASE_BAR_CHART_OPTIONS, (count) => `Active contributors: ${count}` );apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
139-166: String-based metric matching is fragile.The
initializeProgressItemsmethod uses string comparisons onmetric.titleto determine which transform to apply. This is brittle and could break if titles are renamed.Consider using a constant key or enum for matching:
// In constants file export const METRIC_KEYS = { ACTIVE_WEEKS_STREAK: 'active-weeks-streak', HEALTH_SCORE: 'health-score', // ... } as const; // In metric definitions, add a key field { key: METRIC_KEYS.HEALTH_SCORE, title: 'Health Score', ... } // In component if (metric.key === METRIC_KEYS.HEALTH_SCORE) { return this.transformHealthMetricsDaily(healthMetricsData, metric); }This approach decouples display text from programmatic logic.
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (2)
262-262: Track the TODO for Company Bus Factor API integration.The TODO comment indicates that the Company Bus Factor metric is still using mock data. Please ensure this is tracked so the real API integration is not overlooked.
Do you want me to open a new issue to track the Company Bus Factor API endpoint integration?
521-557: Consider adding error handling for API failures.The new data initialization methods (
initializeActiveContributorsDataandinitializeEventsData) follow the established pattern but lack error handling. If the API calls fail, the loading state will be cleared, but the component will silently display zero values or "N/A" without indicating an error occurred to the user.Consider adding error handling in the RxJS pipes, for example:
private initializeActiveContributorsData() { return toSignal( this.selectedFoundationSlug$.pipe( switchMap((foundationSlug) => { this.activeContributorsLoading.set(true); return this.analyticsService.getUniqueContributorsDaily(foundationSlug, 'foundation').pipe( + catchError((error) => { + console.error('Failed to fetch active contributors data:', error); + return of({ + data: [], + avgContributors: 0, + totalDays: 0, + } as UniqueContributorsDailyResponse); + }), finalize(() => this.activeContributorsLoading.set(false)) ); }) ), { initialValue: { data: [], avgContributors: 0, totalDays: 0, } as UniqueContributorsDailyResponse, } ); }Note: If error handling exists at a higher level (e.g., HTTP interceptor with user notifications), this may not be necessary.
Please verify if error handling is implemented at the service or interceptor level. If not, consider adding it to provide better user feedback when API calls fail.
📜 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 (22)
apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(9 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/organization-involvement/organization-involvement.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(5 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts(17 hunks)apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html(1 hunks)apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts(1 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(2 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(1 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(1 hunks)apps/lfx-one/src/server/services/project.service.ts(4 hunks)packages/shared/src/constants/foundation-health.constants.ts(4 hunks)packages/shared/src/constants/organization-involvement.constants.ts(1 hunks)packages/shared/src/constants/progress-metrics.constants.ts(5 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(1 hunks)packages/shared/src/interfaces/dashboard-metric.interface.ts(1 hunks)packages/shared/src/interfaces/dashboard.interface.ts(1 hunks)packages/shared/src/interfaces/foundation-metrics.interface.ts(1 hunks)packages/shared/src/interfaces/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.tspackages/shared/src/constants/foundation-health.constants.tspackages/shared/src/interfaces/dashboard.interface.tspackages/shared/src/interfaces/index.tspackages/shared/src/constants/organization-involvement.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tspackages/shared/src/constants/progress-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/foundation-metrics.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/project.service.tspackages/shared/src/interfaces/dashboard-metric.interface.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
**/*.{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/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlpackages/shared/src/constants/foundation-health.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlpackages/shared/src/interfaces/dashboard.interface.tspackages/shared/src/interfaces/index.tspackages/shared/src/constants/organization-involvement.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlpackages/shared/src/constants/progress-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/foundation-metrics.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/project.service.tspackages/shared/src/interfaces/dashboard-metric.interface.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.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/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.tspackages/shared/src/constants/foundation-health.constants.tspackages/shared/src/interfaces/dashboard.interface.tspackages/shared/src/interfaces/index.tspackages/shared/src/constants/organization-involvement.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tspackages/shared/src/constants/progress-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/analytics-data.interface.tsapps/lfx-one/src/server/controllers/analytics.controller.tspackages/shared/src/interfaces/foundation-metrics.interface.tsapps/lfx-one/src/app/shared/services/analytics.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/project.service.tspackages/shared/src/interfaces/dashboard-metric.interface.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.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/components/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.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/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.htmlapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
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/constants/foundation-health.constants.tspackages/shared/src/interfaces/dashboard.interface.tspackages/shared/src/interfaces/index.tspackages/shared/src/constants/organization-involvement.constants.tspackages/shared/src/constants/progress-metrics.constants.tspackages/shared/src/interfaces/analytics-data.interface.tspackages/shared/src/interfaces/foundation-metrics.interface.tspackages/shared/src/interfaces/dashboard-metric.interface.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/project.service.tsapps/lfx-one/src/server/routes/analytics.route.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
🧠 Learnings (5)
📚 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/dashboard-meeting-card/dashboard-meeting-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.htmlapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.htmlapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to packages/shared/src/**/*.{ts,tsx} : All shared types, interfaces, and constants are centralized in lfx-one/shared package
Applied to files:
packages/shared/src/interfaces/index.tspackages/shared/src/interfaces/foundation-metrics.interface.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript interfaces instead of union types for better maintainability
Applied to files:
packages/shared/src/interfaces/dashboard-metric.interface.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.component.ts : Use Angular 19 zoneless change detection with signals for component state management
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
🧬 Code graph analysis (7)
packages/shared/src/constants/foundation-health.constants.ts (2)
packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
DashboardMetricCard(23-113)packages/shared/src/constants/colors.constants.ts (1)
lfxColors(9-106)
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts (3)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
Component(24-558)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
Component(20-429)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)
Component(37-867)
packages/shared/src/constants/progress-metrics.constants.ts (1)
packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
DashboardMetricCard(23-113)
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (2)
apps/lfx-one/src/app/shared/components/filter-pills/filter-pills.component.ts (2)
Component(12-26)FilterOption(7-10)packages/shared/src/constants/chart-options.constants.ts (2)
BASE_LINE_CHART_OPTIONS(69-82)BASE_BAR_CHART_OPTIONS(88-109)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (3)
HealthMetricsDailyResponse(1224-1239)UniqueContributorsDailyResponse(1327-1342)HealthEventsMonthlyResponse(1420-1435)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (3)
packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
DashboardMetricCard(23-113)packages/shared/src/constants/foundation-health.constants.ts (1)
AGGREGATE_FOUNDATION_METRICS(120-136)packages/shared/src/interfaces/analytics-data.interface.ts (2)
UniqueContributorsDailyResponse(1327-1342)HealthEventsMonthlyResponse(1420-1435)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (7)
packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
DashboardMetricCard(23-113)packages/shared/src/constants/progress-metrics.constants.ts (2)
MAINTAINER_PROGRESS_METRICS(116-194)CORE_DEVELOPER_PROGRESS_METRICS(38-110)packages/shared/src/interfaces/analytics-data.interface.ts (5)
ActiveWeeksStreakResponse(25-40)UserPullRequestsResponse(66-81)UserCodeCommitsResponse(107-122)FoundationContributorsMentoredResponse(612-632)HealthMetricsDailyResponse(1224-1239)packages/shared/src/constants/colors.constants.ts (1)
lfxColors(9-106)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)packages/shared/src/constants/chart-options.constants.ts (1)
BASE_LINE_CHART_OPTIONS(69-82)packages/shared/src/utils/date-time.utils.ts (1)
parseLocalDateString(52-65)
⏰ 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 (34)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
400-470: LGTM! Well-structured analytics methods.The three new methods (
getHealthMetricsDaily,getUniqueContributorsDaily,getHealthEventsMonthly) follow the established service patterns consistently, with proper error handling, fallback data, and clear JSDoc documentation.packages/shared/src/interfaces/analytics-data.interface.ts (1)
1093-1435: LGTM! Comprehensive interface definitions.The new analytics interfaces are well-structured with:
- Consistent Snowflake naming conventions (ALL_CAPS)
- Clear JSDoc documentation for all fields
- Proper separation of foundation-level and project-level data
- Appropriate use of optional fields and union types in response interfaces
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html (1)
32-40: LGTM! Consistent styling improvements.The text color changes to
text-gray-500for headers and secondary information improve visual hierarchy by making less prominent information more subdued.Also applies to: 65-65
apps/lfx-one/src/server/routes/analytics.route.ts (1)
53-60: LGTM! Routes follow established patterns.The three new analytics endpoints are properly registered and follow the same structure as existing routes in the file.
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.html (1)
88-97: LGTM! Effective refactoring to reusable component.Replacing inline markup with
<lfx-metric-card>eliminates code duplication and standardizes the metric card presentation across dashboards, aligning with the PR's unified dashboard metric model.packages/shared/src/interfaces/index.ts (1)
43-44: LGTM! Proper barrel export.The new export for
dashboard-metric.interfacefollows the established pattern and is logically placed with a clear descriptive comment.apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html (1)
4-50: LGTM! Well-structured reusable metric card template.The component template is well-organized with clear sections for header, body (chart/custom content), and footer. The conditional rendering logic is correct, and the non-null assertions on line 28 are safe since they're guarded by
@else if (chartData()).apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts (1)
17-38: LGTM! Component inputs are well-structured using Angular 19 signals.The use of
input()andinput.required()signals follows modern Angular patterns. The logical grouping of inputs (Header, Chart, Footer, Optional indicators) with comments improves readability.apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (3)
65-77: LGTM! Clean switch-case pattern for rendering different metric card types.The sparkline case correctly falls back to
sparklineOptionswhencard.chartOptionsis not provided, ensuring consistent chart rendering.
161-173: LGTM! Default fallback case handles standard metric cards appropriately.The default case passes all relevant properties to the metric card, ensuring any card type not explicitly handled still renders correctly.
92-109: The template reference and ContentChild selector match correctly—no action needed.Verification confirms the
ng-template #customContentin foundation-health.component.html (line 95) perfectly matches the@ContentChild('customContent')selector in MetricCardComponent. The implementation is functioning correctly.Likely an incorrect or invalid review comment.
apps/lfx-one/src/server/controllers/analytics.controller.ts (3)
628-668: LGTM! Endpoint follows established patterns with proper validation and logging.The
getHealthMetricsDailyendpoint correctly validates required parameters, uses Logger helper methods, and follows the same structure as existing endpoints.
675-715: LGTM! Consistent implementation with other analytics endpoints.The
getUniqueContributorsDailyendpoint maintains consistency with the codebase patterns for parameter validation, service calls, and structured logging.
722-762: LGTM! Well-structured endpoint implementation.The
getHealthEventsMonthlyendpoint completes the trio of new analytics endpoints with proper validation and error handling consistent with the rest of the controller.packages/shared/src/constants/organization-involvement.constants.ts (2)
10-13: LGTM! Empty chart data placeholder for live data metrics.The
EMPTY_CHART_DATAconstant is a clean way to indicate that chart data will be populated at runtime from API responses.
20-69: LGTM! Migration toDashboardMetricCard[]type is correct.The PRIMARY_INVOLVEMENT_METRICS array properly uses the unified DashboardMetricCard interface, with appropriate chartType assignments for each metric type.
packages/shared/src/constants/foundation-health.constants.ts (1)
143-213: LGTM! Migration to unifiedDashboardMetricCard[]type is well-structured.The PRIMARY_FOUNDATION_HEALTH_METRICS array correctly uses the unified interface. The
chartTypeassignments are logical:'line'for trend-based metrics and'bar'for categorical data (Events, Project Health Scores). ThecustomContentTypefield is preserved to drive specialized rendering in the template.packages/shared/src/interfaces/foundation-metrics.interface.ts (2)
4-7: LGTM! Type consolidation follows centralization best practices.Re-exporting shared types from
dashboard-metric.interfacealigns with the coding guideline that all shared types should be centralized in the@lfx-one/sharedpackage. This approach maintains backward compatibility for existing imports while consolidating type definitions.Based on coding guidelines, this centralization approach is correct.
9-44: LGTM!AggregateFoundationMetricsinterface properly uses imported types.The interface correctly references the re-exported types (
TopProjectByValue,CompanyBusFactor,ProjectHealthDistribution) from the unified dashboard metric interface.packages/shared/src/constants/progress-metrics.constants.ts (1)
26-32: Good pattern for placeholder data.The
EMPTY_CHART_DATAplaceholder is a clean approach for metrics that will be populated dynamically by transform functions. This avoids unnecessary initial data and makes it clear which metrics expect live API data.apps/lfx-one/src/server/services/project.service.ts (2)
1102-1161: Looks good - production-ready with proper parameterization.The
getHealthMetricsDailymethod is well-structured:
- Uses parameterized queries to prevent SQL injection
- Correctly switches between foundation and project queries
- Properly calculates the current average health score from the most recent row
Note: This method uses production schema (
ANALYTICS.PLATINUM_LFX_ONE) while the other two new methods use dev schema (ANALYTICS_DEV.DEV_MSALOMAO_PLATINUM_LFX_ONE).Please verify that the schema references are intentional:
getHealthMetricsDaily:ANALYTICS.PLATINUM_LFX_ONEgetUniqueContributorsDaily:ANALYTICS_DEV.DEV_MSALOMAO_PLATINUM_LFX_ONEgetHealthEventsMonthly:ANALYTICS_DEV.DEV_MSALOMAO_PLATINUM_LFX_ONE
1216-1265: Implementation is consistent with the pattern.The
getHealthEventsMonthlymethod follows the established pattern. The aggregation logic correctly extractstotalEventsfrom the first row (which is the most recent due toORDER BY MONTH_START_DATE DESC).packages/shared/src/interfaces/dashboard-metric.interface.ts (3)
1-17: Well-structured type definitions.The use of string literal union types for
MetricCategoryandCustomContentTypeis appropriate here - interfaces cannot represent finite sets of string values. The JSDoc documentation is helpful for maintainability.
22-113: Unified interface design looks good.The
DashboardMetricCardinterface is well-organized with clear section groupings. HavingtitleandchartTypeas required while other fields are optional provides flexibility across different dashboard use cases.One consideration: the interface is quite large (20+ properties). If this grows further, consider splitting into composed interfaces (e.g.,
ChartConfig,MembershipConfig) to improve maintainability.
119-167: Supporting interfaces are well-defined.The supporting interfaces (
ProjectHealthDistribution,CompanyBusFactor,TopProjectDisplay,TopProjectByValue) are properly typed with clear JSDoc descriptions.apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (2)
20-26: Component properly migrated to MetricCardComponent.The standalone component correctly imports
MetricCardComponentand removes the obsoleteTooltipModule. The component uses signals for state management as per Angular 19 guidelines.
326-342: Transform methods are well-structured.The transform methods follow a consistent pattern: spread the base metric, override specific properties, and apply chart options. This is clean and maintainable.
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (5)
37-43: Component properly configured with signals.The standalone component correctly imports
MetricCardComponentand uses Angular 19 zoneless change detection patterns with signals for state management.
53-62: Loading state correctly tracks new data source.The
healthMetricsDailyloading flag is properly added to the loading state object, ensuring the component's loading indicator accounts for the new data source.
585-636: Health metrics daily transform is well-implemented.The
transformHealthMetricsDailymethod follows the established pattern:
- Reverses data for chronological display
- Uses appropriate color from the design system
- Implements proper tooltip with date formatting
- Sets trend based on health score threshold
779-802: Data initialization follows established patterns.The
initializeHealthMetricsDailyDatamethod correctly:
- Reacts to project slug changes
- Handles missing slug gracefully
- Updates loading state appropriately
- Uses
finalizefor cleanup
427-437: Console.error usage in tooltip callbacks is acceptable.The try/catch blocks with
console.errorin tooltip callbacks are appropriate for non-critical rendering errors that shouldn't crash the application. The fallback values ensure graceful degradation.Also applies to: 549-552
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (2)
44-45: LGTM! Loading signals properly added.The new loading signals follow the established pattern and are correctly integrated into the
isLoadingcomputed signal.
273-318: Well-implemented chart data transformation.The active contributors chart transformation correctly:
- Reverses data for chronological rendering (oldest to newest)
- Formats dates and contributor values for display
- Provides custom tooltip callbacks with proper formatting
- Uses proper color theming with alpha transparency for filled areas
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Outdated
Show resolved
Hide resolved
packages/shared/src/constants/organization-involvement.constants.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive refactoring of dashboard metrics by adding new health metrics API endpoints, creating a reusable MetricCardComponent, and unifying the metric display interfaces across all dashboard sections.
Key Changes
- Backend API Integration: Added three new analytics endpoints for health metrics daily, unique contributors daily, and health events monthly data from Snowflake
- Unified Component Architecture: Created a reusable
MetricCardComponentthat consolidates metric display logic across foundation health, recent progress, and organization involvement dashboards - Type-Safe Interface: Introduced
DashboardMetricCardinterface to standardize metric definitions with Chart.js integration and custom content projection support
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/dashboard-metric.interface.ts |
New unified interface for all dashboard metric cards with comprehensive JSDoc |
packages/shared/src/interfaces/analytics-data.interface.ts |
Added interfaces for health metrics, contributors, and events data from Snowflake |
packages/shared/src/constants/progress-metrics.constants.ts |
Refactored to use DashboardMetricCard and simplified empty data placeholders |
packages/shared/src/constants/organization-involvement.constants.ts |
Updated to use unified interface with proper chart type definitions |
packages/shared/src/constants/foundation-health.constants.ts |
Consolidated metric definitions with unified interface |
apps/lfx-one/src/server/services/project.service.ts |
Added three new methods for fetching health metrics, contributors, and events data |
apps/lfx-one/src/server/routes/analytics.route.ts |
Registered new analytics API routes |
apps/lfx-one/src/server/controllers/analytics.controller.ts |
Implemented three new endpoint handlers with validation |
apps/lfx-one/src/app/shared/services/analytics.service.ts |
Added frontend service methods with error handling |
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts |
New reusable component for metric display with chart and custom content support |
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts |
Refactored to use MetricCardComponent and added health score metric |
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts |
Refactored to use unified interface with enhanced chart tooltips |
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts |
Integrated live API data for active contributors and events metrics |
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html |
Updated styling from text-muted-foreground to text-gray-500 for consistency |
apps/lfx-one/src/app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts |
Added margin to icon for proper spacing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
...one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
Outdated
Show resolved
Hide resolved
...one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts
Show resolved
Hide resolved
...modules/dashboards/components/organization-involvement/organization-involvement.component.ts
Outdated
Show resolved
Hide resolved
packages/shared/src/constants/organization-involvement.constants.ts
Outdated
Show resolved
Hide resolved
- Consolidate foundation-health, organization-involvement, and progress-metrics constants into single dashboard-metrics.constants.ts file - Move EMPTY_CHART_DATA and NO_TOOLTIP_CHART_OPTIONS to chart-options.constants.ts - Merge dashboard.interface.ts and foundation-metrics.interface.ts into dashboard-metric.interface.ts - Remove sparkline customContentType and use default metric card - Simplify organization-involvement component by using shared imports Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html (1)
26-29: Add chartType to the guard condition to prevent runtime errors.The non-null assertion
chartType()!on line 28 is unsafe because the guard only checkschartData(). IfchartType()returns undefined, this will cause a runtime error.Apply this diff to add chartType to the guard:
- } @else if (chartData()) { + } @else if (chartData() && chartType()) { <div class="mt-3 w-full h-16"> - <lfx-chart [type]="chartType()" [data]="chartData()!" [options]="chartOptions()" height="100%"></lfx-chart> + <lfx-chart [type]="chartType()" [data]="chartData()" [options]="chartOptions()" height="100%"></lfx-chart> </div> }apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts (1)
17-18: Add JSDoc documentation for the component.The
MetricCardComponentlacks component-level JSDoc explaining its purpose, usage, and key features. This is important for a reusable component that will be used across multiple dashboard modules.Add JSDoc like this:
+/** + * Reusable metric card component for dashboard visualizations + * @description Displays a metric with optional icon, chart, and custom content projection. + * Supports both chart-based rendering (line/bar charts via Chart.js) and custom content via ng-template. + * @example + * <lfx-metric-card + * [title]="'Active Contributors'" + * [icon]="'fa-light fa-code'" + * [chartType]="'line'" + * [chartData]="chartData" + * [value]="'1,234'" + * [subtitle]="'Last 30 days'"> + * </lfx-metric-card> + */ export class MetricCardComponent {packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
24-35: Update documentation to accurately reflect required properties.The comment on line 27 states "All properties are optional to support different use cases," but
title(line 35) andchartType(line 54) are not marked as optional. This documentation is misleading.Apply this diff to clarify which properties are required:
/** * Unified dashboard metric card interface * @description Single interface for all dashboard metric cards (progress, foundation health, organization involvement) - * All properties are optional to support different use cases - components validate required fields + * Required properties: title, chartType. All other properties are optional to support different use cases. */ export interface DashboardMetricCard {apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
308-315: Hardcoded placeholder value should come from API.The value changed from
'156'to'0', but it remains a hardcoded placeholder. This should be populated from an API endpoint or frommetric.valuewhen the training enrollments endpoint is implemented.Consider updating to use dynamic data:
private transformTrainingEnrollments(metric: DashboardMetricCard): DashboardMetricCard { + // TODO: Integrate with training enrollments API endpoint return { ...metric, - value: '0', + value: metric.value ?? '0', // Use metric value when API is integrated subtitle: 'Employees enrolled in training', chartOptions: this.createLineChartOptions('Training enrollments'), }; }
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
164-202: Consider extracting chart construction helpers to reduce duplication.While the transform methods correctly implement the DashboardMetricCard pattern, there's significant duplication in chart construction logic. Multiple methods build chartData and chartOptions with nearly identical patterns (especially for line charts: transformTotalProjects, transformTotalMembers, transformActiveContributors, transformMaintainers).
Consider creating helper methods similar to the approach in organization-involvement.component.ts:
private createLineChartWithTooltip( labels: string[], data: number[], tooltipLabel: string, color: string = lfxColors.blue[500] ): { chartData: ChartData<'line'>; chartOptions: ChartOptions<'line'> } { return { chartData: { labels, datasets: [{ data, borderColor: color, backgroundColor: hexToRgba(color, 0.1), fill: true, tension: 0.4, borderWidth: 2, pointRadius: 0, }], }, chartOptions: { ...this.sparklineOptions, plugins: { ...this.sparklineOptions.plugins, tooltip: { ...(this.sparklineOptions.plugins?.tooltip ?? {}), callbacks: { title: (context: TooltipItem<'line'>[]) => context[0].label, label: (context: TooltipItem<'line'>) => `${tooltipLabel}: ${context.parsed.y.toLocaleString()}`, }, }, }, }, }; }Then transform methods could use:
private transformTotalProjects(metric: DashboardMetricCard): DashboardMetricCard { const data = this.totalProjectsData(); const { chartData, chartOptions } = this.createLineChartWithTooltip( data.monthlyLabels, data.monthlyData, 'Total projects' ); return { ...metric, value: data.totalProjects.toLocaleString(), subtitle: `Total ${this.projectContextService.selectedFoundation()?.name} projects`, chartData, chartOptions, }; }Also applies to: 204-242, 265-311, 313-351, 353-396
📜 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 (15)
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(9 hunks)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts(5 hunks)apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html(1 hunks)apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts(1 hunks)packages/shared/src/constants/chart-options.constants.ts(2 hunks)packages/shared/src/constants/dashboard-metrics.constants.ts(1 hunks)packages/shared/src/constants/foundation-health.constants.ts(0 hunks)packages/shared/src/constants/index.ts(1 hunks)packages/shared/src/constants/organization-involvement.constants.ts(0 hunks)packages/shared/src/constants/progress-metrics.constants.ts(0 hunks)packages/shared/src/interfaces/dashboard-metric.interface.ts(1 hunks)packages/shared/src/interfaces/dashboard.interface.ts(0 hunks)packages/shared/src/interfaces/foundation-metrics.interface.ts(0 hunks)packages/shared/src/interfaces/index.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/shared/src/constants/progress-metrics.constants.ts
- packages/shared/src/constants/organization-involvement.constants.ts
- packages/shared/src/interfaces/dashboard.interface.ts
- packages/shared/src/constants/foundation-health.constants.ts
- packages/shared/src/interfaces/foundation-metrics.interface.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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:
packages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/index.tspackages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/dashboard-metric.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
packages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/index.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.htmlpackages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/dashboard-metric.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html
**/*.{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:
packages/shared/src/constants/chart-options.constants.tspackages/shared/src/constants/index.tspackages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tspackages/shared/src/interfaces/dashboard-metric.interface.tspackages/shared/src/interfaces/index.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.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/constants/chart-options.constants.tspackages/shared/src/constants/index.tspackages/shared/src/constants/dashboard-metrics.constants.tspackages/shared/src/interfaces/dashboard-metric.interface.tspackages/shared/src/interfaces/index.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/components/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html
**/*.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/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
🧠 Learnings (4)
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to packages/shared/src/**/*.{ts,tsx} : All shared types, interfaces, and constants are centralized in lfx-one/shared package
Applied to files:
packages/shared/src/constants/index.tspackages/shared/src/interfaces/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/foundation-health/foundation-health.component.htmlapps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript interfaces instead of union types for better maintainability
Applied to files:
packages/shared/src/interfaces/dashboard-metric.interface.ts
🔇 Additional comments (23)
packages/shared/src/interfaces/index.ts (1)
40-41: LGTM! Appropriate consolidation of dashboard metric interfaces.The replacement of separate dashboard and foundation metric exports with a unified dashboard-metric interface aligns with the PR objectives to standardize metric definitions across modules.
packages/shared/src/constants/chart-options.constants.ts (2)
4-14: LGTM! Well-documented placeholder constant.The
EMPTY_CHART_DATAconstant provides a clean way to initialize chart structures before live data is available. The JSDoc clearly explains its purpose for metrics populated by APIs.
175-190: LGTM! Appropriate sparkline configuration.The
NO_TOOLTIP_CHART_OPTIONSconstant is well-suited for simple sparkline visualizations where tooltips and axes are unnecessary. The JSDoc clearly documents its use case.packages/shared/src/constants/index.ts (1)
20-25: LGTM! Export consolidation aligns with refactoring objectives.The addition of
dashboard-metrics.constantsandcookie.constantsexports properly consolidates the metric configuration surface, replacing the previous separate constant files.apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html (1)
4-4: LGTM! Proper data-testid implementation.The component correctly uses the
data-testidattribute for test targeting, following the coding guidelines.apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.html (1)
65-178: LGTM! Well-structured switch-based metric card rendering.The refactoring to use a unified
lfx-metric-cardcomponent with switch-based custom content rendering is clean and maintainable. All cases properly bindchartTypeand other required properties.apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts (1)
1-3: Verify license header compliance with coding guidelines.The coding guidelines state: "Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code." Please confirm whether this code was AI-assisted and update the header accordingly.
As per coding guidelines, if this file was created or significantly assisted by AI, add the attribution:
// Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT +// Generated with Claude Code (https://claude.ai/code)packages/shared/src/constants/dashboard-metrics.constants.ts (3)
1-3: Verify license header compliance with coding guidelines.The coding guidelines require prepending 'Generated with Claude Code (https://claude.ai/code)' if the code was AI-assisted. Please confirm and update if applicable.
As per coding guidelines:
// Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT +// Generated with Claude Code (https://claude.ai/code)
30-92: LGTM! Well-documented metric configurations.The
PRIMARY_FOUNDATION_HEALTH_METRICSprovides a clear, type-safe configuration template for foundation health dashboard cards. The JSDoc correctly notes this is UI configuration only with data coming from APIs.
156-228: Sample data with Math.random() is appropriate for placeholders.The use of
Math.random()to generate sample chart data for metrics like "Issues Resolved & Comments Added" and "Learning Hours" is acceptable for initial placeholder visualization. These will be replaced when API endpoints are available.packages/shared/src/interfaces/dashboard-metric.interface.ts (2)
10-22: LGTM! Union types are appropriate for literal type definitions.The use of union types for
FoundationHealthScore,MetricCategory, andCustomContentTypeis correct. These are literal type enums, not object shapes. The coding guideline preference for interfaces over union types applies to object structures, not to enum-like literal types.
1-3: Verify license header compliance with coding guidelines.The coding guidelines require prepending 'Generated with Claude Code (https://claude.ai/code)' if the code was AI-assisted. Please confirm and update if applicable.
As per coding guidelines:
// Copyright The Linux Foundation and each contributor to LFX. // SPDX-License-Identifier: MIT +// Generated with Claude Code (https://claude.ai/code)apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (5)
9-9: LGTM! Clean import updates align with MetricCard refactoring.The addition of MetricCardComponent and DashboardMetricCard types properly supports the component's migration to the unified metric card pattern. Direct imports follow coding guidelines.
Also applies to: 17-19, 23-23
53-106: LGTM! primaryMetrics correctly implements DashboardMetricCard pattern.The computed signal properly transforms metrics using the new DashboardMetricCard interface and applies filtering logic while preserving membership tier visibility across all filter states.
222-229: LGTM! Transform methods properly implement DashboardMetricCard pattern.All transform methods correctly spread the incoming metric and add chartOptions using the helper methods. The consistent pattern improves maintainability and aligns with the unified MetricCard approach.
Also applies to: 231-238, 240-273, 275-285, 287-297, 299-306
317-324: LGTM! Clean default handler implementation.The transformDefaultMetric method properly handles fallback cases with sensible defaults and conditional chart options selection based on chartType.
326-356: LGTM! Helper methods successfully address previous duplication concerns.The createBarChartOptions and createLineChartOptions methods effectively reduce the code duplication that was flagged in previous reviews (six nearly identical chart option objects). While there's minor similarity between these two helpers, they operate on different chart types ('bar' vs 'line'), making complete consolidation impractical without sacrificing type safety and readability.
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (6)
9-9: LGTM! Import updates align with MetricCard refactoring.The addition of MetricCardComponent and relevant types (DashboardMetricCard, HealthEventsMonthlyResponse, UniqueContributorsDailyResponse) properly supports the component's migration to the unified metric card pattern.
Also applies to: 16-16, 21-21
38-39: LGTM! New data streams properly integrated.The activeContributorsLoading and eventsLoading signals with their corresponding data signals are correctly initialized and integrated into the isLoading computed signal, ensuring proper loading state management for the new metrics.
Also applies to: 47-48, 49-58
73-82: LGTM! Metric cards properly implement DashboardMetricCard pattern.The computed signals correctly transform metrics using the DashboardMetricCard interface and apply filtering logic appropriately.
Also applies to: 84-112
255-263: TODO comment tracked for Company Bus Factor API integration.The transformCompanyBusFactor method includes a TODO for replacing hardcoded COMPANY_BUS_FACTOR data with real API data. This is properly documented and uses a constant for the placeholder data.
Ensure this TODO is tracked in the project's issue tracker so the API integration isn't forgotten.
513-530: LGTM! New data initialization methods follow established patterns.The initializeActiveContributorsData and initializeEventsData methods properly implement the reactive data pipeline pattern used throughout the component. They correctly:
- Use toSignal with switchMap for foundation slug changes
- Manage loading states with finalize
- Pass appropriate entity type ('foundation') to analytics service
- Provide type-safe initial values
Also applies to: 532-549
268-274: LGTM! Clear documentation of data transformation.The comments explaining why data is reversed ("show oldest to newest for chart rendering") provide valuable context for future maintainers and make the intent of the transformation clear.
Also applies to: 356-362
- Add individual API endpoints for organization metrics: - /certified-employees - certification data by foundation - /membership-tier - membership tier data by foundation - /organization-maintainers - maintainer counts - /organization-contributors - contributor counts - /training-enrollments - enrollment data with cumulative charts - Add TypeScript interfaces for Snowflake data mapping - Update organization-involvement component to use live API data - Add dummy chart data for metrics without live chart data yet - Training enrollments shows cumulative chart with formatted dates LFXV2-834 Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
...app/modules/dashboards/components/dashboard-meeting-card/dashboard-meeting-card.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/dashboards/components/my-projects/my-projects.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/metric-card/metric-card.component.html
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
packages/shared/src/constants/dashboard-metrics.constants.ts (3)
103-222: MissingtestIdattributes for involvement metrics.Per coding guidelines, new components should have
data-testidattributes for reliable test targeting. ThePRIMARY_INVOLVEMENT_METRICSarray lackstestIdproperties, unlikePRIMARY_FOUNDATION_HEALTH_METRICS.export const PRIMARY_INVOLVEMENT_METRICS: DashboardMetricCard[] = [ { title: 'Membership Tier', icon: 'fa-light fa-dollar-sign', chartType: 'line', isMembershipTier: true, + testId: 'involvement-card-membership-tier', }, { title: 'Active Contributors', icon: 'fa-light fa-code', chartType: 'bar', + testId: 'involvement-card-active-contributors', chartData: {Apply similar additions for the remaining metrics in this array.
232-304: MissingtestIdattributes for developer progress metrics.The
CORE_DEVELOPER_PROGRESS_METRICSarray should includetestIdproperties for E2E testing consistency.export const CORE_DEVELOPER_PROGRESS_METRICS: DashboardMetricCard[] = [ { title: 'Code Commits', value: '0', trend: 'up', subtitle: 'Last 30 days', chartType: 'line', + testId: 'progress-card-code-commits', chartData: EMPTY_CHART_DATA,
310-388: MissingtestIdattributes for maintainer progress metrics.The
MAINTAINER_PROGRESS_METRICSarray should includetestIdproperties for E2E testing consistency, similar toPRIMARY_FOUNDATION_HEALTH_METRICS.export const MAINTAINER_PROGRESS_METRICS: DashboardMetricCard[] = [ { title: 'Critical Security Issues', icon: 'fa-light fa-shield', value: '19', trend: 'down', subtitle: 'Open critical security vulnerabilities', chartType: 'line', category: 'projectHealth', + testId: 'maintainer-card-critical-security-issues', chartData: {apps/lfx-one/src/server/services/organization.service.ts (1)
237-271: Inconsistent error handling pattern for empty results.
getTrainingEnrollmentsreturns an empty response when no data is found, while other methods (getCertifiedEmployees,getMembershipTier, etc.) throwResourceNotFoundError. This inconsistency could lead to unexpected behavior for API consumers.Consider either:
- Throwing
ResourceNotFoundErrorhere too, or- Documenting that empty data is a valid response for this endpoint
const result = await this.snowflakeService.execute<TrainingEnrollmentDailyRow>(query, [projectSlug, accountId]); - // Calculate total from the last cumulative count or sum of daily counts - const totalEnrollments = result.rows.length > 0 ? result.rows[result.rows.length - 1].CUMULATIVE_COUNT : 0; + if (result.rows.length === 0) { + throw new ResourceNotFoundError('Training enrollments data', accountId, { + operation: 'get_training_enrollments', + }); + } + + // Calculate total from the last cumulative count + const totalEnrollments = result.rows[result.rows.length - 1].CUMULATIVE_COUNT;Alternatively, if empty is valid, add a JSDoc note explaining the design decision.
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (2)
122-167: Consider using constants or enum for metric title matching.The metric title matching using string literals (e.g.,
'Active Weeks Streak','Health Score') is fragile and prone to errors if titles change. Consider using constants or an enum to map metric identifiers to their transform functions.// Example approach using a mapping object private readonly metricTransformers: Record<string, (data: any, metric: DashboardMetricCard) => DashboardMetricCard> = { [MetricIdentifier.ActiveWeeksStreak]: (data, metric) => this.transformActiveWeeksStreak(data, metric), // ... other mappings };
241-244: Inconsistent error handling in tooltip callbacks.Some tooltip callbacks wrap
parseLocalDateStringin try-catch (e.g., lines 427-437) while others don't (here and lines 285-288, 360-363, 622-625). For consistency and robustness, consider applying the same error handling pattern across all callbacks, or use a shared helper.callbacks: { title: (context: TooltipItem<'line'>[]) => { + try { const dateStr = context[0].label; const date = parseLocalDateString(dateStr); return date.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }); + } catch { + return context[0]?.label || ''; + } },apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (1)
350-351: Consider validating date strings before parsing.If
data.startDateordata.endDatecontain invalid or empty strings,new Date()will create an Invalid Date, andtoLocaleDateStringwill return"Invalid Date". Consider adding validation or a fallback.+ const parseDate = (dateStr: string): Date | null => { + const d = new Date(dateStr); + return isNaN(d.getTime()) ? null : d; + }; + - const startDate = new Date(data.startDate); - const endDate = new Date(data.endDate); - const tierSince = startDate.toLocaleDateString('en-US', { month: 'long', year: 'numeric' }); - const nextDue = endDate.toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric' }); + const startDate = parseDate(data.startDate); + const endDate = parseDate(data.endDate); + const tierSince = startDate?.toLocaleDateString('en-US', { month: 'long', year: 'numeric' }) ?? 'N/A'; + const nextDue = endDate?.toLocaleDateString('en-US', { month: 'long', day: 'numeric', year: 'numeric' }) ?? 'N/A';packages/shared/src/interfaces/analytics-data.interface.ts (1)
1224-1239: Consider discriminated union or generic for response type.The
dataproperty uses a union type which requires type guards in consumer code to safely access entity-specific fields (e.g.,FOUNDATION_IDvsPROJECT_ID). Consider using a discriminated union with a type field, or a generic interface:// Option 1: Discriminated union export interface HealthMetricsDailyResponse<T extends 'foundation' | 'project'> { data: T extends 'foundation' ? FoundationHealthMetricsDailyRow[] : ProjectHealthMetricsDailyRow[]; // ... } // Option 2: Add entityType discriminator export interface HealthMetricsDailyResponse { entityType: 'foundation' | 'project'; data: FoundationHealthMetricsDailyRow[] | ProjectHealthMetricsDailyRow[]; // ... }As per coding guidelines, interfaces are preferred over union types for maintainability.
📜 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/components/organization-involvement/organization-involvement.component.ts(6 hunks)apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts(19 hunks)apps/lfx-one/src/app/shared/services/analytics.service.ts(4 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(2 hunks)apps/lfx-one/src/server/routes/analytics.route.ts(2 hunks)apps/lfx-one/src/server/services/organization.service.ts(2 hunks)packages/shared/src/constants/dashboard-metrics.constants.ts(1 hunks)packages/shared/src/interfaces/analytics-data.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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:
packages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/app/shared/services/analytics.service.tspackages/shared/src/interfaces/analytics-data.interface.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
packages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/app/shared/services/analytics.service.tspackages/shared/src/interfaces/analytics-data.interface.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:
packages/shared/src/constants/dashboard-metrics.constants.tsapps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.tsapps/lfx-one/src/server/routes/analytics.route.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.service.tsapps/lfx-one/src/app/shared/services/analytics.service.tspackages/shared/src/interfaces/analytics-data.interface.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/constants/dashboard-metrics.constants.tspackages/shared/src/interfaces/analytics-data.interface.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/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.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/routes/analytics.route.tsapps/lfx-one/src/server/controllers/analytics.controller.tsapps/lfx-one/src/server/services/organization.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
🧠 Learnings (2)
📚 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
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.component.ts : Use Angular 19 zoneless change detection with signals for component state management
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.tsapps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
🧬 Code graph analysis (4)
packages/shared/src/constants/dashboard-metrics.constants.ts (4)
packages/shared/src/interfaces/dashboard-metric.interface.ts (2)
CompanyBusFactor(139-148)DashboardMetricCard(29-116)packages/shared/src/constants/colors.constants.ts (1)
lfxColors(9-106)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)packages/shared/src/constants/chart-options.constants.ts (2)
EMPTY_CHART_DATA(11-14)NO_TOOLTIP_CHART_OPTIONS(179-190)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (6)
packages/shared/src/interfaces/dashboard-metric.interface.ts (1)
DashboardMetricCard(29-116)packages/shared/src/constants/dashboard-metrics.constants.ts (2)
MAINTAINER_PROGRESS_METRICS(310-388)CORE_DEVELOPER_PROGRESS_METRICS(232-304)packages/shared/src/interfaces/analytics-data.interface.ts (5)
ActiveWeeksStreakResponse(25-40)UserPullRequestsResponse(66-81)UserCodeCommitsResponse(107-122)FoundationContributorsMentoredResponse(612-632)HealthMetricsDailyResponse(1224-1239)packages/shared/src/constants/colors.constants.ts (1)
lfxColors(9-106)packages/shared/src/utils/color.utils.ts (1)
hexToRgba(28-56)packages/shared/src/utils/date-time.utils.ts (1)
parseLocalDateString(52-65)
apps/lfx-one/src/server/services/organization.service.ts (2)
packages/shared/src/interfaces/analytics-data.interface.ts (10)
CertifiedEmployeesResponse(1472-1497)CertifiedEmployeesRow(1441-1466)MembershipTierResponse(1569-1629)MembershipTierRow(1503-1563)OrganizationMaintainersResponse(1661-1681)OrganizationMaintainersRow(1635-1655)OrganizationContributorsResponse(1713-1733)OrganizationContributorsRow(1687-1707)TrainingEnrollmentsResponse(1821-1856)TrainingEnrollmentDailyRow(1800-1815)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (8)
OrganizationMaintainersResponse(1661-1681)OrganizationContributorsResponse(1713-1733)MembershipTierResponse(1569-1629)CertifiedEmployeesResponse(1472-1497)TrainingEnrollmentsResponse(1821-1856)HealthMetricsDailyResponse(1224-1239)UniqueContributorsDailyResponse(1327-1342)HealthEventsMonthlyResponse(1420-1435)
⏰ 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). (2)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
🔇 Additional comments (16)
apps/lfx-one/src/server/routes/analytics.route.ts (1)
18-74: LGTM!The new analytics routes are well-organized with clear section comments and follow the existing routing pattern consistently. All routes properly delegate to the corresponding controller methods.
apps/lfx-one/src/server/services/organization.service.ts (1)
170-198: LGTM for organization maintainers method.Clean implementation with proper parameterized query, null coalescing for optional fields, and consistent error handling.
apps/lfx-one/src/server/controllers/analytics.controller.ts (2)
219-257: LGTM!The
getMembershipTiercontroller method follows the established patterns with proper validation, structured logging, and error handling using the Logger helper methods.
728-868: LGTM!The new health metrics endpoints (
getHealthMetricsDaily,getUniqueContributorsDaily,getHealthEventsMonthly) are well-implemented with:
- Consistent validation for
slugandentityTypeparameters- Proper entityType validation restricting to 'foundation' | 'project'
- Structured logging with relevant metrics
- Standard error handling pattern
apps/lfx-one/src/app/shared/services/analytics.service.ts (2)
113-149: LGTM!The renamed
getOrganizationMaintainersand newgetOrganizationContributorsmethods properly map to the backend endpoints with appropriate fallback responses that match the interface shapes.
452-523: LGTM!The new daily/monthly health metrics methods (
getHealthMetricsDaily,getUniqueContributorsDaily,getHealthEventsMonthly) are well-implemented with:
- Proper parameter handling for
slugandentityType- Consistent error handling with meaningful console logging
- Fallback responses that match the expected interface shapes
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (3)
585-636: LGTM - Health Score metric transformation.The
transformHealthMetricsDailyimplementation follows the established pattern and correctly integrates with the DashboardMetricCard interface. The trend threshold of 50 for health scores is reasonable.
816-839: LGTM - Health metrics data initialization.The
initializeHealthMetricsDailyDatamethod follows the established pattern for data initialization, properly handles the empty slug case, and correctly manages loading state withfinalize.
638-661: LGTM - Persona-based data fetching optimization.The conditional data fetching based on persona (core-developer vs maintainer) is a good optimization that prevents unnecessary API calls and correctly manages loading state for skipped fetches.
apps/lfx-one/src/app/modules/dashboards/components/organization-involvement/organization-involvement.component.ts (4)
398-422: Previous hardcoded value issue has been addressed.The
transformTrainingEnrollmentsmethod now correctly usesdata.totalEnrollments.toString()from the API response instead of the previously hardcoded'156'value. The chart data is also properly populated fromdata.dailyData.
433-463: Good refactoring - Chart options helpers reduce duplication.The
createBarChartOptionsandcreateLineChartOptionshelpers effectively address the previous code duplication concern and provide a clean, reusable way to generate chart options with custom tooltip labels.
42-65: LGTM - Individual loading signals with aggregated state.The use of individual loading signals per data source with an aggregated
isLoadingcomputed signal follows good practices for granular state management while providing a convenient overall loading indicator.
404-420: Verify date string format from API.The date formatting in chart labels assumes
row.dateis a valid date string. If the API returns unexpected formats, this could display "Invalid Date". Consider usingparseLocalDateStringfor consistency with other date handling in the codebase.packages/shared/src/interfaces/analytics-data.interface.ts (3)
1817-1856: LGTM - TrainingEnrollmentsResponse structure.The
TrainingEnrollmentsResponseinterface is well-structured with clear JSDoc documentation and appropriate property types for the cumulative enrollment chart data.
1565-1629: LGTM - MembershipTierResponse interface.The interface is well-documented and follows consistent naming conventions. The note about cleaned tier names (line 1601-1602) provides helpful context for consumers.
1093-1157: LGTM - Well-structured health metrics interfaces.The new
FoundationHealthMetricsDailyRowinterface is comprehensive, with proper JSDoc documentation for each field. The optionalTOTAL_PROJECTS_COUNTandTOTAL_SUB_PROJECTS_COUNTfields (lines 1151-1156) appropriately handle the differences between foundation and project level data.
Summary
Changes
Backend (API Endpoints)
GET /api/analytics/health-metrics-daily- Health metrics daily data from SnowflakeGET /api/analytics/unique-contributors-daily- Unique contributors daily dataGET /api/analytics/health-events-monthly- Health events monthly dataFrontend Components
Shared Package
JIRA
LFXV2-834
Generated with Claude Code