Skip to content

feat(dashboards): integrate UI with maintainer dashboard models (PR review & merge velocity, open vs closed issues) - #146

Merged
asithade merged 10 commits into
mainfrom
feat/lfxv2-718-maitainer-dash-issue-resolution-pr-velocity-models
Nov 12, 2025
Merged

feat(dashboards): integrate UI with maintainer dashboard models (PR review & merge velocity, open vs closed issues)#146
asithade merged 10 commits into
mainfrom
feat/lfxv2-718-maitainer-dash-issue-resolution-pr-velocity-models

Conversation

@mauriciozanettisalomao

@mauriciozanettisalomao mauriciozanettisalomao commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Overview

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with Claude Code

This pull request introduces project-level filtering and context management to the maintainer dashboard, along with new backend endpoints and services to support project-specific analytics data. The main changes include adding a project selection filter, persisting the selected project across sessions, and updating both the frontend and backend to fetch and serve project-specific analytics data.

Frontend: Project Filtering and Context Management

  • Added a project filter dropdown to the maintainer dashboard, allowing users to select a project and view analytics scoped to that project. The selected project is persisted in local storage and managed via a new ProjectContextService, ensuring consistent project context across the app. The dashboard now passes the selected projectId to child components, such as lfx-recent-progress. [1] [2] [3] [4] [5]
  • Improved the recent progress component with a loading state indicator and visual cues for live/placeholder data, as well as enhanced tooltips for metric values. [1] [2]

Frontend: Analytics Service Enhancements

  • Extended the AnalyticsService with new methods to fetch the projects list, project issues resolution data, and project pull requests weekly data from the backend. [1] [2]

Backend: New Analytics Endpoints

  • Implemented new API endpoints in the analytics controller and routes to provide the projects list, project issues resolution, and project pull requests weekly data, all sourced from Snowflake via the new project service methods. [1] [2] [3] [4]

Backend: Project Service Enhancements

  • Updated the ProjectService to support fetching projects with maintainers, project issues resolution, and project pull requests weekly data from Snowflake, enabling the new analytics endpoints. [1] [2] [3]

These changes lay the groundwork for project-scoped analytics and make the dashboard more interactive and user-friendly.


Frontend: Project Filtering and Context Management

  • Added a project filter dropdown to the maintainer dashboard, wired to a new ProjectContextService that persists the selected project and provides it to child components. [1] [2] [3] [4]
  • Modified the dashboard and recent progress components to accept and react to the selected projectId. [1] [2]

Frontend: Analytics Service Enhancements

  • Added methods in AnalyticsService to fetch the projects list, project issues resolution, and pull requests weekly data from new backend endpoints. [1] [2]

Frontend: Recent Progress UI Improvements

  • Enhanced recent progress component with loading state, live data indicators, and improved tooltips for metric values. [1] [2]

Backend: New Analytics Endpoints

  • Added new endpoints to serve projects list, project issues resolution, and pull requests weekly analytics, and wired them to the controller and routes. [1] [2] [3] [4]

Backend: Project Service Enhancements

  • Implemented Snowflake-backed methods in ProjectService to support the new analytics endpoints. [1] [2] [3]

…cent progress component

- Add project filter to recent progress component for dynamic data fetching
- Implement new API endpoints for fetching project issues resolution data
- Enhance recent progress UI with connection status indicators and tooltips
- Update maintainer dashboard to include project selection for analytics
- Refactor analytics service and controller to support new project-related queries

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
Copilot AI review requested due to automatic review settings November 11, 2025 17:02

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conventional Commits FTW!

@coderabbitai

coderabbitai Bot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Adds project-scoped analytics: project selector and ProjectContext service, new frontend signals/transforms and loading/tooltips in RecentProgress, backend analytics endpoints backed by ProjectService/Snowflake, new analytics types, chart option constants, and a local YYYY‑MM‑DD date parser.

Changes

Cohort / File(s) Summary
Recent Progress — Template
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
Add loading branch and centered loader; per-item header with label + status dot (connected vs placeholder) and tooltip; conditional value tooltips; adjusted layout and chart container sizing; loop moved inside non-loading branch.
Recent Progress — Logic
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
Add readonly projectId from ProjectContextService; introduce project-scoped data signals (issues resolution, PRs weekly) and tooltip stores; implement initializeXData methods, centralized loading orchestration and defensive tooltip callbacks; standardize date parsing and enrich tooltipText fields; expose project-scoped metrics.
Maintainer Dashboard — Template
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html
Add Maintainer Filters UI: lfx-select project picker bound to form control (selectedProjectId) placed before dashboard grid.
Maintainer Dashboard — Logic
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts
Add reactive FormGroup with selectedProjectId; inject AnalyticsService and ProjectContextService; load projects → signal with error fallback; expose availableProjects signal and sync selection to ProjectContext; include SelectComponent and ReactiveFormsModule.
Frontend Analytics Service
apps/lfx-one/src/app/shared/services/analytics.service.ts
Add getProjects(), getProjectIssuesResolution(projectId), getProjectPullRequestsWeekly(projectId) calling new backend endpoints with error handling and safe defaults.
Backend — Controller & Routes
apps/lfx-one/src/server/controllers/analytics.controller.ts, apps/lfx-one/src/server/routes/analytics.route.ts
Add endpoints /api/analytics/projects, /api/analytics/project-issues-resolution, /api/analytics/project-pull-requests-weekly; wire ProjectService, validate projectId, log metrics, and forward errors.
Backend — Project Service (Snowflake)
apps/lfx-one/src/server/services/project.service.ts
Integrate SnowflakeService; add getProjectsWithMaintainersList(), getProjectIssuesResolution(projectId), getProjectPullRequestsWeekly(projectId) with Snowflake queries, aggregates, fallback resolution and returned response shapes.
Shared — Interfaces
packages/shared/src/interfaces/analytics-data.interface.ts, packages/shared/src/interfaces/components.interface.ts, packages/shared/src/interfaces/project.interface.ts
Add Snowflake-oriented analytics interfaces and response types; make ProgressItem.trend optional; add tooltipText? and isConnected? to ProgressItemWithChart; add ProjectContext interface.
Shared — Utils & Constants
packages/shared/src/utils/date-time.utils.ts, packages/shared/src/constants/progress-metrics.constants.ts
Add parseLocalDateString(dateString: string): Date; introduce PROGRESS_* chart option constants (base, line, bar, dual-line, bar-with-footer) and update issues-trend to dual datasets with enhanced tooltip callbacks.
Project Context Service
apps/lfx-one/src/app/shared/services/project-context.service.ts
Add ProjectContextService exposing persistent selectedProject writable signal, availableProjects, setProject, getProjectId, and localStorage persistence helpers.
Misc — Small changes
apps/lfx-one/src/app/shared/services/persona.service.ts, apps/lfx-one/src/server/services/organization.service.ts, apps/lfx-one/src/server/services/user.service.ts
Set default persona to maintainer with persisted load guard; remove autogenerated header comments from OrganizationService and UserService JSDoc.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant MaintainerUI as Maintainer Dashboard UI
    participant Filter as Project Filter
    participant Recent as RecentProgress Component
    participant Frontend as AnalyticsService (client)
    participant API as Analytics Controller
    participant ProjectSvc as ProjectService (server)
    participant Snowflake as Snowflake DB

    User->>Filter: select project
    Filter->>MaintainerUI: update form.selectedProjectId
    MaintainerUI->>ProjectSvc: set ProjectContext (via ProjectContextService)
    MaintainerUI->>Recent: Recent reads projectId
    Recent->>Frontend: getProjects()
    Frontend->>API: GET /api/analytics/projects
    API->>ProjectSvc: getProjectsWithMaintainersList()
    ProjectSvc->>Snowflake: SQL (projects)
    Snowflake-->>ProjectSvc: rows
    ProjectSvc-->>API: ProjectsListResponse
    API-->>Frontend: response
    Frontend-->>Recent: projects signal updated
    Recent->>Frontend: getProjectIssuesResolution(projectId)
    Frontend->>API: GET /api/analytics/project-issues-resolution?projectId=...
    API->>ProjectSvc: getProjectIssuesResolution(projectId)
    ProjectSvc->>Snowflake: SQL (daily + aggregates)
    Snowflake-->>ProjectSvc: rows
    ProjectSvc-->>API: ProjectIssuesResolutionResponse
    API-->>Frontend: response
    Frontend-->>Recent: issues signal
    Recent->>Recent: transform -> ProgressItemWithChart (datasets + tooltip payloads)
    Recent->>User: render loader → charts, status dot, and tooltips
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Files/areas to focus on:
    • transformProjectIssuesResolution and transformProjectPullRequestsWeekly (dataset ordering, tooltip/footer payloads).
    • Signal/effect loading aggregation and toSignal/toObservable use in recent-progress.component.ts.
    • SQL queries, aggregations, and fallback behavior in project.service.ts.
    • API response shapes vs. frontend transform expectations.
    • ProjectContextService localStorage persistence and form→context synchronization.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The linked issue LFXV2-710 requires UI updates to the meeting-join component with RSVP functionality and compact date formatting. However, the changeset focuses on maintainer dashboard project filtering, analytics endpoints, and recent progress component enhancements with no changes to meeting-join component. The PR changes do not address the meeting-join component UI updates specified in LFXV2-710. Verify that the correct issue is linked or that meeting-join component changes are included in this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat(dashboards): integrate UI with maintainer dashboard models (PR review & merge velocity, open vs closed issues)' accurately describes the primary changes: integrating UI with project-scoped analytics models for the maintainer dashboard.
Description check ✅ Passed The PR description comprehensively documents all major changes including project filtering, ProjectContextService, analytics endpoints, and backend service enhancements, all directly related to the changeset.
Out of Scope Changes check ✅ Passed All changes in the PR align with project-scoped filtering and analytics integration for the maintainer dashboard. Updates to persona.service.ts, organization.service.ts, and user.service.ts are minor documentation or default value changes supporting the overall scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 7c88d99 and 65f2988.

📒 Files selected for processing (1)
  • apps/lfx-one/src/app/shared/services/persona.service.ts (2 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Nov 11, 2025

Copy link
Copy Markdown

🚀 Deployment Status

Your branch has been deployed to: https://ui-pr-146.dev.v2.cluster.linuxfound.info

Deployment Details:

  • Environment: Development
  • Namespace: ui-pr-146
  • ArgoCD App: ui-pr-146

The deployment will be automatically removed when this PR is closed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
packages/shared/src/constants/progress-metrics.constants.ts (2)

238-276: Consider alignment between metric value and chart visualization.

The metric displays a static value of "89%" with subtitle "Issue resolution rate", but the chart now visualizes two separate trends: "Opened Issues" and "Closed Issues". This dual-dataset approach is a good improvement for showing the relationship between opened and closed issues. However, users might expect the 89% value to be derived from or clearly related to the chart data.

Consider one of the following:

  • Update the value/subtitle to better reflect the dual-metric nature (e.g., "Opened vs Closed" trend)
  • Or ensure the backend will provide a calculated resolution rate that corresponds to the ratio shown in the chart

The current mock data ranges (5-15 for opened, 8-18 for closed) show closed issues consistently higher, which is positive, but may not clearly demonstrate the value "89%" to users.


313-318: Optional: Simplify static callback.

The labelPointStyle callback returns a static object that never changes. While this works correctly, it adds minimal value as a callback since it doesn't use the callback parameters.

If Chart.js tooltip configuration supports setting pointStyle directly without a callback, consider simplifying:

-          callbacks: {
-            labelPointStyle: () => {
-              return {
-                pointStyle: 'circle',
-                rotation: 0,
-              };
-            },
-          },

However, if the callback structure is required by Chart.js or if you plan to make this dynamic based on dataset/index in the future, the current implementation is fine.

apps/lfx-one/src/server/services/project.service.ts (1)

660-660: Consider making the LIMIT configurable.

The hardcoded LIMIT 90 restricts results to 90 days. Consider adding a parameter or configuration constant to make this limit adjustable for different use cases.

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (1)

365-429: Consider extracting tooltip configuration to reduce verbosity.

The tooltip configuration (lines 365-429) is highly detailed and could be extracted to a shared constant or helper function if this pattern is reused across multiple charts. This would improve maintainability.

Example refactor:

// Add to a shared constants file or at component level
const DETAILED_TOOLTIP_CONFIG = {
  enabled: true,
  mode: 'index' as const,
  intersect: false,
  yAlign: 'bottom' as const,
  position: 'nearest' as const,
  backgroundColor: 'rgba(255, 255, 255, 0.98)',
  titleColor: '#1f2937',
  bodyColor: '#4b5563',
  // ... rest of config
};

// Then in the method:
chartOptions: {
  // ...
  plugins: {
    legend: { display: false },
    tooltip: {
      ...DETAILED_TOOLTIP_CONFIG,
      callbacks: {
        // Custom callbacks here
      },
    },
  },
}
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8f93946 and 2dded9d.

📒 Files selected for processing (12)
  • 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 (8 hunks)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (1 hunks)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
  • apps/lfx-one/src/app/shared/services/analytics.service.ts (2 hunks)
  • apps/lfx-one/src/server/controllers/analytics.controller.ts (3 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/progress-metrics.constants.ts (1 hunks)
  • packages/shared/src/interfaces/analytics-data.interface.ts (1 hunks)
  • packages/shared/src/interfaces/components.interface.ts (1 hunks)
  • packages/shared/src/utils/date-time.utils.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/maintainer/maintainer-dashboard.component.html
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.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/maintainer/maintainer-dashboard.component.html
  • apps/lfx-one/src/server/routes/analytics.route.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/shared/services/analytics.service.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
  • packages/shared/src/utils/date-time.utils.ts
  • packages/shared/src/interfaces/components.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • packages/shared/src/constants/progress-metrics.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:

  • apps/lfx-one/src/server/routes/analytics.route.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/shared/services/analytics.service.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
  • packages/shared/src/utils/date-time.utils.ts
  • packages/shared/src/interfaces/components.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • packages/shared/src/constants/progress-metrics.constants.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not nest ternary expressions

Files:

  • apps/lfx-one/src/server/routes/analytics.route.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/shared/services/analytics.service.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts
  • packages/shared/src/utils/date-time.utils.ts
  • packages/shared/src/interfaces/components.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • packages/shared/src/constants/progress-metrics.constants.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/components.interface.ts
  • packages/shared/src/interfaces/analytics-data.interface.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/progress-metrics.constants.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: All PrimeNG components are wrapped in LFX components to keep UI library independence

Applied to files:

  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.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/maintainer/maintainer-dashboard.component.ts
🧬 Code graph analysis (3)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
  • ProjectsListResponse (483-492)
  • ProjectIssuesResolutionResponse (559-589)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (4)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
  • Component (14-35)
packages/shared/src/utils/date-time.utils.ts (1)
  • parseLocalDateString (51-54)
packages/shared/src/interfaces/analytics-data.interface.ts (1)
  • ProjectIssuesResolutionResponse (559-589)
packages/shared/src/interfaces/components.interface.ts (1)
  • ProgressItemWithChart (321-334)
apps/lfx-one/src/server/services/project.service.ts (2)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
  • SnowflakeService (26-400)
packages/shared/src/interfaces/analytics-data.interface.ts (5)
  • ProjectsListResponse (483-492)
  • ProjectRow (463-478)
  • ProjectIssuesResolutionResponse (559-589)
  • ProjectIssuesResolutionRow (498-528)
  • ProjectIssuesResolutionAggregatedRow (534-554)
⏰ 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: Agent
  • GitHub Check: build-and-push
🔇 Additional comments (25)
packages/shared/src/constants/progress-metrics.constants.ts (3)

1-418: Verify PR objectives and linked issue.

The PR objectives reference LFXV2-710, which describes changes to a "meeting join component UI to match React design" with RSVP containers and MeetingTimePipe updates. However, the actual code changes in this file modify maintainer dashboard metrics constants (specifically the "Open vs Closed Issues Trend" chart configuration). This appears to be a significant mismatch between the linked issue and the actual implementation.

Please confirm whether the correct issue is linked to this PR, or update the PR description to accurately reflect the dashboard metrics changes.


280-321: Excellent tooltip and interaction improvements!

The addition of interactive tooltips with index mode is a significant UX enhancement for the dual-dataset chart. Users can now clearly see both "Opened Issues" and "Closed Issues" values when hovering over any point on the timeline. The custom styling (colors, padding, fonts, border radius) makes the tooltip polished and professional.


248-274: Well-configured hover states for both datasets.

The hover properties (pointHoverRadius, pointHoverBackgroundColor, pointHoverBorderColor, pointHoverBorderWidth) are consistently applied to both datasets with appropriate values. The white border provides good visual contrast against both the red and green colors, creating a polished interactive experience.

apps/lfx-one/src/server/routes/analytics.route.ts (1)

24-28: LGTM!

The new routes follow the established pattern and properly delegate to the analytics controller. The implementation is clean and consistent with existing endpoints.

packages/shared/src/utils/date-time.utils.ts (1)

45-54: LGTM!

The parseLocalDateString utility correctly handles local date parsing to avoid timezone shifting issues. The implementation properly accounts for zero-indexed months and includes clear documentation.

apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (2)

5-27: LGTM!

The filter section is well-implemented with proper accessibility attributes, data-testid for testing, and uses the wrapped LFX select component. The filter configuration supports search functionality and clear button for good UX.


32-32: LGTM!

The projectId binding correctly passes the selected project to the Recent Progress component using optional chaining with a fallback to undefined.

packages/shared/src/interfaces/components.interface.ts (1)

330-333: LGTM!

The optional properties extend the interface cleanly without breaking existing implementations. These additions enable useful UI features like hover tooltips and live data status indicators.

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (3)

48-57: LGTM!

The connection status indicator provides clear visual feedback with appropriate tooltips. The implementation uses proper conditional styling and PrimeNG tooltip directive.


58-60: LGTM!

Increasing the chart container height from h-8 to h-24 improves chart readability and provides better data visualization.


62-62: LGTM!

The tooltip binding with conditional cursor-help class provides a good user experience when additional information is available.

apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)

4-8: Clean implementation with modern Angular patterns.

The component properly uses signals, computed values, and reactive forms. The structure follows Angular best practices with dependency injection and signal-based reactivity.

Also applies to: 17-17, 21-34

apps/lfx-one/src/app/shared/services/analytics.service.ts (2)

224-239: LGTM!

The getProjects method follows the established service pattern with proper error handling and safe default values. The implementation is consistent with other methods in the service.


241-263: LGTM!

The getProjectIssuesResolution method properly handles optional projectId filtering, includes comprehensive error handling, and returns safe defaults. The implementation matches the service's existing patterns.

apps/lfx-one/src/server/controllers/analytics.controller.ts (3)

10-10: LGTM!

The ProjectService is properly imported, declared, and initialized following the existing controller pattern.

Also applies to: 22-22, 27-27


281-302: LGTM!

The getProjects endpoint implementation follows the established controller pattern with proper logging, error handling, and success metrics. The implementation is consistent with other endpoints in the controller.


304-333: LGTM!

The getProjectIssuesResolution endpoint properly handles optional projectId filtering and includes comprehensive logging with relevant metrics. Error handling follows the established pattern.

apps/lfx-one/src/server/services/project.service.ts (3)

6-16: LGTM: Clean integration of SnowflakeService.

The imports and initialization of SnowflakeService follow the established pattern in this service class.

Also applies to: 25-25, 35-35, 42-42


598-619: LGTM: Clean implementation of projects list endpoint.

The query is straightforward and the transformation from Snowflake column naming to camelCase API format is correct.


621-696: Verify parallel query consistency with your data platform team.

The concern is valid—the method executes queries against two separate tables (PROJECT_ISSUES_RESOLUTION_DAILY and PROJECT_ISSUES_RESOLUTION) in parallel, which could yield inconsistent snapshots if tables aren't synchronized at the Snowflake/ETL level.

This pattern is established elsewhere in the codebase (e.g., organization.service.ts), but without access to your Snowflake table update guarantees or ETL documentation, the consistency risk cannot be confirmed within the codebase itself.

Confirm with your data/analytics team:

  • Are these tables updated atomically or sequentially?
  • Is the aggregated table derived deterministically from the daily table?
  • Is temporal inconsistency (time-of-read differences) acceptable for this use case?

If consistency is critical, consider adding validation logic to detect and handle mismatches between daily sums and aggregated values.

packages/shared/src/interfaces/analytics-data.interface.ts (1)

459-589: LGTM: Well-structured interface definitions.

The new interfaces follow the established pattern in this file:

  • Raw Snowflake types (ProjectRow, ProjectIssuesResolutionRow, ProjectIssuesResolutionAggregatedRow) use UPPER_CASE field naming matching database columns
  • API response types (ProjectsListResponse, ProjectIssuesResolutionResponse) use camelCase for consistency with the frontend
  • Comprehensive JSDoc comments document each field's purpose
  • Proper separation between daily metrics and aggregated metrics
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (4)

33-34: Excellent reactive pattern for project filtering.

The implementation correctly uses Angular signals and RxJS operators to reactively fetch new data when the projectId input changes. The use of toObservable(this.projectId).pipe(switchMap(...)) ensures that the previous request is cancelled when projectId changes, and toSignal with sensible initial values prevents UI flicker.

Also applies to: 72-88


300-437: Well-implemented chart transformation with defensive coding.

The method correctly:

  • Reverses data to show oldest-first for intuitive left-to-right time progression
  • Uses two datasets with appropriate colors (red for opened issues, green for closed)
  • Includes comprehensive tooltip configuration for better UX
  • Implements try/catch blocks in tooltip callbacks to prevent runtime errors
  • Uses parseLocalDateString for correct date parsing
  • Formats numbers with toLocaleString() for better readability

167-168: LGTM: Improved tooltip positioning.

Setting yAlign: 'bottom' and position: 'nearest' provides better tooltip positioning and prevents tooltips from overlapping with the cursor or chart elements.

Also applies to: 221-222


225-227: LGTM: Consistent date parsing across charts.

Using parseLocalDateString utility ensures consistent date parsing across all chart tooltips and prevents timezone-related issues.

Also applies to: 281-283

Comment thread apps/lfx-one/src/server/services/project.service.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates the Maintainer Dashboard UI with backend Snowflake data models for displaying PR review metrics, merge velocity, and open vs closed issues trends. The implementation adds new API endpoints, services, and frontend components to visualize real-time project metrics.

Key Changes:

  • Added Snowflake-backed API endpoints for project data and issues resolution metrics
  • Implemented project filter dropdown with live data on Maintainer Dashboard
  • Enhanced chart visualizations with interactive tooltips and dual-dataset support for issues tracking
  • Added status indicators to distinguish between live and placeholder data

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
packages/shared/src/utils/date-time.utils.ts Added parseLocalDateString utility to parse YYYY-MM-DD dates as local time, avoiding timezone issues
packages/shared/src/interfaces/components.interface.ts Extended ProgressItemWithChart interface with tooltip and connection status properties
packages/shared/src/interfaces/analytics-data.interface.ts Added interfaces for project lists and issues resolution data from Snowflake
packages/shared/src/constants/progress-metrics.constants.ts Updated mock data for Open vs Closed Issues chart with dual datasets and enhanced tooltip configuration
apps/lfx-one/src/server/services/project.service.ts Implemented getProjectsList and getProjectIssuesResolution methods with parallel query execution
apps/lfx-one/src/server/routes/analytics.route.ts Added routes for /projects and /project-issues-resolution endpoints
apps/lfx-one/src/server/controllers/analytics.controller.ts Added controller methods for new project and issues resolution endpoints with logging
apps/lfx-one/src/app/shared/services/analytics.service.ts Implemented frontend service methods with error handling and default responses
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts Added project filter form and reactive data fetching
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html Added project filter dropdown UI with search functionality
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts Integrated live issues resolution data with chart transformation and reactive project filtering
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html Added connection status indicators and increased chart height for better visualization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/lfx-one/src/server/services/project.service.ts Outdated
Comment thread apps/lfx-one/src/server/services/project.service.ts Outdated
Comment thread packages/shared/src/utils/date-time.utils.ts Outdated
- Implement new API endpoint to fetch project pull requests weekly data from Snowflake
- Enhance RecentProgressComponent to display PR review and merge velocity metrics
- Add transformation logic for API response to chart format
- Update analytics service and controller to support new data retrieval
- Introduce new TypeScript interfaces for project pull requests weekly data

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
apps/lfx-one/src/server/services/project.service.ts (1)

664-672: Fix the all-project resolution rate calculation.

Averaging RESOLUTION_RATE_PCT causes tiny projects to influence the global rate as much as large ones. Compute the rate from the summed totals (with a NULLIF guard) so the metric reflects actual opened vs. closed counts. This was flagged previously and still needs correction.

       aggregatedQuery = `
         SELECT 
           SUM(OPENED_ISSUES) AS OPENED_ISSUES,
           SUM(CLOSED_ISSUES) AS CLOSED_ISSUES,
-          ROUND(AVG(RESOLUTION_RATE_PCT), 2) AS RESOLUTION_RATE_PCT,
+          ROUND((SUM(CLOSED_ISSUES) * 100.0) / NULLIF(SUM(OPENED_ISSUES), 0), 2) AS RESOLUTION_RATE_PCT,
           ROUND(AVG(MEDIAN_DAYS_TO_CLOSE), 2) AS MEDIAN_DAYS_TO_CLOSE
         FROM ANALYTICS.PLATINUM_LFX_ONE.PROJECT_ISSUES_RESOLUTION
       `;
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2dded9d and 5658bce.

📒 Files selected for processing (7)
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (8 hunks)
  • apps/lfx-one/src/app/shared/services/analytics.service.ts (2 hunks)
  • apps/lfx-one/src/server/controllers/analytics.controller.ts (3 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/interfaces/analytics-data.interface.ts (1 hunks)
  • packages/shared/src/interfaces/components.interface.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/lfx-one/src/server/routes/analytics.route.ts
  • packages/shared/src/interfaces/components.interface.ts
🧰 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.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.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.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.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.ts
  • packages/shared/src/interfaces/analytics-data.interface.ts
  • apps/lfx-one/src/server/services/project.service.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.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-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 **/*.{ts,tsx} : Use TypeScript interfaces instead of union types for better maintainability

Applied to files:

  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.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 (3)
  • ProjectsListResponse (483-492)
  • ProjectIssuesResolutionResponse (559-589)
  • ProjectPullRequestsWeeklyResponse (625-645)
apps/lfx-one/src/server/services/project.service.ts (2)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
  • SnowflakeService (26-400)
packages/shared/src/interfaces/analytics-data.interface.ts (7)
  • ProjectsListResponse (483-492)
  • ProjectRow (463-478)
  • ProjectIssuesResolutionResponse (559-589)
  • ProjectIssuesResolutionRow (498-528)
  • ProjectIssuesResolutionAggregatedRow (534-554)
  • ProjectPullRequestsWeeklyResponse (625-645)
  • ProjectPullRequestsWeeklyRow (595-620)
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
apps/lfx-one/src/server/services/project.service.ts (1)
  • ProjectService (32-736)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (4)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
  • Component (14-35)
packages/shared/src/utils/date-time.utils.ts (1)
  • parseLocalDateString (51-54)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
  • ProjectIssuesResolutionResponse (559-589)
  • ProjectPullRequestsWeeklyResponse (625-645)
packages/shared/src/interfaces/components.interface.ts (1)
  • ProgressItemWithChart (321-334)

Comment thread apps/lfx-one/src/server/services/project.service.ts
@mauriciozanettisalomao mauriciozanettisalomao changed the title feat(dashboards): WIP - Integrate UI with Maintainer Dashboard Models (PR Review & Merge Velocity, Open vs Closed Issues) feat(dashboards): Integrate UI with Maintainer Dashboard Models (PR Review & Merge Velocity, Open vs Closed Issues) Nov 12, 2025
…e and tooltips

- Implement loading state in the RecentProgressComponent to improve user experience during data fetching.
- Add tooltip functionality for specific metrics to provide additional context when hovering over items.
- Refactor data fetching logic to utilize signals for loading states and ensure proper handling of asynchronous data retrieval.
- Update HTML template to conditionally render loading indicators and tooltips based on data availability.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
@mauriciozanettisalomao mauriciozanettisalomao changed the title feat(dashboards): Integrate UI with Maintainer Dashboard Models (PR Review & Merge Velocity, Open vs Closed Issues) feat(dashboards): integrate UI with maintainer dashboard models (PR review & merge velocity, open vs closed issues) Nov 12, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
packages/shared/src/constants/progress-metrics.constants.ts (1)

278-319: Enhanced tooltip configuration looks good.

The detailed tooltip configuration appropriately enables interactivity for the multi-dataset chart, with mode: 'index' ensuring both datasets' values display when hovering. The styling choices (colors, padding, fonts, corner radius) create a polished, modern tooltip experience.

Optional: Consider extracting tooltip configuration for reusability.

The tooltip configuration spans ~40 lines with detailed styling. If other charts in the dashboard ecosystem need similar interactive tooltips, consider extracting this configuration into a shared constant or factory function (e.g., createInteractiveTooltipConfig()) to promote consistency and reduce duplication.

Optional: Note on consistency.

This is the only metric across both CORE_DEVELOPER_PROGRESS_METRICS and MAINTAINER_PROGRESS_METRICS with tooltips enabled. While this makes sense for a multi-dataset comparison chart, consider whether other metrics might also benefit from tooltips for improved user experience.

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (2)

567-576: Consider error handling for date parsing in tooltip callbacks.

The parseLocalDateString() call at line 569 can throw an error if the date format is invalid. While the data should always be in YYYY-MM-DD format from Snowflake, defensive error handling would prevent chart rendering failures if the data format changes.

               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 (e) {
+                  console.error('Error parsing date in tooltip:', e);
+                  return context[0]?.label || '';
+                }
               },

383-400: Add consistent error handling for date parsing.

The tooltip callbacks in transformPullRequestsMerged and transformCodeCommits (lines 439-443) lack the defensive error handling present in the newer transformProjectPullRequestsWeekly method. For consistency and resilience, add try-catch blocks around parseLocalDateString calls.

Apply this pattern to both methods:

               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 (e) {
+                  console.error('Error parsing date in tooltip:', e);
+                  return context[0]?.label || '';
+                }
               },
apps/lfx-one/src/server/services/project.service.ts (1)

645-656: Consider adding a row limit to the daily query.

The daily query has no LIMIT clause and returns all historical data for a project. Depending on the project's age and data retention, this could return thousands of rows. Consider adding a reasonable limit (e.g., 90 or 180 days) for optimal chart rendering and API performance.

       FROM ANALYTICS.PLATINUM_LFX_ONE.PROJECT_ISSUES_RESOLUTION_DAILY
       WHERE PROJECT_ID = ?
       ORDER BY METRIC_DATE DESC
+      LIMIT 90
     `;
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5658bce and 3b99071.

📒 Files selected for processing (10)
  • 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 (10 hunks)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
  • apps/lfx-one/src/app/shared/services/analytics.service.ts (2 hunks)
  • apps/lfx-one/src/server/controllers/analytics.controller.ts (3 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/progress-metrics.constants.ts (1 hunks)
  • packages/shared/src/interfaces/analytics-data.interface.ts (1 hunks)
  • packages/shared/src/utils/date-time.utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • apps/lfx-one/src/app/shared/services/analytics.service.ts
  • apps/lfx-one/src/server/routes/analytics.route.ts
  • packages/shared/src/utils/date-time.utils.ts
  • apps/lfx-one/src/server/controllers/analytics.controller.ts
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (4)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
  • Component (14-35)
packages/shared/src/utils/date-time.utils.ts (1)
  • parseLocalDateString (52-65)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
  • ProjectIssuesResolutionResponse (505-535)
  • ProjectPullRequestsWeeklyResponse (571-591)
packages/shared/src/interfaces/components.interface.ts (1)
  • ProgressItemWithChart (321-334)
apps/lfx-one/src/server/services/project.service.ts (2)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
  • SnowflakeService (26-400)
packages/shared/src/interfaces/analytics-data.interface.ts (6)
  • ProjectsListResponse (429-438)
  • ProjectRow (409-424)
  • ProjectIssuesResolutionRow (444-474)
  • ProjectIssuesResolutionAggregatedRow (480-500)
  • ProjectPullRequestsWeeklyResponse (571-591)
  • ProjectPullRequestsWeeklyRow (541-566)
🔇 Additional comments (9)
packages/shared/src/constants/progress-metrics.constants.ts (2)

1-3: Note: PR objectives appear to reference a different feature.

The PR objectives mention updating a "meeting join component UI" with RSVP functionality and meeting time formats (LFXV2-710), but the actual changes in this file relate to maintainer dashboard progress metrics. The PR title and AI summary correctly describe the dashboard integration work. This mismatch suggests the PR objectives may have been copied from a different pull request.


247-272: LGTM! Two-dataset structure properly configured.

The addition of separate "Opened Issues" and "Closed Issues" datasets with distinct colors, proper hover styling, and fill: false correctly supports trend comparison. The mock data ranges (5-15 for opened, 8-18 for closed) align well with the 89% resolution rate displayed.

Note: This file continues to use generateMockData() which is appropriate as a fallback or initial state, given the AI summary indicates real data will be fetched from backend analytics endpoints.

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html (2)

43-51: LGTM!

The loading state implementation is clean with appropriate visual feedback and centered layout.


71-81: LGTM! Tooltip implementation is safe.

The [escape]="false" binding is appropriate here since the tooltip content from getTooltipContent() contains only structured HTML with sanitized numeric data (formatted counts and averages).

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (2)

253-288: LGTM! Proper use of effects for tooltip data.

The constructor effect correctly updates tooltip stores outside of computed signals, avoiding write-during-read errors. The calculations properly handle edge cases with length checks and appropriate rounding.


593-729: LGTM! Robust implementation with proper error handling.

The method correctly handles data transformation with:

  • Appropriate data reversal for chronological display
  • Weighted average calculations with zero-division protection
  • Defensive try-catch blocks in tooltip callbacks
  • Proper rounding for display values
apps/lfx-one/src/server/services/project.service.ts (2)

604-621: LGTM! Clean implementation.

The method properly queries Snowflake with parameterized execution and transforms the response to API-friendly camelCase format.


701-729: LGTM! Proper weighted average calculation.

The method correctly implements a weighted average for merge time by multiplying each week's average by its PR count, addressing the statistical concern from previous reviews. The 26-week limit provides a reasonable data window.

packages/shared/src/interfaces/analytics-data.interface.ts (1)

362-591: LGTM! Well-structured interface definitions.

The new interfaces follow consistent conventions:

  • Raw Snowflake rows use ALL_CAPS field names
  • API responses use camelCase
  • Comprehensive JSDoc documentation
  • Proper separation between data and aggregated metrics

- Update the analytics service and controller to require a projectId parameter for fetching project issues resolution data.
- Modify RecentProgressComponent to handle cases where projectId is not provided, ensuring a default response is returned.
- Refactor related documentation to reflect the change from optional to required projectId.

This change improves data integrity and user experience by preventing requests without a valid project identifier.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
…tion in maintainer dashboard

- Introduced ProjectContextService to manage the selected project state and persist it in local storage.
- Updated MaintainerDashboardComponent to utilize the new service for project selection, ensuring the projectId is set based on user selection and restoring previously selected projects.
- Enhanced form handling to automatically select the first project if no project is currently selected.

This change improves user experience by maintaining project context across sessions and simplifying project selection in the dashboard.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
apps/lfx-one/src/app/shared/services/project-context.service.ts (2)

19-25: Consider loading full project details on initialization.

The constructor initializes selectedProject with only projectId from storage, leaving name and slug as empty strings. While the comment indicates this is intentional (to be set when projects are loaded), this creates a temporary inconsistent state where the signal contains a partially populated object.

Consider either:

  1. Storing the complete ProjectContext object (as JSON) in localStorage instead of just the ID
  2. Deferring the signal update until full project data is available
  3. Adding a loaded flag to ProjectContext to distinguish between partial and complete data

Example for storing complete object:

  private loadStoredProjectId(): string | null {
    try {
-     const stored = localStorage.getItem(this.storageKey);
-     return stored || null;
+     const stored = localStorage.getItem(this.storageKey);
+     return stored ? JSON.parse(stored) : null;
    } catch {
      // Invalid data in localStorage, ignore
      return null;
    }
  }
  
  private persistProjectId(projectId: string): void {
-   localStorage.setItem(this.storageKey, projectId);
+   localStorage.setItem(this.storageKey, JSON.stringify(this.selectedProject()));
  }

54-62: Add error logging for storage failures.

The try-catch silently ignores all localStorage errors. While this prevents the application from breaking, it makes debugging storage issues difficult.

Consider logging storage errors:

  private loadStoredProjectId(): string | null {
    try {
      const stored = localStorage.getItem(this.storageKey);
      return stored || null;
-   } catch {
+   } catch (error) {
+     console.warn('Failed to load stored project ID:', error);
      // Invalid data in localStorage, ignore
      return null;
    }
  }
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)

56-80: Consider extracting initialization logic from effect.

The effect contains complex initialization logic with multiple conditional branches. This makes it harder to test and reason about.

Consider extracting the initialization logic to a separate method:

constructor() {
  // Subscribe to form changes to update the project context service
  this.filterForm
    .get('projectId')
    ?.valueChanges.pipe(takeUntilDestroyed())
    .subscribe((projectId) => {
      if (projectId) {
        const project = this.availableProjects().find((p) => p.projectId === projectId);
        if (project) {
          this.projectContextService.setProject(project);
        }
      } else {
        this.projectContextService.clearProject();
      }
    });

  // Initialize project selection when projects are loaded
  effect(() => {
    const projects = this.availableProjects();
    
    if (projects.length > 0 && !this.hasInitialized) {
      this.hasInitialized = true;
      this.initializeProjectSelection(projects);
    }
  });
}

private initializeProjectSelection(projects: Array<{ projectId: string; name: string; slug: string }>): void {
  const storedProjectId = untracked(() => this.projectContextService.getProjectId());
  const currentFormValue = untracked(() => this.filterForm.get('projectId')?.value);
  
  // Try to restore stored project
  if (storedProjectId && !currentFormValue) {
    const storedProject = projects.find((p) => p.projectId === storedProjectId);
    if (storedProject) {
      this.filterForm.get('projectId')?.setValue(storedProject.projectId, { emitEvent: true });
      return;
    }
  }
  
  // If no stored project or stored project not found, auto-select first project
  if (!currentFormValue || currentFormValue === '') {
    this.filterForm.get('projectId')?.setValue(projects[0].projectId, { emitEvent: true });
  }
}

This makes the initialization logic:

  • Easier to test independently
  • More readable with clear method name
  • Simpler to modify or extend
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d4f223a and b6abcd4.

📒 Files selected for processing (2)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
  • apps/lfx-one/src/app/shared/services/project-context.service.ts (1 hunks)
🧰 Additional context used
🧠 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/maintainer/maintainer-dashboard.component.ts
⏰ 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 (1)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)

1-81: PR objectives don't match the actual code changes.

The PR objectives reference LFXV2-710, which describes updates to the meeting join component UI (attendee views, RSVP buttons, meeting time format). However, this file implements project context integration and analytics filtering for the maintainer dashboard - completely different functionality.

Please verify the correct issue is linked and update the PR description to accurately reflect the actual changes.

…tainer dashboard

- Implemented error handling in the MaintainerDashboardComponent to manage failures when fetching project data from the analytics service.
- Utilized RxJS's catchError to log errors and return an empty projects array, ensuring the application remains stable during data retrieval issues.

This change enhances the robustness of the dashboard by preventing crashes due to data fetching errors.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2)

69-79: Restoration logic appears redundant.

Lines 73-79 check for storedProjectId && !currentFormValue, but since the form is initialized from ProjectContextService.getProjectId() on line 29, both values should be identical when the effect runs. This makes the condition unlikely to ever be true under normal circumstances.

The refactoring suggested in the previous comment removes this redundancy while adding proper validation.


76-76: Redundant emitEvent option.

The emitEvent: true option is redundant since true is the default value for setValue(). While being explicit is good for clarity, this is a minor style point.

If you prefer to keep the explicit option for clarity, that's perfectly fine. Otherwise, you can simplify:

-this.filterForm.get('projectId')?.setValue(projects[0].projectId, { emitEvent: true });
+this.filterForm.get('projectId')?.setValue(projects[0].projectId);

Also applies to: 83-83

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between b6abcd4 and e0fa471.

📒 Files selected for processing (1)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
🧰 Additional context used
🧠 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/maintainer/maintainer-dashboard.component.ts
⏰ 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 (4)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (4)

4-22: LGTM! Clean integration of dependencies.

The imports and component metadata correctly include all necessary dependencies for the new project filter functionality (signals, reactive forms, RxJS operators, and custom services).


23-30: Good fix for the hardcoded project ID issue.

The form is now properly initialized from ProjectContextService.getProjectId() with a fallback to an empty string, addressing the previous review feedback about the hardcoded Salesforce ID.


32-44: Excellent error handling implementation.

The catchError operator properly handles API failures and returns a safe fallback value, addressing the previous review feedback. The computed signal cleanly derives the available projects list.


46-60: LGTM! Proper subscription management and project sync logic.

The form subscription correctly uses takeUntilDestroyed() for cleanup and appropriately syncs the selected project to ProjectContextService.

…ashboard

- Updated MaintainerDashboardComponent to validate the current project selection against loaded projects.
- Removed the logic for restoring a stored project ID and streamlined the auto-selection of the first project if no valid selection exists.
- Enhanced user experience by ensuring that only valid project selections are set in the form.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2)

32-41: Error handling looks good.

The catchError operator ensures the signal never errors and component reactivity remains stable. The fallback to an empty projects array is a safe default.

Optional improvement: Consider replacing console.error with a dedicated logging service for production environments to enable better error tracking and monitoring.


66-83: Consider handling the empty projects scenario.

The effect only runs when projects.length > 0. If projects load as an empty array:

  1. The form retains its initial value (potentially a project ID from context service)
  2. The context service is not cleared
  3. The hasInitialized flag prevents the effect from running later if projects become available

While this is an edge case (maintainers typically have at least one project), consider adding an else clause to clear the form and context service when no projects are available:

  effect(() => {
    const projects = this.availableProjects();
    
    if (projects.length > 0 && !this.hasInitialized) {
      this.hasInitialized = true;
      
      const currentFormValue = untracked(() => this.filterForm.get('projectId')?.value);
      
      // Validate current selection exists in loaded projects
      if (currentFormValue) {
        const validProject = projects.find((p) => p.projectId === currentFormValue);
        if (validProject) {
          // Valid selection, ensure service has full project object
          this.projectContextService.setProject(validProject);
          return;
        }
      }
      
      // No valid selection - auto-select first project
      this.filterForm.get('projectId')?.setValue(projects[0].projectId, { emitEvent: true });
+   } else if (projects.length === 0 && !this.hasInitialized) {
+     this.hasInitialized = true;
+     this.filterForm.get('projectId')?.setValue('', { emitEvent: true });
+     this.projectContextService.clearProject();
    }
  });
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e0fa471 and ff1338b.

📒 Files selected for processing (1)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
🧰 Additional context used
🧠 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/maintainer/maintainer-dashboard.component.ts
🔇 Additional comments (3)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (3)

1-86: PR objectives don't match the actual code changes.

The PR description references LFXV2-710 about updating the "meeting join component UI to match React design" with RSVP functionality, but this file implements project selection and analytics integration for the maintainer dashboard. The linked issue and PR title appear to be mismatched with the actual changes.

Please verify that the correct Jira ticket is linked and update the PR description to accurately reflect these changes.


47-60: Form change subscription is well-structured.

The subscription correctly:

  • Uses takeUntilDestroyed() for automatic cleanup
  • Updates the project context when a valid project is selected
  • Clears the context when the selection is removed
  • Safely handles the case where the project might not be found

62-84: Initialization effect validates selection correctly.

The effect properly:

  • Validates that the initial projectId exists in the loaded projects list
  • Ensures the context service has the full project object for valid selections
  • Auto-selects the first project when no valid selection exists
  • Uses untracked correctly to avoid creating unnecessary effect dependencies

This addresses the concerns raised in past review comments about validation.

LFXV2-718

- Add SSR safety checks to all localStorage access points in context services
- Change ProjectContextService to store full ProjectContext objects (projectId, name, slug)
- Simplify maintainer dashboard to match board member dashboard pattern
- Centralize project context in RecentProgressComponent using service injection
- Move ProjectContext interface to shared package for reusability
- Extract chart options to shared constants for consistency
- Refactor chart transformation logic for better maintainability

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>
asithade
asithade previously approved these changes Nov 12, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between ff1338b and 7c88d99.

📒 Files selected for processing (13)
  • 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 (5 hunks)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (1 hunks)
  • apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (2 hunks)
  • apps/lfx-one/src/app/shared/services/analytics.service.ts (2 hunks)
  • apps/lfx-one/src/app/shared/services/persona.service.ts (1 hunks)
  • apps/lfx-one/src/app/shared/services/project-context.service.ts (1 hunks)
  • apps/lfx-one/src/server/controllers/analytics.controller.ts (5 hunks)
  • apps/lfx-one/src/server/services/organization.service.ts (0 hunks)
  • apps/lfx-one/src/server/services/project.service.ts (4 hunks)
  • apps/lfx-one/src/server/services/user.service.ts (0 hunks)
  • packages/shared/src/constants/progress-metrics.constants.ts (3 hunks)
  • packages/shared/src/interfaces/project.interface.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • apps/lfx-one/src/server/services/user.service.ts
  • apps/lfx-one/src/server/services/organization.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/lfx-one/src/app/shared/services/project-context.service.ts
  • apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.html
🧰 Additional context used
🧠 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/maintainer/maintainer-dashboard.component.ts
🧬 Code graph analysis (5)
apps/lfx-one/src/app/shared/services/persona.service.ts (1)
packages/shared/src/interfaces/persona.interface.ts (1)
  • PersonaType (8-8)
apps/lfx-one/src/app/shared/services/analytics.service.ts (1)
packages/shared/src/interfaces/analytics-data.interface.ts (3)
  • ProjectsListResponse (429-438)
  • ProjectIssuesResolutionResponse (505-535)
  • ProjectPullRequestsWeeklyResponse (571-591)
apps/lfx-one/src/server/services/project.service.ts (2)
apps/lfx-one/src/server/services/snowflake.service.ts (1)
  • SnowflakeService (26-400)
packages/shared/src/interfaces/analytics-data.interface.ts (6)
  • ProjectsListResponse (429-438)
  • ProjectRow (409-424)
  • ProjectIssuesResolutionRow (444-474)
  • ProjectIssuesResolutionAggregatedRow (480-500)
  • ProjectPullRequestsWeeklyResponse (571-591)
  • ProjectPullRequestsWeeklyRow (541-566)
apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (5)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (1)
  • Component (17-57)
packages/shared/src/constants/progress-metrics.constants.ts (4)
  • PROGRESS_BAR_CHART_OPTIONS (56-71)
  • PROGRESS_LINE_CHART_OPTIONS (35-50)
  • PROGRESS_DUAL_LINE_CHART_OPTIONS (77-137)
  • PROGRESS_BAR_CHART_WITH_FOOTER_OPTIONS (143-198)
packages/shared/src/utils/date-time.utils.ts (1)
  • parseLocalDateString (52-65)
packages/shared/src/interfaces/analytics-data.interface.ts (2)
  • ProjectIssuesResolutionResponse (505-535)
  • ProjectPullRequestsWeeklyResponse (571-591)
packages/shared/src/interfaces/components.interface.ts (1)
  • ProgressItemWithChart (321-334)
apps/lfx-one/src/server/controllers/analytics.controller.ts (1)
apps/lfx-one/src/server/services/project.service.ts (1)
  • ProjectService (32-731)
⏰ 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 (26)
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.html (1)

5-27: LGTM! Clean filter UI implementation.

The project filter section is well-structured with:

  • Proper form control binding to selectedProjectId
  • Accessibility features (label, inputId)
  • Search functionality with filter configuration
  • Responsive layout classes
packages/shared/src/interfaces/project.interface.ts (1)

163-174: LGTM! Well-defined context interface.

The ProjectContext interface is appropriately scoped for application state management with clear fields and documentation.

packages/shared/src/constants/progress-metrics.constants.ts (2)

14-198: LGTM! Well-organized chart configuration constants.

The new chart option constants provide a clean, reusable configuration structure:

  • Clear separation of concerns (base, line, bar, dual-line, bar with footer)
  • Comprehensive tooltip customization for different chart types
  • Consistent styling across all chart types

426-512: LGTM! Proper dual-dataset configuration for Issues Trend.

The refactored "Open vs Closed Issues Trend" metric now uses:

  • Two separate datasets for opened and closed issues
  • Enhanced tooltip configuration with index mode for multi-dataset interaction
  • Proper point styling callbacks for circular indicators
apps/lfx-one/src/app/modules/dashboards/maintainer/maintainer-dashboard.component.ts (3)

4-23: LGTM! Proper component setup and imports.

The component correctly imports and configures:

  • Reactive forms for project selection
  • Required services (AnalyticsService, ProjectContextService)
  • Necessary UI components and modules

24-44: LGTM! Robust service integration with error handling.

The implementation properly:

  • Initializes form control from persisted project context
  • Handles API errors with graceful fallback to empty projects
  • Exposes projects via a computed signal

46-57: LGTM! Proper form-to-context synchronization.

The constructor correctly:

  • Subscribes to form changes with automatic cleanup
  • Validates selection exists in available projects
  • Updates global project context only when valid project found
apps/lfx-one/src/app/shared/services/analytics.service.ts (3)

191-204: LGTM! Clean projects list endpoint.

The getProjects() method properly:

  • Fetches all projects from the analytics endpoint
  • Includes error handling with empty fallback
  • Returns correctly typed Observable

206-229: LGTM! Proper issues resolution endpoint.

The getProjectIssuesResolution(projectId: string) method correctly:

  • Requires projectId as a mandatory parameter
  • Passes projectId as query parameter
  • Provides comprehensive error handling with safe defaults

231-252: LGTM! Consistent PR velocity endpoint.

The getProjectPullRequestsWeekly(projectId: string) method follows the same pattern as issues resolution:

  • Requires mandatory projectId
  • Proper error handling with safe defaults
  • Correctly typed response
apps/lfx-one/src/server/controllers/analytics.controller.ts (4)

10-10: LGTM! Proper service integration.

ProjectService is correctly imported, declared, and instantiated following the existing pattern.

Also applies to: 20-20, 25-25


240-259: LGTM! Clean projects list endpoint handler.

The getProjects handler properly:

  • Fetches projects via ProjectService
  • Logs relevant metrics (project count)
  • Delegates errors to Express error handling middleware

261-293: LGTM! Robust issues resolution endpoint.

The getProjectIssuesResolution handler correctly:

  • Validates required projectId parameter with 400 response
  • Logs comprehensive metrics (total_days, total_opened, total_closed, resolution_rate, median_days_to_close)
  • Follows consistent error handling pattern

295-325: LGTM! Consistent PR velocity endpoint.

The getProjectPullRequestsWeekly handler follows the same robust pattern:

  • Validates required projectId with 400 response
  • Logs relevant metrics (total_weeks, total_merged_prs, avg_merge_time)
  • Consistent error handling
apps/lfx-one/src/server/services/project.service.ts (4)

27-27: LGTM! Proper Snowflake service integration.

SnowflakeService is correctly imported, declared, and instantiated following the existing service pattern.

Also applies to: 37-37, 44-44


555-577: LGTM! Efficient projects list query.

The getProjectsWithMaintainersList() method correctly:

  • Uses EXISTS subquery for efficient filtering of projects with maintainers
  • Orders results by name for consistent UI presentation
  • Transforms database column names to camelCase for API consistency

579-636: LGTM! Robust issues resolution data retrieval.

The getProjectIssuesResolution(projectId: string) method correctly:

  • Requires mandatory projectId parameter (per previous feedback)
  • Executes daily and aggregated queries in parallel for efficiency
  • Uses parameterized queries to prevent SQL injection
  • Provides safe defaults when aggregated data is absent

638-685: LGTM! Correct weighted average calculation.

The getProjectPullRequestsWeekly(projectId?: string) method properly:

  • Implements weighted average for merge time (weighted by PR count) as per previous review feedback
  • Falls back to first project when projectId not provided
  • Limits to 26 weeks (6 months) of historical data
  • Rounds calculated average to 1 decimal place

Note: The controller validates projectId, so the optional parameter here provides service-level flexibility.

apps/lfx-one/src/app/modules/dashboards/components/recent-progress/recent-progress.component.ts (8)

4-40: LGTM! Comprehensive imports for enhanced functionality.

The component correctly imports:

  • ProjectContextService for project-scoped data
  • Chart option constants for standardized configuration
  • parseLocalDateString utility for date handling
  • TooltipModule for enhanced tooltips (per previous feedback)
  • Proper Chart.js types for type safety

41-66: LGTM! Well-organized component state management.

The component properly:

  • Injects ProjectContextService for project context
  • Tracks loading state for all data sources
  • Derives projectId from context service
  • Initializes data streams through dedicated methods
  • Computes tooltip data separately for clean separation

77-209: LGTM! Standardized transformation methods.

The existing metric transformations now properly:

  • Use shared PROGRESS_*_CHART_OPTIONS constants for consistency
  • Apply parseLocalDateString for reliable date parsing
  • Use properly typed TooltipItem callbacks (addressing previous feedback)

211-290: LGTM! Comprehensive issues resolution transformation.

The transformProjectIssuesResolution method properly:

  • Reverses data for chronological left-to-right display
  • Uses database-calculated resolution rate (rounded to integer)
  • Builds structured HTML tooltip with opened/closed/median metrics
  • Configures dual-line chart with proper tooltip callbacks
  • Uses typed TooltipItem parameters

292-377: LGTM! Robust PR velocity transformation with error handling.

The transformProjectPullRequestsWeekly method correctly:

  • Reverses data for chronological display
  • Uses weighted average merge time from backend
  • Builds structured HTML tooltip with comprehensive metrics
  • Includes try-catch error handling in all tooltip callbacks (addressing previous feedback)
  • Provides footer with merged PR count and average reviewers

379-465: LGTM! Consistent data initialization with proper loading states.

The initialization methods correctly:

  • Use finalize to update loading state after data fetch completes
  • Include defensive checks for projectId in project-scoped methods
  • Return empty data when projectId is unavailable
  • Use switchMap for reactive project changes
  • Maintain consistent patterns across all data sources

467-506: LGTM! Efficient computed signal composition.

The computed signals properly:

  • Aggregate loading state from all data sources
  • Map base metrics to their live data transformations
  • Pass computed tooltip data to transformations
  • Maintain base metric as fallback for unmatched labels

508-544: LGTM! Clean tooltip data computation.

The tooltip data builders correctly:

  • Return null when no data available (safe for optional tooltip display)
  • Format numbers with locale-aware display
  • Calculate meaningful averages for PR velocity metrics
  • Round values appropriately for display

Comment thread apps/lfx-one/src/app/shared/services/persona.service.ts
Comment thread apps/lfx-one/src/app/shared/services/persona.service.ts Outdated
LFXV2-718

- Add SSR safety checks to PersonaService localStorage access
- Implement loadStoredPersona() method to restore persisted persona
- Fix constructor to load from localStorage instead of hardcoding 'maintainer'
- Ensure persona selection persists across page refreshes

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>
@asithade
asithade merged commit 2f38465 into main Nov 12, 2025
9 of 10 checks passed
@asithade
asithade deleted the feat/lfxv2-718-maitainer-dash-issue-resolution-pr-velocity-models branch November 12, 2025 20:06
@github-actions

Copy link
Copy Markdown

🧹 Deployment Removed

The deployment for PR #146 has been removed.

manishdixitlfx pushed a commit that referenced this pull request Mar 31, 2026
…eview & merge velocity, open vs closed issues) (#146)

* feat(dashboards): integrate project issues resolution metrics into recent progress component

- Add project filter to recent progress component for dynamic data fetching
- Implement new API endpoints for fetching project issues resolution data
- Enhance recent progress UI with connection status indicators and tooltips
- Update maintainer dashboard to include project selection for analytics
- Refactor analytics service and controller to support new project-related queries

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(analytics): add project pull requests weekly data integration

- Implement new API endpoint to fetch project pull requests weekly data from Snowflake
- Enhance RecentProgressComponent to display PR review and merge velocity metrics
- Add transformation logic for API response to chart format
- Update analytics service and controller to support new data retrieval
- Introduce new TypeScript interfaces for project pull requests weekly data

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(dashboards): enhance recent progress component with loading state and tooltips

- Implement loading state in the RecentProgressComponent to improve user experience during data fetching.
- Add tooltip functionality for specific metrics to provide additional context when hovering over items.
- Refactor data fetching logic to utilize signals for loading states and ensure proper handling of asynchronous data retrieval.
- Update HTML template to conditionally render loading indicators and tooltips based on data availability.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* fix(analytics): enforce required projectId for project issues resolution

- Update the analytics service and controller to require a projectId parameter for fetching project issues resolution data.
- Modify RecentProgressComponent to handle cases where projectId is not provided, ensuring a default response is returned.
- Refactor related documentation to reflect the change from optional to required projectId.

This change improves data integrity and user experience by preventing requests without a valid project identifier.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(dashboards): implement project context service for project selection in maintainer dashboard

- Introduced ProjectContextService to manage the selected project state and persist it in local storage.
- Updated MaintainerDashboardComponent to utilize the new service for project selection, ensuring the projectId is set based on user selection and restoring previously selected projects.
- Enhanced form handling to automatically select the first project if no project is currently selected.

This change improves user experience by maintaining project context across sessions and simplifying project selection in the dashboard.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* fix(dashboards): add error handling for project data fetching in maintainer dashboard

- Implemented error handling in the MaintainerDashboardComponent to manage failures when fetching project data from the analytics service.
- Utilized RxJS's catchError to log errors and return an empty projects array, ensuring the application remains stable during data retrieval issues.

This change enhances the robustness of the dashboard by preventing crashes due to data fetching errors.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* refactor(dashboards): improve project selection logic in maintainer dashboard

- Updated MaintainerDashboardComponent to validate the current project selection against loaded projects.
- Removed the logic for restoring a stored project ID and streamlined the auto-selection of the first project if no valid selection exists.
- Enhanced user experience by ensuring that only valid project selections are set in the form.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* refactor(dashboards): ssr-safe storage and pattern updates

LFXV2-718

- Add SSR safety checks to all localStorage access points in context services
- Change ProjectContextService to store full ProjectContext objects (projectId, name, slug)
- Simplify maintainer dashboard to match board member dashboard pattern
- Centralize project context in RecentProgressComponent using service injection
- Move ProjectContext interface to shared package for reusability
- Extract chart options to shared constants for consistency
- Refactor chart transformation logic for better maintainability

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* fix(dashboards): add ssr-safe persona persistence

LFXV2-718

- Add SSR safety checks to PersonaService localStorage access
- Implement loadStoredPersona() method to restore persisted persona
- Fix constructor to load from localStorage instead of hardcoding 'maintainer'
- Ensure persona selection persists across page refreshes

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>

---------

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
Signed-off-by: Asitha de Silva <asithade@gmail.com>
Co-authored-by: Asitha de Silva <asithade@gmail.com>
luismoriguerra pushed a commit that referenced this pull request Jun 5, 2026
…eview & merge velocity, open vs closed issues) (#146)

* feat(dashboards): integrate project issues resolution metrics into recent progress component

- Add project filter to recent progress component for dynamic data fetching
- Implement new API endpoints for fetching project issues resolution data
- Enhance recent progress UI with connection status indicators and tooltips
- Update maintainer dashboard to include project selection for analytics
- Refactor analytics service and controller to support new project-related queries

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(analytics): add project pull requests weekly data integration

- Implement new API endpoint to fetch project pull requests weekly data from Snowflake
- Enhance RecentProgressComponent to display PR review and merge velocity metrics
- Add transformation logic for API response to chart format
- Update analytics service and controller to support new data retrieval
- Introduce new TypeScript interfaces for project pull requests weekly data

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-710

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(dashboards): enhance recent progress component with loading state and tooltips

- Implement loading state in the RecentProgressComponent to improve user experience during data fetching.
- Add tooltip functionality for specific metrics to provide additional context when hovering over items.
- Refactor data fetching logic to utilize signals for loading states and ensure proper handling of asynchronous data retrieval.
- Update HTML template to conditionally render loading indicators and tooltips based on data availability.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* fix(analytics): enforce required projectId for project issues resolution

- Update the analytics service and controller to require a projectId parameter for fetching project issues resolution data.
- Modify RecentProgressComponent to handle cases where projectId is not provided, ensuring a default response is returned.
- Refactor related documentation to reflect the change from optional to required projectId.

This change improves data integrity and user experience by preventing requests without a valid project identifier.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* feat(dashboards): implement project context service for project selection in maintainer dashboard

- Introduced ProjectContextService to manage the selected project state and persist it in local storage.
- Updated MaintainerDashboardComponent to utilize the new service for project selection, ensuring the projectId is set based on user selection and restoring previously selected projects.
- Enhanced form handling to automatically select the first project if no project is currently selected.

This change improves user experience by maintaining project context across sessions and simplifying project selection in the dashboard.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* fix(dashboards): add error handling for project data fetching in maintainer dashboard

- Implemented error handling in the MaintainerDashboardComponent to manage failures when fetching project data from the analytics service.
- Utilized RxJS's catchError to log errors and return an empty projects array, ensuring the application remains stable during data retrieval issues.

This change enhances the robustness of the dashboard by preventing crashes due to data fetching errors.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* refactor(dashboards): improve project selection logic in maintainer dashboard

- Updated MaintainerDashboardComponent to validate the current project selection against loaded projects.
- Removed the logic for restoring a stored project ID and streamlined the auto-selection of the first project if no valid selection exists.
- Enhanced user experience by ensuring that only valid project selections are set in the form.

Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-718

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>

* refactor(dashboards): ssr-safe storage and pattern updates

LFXV2-718

- Add SSR safety checks to all localStorage access points in context services
- Change ProjectContextService to store full ProjectContext objects (projectId, name, slug)
- Simplify maintainer dashboard to match board member dashboard pattern
- Centralize project context in RecentProgressComponent using service injection
- Move ProjectContext interface to shared package for reusability
- Extract chart options to shared constants for consistency
- Refactor chart transformation logic for better maintainability

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>

* fix(dashboards): add ssr-safe persona persistence

LFXV2-718

- Add SSR safety checks to PersonaService localStorage access
- Implement loadStoredPersona() method to restore persisted persona
- Fix constructor to load from localStorage instead of hardcoding 'maintainer'
- Ensure persona selection persists across page refreshes

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Asitha de Silva <asithade@gmail.com>

---------

Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
Signed-off-by: Asitha de Silva <asithade@gmail.com>
Co-authored-by: Asitha de Silva <asithade@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants